Re: [PATCH v4 2/6] mtd: spi-nor: add Octal DTR support for Macronix flash

From: Pratyush Yadav <pratyush@kernel.org>
To: Tudor Ambarus <tudor.ambarus@linaro.org>
Cc: Jaime Liao <jaimeliao.tw@gmail.com>,
	 linux-mtd@lists.infradead.org, pratyush@kernel.org,
	 michael@walle.cc,  miquel.raynal@bootlin.com, leoyu@mxic.com.tw,
	 jaimeliao@mxic.com.tw
Subject: Re: [PATCH v4 2/6] mtd: spi-nor: add Octal DTR support for Macronix flash
Date: Thu, 05 Oct 2023 12:18:16 +0200	[thread overview]
Message-ID: <mafs07co1pe3b.fsf@kernel.org> (raw)
In-Reply-To: <911a6f0e-73bd-aa24-1b03-dd38bef1deba@linaro.org> (Tudor Ambarus's message of "Wed, 20 Sep 2023 15:37:06 +0300")

On Wed, Sep 20 2023, Tudor Ambarus wrote:

> Hi, Jaime,
>
> On 08.09.2023 09:43, Jaime Liao wrote:
>> From: JaimeLiao <jaimeliao@mxic.com.tw>
>> 
>> Create Macronix specify method for enable Octal DTR mode and
>> set 20 dummy cycles to allow running at the maximum supported
>> frequency for Macronix Octal flash.
>
> You didn't answer my question. What happens when you specify 20 dummy
> cycles, thus you configure the dummy cycles for the maximum flash speed,
> but you program the flash to work on 1 MHz for example. Do you still
> have reliable results?

I don't know about this particular flash, but in the past, I have tried
doing this with Spansion and Micron flashes, and they work fine on lower
frequencies even with the maximum dummy cycles set.

When you think about it, the only reason higher frequencies put a
restriction on minimum dummy cycles is because the flash actually has a
restriction on _time_. As time for each cycle gets shorter with higher
frequencies, you need more cycles to wait the same amount of time. Dummy
cycles are just a way to ensure you wait more than the minimum amount of
time needed to get the flash ready to transmit data.

So I see no reason for a flash to break because it waited _longer_ than
the minimum time.

>> 
>> Use number of dummy cycles which is parse by SFDP then convert
>> it to bit pattern and set in CR2 register.
>
> What we should do instead is to determine the flash speed at which the
> flash is operated and then to set the correct number of dummy cycles
> according to what the flash requires. There should be a table somewhere
> in the datasheet that specify a required number of dummy cycles for a
> particular frequency, isn't it? Yeah, see "9-3-1. Dummy Cycle and
> Frequency Table (MHz)" of the mx66lm1g45g datasheet.

Right, most flashes do give such a table, though I don't remember any
more if SFDP has something like this as well.

I remember thinking the same thing when I was adding support for Octal
DTR in SPI NOR. The problem is that SPI NOR currently has no way of
knowing what exact speed the flash will be operated at.

It can look at nor->spimem->spi->max_speed_hz (I am not sure it should
do this directly) which comes from the "spi-max-frequency" DT property,
but that is only the maximum. This can be a good starting point to
decide the maximum number of cycles you would need.

But that is only the maximum. The controller does not guarantee using
the maximum speed. It can use something slower as well. And currently
there is no way for the controller to report that speed back to SPI MEM
or SPI NOR. So if we want to waste the least amount of dummy cycles, we
would need to teach the controller drivers to report back the exact
speed it is going to driver the flash at.

I am not sure this might be worth the trouble though. We should first
quantify how much throughput/latency we stand to gain from doing this.
But I do think looking at "spi-max-frequency" is fairly simple and
should be a good enough start.

-- 
Regards,
Pratyush Yadav

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