| Message ID | 20190506234631.113226-1-chris.brandt@renesas.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | usb: Add host and device support for RZ/A2 | expand |
Hi Chris-san, > From: Chris Brandt, Sent: Tuesday, May 7, 2019 8:46 AM > > For the most part, the RZ/A2 has the same USB 2.0 host and device > HW as the R-Car Gen3, so we can reuse a lot of the code. > > However, there are a couple extra register bits, and the CFIFO > register 8-bit access works a little different (weird, no idea why). This is just my gut feeling, but if we set the BIGEND bit in the CFIFOSEL of RZ/A2M (R-Car Gen3 doesn't have such a bit though), could the original code work correctly? Best regards, Yoshihiro Shimoda
Hi Shimodaさん > From: Yoshihiro Shimoda > Sent: Tuesday, May 07, 2019 5:17 AM > > For the most part, the RZ/A2 has the same USB 2.0 host and device > > HW as the R-Car Gen3, so we can reuse a lot of the code. > > > > However, there are a couple extra register bits, and the CFIFO > > register 8-bit access works a little different (weird, no idea why). > > This is just my gut feeling, but if we set the BIGEND bit in the CFIFOSEL > of RZ/A2M (R-Car Gen3 doesn't have such a bit though), could the original > code work correctly? I just tried to set CFIFOSEL.BIGEND = 1 * Set CFIFOSEL.BIGEND = 1 * Write 8-bit values to CFIFO (same method as R-Car) * Set CFIFOSEL.BIGEND = 0 The result is bad. But, then I tried this: * Set CFIFOSEL.MBW = 0 (CFIFO port access = 8-bit) * Write 8-bit values to CFIFO * Set CFIFOSEL.MBW = 2 (CFIFO port access = 32-bit) Code: u16 cfifosel = usbhs_read(priv, fifo->sel); usbhs_write(priv, fifo->sel, cfifosel & 0xF3FF); // MBW = 8-bit for (i = 0; i < len; i++) iowrite8(buf[i], addr); //same address each time usbhs_write(priv, fifo->sel, cfifosel); // MBW = 32-bit This method works good. (I assume this method would work with R-Car also) But...then we have extra register reads and writes. Register accesses are slower, so performance is lower. So, I prefer my original method: if (usbhsc_flags_has(priv, USBHSF_CFIFO_BYTE_ADDR)) for (i = 0; i < len; i++) iowrite8(buf[i], addr + (i & 0x03)); else for (i = 0; i < len; i++) iowrite8(buf[i], addr + (0x03 - (i & 0x03))); Do you agree? Chris
Hi Chrisさん > From: Chris Brandt, Sent: Thursday, May 9, 2019 3:08 AM > > Hi Shimodaさん > > > From: Yoshihiro Shimoda > > Sent: Tuesday, May 07, 2019 5:17 AM > > > For the most part, the RZ/A2 has the same USB 2.0 host and device > > > HW as the R-Car Gen3, so we can reuse a lot of the code. > > > > > > However, there are a couple extra register bits, and the CFIFO > > > register 8-bit access works a little different (weird, no idea why). > > > > This is just my gut feeling, but if we set the BIGEND bit in the CFIFOSEL > > of RZ/A2M (R-Car Gen3 doesn't have such a bit though), could the original > > code work correctly? > > I just tried to set CFIFOSEL.BIGEND = 1 Thank you for trying it! > * Set CFIFOSEL.BIGEND = 1 > * Write 8-bit values to CFIFO (same method as R-Car) > * Set CFIFOSEL.BIGEND = 0 > > The result is bad. I got it... > But, then I tried this: > * Set CFIFOSEL.MBW = 0 (CFIFO port access = 8-bit) > * Write 8-bit values to CFIFO > * Set CFIFOSEL.MBW = 2 (CFIFO port access = 32-bit) > > Code: > u16 cfifosel = usbhs_read(priv, fifo->sel); > > usbhs_write(priv, fifo->sel, cfifosel & 0xF3FF); // MBW = 8-bit > > for (i = 0; i < len; i++) > iowrite8(buf[i], addr); //same address each time > > usbhs_write(priv, fifo->sel, cfifosel); // MBW = 32-bit > > > This method works good. I got it. > (I assume this method would work with R-Car also) Unfortunately, R-Car cannot work with this method... But, "iowrite8(buf[i], addr + 3);" or "iowrite32(buf[i], addr);" works on the R-Car. And then, I realized that R-Car CFIFO register allows 32-bit access only... # So, I'm asking HW guy whether the 8-bit access can be allowed or not now... > But...then we have extra register reads and writes. > Register accesses are slower, so performance is lower. > > So, I prefer my original method: > if (usbhsc_flags_has(priv, USBHSF_CFIFO_BYTE_ADDR)) > for (i = 0; i < len; i++) > iowrite8(buf[i], addr + (i & 0x03)); > else > for (i = 0; i < len; i++) > iowrite8(buf[i], addr + (0x03 - (i & 0x03))); > > > Do you agree? I agree. However, I have some comments about the patch. So, I'll reply on the patch later. Best regards, Yoshihiro Shimoda > Chris
For the most part, the RZ/A2 has the same USB 2.0 host and device HW as the R-Car Gen3, so we can reuse a lot of the code. However, there are a couple extra register bits, and the CFIFO register 8-bit access works a little different (weird, no idea why). There is a dedicated DMAC for the RZ/A2 USB Device HW, but we have not been able to reliably get that working yet, so device operation is pio only at the moment. On the RZ/A2M eval board, both USB channels can be used as either host or device. But, it's not set up for otg (ie, there are jumpers and separate connectors). Therefore, below is an example of what it would look like to enable USB channel 0 as a device instead of a host. &usb2_phy0 { pinctrl-names = "default"; pinctrl-0 = <&usb0_pins>; renesas,uses_usb_x1; dr_mode = "peripheral"; status = "okay"; }; &usbhs0 { status = "okay"; }; Chris Brandt (10): phy: renesas: rcar-gen3-usb2: Add uses_usb_x1 option dt-bindings: rcar-gen3-phy-usb2: Document uses_usb_x1 phy: renesas: rcar-gen3-usb2: Check dr_mode when not using OTG dt-bindings: rcar-gen3-phy-usb2: Document dr_mode dt-bindings: rcar-gen3-phy-usb2: Add r7s9210 support usb: renesas_usbhs: Add support for RZ/A2 dt-bindings: usb: renesas_usbhs: Add support for r7s9210 ARM: dts: r7s9210: Add USB Host support ARM: dts: r7s9210: Add USB Device support ARM: dts: r7s9210-rza2mevb: Add USB host support .../devicetree/bindings/phy/rcar-gen3-phy-usb2.txt | 15 +++- .../devicetree/bindings/usb/renesas_usbhs.txt | 2 + arch/arm/boot/dts/r7s9210-rza2mevb.dts | 37 +++++++++ arch/arm/boot/dts/r7s9210.dtsi | 88 ++++++++++++++++++++++ drivers/phy/renesas/phy-rcar-gen3-usb2.c | 16 +++- drivers/usb/renesas_usbhs/Makefile | 2 +- drivers/usb/renesas_usbhs/common.c | 27 +++++-- drivers/usb/renesas_usbhs/common.h | 13 ++++ drivers/usb/renesas_usbhs/fifo.c | 8 +- drivers/usb/renesas_usbhs/rza.h | 1 + drivers/usb/renesas_usbhs/rza2.c | 82 ++++++++++++++++++++ include/linux/usb/renesas_usbhs.h | 1 + 12 files changed, 277 insertions(+), 15 deletions(-) create mode 100644 drivers/usb/renesas_usbhs/rza2.c