Re: [PATCH 2/2] mtd: spi-nor: Disable the flash quad mode in spi_nor_restore()

From: Pratyush Yadav <p.yadav@ti.com>
To: "Matthias Weißer" <m.weisser.m@gmail.com>
Cc: vigneshr@ti.com, sergei.shtylyov@cogentembedded.com,
	tudor.ambarus@microchip.com, richard@nod.at,
	me@yadavpratyush.com, john.garry@huawei.com, linuxarm@huawei.com,
	Yicong Yang <yangyicong@hisilicon.com>,
	linux-mtd@lists.infradead.org, miquel.raynal@bootlin.com,
	alexander.sverdlin@nokia.com
Subject: Re: [PATCH 2/2] mtd: spi-nor: Disable the flash quad mode in spi_nor_restore()
Date: Tue, 1 Sep 2020 16:41:13 +0530	[thread overview]
Message-ID: <20200901111111.wwazfnc3k5flr6qy@ti.com> (raw)
In-Reply-To: <CAO8h3eF_XQZBAP2szOu2oHv=DxznSo-2bDEOsthsJk+MP1s6ng@mail.gmail.com>

On 01/09/20 12:08PM, Matthias Weißer wrote:
> Am Di., 1. Sept. 2020 um 11:48 Uhr schrieb Pratyush Yadav <p.yadav@ti.com>:
> > On 01/09/20 08:16AM, Matthias Weißer wrote:
> > > Am Di., 16. Juni 2020 um 15:03 Uhr schrieb Yicong Yang
> > > <yangyicong@hisilicon.com>:
> > > > If the flash's quad mode is enabled, it'll remain in the quad mode when
> > > > it's removed. If we drive the flash next time in SPI/Dual mode, then
> > > > problem occurs as the flash's quad enable bit is not cleared.
> > >
> > > On flash devices with a non-volatile quad enable bit (we use S25FL512S)
> > > this will wear out the quad enable bit as on every boot the bit is
> > > set and reset on shutdown. Or do I miss something here?
> >
> > Yes, I think it will. If you always want the bit enabled (and it being
> > non-volatile suggests that is the intended use), why not set
> > nor->params->quad_enable() to NULL? That way it won't be touched at all,
> > neither on boot nor on shutdown.
> 
> Because we want to use plain mainline kernel without any local patches.

IMO SPI NOR should not touch non-volatile bits at all. They should be 
set by some other software and SPI NOR should only read them. So I can 
see a change like this justifiable for merging into mainline.

The other option would be to put the call to spi_nor_quad_enable() 
behind a check for SNOR_F_BROKEN_RESET, like we do for 4-byte addressing 
mode.

Either way, the patch has already landed in mainline. You will have to 
submit a patch with whichever option you think is better.

> > > > Disable the quad mode in spi_nor_restore(), the flash will leave
> > > > quad mode when remove. This will make sure the flash always enter the
> > > > correct mode when loaded.
> > >
> > > We have a system which relies on an enabled quad mode on boot
> > > (bootloader uses quad mode without enabling it) so using this patch
> > > will prevent our device from booting.
> >
> > Out of curiosity, how does the flash get detected by SPI NOR? It issues
> > the Read ID command in 1S-1S-1S mode but the flash is expecting 1S-4S-4S
> > commands. Do you have any extra patches applied to make it work?
> 
> The RDID command (0x9F) is always a 1S-1S-1S command regardless of the
> QE bit state as e.g. the READ command (0x03). The only reason for the QE
> bit is that it remaps the usage of the !WP and !HOLD signals to IO2 and IO3.
> 
> The SPI controller (QSPI on imx6 in our case) have to be programmed
> accordingly to execute the RDID command in 1S-1S-1S mode but then the
> state of the QE bit doesn't matter.
> 
> > I tried solving this problem for 8D-8D-8D mode but never really found a
> > good solution.
> 
> I don't know your hardware but I am sure that should work that way to.

Unfortunately for the hardware I'm using (S28HS512T) once Octal mode is 
enabled RDID is also in 8D-8D-8D mode. So this isn't applicable there. 
Still, thanks for the explanation.

> The current way it is done in your patch may wear out the QE bit (and
> maybe others in status/configuration register) quite quickly if you
> have a device which is booted frequently.

This isn't my patch :-)

-- 
Regards,
Pratyush Yadav
Texas Instruments India

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