[PATCH 0/2] spi: cadence-xspi: support 4bytes sdma-io-width

[PATCH 0/2] spi: cadence-xspi: support 4bytes sdma-io-width

[Jisheng Zhang]

The cdns xspi controller SDMA data port may support wider I/O width.
Wider I/O width can benefit performance. A simple test with QSPI nor
flash on one arm64 platform:

1 byte io width (default):
 # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
 1000+0 records in
 1000+0 records out
 8192000 bytes (7.8MB) copied, 1.368735 seconds, 5.7MB/s

4 bytes io width:
 # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
 1000+0 records in
 1000+0 records out
 8192000 bytes (7.8MB) copied, 1.088787 seconds, 7.2MB/s

Improved by 26.3%!


Jisheng Zhang (2):
  spi: dt-bindings: cdns,xspi: add sdma-io-width
  spi: cadence-xspi: support 4bytes sdma-io-width

 .../devicetree/bindings/spi/cdns,xspi.yaml    |  6 +++
 drivers/spi/spi-cadence-xspi.c                | 43 +++++++++++++++++--
 2 files changed, 45 insertions(+), 4 deletions(-)

-- 
2.53.0

[PATCH 1/2] spi: dt-bindings: cdns,xspi: add sdma-io-width

[Jisheng Zhang]

The cdns xspi controller SDMA data port may support wider I/O width.
Wider I/O width can benefit performance. A simple test with QSPI nor
flash on one arm64 platform:

1 byte io width (default):
 # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
 1000+0 records in
 1000+0 records out
 8192000 bytes (7.8MB) copied, 1.368735 seconds, 5.7MB/s

4 bytes io width:
 # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
 1000+0 records in
 1000+0 records out
 8192000 bytes (7.8MB) copied, 1.088787 seconds, 7.2MB/s

Improved by 26.3%!

Add dt binding for sdma-io-width.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 Documentation/devicetree/bindings/spi/cdns,xspi.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
index 38a5795589de..1fd4b6dfd417 100644
--- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
+++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
@@ -40,6 +40,12 @@ properties:
   interrupts:
     maxItems: 1
 
+  sdma-io-width:
+    description: Slave DMA data port I/O width (in bytes)
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 1
+    enum: [ 1, 4 ]
+
 required:
   - compatible
   - reg
-- 
2.53.0

Re: [PATCH 1/2] spi: dt-bindings: cdns,xspi: add sdma-io-width

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The newly added device tree property `sdma-io-width` lacks a vendor prefix.
--

commit 90424bd22513430cb3aefe4494640a5d950b2a77
Author: Jisheng Zhang <jszhang@kernel.org>

spi: dt-bindings: cdns,xspi: add sdma-io-width

This commit adds the sdma-io-width property to the Cadence XSPI controller
device tree binding to configure the Slave DMA data port I/O width.

> diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> index 38a5795589de2..1fd4b6dfd4178 100644
> --- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> @@ -40,6 +40,12 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  sdma-io-width:

This isn't a bug, but should this property include a vendor prefix? Custom
hardware properties typically use a vendor prefix like cdns,sdma-io-width.

> +    description: Slave DMA data port I/O width (in bytes)
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 1
> +    enum: [ 1, 4 ]
> +
>  required:
>    - compatible
>    - reg

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511031732.3199-1-jszhang@kernel.org?part=1

Re: [PATCH 1/2] spi: dt-bindings: cdns,xspi: add sdma-io-width

[Krzysztof Kozlowski]

On Mon, May 11, 2026 at 11:17:31AM +0800, Jisheng Zhang wrote:
> The cdns xspi controller SDMA data port may support wider I/O width.
> Wider I/O width can benefit performance. A simple test with QSPI nor
> flash on one arm64 platform:
> 
> 1 byte io width (default):
>  # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
>  1000+0 records in
>  1000+0 records out
>  8192000 bytes (7.8MB) copied, 1.368735 seconds, 5.7MB/s
> 
> 4 bytes io width:
>  # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
>  1000+0 records in
>  1000+0 records out
>  8192000 bytes (7.8MB) copied, 1.088787 seconds, 7.2MB/s
> 
> Improved by 26.3%!
> 
> Add dt binding for sdma-io-width.
> 
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
>  Documentation/devicetree/bindings/spi/cdns,xspi.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> index 38a5795589de..1fd4b6dfd417 100644
> --- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> @@ -40,6 +40,12 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  sdma-io-width:
> +    description: Slave DMA data port I/O width (in bytes)

