Re: [PATCH v0] nvmem: core: Export nvmem cell info to userspace

[Gaurav Kohli]

On 3/25/2019 4:28 PM, Srinivas Kandagatla wrote:
> On 24/03/2019 15:25, Gaurav Kohli wrote:
> > On 3/22/2019 8:53 PM, Srinivas Kandagatla wrote:
> > > On 20/03/2019 05:53, Gaurav Kohli wrote:
> > > > From: Shiraz Hashim <shashim@xxxxxxxxxxxxxx>
> > > >
> > > > Existing nvmem framework export full register space
> > > > as nvmem binary, but not exporting child node of nvmem
> > > > which is nvmem cell. Kernel can read the specific cell
> > > > by using nvmem_cell_read but userspace don't have such
> > > > provision.
> > > >
> > > > Add framework to export nvmem cell as well, So
> > > > userspace can use it directly.
> > > >
> > > > Signed-off-by: Shiraz Hashim <shashim@xxxxxxxxxxxxxx>
> > > > Signed-off-by: Gaurav Kohli <gkohli@xxxxxxxxxxxxxx>
> > > > Co-developed-by: Gaurav Kohli <gkohli@xxxxxxxxxxxxxx>
> > > >
> > > > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > >
> > > Thankyou for the patch.
> > >
> > > Why do you need such provision when the userspace can just get the 
> > > cell values using correct offset and size.
> > > This will also bring over head of managing entries dynamically + 
> > > confusing userspace abi.
> > >
> > > Unless you have a valid reason or usecase I don't see the need for 
> > > this.
> >
> >
> > Hi Srinivas,
> >
> >
> > This is mainly for user space convenience, In existing implementation 
> > they have to do manipulation according
>
> > to offset and bit, And with present patch, they just have to do cat 
> > for cell name and which can also be easily maintainable
> Yes, that is expected I guess!
>
> > for different soc. But with current, it is difficult to maintain 
> > users space code as each time we have to change user space code 
> > according to bit.
>
> Which user space code/application are you referring to here? Are these 
> open source?

Hi Srini,

This is not open source, we have a requirement to read certain bits of 
nvmem.

> > This would also help to expose certain bit only as per the bit 
> > parameter mentioned in dt node, which would also help to protect 
> > exposing of
> NVMEM is not just limited for DT users, non dt users use this f/w too.
> So the problem is not as simple as it sounds.
>
> If your issue is just about DT, you could easily parse active device 
> tree via /proc/device-tree and get cell offset, length and names from 
> it, use this information to read from nvmem.
>
> There are other concerns about the userspace ABI w.r.t udev events.
> udev events would race with the creation on this cell entries 
> resulting in a behavior where user-space applications would not see 
> the entries after udev events.
>
> In worst case if we decide to go with adding cells to nvmem then we 
> should do it before the device is even probed using group attributes.
> And this would mean that we can not support cells that are dynamically 
> defined. And there might be some memory freeing issues in this method 
> too!
yes i agree they are dynamic entries as well of nvmem cell, Can you 
please suggest some other way ?
> --srini
>
>
> > other bits to user space.
> >
> > > thanks,
> > > srini
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.