* [PATCH] arm64: dts: rockchip: Update DWC3 modules on RK3399 SoCs
@ 2019-06-13 16:27 ` Enric Balletbo i Serra
0 siblings, 0 replies; 13+ messages in thread
From: Enric Balletbo i Serra @ 2019-06-13 16:27 UTC (permalink / raw)
To: devicetree
Cc: Collabora Kernel ML, Felipe Balbi, Masahiro Yamada,
Heiko Stuebner, linux-kernel, linux-rockchip, Rob Herring,
Mark Rutland, linux-arm-kernel, Vicente Bergas, Klaus Goger,
Christoph Muellner, Randy Li, Tony Xie, Viresh Kumar,
Matthias Brugger
As per binding documentation [1], the DWC3 core should have the "ref",
"bus_early" and "suspend" clocks. As explained in the binding, those
clocks are required for new platforms but not for existing platforms
before commit fe8abf332b8f ("usb: dwc3: support clocks and resets for
DWC3 core").
However, as those clocks are really treated as required, this ends with
having some annoying messages when the "rockchip,rk3399-dwc3" is used:
[ 1.724107] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
[ 1.731893] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
[ 2.495937] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
[ 2.647239] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
In order to remove those annoying messages, update the DWC3 hardware
module node and add all the required clocks. With this change, both, the
glue node and the DWC3 core node, have the clocks defined, but that's
not really a problem and there isn't a side effect on do this. So, we
can get rid of the annoying get clk error messages.
[1] Documentation/devicetree/bindings/usb/dwc3.txt
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 196ac9b78076..a15348d185ce 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -414,6 +414,9 @@
compatible = "snps,dwc3";
reg = <0x0 0xfe800000 0x0 0x100000>;
interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru SCLK_USB3OTG0_REF>, <&cru ACLK_USB3OTG0>,
+ <&cru SCLK_USB3OTG0_SUSPEND>;
+ clock-names = "ref", "bus_early", "suspend";
dr_mode = "otg";
phys = <&u2phy0_otg>, <&tcphy0_usb3>;
phy-names = "usb2-phy", "usb3-phy";
@@ -447,6 +450,9 @@
compatible = "snps,dwc3";
reg = <0x0 0xfe900000 0x0 0x100000>;
interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru SCLK_USB3OTG1_REF>, <&cru ACLK_USB3OTG1>,
+ <&cru SCLK_USB3OTG1_SUSPEND>;
+ clock-names = "ref", "bus_early", "suspend";
dr_mode = "otg";
phys = <&u2phy1_otg>, <&tcphy1_usb3>;
phy-names = "usb2-phy", "usb3-phy";
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH] arm64: dts: rockchip: Update DWC3 modules on RK3399 SoCs
@ 2019-06-13 16:27 ` Enric Balletbo i Serra
0 siblings, 0 replies; 13+ messages in thread
From: Enric Balletbo i Serra @ 2019-06-13 16:27 UTC (permalink / raw)
To: devicetree
Cc: Mark Rutland, Heiko Stuebner, Felipe Balbi, linux-rockchip,
Tony Xie, Viresh Kumar, Randy Li, linux-kernel, Vicente Bergas,
Masahiro Yamada, Rob Herring, Klaus Goger, Matthias Brugger,
Collabora Kernel ML, linux-arm-kernel, Christoph Muellner
As per binding documentation [1], the DWC3 core should have the "ref",
"bus_early" and "suspend" clocks. As explained in the binding, those
clocks are required for new platforms but not for existing platforms
before commit fe8abf332b8f ("usb: dwc3: support clocks and resets for
DWC3 core").
However, as those clocks are really treated as required, this ends with
having some annoying messages when the "rockchip,rk3399-dwc3" is used:
[ 1.724107] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
[ 1.731893] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
[ 2.495937] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
[ 2.647239] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
In order to remove those annoying messages, update the DWC3 hardware
module node and add all the required clocks. With this change, both, the
glue node and the DWC3 core node, have the clocks defined, but that's
not really a problem and there isn't a side effect on do this. So, we
can get rid of the annoying get clk error messages.
[1] Documentation/devicetree/bindings/usb/dwc3.txt
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 196ac9b78076..a15348d185ce 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -414,6 +414,9 @@
compatible = "snps,dwc3";
reg = <0x0 0xfe800000 0x0 0x100000>;
interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru SCLK_USB3OTG0_REF>, <&cru ACLK_USB3OTG0>,
+ <&cru SCLK_USB3OTG0_SUSPEND>;
+ clock-names = "ref", "bus_early", "suspend";
dr_mode = "otg";
phys = <&u2phy0_otg>, <&tcphy0_usb3>;
phy-names = "usb2-phy", "usb3-phy";
@@ -447,6 +450,9 @@
compatible = "snps,dwc3";
reg = <0x0 0xfe900000 0x0 0x100000>;
interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru SCLK_USB3OTG1_REF>, <&cru ACLK_USB3OTG1>,
+ <&cru SCLK_USB3OTG1_SUSPEND>;
+ clock-names = "ref", "bus_early", "suspend";
dr_mode = "otg";
phys = <&u2phy1_otg>, <&tcphy1_usb3>;
phy-names = "usb2-phy", "usb3-phy";
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: Update DWC3 modules on RK3399 SoCs
2019-06-13 16:27 ` Enric Balletbo i Serra
(?)
@ 2019-06-13 16:56 ` Robin Murphy
-1 siblings, 0 replies; 13+ messages in thread
From: Robin Murphy @ 2019-06-13 16:56 UTC (permalink / raw)
To: Enric Balletbo i Serra, devicetree
Cc: Mark Rutland, Heiko Stuebner, Felipe Balbi, linux-rockchip,
Tony Xie, Viresh Kumar, Randy Li, linux-kernel, Vicente Bergas,
Masahiro Yamada, Rob Herring, Klaus Goger, Matthias Brugger,
Collabora Kernel ML, linux-arm-kernel, Christoph Muellner
On 13/06/2019 17:27, Enric Balletbo i Serra wrote:
> As per binding documentation [1], the DWC3 core should have the "ref",
> "bus_early" and "suspend" clocks. As explained in the binding, those
> clocks are required for new platforms but not for existing platforms
> before commit fe8abf332b8f ("usb: dwc3: support clocks and resets for
> DWC3 core").
>
> However, as those clocks are really treated as required, this ends with
> having some annoying messages when the "rockchip,rk3399-dwc3" is used:
>
> [ 1.724107] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
> [ 1.731893] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
> [ 2.495937] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
> [ 2.647239] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
>
> In order to remove those annoying messages, update the DWC3 hardware
> module node and add all the required clocks. With this change, both, the
> glue node and the DWC3 core node, have the clocks defined, but that's
> not really a problem and there isn't a side effect on do this. So, we
> can get rid of the annoying get clk error messages.
Can we not just move these clocks entirely from the glue layer to the
core layer? That didn't seem to break when I tried it, although I'll
admit my 'testing' was no more than booting and mounting a USB 3.0 flash
drive, no suspend or anything fancy.
My own attempt to shut up these errors got sidetracked into c0c61471ef86
("usb: dwc3: of-simple: Convert to bulk clk API"), then apparently
stalled :)
Robin.
>
> [1] Documentation/devicetree/bindings/usb/dwc3.txt
>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>
> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 196ac9b78076..a15348d185ce 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -414,6 +414,9 @@
> compatible = "snps,dwc3";
> reg = <0x0 0xfe800000 0x0 0x100000>;
> interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru SCLK_USB3OTG0_REF>, <&cru ACLK_USB3OTG0>,
> + <&cru SCLK_USB3OTG0_SUSPEND>;
> + clock-names = "ref", "bus_early", "suspend";
> dr_mode = "otg";
> phys = <&u2phy0_otg>, <&tcphy0_usb3>;
> phy-names = "usb2-phy", "usb3-phy";
> @@ -447,6 +450,9 @@
> compatible = "snps,dwc3";
> reg = <0x0 0xfe900000 0x0 0x100000>;
> interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru SCLK_USB3OTG1_REF>, <&cru ACLK_USB3OTG1>,
> + <&cru SCLK_USB3OTG1_SUSPEND>;
> + clock-names = "ref", "bus_early", "suspend";
> dr_mode = "otg";
> phys = <&u2phy1_otg>, <&tcphy1_usb3>;
> phy-names = "usb2-phy", "usb3-phy";
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: Update DWC3 modules on RK3399 SoCs
@ 2019-06-13 16:56 ` Robin Murphy
0 siblings, 0 replies; 13+ messages in thread
From: Robin Murphy @ 2019-06-13 16:56 UTC (permalink / raw)
To: Enric Balletbo i Serra, devicetree
Cc: Mark Rutland, Heiko Stuebner, Felipe Balbi, Masahiro Yamada,
Christoph Muellner, Viresh Kumar, Randy Li, linux-kernel,
Vicente Bergas, linux-rockchip, Tony Xie, Klaus Goger,
Rob Herring, Collabora Kernel ML, linux-arm-kernel,
Matthias Brugger
On 13/06/2019 17:27, Enric Balletbo i Serra wrote:
> As per binding documentation [1], the DWC3 core should have the "ref",
> "bus_early" and "suspend" clocks. As explained in the binding, those
> clocks are required for new platforms but not for existing platforms
> before commit fe8abf332b8f ("usb: dwc3: support clocks and resets for
> DWC3 core").
>
> However, as those clocks are really treated as required, this ends with
> having some annoying messages when the "rockchip,rk3399-dwc3" is used:
>
> [ 1.724107] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
> [ 1.731893] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
> [ 2.495937] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
> [ 2.647239] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
>
> In order to remove those annoying messages, update the DWC3 hardware
> module node and add all the required clocks. With this change, both, the
> glue node and the DWC3 core node, have the clocks defined, but that's
> not really a problem and there isn't a side effect on do this. So, we
> can get rid of the annoying get clk error messages.
Can we not just move these clocks entirely from the glue layer to the
core layer? That didn't seem to break when I tried it, although I'll
admit my 'testing' was no more than booting and mounting a USB 3.0 flash
drive, no suspend or anything fancy.
My own attempt to shut up these errors got sidetracked into c0c61471ef86
("usb: dwc3: of-simple: Convert to bulk clk API"), then apparently
stalled :)
Robin.
>
> [1] Documentation/devicetree/bindings/usb/dwc3.txt
>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>
> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 196ac9b78076..a15348d185ce 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -414,6 +414,9 @@
> compatible = "snps,dwc3";
> reg = <0x0 0xfe800000 0x0 0x100000>;
> interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru SCLK_USB3OTG0_REF>, <&cru ACLK_USB3OTG0>,
> + <&cru SCLK_USB3OTG0_SUSPEND>;
> + clock-names = "ref", "bus_early", "suspend";
> dr_mode = "otg";
> phys = <&u2phy0_otg>, <&tcphy0_usb3>;
> phy-names = "usb2-phy", "usb3-phy";
> @@ -447,6 +450,9 @@
> compatible = "snps,dwc3";
> reg = <0x0 0xfe900000 0x0 0x100000>;
> interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru SCLK_USB3OTG1_REF>, <&cru ACLK_USB3OTG1>,
> + <&cru SCLK_USB3OTG1_SUSPEND>;
> + clock-names = "ref", "bus_early", "suspend";
> dr_mode = "otg";
> phys = <&u2phy1_otg>, <&tcphy1_usb3>;
> phy-names = "usb2-phy", "usb3-phy";
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: Update DWC3 modules on RK3399 SoCs
@ 2019-06-13 16:56 ` Robin Murphy
0 siblings, 0 replies; 13+ messages in thread
From: Robin Murphy @ 2019-06-13 16:56 UTC (permalink / raw)
To: Enric Balletbo i Serra, devicetree
Cc: Mark Rutland, Heiko Stuebner, Felipe Balbi, Masahiro Yamada,
Christoph Muellner, Viresh Kumar, Randy Li, linux-kernel,
Vicente Bergas, linux-rockchip, Tony Xie, Klaus Goger,
Rob Herring, Collabora Kernel ML, linux-arm-kernel,
Matthias Brugger
On 13/06/2019 17:27, Enric Balletbo i Serra wrote:
> As per binding documentation [1], the DWC3 core should have the "ref",
> "bus_early" and "suspend" clocks. As explained in the binding, those
> clocks are required for new platforms but not for existing platforms
> before commit fe8abf332b8f ("usb: dwc3: support clocks and resets for
> DWC3 core").
>
> However, as those clocks are really treated as required, this ends with
> having some annoying messages when the "rockchip,rk3399-dwc3" is used:
>
> [ 1.724107] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
> [ 1.731893] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
> [ 2.495937] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
> [ 2.647239] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
>
> In order to remove those annoying messages, update the DWC3 hardware
> module node and add all the required clocks. With this change, both, the
> glue node and the DWC3 core node, have the clocks defined, but that's
> not really a problem and there isn't a side effect on do this. So, we
> can get rid of the annoying get clk error messages.
Can we not just move these clocks entirely from the glue layer to the
core layer? That didn't seem to break when I tried it, although I'll
admit my 'testing' was no more than booting and mounting a USB 3.0 flash
drive, no suspend or anything fancy.
My own attempt to shut up these errors got sidetracked into c0c61471ef86
("usb: dwc3: of-simple: Convert to bulk clk API"), then apparently
stalled :)
Robin.
>
> [1] Documentation/devicetree/bindings/usb/dwc3.txt
>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>
> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 196ac9b78076..a15348d185ce 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -414,6 +414,9 @@
> compatible = "snps,dwc3";
> reg = <0x0 0xfe800000 0x0 0x100000>;
> interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru SCLK_USB3OTG0_REF>, <&cru ACLK_USB3OTG0>,
> + <&cru SCLK_USB3OTG0_SUSPEND>;
> + clock-names = "ref", "bus_early", "suspend";
> dr_mode = "otg";
> phys = <&u2phy0_otg>, <&tcphy0_usb3>;
> phy-names = "usb2-phy", "usb3-phy";
> @@ -447,6 +450,9 @@
> compatible = "snps,dwc3";
> reg = <0x0 0xfe900000 0x0 0x100000>;
> interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&cru SCLK_USB3OTG1_REF>, <&cru ACLK_USB3OTG1>,
> + <&cru SCLK_USB3OTG1_SUSPEND>;
> + clock-names = "ref", "bus_early", "suspend";
> dr_mode = "otg";
> phys = <&u2phy1_otg>, <&tcphy1_usb3>;
> phy-names = "usb2-phy", "usb3-phy";
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: Update DWC3 modules on RK3399 SoCs
2019-06-13 16:56 ` Robin Murphy
@ 2019-06-13 17:20 ` Enric Balletbo i Serra
-1 siblings, 0 replies; 13+ messages in thread
From: Enric Balletbo i Serra @ 2019-06-13 17:20 UTC (permalink / raw)
To: Robin Murphy, devicetree
Cc: Mark Rutland, Heiko Stuebner, Felipe Balbi, linux-rockchip,
Tony Xie, Viresh Kumar, Randy Li, linux-kernel, Vicente Bergas,
Masahiro Yamada, Rob Herring, Klaus Goger, Matthias Brugger,
Collabora Kernel ML, linux-arm-kernel, Christoph Muellner
Hi Robin,
On 13/6/19 18:56, Robin Murphy wrote:
> On 13/06/2019 17:27, Enric Balletbo i Serra wrote:
>> As per binding documentation [1], the DWC3 core should have the "ref",
>> "bus_early" and "suspend" clocks. As explained in the binding, those
>> clocks are required for new platforms but not for existing platforms
>> before commit fe8abf332b8f ("usb: dwc3: support clocks and resets for
>> DWC3 core").
>>
>> However, as those clocks are really treated as required, this ends with
>> having some annoying messages when the "rockchip,rk3399-dwc3" is used:
>>
>> [ 1.724107] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
>> [ 1.731893] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
>> [ 2.495937] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
>> [ 2.647239] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
>>
>> In order to remove those annoying messages, update the DWC3 hardware
>> module node and add all the required clocks. With this change, both, the
>> glue node and the DWC3 core node, have the clocks defined, but that's
>> not really a problem and there isn't a side effect on do this. So, we
>> can get rid of the annoying get clk error messages.
>
> Can we not just move these clocks entirely from the glue layer to the core
> layer? That didn't seem to break when I tried it, although I'll admit my
> 'testing' was no more than booting and mounting a USB 3.0 flash drive, no
> suspend or anything fancy.
>
AFAICT usb doesn't break, but we won't break backward compability then? (/me
still doesn't know when backward compability is really important or not)
> My own attempt to shut up these errors got sidetracked into c0c61471ef86 ("usb:
> dwc3: of-simple: Convert to bulk clk API"), then apparently stalled :)
>
There was any off the record discussion and stalled or simply you didn't get
feedback?
I'll take a look.
Thanks,
~ Enric
> Robin.
>
>>
>> [1] Documentation/devicetree/bindings/usb/dwc3.txt
>>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>
>> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> index 196ac9b78076..a15348d185ce 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> @@ -414,6 +414,9 @@
>> compatible = "snps,dwc3";
>> reg = <0x0 0xfe800000 0x0 0x100000>;
>> interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
>> + clocks = <&cru SCLK_USB3OTG0_REF>, <&cru ACLK_USB3OTG0>,
>> + <&cru SCLK_USB3OTG0_SUSPEND>;
>> + clock-names = "ref", "bus_early", "suspend";
>> dr_mode = "otg";
>> phys = <&u2phy0_otg>, <&tcphy0_usb3>;
>> phy-names = "usb2-phy", "usb3-phy";
>> @@ -447,6 +450,9 @@
>> compatible = "snps,dwc3";
>> reg = <0x0 0xfe900000 0x0 0x100000>;
>> interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH 0>;
>> + clocks = <&cru SCLK_USB3OTG1_REF>, <&cru ACLK_USB3OTG1>,
>> + <&cru SCLK_USB3OTG1_SUSPEND>;
>> + clock-names = "ref", "bus_early", "suspend";
>> dr_mode = "otg";
>> phys = <&u2phy1_otg>, <&tcphy1_usb3>;
>> phy-names = "usb2-phy", "usb3-phy";
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: Update DWC3 modules on RK3399 SoCs
@ 2019-06-13 17:20 ` Enric Balletbo i Serra
0 siblings, 0 replies; 13+ messages in thread
From: Enric Balletbo i Serra @ 2019-06-13 17:20 UTC (permalink / raw)
To: Robin Murphy, devicetree
Cc: Mark Rutland, Heiko Stuebner, Felipe Balbi, Masahiro Yamada,
Christoph Muellner, Viresh Kumar, Randy Li, linux-kernel,
Vicente Bergas, linux-rockchip, Tony Xie, Klaus Goger,
Rob Herring, Collabora Kernel ML, linux-arm-kernel,
Matthias Brugger
Hi Robin,
On 13/6/19 18:56, Robin Murphy wrote:
> On 13/06/2019 17:27, Enric Balletbo i Serra wrote:
>> As per binding documentation [1], the DWC3 core should have the "ref",
>> "bus_early" and "suspend" clocks. As explained in the binding, those
>> clocks are required for new platforms but not for existing platforms
>> before commit fe8abf332b8f ("usb: dwc3: support clocks and resets for
>> DWC3 core").
>>
>> However, as those clocks are really treated as required, this ends with
>> having some annoying messages when the "rockchip,rk3399-dwc3" is used:
>>
>> [ 1.724107] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
>> [ 1.731893] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
>> [ 2.495937] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
>> [ 2.647239] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
>>
>> In order to remove those annoying messages, update the DWC3 hardware
>> module node and add all the required clocks. With this change, both, the
>> glue node and the DWC3 core node, have the clocks defined, but that's
>> not really a problem and there isn't a side effect on do this. So, we
>> can get rid of the annoying get clk error messages.
>
> Can we not just move these clocks entirely from the glue layer to the core
> layer? That didn't seem to break when I tried it, although I'll admit my
> 'testing' was no more than booting and mounting a USB 3.0 flash drive, no
> suspend or anything fancy.
>
AFAICT usb doesn't break, but we won't break backward compability then? (/me
still doesn't know when backward compability is really important or not)
> My own attempt to shut up these errors got sidetracked into c0c61471ef86 ("usb:
> dwc3: of-simple: Convert to bulk clk API"), then apparently stalled :)
>
There was any off the record discussion and stalled or simply you didn't get
feedback?
I'll take a look.
Thanks,
~ Enric
> Robin.
>
>>
>> [1] Documentation/devicetree/bindings/usb/dwc3.txt
>>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>
>> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> index 196ac9b78076..a15348d185ce 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> @@ -414,6 +414,9 @@
>> compatible = "snps,dwc3";
>> reg = <0x0 0xfe800000 0x0 0x100000>;
>> interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
>> + clocks = <&cru SCLK_USB3OTG0_REF>, <&cru ACLK_USB3OTG0>,
>> + <&cru SCLK_USB3OTG0_SUSPEND>;
>> + clock-names = "ref", "bus_early", "suspend";
>> dr_mode = "otg";
>> phys = <&u2phy0_otg>, <&tcphy0_usb3>;
>> phy-names = "usb2-phy", "usb3-phy";
>> @@ -447,6 +450,9 @@
>> compatible = "snps,dwc3";
>> reg = <0x0 0xfe900000 0x0 0x100000>;
>> interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH 0>;
>> + clocks = <&cru SCLK_USB3OTG1_REF>, <&cru ACLK_USB3OTG1>,
>> + <&cru SCLK_USB3OTG1_SUSPEND>;
>> + clock-names = "ref", "bus_early", "suspend";
>> dr_mode = "otg";
>> phys = <&u2phy1_otg>, <&tcphy1_usb3>;
>> phy-names = "usb2-phy", "usb3-phy";
>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: Update DWC3 modules on RK3399 SoCs
2019-06-13 17:20 ` Enric Balletbo i Serra
@ 2019-06-13 17:44 ` Robin Murphy
-1 siblings, 0 replies; 13+ messages in thread
From: Robin Murphy @ 2019-06-13 17:44 UTC (permalink / raw)
To: Enric Balletbo i Serra, devicetree
Cc: Mark Rutland, Heiko Stuebner, Felipe Balbi, Masahiro Yamada,
Christoph Muellner, Viresh Kumar, Randy Li, linux-kernel,
Vicente Bergas, linux-rockchip, Tony Xie, Klaus Goger,
Rob Herring, Collabora Kernel ML, linux-arm-kernel,
Matthias Brugger
On 13/06/2019 18:20, Enric Balletbo i Serra wrote:
> Hi Robin,
>
> On 13/6/19 18:56, Robin Murphy wrote:
>> On 13/06/2019 17:27, Enric Balletbo i Serra wrote:
>>> As per binding documentation [1], the DWC3 core should have the "ref",
>>> "bus_early" and "suspend" clocks. As explained in the binding, those
>>> clocks are required for new platforms but not for existing platforms
>>> before commit fe8abf332b8f ("usb: dwc3: support clocks and resets for
>>> DWC3 core").
>>>
>>> However, as those clocks are really treated as required, this ends with
>>> having some annoying messages when the "rockchip,rk3399-dwc3" is used:
>>>
>>> [ 1.724107] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
>>> [ 1.731893] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
>>> [ 2.495937] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
>>> [ 2.647239] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
>>>
>>> In order to remove those annoying messages, update the DWC3 hardware
>>> module node and add all the required clocks. With this change, both, the
>>> glue node and the DWC3 core node, have the clocks defined, but that's
>>> not really a problem and there isn't a side effect on do this. So, we
>>> can get rid of the annoying get clk error messages.
>>
>> Can we not just move these clocks entirely from the glue layer to the core
>> layer? That didn't seem to break when I tried it, although I'll admit my
>> 'testing' was no more than booting and mounting a USB 3.0 flash drive, no
>> suspend or anything fancy.
>>
>
> AFAICT usb doesn't break, but we won't break backward compability then? (/me
> still doesn't know when backward compability is really important or not)
Ah, fair point - I was imagining the glue layer's heavy-handed "turn
everything on" approach potentially interfering with the core layer's
ability to exert finer-grained control of the clocks it knows more
about, but the "old kernel, new DT" situation might indeed be a problem.
I guess that's Heiko's call in the end. Plus I'm no CCF expert so my
concern might be unfounded anyway.
>> My own attempt to shut up these errors got sidetracked into c0c61471ef86 ("usb:
>> dwc3: of-simple: Convert to bulk clk API"), then apparently stalled :)
>>
>
> There was any off the record discussion and stalled or simply you didn't get
> feedback?
More that the 3399 board got put away in a freak "tidying the kitchen"
incident, and I've just had too many other things on the go since :)
Robin.
>
> I'll take a look.
>
> Thanks,
> ~ Enric
>
>> Robin.
>>
>>>
>>> [1] Documentation/devicetree/bindings/usb/dwc3.txt
>>>
>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>> ---
>>>
>>> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> index 196ac9b78076..a15348d185ce 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> @@ -414,6 +414,9 @@
>>> compatible = "snps,dwc3";
>>> reg = <0x0 0xfe800000 0x0 0x100000>;
>>> interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
>>> + clocks = <&cru SCLK_USB3OTG0_REF>, <&cru ACLK_USB3OTG0>,
>>> + <&cru SCLK_USB3OTG0_SUSPEND>;
>>> + clock-names = "ref", "bus_early", "suspend";
>>> dr_mode = "otg";
>>> phys = <&u2phy0_otg>, <&tcphy0_usb3>;
>>> phy-names = "usb2-phy", "usb3-phy";
>>> @@ -447,6 +450,9 @@
>>> compatible = "snps,dwc3";
>>> reg = <0x0 0xfe900000 0x0 0x100000>;
>>> interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH 0>;
>>> + clocks = <&cru SCLK_USB3OTG1_REF>, <&cru ACLK_USB3OTG1>,
>>> + <&cru SCLK_USB3OTG1_SUSPEND>;
>>> + clock-names = "ref", "bus_early", "suspend";
>>> dr_mode = "otg";
>>> phys = <&u2phy1_otg>, <&tcphy1_usb3>;
>>> phy-names = "usb2-phy", "usb3-phy";
>>>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: Update DWC3 modules on RK3399 SoCs
@ 2019-06-13 17:44 ` Robin Murphy
0 siblings, 0 replies; 13+ messages in thread
From: Robin Murphy @ 2019-06-13 17:44 UTC (permalink / raw)
To: Enric Balletbo i Serra, devicetree
Cc: Mark Rutland, Heiko Stuebner, Felipe Balbi, linux-rockchip,
Viresh Kumar, Randy Li, linux-kernel, Vicente Bergas,
Masahiro Yamada, Tony Xie, Klaus Goger, Matthias Brugger,
Rob Herring, Collabora Kernel ML, linux-arm-kernel,
Christoph Muellner
On 13/06/2019 18:20, Enric Balletbo i Serra wrote:
> Hi Robin,
>
> On 13/6/19 18:56, Robin Murphy wrote:
>> On 13/06/2019 17:27, Enric Balletbo i Serra wrote:
>>> As per binding documentation [1], the DWC3 core should have the "ref",
>>> "bus_early" and "suspend" clocks. As explained in the binding, those
>>> clocks are required for new platforms but not for existing platforms
>>> before commit fe8abf332b8f ("usb: dwc3: support clocks and resets for
>>> DWC3 core").
>>>
>>> However, as those clocks are really treated as required, this ends with
>>> having some annoying messages when the "rockchip,rk3399-dwc3" is used:
>>>
>>> [ 1.724107] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
>>> [ 1.731893] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
>>> [ 2.495937] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
>>> [ 2.647239] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
>>>
>>> In order to remove those annoying messages, update the DWC3 hardware
>>> module node and add all the required clocks. With this change, both, the
>>> glue node and the DWC3 core node, have the clocks defined, but that's
>>> not really a problem and there isn't a side effect on do this. So, we
>>> can get rid of the annoying get clk error messages.
>>
>> Can we not just move these clocks entirely from the glue layer to the core
>> layer? That didn't seem to break when I tried it, although I'll admit my
>> 'testing' was no more than booting and mounting a USB 3.0 flash drive, no
>> suspend or anything fancy.
>>
>
> AFAICT usb doesn't break, but we won't break backward compability then? (/me
> still doesn't know when backward compability is really important or not)
Ah, fair point - I was imagining the glue layer's heavy-handed "turn
everything on" approach potentially interfering with the core layer's
ability to exert finer-grained control of the clocks it knows more
about, but the "old kernel, new DT" situation might indeed be a problem.
I guess that's Heiko's call in the end. Plus I'm no CCF expert so my
concern might be unfounded anyway.
>> My own attempt to shut up these errors got sidetracked into c0c61471ef86 ("usb:
>> dwc3: of-simple: Convert to bulk clk API"), then apparently stalled :)
>>
>
> There was any off the record discussion and stalled or simply you didn't get
> feedback?
More that the 3399 board got put away in a freak "tidying the kitchen"
incident, and I've just had too many other things on the go since :)
Robin.
>
> I'll take a look.
>
> Thanks,
> ~ Enric
>
>> Robin.
>>
>>>
>>> [1] Documentation/devicetree/bindings/usb/dwc3.txt
>>>
>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>> ---
>>>
>>> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> index 196ac9b78076..a15348d185ce 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>>> @@ -414,6 +414,9 @@
>>> compatible = "snps,dwc3";
>>> reg = <0x0 0xfe800000 0x0 0x100000>;
>>> interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
>>> + clocks = <&cru SCLK_USB3OTG0_REF>, <&cru ACLK_USB3OTG0>,
>>> + <&cru SCLK_USB3OTG0_SUSPEND>;
>>> + clock-names = "ref", "bus_early", "suspend";
>>> dr_mode = "otg";
>>> phys = <&u2phy0_otg>, <&tcphy0_usb3>;
>>> phy-names = "usb2-phy", "usb3-phy";
>>> @@ -447,6 +450,9 @@
>>> compatible = "snps,dwc3";
>>> reg = <0x0 0xfe900000 0x0 0x100000>;
>>> interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH 0>;
>>> + clocks = <&cru SCLK_USB3OTG1_REF>, <&cru ACLK_USB3OTG1>,
>>> + <&cru SCLK_USB3OTG1_SUSPEND>;
>>> + clock-names = "ref", "bus_early", "suspend";
>>> dr_mode = "otg";
>>> phys = <&u2phy1_otg>, <&tcphy1_usb3>;
>>> phy-names = "usb2-phy", "usb3-phy";
>>>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: Update DWC3 modules on RK3399 SoCs
2019-06-13 17:44 ` Robin Murphy
@ 2019-06-16 14:25 ` Heiko Stübner
-1 siblings, 0 replies; 13+ messages in thread
From: Heiko Stübner @ 2019-06-16 14:25 UTC (permalink / raw)
To: Robin Murphy
Cc: Enric Balletbo i Serra, devicetree, Mark Rutland, Felipe Balbi,
Masahiro Yamada, Christoph Muellner, Viresh Kumar, Randy Li,
linux-kernel, Vicente Bergas, linux-rockchip, Tony Xie,
Klaus Goger, Rob Herring, Collabora Kernel ML, linux-arm-kernel,
Matthias Brugger
Am Donnerstag, 13. Juni 2019, 19:44:43 CEST schrieb Robin Murphy:
> On 13/06/2019 18:20, Enric Balletbo i Serra wrote:
> > Hi Robin,
> >
> > On 13/6/19 18:56, Robin Murphy wrote:
> >> On 13/06/2019 17:27, Enric Balletbo i Serra wrote:
> >>> As per binding documentation [1], the DWC3 core should have the "ref",
> >>> "bus_early" and "suspend" clocks. As explained in the binding, those
> >>> clocks are required for new platforms but not for existing platforms
> >>> before commit fe8abf332b8f ("usb: dwc3: support clocks and resets for
> >>> DWC3 core").
> >>>
> >>> However, as those clocks are really treated as required, this ends with
> >>> having some annoying messages when the "rockchip,rk3399-dwc3" is used:
> >>>
> >>> [ 1.724107] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
> >>> [ 1.731893] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
> >>> [ 2.495937] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
> >>> [ 2.647239] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
> >>>
> >>> In order to remove those annoying messages, update the DWC3 hardware
> >>> module node and add all the required clocks. With this change, both, the
> >>> glue node and the DWC3 core node, have the clocks defined, but that's
> >>> not really a problem and there isn't a side effect on do this. So, we
> >>> can get rid of the annoying get clk error messages.
> >>
> >> Can we not just move these clocks entirely from the glue layer to the core
> >> layer? That didn't seem to break when I tried it, although I'll admit my
> >> 'testing' was no more than booting and mounting a USB 3.0 flash drive, no
> >> suspend or anything fancy.
> >>
> >
> > AFAICT usb doesn't break, but we won't break backward compability then? (/me
> > still doesn't know when backward compability is really important or not)
>
> Ah, fair point - I was imagining the glue layer's heavy-handed "turn
> everything on" approach potentially interfering with the core layer's
> ability to exert finer-grained control of the clocks it knows more
> about, but the "old kernel, new DT" situation might indeed be a problem.
> I guess that's Heiko's call in the end.
I really try to stay with the "backwards-compatible" philosophy
(at least with the old dt with new kernel variant) and so far that worked
surprisingly well ;-) .
Personally I don't believe in the other direction, old kernel with new dt,
as we're always adding features, so updating the devicetree without
updating the kernel doesn't look like it would make very much sense.
So if you just keep the old-dt+new-kernel variant intact, I'll be happy.
Heiko
> Plus I'm no CCF expert so my
> concern might be unfounded anyway.
>
> >> My own attempt to shut up these errors got sidetracked into c0c61471ef86 ("usb:
> >> dwc3: of-simple: Convert to bulk clk API"), then apparently stalled :)
> >>
> >
> > There was any off the record discussion and stalled or simply you didn't get
> > feedback?
>
> More that the 3399 board got put away in a freak "tidying the kitchen"
> incident, and I've just had too many other things on the go since :)
>
> Robin.
>
> >
> > I'll take a look.
> >
> > Thanks,
> > ~ Enric
> >
> >> Robin.
> >>
> >>>
> >>> [1] Documentation/devicetree/bindings/usb/dwc3.txt
> >>>
> >>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >>> ---
> >>>
> >>> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 6 ++++++
> >>> 1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >>> index 196ac9b78076..a15348d185ce 100644
> >>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >>> @@ -414,6 +414,9 @@
> >>> compatible = "snps,dwc3";
> >>> reg = <0x0 0xfe800000 0x0 0x100000>;
> >>> interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
> >>> + clocks = <&cru SCLK_USB3OTG0_REF>, <&cru ACLK_USB3OTG0>,
> >>> + <&cru SCLK_USB3OTG0_SUSPEND>;
> >>> + clock-names = "ref", "bus_early", "suspend";
> >>> dr_mode = "otg";
> >>> phys = <&u2phy0_otg>, <&tcphy0_usb3>;
> >>> phy-names = "usb2-phy", "usb3-phy";
> >>> @@ -447,6 +450,9 @@
> >>> compatible = "snps,dwc3";
> >>> reg = <0x0 0xfe900000 0x0 0x100000>;
> >>> interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH 0>;
> >>> + clocks = <&cru SCLK_USB3OTG1_REF>, <&cru ACLK_USB3OTG1>,
> >>> + <&cru SCLK_USB3OTG1_SUSPEND>;
> >>> + clock-names = "ref", "bus_early", "suspend";
> >>> dr_mode = "otg";
> >>> phys = <&u2phy1_otg>, <&tcphy1_usb3>;
> >>> phy-names = "usb2-phy", "usb3-phy";
> >>>
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: Update DWC3 modules on RK3399 SoCs
@ 2019-06-16 14:25 ` Heiko Stübner
0 siblings, 0 replies; 13+ messages in thread
From: Heiko Stübner @ 2019-06-16 14:25 UTC (permalink / raw)
To: Robin Murphy
Cc: Mark Rutland, devicetree, Rob Herring, Felipe Balbi,
linux-rockchip, Viresh Kumar, Randy Li, linux-kernel,
Vicente Bergas, Masahiro Yamada, Tony Xie, Klaus Goger,
Matthias Brugger, Enric Balletbo i Serra, Collabora Kernel ML,
linux-arm-kernel, Christoph Muellner
Am Donnerstag, 13. Juni 2019, 19:44:43 CEST schrieb Robin Murphy:
> On 13/06/2019 18:20, Enric Balletbo i Serra wrote:
> > Hi Robin,
> >
> > On 13/6/19 18:56, Robin Murphy wrote:
> >> On 13/06/2019 17:27, Enric Balletbo i Serra wrote:
> >>> As per binding documentation [1], the DWC3 core should have the "ref",
> >>> "bus_early" and "suspend" clocks. As explained in the binding, those
> >>> clocks are required for new platforms but not for existing platforms
> >>> before commit fe8abf332b8f ("usb: dwc3: support clocks and resets for
> >>> DWC3 core").
> >>>
> >>> However, as those clocks are really treated as required, this ends with
> >>> having some annoying messages when the "rockchip,rk3399-dwc3" is used:
> >>>
> >>> [ 1.724107] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
> >>> [ 1.731893] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
> >>> [ 2.495937] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
> >>> [ 2.647239] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
> >>>
> >>> In order to remove those annoying messages, update the DWC3 hardware
> >>> module node and add all the required clocks. With this change, both, the
> >>> glue node and the DWC3 core node, have the clocks defined, but that's
> >>> not really a problem and there isn't a side effect on do this. So, we
> >>> can get rid of the annoying get clk error messages.
> >>
> >> Can we not just move these clocks entirely from the glue layer to the core
> >> layer? That didn't seem to break when I tried it, although I'll admit my
> >> 'testing' was no more than booting and mounting a USB 3.0 flash drive, no
> >> suspend or anything fancy.
> >>
> >
> > AFAICT usb doesn't break, but we won't break backward compability then? (/me
> > still doesn't know when backward compability is really important or not)
>
> Ah, fair point - I was imagining the glue layer's heavy-handed "turn
> everything on" approach potentially interfering with the core layer's
> ability to exert finer-grained control of the clocks it knows more
> about, but the "old kernel, new DT" situation might indeed be a problem.
> I guess that's Heiko's call in the end.
I really try to stay with the "backwards-compatible" philosophy
(at least with the old dt with new kernel variant) and so far that worked
surprisingly well ;-) .
Personally I don't believe in the other direction, old kernel with new dt,
as we're always adding features, so updating the devicetree without
updating the kernel doesn't look like it would make very much sense.
So if you just keep the old-dt+new-kernel variant intact, I'll be happy.
Heiko
> Plus I'm no CCF expert so my
> concern might be unfounded anyway.
>
> >> My own attempt to shut up these errors got sidetracked into c0c61471ef86 ("usb:
> >> dwc3: of-simple: Convert to bulk clk API"), then apparently stalled :)
> >>
> >
> > There was any off the record discussion and stalled or simply you didn't get
> > feedback?
>
> More that the 3399 board got put away in a freak "tidying the kitchen"
> incident, and I've just had too many other things on the go since :)
>
> Robin.
>
> >
> > I'll take a look.
> >
> > Thanks,
> > ~ Enric
> >
> >> Robin.
> >>
> >>>
> >>> [1] Documentation/devicetree/bindings/usb/dwc3.txt
> >>>
> >>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >>> ---
> >>>
> >>> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 6 ++++++
> >>> 1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >>> index 196ac9b78076..a15348d185ce 100644
> >>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >>> @@ -414,6 +414,9 @@
> >>> compatible = "snps,dwc3";
> >>> reg = <0x0 0xfe800000 0x0 0x100000>;
> >>> interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
> >>> + clocks = <&cru SCLK_USB3OTG0_REF>, <&cru ACLK_USB3OTG0>,
> >>> + <&cru SCLK_USB3OTG0_SUSPEND>;
> >>> + clock-names = "ref", "bus_early", "suspend";
> >>> dr_mode = "otg";
> >>> phys = <&u2phy0_otg>, <&tcphy0_usb3>;
> >>> phy-names = "usb2-phy", "usb3-phy";
> >>> @@ -447,6 +450,9 @@
> >>> compatible = "snps,dwc3";
> >>> reg = <0x0 0xfe900000 0x0 0x100000>;
> >>> interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH 0>;
> >>> + clocks = <&cru SCLK_USB3OTG1_REF>, <&cru ACLK_USB3OTG1>,
> >>> + <&cru SCLK_USB3OTG1_SUSPEND>;
> >>> + clock-names = "ref", "bus_early", "suspend";
> >>> dr_mode = "otg";
> >>> phys = <&u2phy1_otg>, <&tcphy1_usb3>;
> >>> phy-names = "usb2-phy", "usb3-phy";
> >>>
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: Update DWC3 modules on RK3399 SoCs
2019-06-13 16:27 ` Enric Balletbo i Serra
@ 2019-06-27 12:42 ` Heiko Stuebner
-1 siblings, 0 replies; 13+ messages in thread
From: Heiko Stuebner @ 2019-06-27 12:42 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: devicetree, Collabora Kernel ML, Felipe Balbi, Masahiro Yamada,
linux-kernel, linux-rockchip, Rob Herring, Mark Rutland,
linux-arm-kernel, Vicente Bergas, Klaus Goger,
Christoph Muellner, Randy Li, Tony Xie, Viresh Kumar,
Matthias Brugger
Am Donnerstag, 13. Juni 2019, 18:27:45 CEST schrieb Enric Balletbo i Serra:
> As per binding documentation [1], the DWC3 core should have the "ref",
> "bus_early" and "suspend" clocks. As explained in the binding, those
> clocks are required for new platforms but not for existing platforms
> before commit fe8abf332b8f ("usb: dwc3: support clocks and resets for
> DWC3 core").
>
> However, as those clocks are really treated as required, this ends with
> having some annoying messages when the "rockchip,rk3399-dwc3" is used:
>
> [ 1.724107] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
> [ 1.731893] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
> [ 2.495937] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
> [ 2.647239] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
>
> In order to remove those annoying messages, update the DWC3 hardware
> module node and add all the required clocks. With this change, both, the
> glue node and the DWC3 core node, have the clocks defined, but that's
> not really a problem and there isn't a side effect on do this. So, we
> can get rid of the annoying get clk error messages.
>
> [1] Documentation/devicetree/bindings/usb/dwc3.txt
>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
applied for 5.3
Thanks
Heiko
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: Update DWC3 modules on RK3399 SoCs
@ 2019-06-27 12:42 ` Heiko Stuebner
0 siblings, 0 replies; 13+ messages in thread
From: Heiko Stuebner @ 2019-06-27 12:42 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Mark Rutland, devicetree, Felipe Balbi, Masahiro Yamada,
Tony Xie, Viresh Kumar, Randy Li, linux-kernel, Vicente Bergas,
linux-rockchip, Rob Herring, Klaus Goger, Matthias Brugger,
Collabora Kernel ML, linux-arm-kernel, Christoph Muellner
Am Donnerstag, 13. Juni 2019, 18:27:45 CEST schrieb Enric Balletbo i Serra:
> As per binding documentation [1], the DWC3 core should have the "ref",
> "bus_early" and "suspend" clocks. As explained in the binding, those
> clocks are required for new platforms but not for existing platforms
> before commit fe8abf332b8f ("usb: dwc3: support clocks and resets for
> DWC3 core").
>
> However, as those clocks are really treated as required, this ends with
> having some annoying messages when the "rockchip,rk3399-dwc3" is used:
>
> [ 1.724107] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
> [ 1.731893] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
> [ 2.495937] dwc3 fe800000.dwc3: Failed to get clk 'ref': -2
> [ 2.647239] dwc3 fe900000.dwc3: Failed to get clk 'ref': -2
>
> In order to remove those annoying messages, update the DWC3 hardware
> module node and add all the required clocks. With this change, both, the
> glue node and the DWC3 core node, have the clocks defined, but that's
> not really a problem and there isn't a side effect on do this. So, we
> can get rid of the annoying get clk error messages.
>
> [1] Documentation/devicetree/bindings/usb/dwc3.txt
>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
applied for 5.3
Thanks
Heiko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-06-27 12:43 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-13 16:27 [PATCH] arm64: dts: rockchip: Update DWC3 modules on RK3399 SoCs Enric Balletbo i Serra
2019-06-13 16:27 ` Enric Balletbo i Serra
2019-06-13 16:56 ` Robin Murphy
2019-06-13 16:56 ` Robin Murphy
2019-06-13 16:56 ` Robin Murphy
2019-06-13 17:20 ` Enric Balletbo i Serra
2019-06-13 17:20 ` Enric Balletbo i Serra
2019-06-13 17:44 ` Robin Murphy
2019-06-13 17:44 ` Robin Murphy
2019-06-16 14:25 ` Heiko Stübner
2019-06-16 14:25 ` Heiko Stübner
2019-06-27 12:42 ` Heiko Stuebner
2019-06-27 12:42 ` Heiko Stuebner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.