Waht is SDMA? What is Slave DMA? The same?

Why this cannot be deduced from the compatible? DMA is fixed per
platform and SDMA do not sound like SPI device (so slave in older
terminology) property.

Anyway, there is generic property reg-io-width if this is for MMIO,
bus-width and spi-tx/rx-bus-width.

Best regards,
Krzysztof

Re: [PATCH 1/2] spi: dt-bindings: cdns,xspi: add sdma-io-width

[Jisheng Zhang]

On Fri, May 15, 2026 at 10:00:50AM +0200, Krzysztof Kozlowski wrote:
> On Mon, May 11, 2026 at 11:17:31AM +0800, Jisheng Zhang wrote:
> > The cdns xspi controller SDMA data port may support wider I/O width.
> > Wider I/O width can benefit performance. A simple test with QSPI nor
> > flash on one arm64 platform:
> > 
> > 1 byte io width (default):
> >  # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
> >  1000+0 records in
> >  1000+0 records out
> >  8192000 bytes (7.8MB) copied, 1.368735 seconds, 5.7MB/s
> > 
> > 4 bytes io width:
> >  # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
> >  1000+0 records in
> >  1000+0 records out
> >  8192000 bytes (7.8MB) copied, 1.088787 seconds, 7.2MB/s
> > 
> > Improved by 26.3%!
> > 
> > Add dt binding for sdma-io-width.
> > 
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> >  Documentation/devicetree/bindings/spi/cdns,xspi.yaml | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> > index 38a5795589de..1fd4b6dfd417 100644
> > --- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> > +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> > @@ -40,6 +40,12 @@ properties:
> >    interrupts:
> >      maxItems: 1
> >  
> > +  sdma-io-width:
> > +    description: Slave DMA data port I/O width (in bytes)
> 
> Waht is SDMA? What is Slave DMA? The same?
> 
> Why this cannot be deduced from the compatible? DMA is fixed per
> platform and SDMA do not sound like SPI device (so slave in older
> terminology) property.

in cdns xspi, sdma == slave dma. It can be read by any master, such
as DMA engine or CPU. Its purpose and usage is similar as the data
reg in other spi controllers, e.g the DW_SPI_DR in dw spi. 
> 
> Anyway, there is generic property reg-io-width if this is for MMIO,
> bus-width and spi-tx/rx-bus-width.

Indeed, spi dw driver uses reg-io-width to specify the io width which
is for accessing DW_SPI_DR. But I have one question: the "sdma" name
is from cdns xspi HW, I'm not sure whether reg-io-width is better. If
you prefer reg-io-width, I will do it in v2

Thanks

Re: [PATCH 1/2] spi: dt-bindings: cdns,xspi: add sdma-io-width

[Krzysztof Kozlowski]

On 20/05/2026 01:38, Jisheng Zhang wrote:
> On Fri, May 15, 2026 at 10:00:50AM +0200, Krzysztof Kozlowski wrote:
>> On Mon, May 11, 2026 at 11:17:31AM +0800, Jisheng Zhang wrote:
>>> The cdns xspi controller SDMA data port may support wider I/O width.
>>> Wider I/O width can benefit performance. A simple test with QSPI nor
>>> flash on one arm64 platform:
>>>
>>> 1 byte io width (default):
>>>  # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
>>>  1000+0 records in
>>>  1000+0 records out
>>>  8192000 bytes (7.8MB) copied, 1.368735 seconds, 5.7MB/s
>>>
>>> 4 bytes io width:
>>>  # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
>>>  1000+0 records in
>>>  1000+0 records out
>>>  8192000 bytes (7.8MB) copied, 1.088787 seconds, 7.2MB/s
>>>
>>> Improved by 26.3%!
>>>
>>> Add dt binding for sdma-io-width.
>>>
>>> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
>>> ---
>>>  Documentation/devicetree/bindings/spi/cdns,xspi.yaml | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
>>> index 38a5795589de..1fd4b6dfd417 100644
>>> --- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
>>> +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
>>> @@ -40,6 +40,12 @@ properties:
>>>    interrupts:
>>>      maxItems: 1
>>>  
>>> +  sdma-io-width:
>>> +    description: Slave DMA data port I/O width (in bytes)
>>
>> Waht is SDMA? What is Slave DMA? The same?
>>
>> Why this cannot be deduced from the compatible? DMA is fixed per
>> platform and SDMA do not sound like SPI device (so slave in older
>> terminology) property.
> 
> in cdns xspi, sdma == slave dma. It can be read by any master, such
> as DMA engine or CPU. Its purpose and usage is similar as the data
> reg in other spi controllers, e.g the DW_SPI_DR in dw spi. 

