RE: [PATCH v1 2/2] doc: dt: bindings: usb: realtek,dwc3: Add Realtek DHC RTD SoC DWC3 USB

[Stanley Chang[昌育德]]

Hi Krzysztof,

> Subject: Re: [PATCH v1 2/2] doc: dt: bindings: usb: realtek,dwc3: Add Realtek
> DHC RTD SoC DWC3 USB
> 
> On 28/07/2023 05:53, Stanley Chang wrote:
> > Document the DWC3 USB bindings for Realtek SoCs.
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your
> patch is touching.
> 
> My filtering of emails depends on it.

I will revise the subject.

> > diff --git a/Documentation/devicetree/bindings/usb/realtek,dwc3.yaml
> > b/Documentation/devicetree/bindings/usb/realtek,dwc3.yaml
> 
> realtek,rtd-dwc3.yaml

I will rename this file.

> > +$id: "http://devicetree.org/schemas/usb/realtek,dwc3.yaml#";
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#";
> 
> Drop quotes. Wasn't tested, because dtschema reports warnings here...

Okay, I will drop it.

> > +  realtek,unlink-usb3-port:
> > +    description: Disable link between USB 3.0 PHY and USB mac.
> > +      Only for RTD1619 SoC, if the board design support maximum 2.0
> speed.
> > +    type: boolean
> > +
> > +  realtek,disable-usb3-phy:
> > +    description: Close USB 3.0 PHY if the board design not support USB 3.0.
> > +    type: boolean
> > +
> > +  realtek,enable-l4icg:
> > +    description: Enable the power saving feature l4icg by hardware clock.
> > +      gating.
> 
> You described the desired Linux feature or behavior, not the actual hardware.
> The bindings are about the latter, so instead you need to rephrase the property
> and its description to match actual hardware
> capabilities/features/configuration etc.

The l4icg entry is a hardware feature for power saving. 
The term may not clearly convey the function.
So instead of the term "l4icg" I use "hw clock gating".

  realtek,enable-hw-clock-gating:
    description: Enable the clock gated function which will gating sram
      clock when dbus idle.

> > +examples:
> > +  - |
> > +    usb@98013e00 {
> > +        compatible = "realtek,rtd1319d-dwc3", "realtek,rtd-dwc3";
> > +        reg = <0x98013e00 0x200>;
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +        ranges;
> > +        status = "okay";
> 
> Drop status.
> 
Okay.

Thanks,
Stanley