* [PATCH 0/2] phy: rockchip: inno-usb2: fix gadget mode disconnection after 6 seconds
@ 2025-07-22 8:43 Luca Ceresoli
2025-07-22 8:43 ` [PATCH 1/2] phy: rockchip: inno-usb2: fix disconnection in gadget mode Luca Ceresoli
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Luca Ceresoli @ 2025-07-22 8:43 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Heiko Stuebner, William Wu
Cc: Kever Yang, Minas Harutyunyan, Alan Stern, Louis Chauvet,
Hervé Codina, Thomas Petazzoni, linux-phy, linux-rockchip,
linux-usb, linux-arm-kernel, linux-kernel, Luca Ceresoli
The USB OTG port of the RK3308 exibits a bug when:
- configured as peripheral, and
- used in gadget mode, and
- the USB cable is connected since before booting
The symptom is: about 6 seconds after configuring gadget mode the device is
disconnected and then re-enumerated. This happens only once per boot.
Investigation showed that in this configuration the charger detection code
turns off the PHY after 6 seconds. Patch 1 avoids this when a cable is
connected (VBUS present).
After patch 1 the connection is stable but communication stops after 6
seconds. this is addressed by patch 2.
The topic had been discussed in [0]. Thanks Alan and Minas for the
discussion and Louis for having found the 1st issue, leading to patch 1.
[0] https://lore.kernel.org/lkml/20250414185458.7767aabc@booty/
Luca
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
Louis Chauvet (1):
phy: rockchip: inno-usb2: fix disconnection in gadget mode
Luca Ceresoli (1):
phy: rockchip: inno-usb2: fix communication disruption in gadget mode
drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
---
base-commit: 15240745d6cd14183d112249a9fff87fb968b859
change-id: 20250718-rk3308-fix-usb-gadget-phy-disconnect-d7de71fb28b4
Best regards,
--
Luca Ceresoli <luca.ceresoli@bootlin.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] phy: rockchip: inno-usb2: fix disconnection in gadget mode
2025-07-22 8:43 [PATCH 0/2] phy: rockchip: inno-usb2: fix gadget mode disconnection after 6 seconds Luca Ceresoli
@ 2025-07-22 8:43 ` Luca Ceresoli
2025-11-25 15:28 ` Théo Lebrun
2025-07-22 8:43 ` [PATCH 2/2] phy: rockchip: inno-usb2: fix communication disruption " Luca Ceresoli
2025-10-21 12:09 ` [PATCH 0/2] phy: rockchip: inno-usb2: fix gadget mode disconnection after 6 seconds Luca Ceresoli
2 siblings, 1 reply; 10+ messages in thread
From: Luca Ceresoli @ 2025-07-22 8:43 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Heiko Stuebner, William Wu
Cc: Kever Yang, Minas Harutyunyan, Alan Stern, Louis Chauvet,
Hervé Codina, Thomas Petazzoni, linux-phy, linux-rockchip,
linux-usb, linux-arm-kernel, linux-kernel, Luca Ceresoli
From: Louis Chauvet <louis.chauvet@bootlin.com>
When the OTG USB port is used to power to SoC, configured as peripheral and
used in gadget mode, there is a disconnection about 6 seconds after the
gadget is configured and enumerated.
The problem was observed on a Radxa Rock Pi S board, which can only be
powered by the only USB-C connector. That connector is the only one usable
in gadget mode. This implies the USB cable is connected from before boot
and never disconnects while the kernel runs.
The problem happens because of the PHY driver code flow, summarized as:
* UDC start code (triggered via configfs at any time after boot)
-> phy_init
-> rockchip_usb2phy_init
-> schedule_delayed_work(otg_sm_work [A], 6 sec)
-> phy_power_on
-> rockchip_usb2phy_power_on
-> enable clock
-> rockchip_usb2phy_reset
* Now the gadget interface is up and running.
* 6 seconds later otg_sm_work starts [A]
-> rockchip_usb2phy_otg_sm_work():
if (B_IDLE state && VBUS present && ...):
schedule_delayed_work(&rport->chg_work [B], 0);
* immediately the chg_detect_work starts [B]
-> rockchip_chg_detect_work():
if chg_state is UNDEFINED:
if (!rport->suspended):
rockchip_usb2phy_power_off() <--- [X]
At [X], the PHY is powered off, causing a disconnection. This quickly
triggers a new connection and following re-enumeration, but any connection
that had been established during the 6 seconds is broken.
The code already checks for !rport->suspended, so add a guard for VBUS as
well to avoid a disconnection when a cable is connected.
Fixes: 98898f3bc83c ("phy: rockchip-inno-usb2: support otg-port for rk3399")
Closes: https://lore.kernel.org/lkml/20250414185458.7767aabc@booty/
Co-developed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
index b0f23690ec3002202c0f33a6988f5509622fa10e..0106d7b7ae24ead91d9c996daaa56671de02a39a 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
@@ -821,14 +821,16 @@ static void rockchip_chg_detect_work(struct work_struct *work)
container_of(work, struct rockchip_usb2phy_port, chg_work.work);
struct rockchip_usb2phy *rphy = dev_get_drvdata(rport->phy->dev.parent);
struct regmap *base = get_reg_base(rphy);
- bool is_dcd, tmout, vout;
+ bool is_dcd, tmout, vout, vbus_attach;
unsigned long delay;
+ vbus_attach = property_enabled(rphy->grf, &rport->port_cfg->utmi_bvalid);
+
dev_dbg(&rport->phy->dev, "chg detection work state = %d\n",
rphy->chg_state);
switch (rphy->chg_state) {
case USB_CHG_STATE_UNDEFINED:
- if (!rport->suspended)
+ if (!rport->suspended && !vbus_attach)
rockchip_usb2phy_power_off(rport->phy);
/* put the controller in non-driving mode */
property_enable(base, &rphy->phy_cfg->chg_det.opmode, false);
--
2.50.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] phy: rockchip: inno-usb2: fix communication disruption in gadget mode
2025-07-22 8:43 [PATCH 0/2] phy: rockchip: inno-usb2: fix gadget mode disconnection after 6 seconds Luca Ceresoli
2025-07-22 8:43 ` [PATCH 1/2] phy: rockchip: inno-usb2: fix disconnection in gadget mode Luca Ceresoli
@ 2025-07-22 8:43 ` Luca Ceresoli
2025-11-25 15:39 ` Théo Lebrun
2025-10-21 12:09 ` [PATCH 0/2] phy: rockchip: inno-usb2: fix gadget mode disconnection after 6 seconds Luca Ceresoli
2 siblings, 1 reply; 10+ messages in thread
From: Luca Ceresoli @ 2025-07-22 8:43 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Heiko Stuebner, William Wu
Cc: Kever Yang, Minas Harutyunyan, Alan Stern, Louis Chauvet,
Hervé Codina, Thomas Petazzoni, linux-phy, linux-rockchip,
linux-usb, linux-arm-kernel, linux-kernel, Luca Ceresoli
When the OTG USB port is used to power to SoC, configured as peripheral and
used in gadget mode, communication stops without notice about 6 seconds
after the gadget is configured and enumerated.
The problem was observed on a Radxa Rock Pi S board, which can only be
powered by the only USB-C connector. That connector is the only one usable
in gadget mode. This implies the USB cable is connected from before boot
and never disconnects while the kernel runs.
The related code flow in the PHY driver code can be summarized as:
* UDC start code (triggered via configfs at any time after boot)
-> phy_init
-> rockchip_usb2phy_init
-> schedule_delayed_work(otg_sm_work [A], 6 sec)
-> phy_power_on
-> rockchip_usb2phy_power_on
-> enable clock
-> rockchip_usb2phy_reset
* Now the gadget interface is up and running.
* 6 seconds later otg_sm_work starts [A]
-> rockchip_usb2phy_otg_sm_work():
if (B_IDLE state && VBUS present && ...):
schedule_delayed_work(&rport->chg_work [B], 0);
* immediately the chg_detect_work starts [B]
-> rockchip_chg_detect_work():
if chg_state is UNDEFINED:
property_enable(base, &rphy->phy_cfg->chg_det.opmode, false); [Y]
* rockchip_chg_detect_work() changes state and re-triggers itself a few
times until it reached the DETECTED state:
-> rockchip_chg_detect_work():
if chg_state is DETECTED:
property_enable(base, &rphy->phy_cfg->chg_det.opmode, true); [Z]
At [Y] there is no disconnection and the USB device appears still present
to userspace, but all existing communications stop. E.g. using a CDC serial
gadget, the /dev/tty* devices are still present on both host and device,
but no data is transferred anymore. The later call with a 'true' argument
at [Z] does not restore it.
Due to the lack of documentation, what chg_det.opmode does exactly is not
clear, however by code inspection it seems reasonable that is disables
something needed to keep the communication working, and testing proves that
disabling these lines lefs gadget mode keep working. So prevent changes to
chg_det.opmode when there is a cable connected (VBUS present).
Fixes: 98898f3bc83c ("phy: rockchip-inno-usb2: support otg-port for rk3399")
Closes: https://lore.kernel.org/lkml/20250414185458.7767aabc@booty/
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
index 0106d7b7ae24ead91d9c996daaa56671de02a39a..e5efae7b013590f5b0bf65654008cdc167f52e3f 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
@@ -833,7 +833,8 @@ static void rockchip_chg_detect_work(struct work_struct *work)
if (!rport->suspended && !vbus_attach)
rockchip_usb2phy_power_off(rport->phy);
/* put the controller in non-driving mode */
- property_enable(base, &rphy->phy_cfg->chg_det.opmode, false);
+ if (!vbus_attach)
+ property_enable(base, &rphy->phy_cfg->chg_det.opmode, false);
/* Start DCD processing stage 1 */
rockchip_chg_enable_dcd(rphy, true);
rphy->chg_state = USB_CHG_STATE_WAIT_FOR_DCD;
@@ -896,7 +897,8 @@ static void rockchip_chg_detect_work(struct work_struct *work)
fallthrough;
case USB_CHG_STATE_DETECTED:
/* put the controller in normal mode */
- property_enable(base, &rphy->phy_cfg->chg_det.opmode, true);
+ if (!vbus_attach)
+ property_enable(base, &rphy->phy_cfg->chg_det.opmode, true);
rockchip_usb2phy_otg_sm_work(&rport->otg_sm_work.work);
dev_dbg(&rport->phy->dev, "charger = %s\n",
chg_to_string(rphy->chg_type));
--
2.50.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] phy: rockchip: inno-usb2: fix gadget mode disconnection after 6 seconds
2025-07-22 8:43 [PATCH 0/2] phy: rockchip: inno-usb2: fix gadget mode disconnection after 6 seconds Luca Ceresoli
2025-07-22 8:43 ` [PATCH 1/2] phy: rockchip: inno-usb2: fix disconnection in gadget mode Luca Ceresoli
2025-07-22 8:43 ` [PATCH 2/2] phy: rockchip: inno-usb2: fix communication disruption " Luca Ceresoli
@ 2025-10-21 12:09 ` Luca Ceresoli
2 siblings, 0 replies; 10+ messages in thread
From: Luca Ceresoli @ 2025-10-21 12:09 UTC (permalink / raw)
To: Luca Ceresoli, Vinod Koul, Kishon Vijay Abraham I, Heiko Stuebner,
William Wu
Cc: Kever Yang, Minas Harutyunyan, Alan Stern, Louis Chauvet,
Hervé Codina, Thomas Petazzoni, linux-phy, linux-rockchip,
linux-usb, linux-arm-kernel, linux-kernel, linux-phy
Hello,
On Tue Jul 22, 2025 at 10:43 AM CEST, Luca Ceresoli wrote:
> The USB OTG port of the RK3308 exibits a bug when:
>
> - configured as peripheral, and
> - used in gadget mode, and
> - the USB cable is connected since before booting
>
> The symptom is: about 6 seconds after configuring gadget mode the device is
> disconnected and then re-enumerated. This happens only once per boot.
>
> Investigation showed that in this configuration the charger detection code
> turns off the PHY after 6 seconds. Patch 1 avoids this when a cable is
> connected (VBUS present).
>
> After patch 1 the connection is stable but communication stops after 6
> seconds. this is addressed by patch 2.
>
> The topic had been discussed in [0]. Thanks Alan and Minas for the
> discussion and Louis for having found the 1st issue, leading to patch 1.
>
> [0] https://lore.kernel.org/lkml/20250414185458.7767aabc@booty/
>
> Luca
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Anything I should do to move forward with this work?
These two few-liner patches solve real-world bugs, there is no complaint
about them, and both the bug and the solution is explained as clearly as
the public documentation allows.
FWIW, this series applied cleany on current master
(v6.18-rc2-8-g6548d364a3e8).
Best regards,
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] phy: rockchip: inno-usb2: fix disconnection in gadget mode
2025-07-22 8:43 ` [PATCH 1/2] phy: rockchip: inno-usb2: fix disconnection in gadget mode Luca Ceresoli
@ 2025-11-25 15:28 ` Théo Lebrun
2025-11-27 9:22 ` Luca Ceresoli
0 siblings, 1 reply; 10+ messages in thread
From: Théo Lebrun @ 2025-11-25 15:28 UTC (permalink / raw)
To: Luca Ceresoli, Vinod Koul, Kishon Vijay Abraham I, Heiko Stuebner,
William Wu
Cc: Kever Yang, Minas Harutyunyan, Alan Stern, Louis Chauvet,
Hervé Codina, Thomas Petazzoni, linux-phy, linux-rockchip,
linux-usb, linux-arm-kernel, linux-kernel
Hello Luca,
On Tue Jul 22, 2025 at 10:43 AM CEST, Luca Ceresoli wrote:
> From: Louis Chauvet <louis.chauvet@bootlin.com>
>
> When the OTG USB port is used to power to SoC, configured as peripheral and
typo: is used to power *the* SoC
> used in gadget mode, there is a disconnection about 6 seconds after the
> gadget is configured and enumerated.
>
> The problem was observed on a Radxa Rock Pi S board, which can only be
> powered by the only USB-C connector. That connector is the only one usable
"which can only be powered by the only USB-C connector"
double "only" makes it a super exclusive connector!
> in gadget mode. This implies the USB cable is connected from before boot
> and never disconnects while the kernel runs.
>
> The problem happens because of the PHY driver code flow, summarized as:
>
> * UDC start code (triggered via configfs at any time after boot)
> -> phy_init
> -> rockchip_usb2phy_init
> -> schedule_delayed_work(otg_sm_work [A], 6 sec)
> -> phy_power_on
> -> rockchip_usb2phy_power_on
> -> enable clock
> -> rockchip_usb2phy_reset
>
> * Now the gadget interface is up and running.
>
> * 6 seconds later otg_sm_work starts [A]
> -> rockchip_usb2phy_otg_sm_work():
> if (B_IDLE state && VBUS present && ...):
> schedule_delayed_work(&rport->chg_work [B], 0);
>
> * immediately the chg_detect_work starts [B]
> -> rockchip_chg_detect_work():
> if chg_state is UNDEFINED:
> if (!rport->suspended):
> rockchip_usb2phy_power_off() <--- [X]
>
> At [X], the PHY is powered off, causing a disconnection. This quickly
> triggers a new connection and following re-enumeration, but any connection
> that had been established during the 6 seconds is broken.
>
> The code already checks for !rport->suspended, so add a guard for VBUS as
> well to avoid a disconnection when a cable is connected.
Your commit message was clear but I was missing one key point: what
rport->suspended means. It isn't what I first thought. Instead it means
phy is powered off. Naming is bad but unrelated to your series. Maybe
add a comment to your commit message like the following?
The code already checks for !rport->suspended (PHY is powered on), ...
> Fixes: 98898f3bc83c ("phy: rockchip-inno-usb2: support otg-port for rk3399")
> Closes: https://lore.kernel.org/lkml/20250414185458.7767aabc@booty/
> Co-developed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
I believe there is an issue with your SoB ordering. It is meant to
signal the path's route from author into the kernel tree. In that
sense, it should start with author followed by you as submitter. Then
the maintainer will append its own.
Documentation/process/submitting-patches.rst is semi-explicitly writing
that out:
> Any further SoBs [...] following the author's SoB [...]
https://elixir.bootlin.com/linux/v6.17.9/source/Documentation/process/submitting-patches.rst#L449-L453
One good way to check is that all maintainers always append their SoB
rather than prepend.
With that,
Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com>
Thanks,
--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] phy: rockchip: inno-usb2: fix communication disruption in gadget mode
2025-07-22 8:43 ` [PATCH 2/2] phy: rockchip: inno-usb2: fix communication disruption " Luca Ceresoli
@ 2025-11-25 15:39 ` Théo Lebrun
0 siblings, 0 replies; 10+ messages in thread
From: Théo Lebrun @ 2025-11-25 15:39 UTC (permalink / raw)
To: Luca Ceresoli, Vinod Koul, Kishon Vijay Abraham I, Heiko Stuebner,
William Wu
Cc: Kever Yang, Minas Harutyunyan, Alan Stern, Louis Chauvet,
Hervé Codina, Thomas Petazzoni, linux-phy, linux-rockchip,
linux-usb, linux-arm-kernel, linux-kernel
Hello Luca,
On Tue Jul 22, 2025 at 10:43 AM CEST, Luca Ceresoli wrote:
> When the OTG USB port is used to power to SoC, configured as peripheral and
> used in gadget mode, communication stops without notice about 6 seconds
> after the gadget is configured and enumerated.
>
> The problem was observed on a Radxa Rock Pi S board, which can only be
> powered by the only USB-C connector. That connector is the only one usable
> in gadget mode. This implies the USB cable is connected from before boot
> and never disconnects while the kernel runs.
>
> The related code flow in the PHY driver code can be summarized as:
>
> * UDC start code (triggered via configfs at any time after boot)
> -> phy_init
> -> rockchip_usb2phy_init
> -> schedule_delayed_work(otg_sm_work [A], 6 sec)
> -> phy_power_on
> -> rockchip_usb2phy_power_on
> -> enable clock
> -> rockchip_usb2phy_reset
The above code flow summary was important for [PATCH 1/2] but it feels
like not as important for [PATCH 2/2], could you drop or summarise it?
The key point is that the below DCD sequence has invalid assumptions
and does side effects that don't fit if VBUS is already present. I feel
like it distracted me from the main point that is chg_det.opmode
writes.
> * Now the gadget interface is up and running.
>
> * 6 seconds later otg_sm_work starts [A]
> -> rockchip_usb2phy_otg_sm_work():
> if (B_IDLE state && VBUS present && ...):
> schedule_delayed_work(&rport->chg_work [B], 0);
>
> * immediately the chg_detect_work starts [B]
> -> rockchip_chg_detect_work():
> if chg_state is UNDEFINED:
> property_enable(base, &rphy->phy_cfg->chg_det.opmode, false); [Y]
>
> * rockchip_chg_detect_work() changes state and re-triggers itself a few
> times until it reached the DETECTED state:
> -> rockchip_chg_detect_work():
> if chg_state is DETECTED:
> property_enable(base, &rphy->phy_cfg->chg_det.opmode, true); [Z]
>
> At [Y] there is no disconnection and the USB device appears still present
> to userspace, but all existing communications stop. E.g. using a CDC serial
> gadget, the /dev/tty* devices are still present on both host and device,
> but no data is transferred anymore. The later call with a 'true' argument
> at [Z] does not restore it.
You mention "there is no disconnection" but that sounds irrelevant to
this precise commit. The issue at hand is a communication halt.
> Due to the lack of documentation, what chg_det.opmode does exactly is not
> clear, however by code inspection it seems reasonable that is disables
> something needed to keep the communication working, and testing proves that
> disabling these lines lefs gadget mode keep working. So prevent changes to
> chg_det.opmode when there is a cable connected (VBUS present).
"lefs" -> "let's", I think
With those nits
Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com>
Thanks,
--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] phy: rockchip: inno-usb2: fix disconnection in gadget mode
2025-11-25 15:28 ` Théo Lebrun
@ 2025-11-27 9:22 ` Luca Ceresoli
2025-11-27 9:48 ` Théo Lebrun
0 siblings, 1 reply; 10+ messages in thread
From: Luca Ceresoli @ 2025-11-27 9:22 UTC (permalink / raw)
To: Théo Lebrun, Vinod Koul, Kishon Vijay Abraham I,
Heiko Stuebner, William Wu
Cc: Kever Yang, Minas Harutyunyan, Alan Stern, Louis Chauvet,
Hervé Codina, Thomas Petazzoni, linux-phy, linux-rockchip,
linux-usb, linux-arm-kernel, linux-kernel
Hi Théo,
thanks for reviewing this series!
On Tue Nov 25, 2025 at 4:28 PM CET, Théo Lebrun wrote:
[...]
>> The problem was observed on a Radxa Rock Pi S board, which can only be
>> powered by the only USB-C connector. That connector is the only one usable
>
> "which can only be powered by the only USB-C connector"
> double "only" makes it a super exclusive connector!
Hehe, indeed! :-)
>> The code already checks for !rport->suspended, so add a guard for VBUS as
>> well to avoid a disconnection when a cable is connected.
>
> Your commit message was clear but I was missing one key point: what
> rport->suspended means. It isn't what I first thought. Instead it means
> phy is powered off. Naming is bad but unrelated to your series. Maybe
> add a comment to your commit message like the following?
>
> The code already checks for !rport->suspended (PHY is powered on), ...
You are right. I have added a slightly longer text:
The code already checks for !rport->suspended (which, somewhat
counter-intuitively, means the PHY is powered on), ...
Still worth your Reviewed-by?
I fixed the other issues you have reported, for patch 2 as well.
I also added the Cc: stable@vger.kernel.org line, which I noticed being
missing.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] phy: rockchip: inno-usb2: fix disconnection in gadget mode
2025-11-27 9:22 ` Luca Ceresoli
@ 2025-11-27 9:48 ` Théo Lebrun
2025-11-27 10:18 ` Luca Ceresoli
0 siblings, 1 reply; 10+ messages in thread
From: Théo Lebrun @ 2025-11-27 9:48 UTC (permalink / raw)
To: Luca Ceresoli, Théo Lebrun, Vinod Koul,
Kishon Vijay Abraham I, Heiko Stuebner, William Wu
Cc: Kever Yang, Minas Harutyunyan, Alan Stern, Louis Chauvet,
Hervé Codina, Thomas Petazzoni, linux-phy, linux-rockchip,
linux-usb, linux-arm-kernel, linux-kernel
On Thu Nov 27, 2025 at 10:22 AM CET, Luca Ceresoli wrote:
> On Tue Nov 25, 2025 at 4:28 PM CET, Théo Lebrun wrote:
>>> The code already checks for !rport->suspended, so add a guard for VBUS as
>>> well to avoid a disconnection when a cable is connected.
>>
>> Your commit message was clear but I was missing one key point: what
>> rport->suspended means. It isn't what I first thought. Instead it means
>> phy is powered off. Naming is bad but unrelated to your series. Maybe
>> add a comment to your commit message like the following?
>>
>> The code already checks for !rport->suspended (PHY is powered on), ...
>
> You are right. I have added a slightly longer text:
>
> The code already checks for !rport->suspended (which, somewhat
> counter-intuitively, means the PHY is powered on), ...
>
> Still worth your Reviewed-by?
Even more so.
> I fixed the other issues you have reported, for patch 2 as well.
>
> I also added the Cc: stable@vger.kernel.org line, which I noticed being
> missing.
I never add that Cc trailer and only rely on `Fixes:`. I thought it
used to be documented as an alternative to that Cc trailer but it does
not show up in `git log -p Documentation/process/stable-kernel-rules.rst`
There is one indirect mention of "scripts that look for commits
containing a 'Fixes:' tag":
https://elixir.bootlin.com/linux/v6.17.9/source/Documentation/process/stable-kernel-rules.rst#L132-L134
Anyway, you do right by explicitly tagging `Cc: stable@...`.
Thanks,
--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] phy: rockchip: inno-usb2: fix disconnection in gadget mode
2025-11-27 9:48 ` Théo Lebrun
@ 2025-11-27 10:18 ` Luca Ceresoli
2025-11-27 12:20 ` Greg KH
0 siblings, 1 reply; 10+ messages in thread
From: Luca Ceresoli @ 2025-11-27 10:18 UTC (permalink / raw)
To: Théo Lebrun, Vinod Koul, Kishon Vijay Abraham I,
Heiko Stuebner, William Wu
Cc: Kever Yang, Minas Harutyunyan, Alan Stern, Louis Chauvet,
Hervé Codina, Thomas Petazzoni, linux-phy, linux-rockchip,
linux-usb, linux-arm-kernel, linux-kernel
Hi Théo,
On Thu Nov 27, 2025 at 10:48 AM CET, Théo Lebrun wrote:
> On Thu Nov 27, 2025 at 10:22 AM CET, Luca Ceresoli wrote:
>> On Tue Nov 25, 2025 at 4:28 PM CET, Théo Lebrun wrote:
>>>> The code already checks for !rport->suspended, so add a guard for VBUS as
>>>> well to avoid a disconnection when a cable is connected.
>>>
>>> Your commit message was clear but I was missing one key point: what
>>> rport->suspended means. It isn't what I first thought. Instead it means
>>> phy is powered off. Naming is bad but unrelated to your series. Maybe
>>> add a comment to your commit message like the following?
>>>
>>> The code already checks for !rport->suspended (PHY is powered on), ...
>>
>> You are right. I have added a slightly longer text:
>>
>> The code already checks for !rport->suspended (which, somewhat
>> counter-intuitively, means the PHY is powered on), ...
>>
>> Still worth your Reviewed-by?
>
> Even more so.
Thanks, v2 on its way.
>> I also added the Cc: stable@vger.kernel.org line, which I noticed being
>> missing.
>
> I never add that Cc trailer and only rely on `Fixes:`. I thought it
> used to be documented as an alternative to that Cc trailer but it does
> not show up in `git log -p Documentation/process/stable-kernel-rules.rst`
>
> There is one indirect mention of "scripts that look for commits
> containing a 'Fixes:' tag":
> https://elixir.bootlin.com/linux/v6.17.9/source/Documentation/process/stable-kernel-rules.rst#L132-L134
>
> Anyway, you do right by explicitly tagging `Cc: stable@...`.
Theory says Cc: is needed:
> Note: Attaching a Fixes: tag does not subvert the stable kernel rules
> process nor the requirement to Cc: stable@vger.kernel.org on all stable
> patch candidates.
(https://docs.kernel.org/process/submitting-patches.html#reviewer-s-statement-of-oversight)
But in the practice I happened to forget Cc: stable in the past, the patch
got applied and the Fixes: tag was enough for correct cherry-pick in stable
branches.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] phy: rockchip: inno-usb2: fix disconnection in gadget mode
2025-11-27 10:18 ` Luca Ceresoli
@ 2025-11-27 12:20 ` Greg KH
0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2025-11-27 12:20 UTC (permalink / raw)
To: Luca Ceresoli
Cc: Théo Lebrun, Vinod Koul, Kishon Vijay Abraham I,
Heiko Stuebner, William Wu, Kever Yang, Minas Harutyunyan,
Alan Stern, Louis Chauvet, Hervé Codina, Thomas Petazzoni,
linux-phy, linux-rockchip, linux-usb, linux-arm-kernel,
linux-kernel
On Thu, Nov 27, 2025 at 11:18:45AM +0100, Luca Ceresoli wrote:
> Hi Théo,
>
> On Thu Nov 27, 2025 at 10:48 AM CET, Théo Lebrun wrote:
> > On Thu Nov 27, 2025 at 10:22 AM CET, Luca Ceresoli wrote:
> >> On Tue Nov 25, 2025 at 4:28 PM CET, Théo Lebrun wrote:
> >>>> The code already checks for !rport->suspended, so add a guard for VBUS as
> >>>> well to avoid a disconnection when a cable is connected.
> >>>
> >>> Your commit message was clear but I was missing one key point: what
> >>> rport->suspended means. It isn't what I first thought. Instead it means
> >>> phy is powered off. Naming is bad but unrelated to your series. Maybe
> >>> add a comment to your commit message like the following?
> >>>
> >>> The code already checks for !rport->suspended (PHY is powered on), ...
> >>
> >> You are right. I have added a slightly longer text:
> >>
> >> The code already checks for !rport->suspended (which, somewhat
> >> counter-intuitively, means the PHY is powered on), ...
> >>
> >> Still worth your Reviewed-by?
> >
> > Even more so.
>
> Thanks, v2 on its way.
>
> >> I also added the Cc: stable@vger.kernel.org line, which I noticed being
> >> missing.
> >
> > I never add that Cc trailer and only rely on `Fixes:`. I thought it
> > used to be documented as an alternative to that Cc trailer but it does
> > not show up in `git log -p Documentation/process/stable-kernel-rules.rst`
> >
> > There is one indirect mention of "scripts that look for commits
> > containing a 'Fixes:' tag":
> > https://elixir.bootlin.com/linux/v6.17.9/source/Documentation/process/stable-kernel-rules.rst#L132-L134
> >
> > Anyway, you do right by explicitly tagging `Cc: stable@...`.
>
> Theory says Cc: is needed:
>
> > Note: Attaching a Fixes: tag does not subvert the stable kernel rules
> > process nor the requirement to Cc: stable@vger.kernel.org on all stable
> > patch candidates.
> (https://docs.kernel.org/process/submitting-patches.html#reviewer-s-statement-of-oversight)
>
> But in the practice I happened to forget Cc: stable in the past, the patch
> got applied and the Fixes: tag was enough for correct cherry-pick in stable
> branches.
That is never guaranteed, it is a "best effort only when the stable
maintainers are bored" type of thing. Always be explicit, and use cc:
stable, as the documentation has stated for the last 17+ years :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-11-27 12:20 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-22 8:43 [PATCH 0/2] phy: rockchip: inno-usb2: fix gadget mode disconnection after 6 seconds Luca Ceresoli
2025-07-22 8:43 ` [PATCH 1/2] phy: rockchip: inno-usb2: fix disconnection in gadget mode Luca Ceresoli
2025-11-25 15:28 ` Théo Lebrun
2025-11-27 9:22 ` Luca Ceresoli
2025-11-27 9:48 ` Théo Lebrun
2025-11-27 10:18 ` Luca Ceresoli
2025-11-27 12:20 ` Greg KH
2025-07-22 8:43 ` [PATCH 2/2] phy: rockchip: inno-usb2: fix communication disruption " Luca Ceresoli
2025-11-25 15:39 ` Théo Lebrun
2025-10-21 12:09 ` [PATCH 0/2] phy: rockchip: inno-usb2: fix gadget mode disconnection after 6 seconds Luca Ceresoli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).