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

From: <Tudor.Ambarus@microchip.com>
To: <p.yadav@ti.com>
Cc: vigneshr@ti.com, sergei.shtylyov@cogentembedded.com,
	richard@nod.at, me@yadavpratyush.com, john.garry@huawei.com,
	linuxarm@huawei.com, yangyicong@hisilicon.com,
	alexander.sverdlin@nokia.com, miquel.raynal@bootlin.com,
	linux-mtd@lists.infradead.org
Subject: Re: [PATCH 2/2] mtd: spi-nor: Disable the flash quad mode in spi_nor_restore()
Date: Fri, 3 Jul 2020 11:52:09 +0000	[thread overview]
Message-ID: <157c20a4-2a7e-601a-34ef-66baaa17eb7b@microchip.com> (raw)
In-Reply-To: <20200703111924.tyrvzbbgd7ak53qf@ti.com>

On 7/3/20 2:19 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 02/07/20 11:02AM, Tudor.Ambarus@microchip.com wrote:
>> On 6/16/20 4:02 PM, Yicong Yang wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> 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.
>>
>> Please describe the problems that occur. When QE bit is one the flash
>> operates in Standard/Dual/Quad SPI modes. WP# and RESET#/HOLD# are
>> affected as they change their functionality to IO2 and IO3 when QE
>> is 1. Is there anything else?
> 
> IIUC if we do anything that introduces a state on the flash, we want to
> clear that state up on restore. That's what we (will) do for 8D mode and

correct

> for 4-byte addressing mode. Does that not apply here?

yes, it does. I've just asked Yicong to describe in the commit message
the problems that he encounters, for better understanding. Standard and
Dual modes should still work with QE = 1. The only problem that I see
is that WP# and RESET#/HOLD# are changing their functionality to IO2
and IO3 when QE is 1. Is there anything else that I miss?
> 
>> While I find the intention good, there might be some problems here:
>> 1/ w25q jvm variants come with QE "fixed" to 1. This probably means
>> that QE is not writable, and a writing of QE to zero will be ignored,
>> but we have to check.
> 
> In that case they shouldn't have a quad_enable() hook, no?

Right. Although this scenario should be a false positive, probably the
write of QE bit is ignored. There is a superfluous write of QE indeed,
but maybe we can live with it.

> 
>> 2/S25FS128S: CR1NV[1] can set the default power-on state for the
>> CR1V[1] to 1, i.e. QE to be set to 1 at power-on by default. The
>> logic here complicates a bit, and maybe we'll have to amend the
>> patch.
>>

We can come with a patch on top of these for 2/. Yicong, please address
the minor comments and resubmit.

Cheers.

>>>
>>> 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.
>> s/correct/ Standard/Dual SPI
>>
>> Cheers,
>> ta
> 
> --
> Regards,
> Pratyush Yadav
> Texas Instruments India
> 

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