On 2020/1/4 8:32, Rob Herring wrote:
> On Fri, Dec 27, 2019 at 02:36:42PM +0800, Hanjie Lin wrote:
>> The Amlogic A1 SoC Family embeds 1 USB Controllers:
>> - a DWC3 IP configured as Host for USB2 and USB3
>>
>> A glue connects the controllers to the USB2 PHY of A1 SoC.
>>
>> Signed-off-by: Hanjie Lin <hanjie.lin@xxxxxxxxxxx>
>> Signed-off-by: Yue Wang <yue.wang@xxxxxxxxxxx>
>> ---
>> .../bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml | 57 +++++++++++++++++++---
>> 1 file changed, 51 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml b/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml
>> index 4efb77b..6103cc2 100644
>> --- a/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml
>> +++ b/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml
>> @@ -9,6 +9,8 @@ title: Amlogic Meson G12A DWC3 USB SoC Controller Glue
>>
>> maintainers:
>> - Neil Armstrong <narmstrong@xxxxxxxxxxxx>
>> + - Hanjie Lin <hanjie.lin@xxxxxxxxxxx>
>> + - Yue Wang <yue.wang@xxxxxxxxxxx>
>>
>> description: |
>> The Amlogic G12A embeds a DWC3 USB IP Core configured for USB2 and USB3
>> @@ -22,10 +24,14 @@ description: |
>> The DWC3 Glue controls the PHY routing and power, an interrupt line is
>> connected to the Glue to serve as OTG ID change detection.
>>
>> + The Amlogic A1 embeds a DWC3 USB IP Core configured for USB2 in
>> + host-only mode.
>> +
>> properties:
>> compatible:
>> enum:
>> - amlogic,meson-g12a-usb-ctrl
>> + - amlogic,meson-a1-usb-ctrl
>>
>> ranges: true
>>
>> @@ -37,6 +43,11 @@ properties:
>>
>> clocks:
>> minItems: 1
>> + maxItems: 4
>> +
>> + clock-names:
>> + minItems: 1
>> + maxItems: 4
>>
>> resets:
>> minItems: 1
>> @@ -47,17 +58,22 @@ properties:
>> interrupts:
>> maxItems: 1
>>
>> + phys:
>> + minItems: 1
>> + maxItems: 3
>> +
>> phy-names:
>> items:
>> - const: usb2-phy0 # USB2 PHY0 if USBHOST_A port is used
>> - const: usb2-phy1 # USB2 PHY1 if USBOTG_B port is used
>> - const: usb3-phy0 # USB3 PHY if USB3_0 is used
>>
>> - phys:
>> - minItems: 1
>> - maxItems: 3
>> -
>
> Why the unnecessary move?
>
I saw most "phys" attributes are front of "phy-names" in dts, maybe looks pretty no other reasons.
>> - dr_mode: true
>> + dr_mode:
>> + description: usb mode for G12A
>> + enum:
>> + - host
>> + - peripheral
>> + - otg
>
> No, this is a common property that doesn't need to be redefined here. It
> was correct as-is.
>
Ok, I will modify it.
>>
>> power-domains:
>> maxItems: 1
>> @@ -80,9 +96,9 @@ required:
>> - resets
>> - reg
>> - interrupts
>> - - phy-names
>> - phys
>> - dr_mode
>> + - phy-names
>
> Again, unnecessary change.
>
Ok
>>
>> examples:
>> - |
>> @@ -124,4 +140,33 @@ examples:
>> snps,quirk-frame-length-adjustment;
>> };
>> };
>> + - |
>> + a1_usb: usb@ffe09000 {
>
> You are only adding a compatible. No need for a whole new example.
>
Ok, I will fix it.
Thanks,
Hanjie
>> + status = "okay";
>> + compatible = "amlogic,meson-a1-usb-ctrl";
>> + reg = <0 0xffe09000 0x0 0xa0>;
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges;
>>
>> + clocks = <&clkc_periphs 36>,
>> + <&clkc_periphs 85>,
>> + <&clkc_periphs 2>,
>> + <&clkc_periphs 3>;
>> + clock-names = "usb_ctrl", "usb_bus", "xtal_usb_phy",
>> + "xtal_usb_ctrl";
>> +
>> + resets = <&reset 36>;
>> +
>> + phys = <&usb2_phy1>;
>> + phy-names = "usb2-phy1";
>> +
>> + a1_dwc3: usb@ff400000 {
>> + compatible = "snps,dwc3";
>> + reg = <0xff400000 0x100000>;
>> + interrupts = <0 90 4>;
>> + dr_mode = "host";
>> + snps,dis_u2_susphy_quirk;
>> + snps,quirk-frame-length-adjustment = <0x20>;
>> + };
>> + };
>> --
>> 2.7.4
>>
>
> .
>
