[PATCH v4 03/13] dt-bindings: PCI: snps,dw-pcie-ep: Add tx_int{a,b,c,d} legacy irqs

[Niklas Cassel]

On Wed, Jun 05, 2024 at 01:04:02PM +0530, Manivannan Sadhasivam wrote:
> On Wed, May 29, 2024 at 10:28:57AM +0200, Niklas Cassel wrote:
> > The DWC core has four interrupt signals: tx_inta, tx_intb, tx_intc, tx_intd
> > that are triggered when the PCIe controller (when running in Endpoint mode)
> > has sent an Assert_INTA Message to the upstream device.
> >
> > Some DWC controllers have these interrupt in a combined interrupt signal.
> >
> > Add the description of these interrupts to the device tree binding.
> >
> > Signed-off-by: Niklas Cassel <cassel at kernel.org>
>
> Nit: We recently changed the driver instances of 'LEGACY' to 'INTX'. But the
> binding it still using 'legacy'. Considering that the 'legacy' IRQ added to the
> RC binding recently (ebce9f6623a7), should we rename it?
>
> This will force the driver to support both 'legacy' and 'intx' for backwards
> compatibility.

I don't think this is true.


Look at snps,dw-pcie.yaml in 6.10-rc2:

The individual interrupts are called:
            Legacy A/B/C/D interrupt signal. Basically it's triggered by
            receiving a Assert_INT{A,B,C,D}/Desassert_INT{A,B,C,D} message
            from the downstream device.
          pattern: "^int(a|b|c|d)$"

The combined interrupt is called:
            Combined Legacy A/B/C/D interrupt signal. See "^int(a|b|c|d)$" for
            details.
          const: legacy

So you use 'inta', 'intb', 'intc', 'intd' if your SoC has a dedicated
interrupt line for each of these irqs.

If the SoC simply has a single combined interrupt line for these irqs,
then you use 'legacy'


This patch simply adds:
'tx_inta', 'tx_intb', 'tx_intc', 'tx_intd' as individual interrupts
and the combined interrupt 'legacy' to snps,dw-pcie-ep.yaml.


Patch ebce9f6623a7 simply allowed the combined interrupt line 'legacy'
to be used by the rockchip-dw-pcie.yaml binding.
This is because the way that device tree is designed. You need to specify
something both in the generic binding (which specifies everything),
and in the glue driver binding, to specify the subset that is allowed by
the glue driver.


Since a controller cannot run in both EP and RC mode at the same time,
I think that it is fine that this patch reuses the name 'legacy' for the
combined interrupt.

And as you can see in patch 5 in this series, rk3588 actually uses a single
combined IRQ (called legacy) for 'inta', 'intb', 'intc', 'intd', 'tx_inta',
'tx_intb', 'tx_intc', 'tx_intd'.


Kind regards,
Niklas


>
> But irrespective of that,
>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam at linaro.org>
>
> - Mani
>
> > Reviewed-by: Rob Herring (Arm) <robh at kernel.org>
> > ---
> >  Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
> > index f5f12cbc2cb3..f474b9e3fc7e 100644
> > --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
> > +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
> > @@ -151,6 +151,15 @@ properties:
> >              Application-specific IRQ raised depending on the vendor-specific
> >              events basis.
> >            const: app
> > +        - description:
> > +            Interrupts triggered when the controller itself (in Endpoint mode)
> > +            has sent an Assert_INT{A,B,C,D}/Desassert_INT{A,B,C,D} message to
> > +            the upstream device.
> > +          pattern: "^tx_int(a|b|c|d)$"
> > +        - description:
> > +            Combined interrupt signal raised when the controller has sent an
> > +            Assert_INT{A,B,C,D} message. See "^tx_int(a|b|c|d)$" for details.
> > +          const: legacy
> >          - description:
> >              Vendor-specific IRQ names. Consider using the generic names above
> >              for new bindings.
> >
> > --
> > 2.45.1
> >
>
> --
> மணிவண்ணன் சதாசிவம்