Re: [PATCH 1/8] dt-bindings: pci: xilinx-nwl: Add resets

[Manivannan Sadhasivam]

On Thu, Feb 05, 2026 at 10:47:21AM -0500, Sean Anderson wrote:
> On 2/4/26 03:32, Pandey, Radhey Shyam wrote:
> > [AMD Official Use Only - AMD Internal Distribution Only]
> > 
> >> -----Original Message-----
> >> From: Sean Anderson <sean.anderson@xxxxxxxxx>
> >> Sent: Tuesday, February 3, 2026 5:51 AM
> >> To: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>; Vinod Koul
> >> <vkoul@xxxxxxxxxx>; linux-phy@xxxxxxxxxxxxxxxxxxx
> >> Cc: Krzysztof Wilczyński <kwilczynski@xxxxxxxxxx>; Lorenzo Pieralisi
> >> <lpieralisi@xxxxxxxxxx>; Pandey, Radhey Shyam
> >> <radhey.shyam.pandey@xxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; Simek, Michal
> >> <michal.simek@xxxxxxx>; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> >> pci@xxxxxxxxxxxxxxx; Neil Armstrong <neil.armstrong@xxxxxxxxxx>; Rob Herring
> >> <robh@xxxxxxxxxx>; Havalige, Thippeswamy <thippeswamy.havalige@xxxxxxx>;
> >> Manivannan Sadhasivam <mani@xxxxxxxxxx>; Bjorn Helgaas
> >> <bhelgaas@xxxxxxxxxx>; Sean Anderson <sean.anderson@xxxxxxxxx>; Conor
> >> Dooley <conor+dt@xxxxxxxxxx>; Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>;
> >> devicetree@xxxxxxxxxxxxxxx
> >> Subject: [PATCH 1/8] dt-bindings: pci: xilinx-nwl: Add resets
> >>
> >> Add resets so we can hold the bridge in reset while we perform phy calibration.
> > 
> > Seems like this should a required property?
> 
> It's optional as it does not exist in previous versions of the
> devicetree. In the past I have received pushback against making these
> sort of properties required.
> 
> If the resets don't exist we just don't assert them and assume the
> bootloader has deasserted them.
> 

If the resets are pretty much required for the hardware functionality, we can
mark them as required in the binding and accept the ABI breakage. This scenario
keeps coming with devicetree as the initial devicetree bindings lacked full
hardware description in most of the cases.

- Mani

> --Sean
> 
> > Rest looks fine to me.
> > 
> >>
> >> Signed-off-by: Sean Anderson <sean.anderson@xxxxxxxxx>
> >> ---
> >>
> >>  .../devicetree/bindings/pci/xlnx,nwl-pcie.yaml  | 17 +++++++++++++++++
> >>  1 file changed, 17 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
> >> b/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
> >> index 9de3c09efb6e..7efb3dd9955f 100644
> >> --- a/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
> >> +++ b/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
> >> @@ -69,6 +69,18 @@ properties:
> >>    power-domains:
> >>      maxItems: 1
> >>
> >> +  resets:
> >> +    maxItems: 3
> >> +
> >> +  reset-names:
> >> +    items:
> >> +      - description: APB register block reset
> >> +        const: cfg
> >> +      - description: AXI-PCIe bridge reset
> >> +        const: bridge
> >> +      - description: PCIe MAC reset
> >> +        const: ctrl
> >> +
> >>    iommus:
> >>      maxItems: 1
> >>
> >> @@ -117,6 +129,7 @@ examples:
> >>      #include <dt-bindings/interrupt-controller/irq.h>
> >>      #include <dt-bindings/phy/phy.h>
> >>      #include <dt-bindings/power/xlnx-zynqmp-power.h>
> >> +    #include <dt-bindings/reset/xlnx-zynqmp-resets.h>
> >>      soc {
> >>          #address-cells = <2>;
> >>          #size-cells = <2>;
> >> @@ -146,6 +159,10 @@ examples:
> >>              msi-parent = <&nwl_pcie>;
> >>              phys = <&psgtr 0 PHY_TYPE_PCIE 0 0>;
> >>              power-domains = <&zynqmp_firmware PD_PCIE>;
> >> +            resets = <&zynqmp_reset ZYNQMP_RESET_PCIE_CFG>,
> >> +                     <&zynqmp_reset ZYNQMP_RESET_PCIE_BRIDGE>,
> >> +                     <&zynqmp_reset ZYNQMP_RESET_PCIE_CTRL>;
> >> +            reset-names = "cfg", "bridge", "ctrl";
> >>              iommus = <&smmu 0x4d0>;
> >>              pcie_intc: legacy-interrupt-controller {
> >>                  interrupt-controller;
> >> --
> >> 2.35.1.1320.gc452695387.dirty
> > 

-- 
மணிவண்ணன் சதாசிவம்