RE: [PATCH v4 06/17] remoteproc: introduce rproc_add_carveout function
> -Original Message-
> From: Suman Anna
> Sent: mardi 23 octobre 2018 19:05
> To: Loic PALLARDY ; bjorn.anders...@linaro.org;
> o...@wizery.com
> Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org;
> Arnaud POULIQUEN ;
> benjamin.gaign...@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
> > Acked-by: Bjorn Andersson
>
> 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(>node, >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(>node, >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,
> >
RE: [PATCH v4 06/17] remoteproc: introduce rproc_add_carveout function
> -Original Message-
> From: Suman Anna
> Sent: mardi 23 octobre 2018 19:05
> To: Loic PALLARDY ; bjorn.anders...@linaro.org;
> o...@wizery.com
> Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org;
> Arnaud POULIQUEN ;
> benjamin.gaign...@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
> > Acked-by: Bjorn Andersson
>
> 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(>node, >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(>node, >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,
> >
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
> Acked-by: Bjorn Andersson
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(>node, >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(>node, >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,
>
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
> Acked-by: Bjorn Andersson
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(>node, >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(>node, >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,
>
