[PATCH 0/3] usb: renesas: rcar-gen3-usb2: Add support for RZ/A2

[PATCH 0/3] usb: renesas: rcar-gen3-usb2: Add support for RZ/A2

[Chris Brandt]

Add support for RZ/A2. Basically has the same IP as R-Car Gen3.


Chris Brandt (3):
  clk: renesas: r7s9210: Add USB clocks
  phy: renesas: rcar-gen3-usb2: Add support for R7S9210
  dt-bindings: rcar-gen3-phy-usb2: Add r7s9210 support

 Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt | 10 ++++++----
 drivers/clk/renesas/r7s9210-cpg-mssr.c                       |  2 ++
 drivers/phy/renesas/Kconfig                                  |  2 +-
 drivers/phy/renesas/phy-rcar-gen3-usb2.c                     | 12 ++++++++++++
 4 files changed, 21 insertions(+), 5 deletions(-)

-- 
2.16.1

[PATCH 1/3] clk: renesas: r7s9210: Add USB clocks

[Chris Brandt]

Add USB clocks for RZ/A2

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 drivers/clk/renesas/r7s9210-cpg-mssr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/renesas/r7s9210-cpg-mssr.c b/drivers/clk/renesas/r7s9210-cpg-mssr.c
index 3922967ba811..efbbf56e6766 100644
--- a/drivers/clk/renesas/r7s9210-cpg-mssr.c
+++ b/drivers/clk/renesas/r7s9210-cpg-mssr.c
@@ -87,6 +87,8 @@ static const struct mssr_mod_clk r7s9210_mod_clks[] __initconst = {
 	DEF_MOD_STB("scif1",	 46,	R7S9210_CLK_P1C),
 	DEF_MOD_STB("scif0",	 47,	R7S9210_CLK_P1C),
 
+	DEF_MOD_STB("usb1",	 60,	R7S9210_CLK_B),
+	DEF_MOD_STB("usb0",	 61,	R7S9210_CLK_B),
 	DEF_MOD_STB("ether1",	 64,	R7S9210_CLK_B),
 	DEF_MOD_STB("ether0",	 65,	R7S9210_CLK_B),
 
-- 
2.16.1

Re: [PATCH 1/3] clk: renesas: r7s9210: Add USB clocks

[Geert Uytterhoeven]

On Wed, Nov 7, 2018 at 6:37 PM Chris Brandt <chris.brandt@renesas.com> wrote:
> Add USB clocks for RZ/A2
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in clk-renesas-for-v4.21.

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

[PATCH 2/3] phy: renesas: rcar-gen3-usb2: Add support for R7S9210

[Chris Brandt]

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
 	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.
-- 
2.16.1

RE: [PATCH 2/3] phy: renesas: rcar-gen3-usb2: Add support for R7S9210

[Yoshihiro Shimoda]

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

RE: [PATCH 2/3] phy: renesas: rcar-gen3-usb2: Add support for R7S9210

[Chris Brandt]

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

RE: [PATCH 2/3] phy: renesas: rcar-gen3-usb2: Add support for R7S9210

[Yoshihiro Shimoda]

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

RE: [PATCH 2/3] phy: renesas: rcar-gen3-usb2: Add support for R7S9210

[Chris Brandt]

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

RE: [PATCH 2/3] phy: renesas: rcar-gen3-usb2: Add support for R7S9210

[Yoshihiro Shimoda]

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

RE: [PATCH 2/3] phy: renesas: rcar-gen3-usb2: Add support for R7S9210

[Chris Brandt]

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

RE: [PATCH 2/3] phy: renesas: rcar-gen3-usb2: Add support for R7S9210

[Yoshihiro Shimoda]

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

RE: [PATCH 2/3] phy: renesas: rcar-gen3-usb2: Add support for R7S9210

[Chris Brandt]

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

[PATCH 3/3] dt-bindings: rcar-gen3-phy-usb2: Add r7s9210 support

[Chris Brandt]

Document RZ/A2 (R7S9210) SoC bindings.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
index de7b5393c163..b545daf8ccb3 100644
--- a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
+++ b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
@@ -1,10 +1,12 @@
 * Renesas R-Car generation 3 USB 2.0 PHY
 
 This file provides information on what the device node for the R-Car generation
-3 and RZ/G2 USB 2.0 PHY contain.
+3, RZ/G2 and RZ/A2 USB 2.0 PHY contain.
 
 Required properties:
-- compatible: "renesas,usb2-phy-r8a774a1" if the device is a part of an R8A774A1
+- compatible: "renesas,usb2-phy-r7s9210" if the device is a part of an R7S9210
+	      SoC.
+	      "renesas,usb2-phy-r8a774a1" if the device is a part of an R8A774A1
 	      SoC.
 	      "renesas,usb2-phy-r8a7795" if the device is a part of an R8A7795
 	      SoC.
@@ -16,8 +18,8 @@ Required properties:
 	      R8A77990 SoC.
 	      "renesas,usb2-phy-r8a77995" if the device is a part of an
 	      R8A77995 SoC.
-	      "renesas,rcar-gen3-usb2-phy" for a generic R-Car Gen3 or RZ/G2
-	      compatible device.
+	      "renesas,rcar-gen3-usb2-phy" for a generic R-Car Gen3, RZ/G2 or
+	      RZ/A2 compatible device.
 
 	      When compatible with the generic version, nodes must list the
 	      SoC-specific version corresponding to the platform first
-- 
2.16.1

RE: [PATCH 3/3] dt-bindings: rcar-gen3-phy-usb2: Add r7s9210 support

[Yoshihiro Shimoda]

Hi Chris-san,

Thank you for the patch!

> From: Chris Brandt, Sent: Thursday, November 8, 2018 2:36 AM
> 
> Document RZ/A2 (R7S9210) SoC bindings.
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
>  Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
> b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
> index de7b5393c163..b545daf8ccb3 100644
> --- a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
> +++ b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
> @@ -1,10 +1,12 @@
>  * Renesas R-Car generation 3 USB 2.0 PHY
> 
>  This file provides information on what the device node for the R-Car generation
> -3 and RZ/G2 USB 2.0 PHY contain.
> +3, RZ/G2 and RZ/A2 USB 2.0 PHY contain.
> 
>  Required properties:
> -- compatible: "renesas,usb2-phy-r8a774a1" if the device is a part of an R8A774A1
> +- compatible: "renesas,usb2-phy-r7s9210" if the device is a part of an R7S9210
> +	      SoC.
> +	      "renesas,usb2-phy-r8a774a1" if the device is a part of an R8A774A1
>  	      SoC.
>  	      "renesas,usb2-phy-r8a7795" if the device is a part of an R8A7795
>  	      SoC.
> @@ -16,8 +18,8 @@ Required properties:
>  	      R8A77990 SoC.
>  	      "renesas,usb2-phy-r8a77995" if the device is a part of an
>  	      R8A77995 SoC.
> -	      "renesas,rcar-gen3-usb2-phy" for a generic R-Car Gen3 or RZ/G2
> -	      compatible device.
> +	      "renesas,rcar-gen3-usb2-phy" for a generic R-Car Gen3, RZ/G2 or
> +	      RZ/A2 compatible device.
> 
>  	      When compatible with the generic version, nodes must list the
>  	      SoC-specific version corresponding to the platform first
> --
> 2.16.1

As I mentioned the PATCH 2/3, we should describe a new property "renesas,uses_usb_x1"
as optional properties here.

Best regards,
Yoshihiro Shimoda

Re: [PATCH 3/3] dt-bindings: rcar-gen3-phy-usb2: Add r7s9210 support

[Geert Uytterhoeven]

Hi Chris,

On Wed, Nov 7, 2018 at 6:36 PM Chris Brandt <chris.brandt@renesas.com> wrote:
> Document RZ/A2 (R7S9210) SoC bindings.
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
>  Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
> index de7b5393c163..b545daf8ccb3 100644
> --- a/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
> +++ b/Documentation/devicetree/bindings/phy/rcar-gen3-phy-usb2.txt
> @@ -1,10 +1,12 @@
>  * Renesas R-Car generation 3 USB 2.0 PHY
>
>  This file provides information on what the device node for the R-Car generation
> -3 and RZ/G2 USB 2.0 PHY contain.
> +3, RZ/G2 and RZ/A2 USB 2.0 PHY contain.
>
>  Required properties:
> -- compatible: "renesas,usb2-phy-r8a774a1" if the device is a part of an R8A774A1
> +- compatible: "renesas,usb2-phy-r7s9210" if the device is a part of an R7S9210
> +             SoC.
> +             "renesas,usb2-phy-r8a774a1" if the device is a part of an R8A774A1
>               SoC.
>               "renesas,usb2-phy-r8a7795" if the device is a part of an R8A7795
>               SoC.
> @@ -16,8 +18,8 @@ Required properties:
>               R8A77990 SoC.
>               "renesas,usb2-phy-r8a77995" if the device is a part of an
>               R8A77995 SoC.
> -             "renesas,rcar-gen3-usb2-phy" for a generic R-Car Gen3 or RZ/G2
> -             compatible device.
> +             "renesas,rcar-gen3-usb2-phy" for a generic R-Car Gen3, RZ/G2 or
> +             RZ/A2 compatible device.

Is it a good idea to declare RZ/A2 compatible to R-Car Gen3?
Usually we don't do that for Renesas IP cores used in different families[*].
Of course, I know you do have a good relationship with the actual RZ/A2
hardware designers ;-)

