2024-02-23 21:43:44

by Yang Xiwen via B4 Relay

[permalink] [raw]
Subject: [PATCH v2 0/2] usb: dwc3: add glue driver for Hi3798MV200

Hi3798MV200 uses DWC3 with a few more clocks and a dedicated reset.

Note xhci-histb.c can also be used. But since it's DWC3 in fact, trying
to support it with the help of DWC3 framework seems a better solution.

Hi3798CV200 can also try to migrate to this driver too. Thus we can
remove xhci-histb.c in the future.

Signed-off-by: Yang Xiwen <[email protected]>
---
Changes in v2:
- remove histb-clock.h as it's deprecated.
- fix bot error (Rob Herring)
- add a dummy reg property to make simple-bus parent node happy.
(duplicate with subnode, not used in driver)
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Yang Xiwen (2):
dt-bindings: usb: add hisilicon,hi3798mv200-dwc3
usb: dwc3: of-simple: Add compatible for hi3798mv200 DWC3 controller

.../bindings/usb/hisilicon,hi3798mv200-dwc3.yaml | 103 +++++++++++++++++++++
drivers/usb/dwc3/dwc3-of-simple.c | 1 +
2 files changed, 104 insertions(+)
---
base-commit: 8d3dea210042f54b952b481838c1e7dfc4ec751d
change-id: 20240217-dwc3-697828b480aa

Best regards,
--
Yang Xiwen <[email protected]>



2024-02-23 21:43:45

by Yang Xiwen via B4 Relay

[permalink] [raw]
Subject: [PATCH v2 2/2] usb: dwc3: of-simple: Add compatible for hi3798mv200 DWC3 controller

From: Yang Xiwen <[email protected]>

Hi3798MV200 uses dwc3 controller with a few more clocks and a dedicated
resets. Use of_simple driver for it.

Signed-off-by: Yang Xiwen <[email protected]>
---
drivers/usb/dwc3/dwc3-of-simple.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
index d1539fc9eabd..158acae08af5 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -173,6 +173,7 @@ static const struct of_device_id of_dwc3_simple_match[] = {
{ .compatible = "sprd,sc9860-dwc3" },
{ .compatible = "allwinner,sun50i-h6-dwc3" },
{ .compatible = "hisilicon,hi3670-dwc3" },
+ { .compatible = "hisilicon,hi3798mv200-dwc3" },
{ .compatible = "intel,keembay-dwc3" },
{ /* Sentinel */ }
};

--
2.43.0


2024-02-23 21:43:47

by Yang Xiwen via B4 Relay

[permalink] [raw]
Subject: [PATCH v2 1/2] dt-bindings: usb: add hisilicon,hi3798mv200-dwc3

From: Yang Xiwen <[email protected]>

Document the DWC3 controller used by Hi3798MV200.

Signed-off-by: Yang Xiwen <[email protected]>
---
.../bindings/usb/hisilicon,hi3798mv200-dwc3.yaml | 103 +++++++++++++++++++++
1 file changed, 103 insertions(+)

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 <[email protected]>
+
+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";
+ };
+ };

--
2.43.0


2024-02-24 10:29:10

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: usb: add hisilicon,hi3798mv200-dwc3

On 23/02/2024 22:43, Yang Xiwen via B4 Relay wrote:
> From: Yang Xiwen <[email protected]>
>
> Document the DWC3 controller used by Hi3798MV200.
>
> Signed-off-by: Yang Xiwen <[email protected]>


> +
> +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


2024-02-24 11:34:28

by Yang Xiwen

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: usb: add hisilicon,hi3798mv200-dwc3

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 <[email protected]>
>>
>> Document the DWC3 controller used by Hi3798MV200.
>>
>> Signed-off-by: Yang Xiwen <[email protected]>
>
>> +
>> +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
>

--
Regards,
Yang Xiwen


2024-02-24 11:51:42

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: usb: add hisilicon,hi3798mv200-dwc3

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 <[email protected]>
>>>
>>> Document the DWC3 controller used by Hi3798MV200.
>>>
>>> Signed-off-by: Yang Xiwen <[email protected]>
>>
>>> +
>>> +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