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

Message

Quentin Schulz Dec. 16, 2022, 4:29 p.m. UTC

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 <[e-mail]>
To: Greg Kroah-Hartman <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
Cc: Bin Yang <[e-mail]>
Signed-off-by: Quentin Schulz <[e-mail]>
---
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/[e-mail]

---
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,

Comments

Minas Harutyunyan Dec. 20, 2022, 6:02 a.m. UTC | #1

Hi Quentin,

On 12/16/2022 8:29 PM, Quentin Schulz <[e-mail]> wrote:
>From: Quentin Schulz <[e-mail]>
>Sent: Friday, December 16, 2022 8:29 PM
>To: Minas Harutyunyan <[e-mail]>; Greg Kroah-Hartman
><[e-mail]>
>Cc: Quentin Schulz <[e-mail]>; [e-mail];
>[e-mail]; Bin Yang <[e-mail]>; Quentin
>Schulz <[e-mail]>
>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 <[e-mail]>
>To: Greg Kroah-Hartman <[e-mail]>
>Cc: [e-mail]
>Cc: [e-mail]
>Cc: Bin Yang <[e-mail]>
>Signed-off-by: Quentin Schulz <[e-mail]>
>---
>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 <[e-mail]>

Quentin Schulz Jan. 2, 2023, 11:55 a.m. UTC | #2

Hi Minas,

On 12/20/22 07:02, Minas Harutyunyan wrote:
> Hi Quentin,
> 
> On 12/16/2022 8:29 PM, Quentin Schulz <[e-mail]> wrote:
>> From: Quentin Schulz <[e-mail]>
>> Sent: Friday, December 16, 2022 8:29 PM
>> To: Minas Harutyunyan <[e-mail]>; Greg Kroah-Hartman
>> <[e-mail]>
>> Cc: Quentin Schulz <[e-mail]>; [e-mail];
>> [e-mail]; Bin Yang <[e-mail]>; Quentin
>> Schulz <[e-mail]>
>> 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