You did not really answer the questions.

Best regards,
Krzysztof

Re: [PATCH 1/2] spi: dt-bindings: cdns,xspi: add sdma-io-width

[Jisheng Zhang]

On Wed, May 20, 2026 at 09:09:55AM +0200, Krzysztof Kozlowski wrote:
> On 20/05/2026 01:38, Jisheng Zhang wrote:
> > On Fri, May 15, 2026 at 10:00:50AM +0200, Krzysztof Kozlowski wrote:
> >> On Mon, May 11, 2026 at 11:17:31AM +0800, Jisheng Zhang wrote:
> >>> The cdns xspi controller SDMA data port may support wider I/O width.
> >>> Wider I/O width can benefit performance. A simple test with QSPI nor
> >>> flash on one arm64 platform:
> >>>
> >>> 1 byte io width (default):
> >>>  # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
> >>>  1000+0 records in
> >>>  1000+0 records out
> >>>  8192000 bytes (7.8MB) copied, 1.368735 seconds, 5.7MB/s
> >>>
> >>> 4 bytes io width:
> >>>  # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
> >>>  1000+0 records in
> >>>  1000+0 records out
> >>>  8192000 bytes (7.8MB) copied, 1.088787 seconds, 7.2MB/s
> >>>
> >>> Improved by 26.3%!
> >>>
> >>> Add dt binding for sdma-io-width.
> >>>
> >>> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> >>> ---
> >>>  Documentation/devicetree/bindings/spi/cdns,xspi.yaml | 6 ++++++
> >>>  1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> >>> index 38a5795589de..1fd4b6dfd417 100644
> >>> --- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> >>> +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> >>> @@ -40,6 +40,12 @@ properties:
> >>>    interrupts:
> >>>      maxItems: 1
> >>>  
> >>> +  sdma-io-width:
> >>> +    description: Slave DMA data port I/O width (in bytes)
> >>
> >> Waht is SDMA? What is Slave DMA? The same?
> >>
> >> Why this cannot be deduced from the compatible? DMA is fixed per
> >> platform and SDMA do not sound like SPI device (so slave in older
> >> terminology) property.
> > 
> > in cdns xspi, sdma == slave dma. It can be read by any master, such
> > as DMA engine or CPU. Its purpose and usage is similar as the data
> > reg in other spi controllers, e.g the DW_SPI_DR in dw spi. 
> 
> You did not really answer the questions.
> 

If you mean "Why this cannot be deduced from the compatible?", I think
the slave dma port is part of the cdns xspi, so its io width belongs
to xSPI device property.
From another side, we have seen such property in other drivers such as
the reg-io-width for the dw spi DW_SPI_DR port io width.

Thanks

Re: [PATCH 1/2] spi: dt-bindings: cdns,xspi: add sdma-io-width

[Krzysztof Kozlowski]

