mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Louis Chauvet <louis.chauvet@bootlin.com>
To: Luca Ceresoli <luca.ceresoli@bootlin.com>,
	Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>
Cc: "Alan Stern" <stern@rowland.harvard.edu>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"Kever Yang" <kever.yang@rock-chips.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Hervé Codina" <herve.codina@bootlin.com>,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	"Stefan Wahren" <wahrenst@gmx.net>,
	"Fabrice Gasnier" <fabrice.gasnier@foss.st.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-phy@lists.infradead.org, heiko@sntech.de, vkoul@kernel.org,
	kishon@kernel.org, linux-rockchip@lists.infradead.org
Subject: Re: DWC2 gadget: unexpected device reenumeration on Rockchip RK3308
Date: Thu, 12 Jun 2025 15:23:14 +0200	[thread overview]
Message-ID: <cb081d0f-3a53-4b5e-9f14-92745820fc75@bootlin.com> (raw)
In-Reply-To: <20250520140936.08d72db6@booty>



Le 20/05/2025 à 14:09, Luca Ceresoli a écrit :
> Hello Minas,

[...]

> Minas, do you have any hints or advice to understand why there is a
> disconnect about ~6 seconds after a successful enumeration in gadget
> mode?

+CC: vkoul@kernel.org, kishon@kernel.org, 
linux-rockchip@lists.infradead.org, 
linux-arm-kernel@lists.infradead.org, linux-phy@lists.infradead.org, 
heiko@sntech.de

Hello,

I am a colleague of Luca and have been working on an issue related to 
USB peripheral disconnections over the past few days. I believe I have 
found some interesting insights that explain the disconnection.

To provide some context, I have added traces in relevant functions to 
understand the sequence of events (filtered on interesting logs, see [1] 
for full logs):


[... boot proccess ...]

[    0.628744] dwc2 ff400000.usb: mapped PA ff400000 to VA (____ptrval____)
[    0.629358] dwc2 ff400000.usb: supply vusb_d not found, using dummy 
regulator
[    0.630478] dwc2 ff400000.usb: supply vusb_a not found, using dummy 
regulator
[    0.631297] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/usb/dwc2/platform.c:157:dwc2_lowlevel_hw_enable
[    0.632406] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/phy-core.c:230:phy_init
[    0.633233] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:491:rockchip_usb2phy_init
[    0.634262] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:539:rockchip_usb2phy_init 
(before schedule_delayed_work otg_sm_work)
[    0.635602] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/phy-core.c:309:phy_power_on
[    0.636379] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:577:rockchip_usb2phy_power_on
[    0.637496] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:606:rockchip_usb2phy_power_on 
(before rockchip_usb2phy_reset)

[... bunch of dwc2 ff400000.usb logs ...]

[    0.688708] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/usb/dwc2/platform.c:200:dwc2_lowlevel_hw_disable
[    0.689661] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/phy-core.c:360:phy_power_off
[    0.690442] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:620:rockchip_usb2phy_power_off
[    0.691522] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/phy-core.c:271:phy_exit
[    0.692261] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:648:rockchip_usb2phy_exit 
(before cancel_delayed_work_sync otg_sm_work/chg_work)
[    0.698909] dwmmc_rockchip ff490000.mmc: IDMAC supports 32-bit 
address mode.

[... boot finished ...]

[... call to my peripheral setup script ...]

[   44.493316] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/usb/dwc2/platform.c:157:dwc2_lowlevel_hw_enable
[   44.495078] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/phy-core.c:230:phy_init
[   44.495837] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:491:rockchip_usb2phy_init
[   44.496960] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:539:rockchip_usb2phy_init 
(before schedule_delayed_work otg_sm_work)
[   44.498317] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/phy-core.c:309:phy_power_on
[   44.499107] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:577:rockchip_usb2phy_power_on
[   44.500160] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:606:rockchip_usb2phy_power_on 
(before rockchip_usb2phy_reset)


[... bunch of dwc2 ff400000.usb logs ...]
[   44.976346] dwc2 ff400000.usb: dwc2_hsotg_start_req: DXEPCTL=0x80028000

[... ~5 seconds delay ...]

