[PATCH 0/4] Add USB-HOST support to cat874

[PATCH 0/4] Add USB-HOST support to cat874

[Fabrizio Castro]

While trying to add USB-HOST support to the cat874 board,
it came up that of_usb_get_dr_mode_by_phy was selecting
the wrong DT node to use for dr_mode.
Also, drivers/phy/renesas/phy-rcar-gen3-usb2.c was registering
IRQs, no matter the dr_mode.

This series adds all that is required to add USB-HOST support
to the cat874 board, and also removes the hsusb from Draak's DT
as not necessary anymore.

Thanks,
Fab

Fabrizio Castro (4):
  usb: common: Consider only available nodes for dr_mode
  phy: renesas: rcar-gen3-usb2: No need to request IRQ for non-OTG
  arm64: dts: renesas: r8a774c0-cat874: Add USB-HOST support
  arm64: dts: renesas: r8a77995: draak: Remove hsusb node

 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 15 ++++++++++++++
 arch/arm64/boot/dts/renesas/r8a77995-draak.dts  |  5 -----
 drivers/phy/renesas/phy-rcar-gen3-usb2.c        | 26 ++++++++++++-------------
 drivers/usb/common/common.c                     |  2 ++
 4 files changed, 30 insertions(+), 18 deletions(-)

-- 
2.7.4

[PATCH 1/4] usb: common: Consider only available nodes for dr_mode

[Fabrizio Castro]

There are cases where multiple device tree nodes point to the
same phy node by means of the "phys" property, but we should
only consider those nodes that are marked as available rather
than just any node.

