| Message ID | 1594282250-25544-1-git-send-email-chunfeng.yun@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] usb: dwc3: fix maximum_speed check for usb2.0-only core | expand |
Hi, Chunfeng Yun wrote: > The maximum_speed will be USB_SPEED_SUPER_PLUS, but the > maximum_speed check for usb2.0-only core doesn't consider it, > so fix it, and move the ckeck into dwc3_check_params(). > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > --- > Note: > > When I look at the code, find that this may be a problem, but no > platform to test it. > --- > drivers/usb/dwc3/core.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 25c686a7..ffd5ab3 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -930,13 +930,6 @@ static int dwc3_core_init(struct dwc3 *dwc) > */ > dwc3_writel(dwc->regs, DWC3_GUID, LINUX_VERSION_CODE); > > - /* Handle USB2.0-only core configuration */ > - if (DWC3_GHWPARAMS3_SSPHY_IFC(dwc->hwparams.hwparams3) == > - DWC3_GHWPARAMS3_SSPHY_IFC_DIS) { > - if (dwc->maximum_speed == USB_SPEED_SUPER) > - dwc->maximum_speed = USB_SPEED_HIGH; > - } > - > ret = dwc3_phy_setup(dwc); > if (ret) > goto err0; > @@ -1426,6 +1419,13 @@ static void dwc3_check_params(struct dwc3 *dwc) > > break; > } > + > + /* Handle USB2.0-only core configuration */ > + if (DWC3_GHWPARAMS3_SSPHY_IFC(dwc->hwparams.hwparams3) == > + DWC3_GHWPARAMS3_SSPHY_IFC_DIS) { > + if (dwc->maximum_speed > USB_SPEED_HIGH) > + dwc->maximum_speed = USB_SPEED_HIGH; > + } > } > > static int dwc3_probe(struct platform_device *pdev) Actually, the dwc->maximum_speed captures the maximum speed device property value. It maybe be set based on the phy's capability if there's no property specifying it (i.e. maximum_speed is USB_SPEED_UNKNOWN). So, this code should be removed. The fix should be in the check of dwc3_check_params(). If maximum_speed = USB_SPEED_UNKNOWN and the phy's capability is only up to highspeed, then set the maximum_speed to highspeed only. Thanks, Thinh
Hi, Thinh Nguyen wrote: > Hi, > > Chunfeng Yun wrote: >> The maximum_speed will be USB_SPEED_SUPER_PLUS, but the >> maximum_speed check for usb2.0-only core doesn't consider it, >> so fix it, and move the ckeck into dwc3_check_params(). >> >> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> >> --- >> Note: >> >> When I look at the code, find that this may be a problem, but no >> platform to test it. >> --- >> drivers/usb/dwc3/core.c | 14 +++++++------- >> 1 file changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >> index 25c686a7..ffd5ab3 100644 >> --- a/drivers/usb/dwc3/core.c >> +++ b/drivers/usb/dwc3/core.c >> @@ -930,13 +930,6 @@ static int dwc3_core_init(struct dwc3 *dwc) >> */ >> dwc3_writel(dwc->regs, DWC3_GUID, LINUX_VERSION_CODE); >> >> - /* Handle USB2.0-only core configuration */ >> - if (DWC3_GHWPARAMS3_SSPHY_IFC(dwc->hwparams.hwparams3) == >> - DWC3_GHWPARAMS3_SSPHY_IFC_DIS) { >> - if (dwc->maximum_speed == USB_SPEED_SUPER) >> - dwc->maximum_speed = USB_SPEED_HIGH; >> - } >> - >> ret = dwc3_phy_setup(dwc); >> if (ret) >> goto err0; >> @@ -1426,6 +1419,13 @@ static void dwc3_check_params(struct dwc3 *dwc) >> >> break; >> } >> + >> + /* Handle USB2.0-only core configuration */ >> + if (DWC3_GHWPARAMS3_SSPHY_IFC(dwc->hwparams.hwparams3) == >> + DWC3_GHWPARAMS3_SSPHY_IFC_DIS) { >> + if (dwc->maximum_speed > USB_SPEED_HIGH) >> + dwc->maximum_speed = USB_SPEED_HIGH; >> + } >> } >> >> static int dwc3_probe(struct platform_device *pdev) > Actually, the dwc->maximum_speed captures the maximum speed device > property value. It maybe be set based on the phy's capability if there's > no property specifying it (i.e. maximum_speed is USB_SPEED_UNKNOWN). > > So, this code should be removed. The fix should be in the check of > dwc3_check_params(). If maximum_speed = USB_SPEED_UNKNOWN and the phy's > capability is only up to highspeed, then set the maximum_speed to > highspeed only. > Are you going into update and resend this patch? If not I can create one and add your "Reported-by" BR, Thinh
On Fri, 2020-07-24 at 03:32 +0000, Thinh Nguyen wrote: > Hi, > > Thinh Nguyen wrote: > > Hi, > > > > Chunfeng Yun wrote: > >> The maximum_speed will be USB_SPEED_SUPER_PLUS, but the > >> maximum_speed check for usb2.0-only core doesn't consider it, > >> so fix it, and move the ckeck into dwc3_check_params(). > >> > >> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > >> --- > >> Note: > >> > >> When I look at the code, find that this may be a problem, but no > >> platform to test it. > >> --- > >> drivers/usb/dwc3/core.c | 14 +++++++------- > >> 1 file changed, 7 insertions(+), 7 deletions(-) > >> > >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > >> index 25c686a7..ffd5ab3 100644 > >> --- a/drivers/usb/dwc3/core.c > >> +++ b/drivers/usb/dwc3/core.c > >> @@ -930,13 +930,6 @@ static int dwc3_core_init(struct dwc3 *dwc) > >> */ > >> dwc3_writel(dwc->regs, DWC3_GUID, LINUX_VERSION_CODE); > >> > >> - /* Handle USB2.0-only core configuration */ > >> - if (DWC3_GHWPARAMS3_SSPHY_IFC(dwc->hwparams.hwparams3) == > >> - DWC3_GHWPARAMS3_SSPHY_IFC_DIS) { > >> - if (dwc->maximum_speed == USB_SPEED_SUPER) > >> - dwc->maximum_speed = USB_SPEED_HIGH; > >> - } > >> - > >> ret = dwc3_phy_setup(dwc); > >> if (ret) > >> goto err0; > >> @@ -1426,6 +1419,13 @@ static void dwc3_check_params(struct dwc3 *dwc) > >> > >> break; > >> } > >> + > >> + /* Handle USB2.0-only core configuration */ > >> + if (DWC3_GHWPARAMS3_SSPHY_IFC(dwc->hwparams.hwparams3) == > >> + DWC3_GHWPARAMS3_SSPHY_IFC_DIS) { > >> + if (dwc->maximum_speed > USB_SPEED_HIGH) > >> + dwc->maximum_speed = USB_SPEED_HIGH; > >> + } > >> } > >> > >> static int dwc3_probe(struct platform_device *pdev) > > Actually, the dwc->maximum_speed captures the maximum speed device > > property value. It maybe be set based on the phy's capability if there's > > no property specifying it (i.e. maximum_speed is USB_SPEED_UNKNOWN). > > > > So, this code should be removed. The fix should be in the check of > > dwc3_check_params(). If maximum_speed = USB_SPEED_UNKNOWN and the phy's > > capability is only up to highspeed, then set the maximum_speed to > > highspeed only. Sorry for the late reply, my outlook lose your email. > > > > Are you going into update and resend this patch? I don't know how to check "the phy's capability is only up to highspeed" > If not I can create one > and add your "Reported-by" Ok, thanks > > BR, > Thinh
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 25c686a7..ffd5ab3 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -930,13 +930,6 @@ static int dwc3_core_init(struct dwc3 *dwc) */ dwc3_writel(dwc->regs, DWC3_GUID, LINUX_VERSION_CODE); - /* Handle USB2.0-only core configuration */ - if (DWC3_GHWPARAMS3_SSPHY_IFC(dwc->hwparams.hwparams3) == - DWC3_GHWPARAMS3_SSPHY_IFC_DIS) { - if (dwc->maximum_speed == USB_SPEED_SUPER) - dwc->maximum_speed = USB_SPEED_HIGH; - } - ret = dwc3_phy_setup(dwc); if (ret) goto err0; @@ -1426,6 +1419,13 @@ static void dwc3_check_params(struct dwc3 *dwc) break; } + + /* Handle USB2.0-only core configuration */ + if (DWC3_GHWPARAMS3_SSPHY_IFC(dwc->hwparams.hwparams3) == + DWC3_GHWPARAMS3_SSPHY_IFC_DIS) { + if (dwc->maximum_speed > USB_SPEED_HIGH) + dwc->maximum_speed = USB_SPEED_HIGH; + } } static int dwc3_probe(struct platform_device *pdev)
The maximum_speed will be USB_SPEED_SUPER_PLUS, but the maximum_speed check for usb2.0-only core doesn't consider it, so fix it, and move the ckeck into dwc3_check_params(). Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> --- Note: When I look at the code, find that this may be a problem, but no platform to test it. --- drivers/usb/dwc3/core.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)