Message-ID:
 <TYCPR01MB1209315DF46290CF62805F7B1C2CC2@TYCPR01MB12093.jpnprd01.prod.outlook.com>
Date: Fri, 28 Feb 2025 17:28:44 +0000
From: Fabrizio Castro <fabrizio.castro.jz@...esas.com>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
CC: Vinod Koul <vkoul@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof
 Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Magnus
 Damm <magnus.damm@...il.com>, Biju Das <biju.das.jz@...renesas.com>,
	"dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
	Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: RE: [PATCH v4 3/7] dt-bindings: dma: rz-dmac: Document RZ/V2H(P)
 family of SoCs

Hi Geert,

> From: Geert Uytterhoeven <geert@...ux-m68k.org>
> Sent: 28 February 2025 16:44
> Subject: Re: [PATCH v4 3/7] dt-bindings: dma: rz-dmac: Document RZ/V2H(P) family of SoCs
> 
> Hi Fabrizio,
> 
> On Fri, 28 Feb 2025 at 17:32, Fabrizio Castro
> <fabrizio.castro.jz@...esas.com> wrote:
> > > From: Geert Uytterhoeven <geert@...ux-m68k.org>
> > > On Fri, 28 Feb 2025 at 16:38, Fabrizio Castro
> > > <fabrizio.castro.jz@...esas.com> wrote:
> > > > > From: Geert Uytterhoeven <geert@...ux-m68k.org>
> > > > > On Fri, 28 Feb 2025 at 15:55, Fabrizio Castro
> > > > > <fabrizio.castro.jz@...esas.com> wrote:
> > > > > > > From: Geert Uytterhoeven <geert@...ux-m68k.org>
> > > > > > > On Thu, 27 Feb 2025 at 19:16, Fabrizio Castro
> > > > > > > <fabrizio.castro.jz@...esas.com> wrote:
> > > > > > > > > From: Geert Uytterhoeven <geert@...ux-m68k.org>
> > > > > > > > > Sent: 24 February 2025 12:44
> > > > > > > > > Subject: Re: [PATCH v4 3/7] dt-bindings: dma: rz-dmac: Document RZ/V2H(P) family of
> SoCs
> > > > > > > > >
> > > > > > > > > On Thu, 20 Feb 2025 at 16:01, Fabrizio Castro
> > > > > > > > > <fabrizio.castro.jz@...esas.com> wrote:
> > > > > > > > > > Document the Renesas RZ/V2H(P) family of SoCs DMAC block.
> > > > > > > > > > The Renesas RZ/V2H(P) DMAC is very similar to the one found on the
> > > > > > > > > > Renesas RZ/G2L family of SoCs, but there are some differences:
> > > > > > > > > > * It only uses one register area
> > > > > > > > > > * It only uses one clock
> > > > > > > > > > * It only uses one reset
> > > > > > > > > > * Instead of using MID/IRD it uses REQ NO/ACK NO
> > > > > > > > > > * It is connected to the Interrupt Control Unit (ICU)
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@...esas.com>
> > > > > > > > >
> > > > > > > > > > v1->v2:
> > > > > > > > > > * Removed RZ/V2H DMAC example.
> > > > > > > > > > * Improved the readability of the `if` statement.
> > > > > > > > >
> > > > > > > > > Thanks for the update!
> > > > > > > > >
> > > > > > > > > > --- a/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> > > > > > > > > > +++ b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> > > > > > > > > > @@ -61,14 +66,22 @@ properties:
> > > > > > > > > >    '#dma-cells':
> > > > > > > > > >      const: 1
> > > > > > > > > >      description:
> > > > > > > > > > -      The cell specifies the encoded MID/RID values of the DMAC port
> > > > > > > > > > -      connected to the DMA client and the slave channel configuration
> > > > > > > > > > -      parameters.
> > > > > > > > > > +      For the RZ/A1H, RZ/Five, RZ/G2{L,LC,UL}, RZ/V2L, and RZ/G3S SoCs, the cell
> > > > > > > > > > +      specifies the encoded MID/RID values of the DMAC port connected to the
> > > > > > > > > > +      DMA client and the slave channel configuration parameters.
> > > > > > > > > >        bits[0:9] - Specifies MID/RID value
> > > > > > > > > >        bit[10] - Specifies DMA request high enable (HIEN)
> > > > > > > > > >        bit[11] - Specifies DMA request detection type (LVL)
> > > > > > > > > >        bits[12:14] - Specifies DMAACK output mode (AM)
> > > > > > > > > >        bit[15] - Specifies Transfer Mode (TM)
> > > > > > > > > > +      For the RZ/V2H(P) SoC the cell specifies the REQ NO, the ACK NO, and the
> > > > > > > > > > +      slave channel configuration parameters.
> > > > > > > > > > +      bits[0:9] - Specifies the REQ NO
> > > > > > > > >
> > > > > > > > > So REQ_NO is the new name for MID/RID.
> > > > > > >
> > > > > > > These are documented in Table 4.7-22 ("DMA Transfer Request Detection
> > > > > > > Operation Setting Table").
> > > > > >
> > > > > > REQ_NO is documented in both Table 4.7-22 and in Table 4.6-23 (column `DMAC No.`).
> > > > >
> > > > > Indeed. But not for all of them. E.g. RSPI is missing, IIC is present.
> > > >
> > > > I can see the RSPI related `REQ No.` in the version of the manual I am using,
> > > > although one must be very careful to look at the right entry in the table,
> > > > as the table is quite big, and the entries are ordered by `SPI No.`.
> > > >
> > > > For some devices, the SPI numbers are not contiguous therefore the device specific
> > > > bits may end up scattered.
> > > > For example, for `Name` `RSPI_CH0_sp_rxintpls_n` (mind that the `pls_n` substring
> > > > is on a new line in the table) you can see from Table 4.6-23 that
> > > > its `DMAC No.` is 140 (as you said, in decimal...).
> > >
> > > Thanks, I had missed it because the RSPI interrupts are spread across
> > > two places...
> > >
> > > > > And the numbers are shown in decimal instead of in hex ;-)
> > > > >
> > > > > > > > It's certainly similar. I would say that REQ_NO + ACK_NO is the new MID_RID.
> > > > > > > >
> > > > > > > > > > +      bits[10:16] - Specifies the ACK NO
> > > > > > > > >
> > > > > > > > > This is a new field.
> > > > > > > > > However, it is not clear to me which value to specify here, and if this
> > > > > > > > > is a hardware property at all, and thus needs to be specified in DT?
> > > > > > > >
> > > > > > > > It is a HW property. The value to set can be found in Table 4.6-27 from
> > > > > > > > the HW User Manual, column "Ack No".
> > > > > > >
> > > > > > > Thanks, but that table only shows values for SPDIF, SCU, SSIU and PFC
> > > > > > > (for external DMA requests).  The most familiar DMA clients listed
> > > > > > > in Table 4.7-22 are missing.  E.g. RSPI0 uses REQ_NO 0x8C/0x8D, but
> > > > > > > which values does it need for ACK_NO?
> > > > > >
> > > > > > Only a handful of devices need it. For every other device (and use case) only the
> > > > > > default value is needed.
> > > > >
> > > > > The default value is RZV2H_ICU_DMAC_ACK_NO_DEFAULT = 0x7f?
> > >
> > > If you take this out, how to distinguish between ACK_NO = 0 and
> > > the default?
> >
> > I am not sure I understand what you mean, so my answer here may be completely off.
> >
> > ACK No. 0 corresponds to SPDIF, CH0, TX, while ACK No. 0x7F is not valid.
> 
> OK, that was my understanding, too.
> 
> > My understanding of this is that there is a DACK_SEL field per ACK No (23 ICU_DMACKSELk
> > registers, 4 DACK_SEL fields per ICU_DMACKSELk registers -> 23 * 4 = 92 DACK_SEL fields),
> > to match the 92 ACK numbers listed in Table 4.6-27.
> >
> > Each DACK_SEL field should contain the global channel index (5 DMACs, 16 channels per DMAC
> > -> 5 * 16 = 80 channels in total) associated to the ACK No.
> > If DACK_SEL contains a valid channel number (0-79), then the corresponding signal
> > gets controlled accordingly, otherwise a fixed output is generated instead.
> >
> > Mind that the code I sent wasn't dealing with it properly, but wasn't spotted due
> > to limited testing capabilities, and it's safe to take out, as the DACK_SEL fields
> > will all contain invalid channel numbers by default.
> >
> > Looking ahead, there is a similar scenario with the TEND signals as well.
> >
> > So for now the plan is to upstream support for memory/memory and device/memory (REQ No.,
> > tested with RSPI), add support for ACK No later (perhaps testing it with audio, or via
> > an external device), and finally TEND No if we get to it.
> 
> So which values will you put in the dmas property for RSPI?
> I assume:
>        bits[0:9] - Specifies REQ_NO value
>        bit[10] - Specifies DMA request high enable (HIEN)
>        bit[11] - Specifies DMA request detection type (LVL)
>        bits[12:14] - Specifies DMAACK output mode (AM)
>        bit[15] - Specifies Transfer Mode (TM)
> i.e. all remaining bits will be zero?

I see what you mean now. And there would be an ABI mismatch between older DTs and newer kernels,
newer kernels would interpret the values incorrectly (as you said, 0 is a valid number).

> 
> How do you plan to handle adding ACK_NO bits later?
> I.e. how to distinguish between remaining bits zero and remaining
> bits containing a valid ACK_NO value (which can be zero, for SPDIF)?

We could add the ACK No. and the TEND No. to the binding (after TM), and implement it
later in the driver (once we have some practical use case for them)?

There are also a couple of alternatives:
* we could add 1 to ACK No. and TEND No.? At that point 0 would be an invalid number?
  In which case we could add DT and driver support later?
* we could fill up the remaining bits with 1s? We only have 5 TEND numbers, so 0b111 would
  be invalid. Similarly, we have 92 ACK numbers, so 0b1111111 would also be invalid. Also
  this shouldn't break the ABIs once we get around to add the rest?

> 
> I hope I made myself clear this time.

Very clear! Thank you.

> If not, weekend time ;-)
> 
> Have a nice weekend!

Thank you, and you!

Cheers,
Fab

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Powered by blists - more mailing lists