Re: [PATCH 06/11] usb: devicetree: dwc3: Introduce num-lanes and lsm

From: Rob Herring <robh@kernel.org>
To: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	John Youn <John.Youn@synopsys.com>
Subject: Re: [PATCH 06/11] usb: devicetree: dwc3: Introduce num-lanes and lsm
Date: Wed, 22 Jul 2020 08:45:48 -0600	[thread overview]
Message-ID: <CAL_JsqKSrs93wLrxy2gaBEhGfgZs7jpjFarQBoHGxMc6ur3WRQ@mail.gmail.com> (raw)
In-Reply-To: <57fffdfb-a4fa-6e50-1156-1ada3765e362@synopsys.com>

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