Re: [PATCH] Module argument to control whether intel-spi-pci attempts to turn the SPI flash chip writeable

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Wed, Jul 29, 2020 at 05:54:35PM -0300, Daniel Gutson wrote:
> On Mon, Jul 27, 2020 at 12:31 PM Daniel Gutson <daniel@eclypsium.com> wrote:
> >
> > On Mon, Jul 27, 2020 at 12:15 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > >
> > > On Mon, Jul 27, 2020 at 5:05 PM Daniel Gutson <daniel@eclypsium.com> wrote:
> > > > On Sun, Jul 26, 2020 at 4:17 AM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > > >>
> > > >> On Sat, Jul 25, 2020 at 02:20:03PM -0300, Daniel Gutson wrote:
> > > >> > El sáb., 25 jul. 2020 2:56 a. m., Greg Kroah-Hartman <
> > > >> > gregkh@linuxfoundation.org> escribió:
> > > >> >
> > > >> >
> > > >> > 1) I just did the same that intel-spi.c does.
> > > >>
> > > >> No need to copy bad examples :)
> > > >
> > > >
> > > > Didn't know it was a bad example. What's is the current modern mechanism that replaces initialization-time configuration?
> > >
> > > I'd say you'd generally want this to be a per-instance setting, which
> > > could be a sysfs attribute of the physical device, or an ioctl for an
> > > existing user space abstraction.
> >
> > But still, they are not equivalent. The initial configuration remains
> > constant for the rest of the life of the driver, whereas the
> > sysfs attribute is set after the initialization and can be re-set over
> > time. I'm not asking module parameters "to come back" if
> > they are now considered obsolete, I'm just trying to understand.
> >
> > >
> > > In the changelog, you should also explain what this is used for. Do
> > > you actually want to write to a device that is marked read-only, or
> > > are you just trying to make the interface more consistent between the
> > > two drivers?
> >
> > The device can either be locked or unlocked. If it is unlocked, it can
> > be set writable depending on the module
> > argument in intel-spi, or straight writable in intel-spi-pci. Which is
> > dangerous.
> > I wanted to make, as you say, the interface consistent.
> > Exposing an interface to the user (via a sysfs attribute) to try to
> > make the driver writable is definitively a bad idea.
> > I'd rather do nothing (no module arg) rather than open that
> > here-be-dragons door.
> 
> ping.
> This is a real existing problem that I'm trying to address.
> There's currently no way to prevent the kernel to try to turn
> the SPI flash chip writable for the device IDs handled by
> intel-spi-pci. And allowing userspace to switch it between
> writable/non-writable is not an option.
> What other mechanism can I use other than the module parameter,

Again, module parameters are working on a per-chunk-of-code basis, while
you want to work on a per-device basis, which is why you should not use
a module parameter.

good luck!

greg k-h