Re: [PATCH 1/2] spi: dt-bindings: cdns,xspi: add sdma-io-width
From: Jisheng Zhang
Date: Sat May 30 2026 - 08:55:26 EST
On Wed, May 20, 2026 at 04:21:05PM +0200, Krzysztof Kozlowski wrote:
> 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.
Hi all,
Update: after carefully reading the registers' document, I found the
slave dma data width can be known by checking CTRL_FEATURES_REG's
DMA_DATA_WIDTH bit, we can don't need the DT property any more. I'm
cooking v2.
Thanks for all the review comments
>
> >
> > 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