bcm53xx: backport clk driver fix for DT nodes names

It allows dropping downstream patch renaming DT nodes.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
This commit is contained in:
Rafał Miłecki 2022-09-29 05:51:25 +02:00
parent c5e167e0d6
commit 77d9cce604
4 changed files with 144 additions and 124 deletions

View File

@ -0,0 +1,72 @@
From 1b24a132eba7a1c19475ba2510ec1c00af3ff914 Mon Sep 17 00:00:00 2001
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Mon, 5 Sep 2022 09:15:03 -0700
Subject: [PATCH] clk: iproc: Do not rely on node name for correct PLL setup
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
After commit 31fd9b79dc58 ("ARM: dts: BCM5301X: update CRU block
description") a warning from clk-iproc-pll.c was generated due to a
duplicate PLL name as well as the console stopped working. Upon closer
inspection it became clear that iproc_pll_clk_setup() used the Device
Tree node unit name as an unique identifier as well as a parent name to
parent all clocks under the PLL.
BCM5301X was the first platform on which that got noticed because of the
DT node unit name renaming but the same assumptions hold true for any
user of the iproc_pll_clk_setup() function.
The first 'clock-output-names' property is always guaranteed to be
unique as well as providing the actual desired PLL clock name, so we
utilize that to register the PLL and as a parent name of all children
clock.
Fixes: 5fe225c105fd ("clk: iproc: add initial common clock support")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Rafał Miłecki <rafal@milecki.pl>
Link: https://lore.kernel.org/r/20220905161504.1526-1-f.fainelli@gmail.com
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
drivers/clk/bcm/clk-iproc-pll.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
--- a/drivers/clk/bcm/clk-iproc-pll.c
+++ b/drivers/clk/bcm/clk-iproc-pll.c
@@ -736,6 +736,7 @@ void iproc_pll_clk_setup(struct device_n
const char *parent_name;
struct iproc_clk *iclk_array;
struct clk_hw_onecell_data *clk_data;
+ const char *clk_name;
if (WARN_ON(!pll_ctrl) || WARN_ON(!clk_ctrl))
return;
@@ -783,7 +784,12 @@ void iproc_pll_clk_setup(struct device_n
iclk = &iclk_array[0];
iclk->pll = pll;
- init.name = node->name;
+ ret = of_property_read_string_index(node, "clock-output-names",
+ 0, &clk_name);
+ if (WARN_ON(ret))
+ goto err_pll_register;
+
+ init.name = clk_name;
init.ops = &iproc_pll_ops;
init.flags = 0;
parent_name = of_clk_get_parent_name(node, 0);
@@ -803,13 +809,11 @@ void iproc_pll_clk_setup(struct device_n
goto err_pll_register;
clk_data->hws[0] = &iclk->hw;
+ parent_name = clk_name;
/* now initialize and register all leaf clocks */
for (i = 1; i < num_clks; i++) {
- const char *clk_name;
-
memset(&init, 0, sizeof(init));
- parent_name = node->name;
ret = of_property_read_string_index(node, "clock-output-names",
i, &clk_name);

View File

@ -1,62 +0,0 @@
From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <rafal@milecki.pl>
Date: Tue, 23 Nov 2021 13:13:05 +0100
Subject: [PATCH] ARM: dts: BCM5301X: Switch back to old clock nodes names
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
First of all using the same node name prefix resulted in trying to
register 2 clocks under the same "clock-controller" name:
[ 0.000000] __clk_core_init: clk clock-controller already initialized
[ 0.000000] ------------[ cut here ]------------
[ 0.000000] WARNING: CPU: 0 PID: 0 at drivers/clk/bcm/clk-iproc-pll.c:802 iproc_pll_clk_setup+0x4c8/0x4f4
[ 0.000000] Modules linked in:
[ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.10.80 #0
[ 0.000000] Hardware name: BCM5301X
[ 0.000000] [<c0108410>] (unwind_backtrace) from [<c0104bc4>] (show_stack+0x10/0x14)
[ 0.000000] [<c0104bc4>] (show_stack) from [<c03dca28>] (dump_stack+0x94/0xa8)
[ 0.000000] [<c03dca28>] (dump_stack) from [<c0118440>] (__warn+0xb8/0x114)
[ 0.000000] [<c0118440>] (__warn) from [<c0118504>] (warn_slowpath_fmt+0x68/0x78)
[ 0.000000] [<c0118504>] (warn_slowpath_fmt) from [<c043281c>] (iproc_pll_clk_setup+0x4c8/0x4f4)
[ 0.000000] [<c043281c>] (iproc_pll_clk_setup) from [<c0818c04>] (nsp_genpll_clk_init+0x30/0x38)
[ 0.000000] [<c0818c04>] (nsp_genpll_clk_init) from [<c0818634>] (of_clk_init+0x118/0x1f8)
[ 0.000000] [<c0818634>] (of_clk_init) from [<c08039b0>] (time_init+0x24/0x30)
[ 0.000000] [<c08039b0>] (time_init) from [<c0800d14>] (start_kernel+0x398/0x50c)
[ 0.000000] [<c0800d14>] (start_kernel) from [<00000000>] (0x0)
[ 0.000000] ---[ end trace fe236bfe9559ee50 ]---
Secondly using any other names than "lcpll0" and "genpll" breaks output
clocks:
$ cat /sys/kernel/debug/clk/usbclk/clk_rate
0
For some reason iproc_clk_recalc_rate() gets called with "parent_rate"
argument 0 whenever clocks aren't named "lcpll0" and "genpll".
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
arch/arm/boot/dts/bcm5301x.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -421,7 +421,7 @@
#address-cells = <1>;
#size-cells = <1>;
- lcpll0: clock-controller@100 {
+ lcpll0: lcpll0@100 {
#clock-cells = <1>;
compatible = "brcm,nsp-lcpll0";
reg = <0x100 0x14>;
@@ -430,7 +430,7 @@
"sdio", "ddr_phy";
};
- genpll: clock-controller@140 {
+ genpll: genpll@140 {
#clock-cells = <1>;
compatible = "brcm,nsp-genpll";
reg = <0x140 0x24>;

View File

@ -0,0 +1,72 @@
From 1b24a132eba7a1c19475ba2510ec1c00af3ff914 Mon Sep 17 00:00:00 2001
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Mon, 5 Sep 2022 09:15:03 -0700
Subject: [PATCH] clk: iproc: Do not rely on node name for correct PLL setup
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
After commit 31fd9b79dc58 ("ARM: dts: BCM5301X: update CRU block
description") a warning from clk-iproc-pll.c was generated due to a
duplicate PLL name as well as the console stopped working. Upon closer
inspection it became clear that iproc_pll_clk_setup() used the Device
Tree node unit name as an unique identifier as well as a parent name to
parent all clocks under the PLL.
BCM5301X was the first platform on which that got noticed because of the
DT node unit name renaming but the same assumptions hold true for any
user of the iproc_pll_clk_setup() function.
The first 'clock-output-names' property is always guaranteed to be
unique as well as providing the actual desired PLL clock name, so we
utilize that to register the PLL and as a parent name of all children
clock.
Fixes: 5fe225c105fd ("clk: iproc: add initial common clock support")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Rafał Miłecki <rafal@milecki.pl>
Link: https://lore.kernel.org/r/20220905161504.1526-1-f.fainelli@gmail.com
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
drivers/clk/bcm/clk-iproc-pll.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
--- a/drivers/clk/bcm/clk-iproc-pll.c
+++ b/drivers/clk/bcm/clk-iproc-pll.c
@@ -736,6 +736,7 @@ void iproc_pll_clk_setup(struct device_n
const char *parent_name;
struct iproc_clk *iclk_array;
struct clk_hw_onecell_data *clk_data;
+ const char *clk_name;
if (WARN_ON(!pll_ctrl) || WARN_ON(!clk_ctrl))
return;
@@ -783,7 +784,12 @@ void iproc_pll_clk_setup(struct device_n
iclk = &iclk_array[0];
iclk->pll = pll;
- init.name = node->name;
+ ret = of_property_read_string_index(node, "clock-output-names",
+ 0, &clk_name);
+ if (WARN_ON(ret))
+ goto err_pll_register;
+
+ init.name = clk_name;
init.ops = &iproc_pll_ops;
init.flags = 0;
parent_name = of_clk_get_parent_name(node, 0);
@@ -803,13 +809,11 @@ void iproc_pll_clk_setup(struct device_n
goto err_pll_register;
clk_data->hws[0] = &iclk->hw;
+ parent_name = clk_name;
/* now initialize and register all leaf clocks */
for (i = 1; i < num_clks; i++) {
- const char *clk_name;
-
memset(&init, 0, sizeof(init));
- parent_name = node->name;
ret = of_property_read_string_index(node, "clock-output-names",
i, &clk_name);

View File

@ -1,62 +0,0 @@
From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= <rafal@milecki.pl>
Date: Tue, 23 Nov 2021 13:13:05 +0100
Subject: [PATCH] ARM: dts: BCM5301X: Switch back to old clock nodes names
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
First of all using the same node name prefix resulted in trying to
register 2 clocks under the same "clock-controller" name:
[ 0.000000] __clk_core_init: clk clock-controller already initialized
[ 0.000000] ------------[ cut here ]------------
[ 0.000000] WARNING: CPU: 0 PID: 0 at drivers/clk/bcm/clk-iproc-pll.c:802 iproc_pll_clk_setup+0x4c8/0x4f4
[ 0.000000] Modules linked in:
[ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.10.80 #0
[ 0.000000] Hardware name: BCM5301X
[ 0.000000] [<c0108410>] (unwind_backtrace) from [<c0104bc4>] (show_stack+0x10/0x14)
[ 0.000000] [<c0104bc4>] (show_stack) from [<c03dca28>] (dump_stack+0x94/0xa8)
[ 0.000000] [<c03dca28>] (dump_stack) from [<c0118440>] (__warn+0xb8/0x114)
[ 0.000000] [<c0118440>] (__warn) from [<c0118504>] (warn_slowpath_fmt+0x68/0x78)
[ 0.000000] [<c0118504>] (warn_slowpath_fmt) from [<c043281c>] (iproc_pll_clk_setup+0x4c8/0x4f4)
[ 0.000000] [<c043281c>] (iproc_pll_clk_setup) from [<c0818c04>] (nsp_genpll_clk_init+0x30/0x38)
[ 0.000000] [<c0818c04>] (nsp_genpll_clk_init) from [<c0818634>] (of_clk_init+0x118/0x1f8)
[ 0.000000] [<c0818634>] (of_clk_init) from [<c08039b0>] (time_init+0x24/0x30)
[ 0.000000] [<c08039b0>] (time_init) from [<c0800d14>] (start_kernel+0x398/0x50c)
[ 0.000000] [<c0800d14>] (start_kernel) from [<00000000>] (0x0)
[ 0.000000] ---[ end trace fe236bfe9559ee50 ]---
Secondly using any other names than "lcpll0" and "genpll" breaks output
clocks:
$ cat /sys/kernel/debug/clk/usbclk/clk_rate
0
For some reason iproc_clk_recalc_rate() gets called with "parent_rate"
argument 0 whenever clocks aren't named "lcpll0" and "genpll".
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
arch/arm/boot/dts/bcm5301x.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -421,7 +421,7 @@
#address-cells = <1>;
#size-cells = <1>;
- lcpll0: clock-controller@100 {
+ lcpll0: lcpll0@100 {
#clock-cells = <1>;
compatible = "brcm,nsp-lcpll0";
reg = <0x100 0x14>;
@@ -430,7 +430,7 @@
"sdio", "ddr_phy";
};
- genpll: clock-controller@140 {
+ genpll: genpll@140 {
#clock-cells = <1>;
compatible = "brcm,nsp-genpll";
reg = <0x140 0x24>;