All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel Walker (danielwa)" <danielwa@cisco.com>
To: "Jinhua Wu (jinhwu)" <jinhwu@cisco.com>
Cc: Vignesh Raghavendra <vigneshr@ti.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"xe-linux-external(mailer list)" <xe-linux-external@cisco.com>,
	Tudor Ambarus <tudor.ambarus@microchip.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Mika Westerberg <mika.westerberg@linux.intel.com>
Subject: Re: [PATCH 2/2] mtd: spi-nor: intel-spi: fix forced writable option
Date: Tue, 2 Jun 2020 16:36:31 +0000	[thread overview]
Message-ID: <20200602163631.GH23028@zorba> (raw)
In-Reply-To: <190F24BF-EE4C-4C40-9101-C0AE6C9CEF53@cisco.com>

On Thu, May 28, 2020 at 03:55:21PM +0000, Jinhua Wu (jinhwu) wrote:
> On 2020/5/28, 11:48 PM, "Jinhua Wu" <jinhwu@cisco.com> wrote:
> Hi Vignesh,
> BIOS just locked down parts of flash (such as, code region), others are still 
> writeable. Once the SPI locked down,it can't be override unless platfrom reset 
> and set WPD (write protect disable) will fail, so ispi->writeable will always
> be 0, then the driver will always make the whole flash read only, even if we
> have set the parameter writable = 1. 
> Now the flash is totally not writeable, just part of it is read only. Why not  making
> 'writeable' working when explicitly enabled?
> 
> >On 2020/5/28, 7:02 PM, "Vignesh Raghavendra" <vigneshr@ti.com> wrote:
> >    On 18/05/20 11:29 pm, Daniel Walker wrote:
> >    > This option currently doesn't work as expected. If the BIOS has this
> >    > flash as read-only there is no way to change this thru the driver.
> >    > There is a parameter which allows the flash to become writable with the
> >    > "writable" option to the module, but it does nothing if the BIOS has it
> >    > set to read-only.
> >    > 
> >    > I would expect this option would make the flash writable regardless of
> >    > the BIOS settings. This patch changes this option so the BIOS setting
> >    > doesn't stop the writable option from enabling read write on the flash.
> >    > 
> >
> >    I am confused you say "If the BIOS has this flash as read-only there is
> >    no way to change this thru the driver", so is it possible to override
> >    BIOS setting? If yes, where is the code in the driver?
> >
> >    What happens if BIOS is set to allow writes but writeable is set to 0?
> >
> >    Also please send patch series as thread (2/2 in reply to 1/2). You can
> >    use tool like git send-email
> >



Vignesh, do you still have concerns about this change ?


Daniel

WARNING: multiple messages have this Message-ID (diff)
From: "Daniel Walker (danielwa)" <danielwa@cisco.com>
To: "Jinhua Wu (jinhwu)" <jinhwu@cisco.com>
Cc: Vignesh Raghavendra <vigneshr@ti.com>,
	Tudor Ambarus <tudor.ambarus@microchip.com>,
	Richard Weinberger <richard@nod.at>,
	"xe-linux-external\(mailer list\)" <xe-linux-external@cisco.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Mika Westerberg <mika.westerberg@linux.intel.com>
Subject: Re: [PATCH 2/2] mtd: spi-nor: intel-spi: fix forced writable option
Date: Tue, 2 Jun 2020 16:36:31 +0000	[thread overview]
Message-ID: <20200602163631.GH23028@zorba> (raw)
In-Reply-To: <190F24BF-EE4C-4C40-9101-C0AE6C9CEF53@cisco.com>

On Thu, May 28, 2020 at 03:55:21PM +0000, Jinhua Wu (jinhwu) wrote:
> On 2020/5/28, 11:48 PM, "Jinhua Wu" <jinhwu@cisco.com> wrote:
> Hi Vignesh,
> BIOS just locked down parts of flash (such as, code region), others are still 
> writeable. Once the SPI locked down,it can't be override unless platfrom reset 
> and set WPD (write protect disable) will fail, so ispi->writeable will always
> be 0, then the driver will always make the whole flash read only, even if we
> have set the parameter writable = 1. 
> Now the flash is totally not writeable, just part of it is read only. Why not  making
> 'writeable' working when explicitly enabled?
> 
> >On 2020/5/28, 7:02 PM, "Vignesh Raghavendra" <vigneshr@ti.com> wrote:
> >    On 18/05/20 11:29 pm, Daniel Walker wrote:
> >    > This option currently doesn't work as expected. If the BIOS has this
> >    > flash as read-only there is no way to change this thru the driver.
> >    > There is a parameter which allows the flash to become writable with the
> >    > "writable" option to the module, but it does nothing if the BIOS has it
> >    > set to read-only.
> >    > 
> >    > I would expect this option would make the flash writable regardless of
> >    > the BIOS settings. This patch changes this option so the BIOS setting
> >    > doesn't stop the writable option from enabling read write on the flash.
> >    > 
> >
> >    I am confused you say "If the BIOS has this flash as read-only there is
> >    no way to change this thru the driver", so is it possible to override
> >    BIOS setting? If yes, where is the code in the driver?
> >
> >    What happens if BIOS is set to allow writes but writeable is set to 0?
> >
> >    Also please send patch series as thread (2/2 in reply to 1/2). You can
> >    use tool like git send-email
> >



Vignesh, do you still have concerns about this change ?


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

  reply	other threads:[~2020-06-02 16:36 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-18 17:59 [PATCH 2/2] mtd: spi-nor: intel-spi: fix forced writable option Daniel Walker
2020-05-18 17:59 ` Daniel Walker
2020-05-28 11:01 ` Vignesh Raghavendra
2020-05-28 11:01   ` Vignesh Raghavendra
2020-05-28 15:55   ` Jinhua Wu (jinhwu)
2020-05-28 15:55     ` Jinhua Wu (jinhwu)
2020-06-02 16:36     ` Daniel Walker (danielwa) [this message]
2020-06-02 16:36       ` Daniel Walker (danielwa)

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=20200602163631.GH23028@zorba \
    --to=danielwa@cisco.com \
    --cc=akpm@linux-foundation.org \
    --cc=jinhwu@cisco.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 \
    --cc=xe-linux-external@cisco.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.