[PATCH v2 0/2] ARM: shmobile: r8a7791: add for USB-DMAC

[PATCH v2 0/2] ARM: shmobile: r8a7791: add for USB-DMAC

[Yoshihiro Shimoda]

This patch set is based on renesas.git / renesas-devel-20150406-v4.0-rc6
tag. (commit id = 8acf8f0880b6a142e6291b9a7d81f2f38c9425ac)

This patch set is related to the usb-dmac driver and renesas_usbhs driver.
And these drivers have been merged in each subsystem:

[ slave-dma repository ]
https://git.kernel.org/cgit/linux/kernel/git/vkoul/slave-dma.git/commit/?h=for-linus&id=a7267831565030ab94dab76eb1afe5d4b63a1009
https://git.kernel.org/cgit/linux/kernel/git/vkoul/slave-dma.git/commit/?h=for-linus&id=0c1c8ff32fa29e425b4938934c21afdb81104431

[ usb repository ]
https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testing/next&id=7a96b78464bd8ba72c1c3095c543c1402db59e35
https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testing/next&id=9b53d9af7aac09cf249d72bfbf15f08e47c4f7fe
https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testing/next&id=ab330cf3888d8e0779fa05a243d53ba9f53a7ba9

Changes from v1:
 - Rebase the latest renesas-devel tag.

Yoshihiro Shimoda (2):
  ARM: shmobile: r8a7791: add USB-DMAC device nodes
  ARM: shmobile: r8a7791: Enable DMA for HSUSB

 arch/arm/boot/dts/r8a7791.dtsi | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

-- 
1.9.1

[PATCH v2 1/2] ARM: shmobile: r8a7791: add USB-DMAC device nodes

[Yoshihiro Shimoda]

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 arch/arm/boot/dts/r8a7791.dtsi | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 3e9f824..db3772e 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -357,6 +357,28 @@
 		dma-channels = <13>;
 	};
 
+	usb_dmac0: dma-controller@e65a0000 {
+		compatible = "renesas,usb-dmac";
+		reg = <0 0xe65a0000 0 0x100>;
+		interrupts = <0 109 IRQ_TYPE_LEVEL_HIGH
+			      0 109 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "ch0", "ch1";
+		clocks = <&mstp3_clks R8A7791_CLK_USBDMAC0>;
+		#dma-cells = <1>;
+		dma-channels = <2>;
+	};
+
+	usb_dmac1: dma-controller@e65b0000 {
+		compatible = "renesas,usb-dmac";
+		reg = <0 0xe65b0000 0 0x100>;
+		interrupts = <0 110 IRQ_TYPE_LEVEL_HIGH
+			      0 110 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "ch0", "ch1";
+		clocks = <&mstp3_clks R8A7791_CLK_USBDMAC1>;
+		#dma-cells = <1>;
+		dma-channels = <2>;
+	};
+
 	/* The memory map in the User's Manual maps the cores to bus numbers */
 	i2c0: i2c@e6508000 {
 		#address-cells = <1>;
-- 
1.9.1

[PATCH v2 2/2] ARM: shmobile: r8a7791: Enable DMA for HSUSB

[Yoshihiro Shimoda]

This patch adds DMA properties to the HSUSB node.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 arch/arm/boot/dts/r8a7791.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index db3772e..5f20ad4 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -722,6 +722,9 @@
 		renesas,buswait = <4>;
 		phys = <&usb0 1>;
 		phy-names = "usb";
+		dmas = <&usb_dmac0 0>, <&usb_dmac0 1>,
+		       <&usb_dmac1 0>, <&usb_dmac1 1>;
+		dma-names = "rx0", "tx1", "rx2", "tx3";
 		status = "disabled";
 	};
 
-- 
1.9.1

Re: [PATCH v2 1/2] ARM: shmobile: r8a7791: add USB-DMAC device nodes

[Geert Uytterhoeven]

On Tue, Apr 7, 2015 at 9:53 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

Acked-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] ARM: shmobile: r8a7791: Enable DMA for HSUSB

