linux-kernel - Re: [PATCH 1/2] spi: support inter-word delay requirement for devices

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: <f34151c8-28a3-3d53-a27e-b5ff512d9c94@norrbonn.se>
Date:   Fri, 25 Jan 2019 13:06:45 +0100
From:   Jonas Bonn <jonas@...rbonn.se>
To:     Baolin Wang <baolin.wang@...aro.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        linux-spi <linux-spi@...r.kernel.org>,
        Mark Brown <broonie@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        DTML <devicetree@...r.kernel.org>
Subject: Re: [PATCH 1/2] spi: support inter-word delay requirement for devices

Hi,

On 25/01/2019 12:53, Baolin Wang wrote:
> Hi,
> On Fri, 25 Jan 2019 at 19:44, Jonas Bonn <jonas@...rbonn.se> wrote:
>>
>> Some devices are slow and cannot keep up with the SPI bus and therefore
>> require a short delay between words of the SPI transfer.
>>
>> The example of this that I'm looking at is a SAMA5D2 with a minimum SPI
>> clock of 400kHz talking to an AVR-based SPI slave.  The AVR cannot put
>> bytes on the bus fast enough to keep up with the SoC's SPI controller
>> even at the lowest bus speed.
>>
>> This patch introduces the ability to specify a required inter-word
>> delay for SPI devices.  It is up to the controller driver to configure
>> itself accordingly in order to introduce the requested delay.
> 
> Can we configure it at runtime by the device rather than at DT time by
> the controller? If yes, we already have a patch for this, please
> check:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=eeaceb8b7d1fb64b6030249ca0dd1d902ef3069e
> 

It's a characteristic of the SPI slave, in the same sense as CPOL/CPHA 
are, and therefore it makes sense to specify it in the device tree.

Having this as device property rather than a transfer property allows 
this to be configured one time in setup() rather than having to fiddle 
with the configuration register for every transfer.

The two approaches are complementary.

/Jonas

>>
>> Signed-off-by: Jonas Bonn <jonas@...rbonn.se>
>> CC: Mark Brown <broonie@...nel.org>
>> CC: Rob Herring <robh+dt@...nel.org>
>> CC: Mark Rutland <mark.rutland@....com>
>> CC: linux-spi@...r.kernel.org
>> CC: devicetree@...r.kernel.org
>> ---
>>   Documentation/devicetree/bindings/spi/spi-bus.txt | 1 +
>>   drivers/spi/spi.c                                 | 4 ++++
>>   include/linux/spi/spi.h                           | 1 +
>>   3 files changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
>> index 1f6e86f787ef..a5f20060676d 100644
>> --- a/Documentation/devicetree/bindings/spi/spi-bus.txt
>> +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
>> @@ -77,6 +77,7 @@ All slave nodes can contain the following optional properties:
>>                      Defaults to 1 if not present.
>>   - spi-rx-delay-us - Microsecond delay after a read transfer.
>>   - spi-tx-delay-us - Microsecond delay after a write transfer.
>> +- spi-word-delay-us - Microsecond delay between individual words of a transfer
>>
>>   Some SPI controllers and devices support Dual and Quad SPI transfer mode.
>>   It allows data in the SPI system to be transferred using 2 wires (DUAL) or 4
>> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
>> index 9a7def7c3237..cd4d4065eca2 100644
>> --- a/drivers/spi/spi.c
>> +++ b/drivers/spi/spi.c
>> @@ -1692,6 +1692,10 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
>>          }
>>          spi->max_speed_hz = value;
>>
>> +       if (!of_property_read_u32(nc, "spi-word-delay-us", &value)) {
>> +               spi->word_delay = value;
>> +       }
>> +
>>          return 0;
>>   }
>>
>> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
>> index 314d922ca607..e5200dd9d750 100644
>> --- a/include/linux/spi/spi.h
>> +++ b/include/linux/spi/spi.h
>> @@ -164,6 +164,7 @@ struct spi_device {
>>          char                    modalias[SPI_NAME_SIZE];
>>          const char              *driver_override;
>>          int                     cs_gpio;        /* chip select gpio */
>> +       uint16_t                word_delay;     /* inter-word delay (us) */
>>
>>          /* the statistics */
>>          struct spi_statistics   statistics;
>> --
>> 2.19.1
>>
> 
>