| Message ID | 20181107173558.54481-3-chris.brandt@renesas.com (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | usb: renesas: rcar-gen3-usb2: Add support for RZ/A2 | expand |
Hi Chris-san, Thank you for the patch! > From: Chris Brandt, Sent: Thursday, November 8, 2018 2:36 AM > > The RZ/A2 has the same USB2 host controller as R-Car Gen3 with only some > minor differences. > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> > --- > drivers/phy/renesas/Kconfig | 2 +- > drivers/phy/renesas/phy-rcar-gen3-usb2.c | 12 ++++++++++++ > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/phy/renesas/Kconfig b/drivers/phy/renesas/Kconfig > index e340a925bbb1..beebeba31e84 100644 > --- a/drivers/phy/renesas/Kconfig > +++ b/drivers/phy/renesas/Kconfig > @@ -19,7 +19,7 @@ config PHY_RCAR_GEN3_PCIE > config PHY_RCAR_GEN3_USB2 > tristate "Renesas R-Car generation 3 USB 2.0 PHY driver" > depends on ARCH_RENESAS > - depends on EXTCON > + depends on EXTCON || ARCH_R7S9210 Does this mean that you don't want to use EXTCON if ARCH_R7S9210=y? > depends on USB_SUPPORT > select GENERIC_PHY > select USB_COMMON > diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c > index d0f412c25981..96ac75ba40ea 100644 > --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c > +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c > @@ -34,6 +34,7 @@ > #define USB2_VBCTRL 0x60c > #define USB2_LINECTRL1 0x610 > #define USB2_ADPCTRL 0x630 > +#define USB2_PHYCLK_CTRL 0x644 > > /* INT_ENABLE */ > #define USB2_INT_ENABLE_UCOM_INTEN BIT(3) > @@ -88,6 +89,7 @@ struct rcar_gen3_chan { > bool extcon_host; > bool is_otg_channel; > bool uses_otg_pins; > + bool uses_usb_x1; > }; > > /* > @@ -326,6 +328,9 @@ static int rcar_gen3_phy_usb2_init(struct phy *p) > struct rcar_gen3_chan *channel = phy_get_drvdata(p); > void __iomem *usb2_base = channel->base; > > + if (channel->uses_usb_x1) > + writel(0x00000001, usb2_base + USB2_PHYCLK_CTRL); > + > /* Initialize USB2 part */ > writel(USB2_INT_ENABLE_INIT, usb2_base + USB2_INT_ENABLE); > writel(USB2_SPD_RSM_TIMSET_INIT, usb2_base + USB2_SPD_RSM_TIMSET); > @@ -334,6 +339,9 @@ static int rcar_gen3_phy_usb2_init(struct phy *p) > /* Initialize otg part */ > if (channel->is_otg_channel) > rcar_gen3_init_otg(channel); > + else > + /* No otg, so default to host mode */ > + writel(0x00000000, usb2_base + USB2_COMMCTRL); This "else" code will run on R-Car H3 USB port1(host only) for instance. I think adding this code is possible, but I'd like to separate patch from this adding R7S9210 support. What do you think? > return 0; > } > @@ -406,6 +414,7 @@ static irqreturn_t rcar_gen3_phy_usb2_irq(int irq, void *_ch) > } > > static const struct of_device_id rcar_gen3_phy_usb2_match_table[] = { > + { .compatible = "renesas,usb2-phy-r7s9210" }, According to the PATCH 3/3, R7S9210 will use "renesas,rcar-gen3-usb2-phy". So, you can remove this line. > { .compatible = "renesas,usb2-phy-r8a7795" }, > { .compatible = "renesas,usb2-phy-r8a7796" }, > { .compatible = "renesas,usb2-phy-r8a77965" }, > @@ -471,6 +480,9 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev) > } > } > > + if (of_property_read_bool(dev->of_node, "renesas,uses_usb_x1")) > + channel->uses_usb_x1 = true; > + I'll reply the PATCH 3/3 though, we have to describe the new property into the dt-bindings doc. Best regards, Yoshihiro Shimoda > /* > * devm_phy_create() will call pm_runtime_enable(&phy->dev); > * And then, phy-core will manage runtime pm for this device. > -- > 2.16.1
Hi Shimoda-san, > From: Yoshihiro Shimoda > Sent: Wednesday, November 14, 2018 5:50 AM > > config PHY_RCAR_GEN3_USB2 > > tristate "Renesas R-Car generation 3 USB 2.0 PHY driver" > > depends on ARCH_RENESAS > > - depends on EXTCON > > + depends on EXTCON || ARCH_R7S9210 > > Does this mean that you don't want to use EXTCON if ARCH_R7S9210=y? EXTCON is not required for RZ/A2. So, I want to be able to leave EXTCON un-selected (save flash space). > > /* Initialize otg part */ > > if (channel->is_otg_channel) > > rcar_gen3_init_otg(channel); > > + else > > + /* No otg, so default to host mode */ > > + writel(0x00000000, usb2_base + USB2_COMMCTRL); > > This "else" code will run on R-Car H3 USB port1(host only) for instance. > I think adding this code is possible, but I'd like to separate patch from > this adding R7S9210 support. What do you think? This code is required for RZ/A2. The reason is the reset value for COMMCTRL in RZ/A2 is 0x80000000 (Peripheral mode). The reset value in R-Car H3 is 0x00000000 (Host mode). Without this code, RZ/A2 will not work. Chris
Hi Chris-san, > From: Chris Brandt, Sent: Wednesday, November 14, 2018 8:18 PM > > Hi Shimoda-san, > > > From: Yoshihiro Shimoda > > Sent: Wednesday, November 14, 2018 5:50 AM > > > config PHY_RCAR_GEN3_USB2 > > > tristate "Renesas R-Car generation 3 USB 2.0 PHY driver" > > > depends on ARCH_RENESAS > > > - depends on EXTCON > > > + depends on EXTCON || ARCH_R7S9210 > > > > Does this mean that you don't want to use EXTCON if ARCH_R7S9210=y? > > EXTCON is not required for RZ/A2. So, I want to be able to leave EXTCON > un-selected (save flash space). I got it. I added the depend on EXTCON, but R-Car Gen3 environment can build EXTCON=n. However, I realized that build error happens if EXTCON=m. So, I think we have to revise this line as following at first: "depends on EXTCON || !EXTCON # if EXTCON=m, this cannot be built-in" like drivers/phy/qualcomm/Kconfig. > > > /* Initialize otg part */ > > > if (channel->is_otg_channel) > > > rcar_gen3_init_otg(channel); > > > + else > > > + /* No otg, so default to host mode */ > > > + writel(0x00000000, usb2_base + USB2_COMMCTRL); > > > > This "else" code will run on R-Car H3 USB port1(host only) for instance. > > I think adding this code is possible, but I'd like to separate patch from > > this adding R7S9210 support. What do you think? > > This code is required for RZ/A2. > The reason is the reset value for COMMCTRL in RZ/A2 is 0x80000000 (Peripheral mode). This value is the same as all R-Car Gen3 hardware. However, host only ports don't seem to use this value... > The reset value in R-Car H3 is 0x00000000 (Host mode). No. R-Car H3's reset value is also 0x80000000 (Peripheral mode). # Note that R-Car H3 can select the mode by HS-USB / UGCTRL2 register though... > Without this code, RZ/A2 will not work. I'd like to clarify this. Does this mean RZ/A2 will not work as host mode? (In other words, if we use the port as peripheral with the reset value 0x80000000, does it work?) Best regards, Yoshihiro Shimoda > > Chris
Hi Shimoda-san, > From: Yoshihiro Shimoda > Sent: Wednesday, November 14, 2018 7:24 AM > > > > config PHY_RCAR_GEN3_USB2 > > > > tristate "Renesas R-Car generation 3 USB 2.0 PHY driver" > > > > depends on ARCH_RENESAS > > > > - depends on EXTCON > > > > + depends on EXTCON || ARCH_R7S9210 > > > > > > Does this mean that you don't want to use EXTCON if ARCH_R7S9210=y? > > > > EXTCON is not required for RZ/A2. So, I want to be able to leave EXTCON > > un-selected (save flash space). > > I got it. > I added the depend on EXTCON, but R-Car Gen3 environment can build > EXTCON=n. > However, I realized that build error happens if EXTCON=m. > So, I think we have to revise this line as following at first: > > "depends on EXTCON || !EXTCON # if EXTCON=m, this cannot be built-in" > like drivers/phy/qualcomm/Kconfig. OK. I will change it. (should this be a separate patch?) > > Without this code, RZ/A2 will not work. > > I'd like to clarify this. Does this mean RZ/A2 will not work as host mode? Correct. If I remove 'else' code, RZ/A2 host mode does not work. I just tested again now. Host works: else /* No otg, so default to host mode */ writel(0x00000000, usb2_base + USB2_COMMCTRL); Host does NOT work: //else // /* No otg, so default to host mode */ // writel(0x00000000, usb2_base + USB2_COMMCTRL); > (In other words, if we use the port as peripheral with the reset value > 0x80000000, does it work?) We have not been able to get USB peripheral working on RZ/A2 yet. For peripheral, RZ/A2 has HS-USB. After plugging into the PC, HS-USB goes to Suspended state (DVSQ = "0110"). It should go to Configured state (DVSQ = "0011") According to the RZ/A2 Hardware Manual, COMMCTRL should be 0x80000000 when using HS-USB. There are 2 channels of USB on RZ/A2 (host x 2, HS_USB x 2) Chris
Hi Chris-san, > From: Chris Brandt, Sent: Wednesday, November 14, 2018 10:03 PM > > Hi Shimoda-san, > > > From: Yoshihiro Shimoda > > Sent: Wednesday, November 14, 2018 7:24 AM > > > > > config PHY_RCAR_GEN3_USB2 > > > > > tristate "Renesas R-Car generation 3 USB 2.0 PHY driver" > > > > > depends on ARCH_RENESAS > > > > > - depends on EXTCON > > > > > + depends on EXTCON || ARCH_R7S9210 > > > > > > > > Does this mean that you don't want to use EXTCON if ARCH_R7S9210=y? > > > > > > EXTCON is not required for RZ/A2. So, I want to be able to leave EXTCON > > > un-selected (save flash space). > > > > I got it. > > I added the depend on EXTCON, but R-Car Gen3 environment can build > > EXTCON=n. > > However, I realized that build error happens if EXTCON=m. > > So, I think we have to revise this line as following at first: > > > > "depends on EXTCON || !EXTCON # if EXTCON=m, this cannot be built-in" > > like drivers/phy/qualcomm/Kconfig. > > OK. I will change it. > > (should this be a separate patch?) Yes, this should be a separate patch. > > > Without this code, RZ/A2 will not work. > > > > I'd like to clarify this. Does this mean RZ/A2 will not work as host mode? > > Correct. If I remove 'else' code, RZ/A2 host mode does not work. > I just tested again now. > > Host works: > else > /* No otg, so default to host mode */ > writel(0x00000000, usb2_base + USB2_COMMCTRL); > > Host does NOT work: > //else > // /* No otg, so default to host mode */ > // writel(0x00000000, usb2_base + USB2_COMMCTRL); I got it. However, I have a concern how to set the mode to peripheral on RZ/A2 if we applied this code. If someone would like to use the USB as peripheral on his board, this code is not suitable. So, I have an idea to set the default mode by using "dr_mode" property, instead of hardcoded. Since the driver already has such a function, we can reuse rcar_gen3_device_recognition() to set the default value. To achieve that, we need to modify the following though. - Don't enable "is_otg_channel". - Don't call rcar_gen3_enable_vbus_ctrl() to avoid ADPCTRL register because RZ/A2 doesn't have it. - Don't need to call rcar_gen3_set_linectrl to avoid LINECTRL1 register because RZ/A2 doesn't seem to need the setting on host mode. What do you think? > > (In other words, if we use the port as peripheral with the reset value > > 0x80000000, does it work?) > > We have not been able to get USB peripheral working on RZ/A2 yet. > For peripheral, RZ/A2 has HS-USB. > After plugging into the PC, HS-USB goes to Suspended state (DVSQ = "0110"). > It should go to Configured state (DVSQ = "0011") I guess we need to modify ./drivers/usb/renesas_usbhs/rza.c for RZ/A2 because RZ/A2's HS-USB has SYSCFG.CNEN, but doesn't have SYSCFG.UPLLE? > According to the RZ/A2 Hardware Manual, COMMCTRL should be 0x80000000 when > using HS-USB. > > There are 2 channels of USB on RZ/A2 (host x 2, HS_USB x 2) I got it. So, I guess someone wants to use 1 host and 1 peripheral :) Best regards, Yoshihiro Shimoda > Chris
Hi Shimodaさん > From: Yoshihiro Shimoda > Sent: Thursday, November 15, 2018 4:20 AM > > Host does NOT work: > > //else > > // /* No otg, so default to host mode */ > > // writel(0x00000000, usb2_base + USB2_COMMCTRL); > > I got it. However, I have a concern how to set the mode to peripheral on > RZ/A2 > if we applied this code. If someone would like to use the USB as > peripheral > on his board, this code is not suitable. But USB peripheral is a different driver. So, this code will not run. So USB2_COMMCTRL will keep the default value 0x80000000. Correct? > So, I have an idea to set the default mode by using "dr_mode" property, > instead of hardcoded. Since the driver already has such a function, > we can reuse rcar_gen3_device_recognition() to set the default value. > To achieve that, we need to modify the following though. > - Don't enable "is_otg_channel". > - Don't call rcar_gen3_enable_vbus_ctrl() to avoid ADPCTRL register > because RZ/A2 doesn't have it. > - Don't need to call rcar_gen3_set_linectrl to avoid LINECTRL1 register > because RZ/A2 doesn't seem to need the setting on host mode. > > What do you think? If a board is designed for USB peripheral, why would they enable a EHCI host driver for the same USB channel? I am confused. > > > (In other words, if we use the port as peripheral with the reset value > > > 0x80000000, does it work?) > > > > We have not been able to get USB peripheral working on RZ/A2 yet. > > For peripheral, RZ/A2 has HS-USB. > > After plugging into the PC, HS-USB goes to Suspended state (DVSQ = > "0110"). > > It should go to Configured state (DVSQ = "0011") > > I guess we need to modify ./drivers/usb/renesas_usbhs/rza.c for RZ/A2 > because RZ/A2's HS-USB has SYSCFG.CNEN, but doesn't have SYSCFG.UPLLE? Today for RZ/A1, we tell people to use USB0 first in their board design, then use USB1 if they need a second USB channel. USB pins are dedicated (no other function) so there should be no design conflicts. If only USB1 is used, USB0 must also be enabled in DT as a dummy driver (so SYSCFG.UPLLE can get set for USB1). > > According to the RZ/A2 Hardware Manual, COMMCTRL should be 0x80000000 > when > > using HS-USB. > > > > There are 2 channels of USB on RZ/A2 (host x 2, HS_USB x 2) > > I got it. So, I guess someone wants to use 1 host and 1 peripheral :) Yes. Chris
Hi Chris-san, > From: Chris Brandt, Sent: Thursday, November 15, 2018 9:34 PM > > Hi Shimodaさん > > > From: Yoshihiro Shimoda > > Sent: Thursday, November 15, 2018 4:20 AM > > > > Host does NOT work: > > > //else > > > // /* No otg, so default to host mode */ > > > // writel(0x00000000, usb2_base + USB2_COMMCTRL); > > > > I got it. However, I have a concern how to set the mode to peripheral on > > RZ/A2 > > if we applied this code. If someone would like to use the USB as > > peripheral > > on his board, this code is not suitable. > > But USB peripheral is a different driver. So, this code will not run. So > USB2_COMMCTRL will keep the default value 0x80000000. > > Correct? Ah, this is you're correct. R-Car Gen3 code (drivers/usb/renesas_usbhs/rcar3.c) doesn't have phy control for now. > > So, I have an idea to set the default mode by using "dr_mode" property, > > instead of hardcoded. Since the driver already has such a function, > > we can reuse rcar_gen3_device_recognition() to set the default value. > > To achieve that, we need to modify the following though. > > - Don't enable "is_otg_channel". > > - Don't call rcar_gen3_enable_vbus_ctrl() to avoid ADPCTRL register > > because RZ/A2 doesn't have it. > > - Don't need to call rcar_gen3_set_linectrl to avoid LINECTRL1 register > > because RZ/A2 doesn't seem to need the setting on host mode. > > > > What do you think? > > If a board is designed for USB peripheral, why would they enable a EHCI > host driver for the same USB channel? > I am confused. I meant that USB peripheral of RZ/A2 need the phy driver, not a EHCI host driver. And I read Figure 32.1 of the RZ/A2 documentation and I wonder if we need to release USBCTR.PLL_RST even if we use USB peripheral mode. # Since the documentations doesn't mention the PHY area exactly, it's confusable for us though. # But, the phy driver assumed the driver handled "AHB Bridge", "Core" and "UCOM" registers. > > > > (In other words, if we use the port as peripheral with the reset value > > > > 0x80000000, does it work?) > > > > > > We have not been able to get USB peripheral working on RZ/A2 yet. > > > For peripheral, RZ/A2 has HS-USB. > > > After plugging into the PC, HS-USB goes to Suspended state (DVSQ = > > "0110"). > > > It should go to Configured state (DVSQ = "0011") > > > > I guess we need to modify ./drivers/usb/renesas_usbhs/rza.c for RZ/A2 > > because RZ/A2's HS-USB has SYSCFG.CNEN, but doesn't have SYSCFG.UPLLE? > > Today for RZ/A1, we tell people to use USB0 first in their board design, > then use USB1 if they need a second USB channel. USB pins are dedicated > (no other function) so there should be no design conflicts. > > If only USB1 is used, USB0 must also be enabled in DT as a dummy driver > (so SYSCFG.UPLLE can get set for USB1). Oh, I heard RZ/G1C also has such a hardware register [1]. [1] https://patchwork.kernel.org/patch/10655855/ RZ/G1C channel 0 only has UGCTRL register, but need to set for channel 1. Best regards, Yoshihiro Shimoda > > > According to the RZ/A2 Hardware Manual, COMMCTRL should be 0x80000000 > > when > > > using HS-USB. > > > > > > There are 2 channels of USB on RZ/A2 (host x 2, HS_USB x 2) > > > > I got it. So, I guess someone wants to use 1 host and 1 peripheral :) > > Yes. > > Chris
Hi Shimodaさん From: Yoshihiro Shimoda Sent: Monday, November 19, 2018 3:58 AM > And I read Figure 32.1 of the RZ/A2 documentation and I wonder if we need > to release > USBCTR.PLL_RST even if we use USB peripheral mode. I will ask the RZ/A2 design team to confirm. If this setting is required, I think we should use your suggestion: From: Yoshihiro Shimoda Sent: Thursday, November 15, 2018 4:20 AM > So, I have an idea to set the default mode by using "dr_mode" property, > instead of hardcoded. Since the driver already has such a function, > we can reuse rcar_gen3_device_recognition() to set the default value. > To achieve that, we need to modify the following though. > - Don't enable "is_otg_channel". > - Don't call rcar_gen3_enable_vbus_ctrl() to avoid ADPCTRL register > because RZ/A2 doesn't have it. > - Don't need to call rcar_gen3_set_linectrl to avoid LINECTRL1 register > because RZ/A2 doesn't seem to need the setting on host mode. Chris
diff --git a/drivers/phy/renesas/Kconfig b/drivers/phy/renesas/Kconfig index e340a925bbb1..beebeba31e84 100644 --- a/drivers/phy/renesas/Kconfig +++ b/drivers/phy/renesas/Kconfig @@ -19,7 +19,7 @@ config PHY_RCAR_GEN3_PCIE config PHY_RCAR_GEN3_USB2 tristate "Renesas R-Car generation 3 USB 2.0 PHY driver" depends on ARCH_RENESAS - depends on EXTCON + depends on EXTCON || ARCH_R7S9210 depends on USB_SUPPORT select GENERIC_PHY select USB_COMMON diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c index d0f412c25981..96ac75ba40ea 100644 --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c @@ -34,6 +34,7 @@ #define USB2_VBCTRL 0x60c #define USB2_LINECTRL1 0x610 #define USB2_ADPCTRL 0x630 +#define USB2_PHYCLK_CTRL 0x644 /* INT_ENABLE */ #define USB2_INT_ENABLE_UCOM_INTEN BIT(3) @@ -88,6 +89,7 @@ struct rcar_gen3_chan { bool extcon_host; bool is_otg_channel; bool uses_otg_pins; + bool uses_usb_x1; }; /* @@ -326,6 +328,9 @@ static int rcar_gen3_phy_usb2_init(struct phy *p) struct rcar_gen3_chan *channel = phy_get_drvdata(p); void __iomem *usb2_base = channel->base; + if (channel->uses_usb_x1) + writel(0x00000001, usb2_base + USB2_PHYCLK_CTRL); + /* Initialize USB2 part */ writel(USB2_INT_ENABLE_INIT, usb2_base + USB2_INT_ENABLE); writel(USB2_SPD_RSM_TIMSET_INIT, usb2_base + USB2_SPD_RSM_TIMSET); @@ -334,6 +339,9 @@ static int rcar_gen3_phy_usb2_init(struct phy *p) /* Initialize otg part */ if (channel->is_otg_channel) rcar_gen3_init_otg(channel); + else + /* No otg, so default to host mode */ + writel(0x00000000, usb2_base + USB2_COMMCTRL); return 0; } @@ -406,6 +414,7 @@ static irqreturn_t rcar_gen3_phy_usb2_irq(int irq, void *_ch) } static const struct of_device_id rcar_gen3_phy_usb2_match_table[] = { + { .compatible = "renesas,usb2-phy-r7s9210" }, { .compatible = "renesas,usb2-phy-r8a7795" }, { .compatible = "renesas,usb2-phy-r8a7796" }, { .compatible = "renesas,usb2-phy-r8a77965" }, @@ -471,6 +480,9 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev) } } + if (of_property_read_bool(dev->of_node, "renesas,uses_usb_x1")) + channel->uses_usb_x1 = true; + /* * devm_phy_create() will call pm_runtime_enable(&phy->dev); * And then, phy-core will manage runtime pm for this device.
The RZ/A2 has the same USB2 host controller as R-Car Gen3 with only some minor differences. Signed-off-by: Chris Brandt <chris.brandt@renesas.com> --- drivers/phy/renesas/Kconfig | 2 +- drivers/phy/renesas/phy-rcar-gen3-usb2.c | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-)