| Message ID | 9684a2b2adb01b6b1a8c513928ea49b4a6436184.1594935978.git.thinhn@synopsys.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | usb: Handle different sublink speeds | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 15 lines checked |
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@synopsys.com> > --- > 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). Rob
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@synopsys.com> >> --- >> 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"? Thanks, Thinh
On Mon, Jul 20, 2020 at 11:01 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> 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@synopsys.com> > >> --- > >> 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. Rob
Rob Herring wrote: > On Mon, Jul 20, 2020 at 11:01 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> 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@synopsys.com> >>>> --- >>>> 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. BR, Thinh
Hi, Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes: > Rob Herring wrote: >> On Mon, Jul 20, 2020 at 11:01 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> 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@synopsys.com> >>>>> --- >>>>> 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. I agree. In USB land we should refer solely to the USB specification naming schemes.
On Tue, Jul 21, 2020 at 10:42 AM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: > > Rob Herring wrote: > > On Mon, Jul 20, 2020 at 11:01 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> 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@synopsys.com> > >>>> --- > >>>> 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
Rob Herring wrote: > On Tue, Jul 21, 2020 at 10:42 AM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: >> Rob Herring wrote: >>> On Mon, Jul 20, 2020 at 11:01 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> 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@synopsys.com> >>>>>> --- >>>>>> 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. BR, Thinh
Thinh Nguyen wrote: > Rob Herring wrote: >> On Tue, Jul 21, 2020 at 10:42 AM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: >>> Rob Herring wrote: >>>> On Mon, Jul 20, 2020 at 11:01 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> 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@synopsys.com> >>>>>>> --- >>>>>>> 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 BR, Thinh
Thinh Nguyen wrote: > Thinh Nguyen wrote: >> Rob Herring wrote: >>> On Tue, Jul 21, 2020 at 10:42 AM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: >>>> Rob Herring wrote: >>>>> On Mon, Jul 20, 2020 at 11:01 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> 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@synopsys.com> >>>>>>>> --- >>>>>>>> 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
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. - snps,rx-thr-num-pkt-prd: periodic ESS RX packet threshold count - host mode only. Set this and rx-max-burst-prd to a valid, non-zero value 1-16 (DWC_usb31 programming guide
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@synopsys.com> --- Documentation/devicetree/bindings/usb/dwc3.txt | 9 +++++++++ 1 file changed, 9 insertions(+)