On 20/05/2026 13:48, Jisheng Zhang wrote:
> On Wed, May 20, 2026 at 09:09:55AM +0200, Krzysztof Kozlowski wrote:
>> On 20/05/2026 01:38, Jisheng Zhang wrote:
>>> On Fri, May 15, 2026 at 10:00:50AM +0200, Krzysztof Kozlowski wrote:
>>>> On Mon, May 11, 2026 at 11:17:31AM +0800, Jisheng Zhang wrote:
>>>>> The cdns xspi controller SDMA data port may support wider I/O width.
>>>>> Wider I/O width can benefit performance. A simple test with QSPI nor
>>>>> flash on one arm64 platform:
>>>>>
>>>>> 1 byte io width (default):
>>>>>  # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
>>>>>  1000+0 records in
>>>>>  1000+0 records out
>>>>>  8192000 bytes (7.8MB) copied, 1.368735 seconds, 5.7MB/s
>>>>>
>>>>> 4 bytes io width:
>>>>>  # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
>>>>>  1000+0 records in
>>>>>  1000+0 records out
>>>>>  8192000 bytes (7.8MB) copied, 1.088787 seconds, 7.2MB/s
>>>>>
>>>>> Improved by 26.3%!
>>>>>
>>>>> Add dt binding for sdma-io-width.
>>>>>
>>>>> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/spi/cdns,xspi.yaml | 6 ++++++
>>>>>  1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
>>>>> index 38a5795589de..1fd4b6dfd417 100644
>>>>> --- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
>>>>> +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
>>>>> @@ -40,6 +40,12 @@ properties:
>>>>>    interrupts:
>>>>>      maxItems: 1
>>>>>  
>>>>> +  sdma-io-width:
>>>>> +    description: Slave DMA data port I/O width (in bytes)
>>>>
>>>> Waht is SDMA? What is Slave DMA? The same?
>>>>
>>>> Why this cannot be deduced from the compatible? DMA is fixed per
>>>> platform and SDMA do not sound like SPI device (so slave in older
>>>> terminology) property.
>>>
>>> in cdns xspi, sdma == slave dma. It can be read by any master, such
>>> as DMA engine or CPU. Its purpose and usage is similar as the data
>>> reg in other spi controllers, e.g the DW_SPI_DR in dw spi. 
>>
>> You did not really answer the questions.
>>
> 
> If you mean "Why this cannot be deduced from the compatible?", I think
> the slave dma port is part of the cdns xspi, so its io width belongs
> to xSPI device property.
> From another side, we have seen such property in other drivers such as
> the reg-io-width for the dw spi DW_SPI_DR port io width.

So you mean it depends on SPI device? Then why spi-peripheral-props is
not applicable here?

If this is not bus width, but DMA-something, is not really then SPI
device dependent, but rather DMA controller limitation, so either
deducible from compatible or something else is missing here.

Argument that some drivers do that is obviously wrong, so I am jumping
there.

Best regards,
Krzysztof

Re: [PATCH 1/2] spi: dt-bindings: cdns,xspi: add sdma-io-width

[Jisheng Zhang]

On Wed, May 20, 2026 at 02:16:21PM +0200, Krzysztof Kozlowski wrote:
> On 20/05/2026 13:48, Jisheng Zhang wrote:
> > On Wed, May 20, 2026 at 09:09:55AM +0200, Krzysztof Kozlowski wrote:
> >> On 20/05/2026 01:38, Jisheng Zhang wrote:
> >>> On Fri, May 15, 2026 at 10:00:50AM +0200, Krzysztof Kozlowski wrote:
> >>>> On Mon, May 11, 2026 at 11:17:31AM +0800, Jisheng Zhang wrote:
> >>>>> The cdns xspi controller SDMA data port may support wider I/O width.
> >>>>> Wider I/O width can benefit performance. A simple test with QSPI nor
> >>>>> flash on one arm64 platform:
> >>>>>
> >>>>> 1 byte io width (default):
> >>>>>  # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
> >>>>>  1000+0 records in
> >>>>>  1000+0 records out
> >>>>>  8192000 bytes (7.8MB) copied, 1.368735 seconds, 5.7MB/s
> >>>>>
> >>>>> 4 bytes io width:
> >>>>>  # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
> >>>>>  1000+0 records in
> >>>>>  1000+0 records out
> >>>>>  8192000 bytes (7.8MB) copied, 1.088787 seconds, 7.2MB/s
> >>>>>
> >>>>> Improved by 26.3%!
> >>>>>
> >>>>> Add dt binding for sdma-io-width.
> >>>>>
> >>>>> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> >>>>> ---
> >>>>>  Documentation/devicetree/bindings/spi/cdns,xspi.yaml | 6 ++++++
> >>>>>  1 file changed, 6 insertions(+)
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> >>>>> index 38a5795589de..1fd4b6dfd417 100644
> >>>>> --- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> >>>>> +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> >>>>> @@ -40,6 +40,12 @@ properties:
> >>>>>    interrupts:
> >>>>>      maxItems: 1
> >>>>>  
> >>>>> +  sdma-io-width:
> >>>>> +    description: Slave DMA data port I/O width (in bytes)
> >>>>
> >>>> Waht is SDMA? What is Slave DMA? The same?
> >>>>
> >>>> Why this cannot be deduced from the compatible? DMA is fixed per
> >>>> platform and SDMA do not sound like SPI device (so slave in older
> >>>> terminology) property.
> >>>
> >>> in cdns xspi, sdma == slave dma. It can be read by any master, such
> >>> as DMA engine or CPU. Its purpose and usage is similar as the data
> >>> reg in other spi controllers, e.g the DW_SPI_DR in dw spi. 
> >>
> >> You did not really answer the questions.
> >>
> > 
> > If you mean "Why this cannot be deduced from the compatible?", I think
> > the slave dma port is part of the cdns xspi, so its io width belongs
> > to xSPI device property.
> > From another side, we have seen such property in other drivers such as
> > the reg-io-width for the dw spi DW_SPI_DR port io width.
> 
> So you mean it depends on SPI device? Then why spi-peripheral-props is

