回复: 回复: [PATCH 1/3] dt-bindings: spi: spi-cadence: document optional fifo-width DT property

[Jun Guo]

On Tue, Oct 02, 2025 at 02:04:00AM +0000, Conor Dooley wrote:
>On Wed, Oct 01, 2025 at 02:36:44PM +0000, Jun Guo wrote:
> > On Tue, Oct 01, 2025 at 02:52:00AM +0800, Conor Dooley wrote:
> > > On Tue, Sep 30, 2025 at 03:56:42PM +0800, Jun Guo wrote:
> > > > Add documentation for the optional 'fifo-width' device tree property
> > > > for the Cadence SPI controller.
> > > >
> > > > Signed-off-by: Jun Guo <jun.guo at cixtech.com>
> > > > ---
> > > >  .../devicetree/bindings/spi/spi-cadence.yaml          | 11 +++++++++++
> > > >  1 file changed, 11 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/spi/spi-cadence.yaml b/Documentation/devicetree/bindings/spi/spi-cadence.yaml
> > > > index 8de96abe9da1..b2e3f217473b 100644
> > > > --- a/Documentation/devicetree/bindings/spi/spi-cadence.yaml
> > > > +++ b/Documentation/devicetree/bindings/spi/spi-cadence.yaml
> > > > @@ -62,6 +62,17 @@ properties:
> > > >      items:
> > > >        - const: spi
> > > >
> > > > +  fifo-width:
> > > > +    description: |
> > > > +      This property specifies the FIFO data width (in bits) of the hardware.
> > > > +      It must be configured according to the actual FIFO width set during
> > > > +      the IP design. For instance, if the hardware FIFO is 32 bits wide,
> > > > +      this property should be set to 32.
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    minimum: 8
> > > > +    maximum: 32
> > > > +    default: 8
> > >
> > > I assume this differs from fifo-depth because this is the actual width
> > > of the registers rather than the number of elements of that width the
> > > FIFO can contain?
> >
> > Thank you for your review. You are absolutely correct. The `fifo-width`
> > indeed refers to the physical width of the FIFO data registers (e.g., 8,
> > 16, or 32 bits), whereas `fifo-depth` describes how many elements of
> >  that width the FIFO can store.
> >
> > > However, this isn't something defined as common in spi-controller.yaml
> > > so you'll need a vendor prefix for the property if the property stays.
> > > This does, however, seem like something that can just be determined by
> > > the compatible and that your omission of a soc-specific one is what's
> > > lead you to introduce this property. Why not just use a sky1-specific
> > > compatible here?
> >
> > You raise an excellent point, and I initially had the same thought. However,
> > after further consideration, I realized that the IP of Cadence SPI actually
> > supports configurable FIFO width as a feature. The choice of using 8-bit,
> > 16-bit, or 32-bit FIFO width can be made by the SoC integrator based on
> > their specific requirements. This is therefore a feature of the Cadence IP
> > itself, rather than a chip vendor-specific design constraint.
> >
> > For this reason, I believe defining a common `fifo-width` property for
> > Cadence SPI controllers is more appropriate, as it allows any SoC using
> > this IP with different FIFO width configurations to utilize this property,
> > without needing to create a specific compatible string for each SoC variant.

>Except, you do need to create a soc-specific compatible string for every
> device, the fact that you didn't add one for your sky1 SoC was a mistake
> that you should fix. SoC-specific compatibles are a requirement.
> The "cnds,spi-r1p6" compatible seems to be used on Xilinx platforms,
> including a zynq platform that should probably be using the zynq
> soc-specific compatible. r1p6 sounds like some sort of version info, is
> that the version you are even using?

> Once you have added a compatible for the sky1, this property is not
> needed, since the depth can be determined from that. Any other user that
> wants to use non-default depths can also use their soc-specific
> compatibles for that purpose.

Thank you for the further explanation and suggestions. I see your
point now, and I will rethink the approach for this patch.

> >
> > Thank you for your valuable time and insightful suggestions. I look forward to
> > your further feedback on this approach.

Best wishes,
Jun Guo



________________________________________
发件人: Conor Dooley <conor at kernel.org>
发送时间: 2025年10月2日 2:04
收件人: Jun Guo
抄送: Peter Chen; Fugang Duan; robh at kernel.org; krzk+dt at kernel.org; conor+dt at kernel.org; broonie at kernel.org; linux-spi at vger.kernel.org; michal.simek at amd.com; cix-kernel-upstream; linux-arm-kernel at lists.infradead.org; devicetree at vger.kernel.org; linux-kernel at vger.kernel.org
主题: Re: 回复: [PATCH 1/3] dt-bindings: spi: spi-cadence: document optional fifo-width DT property

EXTERNAL EMAIL