[1/2] nvmem: core: clear sysfs attributes for each NVMEM device

  Set nvmem_cells_group.bin_attrs to NULL in case of an NVMEM device not
having any cells in order to make sure sysfs attributes of a previously
registered NVMEM device are not accidentally reused for a follow-up
device which doesn't have any cells.

Fixes: 757f8b3835c9 ("nvmem: core: Expose cells through sysfs")
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/nvmem/core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

On Tue, Jul 18, 2023 at 02:55:31PM +0100, Daniel Golle wrote:
> Set nvmem_cells_group.bin_attrs to NULL in case of an NVMEM device not
> having any cells in order to make sure sysfs attributes of a previously
> registered NVMEM device are not accidentally reused for a follow-up
> device which doesn't have any cells.
> 
> Fixes: 757f8b3835c9 ("nvmem: core: Expose cells through sysfs")

Where is this git commit id?  I don't see it in Linus's tree.

> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  drivers/nvmem/core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Your patches were not threaded/attached to each other either, so our
tools can't take them together.  Can you fix that up and use 'git
send-email' or other such tools to have this show up properly when you
send a v2?

thanks,

greg k-h
  

On Tue, Jul 18, 2023 at 02:55:31PM +0100, Daniel Golle wrote:
> Set nvmem_cells_group.bin_attrs to NULL in case of an NVMEM device not
> having any cells in order to make sure sysfs attributes of a previously
> registered NVMEM device are not accidentally reused for a follow-up
> device which doesn't have any cells.

Wait, attributes and devices should NEVER be reused, how is that
happening here?

And just setting the attribute field to NULL doesn't free or clean up
anything, right?  Did memory just leak with this?

confused,

greg k-h
  

HI Daniel,

On 18/07/2023 14:55, Daniel Golle wrote:
> Set nvmem_cells_group.bin_attrs to NULL in case of an NVMEM device not
> having any cells in order to make sure sysfs attributes of a previously
> registered NVMEM device are not accidentally reused for a follow-up
> device which doesn't have any cells.
> 
> Fixes: 757f8b3835c9 ("nvmem: core: Expose cells through sysfs")

These patches are dropped out of nvmem next branch as it was breaking 
some existing users.


--srini
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>   drivers/nvmem/core.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 6c04a9cf6919f..70e951088826d 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -458,9 +458,10 @@ static int nvmem_populate_sysfs_cells(struct nvmem_device *nvmem)
>   
>   	mutex_lock(&nvmem_mutex);
>   
> -	if (list_empty(&nvmem->cells))
> +	if (list_empty(&nvmem->cells)) {
> +		nvmem_cells_group.bin_attrs = NULL;
>   		goto unlock_mutex;
> -
> +	}
>   	/* Allocate an array of attributes with a sentinel */
>   	ncells = list_count_nodes(&nvmem->cells);
>   	cells_attrs = devm_kcalloc(&nvmem->dev, ncells + 1,
  

On Tue, Jul 18, 2023 at 03:55:56PM +0100, Srinivas Kandagatla wrote:
> HI Daniel,
> 
> On 18/07/2023 14:55, Daniel Golle wrote:
> > Set nvmem_cells_group.bin_attrs to NULL in case of an NVMEM device not
> > having any cells in order to make sure sysfs attributes of a previously
> > registered NVMEM device are not accidentally reused for a follow-up
> > device which doesn't have any cells.
> > 
> > Fixes: 757f8b3835c9 ("nvmem: core: Expose cells through sysfs")
> 
> These patches are dropped out of nvmem next branch as it was breaking some
> existing users.

Ok. I've encountered those commits in linux-next and can confirm that
they were definitely also breaking things here, hence my patches at
least partially fixing that.

I agree that reverting them for now and reworking them seems to be the
better option in this case, hence my patches won't be needed as such.

> 
> 
> --srini
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> >   drivers/nvmem/core.c | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > index 6c04a9cf6919f..70e951088826d 100644
> > --- a/drivers/nvmem/core.c
> > +++ b/drivers/nvmem/core.c
> > @@ -458,9 +458,10 @@ static int nvmem_populate_sysfs_cells(struct nvmem_device *nvmem)
> >   	mutex_lock(&nvmem_mutex);
> > -	if (list_empty(&nvmem->cells))
> > +	if (list_empty(&nvmem->cells)) {
> > +		nvmem_cells_group.bin_attrs = NULL;
> >   		goto unlock_mutex;
> > -
> > +	}
> >   	/* Allocate an array of attributes with a sentinel */
> >   	ncells = list_count_nodes(&nvmem->cells);
> >   	cells_attrs = devm_kcalloc(&nvmem->dev, ncells + 1,
  

Hi Daniel,

daniel@makrotopia.org wrote on Tue, 18 Jul 2023 16:29:07 +0100:

> On Tue, Jul 18, 2023 at 03:55:56PM +0100, Srinivas Kandagatla wrote:
> > HI Daniel,
> > 
> > On 18/07/2023 14:55, Daniel Golle wrote:  
> > > Set nvmem_cells_group.bin_attrs to NULL in case of an NVMEM device not
> > > having any cells in order to make sure sysfs attributes of a previously
> > > registered NVMEM device are not accidentally reused for a follow-up
> > > device which doesn't have any cells.
> > > 
> > > Fixes: 757f8b3835c9 ("nvmem: core: Expose cells through sysfs")  
> > 
> > These patches are dropped out of nvmem next branch as it was breaking some
> > existing users.  
> 
> Ok. I've encountered those commits in linux-next and can confirm that
> they were definitely also breaking things here, hence my patches at
> least partially fixing that.
> 
> I agree that reverting them for now and reworking them seems to be the
> better option in this case, hence my patches won't be needed as such.

Quite the opposite, on my setup I don't have any breakage but as you
already fixed the cell name issue (also reported by Chen-yu), if you
agree, I will include these fixes (or an improved version) in my next
proposal.

Thanks,
Miquèl