Re: [PATCH v6 1/3] ABI: sysfs-nvmem-cells: Expose cells through sysfs

[Miquel Raynal]

Hello,

srinivas.kandagatla@xxxxxxxxxx wrote on Tue, 1 Aug 2023 10:06:14 +0100:

> On 31/07/2023 16:51, Miquel Raynal wrote:
> > Hi John,
> > 
> > Srinivas, a question for you below.
> > 
> > lists@xxxxxxxxxxxxxxxxxxxxxxxxxxx wrote on Sun, 23 Jul 2023 19:39:50
> > +0000:
> >   
> >> Hi Miquel,
> >>
> >> On Mon, 17 Jul 2023, at 07:51, Miquel Raynal wrote:  
> >>> The binary content of nvmem devices is available to the user so in the
> >>> easiest cases, finding the content of a cell is rather easy as it is
> >>> just a matter of looking at a known and fixed offset. However, nvmem
> >>> layouts have been recently introduced to cope with more advanced
> >>> situations, where the offset and size of the cells is not known in
> >>> advance or is dynamic. When using layouts, more advanced parsers are
> >>> used by the kernel in order to give direct access to the content of each
> >>> cell regardless of their position/size in the underlying device, but
> >>> these information were not accessible to the user.
> >>>
> >>> By exposing the nvmem cells to the user through a dedicated cell/ folder
> >>> containing one file per cell, we provide a straightforward access to
> >>> useful user information without the need for re-writing a userland
> >>> parser. Content of nvmem cells is usually: product names, manufacturing
> >>> date, MAC addresses, etc,
> >>>
> >>> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> >>> Reviewed-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> >>> ---
> >>>   Documentation/ABI/testing/sysfs-nvmem-cells | 19 +++++++++++++++++++
> >>>   1 file changed, 19 insertions(+)
> >>>   create mode 100644 Documentation/ABI/testing/sysfs-nvmem-cells
> >>>
> >>> diff --git a/Documentation/ABI/testing/sysfs-nvmem-cells
> >>> b/Documentation/ABI/testing/sysfs-nvmem-cells
> >>> new file mode 100644
> >>> index 000000000000..b2d15a8d36e5
> >>> --- /dev/null
> >>> +++ b/Documentation/ABI/testing/sysfs-nvmem-cells
> >>> @@ -0,0 +1,19 @@
> >>> +What:		/sys/bus/nvmem/devices/.../cells/<cell-name>
> >>> +Date:		May 2023
> >>> +KernelVersion:	6.5
> >>> +Contact:	Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> >>> +Description:
> >>> +		The "cells" folder contains one file per cell exposed by
> >>> +		the nvmem device. The name of the file is the cell name.  
> >>
> >> Could we consider using a file within a folder (name defined by cell propertys) to access the cell bytes?
> >> Example (pick the best path and filename):
> >> /sys/bus/nvmem/devices/.../cells/<cell-name>/bytes
> >>
> >> That way, it is much easier to expand this at a later stage,
> >> like adding an of_node link at
> >> /sys/bus/nvmem/devices/.../cells/<cell-name>/of_node
> >> or exposing other nvmem cell properties.  
> > 
> > I have no strong opinion. Srinivas what do you prefer? I'm fine either
> > ways. I like the simplicity of the current approach more, but it's true
> > that it is more easy to make it grow if we follow John idea.  
> 
> Sounds sensible to me.

I've looked a bit more in depth how to do that and to be honest I did
not find an easy way. Attributes and attribute groups are meant to be
used with only one indirection level and making an additional one seems
terribly more complex. Maybe I'm wrong, if you have a piece of code
doing that please share it and I'll make my best to integrate it,
otherwise I think I'll keep the simplest approach.

> >> This is particularly relevant given the cell-name alone does not always
> >> uniquely represent a cell on an nvmem device.
> >> https://lore.kernel.org/lkml/ZLaZ7fzUSsa0Igx1@xxxxxxxxxxxxxx/  
> > 
> > It seems like this is gonna be fixed by suffixing @<offset> to the
> > name, as anyway whatever solution we choose, it is gonna be needed.  
> 
> we have to be careful here not to break the nvmem_cell_get() users.

I believe this only applies to sysfs names, so nvmem_cell_get() which
uses real cells names should not be affected.

Thanks,
Miquèl