[PATCH v2 0/2] usb: dwc2: fix USB peripheral role in dual-role mode on PX30

[PATCH v2 0/2] usb: dwc2: fix USB peripheral role in dual-role mode on PX30

[Quentin Schulz]

On Theobroma Ringneck SoM, USB peripheral role when in dual-role mode does not
work and displays the following error message:
dwc2 ff300000.usb: dwc2_core_reset: HANG! Soft Reset timeout GRSTCTL_CSFTRST

The USB sniffer shows nothing and dumping the host registers is stuck on
HCDMA(0) register.

Note that for some reason it works "fine" on PX30-EVB (there's another issue but
not related to/fixed in this patch series).

Since there's no documentation available for this IP, this patch series is
basically just slightly adapted downstream BSP vendor kernel patches and I
cannot unfortunately give more information than what I have.

This patch series was tested on Theobroma Ringneck SoM on Haikou devkit and PX30
EVB. It fixes Ringneck support and does not break PX30-EVB's.

For reference, the content of those commits can be found in tag
linux-5.10-gen-rkr1, and the following commits have been used:
964d50060bf53a8defd1fc561b9261424f25ddad
ad81c375602819a538ad68d979906c05663046e2
6e6adab8f735bc4fe27a67bdc3144d8fa89250d4
7c3a4e60247fd7f7b04d95d15cb12c63a5c20408

Note that Rockchip kernel called a slightly different implementation of
__dwc2_lowlevel_hw_enable instead of dwc2_lowlevel_hw_enable but it seemed more
right to me to call dwc2_lowlevel_hw_enable as done for the forced peripheral
mode.

Note that it is still not flawlessly working but the support is improved
(somewhat working vs not working at all).

To: Minas Harutyunyan <hminas@synopsys.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Bin Yang <yangbin@rock-chips.com>
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
Changes in v2:
- add "shadow" variable for HCD skip_phy_initialization so that it can be
  configured on a per-platform basis,
- Patch 1 and Patch 2 of the v1 are already in linux-next next-20221216, so removed from this series,
- Link to v1: https://lore.kernel.org/r/20221206-dwc2-gadget-dual-role-v1-0-36515e1092cd@theobroma-systems.com

---
Quentin Schulz (2):
      usb: dwc2: allow platforms to prevent core phy initialisation
      usb: dwc2: prevent core PHY initialization on Rockchip

 drivers/usb/dwc2/core.h   | 6 ++++++
 drivers/usb/dwc2/hcd.c    | 2 ++
 drivers/usb/dwc2/params.c | 1 +
 3 files changed, 9 insertions(+)
---
base-commit: ca39c4daa6f7f770b1329ffb46f1e4a6bcc3f291
change-id: 20221206-dwc2-gadget-dual-role-aac67e6d42fd

Best regards,
-- 
Quentin Schulz <quentin.schulz@theobroma-systems.com>

[PATCH v2 1/2] usb: dwc2: allow platforms to prevent core phy initialisation

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

Some USB PHYs need to be controlled dynamically on some platforms.

