| Message ID | 1428393231-5272-3-git-send-email-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Simon Horman |
| Headers | show |
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! > 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? 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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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"; };
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(+)