Add clock and reset in cdns3 platform
v2 cover Add clock and reset in cdns3 platform
The patchset is add generic clock and reset init codes in Cadence USBSS controller.
This patchset is base on v6.4-rc1.
patch 1 is add clock and reset dts in cdns3 dt-binding doc. patch 2 is cdns3 platform codes changes.
previous commits
v1: https://patchwork.kernel.org/project/linux-usb/cover/[email protected]/
changes:
v2:
patch 1
- add maxItem in clocks and resets.
- add clocks and reset in example.
patch 2
- suspend/resume just disable/enable clocks.
Minda Chen (2):
dt-bindings: cdns,usb3: Add clock and reset
usb: cdns3: cdns3-plat: Add clk and reset init
.../devicetree/bindings/usb/cdns,usb3.yaml | 14 +++++ drivers/usb/cdns3/cdns3-plat.c | 58 +++++++++++++++++++ drivers/usb/cdns3/core.h | 3 + 3 files changed, 75 insertions(+)
base-commit: ac9a78681b921877518763ba0e89202254349d1b
Signature · 1 line
2.17.1
PATCH v2 1/2 dt-bindings: cdns,usb3: Add clock and reset
To support generic clock and reset init in Cadence USBSS controller, add clock and reset dts configuration.
--- .../devicetree/bindings/usb/cdns,usb3.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
Documentation/devicetree/bindings/usb/cdns,usb3.yaml +14 -0
@@ -42,6 +42,18 @@ properties: - const: otg - const: wakeup + clocks: + minItems: 1 + maxItems: 8 + description: + USB controller clocks. + + resets: + minItems: 1 + maxItems: 8 + description: + USB controller generic resets. + dr_mode: enum: [host, otg, peripheral]
@@ -98,5 +110,7 @@ examples: interrupt-names = "host", "peripheral", "otg"; maximum-speed = "super-speed"; dr_mode = "otg"; + clocks = <&clk 1>, <&clk 2>, <&clk 3>; + resets = <&rst 1>, <&rst 2>, <&rst 3>; }; };
Signature · 1 line
2.17.1
Re: [PATCH v2 1/2] dt-bindings: cdns,usb3: Add clock and reset
Quoted 13 lines · Minda Chen
To support generic clock and reset init in Cadence USBSS controller, add clock and reset dts configuration.
Signed-off-by Minda Chen <[email protected]>--- .../devicetree/bindings/usb/cdns,usb3.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+)Documentation/devicetree/bindings/usb/cdns,usb3.yaml +0
-0
Documentation/devicetree/bindings/usb/cdns,usb3.yaml properties: · line 49
Quoted 8 lines
Documentation/devicetree/bindings/usb/cdns,usb3.yaml +5
-0@@ -42,6 +42,18 @@ properties: - const: otg - const: wakeup + clocks: + minItems: 1 + maxItems: 8 + description: + USB controller clocks.
You need to list the items. And why is it variable? Your clock choice in the example is poor, I doubt it is real.
Documentation/devicetree/bindings/usb/cdns,usb3.yaml properties: · line 55
Quoted 6 lines
Documentation/devicetree/bindings/usb/cdns,usb3.yaml +6
-0+ + resets: + minItems: 1 + maxItems: 8 + description: + USB controller generic resets.
Here as well.
You had one clock last time, thus the review was - drop the names. Now you changed it to 8 clocks... I don't understand.
Best regards,
Krzysztof
Roger Quadros
reviewerRe: [PATCH v2 1/2] dt-bindings: cdns,usb3: Add clock and reset
Quoted 33 lines · Krzysztof Kozlowski
Quoted 13 lines · Minda Chen
To support generic clock and reset init in Cadence USBSS controller, add clock and reset dts configuration.
Signed-off-by Minda Chen <[email protected]>--- .../devicetree/bindings/usb/cdns,usb3.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+)Documentation/devicetree/bindings/usb/cdns,usb3.yaml +0
-0Documentation/devicetree/bindings/usb/cdns,usb3.yaml properties: · line 49
Quoted 8 lines
Documentation/devicetree/bindings/usb/cdns,usb3.yaml +5
-0@@ -42,6 +42,18 @@ properties: - const: otg - const: wakeup + clocks: + minItems: 1 + maxItems: 8 + description: + USB controller clocks.You need to list the items. And why is it variable? Your clock choice in the example is poor, I doubt it is real.
Documentation/devicetree/bindings/usb/cdns,usb3.yaml properties: · line 55
Quoted 6 lines
Documentation/devicetree/bindings/usb/cdns,usb3.yaml +6
-0+ + resets: + minItems: 1 + maxItems: 8 + description: + USB controller generic resets.
Quoted 5 lines
Here as well.
You had one clock last time, thus the review was - drop the names. Now you changed it to 8 clocks... I don't understand.
Different platforms may have different number of clocks/resets or none. So I don't think minItems/maxItems should be specified.
Signature · 2 lines
cheers, -roger
Re: [PATCH v2 1/2] dt-bindings: cdns,usb3: Add clock and reset
Quoted 36 lines · Roger Quadros
Quoted 33 lines · Krzysztof Kozlowski
Quoted 13 lines · Minda Chen
To support generic clock and reset init in Cadence USBSS controller, add clock and reset dts configuration.
Signed-off-by Minda Chen <[email protected]>--- .../devicetree/bindings/usb/cdns,usb3.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+)Documentation/devicetree/bindings/usb/cdns,usb3.yaml +0
-0Documentation/devicetree/bindings/usb/cdns,usb3.yaml properties: · line 49
Quoted 8 lines
Documentation/devicetree/bindings/usb/cdns,usb3.yaml +5
-0@@ -42,6 +42,18 @@ properties: - const: otg - const: wakeup + clocks: + minItems: 1 + maxItems: 8 + description: + USB controller clocks.You need to list the items. And why is it variable? Your clock choice in the example is poor, I doubt it is real.
Quoted 6 lines
Documentation/devicetree/bindings/usb/cdns,usb3.yaml +6
-0+ + resets: + minItems: 1 + maxItems: 8 + description: + USB controller generic resets.
Quoted 8 lines
Quoted 5 lines
Here as well.
You had one clock last time, thus the review was - drop the names. Now you changed it to 8 clocks... I don't understand.
Different platforms may have different number of clocks/resets or none. So I don't think minItems/maxItems should be specified.
Yeah, but we want the clocks to be specific per platform. Not anything anywhere.
Best regards,
Krzysztof
Re: [PATCH v2 1/2] dt-bindings: cdns,usb3: Add clock and reset
Quoted 43 lines · Krzysztof Kozlowski
Quoted 42 lines · Roger Quadros
Quoted 38 lines · Krzysztof Kozlowski
Quoted 13 lines · Minda Chen
To support generic clock and reset init in Cadence USBSS controller, add clock and reset dts configuration.
Signed-off-by Minda Chen <[email protected]>--- .../devicetree/bindings/usb/cdns,usb3.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+)Documentation/devicetree/bindings/usb/cdns,usb3.yaml +0
-0Documentation/devicetree/bindings/usb/cdns,usb3.yaml properties: · line 49
Quoted 8 lines
Documentation/devicetree/bindings/usb/cdns,usb3.yaml +5
-0@@ -42,6 +42,18 @@ properties: - const: otg - const: wakeup + clocks: + minItems: 1 + maxItems: 8 + description: + USB controller clocks.You need to list the items. And why is it variable? Your clock choice in the example is poor, I doubt it is real.
Documentation/devicetree/bindings/usb/cdns,usb3.yaml properties: · line 55
Quoted 6 lines
Documentation/devicetree/bindings/usb/cdns,usb3.yaml +6
-0+ + resets: + minItems: 1 + maxItems: 8 + description: + USB controller generic resets.Here as well.
You had one clock last time, thus the review was - drop the names. Now you changed it to 8 clocks... I don't understand.
Quoted 9 lines
Quoted 2 lines
Different platforms may have different number of clocks/resets or none. So I don't think minItems/maxItems should be specified.
Yeah, but we want the clocks to be specific per platform. Not anything anywhere.
Best regards,
Krzysztof
I can change like these. Are these changes can be approved?
lpm , bus clock and "pwrup" reset can be specific cases. (The changes are from snps,dwc3.yaml.)
clocks:
description:
In general the core supports two types of clocks. bus is a SoC Bus Clock(AHB/AXI/APB). lpm is a link power management clock. But particular cases may differ from that having less or more clock sources with another names.
clock-names:
contains:
anyOf:
- enum: [bus, lpm]
- true
resets:
description:
In general the core supports controller power-up reset. Also clock and other resets can be added. Particular cases may differ from that having less or more resets with another names.
reset-names:
contains:
anyOf:
- const: pwrup
- true
Re: [PATCH v2 1/2] dt-bindings: cdns,usb3: Add clock and reset
Quoted 66 lines · Minda Chen
Quoted 43 lines · Krzysztof Kozlowski
Quoted 42 lines · Roger Quadros
Quoted 38 lines · Krzysztof Kozlowski
Quoted 13 lines · Minda Chen
To support generic clock and reset init in Cadence USBSS controller, add clock and reset dts configuration.
Signed-off-by Minda Chen <[email protected]>--- .../devicetree/bindings/usb/cdns,usb3.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+)Documentation/devicetree/bindings/usb/cdns,usb3.yaml +0
-0Documentation/devicetree/bindings/usb/cdns,usb3.yaml properties: · line 49
Quoted 8 lines
Documentation/devicetree/bindings/usb/cdns,usb3.yaml +5
-0@@ -42,6 +42,18 @@ properties: - const: otg - const: wakeup + clocks: + minItems: 1 + maxItems: 8 + description: + USB controller clocks.You need to list the items. And why is it variable? Your clock choice in the example is poor, I doubt it is real.
Documentation/devicetree/bindings/usb/cdns,usb3.yaml properties: · line 55
Quoted 6 lines
Documentation/devicetree/bindings/usb/cdns,usb3.yaml +6
-0+ + resets: + minItems: 1 + maxItems: 8 + description: + USB controller generic resets.Here as well.
You had one clock last time, thus the review was - drop the names. Now you changed it to 8 clocks... I don't understand.
Quoted 9 lines
Quoted 2 lines
Different platforms may have different number of clocks/resets or none. So I don't think minItems/maxItems should be specified.
Yeah, but we want the clocks to be specific per platform. Not anything anywhere.
Best regards,
KrzysztofI can change like these. Are these changes can be approved?
lpm , bus clock and "pwrup" reset can be specific cases. (The changes are from snps,dwc3.yaml.)clocks:
description:
In general the core supports two types of clocks. bus is a SoC Bus Clock(AHB/AXI/APB). lpm is a link power management clock. But particular cases may differ from that having less or more clock sources with another names.
Quoted 6 lines
clock-names:
contains:
anyOf:
- enum: [bus, lpm]
- true
No because this does not solve my concern. You allow here anything, which is not desired. The device bindings should specify what clocks (and resets) you have here. Order is also fixed (with exceptions).
Now, if this is generic IP block used by different SoC vendors and it has different clocks in different implementations, it means one compatible for all of them is not enough anymore.
Best regards,
Krzysztof
Roger Quadros
reviewerRe: [PATCH v2 1/2] dt-bindings: cdns,usb3: Add clock and reset
In reply to Krzysztof KozlowskiQuoted 43 lines · Krzysztof Kozlowski
Quoted 42 lines · Roger Quadros
Quoted 38 lines · Krzysztof Kozlowski
Quoted 13 lines · Minda Chen
To support generic clock and reset init in Cadence USBSS controller, add clock and reset dts configuration.
Signed-off-by Minda Chen <[email protected]>--- .../devicetree/bindings/usb/cdns,usb3.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+)Documentation/devicetree/bindings/usb/cdns,usb3.yaml +0
-0Documentation/devicetree/bindings/usb/cdns,usb3.yaml properties: · line 49
Quoted 8 lines
Documentation/devicetree/bindings/usb/cdns,usb3.yaml +5
-0@@ -42,6 +42,18 @@ properties: - const: otg - const: wakeup + clocks: + minItems: 1 + maxItems: 8 + description: + USB controller clocks.You need to list the items. And why is it variable? Your clock choice in the example is poor, I doubt it is real.
Documentation/devicetree/bindings/usb/cdns,usb3.yaml properties: · line 55
Quoted 6 lines
Documentation/devicetree/bindings/usb/cdns,usb3.yaml +6
-0+ + resets: + minItems: 1 + maxItems: 8 + description: + USB controller generic resets.Here as well.
You had one clock last time, thus the review was - drop the names. Now you changed it to 8 clocks... I don't understand.
Quoted 6 lines
Quoted 2 lines
Different platforms may have different number of clocks/resets or none. So I don't think minItems/maxItems should be specified.
Yeah, but we want the clocks to be specific per platform. Not anything anywhere.
Agreed. So we don't specify min/maxItems at top level but use conditional constraints per platform?
Which means we will need to add platform specific compatibles as well.
Signature · 2 lines
cheers, -roger
Re: [PATCH v2 1/2] dt-bindings: cdns,usb3: Add clock and reset
Quoted 49 lines · Roger Quadros
Quoted 46 lines · Krzysztof Kozlowski
Quoted 44 lines · Roger Quadros
Quoted 33 lines · Krzysztof Kozlowski
Quoted 13 lines · Minda Chen
To support generic clock and reset init in Cadence USBSS controller, add clock and reset dts configuration.
Signed-off-by Minda Chen <[email protected]>--- .../devicetree/bindings/usb/cdns,usb3.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+)Documentation/devicetree/bindings/usb/cdns,usb3.yaml +0
-0Documentation/devicetree/bindings/usb/cdns,usb3.yaml properties: · line 49
Quoted 8 lines
Documentation/devicetree/bindings/usb/cdns,usb3.yaml +5
-0@@ -42,6 +42,18 @@ properties: - const: otg - const: wakeup + clocks: + minItems: 1 + maxItems: 8 + description: + USB controller clocks.You need to list the items. And why is it variable? Your clock choice in the example is poor, I doubt it is real.
Documentation/devicetree/bindings/usb/cdns,usb3.yaml properties: · line 55
Quoted 6 lines
Documentation/devicetree/bindings/usb/cdns,usb3.yaml +6
-0+ + resets: + minItems: 1 + maxItems: 8 + description: + USB controller generic resets.Quoted 5 lines
Here as well.
You had one clock last time, thus the review was - drop the names. Now you changed it to 8 clocks... I don't understand.
Different platforms may have different number of clocks/resets or none. So I don't think minItems/maxItems should be specified.
Quoted 7 lines
Quoted 3 lines
Yeah, but we want the clocks to be specific per platform. Not anything anywhere.
Agreed. So we don't specify min/maxItems at top level but use conditional constraints per platform?
Which means we will need to add platform specific compatibles as well.
Yes, exactly. This can be done here in this binding or through some re-usable common part and then multiple bindings using it and customizing.
Best regards,
Krzysztof
PATCH v2 2/2 usb: cdns3: cdns3-plat: Add clk and reset init
In reply to Minda ChenAdd gereric clk and reset init codes to Cadence USBSS controller. The codes has been tested by starfive vf2 board.
--- drivers/usb/cdns3/cdns3-plat.c | 58 ++++++++++++++++++++++++++++++++++ drivers/usb/cdns3/core.h | 3 ++ 2 files changed, 61 insertions(+)
drivers/usb/cdns3/cdns3-plat.c +58 -0
@@ -12,11 +12,13 @@* Roger Quadros <[email protected]> */ +#include <linux/clk.h> #include <linux/module.h> #include <linux/irq.h> #include <linux/kernel.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> +#include <linux/reset.h> #include "core.h" #include "gadget-export.h"
@@ -43,6 +45,40 @@ static void set_phy_power_off(struct cdns *cdns)phy_power_off (cdns->usb2_phy); } +static int cdns3_clk_rst_init (struct cdns *cdns,bool clk_only) +{ +int ret =0 ; + +if (cdns->num_clks) { + ret =clk_bulk_prepare_enable (cdns->num_clks, cdns->clks); +if (ret) +return ret; + } + +if (clk_only) +return ret; + + ret =reset_control_deassert (cdns->resets); +if (ret) + goto err_clk_init; + +return ret; + +err_clk_init: +if (cdns->num_clks) +clk_bulk_disable_unprepare (cdns->num_clks, cdns->clks); +return ret; +} + +static void cdns3_clk_rst_deinit (struct cdns *cdns,bool clk_only) +{ +if (!clk_only) +reset_control_assert (cdns->resets); + +if (cdns->num_clks) +clk_bulk_disable_unprepare (cdns->num_clks, cdns->clks); +} +/** * cdns3_plat_probe - probe for cdns3 core device * @pdev: Pointer to cdns3 core platform device
@@ -116,6 +152,16 @@ static int cdns3_plat_probe(struct platform_device *pdev) cdns->wakeup_irq =0x0 ; } + ret =devm_clk_bulk_get_all (dev, &cdns->clks); +if (ret <0 ) +return ret; + + cdns->num_clks = ret; + + cdns->resets =devm_reset_control_array_get_optional_exclusive (dev); +if (IS_ERR (cdns->resets)) +return PTR_ERR (cdns->resets); + cdns->usb2_phy =devm_phy_optional_get (dev,"cdns3,usb2-phy" );if (IS_ERR (cdns->usb2_phy))return PTR_ERR (cdns->usb2_phy);
@@ -128,6 +174,10 @@ static int cdns3_plat_probe(struct platform_device *pdev)if (IS_ERR (cdns->usb3_phy))return PTR_ERR (cdns->usb3_phy); + ret =cdns3_clk_rst_init (cdns, false); +if (ret) +return ret; + ret =phy_init (cdns->usb3_phy);if (ret) goto err_phy3_init;
@@ -165,6 +215,7 @@ static int cdns3_plat_probe(struct platform_device *pdev)phy_exit (cdns->usb3_phy); err_phy3_init:phy_exit (cdns->usb2_phy); +cdns3_clk_rst_deinit (cdns, false);return ret; }
@@ -187,6 +238,8 @@ static int cdns3_plat_remove(struct platform_device *pdev)set_phy_power_off (cdns);phy_exit (cdns->usb2_phy);phy_exit (cdns->usb3_phy); +cdns3_clk_rst_deinit (cdns, false); +return 0 ; }
@@ -220,6 +273,8 @@ static int cdns3_controller_suspend(struct device *dev, pm_message_t msg)cdns3_set_platform_suspend (cdns->dev, true, wakeup);set_phy_power_off (cdns); +if (!PMSG_IS_AUTO (msg)) +cdns3_clk_rst_deinit (cdns, true);spin_lock_irqsave (&cdns->lock, flags); cdns->in_lpm = true;spin_unlock_irqrestore (&cdns->lock, flags);
@@ -237,6 +292,9 @@ static int cdns3_controller_resume(struct device *dev, pm_message_t msg)if (!cdns->in_lpm)return 0 ; +if (!PMSG_IS_AUTO (msg)) +cdns3_clk_rst_init (cdns, true); +if (cdns_power_is_lost (cdns)) {phy_exit (cdns->usb2_phy); ret =phy_init (cdns->usb2_phy);
drivers/usb/cdns3/core.h +3 -0
@@ -111,6 +111,9 @@ struct cdns {struct mutex mutex;enum usb_dr_mode dr_mode;struct usb_role_switch *role_sw; +struct reset_control *resets; +struct clk_bulk_data *clks; +int num_clks;bool in_lpm;bool wakeup_pending;struct cdns3_platform_data *pdata;
Signature · 1 line
2.17.1