From: Tudor Ambarus <tudor.ambarus@linaro.org>
To: Michael Walle <michael@walle.cc>
Cc: Serge Semin <fancer.lancer@gmail.com>,
Sergiu.Moga@microchip.com, Mark Brown <broonie@kernel.org>,
Tudor Ambarus <tudor.ambarus@microchip.com>,
Pratyush Yadav <pratyush@kernel.org>,
miquel.raynal@bootlin.com, richard@nod.at, vigneshr@ti.com,
Nicolas.Ferre@microchip.com, alexandre.belloni@bootlin.com,
Claudiu.Beznea@microchip.com, chin-ting_kuo@aspeedtech.com,
clg@kaod.org, joel@jms.id.au, andrew@aj.id.au,
kdasu.kdev@gmail.com, han.xu@nxp.com, john.garry@huawei.com,
matthias.bgg@gmail.com, avifishman70@gmail.com,
tmaimon77@gmail.com, tali.perry1@gmail.com, venture@google.com,
yuenn@google.com, benjaminfair@google.com, haibo.chen@nxp.com,
yogeshgaur.83@gmail.com, heiko@sntech.de,
mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com,
michal.simek@xilinx.com, bcm-kernel-feedback-list@broadcom.com,
linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
linux-spi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-aspeed@lists.ozlabs.org, openbmc@lists.ozlabs.org,
linux-mediatek@lists.infradead.org,
linux-rockchip@lists.infradead.org,
linux-stm32@st-md-mailman.stormreply.com
Subject: Re: [PATCH] spi: Replace `dummy.nbytes` with `dummy.ncycles`
Date: Thu, 9 Mar 2023 15:23:34 +0200 [thread overview]
Message-ID: <6c2090bf-d102-a333-3a83-03abe81ff70e@linaro.org> (raw)
In-Reply-To: <03a9f117316ab81f1b5a18100f771e65@walle.cc>
On 09.03.2023 14:35, Michael Walle wrote:
> 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..
>
You miss the fact that you can have 1-1-4. What buswidth do you use for
dummy, the address buswidth or the data buswidth?
What happens if crazy protocols like 1S-1S-8D appear? What buswidth and
transfer mode are you going to use for dummy?
And please don't tell me that "we're going to assume that dummy.buswidth
= address.buswidth because that's what we currently do in SPI NOR",
because I'm not convinced that the assumption is correct.
> 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
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2023-03-09 13:24 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-11 17:45 [PATCH] spi: Replace `dummy.nbytes` with `dummy.ncycles` Sergiu Moga
2022-09-12 9:44 ` Patrice CHOTARD
2022-09-12 10:58 ` Tudor.Ambarus
2022-09-12 11:52 ` Mark Brown
2022-09-25 22:03 ` Serge Semin
2022-09-26 9:05 ` Sergiu.Moga
2022-09-26 17:24 ` Serge Semin
2022-09-27 8:21 ` Sergiu.Moga
2023-03-08 9:04 ` Tudor Ambarus
2023-03-09 8:38 ` Michael Walle
2023-03-09 10:42 ` Tudor Ambarus
2023-03-09 10:56 ` Michael Walle
2023-03-09 12:09 ` Tudor Ambarus
2023-03-09 12:35 ` Michael Walle
2023-03-09 13:23 ` Tudor Ambarus [this message]
2023-03-09 13:33 ` Michael Walle
2023-03-09 13:54 ` Tudor Ambarus
2023-03-09 14:01 ` Michael Walle
2023-03-09 14:19 ` Chuanhong Guo
2023-03-09 15:41 ` Michael Walle
2023-03-09 15:41 ` Tudor Ambarus
2023-03-04 5:59 ` Tudor Ambarus
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=6c2090bf-d102-a333-3a83-03abe81ff70e@linaro.org \
--to=tudor.ambarus@linaro.org \
--cc=Claudiu.Beznea@microchip.com \
--cc=Nicolas.Ferre@microchip.com \
--cc=Sergiu.Moga@microchip.com \
--cc=alexandre.belloni@bootlin.com \
--cc=alexandre.torgue@foss.st.com \
--cc=andrew@aj.id.au \
--cc=avifishman70@gmail.com \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=benjaminfair@google.com \
--cc=broonie@kernel.org \
--cc=chin-ting_kuo@aspeedtech.com \
--cc=clg@kaod.org \
--cc=fancer.lancer@gmail.com \
--cc=haibo.chen@nxp.com \
--cc=han.xu@nxp.com \
--cc=heiko@sntech.de \
--cc=joel@jms.id.au \
--cc=john.garry@huawei.com \
--cc=kdasu.kdev@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-aspeed@lists.ozlabs.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-mtd@lists.infradead.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux-spi@vger.kernel.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=matthias.bgg@gmail.com \
--cc=mcoquelin.stm32@gmail.com \
--cc=michael@walle.cc \
--cc=michal.simek@xilinx.com \
--cc=miquel.raynal@bootlin.com \
--cc=openbmc@lists.ozlabs.org \
--cc=pratyush@kernel.org \
--cc=richard@nod.at \
--cc=tali.perry1@gmail.com \
--cc=tmaimon77@gmail.com \
--cc=tudor.ambarus@microchip.com \
--cc=venture@google.com \
--cc=vigneshr@ti.com \
--cc=yogeshgaur.83@gmail.com \
--cc=yuenn@google.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).