Message-ID: <2023071724-twiddling-morale-157e@gregkh>
Date:   Mon, 17 Jul 2023 18:59:52 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Miquel Raynal <miquel.raynal@...tlin.com>
Cc:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        linux-kernel@...r.kernel.org,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Robert Marko <robert.marko@...tura.hr>,
        Luka Perkov <luka.perkov@...tura.hr>,
        Michael Walle <michael@...le.cc>,
        Randy Dunlap <rdunlap@...radead.org>
Subject: Re: [PATCH v6 3/3] nvmem: core: Expose cells through sysfs

On Mon, Jul 17, 2023 at 06:33:23PM +0200, Miquel Raynal wrote:
> Hi Greg,
> 
> gregkh@...uxfoundation.org wrote on Mon, 17 Jul 2023 16:32:09 +0200:
> 
> > On Mon, Jul 17, 2023 at 09:51:47AM +0200, 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 its position/size in the underlying
> > > device. Unfortunately, these information are not accessible by users,
> > > unless by fully re-implementing the parser logic in userland.
> > > 
> > > Let's expose the cells and their content through sysfs to avoid these
> > > situations. Of course the relevant NVMEM sysfs Kconfig option must be
> > > enabled for this support to be available.
> > > 
> > > Not all nvmem devices expose cells. Indeed, the .bin_attrs attribute
> > > group member will be filled at runtime only when relevant and will
> > > remain empty otherwise. In this case, as the cells attribute group will
> > > be empty, it will not lead to any additional folder/file creation.
> > > 
> > > Exposed cells are read-only. There is, in practice, everything in the
> > > core to support a write path, but as I don't see any need for that, I
> > > prefer to keep the interface simple (and probably safer). The interface
> > > is documented as being in the "testing" state which means we can later
> > > add a write attribute if though relevant.
> > > 
> > > There is one limitation though: if a layout is built as a module but is
> > > not properly installed in the system and loaded manually with insmod
> > > while the nvmem device driver was built-in, the cells won't appear in
> > > sysfs. But if done like that, the cells won't be usable by the built-in
> > > kernel drivers anyway.  
> > 
> > Wait, what?  That should not be an issue here, if so, then this change
> > is not correct and should be fixed as this is NOT an issue for sysfs
> > (otherwise the whole tree wouldn't work.)
> > 
> > Please fix up your dependancies if this is somehow not working properly.
> 
> I'm not sure I fully get your point.
> 
> There is no way we can describe any dependency between a storage device
> driver and an nvmem layout. NVMEM is a pure software abstraction, the
> layout that will be chosen depends on the device tree, but if the
> layout has not been installed, there is no existing mechanism in
> the kernel to prevent it from being loaded (how do you know it's
> not on purpose?).

Once a layout has been loaded, the sysfs files should show up, right?
Otherwise what does a "layout" do?  (hint, I have no idea, it's an odd
term to me...)

thanks,

greg k-h

Powered by blists - more mailing lists