Re: [PATCH v5] usb: common: usb-conn-gpio: Set last role to unknown before initial detection

From: Prashanth K
Date: Mon May 29 2023 - 14:30:37 EST

On 28-05-23 05:03 pm, Greg Kroah-Hartman wrote:
diff --git a/drivers/usb/common/usb-conn-gpio.c b/drivers/usb/common/usb-conn-gpio.c
index e20874c..30bdb81 100644
--- a/drivers/usb/common/usb-conn-gpio.c
+++ b/drivers/usb/common/usb-conn-gpio.c
@@ -257,6 +257,9 @@ static int usb_conn_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, info);
device_set_wakeup_capable(&pdev->dev, true);
+ /* Set last role to unknown before performing the initial detection */
+ info->last_role = USB_ROLE_UNKNOWN;

Shouldn't last_role have already been set to 0? If so, why not just
have this enum value be 0?
Last role would be 0 during first detection, that's the problem here.
During initial detection, if the the new role is detected as USB_ROLE_NONE
(0), then we wouldn't call the set_role(). But it should send the current
role to gadget after the inital detection.

So you are hoping that the old enum type is still assigned to 0? That's
brave, please make it explicit otherwise it's very hard to follow or
ensure that this really will happen. And most of all, document it so
that that value remains 0 in the future, otherwise a list of enum types
without explicit values are seen as if the values do not matter.


greg k-h

So I think it would be better to add USB_ROLE_UNKNOWN towards the end of enum usb_role, so that we can avoid explicit declaration. Is that fine?

enum usb_role {

Prashanth K