| Message ID | 20230728035318.18741-2-stanley_chang@realtek.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | None | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
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. > > Signed-off-by: Stanley Chang <stanley_chang@realtek.com> > --- > .../devicetree/bindings/usb/realtek,dwc3.yaml | 107 ++++++++++++++++++ > 1 file changed, 107 insertions(+) > create mode 100644 Documentation/devicetree/bindings/usb/realtek,dwc3.yaml > > diff --git a/Documentation/devicetree/bindings/usb/realtek,dwc3.yaml b/Documentation/devicetree/bindings/usb/realtek,dwc3.yaml realtek,rtd-dwc3.yaml > new file mode 100644 > index 000000000000..74d388809924 > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/realtek,dwc3.yaml > @@ -0,0 +1,107 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +# Copyright 2023 Realtek Semiconductor Corporation > +%YAML 1.2 > +--- > +$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... > + > +title: Realtek DWC3 USB SoC Controller Glue > + > +maintainers: > + - Stanley Chang <stanley_chang@realtek.com> > + > +description: > + The Realtek DHC SoC embeds a DWC3 USB IP Core configured for USB 2.0 > + and USB 3.0 in host or dual-role mode. > + > +properties: > + compatible: > + items: > + - enum: > + - realtek,rtd1295-dwc3 > + - realtek,rtd1315e-dwc3 > + - realtek,rtd1319-dwc3 > + - realtek,rtd1319d-dwc3 > + - realtek,rtd1395-dwc3 > + - realtek,rtd1619-dwc3 > + - realtek,rtd1619b-dwc3 > + - const: realtek,rtd-dwc3 > + > + reg: > + maxItems: 1 > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 1 > + > + ranges: true > + > + 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. > + type: boolean > + > +patternProperties: > + "^usb@[0-9a-f]+$": > + $ref: snps,dwc3.yaml# > + description: Required child node > + > +required: > + - compatible > + - reg > + - "#address-cells" > + - "#size-cells" > + - ranges > + > +allOf: > + - if: > + not: > + properties: > + compatible: > + contains: > + enum: > + - realtek,rtd1619-dwc3 > + then: > + properties: > + realtek,unlink-usb3-port: false > + > +additionalProperties: false > + > +examples: > + - | > + usb@98013e00 { > + compatible = "realtek,rtd1319d-dwc3", "realtek,rtd-dwc3"; > + reg = <0x98013e00 0x200>; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + status = "okay"; Drop status. > + realtek,disable-usb3-phy; > + realtek,enable-l4icg; > + > + usb@98050000 { > + compatible = "snps,dwc3"; > + reg = <0x98050000 0x9000>; > + interrupts = <0 94 4>; > + phys = <&usb2phy &usb3phy>; > + phy-names = "usb2-phy", "usb3-phy"; > + dr_mode = "otg"; > + usb-role-switch; > + role-switch-default-mode = "host"; > + snps,dis_u2_susphy_quirk; > + snps,parkmode-disable-ss-quirk; > + snps,parkmode-disable-hs-quirk; > + maximum-speed = "high-speed"; > + }; > + }; Best regards, Krzysztof
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
On 28/07/2023 19:37, Stanley Chang[昌育德] wrote: >>> + >>> + 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. And why this should be configurable per board? It looks you like to put OS policy into DT, so no. Best regards, Krzysztof
Hi Krzysztof, > >>> + 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. > > And why this should be configurable per board? It looks you like to put OS > policy into DT, so no. > Ok, I'll remove it from DT. I put it in DT because hw-clock-gating is not an OS policy. This is a pure hardware function and is not controlled by software. I want to control enabling or disabling on different SoCs or boards. But for power saving, it should be enabled by default except for rtd1295 SoC. So I would handle this special case of rtd1295 in the driver. Thanks, Stanley
On Fri, Jul 28, 2023 at 11:53:13AM +0800, Stanley Chang wrote: > Document the DWC3 USB bindings for Realtek SoCs. > > Signed-off-by: Stanley Chang <stanley_chang@realtek.com> > --- > .../devicetree/bindings/usb/realtek,dwc3.yaml | 107 ++++++++++++++++++ > 1 file changed, 107 insertions(+) > create mode 100644 Documentation/devicetree/bindings/usb/realtek,dwc3.yaml > > diff --git a/Documentation/devicetree/bindings/usb/realtek,dwc3.yaml b/Documentation/devicetree/bindings/usb/realtek,dwc3.yaml > new file mode 100644 > index 000000000000..74d388809924 > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/realtek,dwc3.yaml > @@ -0,0 +1,107 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +# Copyright 2023 Realtek Semiconductor Corporation > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/usb/realtek,dwc3.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: Realtek DWC3 USB SoC Controller Glue > + > +maintainers: > + - Stanley Chang <stanley_chang@realtek.com> > + > +description: > + The Realtek DHC SoC embeds a DWC3 USB IP Core configured for USB 2.0 > + and USB 3.0 in host or dual-role mode. > + > +properties: > + compatible: > + items: > + - enum: > + - realtek,rtd1295-dwc3 > + - realtek,rtd1315e-dwc3 > + - realtek,rtd1319-dwc3 > + - realtek,rtd1319d-dwc3 > + - realtek,rtd1395-dwc3 > + - realtek,rtd1619-dwc3 > + - realtek,rtd1619b-dwc3 > + - const: realtek,rtd-dwc3 > + > + reg: > + maxItems: 1 Some details on what these registers are would be useful. Or describing what's in the 'glue' device? > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 1 > + > + ranges: true > + > + 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 We have a property for this: maximum-speed That belongs in the USB controller node though. > + > + realtek,disable-usb3-phy: > + description: Close USB 3.0 PHY if the board design not support USB 3.0. > + type: boolean Can't this be determined by not having a USB3 phy listed? > + > + realtek,enable-l4icg: > + description: Enable the power saving feature l4icg by hardware clock. > + gating. > + type: boolean > + > +patternProperties: > + "^usb@[0-9a-f]+$": > + $ref: snps,dwc3.yaml# > + description: Required child node > + > +required: > + - compatible > + - reg > + - "#address-cells" > + - "#size-cells" > + - ranges > + > +allOf: > + - if: > + not: > + properties: > + compatible: > + contains: > + enum: > + - realtek,rtd1619-dwc3 > + then: > + properties: > + realtek,unlink-usb3-port: false > + > +additionalProperties: false > + > +examples: > + - | > + usb@98013e00 { > + compatible = "realtek,rtd1319d-dwc3", "realtek,rtd-dwc3"; > + reg = <0x98013e00 0x200>; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + status = "okay"; Drop status from examples. > + realtek,disable-usb3-phy; > + realtek,enable-l4icg; > + > + usb@98050000 { > + compatible = "snps,dwc3"; > + reg = <0x98050000 0x9000>; > + interrupts = <0 94 4>; > + phys = <&usb2phy &usb3phy>; > + phy-names = "usb2-phy", "usb3-phy"; > + dr_mode = "otg"; > + usb-role-switch; > + role-switch-default-mode = "host"; > + snps,dis_u2_susphy_quirk; > + snps,parkmode-disable-ss-quirk; > + snps,parkmode-disable-hs-quirk; > + maximum-speed = "high-speed"; > + }; > + }; > -- > 2.34.1 >
Hi Rob, > > +properties: > > + compatible: > > + items: > > + - enum: > > + - realtek,rtd1295-dwc3 > > + - realtek,rtd1315e-dwc3 > > + - realtek,rtd1319-dwc3 > > + - realtek,rtd1319d-dwc3 > > + - realtek,rtd1395-dwc3 > > + - realtek,rtd1619-dwc3 > > + - realtek,rtd1619b-dwc3 > > + - const: realtek,rtd-dwc3 > > + > > + reg: > > + maxItems: 1 > > Some details on what these registers are would be useful. Or describing what's > in the 'glue' device? Ok, I will add the description. > > + 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 > > We have a property for this: maximum-speed > > That belongs in the USB controller node though. I can try to use this property. But I have a question. This property belongs to dwc3. When the dwc3-rtk setting disables USB 3.0 phy, dwc3 is not probed. So I can't use api usb_get_maximum_speed(dev) to get maximum_speed. I can add this property "maximum-speed" to the dwc3-rtk node, which is the same as the dwc3 core node. Is this configuration appropriate? For example usb@98013e00 { compatible = "realtek,rtd1319d-dwc3", "realtek,rtd-dwc3"; reg = <0x98013e00 0x200>; #address-cells = <1>; #size-cells = <1>; ranges; maximum-speed = "high-speed"; usb@98050000 { compatible = "snps,dwc3"; reg = <0x98050000 0x9000>; interrupts = <0 94 4>; phys = <&usb2phy &usb3phy>; phy-names = "usb2-phy", "usb3-phy"; dr_mode = "otg"; usb-role-switch; role-switch-default-mode = "host"; snps,dis_u2_susphy_quirk; snps,parkmode-disable-ss-quirk; snps,parkmode-disable-hs-quirk; maximum-speed = "high-speed"; }; }; > > + > > + realtek,disable-usb3-phy: > > + description: Close USB 3.0 PHY if the board design not support USB 3.0. > > + type: boolean > > Can't this be determined by not having a USB3 phy listed? I will try to use maximum-speed. > > +examples: > > + - | > > + usb@98013e00 { > > + compatible = "realtek,rtd1319d-dwc3", "realtek,rtd-dwc3"; > > + reg = <0x98013e00 0x200>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + ranges; > > + status = "okay"; > > Drop status from examples. Ok. Thanks, Stanley
diff --git a/Documentation/devicetree/bindings/usb/realtek,dwc3.yaml b/Documentation/devicetree/bindings/usb/realtek,dwc3.yaml new file mode 100644 index 000000000000..74d388809924 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/realtek,dwc3.yaml @@ -0,0 +1,107 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright 2023 Realtek Semiconductor Corporation +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/usb/realtek,dwc3.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Realtek DWC3 USB SoC Controller Glue + +maintainers: + - Stanley Chang <stanley_chang@realtek.com> + +description: + The Realtek DHC SoC embeds a DWC3 USB IP Core configured for USB 2.0 + and USB 3.0 in host or dual-role mode. + +properties: + compatible: + items: + - enum: + - realtek,rtd1295-dwc3 + - realtek,rtd1315e-dwc3 + - realtek,rtd1319-dwc3 + - realtek,rtd1319d-dwc3 + - realtek,rtd1395-dwc3 + - realtek,rtd1619-dwc3 + - realtek,rtd1619b-dwc3 + - const: realtek,rtd-dwc3 + + reg: + maxItems: 1 + + '#address-cells': + const: 1 + + '#size-cells': + const: 1 + + ranges: true + + 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. + type: boolean + +patternProperties: + "^usb@[0-9a-f]+$": + $ref: snps,dwc3.yaml# + description: Required child node + +required: + - compatible + - reg + - "#address-cells" + - "#size-cells" + - ranges + +allOf: + - if: + not: + properties: + compatible: + contains: + enum: + - realtek,rtd1619-dwc3 + then: + properties: + realtek,unlink-usb3-port: false + +additionalProperties: false + +examples: + - | + usb@98013e00 { + compatible = "realtek,rtd1319d-dwc3", "realtek,rtd-dwc3"; + reg = <0x98013e00 0x200>; + #address-cells = <1>; + #size-cells = <1>; + ranges; + status = "okay"; + realtek,disable-usb3-phy; + realtek,enable-l4icg; + + usb@98050000 { + compatible = "snps,dwc3"; + reg = <0x98050000 0x9000>; + interrupts = <0 94 4>; + phys = <&usb2phy &usb3phy>; + phy-names = "usb2-phy", "usb3-phy"; + dr_mode = "otg"; + usb-role-switch; + role-switch-default-mode = "host"; + snps,dis_u2_susphy_quirk; + snps,parkmode-disable-ss-quirk; + snps,parkmode-disable-hs-quirk; + maximum-speed = "high-speed"; + }; + };
Document the DWC3 USB bindings for Realtek SoCs. Signed-off-by: Stanley Chang <stanley_chang@realtek.com> --- .../devicetree/bindings/usb/realtek,dwc3.yaml | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/realtek,dwc3.yaml