| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <BYAPR02MB5591B928413B5118142841BDA70F0@BYAPR02MB5591.namprd02.prod.outlook.com>
Date: Mon, 13 May 2019 14:15:07 +0000
From: Anurag Kumar Vulisha <anuragku@...inx.com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Felipe Balbi <balbi@...nel.org>,
"Claus H. Stovgaard" <cst@...seone.com>
CC: "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"v.anuragkumar@...il.com" <v.anuragkumar@...il.com>
Subject: RE: [PATCH v2 3/3] usb: dwc3: gadget: Add support for disabling U1
and U2 entries
Hi Thinh,
>-----Original Message-----
>From: Thinh Nguyen [mailto:Thinh.Nguyen@...opsys.com]
>Sent: Saturday, May 11, 2019 7:18 AM
>To: Anurag Kumar Vulisha <anuragku@...inx.com>; Thinh Nguyen
><Thinh.Nguyen@...opsys.com>; Greg Kroah-Hartman
><gregkh@...uxfoundation.org>; Rob Herring <robh+dt@...nel.org>; Mark Rutland
><mark.rutland@....com>; Felipe Balbi <balbi@...nel.org>; Claus H. Stovgaard
><cst@...seone.com>
>Cc: linux-usb@...r.kernel.org; devicetree@...r.kernel.org; linux-
>kernel@...r.kernel.org; v.anuragkumar@...il.com
>Subject: Re: [PATCH v2 3/3] usb: dwc3: gadget: Add support for disabling U1 and U2
>entries
>
>Hi,
>
>Anurag Kumar Vulisha wrote:
>> Hi Thinh,
>>
>>> -----Original Message-----
>>> From: Thinh Nguyen [mailto:Thinh.Nguyen@...opsys.com]
>>> Sent: Friday, May 10, 2019 5:30 AM
>>> To: Anurag Kumar Vulisha <anuragku@...inx.com>; Thinh Nguyen
>>> <Thinh.Nguyen@...opsys.com>; Greg Kroah-Hartman
>>> <gregkh@...uxfoundation.org>; Rob Herring <robh+dt@...nel.org>; Mark Rutland
>>> <mark.rutland@....com>; Felipe Balbi <balbi@...nel.org>; Claus H. Stovgaard
>>> <cst@...seone.com>
>>> Cc: linux-usb@...r.kernel.org; devicetree@...r.kernel.org; linux-
>>> kernel@...r.kernel.org; v.anuragkumar@...il.com
>>> 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
>>
>
>I want to try something. Can you see if this helps with your performance
>test without setting the U1/U2 exit latency to 0?
>(No need to change what you have in your patch. This is just for testing).
>
With your patch , the link doesn't enter into U1/U2 and I am also getting
better performance
Thanks,
Anurag Kumar Vulisha
>diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>index 2f94568ba385..619351c581cf 100644
>--- a/drivers/usb/core/hub.c
>+++ b/drivers/usb/core/hub.c
>@@ -4057,8 +4057,18 @@ static void usb_enable_link_state(struct usb_hcd
>*hcd, struct usb_device *udev,
> /* Only a configured device will accept the Set Feature
> * U1/U2_ENABLE
> */
>- if (udev->actconfig)
>- usb_set_device_initiated_lpm(udev, state, true);
>+ if (udev->actconfig) {
>+ if (usb_set_device_initiated_lpm(udev, state,
>true)) {
>+ /*
>+ * Don't request U1/U2 entry if the device
>+ * cannot enable U1/U2.
>+ */
>+ usb_set_lpm_timeout(udev, state, 0);
>+
>hcd->driver->disable_usb3_lpm_timeout(hcd, udev,
>+
>state);
>+ return;
>+ }
>+ }
>
> /* As soon as usb_set_lpm_timeout(timeout) returns 0, the
> * hub-initiated LPM is enabled. Thus, LPM is enabled no
>
>
Powered by blists - more mailing lists