On 2/17/2016 10:39 PM, Felipe Balbi wrote: > John Youn <John.Youn@xxxxxxxxxxxx> writes: >> On 2/17/2016 12:39 AM, Felipe Balbi wrote: >>> >>> Hi, >>> >>> John Youn <John.Youn@xxxxxxxxxxxx> writes: >>>> Enable SuperSpeedPlus by programming the DCFG.speed and after >>>> enumerating, set gadget->speed appropriately. >>>> >>>> Signed-off-by: John Youn <johnyoun@xxxxxxxxxxxx> >>>> --- >>>> drivers/usb/dwc3/gadget.c | 8 ++++++++ >>>> 1 file changed, 8 insertions(+) >>>> >>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>>> index 73db723..5bbdf5d 100644 >>>> --- a/drivers/usb/dwc3/gadget.c >>>> +++ b/drivers/usb/dwc3/gadget.c >>>> @@ -1629,6 +1629,9 @@ static int dwc3_gadget_start(struct usb_gadget *g, >>>> case USB_SPEED_HIGH: >>>> reg |= DWC3_DSTS_HIGHSPEED; >>>> break; >>>> + case USB_SPEED_SUPER_PLUS: >>>> + reg |= DWC3_DSTS_SUPERSPEED_PLUS; >>>> + break; >>>> case USB_SPEED_SUPER: /* FALLTHROUGH */ >>>> case USB_SPEED_UNKNOWN: /* FALTHROUGH */ >>>> default: >>> >>> I'm thinking about amending this change to this patch: >>> >>> 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? Regards, John -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
