[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230630-trowel-pleat-5312a869dda2@spud>
Date: Fri, 30 Jun 2023 19:21:49 +0100
From: Conor Dooley <conor@...nel.org>
To: Stanley Chang[昌育德]
<stanley_chang@...ltek.com>
Cc: Rob Herring <robh@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Vinod Koul <vkoul@...nel.org>,
Kishon Vijay Abraham I <kishon@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Alan Stern <stern@...land.harvard.edu>,
Douglas Anderson <dianders@...omium.org>,
Matthias Kaehlcke <mka@...omium.org>,
Bagas Sanjaya <bagasdotme@...il.com>,
Flavio Suligoi <f.suligoi@...m.it>,
Ray Chi <raychi@...gle.com>,
"linux-phy@...ts.infradead.org" <linux-phy@...ts.infradead.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
Subject: Re: [PATCH v6 4/5] dt-bindings: phy: realtek: Add the doc about the
Realtek SoC USB 2.0 PHY
On Fri, Jun 30, 2023 at 07:33:45AM +0000, Stanley Chang[昌育德] wrote:
> Hi Rob,
>
> > On Thu, Jun 29, 2023 at 01:45:12PM +0800, Stanley Chang wrote:
> > > Add the documentation explain the property about Realtek USB PHY driver.
> >
> > In the subject, drop "the doc about the". It's redundant. And perhaps add 'DHC
> > RTD SoC' if this isn't for *all* Realtek SoCs.
> >
> > > Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB
> > > controller. Added the driver to drive the USB 2.0 PHY transceivers.
> >
> > driver? This is a binding for the h/w.
>
> I mean, the driver is drivers/phy/realtek/phy-rtk-usb2.c
> I will revise as
> dt-bindings: phy: realtek: Add the Realtek DHC RTD SoC USB 2.0 PHY
>
> Add the documentation explain the property about Realtek USB PHY driver.
>
> Realtek DHC (digital home center) RTD SoCs support DWC3 XHCI USB
> controller and uses phy-rtk-usb2 as driver for USB 2.0 PHY transceiver..
No, you should mention nothing to do with how a particular operating
system chooses to structure its code here. Bindings describe hardware,
and the commit message should reflect that.
>
> > > +$id: http://devicetree.org/schemas/phy/realtek,usb2phy.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Realtek DHC SoCs USB 2.0 PHY
> > > +
> > > +maintainers:
> > > + - Stanley Chang <stanley_chang@...ltek.com>
> > > +
> > > +description:
> >
> > You need '|' if formatting (line breaks) are important.
>
> I think I need it. I will add it.
>
> > > + realtek,inverse-hstx-sync-clock:
> > > + description:
> > > + For one of the phys of RTD1619b SoC, the synchronous clock of the
> > > + high-speed tx must be inverted.
> >
> > "invert" assumes I know what non-inverted means. I do not. Better to state in
> > terms of active high, low, falling edge, rising edge, etc.
>
> Meaning, the clock must be reversed.
Maybe that means something to Rob, but "reversed" doesn't seem any more
meaningful than inverse. I agree that it should be described in terms of
"active high" etc, as they have well understood meanings.
> > > + type: boolean
> > > +
> > > + realtek,driving-level:
> > > + description:
> > > + Control the magnitude of High speed Dp/Dm output swing.
> > > + For a different board or port, the original magnitude maybe not
> > meet
> > > + the specification. In this situation we can adjust the value to meet
> > > + the specification.
> >
> > What are the units?
>
> There is no unit. It is only a gain for adjusting the magnitude.
Gain has units too.
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + default: 8
> > > + minimum: 0
> > > + maximum: 31
> > > +
> > > + realtek,driving-compensate:
> >
> > compensate what?
>
> It is to compensate the driving level.
Should it be called "driving-level-compensate" then?
> In other word, to adjust the driving level.
So, "realtek,driving-level" sets the gain and
"realtek,driving-compensate" adjusts the driving level.
By that logic, is this also a gain?
Also this property is only for the RTD1315e? That should be
described in/constrained by the binding itself, not in the text
description alone IMO.
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists