Re: [PATCH v3 2/6] dt-bindings: PCI: dwc: rockchip: Add optional dma interrupts

Serge Semin
2023-10-31T01:10:26Z

On Fri, Oct 27, 2023 at 04:54:14PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
>
> Even though rockchip-dw-pcie.yaml inherits snps,dw-pcie.yaml
> using:
>
> allOf:
> - $ref: /schemas/pci/snps,dw-pcie.yaml#
>
> and snps,dw-pcie.yaml does have the dma interrupts defined, in order to be
> able to use these interrupts, while still making sure 'make CHECK_DTBS=y'
> pass, we need to add these interrupts to rockchip-dw-pcie.yaml.
>
> These dedicated interrupts for the eDMA are not always wired to all the
> PCIe controllers on the same SoC. In other words, even for a specific
> compatible, e.g. rockchip,rk3588-pcie, these dedicated eDMA interrupts
> may or may not be wired.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
> .../bindings/pci/rockchip-dw-pcie.yaml | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> index 6ca87ff4ae20..7ad6e1283784 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> @@ -63,6 +63,7 @@ properties:
> - const: pipe
>
> interrupts:
> + minItems: 5
> items:
> - description:
> Combined system interrupt, which is used to signal the following
> @@ -86,14 +87,31 @@ properties:
> interrupts - aer_rc_err, aer_rc_err_msi, rx_cpl_timeout,
> tx_cpl_timeout, cor_err_sent, nf_err_sent, f_err_sent, cor_err_rx,
> nf_err_rx, f_err_rx, radm_qoverflow
> + - description:
> + Indicates that the eDMA Tx/Rx transfer is complete or that an
> + error has occurred on the corresponding channel.
> + - description:
> + Indicates that the eDMA Tx/Rx transfer is complete or that an
> + error has occurred on the corresponding channel.
> + - description:
> + Indicates that the eDMA Tx/Rx transfer is complete or that an
> + error has occurred on the corresponding channel.
> + - description:
> + Indicates that the eDMA Tx/Rx transfer is complete or that an
> + error has occurred on the corresponding channel.
>
> interrupt-names:
> + minItems: 5
> items:
> - const: sys
> - const: pmc
> - const: msg
> - const: legacy
> - const: err

> + - const: dma0
> + - const: dma1
> + - const: dma2
> + - const: dma3

No. As you said yourself here
https://lore.kernel.org/linux-pci/ZTl1ZsHvk3DDHWsm@x1-carbon/
and in the response to my last message in v2, which for some reason
hasn't got to the lore archive:

On Fri, Oct 27, 2023 at 05:51:14PM +0200, Niklas Cassel wrote:
> However, e.g. rk3568 only has one channel for reads and one for writes.
> (Now this SoC doesn't have dedicated IRQs for the eDMA, but let's pretend
> that it did.)
>
> So for rk3568, it would then instead be:
> dma0: wr0
> dma1: rd0
> dma2: <unused>
> dma3: <unused>

rk3568 doesn't have IRQs supplied in a normal way, as separate
signals. Instead they are combined in the 'sys' IRQ. So you should
define the IRQs constraint being device-specific by using for example
the "allOf: if-else" pattern.

-Serge(y)

>
> legacy-interrupt-controller:
> description: Interrupt controller node for handling legacy PCI interrupts.
> --
> 2.41.0
>