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@xxxxxxxxxxx> > > > + > > > +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.
Attachment:
signature.asc
Description: PGP signature
