[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<IA0PR12MB76999B696A9BA0834644AC71DC6B2@IA0PR12MB7699.namprd12.prod.outlook.com>
Date: Fri, 27 Sep 2024 09:30:57 +0000
From: "Mahapatra, Amit Kumar" <amit.kumar-mahapatra@....com>
To: Conor Dooley <conor@...nel.org>
CC: "broonie@...nel.org" <broonie@...nel.org>, "robh@...nel.org"
<robh@...nel.org>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
"conor+dt@...nel.org" <conor+dt@...nel.org>, "Simek, Michal"
<michal.simek@....com>, "linux-spi@...r.kernel.org"
<linux-spi@...r.kernel.org>, "devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "git (AMD-Xilinx)" <git@....com>,
"amitrkcian2002@...il.com" <amitrkcian2002@...il.com>
Subject: RE: [PATCH] dt-bindings: spi: xilinx: Add clocks & clock-names
properties
Hello Conor,
> > > Subject: Re: [PATCH] dt-bindings: spi: xilinx: Add clocks &
> > > clock-names properties
> > >
> > > On Mon, Sep 23, 2024 at 06:02:42PM +0530, Amit Kumar Mahapatra wrote:
> > > > Include the 'clocks' and 'clock-names' properties in the AXI
> > > > Quad-SPI bindings. When the AXI4-Lite interface is enabled, the
> > > > core operates in legacy mode, maintaining backward compatibility
> > > > with version 1.00, and uses 's_axi_aclk' and 'ext_spi_clk'. For
> > > > the AXI interface, it uses 's_axi4_aclk' and 'ext_spi_clk'.
> > > >
> > > > Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@....com>
> > > > ---
> > > > BRANCH: for-next
> > > > ---
> > > > .../devicetree/bindings/spi/spi-xilinx.yaml | 29 +++++++++++++++++++
> > > > 1 file changed, 29 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/spi/spi-xilinx.yaml
> > > > b/Documentation/devicetree/bindings/spi/spi-xilinx.yaml
> > > > index 4beb3af0416d..9dfec195ecd4 100644
> > > > --- a/Documentation/devicetree/bindings/spi/spi-xilinx.yaml
> > > > +++ b/Documentation/devicetree/bindings/spi/spi-xilinx.yaml
> > > > @@ -12,6 +12,25 @@ maintainers:
> > > > allOf:
> > > > - $ref: spi-controller.yaml#
> > >
> > > Please move the allOf block down to the end of the binding, after
> > > the property definitions.
> >
> > Sure, I'll take care of it in the next series
> > >
> > > > + - if:
> > > > + properties:
> > > > + compatible:
> > > > + contains:
> > > > + const: xlnx,axi-quad-spi-1.00.a
> > > > + then:
> > > > + properties:
> > > > + clock-names:
> > > > + items:
> > > > + - const: s_axi_aclk
> > > > + - const: ext_spi_clk
> > >
> > > These are all clocks, there should be no need to have "clk" in the names.
> >
> > These are the names exported by the IP and used by the DTG.
>
> So? This is a binding, not a verilog file.
Axi Quad SPI is an FPGA-based IP, and the clock names are derived from the
IP signal names as specified in the IP documentation [1].
We chose these names to ensure alignment with the I/O signal names listed
in Table 2-2 on page 19 of [1].
[1] chrome-extension://efaidnbmnnnibpcajpcglclefindmkaj/https://www.amd.com/content/dam/xilinx/support/documents/ip_documentation/axi_quad_spi/v3_2/pg153-axi-quad-spi.pdf
>
> > > > +
> > > > + else:
> > > > + properties:
> > > > + clock-names:
> > > > + items:
> > > > + - const: s_axi4_aclk
> > > > + - const: ext_spi_clk
> > > > +
> > > > properties:
> > > > compatible:
> > > > enum:
> > > > @@ -25,6 +44,12 @@ properties:
> > > > interrupts:
> > > > maxItems: 1
> > > >
> > > > + clocks:
> > > > + maxItems: 2
> > > > +
> > > > + clock-names:
> > > > + maxItems: 2
> > > > +
> > > > xlnx,num-ss-bits:
> > > > description: Number of chip selects used.
> > > > minimum: 1
> > > > @@ -39,6 +64,8 @@ required:
> > > > - compatible
> > > > - reg
> > > > - interrupts
> > > > + - clocks
> > > > + - clock-names
> > >
> > > New required properties are an ABI break, where is the driver patch
> > > that makes use of these clocks?
> >
> > Alright, I will remove these from the required properties to avoid
> > breaking the ABI. We're working on the driver patch and will send it
> > once it's ready.
>
> What changed to make the clocks needed now? It's possible that making them
> required is the correct thing to do, so breaking the ABI would be justified (provided
> the driver can still handle there being no clocks).
Axi Quad SPI is an FPGA-based IP that was previously tested on MicroBlaze
soft-core systems, where the driver didn't need to enable the clock, as it
would already be enabled before the PL is loaded. However, when used
with ARM hard-core SoCs, the driver must explicitly enable the clocks,
making it necessary to provide the clock information.
Regards,
Amit
Powered by blists - more mailing lists