Hi, John Youn <John.Youn@xxxxxxxxxxxx> writes: >>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>>> index 5bbdf5d9c35e..d8566ad60d9b 100644 >>>> --- a/drivers/usb/dwc3/gadget.c >>>> +++ b/drivers/usb/dwc3/gadget.c >>>> @@ -1633,9 +1633,14 @@ static int dwc3_gadget_start(struct usb_gadget *g, >>>> reg |= DWC3_DSTS_SUPERSPEED_PLUS; >>>> break; >>>> case USB_SPEED_SUPER: /* FALLTHROUGH */ >>>> + reg |= DWC3_DSTS_SUPERSPEED; >>>> + break; >>>> case USB_SPEED_UNKNOWN: /* FALTHROUGH */ >>>> default: >>>> - reg |= DWC3_DSTS_SUPERSPEED; >>>> + if (dwc_is_usb31(dwc)) >>>> + reg |= DWC3_DSTS_SUPERSPEED_PLUS; >>>> + else >>>> + reg |= DWC3_DSTS_SUPERSPEED; >>>> } >>>> } >>>> dwc3_writel(dwc->regs, DWC3_DCFG, reg); >>>> >>>> the reason being that default speed should always be the fastest speed >>>> supported by current core and, after this patchset we start support >>>> superspeed plus. >>>> >>>> What do you think ? >>>> >>> >>> Hi Felipe, >>> >>> That check is already done on the dwc->maximum_speed parameter in the >>> probe when set to UNKNOWN. Though that check also involves checking >>> the GHWPARAMS3.SSPHY_IF. Which, I think you reported before that some >>> systems may not set correctly. I was thinking those systems can just >>> set maximum_speed if needed. >> >> In that case, the default case statement here is unnecessary. >> > > The maximum_speed is not fully validated. It is only adjusted on > USB_SPEED_UNKNOWN. So if a user passes in some completely invalid > value then and we could end up in the default condition here. > > I'll fix it so that can't occur. > > But even then, I think it is reasonable to handle the default > condition in the switch, if only to guard against programmer error in > the future. Maybe print an error and have some safe fall-back. Are you > ok with that? sounds ok to me. :-) -- balbi
Attachment:
signature.asc
Description: PGP signature
