Messages in this thread | | | Date | Tue, 30 Jun 2020 18:55:12 +0200 | From | Greg Kroah-Hartman <> | Subject | Re: [PATCH] SPI LPC information kernel module |
| |
On Tue, Jun 30, 2020 at 01:18:28PM -0300, Daniel Gutson wrote: > On Tue, Jun 30, 2020 at 12:28 PM Greg Kroah-Hartman < > gregkh@linuxfoundation.org> wrote: > > > On Tue, Jun 30, 2020 at 11:42:58AM -0300, Daniel Gutson wrote: > > > On Tue, Jun 30, 2020 at 5:56 AM Greg Kroah-Hartman < > > > gregkh@linuxfoundation.org> wrote: > > > > > > > On Mon, Jun 29, 2020 at 07:59:32PM -0300, Daniel Gutson wrote: > > > > > This kernel module exports configuration attributes for the > > > > > system SPI chip. > > > > > This initial version exports the BIOS Write Enable (bioswe), > > > > > BIOS Lock Enable (ble), and the SMM Bios Write Protect (SMM_BWP) > > > > > fields of the Bios Control register. The idea is to keep adding more > > > > > flags, not only from the BC but also from other registers in > > following > > > > > versions. > > > > > > > > > > The goal is that the attributes are avilable to fwupd when SecureBoot > > > > > is turned on. > > > > > > > > > > A technical note: I check if *ppos == BUFFER_SIZE in the reading > > function > > > > > to exit early and avoid an extra access to the HW, for example when > > using > > > > > the 'cat' command, which causes two read operations. > > > > > > > > Why not use the simple_* functions which should prevent that type of > > > > thing? > > > > > > > > > > a hint please? I don't see how to do it with simple_read_from_buffer, I > > > need to return in the read fop the amount of read bytes, but don't know > > > how to mark EOF. Because of that, 'cat' reads again just for me to tell > > it > > > there's nothing else to read. > > > > That's fine, the kernel does not tell userspace "EOF", that is up to the > > libc to determine. If you read the data from the hardware once, and > > keep it in your buffer, simple_read_from_buffer() will handle all of > > that logic for you, please let it do that. > > > > The only way I see to do this is to dynamically allocate the buffer in the > open fop, in order to avoid concurrency issues. > Is this correct?
Or use a lock, depends on what you want to do here.
But sysfs should handle all of this for you, when you switch to it.
> > > We debated this but didn't find a better match, since cpu/arch-specific > > > seemed too core to put informational drivers. > > > Do you have a suggestion? > > > > Make it explicitly hardware specific in your userspace location. > > > > What do you mean by "your userspace location"?
Where your files show up to userspace. sysfs is a hierarchy, don't put hardware-specific stuff at the "root" of it, otherwise that doesn't make any sense. Look at what is there today for examples of what to do.
good luck!
greg k-h
| |