Hi Thinh, >-----Original Message----- >From: Thinh Nguyen [mailto:Thinh.Nguyen@xxxxxxxxxxxx] >Sent: Friday, May 10, 2019 5:30 AM >To: Anurag Kumar Vulisha <anuragku@xxxxxxxxxx>; Thinh Nguyen ><Thinh.Nguyen@xxxxxxxxxxxx>; Greg Kroah-Hartman ><gregkh@xxxxxxxxxxxxxxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx>; Mark Rutland ><mark.rutland@xxxxxxx>; Felipe Balbi <balbi@xxxxxxxxxx>; Claus H. Stovgaard ><cst@xxxxxxxxxxxx> >Cc: linux-usb@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux- >kernel@xxxxxxxxxxxxxxx; v.anuragkumar@xxxxxxxxx >Subject: Re: [PATCH v2 3/3] usb: dwc3: gadget: Add support for disabling U1 and U2 >entries > >Hi Anurag, > >Anurag Kumar Vulisha wrote: >>>> + return -EINVAL; >>>> >>>> reg = dwc3_readl(dwc->regs, DWC3_DCTL); >>>> if (set) >>>> @@ -626,7 +630,10 @@ static int dwc3_ep0_set_config(struct dwc3 *dwc, >struct >>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>>> index e293400..f2d3112 100644 >>>> --- a/drivers/usb/dwc3/gadget.c >>>> +++ b/drivers/usb/dwc3/gadget.c >>>> @@ -2073,6 +2073,24 @@ static int dwc3_gadget_stop(struct usb_gadget *g) >>>> return 0; >>>> } >>>> >>>> +static void dwc3_gadget_config_params(struct usb_gadget *g, >>>> + struct usb_dcd_config_params *params) >>>> +{ >>>> + struct dwc3 *dwc = gadget_to_dwc(g); >>>> + >>>> + /* U1 Device exit Latency */ >>>> + if (dwc->dis_u1_entry_quirk) >>>> + params->bU1devExitLat = 0; >>> It doesn't make sense to have exit latency of 0. Rejecting >>> SET_FEATURE(enable U1/U2) should already let the host know that the >>> device doesn't support U1/U2. >>> >> I am okay to remove this, but I feel that it is better to report zero value instead >> of a non-zero value in exit latency of BOS when U1 or U2 entries are not supported. >> Advantage of reporting 0 is that some hosts doesn't even send >SET_FEATURE(U1/U2) >> requests on seeing zero value in BOS descriptor. Also there can be cases where U1 is >> disabled and U2 entry is allowed or vice versa, for these kind of cases the driver can >> set zero exit latency value for U1 and non-zero exit latency value for U2 . Based on >this >> I think it would be better to report 0 when U1/U2 states are not enabled. Please >provide >> your opinion on this. > >Hm... I assume you're testing against linux usb stack and xhci host. If >that's the case, it looks like host will still request the device to >enter U1/U2 despite the device rejecting SET_FEATURE(enable U1/U2). This >needs to be fixed. I think what you have is fine to workaround this issue. Thanks . Will send the next series with the other fixes that you have suggested Best Regards, Anurag Kumar Vulisha
