On 30-05-23 12:31 am, Greg Kroah-Hartman wrote:
On Tue, May 30, 2023 at 12:00:15AM +0530, Prashanth K wrote:
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.
thanks,
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 {
USB_ROLE_NONE,
USB_ROLE_HOST,
USB_ROLE_DEVICE,
+ USB_ROLE_UNKNOWN,
Either is fine, be explicit, or not, just don't mix the two please.
thanks,
greg k-h
Thanks for the suggestion, will update it in next patch.
Regards,
Prashanth K
