RE: [PATCH v4 1/2] dt-bindings: PCI: xilinx-cpm: Add reset-gpios for PCIe RP PERST#

[Musham, Sai Krishna]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
> Sent: Tuesday, March 18, 2025 3:23 PM
> To: Musham, Sai Krishna <sai.krishna.musham@xxxxxxx>;
> bhelgaas@xxxxxxxxxx; lpieralisi@xxxxxxxxxx; kw@xxxxxxxxx;
> manivannan.sadhasivam@xxxxxxxxxx; robh@xxxxxxxxxx; krzk+dt@xxxxxxxxxx;
> conor+dt@xxxxxxxxxx; cassel@xxxxxxxxxx
> Cc: linux-pci@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Simek, Michal <michal.simek@xxxxxxx>; Gogada, Bharat
> Kumar <bharat.kumar.gogada@xxxxxxx>; Havalige, Thippeswamy
> <thippeswamy.havalige@xxxxxxx>
> Subject: Re: [PATCH v4 1/2] dt-bindings: PCI: xilinx-cpm: Add reset-gpios for PCIe
> RP PERST#
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On 18/03/2025 10:26, Sai Krishna Musham wrote:
> > Changes for v2:
> > - Add define from include/dt-bindings/gpio/gpio.h for PERST# polarity
> > - Update commit message
> > ---
> >  .../bindings/pci/xilinx-versal-cpm.yaml       | 21 ++++++++++++++-----
> >  1 file changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> > b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> > index d674a24c8ccc..904594138af2 100644
> > --- a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> > +++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> > @@ -24,15 +24,20 @@ properties:
> >      items:
> >        - description: CPM system level control and status registers.
> >        - description: Configuration space region and bridge registers.
> > +      - description: CPM clock and reset control registers.
> >        - description: CPM5 control and status registers.
>
> You cannot add items to the middle, that's an ABI break. Adding required properties
> is also an ABI break. Why you cannot add it to the end of the list?
>
> Or at least explain ABI break impact in commit msg?
>
When I add property at the end, I'm observing failure during dt_binding_check.
$ make DT_CHECKER_FLAGS=-m dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
Documentation/devicetree/bindings/pci/xilinx-versal-cpm.example.dtb: pcie@fca10000: reg-names:2: 'cpm_csr' was expected
        from schema $id: http://devicetree.org/schemas/pci/xilinx-versal-cpm.yaml#
>
> > -    minItems: 2
> > +    minItems: 3
> >
> >    reg-names:
> >      items:
> >        - const: cpm_slcr
> >        - const: cfg
> > +      - const: cpm_crx
> >        - const: cpm_csr
> > -    minItems: 2
> > +    minItems: 3
> > +
> > +  reset-gpios:
> > +    description: GPIO used as PERST# signal
>
> Isn't this already in pci-bus-common.yaml?
>
>
> Best regards,
> Krzysztof