[Geert Uytterhoeven]

Hi Shimoda-san,

On Tue, Apr 7, 2015 at 9:53 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> This patch adds DMA properties to the HSUSB node.

Thank you for your patch!

> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> ---
>  arch/arm/boot/dts/r8a7791.dtsi | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
> index db3772e..5f20ad4 100644
> --- a/arch/arm/boot/dts/r8a7791.dtsi
> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> @@ -722,6 +722,9 @@
>                 renesas,buswait = <4>;
>                 phys = <&usb0 1>;
>                 phy-names = "usb";
> +               dmas = <&usb_dmac0 0>, <&usb_dmac0 1>,
> +                      <&usb_dmac1 0>, <&usb_dmac1 1>;
> +               dma-names = "rx0", "tx1", "rx2", "tx3";

The numbering looks a bit strange, given the code looks up both tx and rx
DMA channels for each channel index. Is it correct?

The binding documentation (which lacks on example) states:

  - dma-names : Must contain a list of DMA names:
   - tx0 ... tx<n>
   - rx0 ... rx<n>
    - This <n> means DnFIFO in USBHS module.

As there are 4 DnFIFOs, I'd expect tx0..tx3 and rx0..rx3.

Can you please clarify?
Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2 2/2] ARM: shmobile: r8a7791: Enable DMA for HSUSB

[Yoshihiro Shimoda]

Hi Geert-san,

> Hi Shimoda-san,
> 
> On Tue, Apr 7, 2015 at 9:53 AM, Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > This patch adds DMA properties to the HSUSB node.
> 
> Thank you for your patch!

Thank you for your review!

> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  arch/arm/boot/dts/r8a7791.dtsi | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
> > index db3772e..5f20ad4 100644
> > --- a/arch/arm/boot/dts/r8a7791.dtsi
> > +++ b/arch/arm/boot/dts/r8a7791.dtsi
> > @@ -722,6 +722,9 @@
> >                 renesas,buswait = <4>;
> >                 phys = <&usb0 1>;
> >                 phy-names = "usb";
> > +               dmas = <&usb_dmac0 0>, <&usb_dmac0 1>,
> > +                      <&usb_dmac1 0>, <&usb_dmac1 1>;
> > +               dma-names = "rx0", "tx1", "rx2", "tx3";
> 
> The numbering looks a bit strange, given the code looks up both tx and rx
> DMA channels for each channel index. Is it correct?
> 
> The binding documentation (which lacks on example) states:
> 
>   - dma-names : Must contain a list of DMA names:
>    - tx0 ... tx<n>
>    - rx0 ... rx<n>
>     - This <n> means DnFIFO in USBHS module.
> 
> As there are 4 DnFIFOs, I'd expect tx0..tx3 and rx0..rx3.
> 
> Can you please clarify?

I wrote some information below.
But, since it is complex hardware, I don't know I can explain this as well...

At first, R-Car Gen2 has 2 USB-DMACs and 4 channels:
 USB-DMAC 0: ch0
 USB-DMAC 0: ch1
 USB-DMAC 1: ch0
 USB-DMAC 1: ch1
Remarks: I don't know why but performance of USB-DMAC 0 is good than USB-DMAC 1.
          (Please refer to the Table 64.1 of the datasheet.)

So, I wrote dmas parameter in hsusb as the following:

> > +               dmas = <&usb_dmac0 0>, <&usb_dmac0 1>,
> > +                      <&usb_dmac1 0>, <&usb_dmac1 1>;


And, R-Car Gen2 has 4 DnFIFOs in HSUSB. to avoid complex handling for DnFIFOs,
the renesas_usbhs driver uses each DnFIFO as TX or RX direction (not bi-direction).
So, I wrote dma-names parameter as the following:

> > +               dma-names = "rx0", "tx1", "rx2", "tx3";

