linux-kernel - Re: [PATCH v5 07/16] spi: dt-bindings: axi-spi-engine: add SPI offload properties

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: <f0e35a14-f870-400f-b26a-ce6f212484ca@baylibre.com>
Date: Tue, 19 Nov 2024 11:02:50 -0600
From: David Lechner <dlechner@...libre.com>
To: Rob Herring <robh@...nel.org>
Cc: Mark Brown <broonie@...nel.org>, Jonathan Cameron <jic23@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Nuno Sá <nuno.sa@...log.com>,
 Uwe Kleine-König <ukleinek@...nel.org>,
 Michael Hennerich <Michael.Hennerich@...log.com>,
 Lars-Peter Clausen <lars@...afoo.de>, David Jander <david@...tonic.nl>,
 Martin Sperl <kernel@...tin.sperl.org>, linux-spi@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-iio@...r.kernel.org, linux-pwm@...r.kernel.org
Subject: Re: [PATCH v5 07/16] spi: dt-bindings: axi-spi-engine: add SPI
 offload properties

On 11/19/24 10:56 AM, Rob Herring wrote:
> On Fri, Nov 15, 2024 at 02:18:46PM -0600, David Lechner wrote:
>> The AXI SPI Engine has support for hardware offloading capabilities.
>> This includes a connection to a DMA controller for streaming RX data
>> and a trigger input for starting execution of the SPI message programmed
>> in the offload.
>>
>> Each SPI Engine may have up to 1 offload. The spec actually says that
>> it could support up to 32, so we are using an index number in the
>> dma-names (e.g. offload0-rx) to allow for this possibility in the
>> future.
>>
>> Signed-off-by: David Lechner <dlechner@...libre.com>
>> ---
>>
>> v5 changes:
>> * Also document offload0-tx DMA names since the hardware can support
>>   that now.
>> * Limit the number of offloads to 1 for now since it would require
>>   significant hardware changes to actually support more than that.
>>
>> v4 changes:
>> * Dropped #spi-offload-cells property.
>> * Changed subject line.
>>
>> v3 changes:
>> * Added #spi-offload-cells property.
>> * Added properties for triggers and RX data stream connected to DMA.
>>
>> v2 changes:
>> * This is basically a new patch. It partially replaces "dt-bindings:
>>   iio: offload: add binding for PWM/DMA triggered buffer".
>> * The controller no longer has an offloads object node and the
>>   spi-offloads property is now a standard SPI peripheral property.
>> ---
>>  .../bindings/spi/adi,axi-spi-engine.yaml           | 24 ++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
>> index d48faa42d025..d703b47eb498 100644
>> --- a/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
>> +++ b/Documentation/devicetree/bindings/spi/adi,axi-spi-engine.yaml
>> @@ -41,6 +41,26 @@ properties:
>>        - const: s_axi_aclk
>>        - const: spi_clk
>>  
>> +  trigger-sources:
>> +    description:
>> +      An array of trigger source phandles for offload instances. The index in
>> +      the array corresponds to the offload instance number.
> 
> How can you have an index when you only allow 1 entry (other than 0 of 
> course)?

Same reason as below, but we can drop that part of the description for now.

> 
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> 
> With my other comments implemented, this should be dropped.
> 
>> +    maxItems: 1
>> +
>> +  dmas:
>> +    description:
>> +      DMA channels connected to the input or output stream interface of an
>> +      offload instance.
>> +    minItems: 1
>> +    maxItems: 2
>> +
>> +  dma-names:
>> +    items:
>> +      pattern: "^offload0-[tr]x$"
> 
> Do you expect an offload1 or something?

Yes, I tried to make this clear in the commit message. The hardware
is designed to have up to 32 offload instances, but in practice
there can only be 1 for now. So I included the 0 here to make it
future-proof in case that changes in the future.

> 
>> +    minItems: 1
>> +    maxItems: 2
>> +
>>  required:
>>    - compatible
>>    - reg
>> @@ -59,6 +79,10 @@ examples:
>>          clocks = <&clkc 15>, <&clkc 15>;
>>          clock-names = "s_axi_aclk", "spi_clk";
>>  
>> +        trigger-sources = <&trigger_clock>;
>> +        dmas = <&dma 0>;
>> +        dma-names = "offload0-rx";
>> +
>>          #address-cells = <1>;
>>          #size-cells = <0>;
>>  
>>
>> -- 
>> 2.43.0
>>