Re: [RFC PATCH] usb: dwc3: fix maximum_speed check for usb2.0-only core
On Fri, 2020-07-24 at 03:32 +, 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 > >> --- > >> 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
Re: [RFC PATCH] usb: dwc3: fix maximum_speed check for usb2.0-only core
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 >> --- >> 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
Re: [RFC PATCH] usb: dwc3: fix maximum_speed check for usb2.0-only core
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 > --- > 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
[RFC PATCH] usb: dwc3: fix maximum_speed check for usb2.0-only core
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 --- 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) -- 1.9.1