Nope, it depends on the spi controller itself.

> not applicable here?
> 
> If this is not bus width, but DMA-something, is not really then SPI
> device dependent, but rather DMA controller limitation, so either

The "sdma" name is a bit misleading but that's from the hw. It's
a data io port in fact. Any SPI controller has this data io reg or
port. It's not DMA controller at all. IMHO, what io width
may be supported is determined by the setting during IP configuration. 

So from this point of view, is it SPI controller property?

> deducible from compatible or something else is missing here.
> 
> Argument that some drivers do that is obviously wrong, so I am jumping
> there.
> 
> Best regards,
> Krzysztof

Re: [PATCH 1/2] spi: dt-bindings: cdns,xspi: add sdma-io-width

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1330 bytes --]

On Wed, May 20, 2026 at 02:16:21PM +0200, Krzysztof Kozlowski wrote:
> On 20/05/2026 13:48, Jisheng Zhang wrote:

> > If you mean "Why this cannot be deduced from the compatible?", I think
> > the slave dma port is part of the cdns xspi, so its io width belongs
> > to xSPI device property.
> > From another side, we have seen such property in other drivers such as
> > the reg-io-width for the dw spi DW_SPI_DR port io width.

> So you mean it depends on SPI device? Then why spi-peripheral-props is
> not applicable here?

That will be controller side, not device side.

> If this is not bus width, but DMA-something, is not really then SPI
> device dependent, but rather DMA controller limitation, so either
> deducible from compatible or something else is missing here.

My understanding is that this is a connection between the SPI and DMA
controllers so it's not as obvious as it could be which side of that
link should have the property, eg:

  https://www.cadence.com/content/dam/cadence-www/global/en_US/documents/tools/silicon-solutions/design-ip/controller-ip-qspi.pdf

shows a separate direct connection between the DMA controller and the
xSPI controller, the DMA controller isn't interacting with registers on
the CPU visible buses.  The width is probably a design time configurable
option on both sides of the link.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/2] spi: dt-bindings: cdns,xspi: add sdma-io-width

[Krzysztof Kozlowski]

On 20/05/2026 14:30, Mark Brown wrote:
> On Wed, May 20, 2026 at 02:16:21PM +0200, Krzysztof Kozlowski wrote:
>> On 20/05/2026 13:48, Jisheng Zhang wrote:
> 
>>> If you mean "Why this cannot be deduced from the compatible?", I think
>>> the slave dma port is part of the cdns xspi, so its io width belongs
>>> to xSPI device property.
>>> From another side, we have seen such property in other drivers such as
>>> the reg-io-width for the dw spi DW_SPI_DR port io width.
> 
>> So you mean it depends on SPI device? Then why spi-peripheral-props is
>> not applicable here?
> 
> That will be controller side, not device side.
> 
>> If this is not bus width, but DMA-something, is not really then SPI
>> device dependent, but rather DMA controller limitation, so either
>> deducible from compatible or something else is missing here.
> 
> My understanding is that this is a connection between the SPI and DMA
> controllers so it's not as obvious as it could be which side of that
> link should have the property, eg:
> 
>   https://www.cadence.com/content/dam/cadence-www/global/en_US/documents/tools/silicon-solutions/design-ip/controller-ip-qspi.pdf
> 
> shows a separate direct connection between the DMA controller and the
> xSPI controller, the DMA controller isn't interacting with registers on
> the CPU visible buses.  The width is probably a design time configurable
> option on both sides of the link.

