Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 17 Jul 2023 18:59:52 +0200
From:   Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To:     Miquel Raynal <miquel.raynal@bootlin.com>
Cc:     Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
        linux-kernel@vger.kernel.org,
        Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
        Robert Marko <robert.marko@sartura.hr>,
        Luka Perkov <luka.perkov@sartura.hr>,
        Michael Walle <michael@walle.cc>,
        Randy Dunlap <rdunlap@infradead.org>
Subject: Re: [PATCH v6 3/3] nvmem: core: Expose cells through sysfs
Message-ID: <2023071724-twiddling-morale-157e@gregkh>
References: <20230717075147.43326-1-miquel.raynal@bootlin.com>
 <20230717075147.43326-4-miquel.raynal@bootlin.com>
 <2023071717-channel-supernova-4cc9@gregkh>
 <20230717183323.49a55ad0@xps-13>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230717183323.49a55ad0@xps-13>
Precedence: bulk

On Mon, Jul 17, 2023 at 06:33:23PM +0200, Miquel Raynal wrote:
> Hi Greg,
> 
> gregkh@linuxfoundation.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