Re: [PATCH v2 3/4] dt-bindings: PCI: dwc: rockchip: Add dma properties

[Niklas Cassel <Niklas.Cassel@wdc.com>]

On Thu, Oct 26, 2023 at 05:32:44PM +0300, Serge Semin wrote:
> On Tue, Oct 24, 2023 at 05:10:10PM +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 properties defined, in order to be
> > able to use these properties, while still making sure 'make CHECK_DTBS=y'
> > pass, we need to add these properties to rockchip-dw-pcie.yaml.
> > 
> > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > ---
> >  .../bindings/pci/rockchip-dw-pcie.yaml        | 20 +++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > index 229f8608c535..633f8e0e884f 100644
> > --- a/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml
> > @@ -35,6 +35,7 @@ properties:
> >        - description: Rockchip designed configuration registers
> >        - description: Config registers
> >        - description: iATU registers
> > +      - description: eDMA registers
> >  
> >    reg-names:
> >      minItems: 3
> > @@ -43,6 +44,7 @@ properties:
> >        - const: apb
> >        - const: config
> >        - const: atu
> > +      - const: dma
> >  
> >    clocks:
> >      minItems: 5
> > @@ -65,6 +67,7 @@ properties:
> >        - const: pipe
> >  
> >    interrupts:
> > +    minItems: 5
> >      items:
> >        - description:
> >            Combined system interrupt, which is used to signal the following
> > @@ -88,14 +91,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.
> 
> They aren't identical. Some IRQs indicate events on the write eDMA
> channels, some - read eDMA channels. The respective channel ID would
> be also useful to have in the description.

Hello Serge,

As you know, the IRQs for the write channels have to be specified
before the IRQs for the read channels in "interrupts".

So for rk3588, it will be:
dma0: wr0
dma1: wr1
dma2: rd0
dma3: rd1

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>

So I would rather not add read/write or channel number to the descriptions,
as both the channel number and read/write will depend on how the eDMA in the
specific controller was synthesized. So I'd rather have no description rather
than a description that will be wrong for all rockchip SoCs other than rk3588.


Kind regards,
Niklas