In light of Shimoda-san's comment w.r.t. R-Car H3 USB port1 in the driver
code, perhaps "renesas,rcar-gen3-usb2-phy" should not be used?

[*] Sole exceptions I'm aware of are:
      - "renesas,rmobile-iic", in addition to "renesas,rcar-gen2-iic" or
        "renesas,rcar-gen3-iic" on R-Car Gen2/3,
      - "renesas,rcar-gen2-cmt1" and "renesas,rcar-thermal" on R-Mobile APE6.

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 3/3] dt-bindings: rcar-gen3-phy-usb2: Add r7s9210 support

[Chris Brandt]

Hi Geert,

On Wednesday, November 14, 2018, Geert Uytterhoeven wrote:
> >  Required properties:
> > -- compatible: "renesas,usb2-phy-r8a774a1" if the device is a part of an
> R8A774A1
> > +- compatible: "renesas,usb2-phy-r7s9210" if the device is a part of an
> R7S9210
> > +             SoC.
> > +             "renesas,usb2-phy-r8a774a1" if the device is a part of an
> R8A774A1
> >               SoC.
> >               "renesas,usb2-phy-r8a7795" if the device is a part of an
> R8A7795
> >               SoC.
> > @@ -16,8 +18,8 @@ Required properties:
> >               R8A77990 SoC.
> >               "renesas,usb2-phy-r8a77995" if the device is a part of an
> >               R8A77995 SoC.
> > -             "renesas,rcar-gen3-usb2-phy" for a generic R-Car Gen3 or
> RZ/G2
> > -             compatible device.
> > +             "renesas,rcar-gen3-usb2-phy" for a generic R-Car Gen3,
> RZ/G2 or
> > +             RZ/A2 compatible device.
> 
> Is it a good idea to declare RZ/A2 compatible to R-Car Gen3?
> Usually we don't do that for Renesas IP cores used in different
> families[*].
> Of course, I know you do have a good relationship with the actual RZ/A2
> hardware designers ;-)
> 
> In light of Shimoda-san's comment w.r.t. R-Car H3 USB port1 in the driver
> code, perhaps "renesas,rcar-gen3-usb2-phy" should not be used?

