| Message ID | 20240224-dwc3-v2-1-8e4fcd757175@outlook.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: dwc3: add glue driver for Hi3798MV200 | expand |
On 23/02/2024 22:43, Yang Xiwen via B4 Relay wrote: > From: Yang Xiwen <forbidden405@outlook.com> > > Document the DWC3 controller used by Hi3798MV200. > > Signed-off-by: Yang Xiwen <forbidden405@outlook.com> > + > +properties: > + compatible: > + const: hisilicon,hi3798mv200-dwc3 > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 1 > + > + reg: true Constraints. maxItems: X > + > + ranges: true > + > + clocks: > + items: > + - description: Controller bus clock > + - description: Controller suspend clock > + - description: Controller reference clock > + - description: Controller gm clock > + - description: Controller gs clock > + - description: Controller utmi clock > + - description: Controller pipe clock > + > + clock-names: > + items: > + - const: bus > + - const: suspend > + - const: ref > + - const: gm > + - const: gs > + - const: utmi > + - const: pipe > + > + resets: > + maxItems: 1 > + > + reset-names: > + const: soft > + > +patternProperties: > + '^usb@[0-9a-z]+$': unit addresses are in hex, so a-f Open existing bindings and look how it is done there. There are no bindings for DWC3 glue/wrapper device having a-z. > + $ref: snps,dwc3.yaml# > + > +additionalProperties: false Same comments: open existing bindings and take a look how it is there. This goes after 'required:' block. > + > +required: > + - compatible > + - '#address-cells' > + - '#size-cells' > + - ranges > + - reg > + - clocks > + - clock-names > + - resets > + - reset-names > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + usb@98a0000 { > + compatible = "hisilicon,hi3798mv200-dwc3"; reg is always the second property. ranges is third. > + #address-cells = <1>; > + #size-cells = <1>; Use 4 spaces for example indentation. Best regards, Krzysztof
On 2/24/2024 6:28 PM, Krzysztof Kozlowski wrote: > On 23/02/2024 22:43, Yang Xiwen via B4 Relay wrote: >> From: Yang Xiwen <forbidden405@outlook.com> >> >> Document the DWC3 controller used by Hi3798MV200. >> >> Signed-off-by: Yang Xiwen <forbidden405@outlook.com> > >> + >> +properties: >> + compatible: >> + const: hisilicon,hi3798mv200-dwc3 >> + >> + '#address-cells': >> + const: 1 >> + >> + '#size-cells': >> + const: 1 >> + >> + reg: true > Constraints. maxItems: X Is it mandatory to have this property if this node is going to be under a "simple-bus"? I'm taking rk3399-dwc3.yaml as reference. In fact, dwc3 wrapper on mv200 does not have an extra register space. The wrapper only needs to turn on the clocks and deassert the resets. It does not need/have a register space. I don't think it makes sense duplicating the same address twice. But reg property is required by "simple-bus" so i don't know why there is no warning for rk3399-dwc3. > >> + >> + ranges: true >> + >> + clocks: >> + items: >> + - description: Controller bus clock >> + - description: Controller suspend clock >> + - description: Controller reference clock >> + - description: Controller gm clock >> + - description: Controller gs clock >> + - description: Controller utmi clock >> + - description: Controller pipe clock >> + >> + clock-names: >> + items: >> + - const: bus >> + - const: suspend >> + - const: ref >> + - const: gm >> + - const: gs >> + - const: utmi >> + - const: pipe >> + >> + resets: >> + maxItems: 1 >> + >> + reset-names: >> + const: soft >> + >> +patternProperties: >> + '^usb@[0-9a-z]+$': > unit addresses are in hex, so a-f > > Open existing bindings and look how it is done there. There are no > bindings for DWC3 glue/wrapper device having a-z. > > >> + $ref: snps,dwc3.yaml# >> + >> +additionalProperties: false > Same comments: open existing bindings and take a look how it is there. > This goes after 'required:' block. > >> + >> +required: >> + - compatible >> + - '#address-cells' >> + - '#size-cells' >> + - ranges >> + - reg >> + - clocks >> + - clock-names >> + - resets >> + - reset-names >> + >> +examples: >> + - | >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + >> + usb@98a0000 { >> + compatible = "hisilicon,hi3798mv200-dwc3"; > reg is always the second property. ranges is third. > > >> + #address-cells = <1>; >> + #size-cells = <1>; > Use 4 spaces for example indentation. > > > > Best regards, > Krzysztof >
On 24/02/2024 12:33, Yang Xiwen wrote: > On 2/24/2024 6:28 PM, Krzysztof Kozlowski wrote: >> On 23/02/2024 22:43, Yang Xiwen via B4 Relay wrote: >>> From: Yang Xiwen <forbidden405@outlook.com> >>> >>> Document the DWC3 controller used by Hi3798MV200. >>> >>> Signed-off-by: Yang Xiwen <forbidden405@outlook.com> >> >>> + >>> +properties: >>> + compatible: >>> + const: hisilicon,hi3798mv200-dwc3 >>> + >>> + '#address-cells': >>> + const: 1 >>> + >>> + '#size-cells': >>> + const: 1 >>> + >>> + reg: true >> Constraints. maxItems: X > > > Is it mandatory to have this property if this node is going to be under > a "simple-bus"? I'm taking rk3399-dwc3.yaml as reference. In fact, dwc3 > wrapper on mv200 does not have an extra register space. The wrapper only > needs to turn on the clocks and deassert the resets. It does not > need/have a register space. Then why did you add it? No, the property is not mandatory. Write bindings in a way they match hardware... > > > I don't think it makes sense duplicating the same address twice. > > > But reg property is required by "simple-bus" so i don't know why there > is no warning for rk3399-dwc3. I don't think it is. ranges or reg is. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/usb/hisilicon,hi3798mv200-dwc3.yaml b/Documentation/devicetree/bindings/usb/hisilicon,hi3798mv200-dwc3.yaml new file mode 100644 index 000000000000..2a93d659414d --- /dev/null +++ b/Documentation/devicetree/bindings/usb/hisilicon,hi3798mv200-dwc3.yaml @@ -0,0 +1,103 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/usb/hisilicon,hi3798mv200-dwc3.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: HiSilicon Hi3798MV200 DWC3 USB SoC controller + +maintainers: + - Yang Xiwen <forbidden405@foxmail.com> + +properties: + compatible: + const: hisilicon,hi3798mv200-dwc3 + + '#address-cells': + const: 1 + + '#size-cells': + const: 1 + + reg: true + + ranges: true + + clocks: + items: + - description: Controller bus clock + - description: Controller suspend clock + - description: Controller reference clock + - description: Controller gm clock + - description: Controller gs clock + - description: Controller utmi clock + - description: Controller pipe clock + + clock-names: + items: + - const: bus + - const: suspend + - const: ref + - const: gm + - const: gs + - const: utmi + - const: pipe + + resets: + maxItems: 1 + + reset-names: + const: soft + +patternProperties: + '^usb@[0-9a-z]+$': + $ref: snps,dwc3.yaml# + +additionalProperties: false + +required: + - compatible + - '#address-cells' + - '#size-cells' + - ranges + - reg + - clocks + - clock-names + - resets + - reset-names + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + + usb@98a0000 { + compatible = "hisilicon,hi3798mv200-dwc3"; + #address-cells = <1>; + #size-cells = <1>; + reg = <0x98a0000 0x10000>; + ranges; + clocks = <&clk_bus>, + <&clk_suspend>, + <&clk_ref>, + <&clk_gm>, + <&clk_gs>, + <&clk_utmi>, + <&clk_pipe>; + clock-names = "bus", "suspend", "ref", "gm", "gs", "utmi", "pipe"; + resets = <&crg 0xb0 12>; + reset-names = "soft"; + + usb@98a0000 { + compatible = "snps,dwc3"; + reg = <0x98a0000 0x10000>; + interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk_bus>, + <&clk_suspend>, + <&clk_ref>; + clock-names = "bus_early", "suspend", "ref"; + phys = <&usb2_phy1_port2>, <&combphy0 0>; + phy-names = "usb2-phy", "usb3-phy"; + maximum-speed = "super-speed"; + dr_mode = "host"; + }; + };