Yes and that sounds a lot specific to particular controller, thus should
be implied by / deducible from the compatible.

Best regards,
Krzysztof

Re: [PATCH 1/2] spi: dt-bindings: cdns,xspi: add sdma-io-width

[Jisheng Zhang]

On Wed, May 20, 2026 at 03:22:07PM +0200, Krzysztof Kozlowski wrote:
> On 20/05/2026 14:30, Mark Brown wrote:
> > On Wed, May 20, 2026 at 02:16:21PM +0200, Krzysztof Kozlowski wrote:
> >> On 20/05/2026 13:48, Jisheng Zhang wrote:
> > 
> >>> If you mean "Why this cannot be deduced from the compatible?", I think
> >>> the slave dma port is part of the cdns xspi, so its io width belongs
> >>> to xSPI device property.
> >>> From another side, we have seen such property in other drivers such as
> >>> the reg-io-width for the dw spi DW_SPI_DR port io width.
> > 
> >> So you mean it depends on SPI device? Then why spi-peripheral-props is
> >> not applicable here?
> > 
> > That will be controller side, not device side.
> > 
> >> If this is not bus width, but DMA-something, is not really then SPI
> >> device dependent, but rather DMA controller limitation, so either
> >> deducible from compatible or something else is missing here.
> > 
> > My understanding is that this is a connection between the SPI and DMA
> > controllers so it's not as obvious as it could be which side of that
> > link should have the property, eg:
> > 
> >   https://www.cadence.com/content/dam/cadence-www/global/en_US/documents/tools/silicon-solutions/design-ip/controller-ip-qspi.pdf
> > 
> > shows a separate direct connection between the DMA controller and the
> > xSPI controller, the DMA controller isn't interacting with registers on
> > the CPU visible buses.  The width is probably a design time configurable
> > option on both sides of the link.
> 
> Yes and that sounds a lot specific to particular controller, thus should
> be implied by / deducible from the compatible.

This is IP feature, so if we couple the IP's feature with platform
compatible, I would see some unnecessary LoCs. For example,
Let's assume the IP has 10 users, they all support 4 bytes io width,
other features are the same. 

If implied by the compatible string, we need to add 10 compatible
string support both in code and dt-bindings.

vs

If supported by "sdma-io-width", nothing is needed after this patch

IMHO, the 2nd sounds better, what do you think?

Thanks

Re: [PATCH 1/2] spi: dt-bindings: cdns,xspi: add sdma-io-width

[Krzysztof Kozlowski]

On 20/05/2026 15:18, Jisheng Zhang wrote:
> On Wed, May 20, 2026 at 03:22:07PM +0200, Krzysztof Kozlowski wrote:
>> On 20/05/2026 14:30, Mark Brown wrote:
>>> On Wed, May 20, 2026 at 02:16:21PM +0200, Krzysztof Kozlowski wrote:
>>>> On 20/05/2026 13:48, Jisheng Zhang wrote:
>>>
>>>>> If you mean "Why this cannot be deduced from the compatible?", I think
>>>>> the slave dma port is part of the cdns xspi, so its io width belongs
>>>>> to xSPI device property.
>>>>> From another side, we have seen such property in other drivers such as
>>>>> the reg-io-width for the dw spi DW_SPI_DR port io width.
>>>
>>>> So you mean it depends on SPI device? Then why spi-peripheral-props is
>>>> not applicable here?
>>>
>>> That will be controller side, not device side.
>>>
>>>> If this is not bus width, but DMA-something, is not really then SPI
>>>> device dependent, but rather DMA controller limitation, so either
>>>> deducible from compatible or something else is missing here.
>>>
>>> My understanding is that this is a connection between the SPI and DMA
>>> controllers so it's not as obvious as it could be which side of that
>>> link should have the property, eg:
>>>
>>>   https://www.cadence.com/content/dam/cadence-www/global/en_US/documents/tools/silicon-solutions/design-ip/controller-ip-qspi.pdf
>>>
>>> shows a separate direct connection between the DMA controller and the
>>> xSPI controller, the DMA controller isn't interacting with registers on
>>> the CPU visible buses.  The width is probably a design time configurable
>>> option on both sides of the link.
>>
>> Yes and that sounds a lot specific to particular controller, thus should
>> be implied by / deducible from the compatible.
> 
> This is IP feature, so if we couple the IP's feature with platform
> compatible, I would see some unnecessary LoCs. For example,
> Let's assume the IP has 10 users, they all support 4 bytes io width,
> other features are the same. 
> 
> If implied by the compatible string, we need to add 10 compatible
> string support both in code and dt-bindings.