Fixes: 98bfb3946695 ("usb: of: add an api to get dr_mode by the phy node")
Cc: stable@vger.kernel.org # v4.4+
Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
---
 drivers/usb/common/common.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index 48277bb..73c8e65 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -145,6 +145,8 @@ enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *np, int arg0)
 
 	do {
 		controller = of_find_node_with_property(controller, "phys");
+		if (!of_device_is_available(controller))
+			continue;
 		index = 0;
 		do {
 			if (arg0 == -1) {
-- 
2.7.4

[PATCH 2/4] phy: renesas: rcar-gen3-usb2: No need to request IRQ for non-OTG

[Fabrizio Castro]

As stated in the comment found on top of rcar_gen3_phy_usb2_work,
we should not be registering the IRQ if !otg_channel || !uses_otg_pins.
On top of that, we should also make sure that extcon has been
allocated before requesting the irq as rcar_gen3_phy_usb2_work
uses it, hence the patch.

Fixes: 2b38543c8db1 ("phy: rcar-gen3-usb2: add extcon support")
Fixes: 73801b90a38f ("phy: renesas: rcar-gen3-usb2: change a condition "dr_mode"")
Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
---
 drivers/phy/renesas/phy-rcar-gen3-usb2.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
index 0a34782..be1a392 100644
--- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
+++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
@@ -447,20 +447,8 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
 	if (IS_ERR(channel->base))
 		return PTR_ERR(channel->base);
 
-	/* call request_irq for OTG */
-	irq = platform_get_irq(pdev, 0);
-	if (irq >= 0) {
-		INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work);
-		irq = devm_request_irq(dev, irq, rcar_gen3_phy_usb2_irq,
-				       IRQF_SHARED, dev_name(dev), channel);
-		if (irq < 0)
-			dev_err(dev, "No irq handler (%d)\n", irq);
-	}
-
 	channel->dr_mode = of_usb_get_dr_mode_by_phy(dev->of_node, 0);
-	if (channel->dr_mode != USB_DR_MODE_UNKNOWN) {
-		int ret;
-
+	if (channel->dr_mode == USB_DR_MODE_OTG) {
 		channel->is_otg_channel = true;
 		channel->uses_otg_pins = !of_property_read_bool(dev->of_node,
 							"renesas,no-otg-pins");
@@ -476,6 +464,18 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
 		}
 	}
 
+	if (channel->is_otg_channel && channel->uses_otg_pins) {
+		/* call request_irq for OTG */
+		irq = platform_get_irq(pdev, 0);
+		if (irq >= 0) {
+			INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work);
+			irq = devm_request_irq(dev, irq, rcar_gen3_phy_usb2_irq,
+					       IRQF_SHARED, dev_name(dev), channel);
+			if (irq < 0)
+				dev_err(dev, "No irq handler (%d)\n", irq);
+		}
+	}
+
 	/*
 	 * devm_phy_create() will call pm_runtime_enable(&phy->dev);
 	 * And then, phy-core will manage runtime pm for this device.
-- 
2.7.4

[PATCH 3/4] arm64: dts: renesas: r8a774c0-cat874: Add USB-HOST support

[Fabrizio Castro]

This patch adds USB 2.0 HOST support to the CAT874 board.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
index f86b3628..7d3b76f 100644
--- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
@@ -76,10 +76,20 @@
 	};
 };
 
+&ehci0 {
+	dr_mode = "host";
+	status = "okay";
+};
+
 &extal_clk {
 	clock-frequency = <48000000>;
 };
 
+&ohci0 {
+	dr_mode = "host";
+	status = "okay";
+};
+
 &pcie_bus_clk {
 	clock-frequency = <100000000>;
 };
@@ -128,3 +138,8 @@
 	sd-uhs-sdr104;
 	status = "okay";
 };
+
+&usb2_phy0 {
+	renesas,no-otg-pins;
+	status = "okay";
+};
-- 
2.7.4

[PATCH 4/4] arm64: dts: renesas: r8a77995: draak: Remove hsusb node

[Fabrizio Castro]

Patch "usb: common: Consider only available nodes for dr_mode"
fixes the way the DT gets parsed to identify the USB controller
driving the PHY, as such we don't need hsusb to specify dr_mode,
in fact we don't need to enable the hsusb node at all since
the USB interface on the board works as USB host only.

Fixes: 5c6479d9b25b ("arm64: dts: renesas: r8a7799{0|5}: add/enable USB2.0 peripheral")
Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index db2bed1..5f13b8b 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -207,11 +207,6 @@
 	clock-frequency = <48000000>;
 };
 
-&hsusb {
-	dr_mode = "host";
-	status = "okay";
-};
-
 &i2c0 {
 	pinctrl-0 = <&i2c0_pins>;
 	pinctrl-names = "default";
-- 
2.7.4

Re: [PATCH 0/4] Add USB-HOST support to cat874

[Simon Horman]

On Fri, Mar 01, 2019 at 11:05:44AM +0000, Fabrizio Castro wrote:
> While trying to add USB-HOST support to the cat874 board,
> it came up that of_usb_get_dr_mode_by_phy was selecting
> the wrong DT node to use for dr_mode.
> Also, drivers/phy/renesas/phy-rcar-gen3-usb2.c was registering
> IRQs, no matter the dr_mode.
> 
> This series adds all that is required to add USB-HOST support
> to the cat874 board, and also removes the hsusb from Draak's DT
> as not necessary anymore.
> 
> Thanks,
> Fab
> 
> Fabrizio Castro (4):
>   usb: common: Consider only available nodes for dr_mode
>   phy: renesas: rcar-gen3-usb2: No need to request IRQ for non-OTG

Hi Fabrizio,

I am marking the patches below deferred pending acceptance of the patches
above.

>   arm64: dts: renesas: r8a774c0-cat874: Add USB-HOST support
>   arm64: dts: renesas: r8a77995: draak: Remove hsusb node
> 
>  arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 15 ++++++++++++++
>  arch/arm64/boot/dts/renesas/r8a77995-draak.dts  |  5 -----
>  drivers/phy/renesas/phy-rcar-gen3-usb2.c        | 26 ++++++++++++-------------
>  drivers/usb/common/common.c                     |  2 ++
>  4 files changed, 30 insertions(+), 18 deletions(-)
> 
> -- 
> 2.7.4
> 

Re: [PATCH 2/4] phy: renesas: rcar-gen3-usb2: No need to request IRQ for non-OTG

[Kishon Vijay Abraham I]

On 01/03/19 4:35 PM, Fabrizio Castro wrote:
> As stated in the comment found on top of rcar_gen3_phy_usb2_work,
> we should not be registering the IRQ if !otg_channel || !uses_otg_pins.
> On top of that, we should also make sure that extcon has been
> allocated before requesting the irq as rcar_gen3_phy_usb2_work
> uses it, hence the patch.
> 
> Fixes: 2b38543c8db1 ("phy: rcar-gen3-usb2: add extcon support")
> Fixes: 73801b90a38f ("phy: renesas: rcar-gen3-usb2: change a condition "dr_mode"")
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

merged, thanks.

-Kishon
> ---
>  drivers/phy/renesas/phy-rcar-gen3-usb2.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> index 0a34782..be1a392 100644
> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> @@ -447,20 +447,8 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
>  	if (IS_ERR(channel->base))
>  		return PTR_ERR(channel->base);
>  
> -	/* call request_irq for OTG */
> -	irq = platform_get_irq(pdev, 0);
> -	if (irq >= 0) {
> -		INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work);
> -		irq = devm_request_irq(dev, irq, rcar_gen3_phy_usb2_irq,
> -				       IRQF_SHARED, dev_name(dev), channel);
> -		if (irq < 0)
> -			dev_err(dev, "No irq handler (%d)\n", irq);
> -	}
> -
>  	channel->dr_mode = of_usb_get_dr_mode_by_phy(dev->of_node, 0);
> -	if (channel->dr_mode != USB_DR_MODE_UNKNOWN) {
> -		int ret;
> -
> +	if (channel->dr_mode == USB_DR_MODE_OTG) {
>  		channel->is_otg_channel = true;
>  		channel->uses_otg_pins = !of_property_read_bool(dev->of_node,
>  							"renesas,no-otg-pins");
> @@ -476,6 +464,18 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	if (channel->is_otg_channel && channel->uses_otg_pins) {
> +		/* call request_irq for OTG */
> +		irq = platform_get_irq(pdev, 0);
> +		if (irq >= 0) {
> +			INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work);
> +			irq = devm_request_irq(dev, irq, rcar_gen3_phy_usb2_irq,
> +					       IRQF_SHARED, dev_name(dev), channel);
> +			if (irq < 0)
> +				dev_err(dev, "No irq handler (%d)\n", irq);
> +		}
> +	}
> +
>  	/*
>  	 * devm_phy_create() will call pm_runtime_enable(&phy->dev);
>  	 * And then, phy-core will manage runtime pm for this device.
> 

RE: [PATCH 1/4] usb: common: Consider only available nodes for dr_mode

[Yoshihiro Shimoda]

Hi Fabrizio-san,

Thank you for the patch!

> From: Fabrizio Castro, Sent: Friday, March 1, 2019 8:06 PM
> 
> There are cases where multiple device tree nodes point to the
> same phy node by means of the "phys" property, but we should
> only consider those nodes that are marked as available rather
> than just any node.
> 
> Fixes: 98bfb3946695 ("usb: of: add an api to get dr_mode by the phy node")
> Cc: stable@vger.kernel.org # v4.4+
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> ---

I'm guessing this code needs for phy-rcar-gen3-usb2.c only because
the phy driver only gets the dr_mode from index 0 like below:

	channel->dr_mode = of_usb_get_dr_mode_by_phy(dev->of_node, 0);

Yesterday, I submitted patches to get multiple indexes from controller
device nodes:

https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=99561

So, would you check the phy patches can get the dr_mode without this changing common patch?

Best regards,
Yoshihiro Shimoda

>  drivers/usb/common/common.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> index 48277bb..73c8e65 100644
> --- a/drivers/usb/common/common.c
> +++ b/drivers/usb/common/common.c
> @@ -145,6 +145,8 @@ enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *np, int arg0)
> 
>  	do {
>  		controller = of_find_node_with_property(controller, "phys");
> +		if (!of_device_is_available(controller))
> +			continue;
>  		index = 0;
>  		do {
>  			if (arg0 == -1) {
> --
> 2.7.4

RE: [PATCH 2/4] phy: renesas: rcar-gen3-usb2: No need to request IRQ for non-OTG

[Yoshihiro Shimoda]

Hi Kishon, Fabrizio,

> From: Kishon Vijay Abraham I, Sent: Monday, April 1, 2019 9:26 PM
> 
> On 01/03/19 4:35 PM, Fabrizio Castro wrote:
> > As stated in the comment found on top of rcar_gen3_phy_usb2_work,
> > we should not be registering the IRQ if !otg_channel || !uses_otg_pins.
> > On top of that, we should also make sure that extcon has been
> > allocated before requesting the irq as rcar_gen3_phy_usb2_work
> > uses it, hence the patch.
> >
> > Fixes: 2b38543c8db1 ("phy: rcar-gen3-usb2: add extcon support")
> > Fixes: 73801b90a38f ("phy: renesas: rcar-gen3-usb2: change a condition "dr_mode"")
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> 
> merged, thanks.

I'm very sorry, I didn't notice this patch...
I'd like to fix this patch because this patch breaks for R-Car D3.
In this case, should I submit an incremental patch?

> -Kishon
> > ---
> >  drivers/phy/renesas/phy-rcar-gen3-usb2.c | 26 +++++++++++++-------------
> >  1 file changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> > index 0a34782..be1a392 100644
> > --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> > +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> > @@ -447,20 +447,8 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
> >  	if (IS_ERR(channel->base))
> >  		return PTR_ERR(channel->base);
> >
> > -	/* call request_irq for OTG */
> > -	irq = platform_get_irq(pdev, 0);
> > -	if (irq >= 0) {
> > -		INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work);
> > -		irq = devm_request_irq(dev, irq, rcar_gen3_phy_usb2_irq,
> > -				       IRQF_SHARED, dev_name(dev), channel);
> > -		if (irq < 0)
> > -			dev_err(dev, "No irq handler (%d)\n", irq);
> > -	}
> > -
> >  	channel->dr_mode = of_usb_get_dr_mode_by_phy(dev->of_node, 0);
> > -	if (channel->dr_mode != USB_DR_MODE_UNKNOWN) {
> > -		int ret;
> > -
> > +	if (channel->dr_mode == USB_DR_MODE_OTG) {

R-Car D3 (r8a77995-draak.dts) has dr_mode with "host". So, the board can use host mode
as default and "role" mode to switch the role for peripheral later. However, after we
applied this patch, the R-Car D3 USB2.0 cannot switch to the peripheral mode.

Best regards,
Yoshihiro Shimoda

> >  		channel->is_otg_channel = true;
> >  		channel->uses_otg_pins = !of_property_read_bool(dev->of_node,
> >  							"renesas,no-otg-pins");
> > @@ -476,6 +464,18 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
> >  		}
> >  	}
> >
> > +	if (channel->is_otg_channel && channel->uses_otg_pins) {
> > +		/* call request_irq for OTG */
> > +		irq = platform_get_irq(pdev, 0);
> > +		if (irq >= 0) {
> > +			INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work);
> > +			irq = devm_request_irq(dev, irq, rcar_gen3_phy_usb2_irq,
> > +					       IRQF_SHARED, dev_name(dev), channel);
> > +			if (irq < 0)
> > +				dev_err(dev, "No irq handler (%d)\n", irq);
> > +		}
> > +	}
> > +
> >  	/*
> >  	 * devm_phy_create() will call pm_runtime_enable(&phy->dev);
> >  	 * And then, phy-core will manage runtime pm for this device.
> >

RE: [PATCH 1/4] usb: common: Consider only available nodes for dr_mode

[Fabrizio Castro]

Hi Yoshihiro-san,

Thank you for your feedback.

> From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Sent: 02 April 2019 02:54
> Subject: RE: [PATCH 1/4] usb: common: Consider only available nodes for dr_mode
> 
> Hi Fabrizio-san,
> 
> Thank you for the patch!
> 
> > From: Fabrizio Castro, Sent: Friday, March 1, 2019 8:06 PM
> >
> > There are cases where multiple device tree nodes point to the
> > same phy node by means of the "phys" property, but we should
> > only consider those nodes that are marked as available rather
> > than just any node.
> >
> > Fixes: 98bfb3946695 ("usb: of: add an api to get dr_mode by the phy node")
> > Cc: stable@vger.kernel.org # v4.4+
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > ---
> 
> I'm guessing this code needs for phy-rcar-gen3-usb2.c only because
> the phy driver only gets the dr_mode from index 0 like below:
> 
> 	channel->dr_mode = of_usb_get_dr_mode_by_phy(dev->of_node, 0);
> 
> Yesterday, I submitted patches to get multiple indexes from controller
> device nodes:
> 
> https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=99561
> 
> So, would you check the phy patches can get the dr_mode without this changing common patch?

I will go through your patch series, but I can't see why any driver would be interested
in considering a disabled node for getting dr_mode, can you?
Do you have a use case for this?

Thanks,
Fab

> 
> Best regards,
> Yoshihiro Shimoda
> 
> >  drivers/usb/common/common.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 48277bb..73c8e65 100644
> > --- a/drivers/usb/common/common.c
> > +++ b/drivers/usb/common/common.c
> > @@ -145,6 +145,8 @@ enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *np, int arg0)
> >
> >  	do {
> >  		controller = of_find_node_with_property(controller, "phys");
> > +		if (!of_device_is_available(controller))
> > +			continue;
> >  		index = 0;
> >  		do {
> >  			if (arg0 == -1) {
> > --
> > 2.7.4

RE: [PATCH 1/4] usb: common: Consider only available nodes for dr_mode

[Yoshihiro Shimoda]

Hi Fabrizio-san,

> From: Fabrizio Castro, Sent: Tuesday, April 2, 2019 6:22 PM
> 
> Hi Yoshihiro-san,
> 
> Thank you for your feedback.
> 
> > From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > Sent: 02 April 2019 02:54
> > Subject: RE: [PATCH 1/4] usb: common: Consider only available nodes for dr_mode
> >
> > Hi Fabrizio-san,
> >
> > Thank you for the patch!
> >
> > > From: Fabrizio Castro, Sent: Friday, March 1, 2019 8:06 PM
> > >
> > > There are cases where multiple device tree nodes point to the
> > > same phy node by means of the "phys" property, but we should
> > > only consider those nodes that are marked as available rather
> > > than just any node.
> > >
> > > Fixes: 98bfb3946695 ("usb: of: add an api to get dr_mode by the phy node")
> > > Cc: stable@vger.kernel.org # v4.4+
> > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > ---
> >
> > I'm guessing this code needs for phy-rcar-gen3-usb2.c only because
> > the phy driver only gets the dr_mode from index 0 like below:
> >
> > 	channel->dr_mode = of_usb_get_dr_mode_by_phy(dev->of_node, 0);
> >
> > Yesterday, I submitted patches to get multiple indexes from controller
> > device nodes:
> >
> > https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=99561
> >
> > So, would you check the phy patches can get the dr_mode without this changing common patch?
> 
> I will go through your patch series, but I can't see why any driver would be interested
> in considering a disabled node for getting dr_mode, can you?
> Do you have a use case for this?

This is not a use case though, the commit 98bfb3946695 is made by a TI engineer so that
I checked TI's dts/dtsi file on armv7 arch (am335x-boneblue.dts) a little and then
it has one usb controller node only. So, the commit didn't take care of a disabled node, I guess.
However, as you know, on R-Car Gen3 and RZ/G2, they have multiple controller nodes. So,
one of the controller can be disabled.

By the way, if we applied this patch, since the behavior will be changed at least,
so all of this api user have to test whether this code doesn't break or not.

Best regards,
Yoshihiro Shimoda

> Thanks,
> Fab
> 
> >
> > Best regards,
> > Yoshihiro Shimoda
> >
> > >  drivers/usb/common/common.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > > index 48277bb..73c8e65 100644
> > > --- a/drivers/usb/common/common.c
> > > +++ b/drivers/usb/common/common.c
> > > @@ -145,6 +145,8 @@ enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *np, int arg0)
> > >
> > >  	do {
> > >  		controller = of_find_node_with_property(controller, "phys");
> > > +		if (!of_device_is_available(controller))
> > > +			continue;
> > >  		index = 0;
> > >  		do {
> > >  			if (arg0 == -1) {
> > > --
> > > 2.7.4

RE: [PATCH 1/4] usb: common: Consider only available nodes for dr_mode

[Fabrizio Castro]

Hi Yoshihiro-san,

Thank you for your feedback.

> -----Original Message-----
> From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Sent: 02 April 2019 12:10
> To: Fabrizio Castro <fabrizio.castro@bp.renesas.com>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Geert Uytterhoeven
> <geert+renesas@glider.be>
> Cc: Arnd Bergmann <arnd@arndb.de>; linux-usb@vger.kernel.org; Simon Horman <horms@verge.net.au>; Chris Paterson
> <Chris.Paterson2@renesas.com>; Biju Das <biju.das@bp.renesas.com>; linux-renesas-soc@vger.kernel.org; stable@vger.kernel.org
> Subject: RE: [PATCH 1/4] usb: common: Consider only available nodes for dr_mode
> 
> Hi Fabrizio-san,
> 
> > From: Fabrizio Castro, Sent: Tuesday, April 2, 2019 6:22 PM
> >
> > Hi Yoshihiro-san,
> >
> > Thank you for your feedback.
> >
> > > From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > Sent: 02 April 2019 02:54
> > > Subject: RE: [PATCH 1/4] usb: common: Consider only available nodes for dr_mode
> > >
> > > Hi Fabrizio-san,
> > >
> > > Thank you for the patch!
> > >
> > > > From: Fabrizio Castro, Sent: Friday, March 1, 2019 8:06 PM
> > > >
> > > > There are cases where multiple device tree nodes point to the
> > > > same phy node by means of the "phys" property, but we should
> > > > only consider those nodes that are marked as available rather
> > > > than just any node.
> > > >
> > > > Fixes: 98bfb3946695 ("usb: of: add an api to get dr_mode by the phy node")
> > > > Cc: stable@vger.kernel.org # v4.4+
> > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > > ---
> > >
> > > I'm guessing this code needs for phy-rcar-gen3-usb2.c only because
> > > the phy driver only gets the dr_mode from index 0 like below:
> > >
> > > 	channel->dr_mode = of_usb_get_dr_mode_by_phy(dev->of_node, 0);
> > >
> > > Yesterday, I submitted patches to get multiple indexes from controller
> > > device nodes:
> > >
> > > https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=99561
> > >
> > > So, would you check the phy patches can get the dr_mode without this changing common patch?
> >
> > I will go through your patch series, but I can't see why any driver would be interested
> > in considering a disabled node for getting dr_mode, can you?
> > Do you have a use case for this?
> 
> This is not a use case though, the commit 98bfb3946695 is made by a TI engineer so that
> I checked TI's dts/dtsi file on armv7 arch (am335x-boneblue.dts) a little and then
> it has one usb controller node only. So, the commit didn't take care of a disabled node, I guess.
> However, as you know, on R-Car Gen3 and RZ/G2, they have multiple controller nodes. So,
> one of the controller can be disabled.

The real question is, are we going to need to read dr_mode from a disabled node?
Because this patch gets in the way of that, but looking through the code and
device trees it doesn't look like anybody needs that. I have tested this patch in isolation
on a few R-Car platforms, and it all seemed good to me back when I submitted the patch,
but it would probably need thorough testing, and more testing from your side is more than
welcome. Besides this point, of_usb_get_dr_mode_by_phy returns the dr_mode value that
matches the argument the first, which means what you get back from it depends from
the order in which you specify the nodes within the device tree in case you have multiple
nodes meeting the same requirements (from a of_usb_get_dr_mode_by_phy perspective),
a.k.a. "luck".

We should make sure there is only one node that can match our requirement at any one
time for this to work properly, and parsing only enabled nodes gets in that direction,
therefore I still think this patch is valid, and belongs to stable versions of the kernel
as well.

After reading your comment on patch:
"phy: renesas: rcar-gen3-usb2: No need to request IRQ for non-OTG"

I can now see that patch "arm64: dts: renesas: r8a77995: draak: Remove hsusb node" is wrong
for your case, and that we need to find a better solution for patch:
"phy: renesas: rcar-gen3-usb2: No need to request IRQ for non-OTG"

I am looking into your series now, I'll come back to you soon on the other patch.

Thanks,
Fab

> 
> By the way, if we applied this patch, since the behavior will be changed at least,
> so all of this api user have to test whether this code doesn't break or not.
> 
> Best regards,
> Yoshihiro Shimoda
> 
> > Thanks,
> > Fab
> >
> > >
> > > Best regards,
> > > Yoshihiro Shimoda
> > >
> > > >  drivers/usb/common/common.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > > > index 48277bb..73c8e65 100644
> > > > --- a/drivers/usb/common/common.c
> > > > +++ b/drivers/usb/common/common.c
> > > > @@ -145,6 +145,8 @@ enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *np, int arg0)
> > > >
> > > >  	do {
> > > >  		controller = of_find_node_with_property(controller, "phys");
> > > > +		if (!of_device_is_available(controller))
> > > > +			continue;
> > > >  		index = 0;
> > > >  		do {
> > > >  			if (arg0 == -1) {
> > > > --
> > > > 2.7.4

RE: [PATCH 2/4] phy: renesas: rcar-gen3-usb2: No need to request IRQ for non-OTG

[Fabrizio Castro]

Hello Yoshihiro-san, Kishon

> From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Sent: 02 April 2019 03:11
> Subject: RE: [PATCH 2/4] phy: renesas: rcar-gen3-usb2: No need to request IRQ for non-OTG
> 
> Hi Kishon, Fabrizio,
> 
> > From: Kishon Vijay Abraham I, Sent: Monday, April 1, 2019 9:26 PM
> >
> > On 01/03/19 4:35 PM, Fabrizio Castro wrote:
> > > As stated in the comment found on top of rcar_gen3_phy_usb2_work,
> > > we should not be registering the IRQ if !otg_channel || !uses_otg_pins.
> > > On top of that, we should also make sure that extcon has been
> > > allocated before requesting the irq as rcar_gen3_phy_usb2_work
> > > uses it, hence the patch.
> > >
> > > Fixes: 2b38543c8db1 ("phy: rcar-gen3-usb2: add extcon support")
> > > Fixes: 73801b90a38f ("phy: renesas: rcar-gen3-usb2: change a condition "dr_mode"")
> > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >
> > merged, thanks.
> 
> I'm very sorry, I didn't notice this patch...
> I'd like to fix this patch because this patch breaks for R-Car D3.
> In this case, should I submit an incremental patch?

Kishon, is it too late to drop the patch?

> 
> > -Kishon
> > > ---
> > >  drivers/phy/renesas/phy-rcar-gen3-usb2.c | 26 +++++++++++++-------------
> > >  1 file changed, 13 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> > > index 0a34782..be1a392 100644
> > > --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> > > +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> > > @@ -447,20 +447,8 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
> > >  	if (IS_ERR(channel->base))
> > >  		return PTR_ERR(channel->base);
> > >
> > > -	/* call request_irq for OTG */
> > > -	irq = platform_get_irq(pdev, 0);
> > > -	if (irq >= 0) {
> > > -		INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work);
> > > -		irq = devm_request_irq(dev, irq, rcar_gen3_phy_usb2_irq,
> > > -				       IRQF_SHARED, dev_name(dev), channel);
> > > -		if (irq < 0)
> > > -			dev_err(dev, "No irq handler (%d)\n", irq);
> > > -	}
> > > -
> > >  	channel->dr_mode = of_usb_get_dr_mode_by_phy(dev->of_node, 0);
> > > -	if (channel->dr_mode != USB_DR_MODE_UNKNOWN) {
> > > -		int ret;
> > > -
> > > +	if (channel->dr_mode == USB_DR_MODE_OTG) {
> 
> R-Car D3 (r8a77995-draak.dts) has dr_mode with "host". So, the board can use host mode
> as default and "role" mode to switch the role for peripheral later. However, after we
> applied this patch, the R-Car D3 USB2.0 cannot switch to the peripheral mode.

I understand, I am sorry for breaking it.
The probing function still needs some fixing, devm_request_irq uses rcar_gen3_phy_usb2_irq,
rcar_gen3_phy_usb2_irq  calls rcar_gen3_device_recognition, rcar_gen3_device_recognition may
call either rcar_gen3_init_for_host or rcar_gen3_init_for_peri, both rcar_gen3_init_for_host and
rcar_gen3_init_for_peri schedule work for the work queue that calls into rcar_gen3_phy_usb2_work,
rcar_gen3_phy_usb2_work uses extcon, extcon gets initialized after devm_request_irq, which
means we should move devm_request_irq after extcon gets initialized,.
Also, we may register the irq, and never deal with extcon if dr_mode == USB_DR_MODE_UNKNOWN.

Would you like to come up with a patch to address those points?

Thanks,
Fab

> 
> Best regards,
> Yoshihiro Shimoda
> 
> > >  		channel->is_otg_channel = true;
> > >  		channel->uses_otg_pins = !of_property_read_bool(dev->of_node,
> > >  							"renesas,no-otg-pins");
> > > @@ -476,6 +464,18 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
> > >  		}
> > >  	}
> > >
> > > +	if (channel->is_otg_channel && channel->uses_otg_pins) {
> > > +		/* call request_irq for OTG */
> > > +		irq = platform_get_irq(pdev, 0);
> > > +		if (irq >= 0) {
> > > +			INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work);
> > > +			irq = devm_request_irq(dev, irq, rcar_gen3_phy_usb2_irq,
> > > +					       IRQF_SHARED, dev_name(dev), channel);
> > > +			if (irq < 0)
> > > +				dev_err(dev, "No irq handler (%d)\n", irq);
> > > +		}
> > > +	}
> > > +
> > >  	/*
> > >  	 * devm_phy_create() will call pm_runtime_enable(&phy->dev);
> > >  	 * And then, phy-core will manage runtime pm for this device.
> > >

RE: [PATCH 4/4] arm64: dts: renesas: r8a77995: draak: Remove hsusb node

[Fabrizio Castro]

Hello Simon,

I would like to drop this patch.

Thanks,
Fab

> From: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Sent: 01 March 2019 11:06
> Subject: [PATCH 4/4] arm64: dts: renesas: r8a77995: draak: Remove hsusb node
> 
> Patch "usb: common: Consider only available nodes for dr_mode"
> fixes the way the DT gets parsed to identify the USB controller
> driving the PHY, as such we don't need hsusb to specify dr_mode,
> in fact we don't need to enable the hsusb node at all since
> the USB interface on the board works as USB host only.
> 
> Fixes: 5c6479d9b25b ("arm64: dts: renesas: r8a7799{0|5}: add/enable USB2.0 peripheral")
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> ---
>  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> index db2bed1..5f13b8b 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> @@ -207,11 +207,6 @@
>  	clock-frequency = <48000000>;
>  };
> 
> -&hsusb {
> -	dr_mode = "host";
> -	status = "okay";
> -};
> -
>  &i2c0 {
>  	pinctrl-0 = <&i2c0_pins>;
>  	pinctrl-names = "default";
> --
> 2.7.4

RE: [PATCH 3/4] arm64: dts: renesas: r8a774c0-cat874: Add USB-HOST support

[Fabrizio Castro]

Hello Simon,

Patch "usb: common: Consider only available nodes for dr_mode" has been taken,
and that is enough to get USB 2.0 host to work on the EK874, do you think you
can take this patch now?

Thanks,
Fab

> From: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Sent: 01 March 2019 11:06
> Subject: [PATCH 3/4] arm64: dts: renesas: r8a774c0-cat874: Add USB-HOST support
> 
> This patch adds USB 2.0 HOST support to the CAT874 board.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> ---
>  arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> index f86b3628..7d3b76f 100644
> --- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> @@ -76,10 +76,20 @@
>  	};
>  };
> 
> +&ehci0 {
> +	dr_mode = "host";
> +	status = "okay";
> +};
> +
>  &extal_clk {
>  	clock-frequency = <48000000>;
>  };
> 
> +&ohci0 {
> +	dr_mode = "host";
> +	status = "okay";
> +};
> +
>  &pcie_bus_clk {
>  	clock-frequency = <100000000>;
>  };
> @@ -128,3 +138,8 @@
>  	sd-uhs-sdr104;
>  	status = "okay";
>  };
> +
> +&usb2_phy0 {
> +	renesas,no-otg-pins;
> +	status = "okay";
> +};
> --
> 2.7.4

Re: [PATCH 4/4] arm64: dts: renesas: r8a77995: draak: Remove hsusb node

[Simon Horman]

On Wed, Apr 03, 2019 at 10:26:18AM +0000, Fabrizio Castro wrote:
> Hello Simon,
> 
> I would like to drop this patch.

Thanks Fabrizio,

I have marked it as "Rejected" in patchwork.

Re: [PATCH 3/4] arm64: dts: renesas: r8a774c0-cat874: Add USB-HOST support

[Simon Horman]

On Wed, Apr 03, 2019 at 10:29:39AM +0000, Fabrizio Castro wrote:
> Hello Simon,
> 
> Patch "usb: common: Consider only available nodes for dr_mode" has been taken,
> and that is enough to get USB 2.0 host to work on the EK874, do you think you
> can take this patch now?

Thanks Fabrizio,

can do. I have applied this for v5.2.

RE: [PATCH 1/4] usb: common: Consider only available nodes for dr_mode

[Yoshihiro Shimoda]

Hi Fabrizio-san,

> From: Fabrizio Castro, Sent: Tuesday, April 2, 2019 8:47 PM
> 
> Hi Yoshihiro-san,
> 
> Thank you for your feedback.
> 
> > Hi Fabrizio-san,
> >
> > > From: Fabrizio Castro, Sent: Tuesday, April 2, 2019 6:22 PM
> > >
> > > Hi Yoshihiro-san,
> > >
> > > Thank you for your feedback.
> > >
> > > > From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > > Sent: 02 April 2019 02:54
> > > > Subject: RE: [PATCH 1/4] usb: common: Consider only available nodes for dr_mode
> > > >
> > > > Hi Fabrizio-san,
> > > >
> > > > Thank you for the patch!
> > > >
> > > > > From: Fabrizio Castro, Sent: Friday, March 1, 2019 8:06 PM
> > > > >
> > > > > There are cases where multiple device tree nodes point to the
> > > > > same phy node by means of the "phys" property, but we should
> > > > > only consider those nodes that are marked as available rather
> > > > > than just any node.
> > > > >
> > > > > Fixes: 98bfb3946695 ("usb: of: add an api to get dr_mode by the phy node")
> > > > > Cc: stable@vger.kernel.org # v4.4+
> > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > > > ---
> > > >
> > > > I'm guessing this code needs for phy-rcar-gen3-usb2.c only because
> > > > the phy driver only gets the dr_mode from index 0 like below:
> > > >
> > > > 	channel->dr_mode = of_usb_get_dr_mode_by_phy(dev->of_node, 0);
> > > >
> > > > Yesterday, I submitted patches to get multiple indexes from controller
> > > > device nodes:
> > > >
> > > > https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=99561
> > > >
> > > > So, would you check the phy patches can get the dr_mode without this changing common patch?
> > >
> > > I will go through your patch series, but I can't see why any driver would be interested
> > > in considering a disabled node for getting dr_mode, can you?
> > > Do you have a use case for this?
> >
> > This is not a use case though, the commit 98bfb3946695 is made by a TI engineer so that
> > I checked TI's dts/dtsi file on armv7 arch (am335x-boneblue.dts) a little and then
> > it has one usb controller node only. So, the commit didn't take care of a disabled node, I guess.
> > However, as you know, on R-Car Gen3 and RZ/G2, they have multiple controller nodes. So,
> > one of the controller can be disabled.
> 
> The real question is, are we going to need to read dr_mode from a disabled node?
> Because this patch gets in the way of that, but looking through the code and
> device trees it doesn't look like anybody needs that. I have tested this patch in isolation
> on a few R-Car platforms, and it all seemed good to me back when I submitted the patch,
> but it would probably need thorough testing, and more testing from your side is more than
> welcome. Besides this point, of_usb_get_dr_mode_by_phy returns the dr_mode value that
> matches the argument the first, which means what you get back from it depends from
> the order in which you specify the nodes within the device tree in case you have multiple
> nodes meeting the same requirements (from a of_usb_get_dr_mode_by_phy perspective),
> a.k.a. "luck".

Thank you very much for the detailed explanation. I agree with you. This API should not get
the dr_mode from a disabled node. For example, descriping the following device nodes are nonsense:

&host {
	dr_mode = "peripheral";		// <--- here
	status = "disabled";
}

&peripheral {
	dr_mode = "peripheral";
	status = "okay";
}

> We should make sure there is only one node that can match our requirement at any one
> time for this to work properly, and parsing only enabled nodes gets in that direction,
> therefore I still think this patch is valid, and belongs to stable versions of the kernel
> as well.

I agree this patch is valid for both mainline and stable versions because all this API users
in v5.1-rc4 are called with the second argument -1 or 0 and then the behavior should be
the same as before. So,

Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Best regards,
Yoshihiro Shimoda

> After reading your comment on patch:
> "phy: renesas: rcar-gen3-usb2: No need to request IRQ for non-OTG"
> 
> I can now see that patch "arm64: dts: renesas: r8a77995: draak: Remove hsusb node" is wrong
> for your case, and that we need to find a better solution for patch:
> "phy: renesas: rcar-gen3-usb2: No need to request IRQ for non-OTG"
> 
> I am looking into your series now, I'll come back to you soon on the other patch.
> 
> Thanks,
> Fab
> 
> >
> > By the way, if we applied this patch, since the behavior will be changed at least,
> > so all of this api user have to test whether this code doesn't break or not.
> >
> > Best regards,
> > Yoshihiro Shimoda
> >
> > > Thanks,
> > > Fab
> > >
> > > >
> > > > Best regards,
> > > > Yoshihiro Shimoda
> > > >
> > > > >  drivers/usb/common/common.c | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > > > > index 48277bb..73c8e65 100644
> > > > > --- a/drivers/usb/common/common.c
> > > > > +++ b/drivers/usb/common/common.c
> > > > > @@ -145,6 +145,8 @@ enum usb_dr_mode of_usb_get_dr_mode_by_phy(struct device_node *np, int arg0)
> > > > >
> > > > >  	do {
> > > > >  		controller = of_find_node_with_property(controller, "phys");
> > > > > +		if (!of_device_is_available(controller))
> > > > > +			continue;
> > > > >  		index = 0;
> > > > >  		do {
> > > > >  			if (arg0 == -1) {
> > > > > --
> > > > > 2.7.4

Re: [PATCH 2/4] phy: renesas: rcar-gen3-usb2: No need to request IRQ for non-OTG

[Kishon Vijay Abraham I]

Hi,

On 03/04/19 3:54 PM, Fabrizio Castro wrote:
> Hello Yoshihiro-san, Kishon
> 
>> From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>> Sent: 02 April 2019 03:11
>> Subject: RE: [PATCH 2/4] phy: renesas: rcar-gen3-usb2: No need to request IRQ for non-OTG
>>
>> Hi Kishon, Fabrizio,
>>
>>> From: Kishon Vijay Abraham I, Sent: Monday, April 1, 2019 9:26 PM
>>>
>>> On 01/03/19 4:35 PM, Fabrizio Castro wrote:
>>>> As stated in the comment found on top of rcar_gen3_phy_usb2_work,
>>>> we should not be registering the IRQ if !otg_channel || !uses_otg_pins.
>>>> On top of that, we should also make sure that extcon has been
>>>> allocated before requesting the irq as rcar_gen3_phy_usb2_work
>>>> uses it, hence the patch.
>>>>
>>>> Fixes: 2b38543c8db1 ("phy: rcar-gen3-usb2: add extcon support")
>>>> Fixes: 73801b90a38f ("phy: renesas: rcar-gen3-usb2: change a condition "dr_mode"")
>>>> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
>>>
>>> merged, thanks.
>>
>> I'm very sorry, I didn't notice this patch...
>> I'd like to fix this patch because this patch breaks for R-Car D3.
>> In this case, should I submit an incremental patch?
> 
> Kishon, is it too late to drop the patch?

I have to send a new pul request. I'll drop this patch then. Thanks for letting
me know.

-Kishon

> 
>>
>>> -Kishon
>>>> ---
>>>>  drivers/phy/renesas/phy-rcar-gen3-usb2.c | 26 +++++++++++++-------------
>>>>  1 file changed, 13 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
>>>> index 0a34782..be1a392 100644
>>>> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
>>>> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
>>>> @@ -447,20 +447,8 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
>>>>  	if (IS_ERR(channel->base))
>>>>  		return PTR_ERR(channel->base);
>>>>
>>>> -	/* call request_irq for OTG */
>>>> -	irq = platform_get_irq(pdev, 0);
>>>> -	if (irq >= 0) {
>>>> -		INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work);
>>>> -		irq = devm_request_irq(dev, irq, rcar_gen3_phy_usb2_irq,
>>>> -				       IRQF_SHARED, dev_name(dev), channel);
>>>> -		if (irq < 0)
>>>> -			dev_err(dev, "No irq handler (%d)\n", irq);
>>>> -	}
>>>> -
>>>>  	channel->dr_mode = of_usb_get_dr_mode_by_phy(dev->of_node, 0);
>>>> -	if (channel->dr_mode != USB_DR_MODE_UNKNOWN) {
>>>> -		int ret;
>>>> -
>>>> +	if (channel->dr_mode == USB_DR_MODE_OTG) {
>>
>> R-Car D3 (r8a77995-draak.dts) has dr_mode with "host". So, the board can use host mode
>> as default and "role" mode to switch the role for peripheral later. However, after we
>> applied this patch, the R-Car D3 USB2.0 cannot switch to the peripheral mode.
> 
> I understand, I am sorry for breaking it.
> The probing function still needs some fixing, devm_request_irq uses rcar_gen3_phy_usb2_irq,
> rcar_gen3_phy_usb2_irq  calls rcar_gen3_device_recognition, rcar_gen3_device_recognition may
> call either rcar_gen3_init_for_host or rcar_gen3_init_for_peri, both rcar_gen3_init_for_host and
> rcar_gen3_init_for_peri schedule work for the work queue that calls into rcar_gen3_phy_usb2_work,
> rcar_gen3_phy_usb2_work uses extcon, extcon gets initialized after devm_request_irq, which
> means we should move devm_request_irq after extcon gets initialized,.
> Also, we may register the irq, and never deal with extcon if dr_mode == USB_DR_MODE_UNKNOWN.
> 
> Would you like to come up with a patch to address those points?
> 
> Thanks,
> Fab
> 
>>
>> Best regards,
>> Yoshihiro Shimoda
>>
>>>>  		channel->is_otg_channel = true;
>>>>  		channel->uses_otg_pins = !of_property_read_bool(dev->of_node,
>>>>  							"renesas,no-otg-pins");
>>>> @@ -476,6 +464,18 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
>>>>  		}
>>>>  	}
>>>>
>>>> +	if (channel->is_otg_channel && channel->uses_otg_pins) {
>>>> +		/* call request_irq for OTG */
>>>> +		irq = platform_get_irq(pdev, 0);
>>>> +		if (irq >= 0) {
>>>> +			INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work);
>>>> +			irq = devm_request_irq(dev, irq, rcar_gen3_phy_usb2_irq,
>>>> +					       IRQF_SHARED, dev_name(dev), channel);
>>>> +			if (irq < 0)
>>>> +				dev_err(dev, "No irq handler (%d)\n", irq);
>>>> +		}
>>>> +	}
>>>> +
>>>>  	/*
>>>>  	 * devm_phy_create() will call pm_runtime_enable(&phy->dev);
>>>>  	 * And then, phy-core will manage runtime pm for this device.
>>>>