All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Daniel Gutson <daniel@eclypsium.com>
Cc: Tudor Ambarus <tudor.ambarus@microchip.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Boris Brezillon <bbrezillon@kernel.org>,
	linux-mtd <linux-mtd@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Alex Bazhaniuk <alex@eclypsium.com>,
	Richard Hughes <hughsient@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH] mtd: spi-nor: intel-spi: Do not try to make the SPI flash chip writable
Date: Tue, 4 Aug 2020 21:06:34 +0200	[thread overview]
Message-ID: <CAK8P3a27bTYyn3N+tX=i_6f4KrQkOmkUA1zUQfvCW7qw6smSkQ@mail.gmail.com> (raw)
In-Reply-To: <CAFmMkTHEm8k+5GZkVJbDZMEhMwpsqVKRb-hGskSpBstdLRuFyA@mail.gmail.com>

On Tue, Aug 4, 2020 at 5:49 PM Daniel Gutson <daniel@eclypsium.com> wrote:
> On Tue, Aug 4, 2020 at 12:21 PM Arnd Bergmann <arnd@arndb.de> wrote:
>>
>> On Tue, Aug 4, 2020 at 3:58 PM Daniel Gutson
>> <daniel.gutson@eclypsium.com> wrote:
>> >
>> > Context, the intel-spi has a module argument that controls
>> > whether the driver attempts to turn the SPI flash chip writeable.
>> > The default value is FALSE (don't try to make it writeable).
>> > However, this flag applies only for a number of devices, coming from the
>> > platform driver, whereas the devices detected through the PCI driver
>> > (intel-spi-pci) are not subject to this check since the configuration
>> > takes place in intel-spi-pci which doesn't have an argument.
>>
>> This is still factually incorrect, as explained at least three times
>> now.
>>
>> Please either make the same change for both the Bay Trail
>> platform driver and the PCI driver, or explain why you want them to
>> be different rather than incorrectly claiming that you change them to
>> be the same.
>
>
> What about just saying
>
> "This patch removes the attempt by the intel-spi-pci driver to
> make the chip always writable."

Yes, that is much better, though it still sounds like it would at the
moment allow writing to the device from software without also
setting the module parameter. I would say something like

"Disallow overriding the write protection in the PCI driver
with a module parameter and instead honor the current
state of the write protection as set by the firmware."

(note also: imperative form in the patch description rather than
"this patch ...").

      Arnd

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: Daniel Gutson <daniel@eclypsium.com>
Cc: Richard Hughes <hughsient@gmail.com>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Boris Brezillon <bbrezillon@kernel.org>,
	Richard Weinberger <richard@nod.at>,
	Tudor Ambarus <tudor.ambarus@microchip.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-mtd <linux-mtd@lists.infradead.org>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Alex Bazhaniuk <alex@eclypsium.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>
Subject: Re: [PATCH] mtd: spi-nor: intel-spi: Do not try to make the SPI flash chip writable
Date: Tue, 4 Aug 2020 21:06:34 +0200	[thread overview]
Message-ID: <CAK8P3a27bTYyn3N+tX=i_6f4KrQkOmkUA1zUQfvCW7qw6smSkQ@mail.gmail.com> (raw)
In-Reply-To: <CAFmMkTHEm8k+5GZkVJbDZMEhMwpsqVKRb-hGskSpBstdLRuFyA@mail.gmail.com>

On Tue, Aug 4, 2020 at 5:49 PM Daniel Gutson <daniel@eclypsium.com> wrote:
> On Tue, Aug 4, 2020 at 12:21 PM Arnd Bergmann <arnd@arndb.de> wrote:
>>
>> On Tue, Aug 4, 2020 at 3:58 PM Daniel Gutson
>> <daniel.gutson@eclypsium.com> wrote:
>> >
>> > Context, the intel-spi has a module argument that controls
>> > whether the driver attempts to turn the SPI flash chip writeable.
>> > The default value is FALSE (don't try to make it writeable).
>> > However, this flag applies only for a number of devices, coming from the
>> > platform driver, whereas the devices detected through the PCI driver
>> > (intel-spi-pci) are not subject to this check since the configuration
>> > takes place in intel-spi-pci which doesn't have an argument.
>>
>> This is still factually incorrect, as explained at least three times
>> now.
>>
>> Please either make the same change for both the Bay Trail
>> platform driver and the PCI driver, or explain why you want them to
>> be different rather than incorrectly claiming that you change them to
>> be the same.
>
>
> What about just saying
>
> "This patch removes the attempt by the intel-spi-pci driver to
> make the chip always writable."

Yes, that is much better, though it still sounds like it would at the
moment allow writing to the device from software without also
setting the module parameter. I would say something like

"Disallow overriding the write protection in the PCI driver
with a module parameter and instead honor the current
state of the write protection as set by the firmware."

(note also: imperative form in the patch description rather than
"this patch ...").

      Arnd

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  parent reply	other threads:[~2020-08-04 19:06 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-04 13:58 [PATCH] mtd: spi-nor: intel-spi: Do not try to make the SPI flash chip writable Daniel Gutson
2020-08-04 13:58 ` Daniel Gutson
2020-08-04 14:08 ` Mika Westerberg
2020-08-04 14:08   ` Mika Westerberg
2020-08-04 15:21 ` Arnd Bergmann
2020-08-04 15:21   ` Arnd Bergmann
     [not found]   ` <CAFmMkTHEm8k+5GZkVJbDZMEhMwpsqVKRb-hGskSpBstdLRuFyA@mail.gmail.com>
2020-08-04 19:06     ` Arnd Bergmann [this message]
2020-08-04 19:06       ` Arnd Bergmann
2020-08-04 19:57       ` Daniel Gutson
2020-08-04 19:57         ` Daniel Gutson
2020-08-04 20:46         ` Arnd Bergmann
2020-08-04 20:46           ` Arnd Bergmann
2020-08-04 21:26           ` Daniel Gutson
2020-08-04 21:26             ` Daniel Gutson
2020-08-12 15:41             ` Daniel Gutson
2020-08-12 15:41               ` Daniel Gutson
2020-08-13  9:01               ` Greg Kroah-Hartman
2020-08-13  9:01                 ` Greg Kroah-Hartman
     [not found]                 ` <CAFmMkTFgjW+9gNfx=2SU7B0foww=SLiiyVi+P-hZpEFDbMTf2Q@mail.gmail.com>
2020-08-13 11:47                   ` Greg Kroah-Hartman
2020-08-13 11:47                     ` Greg Kroah-Hartman
2020-08-13 15:41             ` Arnd Bergmann
2020-08-13 15:41               ` Arnd Bergmann
2020-08-13 21:40               ` Daniel Gutson
2020-08-13 21:40                 ` Daniel Gutson
2020-08-16  8:41                 ` Arnd Bergmann
2020-08-16  8:41                   ` Arnd Bergmann
2020-08-18 15:55                   ` Daniel Gutson
2020-08-18 15:55                     ` Daniel Gutson
2020-08-19  6:57                     ` Mika Westerberg
2020-08-19  6:57                       ` Mika Westerberg
2020-08-19  8:38                       ` Arnd Bergmann
2020-08-19  8:38                         ` Arnd Bergmann
2020-08-19  9:11                         ` Mika Westerberg
2020-08-19  9:11                           ` Mika Westerberg
2020-08-22 16:06                           ` Arnd Bergmann
2020-08-22 16:06                             ` Arnd Bergmann
2020-08-24  8:22                             ` Mika Westerberg
2020-08-24  8:22                               ` Mika Westerberg
2020-08-24  9:08                               ` Arnd Bergmann
2020-08-24  9:08                                 ` Arnd Bergmann
2020-08-24  9:15                                 ` Mika Westerberg
2020-08-24  9:15                                   ` Mika Westerberg
2020-08-24  9:31                                   ` Arnd Bergmann
2020-08-24  9:31                                     ` Arnd Bergmann
2020-08-24  9:44                                     ` Mika Westerberg
2020-08-24  9:44                                       ` Mika Westerberg
2020-08-24 16:00                                       ` Daniel Gutson
2020-08-24 16:00                                         ` Daniel Gutson
2020-08-19  9:19                         ` David Laight
2020-08-19  9:19                           ` David Laight

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAK8P3a27bTYyn3N+tX=i_6f4KrQkOmkUA1zUQfvCW7qw6smSkQ@mail.gmail.com' \
    --to=arnd@arndb.de \
    --cc=alex@eclypsium.com \
    --cc=bbrezillon@kernel.org \
    --cc=daniel@eclypsium.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hughsient@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=miquel.raynal@bootlin.com \
    --cc=richard@nod.at \
    --cc=tudor.ambarus@microchip.com \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.