You always need 10 compatible strings in bindings.

But driver would need only one, since devices are compatible as you
described.

> 
> vs
> 
> If supported by "sdma-io-width", nothing is needed after this patch
> 
> IMHO, the 2nd sounds better, what do you think?

We answered this in writing bindings. Properties are not replacement for
specific compatible. I don't know how to write that sentence in bindings
clearer - it's exactly this case.

Best regards,
Krzysztof

[PATCH 2/2] spi: cadence-xspi: support 4bytes sdma-io-width

[Jisheng Zhang]

The cdns xspi controller SDMA data port may support wider I/O width.
Wider I/O width can benefit performance. A simple test with QSPI nor
flash on one arm64 platform:

1 byte io width (default):
 # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
 1000+0 records in
 1000+0 records out
 8192000 bytes (7.8MB) copied, 1.368735 seconds, 5.7MB/s

4 bytes io width:
 # dd if=/dev/mtdblock0 of=/dev/null bs=8192 count=1000
 1000+0 records in
 1000+0 records out
 8192000 bytes (7.8MB) copied, 1.088787 seconds, 7.2MB/s

Improved by 26.3%!

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 drivers/spi/spi-cadence-xspi.c | 43 ++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
index 895b4b3276a5..c1fb749540e7 100644
--- a/drivers/spi/spi-cadence-xspi.c
+++ b/drivers/spi/spi-cadence-xspi.c
@@ -369,6 +369,7 @@ struct cdns_xspi_dev {
 
 	void *in_buffer;
 	const void *out_buffer;
+	u32 sdma_io_width;
 
 	u8 hw_num_banks;
 
@@ -578,6 +579,38 @@ static int cdns_xspi_controller_init(struct cdns_xspi_dev *cdns_xspi)
 	return 0;
 }
 
+static inline void cdns_xspi_sdma_read(struct cdns_xspi_dev *cdns_xspi, size_t len)
+{
+	void __iomem *src = cdns_xspi->sdmabase;
+	void *buf = cdns_xspi->in_buffer;
+	size_t offset = 0;
+
+	if (cdns_xspi->sdma_io_width == 4) {
+		if (IS_ALIGNED((uintptr_t)src, 4) && IS_ALIGNED((uintptr_t)buf, 4)) {
+			ioread32_rep(src, buf, len >> 2);
+			offset = len & ~0x3;
+			len -= offset;
+		}
+	}
+	ioread8_rep(src, (u8 *)buf + offset, len);
+}
+
+static inline void cdns_xspi_sdma_write(struct cdns_xspi_dev *cdns_xspi, size_t len)
+{
+	void __iomem *dst = cdns_xspi->sdmabase;
+	const void *buf = cdns_xspi->out_buffer;
+	size_t offset = 0;
+
+	if (cdns_xspi->sdma_io_width == 4) {
+		if (IS_ALIGNED((uintptr_t)dst, 4) && IS_ALIGNED((uintptr_t)buf, 4)) {
+			iowrite32_rep(dst, buf, len >> 2);
+			offset = len & ~0x3;
+			len -= offset;
+		}
+	}
+	iowrite8_rep(dst, (const u8 *)buf + offset, len);
+}
+
 static void cdns_xspi_sdma_handle(struct cdns_xspi_dev *cdns_xspi)
 {
 	u32 sdma_size, sdma_trd_info;
@@ -589,13 +622,11 @@ static void cdns_xspi_sdma_handle(struct cdns_xspi_dev *cdns_xspi)
 
 	switch (sdma_dir) {
 	case CDNS_XSPI_SDMA_DIR_READ:
-		ioread8_rep(cdns_xspi->sdmabase,
-			    cdns_xspi->in_buffer, sdma_size);
+		cdns_xspi_sdma_read(cdns_xspi, sdma_size);
 		break;
 
 	case CDNS_XSPI_SDMA_DIR_WRITE:
-		iowrite8_rep(cdns_xspi->sdmabase,
-			     cdns_xspi->out_buffer, sdma_size);
+		cdns_xspi_sdma_write(cdns_xspi, sdma_size);
 		break;
 	}
 }
