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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi Geert-san,

> Hi Shimoda-san,
> 
> On Wed, Apr 8, 2015 at 4:20 AM, Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
> >> On Tue, Apr 7, 2015 at 9:53 AM, Yoshihiro Shimoda
> >> <yoshihiro.shimoda.uh@xxxxxxxxxxx> 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@xxxxxxxxxxxxxx
> 
> 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
��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux