Message-ID: <03a9f117316ab81f1b5a18100f771e65@walle.cc>
Date:   Thu, 09 Mar 2023 13:35:35 +0100
From:   Michael Walle <michael@...le.cc>
To:     Tudor Ambarus <tudor.ambarus@...aro.org>
Cc:     Serge Semin <fancer.lancer@...il.com>, Sergiu.Moga@...rochip.com,
        Mark Brown <broonie@...nel.org>,
        Tudor Ambarus <tudor.ambarus@...rochip.com>,
        Pratyush Yadav <pratyush@...nel.org>,
        miquel.raynal@...tlin.com, richard@....at, vigneshr@...com,
        Nicolas.Ferre@...rochip.com, alexandre.belloni@...tlin.com,
        Claudiu.Beznea@...rochip.com, chin-ting_kuo@...eedtech.com,
        clg@...d.org, joel@....id.au, andrew@...id.au,
        kdasu.kdev@...il.com, han.xu@....com, john.garry@...wei.com,
        matthias.bgg@...il.com, avifishman70@...il.com,
        tmaimon77@...il.com, tali.perry1@...il.com, venture@...gle.com,
        yuenn@...gle.com, benjaminfair@...gle.com, haibo.chen@....com,
        yogeshgaur.83@...il.com, heiko@...ech.de,
        mcoquelin.stm32@...il.com, alexandre.torgue@...s.st.com,
        michal.simek@...inx.com, bcm-kernel-feedback-list@...adcom.com,
        linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-spi@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-aspeed@...ts.ozlabs.org, openbmc@...ts.ozlabs.org,
        linux-mediatek@...ts.infradead.org,
        linux-rockchip@...ts.infradead.org,
        linux-stm32@...md-mailman.stormreply.com
Subject: Re: [PATCH] spi: Replace `dummy.nbytes` with `dummy.ncycles`

Am 2023-03-09 13:09, schrieb Tudor Ambarus:
> On 3/9/23 10:56, Michael Walle wrote:
>> Am 2023-03-09 11:42, schrieb Tudor Ambarus:
>>> On 09.03.2023 10:38, Michael Walle wrote:
>>>>> In an ideal world, where both the controller and the device talk 
>>>>> about
>>>>> dummy number of cycles, I would agree with you, buswidth and dtr 
>>>>> should
>>>>> not be relevant for the number of dummy cycles. But it seems that 
>>>>> there
>>>>> are old controllers (e.g. spi-hisi-sfc-v3xx.c, spi-mt65xx.c,
>>>>> spi-mxic.c)
>>>>> that support buswidths > 1 and work only with dummy nbytes, they 
>>>>> are
>>>>> not
>>>>> capable of specifying a smaller granularity (ncycles). Thus the 
>>>>> older
>>>>> controllers would have to convert the dummy ncycles to dummy 
>>>>> nbytes.
>>>>> Since mixed transfer modes are a thing (see jesd251, it talks about
>>>>> 4S-4D-4D), where single transfer mode (S) can be mixed with double
>>>>> transfer mode (D) for a command, the controller would have to guess 
>>>>> the
>>>>> buswidth and dtr of the dummy. Shall they replicate the buswidth 
>>>>> and
>>>>> dtr
>>>>> of the address or of the data? There's no rule for that.
>>>> 
>>>> But in the end that doesn't matter because they are just dummy clock
>>>> cycles and the mode will only affect the data/address/command.
>>>> Therefore,
>>>> the controller is free to choose the mode that suits it best.
>>>>  > But that begs the question, is ncycles in regard to DTR or SDR?
>>>> That is,
>>>> are you counting just one type of edges or both the falling and 
>>>> rising
>>>> edges. The smallest granularity would be ncycles in regard of DTR. 
>>>> To
>>>> me,
>>>> it's not obvious what the SEMPER Nano Flash [1] uses. I'd say we 
>>>> choose
>>>> the smallest granularty in spi-mem to be future proof and maybe 
>>>> provide
>>>> some spi-mem helper to help setting the cycles for SDR/DTR. As an
>>>> example,
>>>> if you want to wait 4 cycles in SDR you'd have ncycles=8 in spi-mem.
>>>> 
>>> 
>>> No, we can't invent our own measuring units. We have cycles and half
>>> cycles (regardless of the transfer mode used (STR, DTR)).
>> 
>> That is basically what I was saying, just using the correct term.
>> Ok. So we don't need the dtr property, right? I'm still not sure,
> 
> We do.
> 
> As of now you can't specify 20 dummy cycles for READID in 8D-8D-8D mode
> because all the layers treats dummy as bytes, whereas they should treat
> it as cycles. One dummy byte in 8D-8D-8D means 16 dummy cycles. 20 
> dummy
> cycles in 8D-8D-8D means one byte and a quarter? This is a non-sense.

Agreed.

> As the code is now, SPI NAND uses dummy cycles that are multiple of 8.
> SPI NOR requires a variable number of dummy cycles, there's no
> restrictions. In SPI NOR we get from SFDP or datasheets the number of
> dummy cycles, and in the code we convert them to dummy nbytes. Then 
> when
> we get at the controller side, the majority of the controllers undo the
> operation, they take the dummy nbytes and convert them to dummy 
> ncycles.
> Isn't better to use dummy ncycles from the beginning?

Yes, but now we should define what *one* cycle is. And that it is 
defined
regardless of the mode, because the mode only affects the IO lines. But
a clock cycle refers to the clock line. [coming back to here] And as you
said one cycle is one full clock cycle, it is also independent of the 
dtr
setting.

> The controllers that can talk in dummy ncycles don't need the
> dummy.{buswidth, dtr} fields.
> 
> The controllers that can't talk in dummy cycles, but only on a "byte"
> boundary need both buswidth and dtr fields. Assume a flash needs 32
> dummy cycles for an op on 8D-8D-8D mode. If the controller does not 
> have
> the buswidth and dtr info, it can't convert the dummy ncycles to 
> nbytes.
> If he knows only that buswidth is 8, it will convert ncycles to 4 
> bytes.
> If dtr is also specified it converts ncycles to 2 bytes.

No they don't need it. Lets take your semper flash and assume it needs
12 latency cycles. SPI-NOR will set ncycles to 12 *regardless of the 
mode
or dtr setting*. The controller then knows we need 12 clock cycles. It 
has
then to figure out how that can be achieved. E.g. if it can only do the
"old" byte programming and is in quad mode, good for it. It will send 6
dummy bytes, which will result in 12 dummy clock cycles, because 1 byte
takes two clock cycles in quad SDR mode. If its in octal mode, send 12
bytes. If its in dual mode, send 3 bytes. Obiously, it cannot be in
single bit mode, because it cannot send 1.5 bytes..

If it's freely programmable, it will just tell the hardware to insert
12 dummy clock cycles.

>> what the semper nano flash uses. Half cycles? But according to your
> 
> there's no spimem flash code that use half cycles for now.

Ahh, I just saw the semper flash doesn't support DTR at all. Ok then,
makes things even simpler.

>> naming you'd specify full cylces?
> 
> A clock period, yes.

Ok.

-michael

Powered by blists - more mailing lists