RE: [PATCH 3/3] usb: dwc3: gadget: Add support for disabling U1 and U2 entries

From: Anurag Kumar Vulisha
Date: Tue May 07 2019 - 10:10:18 EST


Hi Claus,

>-----Original Message-----
>From: Claus H. Stovgaard [mailto:cst@xxxxxxxxxxxx]
>Sent: Tuesday, May 07, 2019 6:47 PM
>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>
>Cc: linux-usb@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
>kernel@xxxxxxxxxxxxxxx; v.anuragkumar@xxxxxxxxx
>Subject: Re: [PATCH 3/3] usb: dwc3: gadget: Add support for disabling U1 and U2
>entries
>
>Hi Anurag
>
>> > > Please use "-" rather than "_" in the property names.
>> > I have thought about this feature over the weekend, and think the
>> > naming should be
>> > changed to something like "snps,bos-u1-exit-lat-in-us"
>> > and named the same in the code. And then be the value used by the
>> > get_config_params. E.g. the device-tree is used to set the values
>> > directly used for
>> > bUxdevExitLat instead of named something not related to exit
>> > latency.
>> >
>> > With this the name and function is a 1 to 1 match, and you can
>> > among others set it to
>> > 0 for optaining what Anurag wants.
>> >
>> Your suggestion looks good but the problem is the U1 and U2 exit
>> latencies are
>> fixed values in dwc3 controller(can be found in HCSPARAMS3). Adding
>> different
>> exit latencies may modify the U1SEL/U2SEL values sent from the host
>> but the real
>> dwc3 controller exit latencies are not getting changed. Because of
>> this reason I
>> had opted "snps,dis_u1_entry_quirk", so that the U1/U2 exit latency
>> values
>> reported in BOS descriptor can be either be zero (when U1/U2 entries
>> needs to be
>> disabled) or non-zero value (reported in HCSPARAMS3) when U1/U2
>> states allowed.
>> Based on this I think it is better if we can continue with "snps,dis-
>> u1-entry-quirk"
>> instead of the "snps,bos-u1-exit-lat-in-us". PleaseÂÂprovide your
>> opinion on this.
>
>With this in mind I can see why having direct control over the exit
>latency value might not be optimum in many situations.
>Regarding the name, I think the snps,dis_u1_entry_quirk will be a good
>name, if it is combined with the DCTL control. E.g. remove the configfs
>part of my patch, and merge the DCTL control with your patches.
>If the dt-binding still only control the bos descriptor I think a
>better name is something with u1_force_exist_lat_0 or similar.
>
>I don't think setting bos to 0 or controlling DCTL will be used
>individual, so to keep things simple I will vote for
>snps,dis_u1_entry_quirk, and then just control all elements regarding
>disabling U1/U2 from this dt-binding.
>
>Please cut what your need from my patch.
>

Thanks for providing your input. Merging your solution and mine would be
good. I will modify the patch to include your changes as well and send it to you.
Once you are okay with the changes, we can post the patch to mainline.

Thanks,
Anurag Kumar Vulisha