Thinh Nguyen wrote: > Thinh Nguyen wrote: >> Rob Herring wrote: >>> On Tue, Jul 21, 2020 at 10:42 AM Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> wrote: >>>> Rob Herring wrote: >>>>> On Mon, Jul 20, 2020 at 11:01 PM Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> wrote: >>>>>> Rob Herring wrote: >>>>>>> On Thu, Jul 16, 2020 at 02:59:08PM -0700, Thinh Nguyen wrote: >>>>>>>> Introduce num-lanes and lane-speed-mantissa-gbps for devices operating >>>>>>>> in super-speed-plus. DWC_usb32 IP supports multiple lanes and can >>>>>>>> operate in different sublink speeds. Currently the device controller >>>>>>>> does not have the information of the phy's number of lanes supported. As >>>>>>>> a result, the user can specify them through these properties if they are >>>>>>>> different than the default setting. >>>>>>>> >>>>>>>> Signed-off-by: Thinh Nguyen <thinhn@xxxxxxxxxxxx> >>>>>>>> --- >>>>>>>> Documentation/devicetree/bindings/usb/dwc3.txt | 9 +++++++++ >>>>>>>> 1 file changed, 9 insertions(+) >>>>>>>> >>>>>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt >>>>>>>> index d03edf9d3935..4eba0615562f 100644 >>>>>>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt >>>>>>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt >>>>>>>> @@ -86,6 +86,15 @@ Optional properties: >>>>>>>> - snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of GFLADJ >>>>>>>> register for post-silicon frame length adjustment when the >>>>>>>> fladj_30mhz_sdbnd signal is invalid or incorrect. >>>>>>>> + - snps,num-lanes: set to specify the number of lanes to use. Valid inputs are >>>>>>>> + 1 or 2. Apply if the maximum-speed is super-speed-plus >>>>>>>> + only. Default value is 2 for DWC_usb32. For DWC_usb31, >>>>>>>> + it is always 1 at super-speed-plus. >>>>>>>> + - snps,lane-speed-mantissa-gbps: set to specify the symmetric lane speed >>>>>>>> + mantissa in Gbps. Valid inputs are 5 or 10. Apply if >>>>>>>> + the maximum-speed is super-speed-plus only. Default >>>>>>>> + value is 10. For DWC_usb31, it's always 10 at >>>>>>>> + super-speed-plus. >>>>>>> This is all common USB things and should be common properties (which we >>>>>>> may already have). >>>>>> Sure. For "num-lanes" is simple, any objection if we use >>>>>> "lane-speed-mantissa-gbps"? Or should we add "lane-speed-exponent"? >>>>> 'num-lanes' is good as that's what PCIe uses. Document that with >>>>> 'maximum-speed'. >>>>> >>>>> I think 'super-speed-plus' should mean gen 2 10G per lane. Then >>>>> between num-lanes and maximum-speed you can define all 4 possible >>>>> rates. >>>> That may confuse the user because now we'd use 'super-speed-plus' to >>>> define the speed of the lane rather than the device itself. >>>> >>>> According to the USB 3.2 spec, super-speed-plus can mean gen2x1, gen1x2, >>>> or gen2x2. >>> Then add new strings as needed to make it clear: super-speed-plus-gen1x2 >>> >>> It's obvious that what 'super-speed-plus' means is not clear since >>> USB-IF extended its meaning. >>> >>> Rob >> If we introduce a new enum for gen1x2, now we'd have to go back and >> inspect all the checks for all the drivers where for example speed == >> USB_SPEED_SUPER_PLUS. It seems to be more clunky and may introduce more >> bugs. >> > In my opinion, the better option would be to introduce a new property > for lane speed such as "lane-speed-mantissa-gbps" because: > > 1) It still follows the spec, easier for the user to understand > 2) We only need to update the drivers where the number of lanes and lane > speed matter > 3) Easier speed comparison between usb_device_speed enum > 4) Easier to backport new code where there's speed comparison > 5) Easily extendable to new/different lane speeds > Let me send out v2 of this series so that others can also provide more feedback on other patches. We can continue with this discussion as needed in the meanwhile. BR, Thinh
