diff mbox series

[v4,06/17] remoteproc: introduce rproc_add_carveout function

Message ID 1532697292-14272-7-git-send-email-loic.pallardy@st.com
State New
Headers show
Series remoteproc: add fixed memory region support | expand

Commit Message

Loic Pallardy July 27, 2018, 1:14 p.m. UTC
This patch introduces a new API to allow platform driver to register
platform specific carveout regions.

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>

Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>

---
 drivers/remoteproc/remoteproc_core.c | 16 +++++++++++++++-
 include/linux/remoteproc.h           |  2 ++
 2 files changed, 17 insertions(+), 1 deletion(-)

-- 
1.9.1

Comments

Suman Anna Oct. 23, 2018, 5:05 p.m. UTC | #1
Hi Bjorn, Loic,

On 7/27/18 8:14 AM, Loic Pallardy wrote:
> This patch introduces a new API to allow platform driver to register

> platform specific carveout regions.

> 

> Signed-off-by: Loic Pallardy <loic.pallardy@st.com>

> Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>


Hmm, I do not prefer that this function be exported. It adds no value,
and this adds an asymmetric API for remoteproc drivers to add a mem
entry to the internal remoteproc lists, while relying on the remoteproc
core to remove them. The carveout list maintenance is a remoteproc
internal scheme, and so should be maintained that way. Further more, on
the current series, all the existing calls are immediately after a
rproc_of_resm_mem_entry_init() or a rproc_mem_entry_init() function,
which means this could very well be folded into those individual functions.

regards
Suman

> ---

>  drivers/remoteproc/remoteproc_core.c | 16 +++++++++++++++-

>  include/linux/remoteproc.h           |  2 ++

>  2 files changed, 17 insertions(+), 1 deletion(-)

> 

> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c

> index b76760e..fe6c4e4 100644

> --- a/drivers/remoteproc/remoteproc_core.c

> +++ b/drivers/remoteproc/remoteproc_core.c

