linux-kernel - Re: [PATCH] dt-bindings: spi: xilinx: Add clocks & clock-names 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: <20240925-trapdoor-stunt-33516665fdc5@spud>
Date: Wed, 25 Sep 2024 13:46:40 +0100
From: Conor Dooley <conor@...nel.org>
To: "Mahapatra, Amit Kumar" <amit.kumar-mahapatra@....com>
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

On Wed, Sep 25, 2024 at 11:35:56AM +0000, Mahapatra, Amit Kumar wrote:
> Hello Conor,
> 
> 
> > -----Original Message-----
> > From: Conor Dooley <conor@...nel.org>
> > Sent: Tuesday, September 24, 2024 10:07 PM
> > To: Mahapatra, Amit Kumar <amit.kumar-mahapatra@....com>
> > Cc: broonie@...nel.org; robh@...nel.org; krzk+dt@...nel.org;
> > conor+dt@...nel.org; Simek, Michal <michal.simek@....com>; linux-
> > spi@...r.kernel.org; devicetree@...r.kernel.org; linux-arm-
> > kernel@...ts.infradead.org; linux-kernel@...r.kernel.org; git (AMD-Xilinx)
> > <git@....com>; amitrkcian2002@...il.com
> > 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.

> > > +
> > > +    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).

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)