[   50.660998] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:670:rockchip_usb2phy_otg_sm_work
[   50.662128] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:685:rockchip_usb2phy_otg_sm_work 
(rport->state=0 vbus_attach=1 rphy->chg_state=0 rphy->chg_type=0 
extcon_get_state=0)
[   50.663862] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:831:rockchip_chg_detect_work
[   50.664973] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:839:rockchip_chg_detect_work 
(rphy->chg_state=0 rport->suspended=0)
[   50.666316] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:620:rockchip_usb2phy_power_off
[   50.769027] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:831:rockchip_chg_detect_work
[   50.770093] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:839:rockchip_chg_detect_work 
(rphy->chg_state=1 rport->suspended=1)
[   50.813030] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:831:rockchip_chg_detect_work
[   50.814083] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:839:rockchip_chg_detect_work 
(rphy->chg_state=2 rport->suspended=1)
[   50.857013] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:831:rockchip_chg_detect_work
[   50.858067] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:839:rockchip_chg_detect_work 
(rphy->chg_state=3 rport->suspended=1)
[   50.859409] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:670:rockchip_usb2phy_otg_sm_work
[   50.860475] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:685:rockchip_usb2phy_otg_sm_work 
(rport->state=1 vbus_attach=1 rphy->chg_state=5 rphy->chg_type=6 
extcon_get_state=0)
[   50.862267] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:577:rockchip_usb2phy_power_on
[   50.863326] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:606:rockchip_usb2phy_power_on 
(before rockchip_usb2phy_reset)

[... bunch of dwc2 ff400000.usb logs ...]

[   52.868992] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:670:rockchip_usb2phy_otg_sm_work
[   52.870108] phy phy-ff008000.syscon:usb2phy@100.0: 
drivers/phy/rockchip/phy-rockchip-inno-usb2.c:685:rockchip_usb2phy_otg_sm_work 
(rport->state=3 vbus_attach=1 rphy->chg_state=5 rphy->chg_type=6 
extcon_get_state=0)

[... repeated every 2 seconds ...]



I found two potential issues that may be relevant:

1 - The workers rockchip_usb2phy_otg_sm_work and 
rockchip_chg_detect_work call rockchip_usb2phy_power_off/on directly, 
bypassing phy_power_off/on. This means the state of the PHY after a call 
to phy_power_on may not be the one expected by users of the struct phy api.
2 -  The worker rockchip_chg_detect_work always disables the PHY at its 
first call, which creates the disconnection.


For issue 1, here is the phy_power_on/off reference count:

[    0.635602] phy_power_on	count = 1
[    0.689661] phy_power_off	count = 0
[   44.498317] phy_power_on	count = 1 // ./setup-cdc-serial
[   50.666316] rockchip_usb2phy_power_off => direct call to power_off! 
This causes issues in the dwc2 driver because the PHY is powered off 
even if the phy_power_on counter is not zero, leading to the disconnection
[   50.862267] rockchip_usb2phy_power_on => direct call to power_on and 
reset! This will reset the PHY even if the phy_power_on counter is not zero.

This is my first time digging into the USB/PHY subsystem, so I might be 
wrong, but this behavior seems very strange. Once phy_power_on is 
called, I expect the PHY to never go off until phy_power_off is called.


For issue 2, this is also a strange behavior. I understand that 
resetting the PHY before its first use might be necessary, but I don't 
understand why this must be delayed by 6 seconds. In my opinion, this 
reset sequence should occur synchronously during the phy_init call.

I quickly looked at other PHY drivers, and none of them seem to have 
asynchronous behavior or any internal calls to power_on/power_off.

Could you confirm if what I found is indeed the issue and if the 
rockchip_usb2phy driver needs to be fixed?

Does anyone know why rockchip_usb2phy needs to perform these 
asynchronous tasks and these power_on/off calls?

Let me know if you need more logs to understand the issue.

Thanks a lot,
Louis Chauvet


[1]:https://paste.sr.ht/~fomys/26e0d03d03537a6eb3757fb709b6d5a81484eee3

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


      reply	other threads:[~2025-06-12 13:24 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-14 16:54 Luca Ceresoli
2025-04-14 17:26 ` Alan Stern
2025-04-15 11:26   ` Minas Harutyunyan
2025-04-15 14:28     ` Luca Ceresoli
2025-04-15 15:58       ` Luca Ceresoli
2025-04-15 16:14       ` Alan Stern
2025-05-02 13:53         ` Luca Ceresoli
2025-05-02 17:56           ` Alan Stern
2025-05-09  7:17             ` Luca Ceresoli
2025-05-13  7:35               ` Minas Harutyunyan
2025-05-20 12:09                 ` Luca Ceresoli
2025-06-12 13:23                   ` Louis Chauvet [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cb081d0f-3a53-4b5e-9f14-92745820fc75@bootlin.com \
    --to=louis.chauvet@bootlin.com \
    --cc=Minas.Harutyunyan@synopsys.com \
    --cc=fabrice.gasnier@foss.st.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=heiko@sntech.de \
    --cc=herve.codina@bootlin.com \
    --cc=kever.yang@rock-chips.com \
    --cc=kishon@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=luca.ceresoli@bootlin.com \
    --cc=stern@rowland.harvard.edu \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=vkoul@kernel.org \
    --cc=wahrenst@gmx.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®