| Message ID | 1684936207-23529-1-git-send-email-quic_prashk@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] usb: common: usb-conn-gpio: Set last role to unknown before initial detection | expand |
On Wed, 2023-05-24 at 19:20 +0530, Prashanth K wrote: > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > Currently if we bootup a device without cable connected, then > usb-conn-gpio won't call set_role() since last_role is same as > current role. This happens because during probe last_role gets > initialized to zero. > > To avoid this, added a new constant in enum usb_role, last_role > is set to USB_ROLE_UNKNOWN before performing initial detection. > > Fixes: 4602f3bff266 ("usb: common: add USB GPIO based connection > detection driver") > Signed-off-by: Prashanth K <quic_prashk@quicinc.com> > --- > v2: Added USB_ROLE_UNKNWON to enum usb_role > > drivers/usb/common/usb-conn-gpio.c | 3 +++ > include/linux/usb/role.h | 1 + > 2 files changed, 4 insertions(+) > > 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; Do you only use vbus-pin? This driver assumes that the gadget driver's default role is none. > + > /* Perform initial detection */ > usb_conn_queue_dwork(info, 0); > > diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h > index b5deafd..221d462 100644 > --- a/include/linux/usb/role.h > +++ b/include/linux/usb/role.h > @@ -8,6 +8,7 @@ > struct usb_role_switch; > > enum usb_role { > + USB_ROLE_UNKNOWN = -1, > USB_ROLE_NONE, > USB_ROLE_HOST, > USB_ROLE_DEVICE, > -- > 2.7.4 >
Il 24/05/23 15:50, Prashanth K ha scritto: > Currently if we bootup a device without cable connected, then > usb-conn-gpio won't call set_role() since last_role is same as > current role. This happens because during probe last_role gets > initialized to zero. > > To avoid this, added a new constant in enum usb_role, last_role > is set to USB_ROLE_UNKNOWN before performing initial detection. > > Fixes: 4602f3bff266 ("usb: common: add USB GPIO based connection detection driver") > Signed-off-by: Prashanth K <quic_prashk@quicinc.com> There's an issue with drivers/usb/cdns3/core.c as pointed out by the test robot; the solution is to handle `default` in the switch, I'd say that it would be safe to handle it as default: break; after solving that: Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- > v2: Added USB_ROLE_UNKNWON to enum usb_role > > drivers/usb/common/usb-conn-gpio.c | 3 +++ > include/linux/usb/role.h | 1 + > 2 files changed, 4 insertions(+) > > 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; > + > /* Perform initial detection */ > usb_conn_queue_dwork(info, 0); > > diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h > index b5deafd..221d462 100644 > --- a/include/linux/usb/role.h > +++ b/include/linux/usb/role.h > @@ -8,6 +8,7 @@ > struct usb_role_switch; > > enum usb_role { > + USB_ROLE_UNKNOWN = -1, > USB_ROLE_NONE, > USB_ROLE_HOST, > USB_ROLE_DEVICE,
On 25-05-23 12:27 pm, Chunfeng Yun (云春峰) wrote: > On Wed, 2023-05-24 at 19:20 +0530, Prashanth K wrote: >> External email : Please do not click links or open attachments until >> you have verified the sender or the content. >> >> >> Currently if we bootup a device without cable connected, then >> usb-conn-gpio won't call set_role() since last_role is same as >> current role. This happens because during probe last_role gets >> initialized to zero. >> >> To avoid this, added a new constant in enum usb_role, last_role >> is set to USB_ROLE_UNKNOWN before performing initial detection. >> >> Fixes: 4602f3bff266 ("usb: common: add USB GPIO based connection >> detection driver") >> Signed-off-by: Prashanth K <quic_prashk@quicinc.com> >> --- >> v2: Added USB_ROLE_UNKNWON to enum usb_role >> >> drivers/usb/common/usb-conn-gpio.c | 3 +++ >> include/linux/usb/role.h | 1 + >> 2 files changed, 4 insertions(+) >> >> 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; > > Do you only use vbus-pin? This driver has support for both Vbus and ID GPIOs. > > This driver assumes that the gadget driver's default role is none. No, after probe it calls set role based on the state of Vbus and ID pin. If Vbus is low, then it should issue none role to the gadget. But currently it doesnt call set_role if initial role is none. Regards
On 25-05-23 01:37 pm, AngeloGioacchino Del Regno wrote: > Il 24/05/23 15:50, Prashanth K ha scritto: >> Currently if we bootup a device without cable connected, then >> usb-conn-gpio won't call set_role() since last_role is same as >> current role. This happens because during probe last_role gets >> initialized to zero. >> >> To avoid this, added a new constant in enum usb_role, last_role >> is set to USB_ROLE_UNKNOWN before performing initial detection. >> >> Fixes: 4602f3bff266 ("usb: common: add USB GPIO based connection >> detection driver") >> Signed-off-by: Prashanth K <quic_prashk@quicinc.com> > > There's an issue with drivers/usb/cdns3/core.c as pointed out by the > test robot; the solution is to handle `default` in the switch, I'd say > that it would be safe to handle it as > > default: > break; > > after solving that: > > Reviewed-by: AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> Yea sure, thanks for the suggestion Agnelo Regards, Prashanth K
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; + /* Perform initial detection */ usb_conn_queue_dwork(info, 0); diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h index b5deafd..221d462 100644 --- a/include/linux/usb/role.h +++ b/include/linux/usb/role.h @@ -8,6 +8,7 @@ struct usb_role_switch; enum usb_role { + USB_ROLE_UNKNOWN = -1, USB_ROLE_NONE, USB_ROLE_HOST, USB_ROLE_DEVICE,
Currently if we bootup a device without cable connected, then usb-conn-gpio won't call set_role() since last_role is same as current role. This happens because during probe last_role gets initialized to zero. To avoid this, added a new constant in enum usb_role, last_role is set to USB_ROLE_UNKNOWN before performing initial detection. Fixes: 4602f3bff266 ("usb: common: add USB GPIO based connection detection driver") Signed-off-by: Prashanth K <quic_prashk@quicinc.com> --- v2: Added USB_ROLE_UNKNWON to enum usb_role drivers/usb/common/usb-conn-gpio.c | 3 +++ include/linux/usb/role.h | 1 + 2 files changed, 4 insertions(+)