- D0FIFO as RX (Also it is connected to USB-DMAC 0: ch0)
- D1FIFO as TX (Also it is connected to USB-DMAC 0: ch1)
- D2FIFO as RX (Also it is connected to USB-DMAC 1: ch0)
- D3FIFO as TX (Also it is connected to USB-DMAC 1: ch1)

Best regards,
Yoshihiro Shimoda

> Thanks!
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-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

Re: [PATCH v2 2/2] ARM: shmobile: r8a7791: Enable DMA for HSUSB

[Geert Uytterhoeven]

Hi Shimoda-san,

On Wed, Apr 8, 2015 at 4:20 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
>> On Tue, Apr 7, 2015 at 9:53 AM, Yoshihiro Shimoda
>> <yoshihiro.shimoda.uh@renesas.com> wrote:
>> > diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
>> > index db3772e..5f20ad4 100644
>> > --- a/arch/arm/boot/dts/r8a7791.dtsi
>> > +++ b/arch/arm/boot/dts/r8a7791.dtsi
>> > @@ -722,6 +722,9 @@
>> >                 renesas,buswait = <4>;
>> >                 phys = <&usb0 1>;
>> >                 phy-names = "usb";
>> > +               dmas = <&usb_dmac0 0>, <&usb_dmac0 1>,
>> > +                      <&usb_dmac1 0>, <&usb_dmac1 1>;
>> > +               dma-names = "rx0", "tx1", "rx2", "tx3";
>>
>> The numbering looks a bit strange, given the code looks up both tx and rx
>> DMA channels for each channel index. Is it correct?
>>
>> The binding documentation (which lacks on example) states:
>>
>>   - dma-names : Must contain a list of DMA names:
>>    - tx0 ... tx<n>
>>    - rx0 ... rx<n>
>>     - This <n> means DnFIFO in USBHS module.
>>
>> As there are 4 DnFIFOs, I'd expect tx0..tx3 and rx0..rx3.
>>
>> Can you please clarify?
>
> I wrote some information below.
> But, since it is complex hardware, I don't know I can explain this as well...
>
> At first, R-Car Gen2 has 2 USB-DMACs and 4 channels:
>  USB-DMAC 0: ch0
>  USB-DMAC 0: ch1
>  USB-DMAC 1: ch0
>  USB-DMAC 1: ch1
> Remarks: I don't know why but performance of USB-DMAC 0 is good than USB-DMAC 1.
>           (Please refer to the Table 64.1 of the datasheet.)
>
> So, I wrote dmas parameter in hsusb as the following:
>
>> > +               dmas = <&usb_dmac0 0>, <&usb_dmac0 1>,
>> > +                      <&usb_dmac1 0>, <&usb_dmac1 1>;
>
> And, R-Car Gen2 has 4 DnFIFOs in HSUSB. to avoid complex handling for DnFIFOs,
> the renesas_usbhs driver uses each DnFIFO as TX or RX direction (not bi-direction).
> So, I wrote dma-names parameter as the following:
>
>> > +               dma-names = "rx0", "tx1", "rx2", "tx3";
>
> - D0FIFO as RX (Also it is connected to USB-DMAC 0: ch0)
> - D1FIFO as TX (Also it is connected to USB-DMAC 0: ch1)
> - D2FIFO as RX (Also it is connected to USB-DMAC 1: ch0)
> - D3FIFO as TX (Also it is connected to USB-DMAC 1: ch1)

Thanks for the explanation!

Hence the "strange" numbering is a limitation of the driver, not of the
hardware?

In that case I think the DT should describe the hardware, not the driver
limitation. As the FIFOs are bidirectional, there's just a one-to-one mapping
of 4 DnFIFOs to 4 (2 channels x 2 DMACs) DMA channels.
Which DnFIFO/channel is used for TX and which is used for RX is to be chosen
by the driver, and can be fixed (e.g. use even channels for RX, odd for
TX, like the current situation).

What do you (and other people) think?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-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

RE: [PATCH v2 2/2] ARM: shmobile: r8a7791: Enable DMA for HSUSB

[Yoshihiro Shimoda]

Hi Geert-san,

