Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH 1/2] spi: support inter-word delay requirement for devices
To:     Baolin Wang <baolin.wang@linaro.org>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        linux-spi <linux-spi@vger.kernel.org>,
        Mark Brown <broonie@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        DTML <devicetree@vger.kernel.org>
References: <20190125114429.20066-1-jonas@norrbonn.se>
 <20190125114429.20066-2-jonas@norrbonn.se>
 <CAMz4ku+sqzK19994BpdSfaDjNdXW_-gnCutr_fYQiv5ynxOq2g@mail.gmail.com>
From:   Jonas Bonn <jonas@norrbonn.se>
Message-ID: <f34151c8-28a3-3d53-a27e-b5ff512d9c94@norrbonn.se>
Date:   Fri, 25 Jan 2019 13:06:45 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.2.1
MIME-Version: 1.0
In-Reply-To: <CAMz4ku+sqzK19994BpdSfaDjNdXW_-gnCutr_fYQiv5ynxOq2g@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi,

On 25/01/2019 12:53, Baolin Wang wrote:
> Hi,
> On Fri, 25 Jan 2019 at 19:44, Jonas Bonn <jonas@norrbonn.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@norrbonn.se>
>> CC: Mark Brown <broonie@kernel.org>
>> CC: Rob Herring <robh+dt@kernel.org>
>> CC: Mark Rutland <mark.rutland@arm.com>
>> CC: linux-spi@vger.kernel.org
>> CC: devicetree@vger.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
>>
> 
>