@@ -1215,6 +1246,10 @@ static int cdns_xspi_probe(struct platform_device *pdev)
 		}
 	}
 
+	if (device_property_read_u32(&pdev->dev, "sdma-io-width",
+				     &cdns_xspi->sdma_io_width))
+		cdns_xspi->sdma_io_width = 1;
+
 	cdns_xspi->irq = platform_get_irq(pdev, 0);
 	if (cdns_xspi->irq < 0)
 		return -ENXIO;
-- 
2.53.0

Re: [PATCH 2/2] spi: cadence-xspi: support 4bytes sdma-io-width

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Mixing 32-bit and 8-bit stream accessors on the same SDMA port causes data corruption on big-endian systems and potential bus faults.
- [Low] Casting an __iomem pointer directly to uintptr_t drops the __iomem annotation and the alignment check on sdmabase is redundant.
--

commit e78d668225027044d0694ef2be9788b6fddb0ef3
Author: Jisheng Zhang <jszhang@kernel.org>

spi: cadence-xspi: support 4bytes sdma-io-width

This commit adds support for 4-byte SDMA I/O width to the Cadence XSPI
controller. It introduces new read and write functions that utilize 32-bit
I/O accessors when the buffer is aligned and the hardware width is configured
to 4 bytes.

> diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
> index 895b4b3276a56..c1fb749540e7a 100644
> --- a/drivers/spi/spi-cadence-xspi.c
> +++ b/drivers/spi/spi-cadence-xspi.c

[ ... ]

> @@ -578,6 +579,38 @@ static int cdns_xspi_controller_init(struct cdns_xspi_dev *cdns_xspi)
>  	return 0;
>  }
>  
> +static inline void cdns_xspi_sdma_read(struct cdns_xspi_dev *cdns_xspi, size_t len)
> +{
> +	void __iomem *src = cdns_xspi->sdmabase;
> +	void *buf = cdns_xspi->in_buffer;
> +	size_t offset = 0;
> +
> +	if (cdns_xspi->sdma_io_width == 4) {
> +		if (IS_ALIGNED((uintptr_t)src, 4) && IS_ALIGNED((uintptr_t)buf, 4)) {

This isn't a bug, but does casting the __iomem pointer to uintptr_t drop the
address space annotation and trigger a sparse warning?

Since sdmabase is mapped via devm_ioremap_resource, is the alignment check on
src redundant?

> +			ioread32_rep(src, buf, len >> 2);
> +			offset = len & ~0x3;
> +			len -= offset;
> +		}
> +	}
> +	ioread8_rep(src, (u8 *)buf + offset, len);

Can mixing 32-bit and 8-bit stream accessors on the same SDMA port cause data
corruption on big-endian architectures?

On big-endian systems, reading memory in 32-bit native words places bytes in a
different order than processing them sequentially in 8-bit chunks.

Additionally, if the hardware port is explicitly configured for 32-bit I/O,
could falling back to 8-bit accessors for remainders or unaligned buffers
trigger bus faults?

Using 32-bit stream accessors exclusively with a temporary 32-bit variable and
memcpy for unaligned buffers or remainders would resolve this.

> +}
> +
> +static inline void cdns_xspi_sdma_write(struct cdns_xspi_dev *cdns_xspi, size_t len)
> +{
> +	void __iomem *dst = cdns_xspi->sdmabase;
> +	const void *buf = cdns_xspi->out_buffer;
> +	size_t offset = 0;
> +
> +	if (cdns_xspi->sdma_io_width == 4) {
> +		if (IS_ALIGNED((uintptr_t)dst, 4) && IS_ALIGNED((uintptr_t)buf, 4)) {
> +			iowrite32_rep(dst, buf, len >> 2);
> +			offset = len & ~0x3;
> +			len -= offset;
> +		}
> +	}
> +	iowrite8_rep(dst, (const u8 *)buf + offset, len);

Does the same accessor mixing issue apply here when writing to the SDMA port?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511031732.3199-1-jszhang@kernel.org?part=2