Messages in this thread | | | From | Nicholas Johnson <> | Subject | Re: [PATCH 5/5] nvmem: Add support for write-only instances | Date | Tue, 24 Mar 2020 15:59:58 +0000 |
| |
On Tue, Mar 24, 2020 at 04:18:31PM +0100, Greg KH wrote: > On Tue, Mar 24, 2020 at 02:24:21PM +0000, Nicholas Johnson wrote: > > On Tue, Mar 24, 2020 at 01:25:46PM +0000, Srinivas Kandagatla wrote: > > > > > > > > > On 24/03/2020 12:29, Greg KH wrote: > > > > > But the Idea here is : > > > > > We ended up with providing different options like read-only,root-only to > > > > > nvmem providers combined with read/write callbacks. > > > > > With that, there are some cases which are totally invalid, existing code > > > > > does very minimal check to ensure that before populating with correct > > > > > attributes to sysfs file. One of such case is with thunderbolt provider > > > > > which supports only write callback. > > > > > > > > > > With this new checks in place these flags and callbacks are correctly > > > > > validated, would result in correct file attributes. > > > > Why this crazy set of different groups? You can set the mode of a sysfs > > > > file in the callback for when the file is about to be created, that's so > > > > much simpler and is what it is for. This feels really hacky and almost > > > > impossible to follow:( > > > Thanks for the inputs, That definitely sounds much simpler to deal with. > > > > > > Am guessing you are referring to is_bin_visible callback? > > > > > > I will try to clean this up! > > I am still onboard and willing do the work, but we may need to discuss > > to be on the same page with new plans. How do you wish to do this? > > > > Does this new approach still allow us to abort if we receive an invalid > > configuration? Or do we still need to have something in nvmem_register() > > to abort in invalid case? > > > > The documentation of is_bin_visible says only read/write permissions are > > accepted. Does this mean that it will not take read-only or write-only? > > That is one way of interpreting it. > > That's a funny way of interpreting it :) Now that I look back, yes.
> > Please be sane, you pass back the permissions of the file, look at all > of the places in the kernel is it used for examples... It's more inexperience and sleep deprivation than insanity. I am working on those. :)
There is only one use of is_bin_visible but a lot for is_visible, so I will go off those.
Regards, Nicholas
> > thanks, > > greg k-h
| |