Let's allow platforms to define whether they should prevent core PHY
initialization by providing a "shadow" value to HCD's
skip_phy_initialization flag before HCD is actually created (e.g. from
a dwc2_set_*_params callback).

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 drivers/usb/dwc2/core.h | 6 ++++++
 drivers/usb/dwc2/hcd.c  | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 40cf2880d7e5..6aa9aeb374eb 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -423,6 +423,10 @@ enum dwc2_ep0_state {
  *                      0 - No
  *                      1 - Yes
  *
+ * @hcd_skip_phy_initialization: Shadow value of HCD skip_phy_initialization
+ *				 flag.
+ *				 See include/linux/usb/hcd.h for documentation.
+ *
  * The following parameters may be specified when starting the module. These
  * parameters define how the DWC_otg controller should be configured. A
  * value of -1 (or any other out of range value) for any parameter means
@@ -499,6 +503,8 @@ struct dwc2_core_params {
 	u32 g_tx_fifo_size[MAX_EPS_CHANNELS];
 
 	bool change_speed_quirk;
+
+	unsigned hcd_skip_phy_initialization:1;
 };
 
 /**
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 657f1f659ffa..e2fa9a491cab 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -5315,6 +5315,8 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg)
 	if (!IS_ERR_OR_NULL(hsotg->uphy))
 		otg_set_host(hsotg->uphy->otg, &hcd->self);
 
+	hcd->skip_phy_initialization = hsotg->params.hcd_skip_phy_initialization;
+
 	/*
 	 * Finish generic HCD initialization and start the HCD. This function
 	 * allocates the DMA buffer pool, registers the USB bus, requests the

-- 
2.38.1

[PATCH v2 2/2] usb: dwc2: prevent core PHY initialization on Rockchip

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

In Rockchip vendor kernel, the core PHY initialization is disabled with
the following justification:

 The usb phys need to be controlled dynamically on some Rockchip SoCs.
 So set the new HCD flag which prevents USB core from trying to manage
 our phys.

This is required to get USB gadget working in dual-role mode on Ringneck
PX30 SoM on a Haikou Devkit.

Cc: Bin Yang <yangbin@rock-chips.com>
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 drivers/usb/dwc2/params.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 9ed9fd956940..9095437b3236 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -117,6 +117,7 @@ static void dwc2_set_rk_params(struct dwc2_hsotg *hsotg)
 	p->lpm_clock_gating = false;
 	p->besl = false;
 	p->hird_threshold_en = false;
+	p->hcd_skip_phy_initialization = 1;
 }
 
 static void dwc2_set_ltq_params(struct dwc2_hsotg *hsotg)

-- 
2.38.1

RE: [PATCH v2 2/2] usb: dwc2: prevent core PHY initialization on Rockchip

[Minas Harutyunyan]

Hi Quentin,

On 12/16/2022 8:29 PM, Quentin Schulz <foss+kernel@0leil.net> wrote:
>From: Quentin Schulz <foss+kernel@0leil.net>
>Sent: Friday, December 16, 2022 8:29 PM
>To: Minas Harutyunyan <hminas@synopsys.com>; Greg Kroah-Hartman
><gregkh@linuxfoundation.org>
>Cc: Quentin Schulz <foss+kernel@0leil.net>; linux-usb@vger.kernel.org;
>linux-kernel@vger.kernel.org; Bin Yang <yangbin@rock-chips.com>; Quentin
>Schulz <quentin.schulz@theobroma-systems.com>
>Subject: [PATCH v2 2/2] usb: dwc2: prevent core PHY initialization on
>Rockchip
>
>From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
>In Rockchip vendor kernel, the core PHY initialization is disabled with the
>following justification:
>
> The usb phys need to be controlled dynamically on some Rockchip SoCs.
> So set the new HCD flag which prevents USB core from trying to manage  our
>phys.
>
>This is required to get USB gadget working in dual-role mode on Ringneck
>PX30 SoM on a Haikou Devkit.
>
>Cc: Bin Yang <yangbin@rock-chips.com>
>Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>---
> drivers/usb/dwc2/params.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c index
>9ed9fd956940..9095437b3236 100644
>--- a/drivers/usb/dwc2/params.c
>+++ b/drivers/usb/dwc2/params.c
>@@ -117,6 +117,7 @@ static void dwc2_set_rk_params(struct dwc2_hsotg *hsotg)
> 	p->lpm_clock_gating = false;
> 	p->besl = false;
> 	p->hird_threshold_en = false;
>+	p->hcd_skip_phy_initialization = 1;

I'm not familiar with all "rk" platforms, but are you sure that
p->hcd_skip_phy_initialization = 1 required for all of them?

> }
>
> static void dwc2_set_ltq_params(struct dwc2_hsotg *hsotg)
>
>--
>2.38.1

To avoid any misinterpretation please add to dwc2_set_default_params()
function setting hcd_skip_phy_initialization to 0.

Thanks,
Minas

RE: [PATCH v2 0/2] usb: dwc2: fix USB peripheral role in dual-role mode on PX30

[Minas Harutyunyan]

Hi Quentin,

On 12/16/2022 8:29 PM, Quentin Schulz <foss+kernel@0leil.net> wrote:
>From: Quentin Schulz <foss+kernel@0leil.net>
>Sent: Friday, December 16, 2022 8:29 PM
>To: Minas Harutyunyan <hminas@synopsys.com>; Greg Kroah-Hartman
><gregkh@linuxfoundation.org>
>Cc: Quentin Schulz <foss+kernel@0leil.net>; linux-usb@vger.kernel.org;
>linux-kernel@vger.kernel.org; Bin Yang <yangbin@rock-chips.com>; Quentin
>Schulz <quentin.schulz@theobroma-systems.com>
>Subject: [PATCH v2 0/2] usb: dwc2: fix USB peripheral role in dual-role mode
>on PX30
>
>On Theobroma Ringneck SoM, USB peripheral role when in dual-role mode does
>not work and displays the following error message:
>dwc2 ff300000.usb: dwc2_core_reset: HANG! Soft Reset timeout GRSTCTL_CSFTRST
>

This mostly happen if PHY not initialized correctly.

>The USB sniffer shows nothing and dumping the host registers is stuck on
>HCDMA(0) register.
>
>Note that for some reason it works "fine" on PX30-EVB (there's another issue
>but not related to/fixed in this patch series).
>
>Since there's no documentation available for this IP, this patch series is
>basically just slightly adapted downstream BSP vendor kernel patches and I
>cannot unfortunately give more information than what I have.
>
>This patch series was tested on Theobroma Ringneck SoM on Haikou devkit and
>PX30 EVB. It fixes Ringneck support and does not break PX30-EVB's.
>
>For reference, the content of those commits can be found in tag linux-5.10-
>gen-rkr1, and the following commits have been used:
>964d50060bf53a8defd1fc561b9261424f25ddad
>ad81c375602819a538ad68d979906c05663046e2
>6e6adab8f735bc4fe27a67bdc3144d8fa89250d4
>7c3a4e60247fd7f7b04d95d15cb12c63a5c20408
>
>Note that Rockchip kernel called a slightly different implementation of
>__dwc2_lowlevel_hw_enable instead of dwc2_lowlevel_hw_enable but it seemed
>more right to me to call dwc2_lowlevel_hw_enable as done for the forced
>peripheral mode.
>
>Note that it is still not flawlessly working but the support is improved
>(somewhat working vs not working at all).

If "still not flawlessly working" maybe required deeper investigate issue
and submit "flawlessly working" patch.

Thanks,
Minas

>
>To: Minas Harutyunyan <hminas@synopsys.com>
>To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>Cc: linux-usb@vger.kernel.org
>Cc: linux-kernel@vger.kernel.org
>Cc: Bin Yang <yangbin@rock-chips.com>
>Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>---
>Changes in v2:
>- add "shadow" variable for HCD skip_phy_initialization so that it can be
>  configured on a per-platform basis,
>- Patch 1 and Patch 2 of the v1 are already in linux-next next-20221216, so
>removed from this series,
>- Link to v1:
>https://urldefense.com/v3/__https://lore.kernel.org/r/20221206-dwc2-gadget-
>dual-role-v1-0-36515e1092cd@theobroma-
>systems.com__;!!A4F2R9G_pg!abD6i25j11WuiZxeHgqSXPLLwpjHNNNR2b18aw-
>lsRUROS_fdnLEnEw-t4kodoTU8qnqXrzkT2N_9w7e6L-q$
>
>---
>Quentin Schulz (2):
>      usb: dwc2: allow platforms to prevent core phy initialisation
>      usb: dwc2: prevent core PHY initialization on Rockchip
>
> drivers/usb/dwc2/core.h   | 6 ++++++
> drivers/usb/dwc2/hcd.c    | 2 ++
> drivers/usb/dwc2/params.c | 1 +
> 3 files changed, 9 insertions(+)
>---
>base-commit: ca39c4daa6f7f770b1329ffb46f1e4a6bcc3f291
>change-id: 20221206-dwc2-gadget-dual-role-aac67e6d42fd
>
>Best regards,
>--
>Quentin Schulz <quentin.schulz@theobroma-systems.com>

Re: [PATCH v2 2/2] usb: dwc2: prevent core PHY initialization on Rockchip

[Quentin Schulz]

Hi Minas,

On 12/20/22 07:02, Minas Harutyunyan wrote:
> Hi Quentin,
> 
> On 12/16/2022 8:29 PM, Quentin Schulz <foss+kernel@0leil.net> wrote:
>> From: Quentin Schulz <foss+kernel@0leil.net>
>> Sent: Friday, December 16, 2022 8:29 PM
>> To: Minas Harutyunyan <hminas@synopsys.com>; Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org>
>> Cc: Quentin Schulz <foss+kernel@0leil.net>; linux-usb@vger.kernel.org;
>> linux-kernel@vger.kernel.org; Bin Yang <yangbin@rock-chips.com>; Quentin
>> Schulz <quentin.schulz@theobroma-systems.com>
>> Subject: [PATCH v2 2/2] usb: dwc2: prevent core PHY initialization on
>> Rockchip
>>
>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>
>> In Rockchip vendor kernel, the core PHY initialization is disabled with the
>> following justification:
>>
>> The usb phys need to be controlled dynamically on some Rockchip SoCs.
>> So set the new HCD flag which prevents USB core from trying to manage  our
>> phys.
>>
>> This is required to get USB gadget working in dual-role mode on Ringneck
>> PX30 SoM on a Haikou Devkit.
>>
>> Cc: Bin Yang <yangbin@rock-chips.com>
>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>> ---
>> drivers/usb/dwc2/params.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c index
>> 9ed9fd956940..9095437b3236 100644
>> --- a/drivers/usb/dwc2/params.c
>> +++ b/drivers/usb/dwc2/params.c
>> @@ -117,6 +117,7 @@ static void dwc2_set_rk_params(struct dwc2_hsotg *hsotg)
>> 	p->lpm_clock_gating = false;
>> 	p->besl = false;
>> 	p->hird_threshold_en = false;
>> +	p->hcd_skip_phy_initialization = 1;
> 
> I'm not familiar with all "rk" platforms, but are you sure that
> p->hcd_skip_phy_initialization = 1 required for all of them?
> 

No I am not. This is however applied to all devices (even non-Rockchip 
ones) in Rockchip vendor kernel.

I could check on an RK3368 since we have a module available for that, 
but I don't have access to any other Rockchip platform. From a quick 
grep, the following seems to be susceptible to this change:

rk3128
rv1108
rk3288
rk3036
rk3228-evb
rk3229
rk3066a
rk3188
rk3328
px30
rk3308
rk3368

Cheers,
Quentin

Re: [PATCH v2 0/2] usb: dwc2: fix USB peripheral role in dual-role mode on PX30

[Quentin Schulz]

Hi Minas,

On 12/20/22 07:02, Minas Harutyunyan wrote:
> Hi Quentin,
> 
> On 12/16/2022 8:29 PM, Quentin Schulz <foss+kernel@0leil.net> wrote:
>> From: Quentin Schulz <foss+kernel@0leil.net>
>> Sent: Friday, December 16, 2022 8:29 PM
>> To: Minas Harutyunyan <hminas@synopsys.com>; Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org>
>> Cc: Quentin Schulz <foss+kernel@0leil.net>; linux-usb@vger.kernel.org;
>> linux-kernel@vger.kernel.org; Bin Yang <yangbin@rock-chips.com>; Quentin
>> Schulz <quentin.schulz@theobroma-systems.com>
>> Subject: [PATCH v2 0/2] usb: dwc2: fix USB peripheral role in dual-role mode
>> on PX30
>>
>> On Theobroma Ringneck SoM, USB peripheral role when in dual-role mode does
>> not work and displays the following error message:
>> dwc2 ff300000.usb: dwc2_core_reset: HANG! Soft Reset timeout GRSTCTL_CSFTRST
>>
> 
> This mostly happen if PHY not initialized correctly.
> 

Any way to know which part of the PHY wouldn't be correctly initialized? 
or is this just a whack-a-mole game with register bits?

>> The USB sniffer shows nothing and dumping the host registers is stuck on
>> HCDMA(0) register.
>>
>> Note that for some reason it works "fine" on PX30-EVB (there's another issue
>> but not related to/fixed in this patch series).
>>
>> Since there's no documentation available for this IP, this patch series is
>> basically just slightly adapted downstream BSP vendor kernel patches and I
>> cannot unfortunately give more information than what I have.
>>
>> This patch series was tested on Theobroma Ringneck SoM on Haikou devkit and
>> PX30 EVB. It fixes Ringneck support and does not break PX30-EVB's.
>>
>> For reference, the content of those commits can be found in tag linux-5.10-
>> gen-rkr1, and the following commits have been used:
>> 964d50060bf53a8defd1fc561b9261424f25ddad
>> ad81c375602819a538ad68d979906c05663046e2
>> 6e6adab8f735bc4fe27a67bdc3144d8fa89250d4
>> 7c3a4e60247fd7f7b04d95d15cb12c63a5c20408
>>
>> Note that Rockchip kernel called a slightly different implementation of
>> __dwc2_lowlevel_hw_enable instead of dwc2_lowlevel_hw_enable but it seemed
>> more right to me to call dwc2_lowlevel_hw_enable as done for the forced
>> peripheral mode.
>>
>> Note that it is still not flawlessly working but the support is improved
>> (somewhat working vs not working at all).
> 
> If "still not flawlessly working" maybe required deeper investigate issue
> and submit "flawlessly working" patch.
> 

I would love to honestly. But no documentation and the vendor kernel 
works differently but not better. I get the point, not sure I'll be able 
to provide any meaningful improvement to this patch though. Let's see if 
I'll have time to dig into this sometime soon (I expect not :/).

I'm all ears if someone wants to share how they would start debugging 
this though.

Cheers,
Quentin