linux-kernel - Re: [PATCH] spi: Replace `dummy.nbytes` with `dummy.ncycles`

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5183a184-c72d-3acd-70cd-6aa1e31533f5@linaro.org>
Date:   Thu, 9 Mar 2023 12:09:48 +0000
From:   Tudor Ambarus <tudor.ambarus@...aro.org>
To:     Michael Walle <michael@...le.cc>
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`



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.

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?

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.

> what the semper nano flash uses. Half cycles? But according to your

there's no spimem flash code that use half cycles for now.

> naming you'd specify full cylces?

A clock period, yes.

Hope it helps.
ta