> @@ -747,7 +747,7 @@ static int rproc_handle_carveout(struct rproc *rproc,

>  	if (!carveout)

>  		goto free_carv;

>  

> -	list_add_tail(&carveout->node, &rproc->carveouts);

> +	rproc_add_carveout(rproc, carveout);

>  

>  	return 0;

>  

> @@ -761,6 +761,20 @@ static int rproc_handle_carveout(struct rproc *rproc,

>  }

>  

>  /**

> + * rproc_add_carveout() - register an allocated carveout region

> + * @rproc: rproc handle

> + * @mem: memory entry to register

> + *

> + * This function registers specified memory entry in @rproc carveouts list.

> + * Specified carveout should have been allocated before registering.

> + */

> +void rproc_add_carveout(struct rproc *rproc, struct rproc_mem_entry *mem)

> +{

> +	list_add_tail(&mem->node, &rproc->carveouts);

> +}

> +EXPORT_SYMBOL(rproc_add_carveout);

> +

> +/**

>   * rproc_mem_entry_init() - allocate and initialize rproc_mem_entry struct

>   * @dev: pointer on device struct

>   * @va: virtual address

> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h

> index 4bc961f..55f30fc 100644

> --- a/include/linux/remoteproc.h

> +++ b/include/linux/remoteproc.h

> @@ -558,6 +558,8 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,

>  int rproc_del(struct rproc *rproc);

>  void rproc_free(struct rproc *rproc);

>  

> +void rproc_add_carveout(struct rproc *rproc, struct rproc_mem_entry *mem);

> +

>  struct rproc_mem_entry *

>  rproc_mem_entry_init(struct device *dev,

>  		     void *va, dma_addr_t dma, int len, u32 da,

>
Loic Pallardy Oct. 23, 2018, 7:48 p.m. UTC | #2
> -----Original Message-----

> From: Suman Anna <s-anna@ti.com>

> Sent: mardi 23 octobre 2018 19:05

> To: Loic PALLARDY <loic.pallardy@st.com>; bjorn.andersson@linaro.org;

> ohad@wizery.com

> Cc: linux-remoteproc@vger.kernel.org; linux-kernel@vger.kernel.org;

> Arnaud POULIQUEN <arnaud.pouliquen@st.com>;

> benjamin.gaignard@linaro.org

> Subject: Re: [PATCH v4 06/17] remoteproc: introduce rproc_add_carveout

> function

> 

> Hi Bjorn, Loic,

> 

> On 7/27/18 8:14 AM, Loic Pallardy wrote:

> > This patch introduces a new API to allow platform driver to register

> > platform specific carveout regions.

> >

> > Signed-off-by: Loic Pallardy <loic.pallardy@st.com>

> > Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> 

> Hmm, I do not prefer that this function be exported. It adds no value,

> and this adds an asymmetric API for remoteproc drivers to add a mem

> entry to the internal remoteproc lists, while relying on the remoteproc

> core to remove them. The carveout list maintenance is a remoteproc

> internal scheme, and so should be maintained that way. Further more, on

> the current series, all the existing calls are immediately after a

> rproc_of_resm_mem_entry_init() or a rproc_mem_entry_init() function,

> which means this could very well be folded into those individual functions.

> 

I think it has been introduced in first series. Look that rproc_add_carveout() call could be integrated in helper functions now.
I can propose a patch to correct this.

Regards,
Loic
> regards

> Suman

> 

> > ---

> >  drivers/remoteproc/remoteproc_core.c | 16 +++++++++++++++-

> >  include/linux/remoteproc.h           |  2 ++

> >  2 files changed, 17 insertions(+), 1 deletion(-)

> >

> > diff --git a/drivers/remoteproc/remoteproc_core.c

> b/drivers/remoteproc/remoteproc_core.c

> > index b76760e..fe6c4e4 100644

> > --- a/drivers/remoteproc/remoteproc_core.c

> > +++ b/drivers/remoteproc/remoteproc_core.c

> > @@ -747,7 +747,7 @@ static int rproc_handle_carveout(struct rproc

> *rproc,

> >  	if (!carveout)

> >  		goto free_carv;

> >

> > -	list_add_tail(&carveout->node, &rproc->carveouts);

> > +	rproc_add_carveout(rproc, carveout);

> >

> >  	return 0;

> >

> > @@ -761,6 +761,20 @@ static int rproc_handle_carveout(struct rproc

> *rproc,

> >  }

> >

> >  /**

> > + * rproc_add_carveout() - register an allocated carveout region

> > + * @rproc: rproc handle

> > + * @mem: memory entry to register

> > + *

> > + * This function registers specified memory entry in @rproc carveouts list.

> > + * Specified carveout should have been allocated before registering.

> > + */

> > +void rproc_add_carveout(struct rproc *rproc, struct rproc_mem_entry

> *mem)

> > +{

> > +	list_add_tail(&mem->node, &rproc->carveouts);

> > +}

> > +EXPORT_SYMBOL(rproc_add_carveout);

> > +

> > +/**

> >   * rproc_mem_entry_init() - allocate and initialize rproc_mem_entry struct

> >   * @dev: pointer on device struct

> >   * @va: virtual address

> > diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h

> > index 4bc961f..55f30fc 100644

> > --- a/include/linux/remoteproc.h

> > +++ b/include/linux/remoteproc.h

> > @@ -558,6 +558,8 @@ struct rproc *rproc_alloc(struct device *dev, const

> char *name,

> >  int rproc_del(struct rproc *rproc);

> >  void rproc_free(struct rproc *rproc);

> >

> > +void rproc_add_carveout(struct rproc *rproc, struct rproc_mem_entry

> *mem);

> > +

> >  struct rproc_mem_entry *

> >  rproc_mem_entry_init(struct device *dev,

> >  		     void *va, dma_addr_t dma, int len, u32 da,

> >
diff mbox series

Patch

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index b76760e..fe6c4e4 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -747,7 +747,7 @@  static int rproc_handle_carveout(struct rproc *rproc,
 	if (!carveout)
 		goto free_carv;
 
-	list_add_tail(&carveout->node, &rproc->carveouts);
+	rproc_add_carveout(rproc, carveout);
 
 	return 0;
 
@@ -761,6 +761,20 @@  static int rproc_handle_carveout(struct rproc *rproc,
 }
 
 /**
+ * rproc_add_carveout() - register an allocated carveout region
+ * @rproc: rproc handle
+ * @mem: memory entry to register
+ *
+ * This function registers specified memory entry in @rproc carveouts list.
+ * Specified carveout should have been allocated before registering.
+ */
+void rproc_add_carveout(struct rproc *rproc, struct rproc_mem_entry *mem)
+{
+	list_add_tail(&mem->node, &rproc->carveouts);
+}
+EXPORT_SYMBOL(rproc_add_carveout);
+
+/**
  * rproc_mem_entry_init() - allocate and initialize rproc_mem_entry struct
  * @dev: pointer on device struct
  * @va: virtual address
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 4bc961f..55f30fc 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -558,6 +558,8 @@  struct rproc *rproc_alloc(struct device *dev, const char *name,
 int rproc_del(struct rproc *rproc);
 void rproc_free(struct rproc *rproc);
 
+void rproc_add_carveout(struct rproc *rproc, struct rproc_mem_entry *mem);
+
 struct rproc_mem_entry *
 rproc_mem_entry_init(struct device *dev,
 		     void *va, dma_addr_t dma, int len, u32 da,