Well, I think the IP block seems the same....however...
I did point out that the COMMCTRL register defaults to a different 
value. No idea why that is. Honestly, I did not go and check all the other 
registers bits. So I guess I wonder what "compatible" really means:

"Compatible: Different, but close enough that it will work" ???

Chris

RE: [PATCH 3/3] dt-bindings: rcar-gen3-phy-usb2: Add r7s9210 support

[Yoshihiro Shimoda]

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Wednesday, November 14, 2018 8:03 PM
> 
> Hi Chris,
> 
> On Wed, Nov 7, 2018 at 6:36 PM Chris Brandt <chris.brandt@renesas.com> wrote:
<snip>
> > @@ -16,8 +18,8 @@ Required properties:
> >               R8A77990 SoC.
> >               "renesas,usb2-phy-r8a77995" if the device is a part of an
> >               R8A77995 SoC.
> > -             "renesas,rcar-gen3-usb2-phy" for a generic R-Car Gen3 or RZ/G2
> > -             compatible device.
> > +             "renesas,rcar-gen3-usb2-phy" for a generic R-Car Gen3, RZ/G2 or
> > +             RZ/A2 compatible device.
> 
> Is it a good idea to declare RZ/A2 compatible to R-Car Gen3?
> Usually we don't do that for Renesas IP cores used in different families[*].
> Of course, I know you do have a good relationship with the actual RZ/A2
> hardware designers ;-)
> 
> In light of Shimoda-san's comment w.r.t. R-Car H3 USB port1 in the driver
> code, perhaps "renesas,rcar-gen3-usb2-phy" should not be used?

Sorry for lack explanation. 
 - The default value of COMMCTRL on each port of R-Car Gen3 is the same (0x80000000).
 - However, R-Car H3 USB port1 seems to ignore this value because
   the port always acts as host. If I changed the register to 0 on R-Car H3 ES3.0,
   the port1 can work as host.

So, I meant all "renesas,rcar-gen3-usb2-phy" devices without "is_otg_channel"
can set the COMMCTRL register to 0.

Of course, adding a new RZ/A2 family compatible is OK to me.

Best regards,
Yoshihiro Shimoda

> [*] Sole exceptions I'm aware of are:
>       - "renesas,rmobile-iic", in addition to "renesas,rcar-gen2-iic" or
>         "renesas,rcar-gen3-iic" on R-Car Gen2/3,
>       - "renesas,rcar-gen2-cmt1" and "renesas,rcar-thermal" on R-Mobile APE6.
> 
> 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