> Hi Shimoda-san,
> 
> On Wed, Apr 8, 2015 at 4:20 AM, Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> >> On Tue, Apr 7, 2015 at 9:53 AM, Yoshihiro Shimoda
> >> <yoshihiro.shimoda.uh@renesas.com> wrote:
> >> > diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
> >> > index db3772e..5f20ad4 100644
> >> > --- a/arch/arm/boot/dts/r8a7791.dtsi
> >> > +++ b/arch/arm/boot/dts/r8a7791.dtsi
> >> > @@ -722,6 +722,9 @@
> >> >                 renesas,buswait = <4>;
> >> >                 phys = <&usb0 1>;
> >> >                 phy-names = "usb";
> >> > +               dmas = <&usb_dmac0 0>, <&usb_dmac0 1>,
> >> > +                      <&usb_dmac1 0>, <&usb_dmac1 1>;
> >> > +               dma-names = "rx0", "tx1", "rx2", "tx3";
> >>
> >> The numbering looks a bit strange, given the code looks up both tx and rx
> >> DMA channels for each channel index. Is it correct?
> >>
> >> The binding documentation (which lacks on example) states:
> >>
> >>   - dma-names : Must contain a list of DMA names:
> >>    - tx0 ... tx<n>
> >>    - rx0 ... rx<n>
> >>     - This <n> means DnFIFO in USBHS module.
> >>
> >> As there are 4 DnFIFOs, I'd expect tx0..tx3 and rx0..rx3.
> >>
> >> Can you please clarify?
> >
> > I wrote some information below.
> > But, since it is complex hardware, I don't know I can explain this as well...
> >
> > At first, R-Car Gen2 has 2 USB-DMACs and 4 channels:
> >  USB-DMAC 0: ch0
> >  USB-DMAC 0: ch1
> >  USB-DMAC 1: ch0
> >  USB-DMAC 1: ch1
> > Remarks: I don't know why but performance of USB-DMAC 0 is good than USB-DMAC 1.
> >           (Please refer to the Table 64.1 of the datasheet.)
> >
> > So, I wrote dmas parameter in hsusb as the following:
> >
> >> > +               dmas = <&usb_dmac0 0>, <&usb_dmac0 1>,
> >> > +                      <&usb_dmac1 0>, <&usb_dmac1 1>;
> >
> > And, R-Car Gen2 has 4 DnFIFOs in HSUSB. to avoid complex handling for DnFIFOs,
> > the renesas_usbhs driver uses each DnFIFO as TX or RX direction (not bi-direction).
> > So, I wrote dma-names parameter as the following:
> >
> >> > +               dma-names = "rx0", "tx1", "rx2", "tx3";
> >
> > - D0FIFO as RX (Also it is connected to USB-DMAC 0: ch0)
> > - D1FIFO as TX (Also it is connected to USB-DMAC 0: ch1)
> > - D2FIFO as RX (Also it is connected to USB-DMAC 1: ch0)
> > - D3FIFO as TX (Also it is connected to USB-DMAC 1: ch1)
> 
> Thanks for the explanation!
> 
> Hence the "strange" numbering is a limitation of the driver, not of the
> hardware?

Thank you for the point! That's correct.

> In that case I think the DT should describe the hardware, not the driver
> limitation. As the FIFOs are bidirectional, there's just a one-to-one mapping
> of 4 DnFIFOs to 4 (2 channels x 2 DMACs) DMA channels.

I understood it.
(Sometimes I forget that the DT should describe the hardware...)

> Which DnFIFO/channel is used for TX and which is used for RX is to be chosen
> by the driver, and can be fixed (e.g. use even channels for RX, odd for
> TX, like the current situation).
> 
> What do you (and other people) think?

Thank you for the suggestion!
I will modify the renesas_usbhs driver.
Also I would like to revise the binding documentation as the following:

- dma-names : named "ch%u", where %u is the channel number ranging from zero
              to the number of channels (DnFIFOs) minus one.

Best regards,
Yoshihiro Shimoda

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-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