linux-kernel - AW: [PATCH 1/4] spi: Add parameter for clock to rx delay

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: <DB9PR10MB82468A8BD333B12D3FCB3C43F1DCA@DB9PR10MB8246.EURPRD10.PROD.OUTLOOK.COM>
Date:   Fri, 27 Oct 2023 08:38:54 +0000
From:   "Stoll, Eberhard" <eberhard.stoll@...tron.de>
To:     Miquel Raynal <miquel.raynal@...tlin.com>,
        Eberhard Stoll <estl@....net>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
        Mark Brown <broonie@...nel.org>,
        "Schrempf, Frieder" <frieder.schrempf@...tron.de>,
        Amit Kumar Mahapatra <amit.kumar-mahapatra@....com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Christophe JAILLET <christophe.jaillet@...adoo.fr>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Krishna Yarlagadda <kyarlagadda@...dia.com>,
        Leonard Göhrs <l.goehrs@...gutronix.de>,
        Yang Yingliang <yangyingliang@...wei.com>
Subject: AW: [PATCH 1/4] spi: Add parameter for clock to rx delay

Hello,

> > For spi devices the master clock output defines the sampling point
> > for receive data input stream (rising or falling edge). The receive
> > data stream from the device is delayed in relation to the master
> > clock output.
> >
> > For some devices this delay is larger than one half clock period,
> 
> Can you be more specific? I am wondering how big the need is.

In our case it's a QSPI NAND chip (Winbond W25N02KV). This device
can operate at 104MHz SPI clock. But it also has a tCLQV value of 7ns.
The tCLQV value limits the SPI clock speed for this device to 2x7ns
(if it is not adjustable in the SPI controller) which is approximately
70MHz.

Without the ability to set the tCLQV value, the SPI clock has to be
limited to 70MHz in device tree for this bus.

In our case the Winbond W25N02KV chip is a replacement of an
older chip. The older chip can operate at 104MHz and does not
have the tCLQV restrictions as this new one.
The new chip is mostly is better than the data sheet and meet the
timing requirements for 104MHz. But on higher temperatures
devices fail.

In device tree QSPI NAND chips are configured by a compatible
property of 'spi-nand'. The mtd layer detects the real device
and fetches the properties of this device from the appropriate
driver.

So for our case (boards containing the old and new chip) we well
have to reduce the SPI clock for the entire QSPI bus to 70MHz, even
for the elder chips which can operate well also with 104MHz. 

> 
> > which is normally the sampling point for receive data. In this case
> > receive data is sampled too early and the device fails to operate.
> > In consequence the spi clock has to be reduced to match the delay
> > characteristics and this reduces performance and is therefore not
> > recommended.
> >
> > Some spi controllers implement a 'clock to receive data delay'
> > compensation which shifts the receive sampling point. So we need
> > a property to set this value for each spi device.
> 
> What if the spi controller does not support this feature? Shall we add
> a capability? Shall we refuse to probe if the controller is not capable
> of sampling at the right moment?
> 

Refuse to probe is not necessary IMHO. The device can operate well
even with controllers which do not implement the tCLQV functionality.
The SPI clock has simply to be reduced and all works well. In this case
not the maximum SPI clock frequency of the device limits the SPI bus
clock, but the tCLQV value!

IMHO it's the responsibility of the writer of the device tree configuration.

For SPI controllers which do not support this setting, the SPI framework
could check whether the max SPI clock frequency of the device or the
tCLQV value limits the SPI bus speed and adjust it appropriately.

For our case this seems a little bit of overkill.

With 'discoverable' devices on the SPI bus like SPI NAND chips, the
'max_speed_hz' in 'struct spi_device' is no more really device specific,
but more like chip select specific. The real chips 'max_speed_hz' data
sheet value could then e.g. be propagated from the discovered chips SPI
device driver to the frameworks  'chip select specific'  'max_speed_hz'
property. We could introduce a 'probe_speed_hz' setting and maybe
many other things ...

... but IMHO this would be far too much of overkill (at least currently) ...

Thanks,
Eberhard