2019-10-27 17:20:57

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH v2 0/5] provide the XTAL clock via OF on Meson8/8b/8m2

So far the HHI clock controller has been providing the XTAL clock on
Amlogic Meson8/Meson8b/Meson8m2 SoCs.
This is not correct because the XTAL is actually a crystal on the
boards and the SoC has a dedicated input for it.

This updates the dt-bindings of the HHI clock controller and defines
a fixed-clock in meson.dtsi (along with switching everything over to
use this clock).
The clock driver needs three updates to use this:
- patch #2 uses clk_hw_set_parent in the CPU clock notifier. This drops
the explicit reference to CLKID_XTAL while at the same time making
the code much easier (thanks to Neil for providing this new method
as part of the G12A CPU clock bringup!)
- patch #3 ensures that the clock driver doesn't rely on it's internal
XTAL clock while not losing support for older .dtbs that don't have
the XTAL clock input yet
- with patch #4 the clock controller's own XTAL clock is not registered
anymore when a clock input is provided via OF

This series is a functional no-op. It's main goal is to better represent
how the actual hardware looks like.


Changes since v1 at [0]:
- add Rob's Reviewed-by to the dt-bindings patch
- check that "xtal" clock is actually passed via OF instead of checking
that there's any parent at all (which in the worst case may not be the
xtal clock) as suggested by Jerome


[0] https://patchwork.kernel.org/cover/11155515/


Martin Blumenstingl (5):
dt-bindings: clock: meson8b: add the clock inputs
clk: meson: meson8b: use clk_hw_set_parent in the CPU clock notifier
clk: meson: meson8b: change references to the XTAL clock to use the
name
clk: meson: meson8b: don't register the XTAL clock when provided via
OF
ARM: dts: meson: provide the XTAL clock using a fixed-clock

.../bindings/clock/amlogic,meson8b-clkc.txt | 5 +
arch/arm/boot/dts/meson.dtsi | 7 ++
arch/arm/boot/dts/meson6.dtsi | 7 --
arch/arm/boot/dts/meson8.dtsi | 15 +--
arch/arm/boot/dts/meson8b-ec100.dts | 2 +-
arch/arm/boot/dts/meson8b-mxq.dts | 2 +-
arch/arm/boot/dts/meson8b-odroidc1.dts | 2 +-
arch/arm/boot/dts/meson8b.dtsi | 15 +--
drivers/clk/meson/meson8b.c | 106 +++++++++---------
9 files changed, 87 insertions(+), 74 deletions(-)

--
2.23.0


2019-10-27 17:23:55

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH v2 3/5] clk: meson: meson8b: change references to the XTAL clock to use the name

The XTAL clock is an actual crystal which is mounted on the PCB. Thus
the meson8b clock controller driver should not provide the XTAL clock.

The meson8b clock controller driver must not use references to
the meson8b_xtal clock anymore before we can provide the XTAL clock
via OF. Replace the references to the meson8b_xtal.hw by using
clk_parent_data.name = "xtal" (along with index = -1) because this works
regardless how the XTAL clock is registered (either as fixed-clock in
the .dtb or - if missing - when registered in the meson8b clock
controller driver).

Signed-off-by: Martin Blumenstingl <[email protected]>
---
drivers/clk/meson/meson8b.c | 73 ++++++++++++++++++++-----------------
1 file changed, 39 insertions(+), 34 deletions(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index d376f80e806d..b785b67baf2b 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -97,8 +97,9 @@ static struct clk_regmap meson8b_fixed_pll_dco = {
.hw.init = &(struct clk_init_data){
.name = "fixed_pll_dco",
.ops = &meson_clk_pll_ro_ops,
- .parent_hws = (const struct clk_hw *[]) {
- &meson8b_xtal.hw
+ .parent_data = &(const struct clk_parent_data) {
+ .name = "xtal",
+ .index = -1,
},
.num_parents = 1,
},
@@ -162,8 +163,9 @@ static struct clk_regmap meson8b_hdmi_pll_dco = {
/* sometimes also called "HPLL" or "HPLL PLL" */
.name = "hdmi_pll_dco",
.ops = &meson_clk_pll_ro_ops,
- .parent_hws = (const struct clk_hw *[]) {
- &meson8b_xtal.hw
+ .parent_data = &(const struct clk_parent_data) {
+ .name = "xtal",
+ .index = -1,
},
.num_parents = 1,
},
@@ -237,8 +239,9 @@ static struct clk_regmap meson8b_sys_pll_dco = {
.hw.init = &(struct clk_init_data){
.name = "sys_pll_dco",
.ops = &meson_clk_pll_ops,
- .parent_hws = (const struct clk_hw *[]) {
- &meson8b_xtal.hw
+ .parent_data = &(const struct clk_parent_data) {
+ .name = "xtal",
+ .index = -1,
},
.num_parents = 1,
},
@@ -631,9 +634,9 @@ static struct clk_regmap meson8b_cpu_in_sel = {
.hw.init = &(struct clk_init_data){
.name = "cpu_in_sel",
.ops = &clk_regmap_mux_ops,
- .parent_hws = (const struct clk_hw *[]) {
- &meson8b_xtal.hw,
- &meson8b_sys_pll.hw,
+ .parent_data = (const struct clk_parent_data[]) {
+ { .name = "xtal", .index = -1, },
+ { .hw = &meson8b_sys_pll.hw, },
},
.num_parents = 2,
.flags = (CLK_SET_RATE_PARENT |
@@ -736,9 +739,9 @@ static struct clk_regmap meson8b_cpu_clk = {
.hw.init = &(struct clk_init_data){
.name = "cpu_clk",
.ops = &clk_regmap_mux_ops,
- .parent_hws = (const struct clk_hw *[]) {
- &meson8b_xtal.hw,
- &meson8b_cpu_scale_out_sel.hw,
+ .parent_data = (const struct clk_parent_data[]) {
+ { .name = "xtal", .index = -1, },
+ { .hw = &meson8b_cpu_scale_out_sel.hw, },
},
.num_parents = 2,
.flags = (CLK_SET_RATE_PARENT |
@@ -758,12 +761,12 @@ static struct clk_regmap meson8b_nand_clk_sel = {
.name = "nand_clk_sel",
.ops = &clk_regmap_mux_ops,
/* FIXME all other parents are unknown: */
- .parent_hws = (const struct clk_hw *[]) {
- &meson8b_fclk_div4.hw,
- &meson8b_fclk_div3.hw,
- &meson8b_fclk_div5.hw,
- &meson8b_fclk_div7.hw,
- &meson8b_xtal.hw,
+ .parent_data = (const struct clk_parent_data[]) {
+ { .hw = &meson8b_fclk_div4.hw, },
+ { .hw = &meson8b_fclk_div3.hw, },
+ { .hw = &meson8b_fclk_div5.hw, },
+ { .hw = &meson8b_fclk_div7.hw, },
+ { .name = "xtal", .index = -1, },
},
.num_parents = 5,
.flags = CLK_SET_RATE_PARENT,
@@ -1721,8 +1724,9 @@ static struct clk_regmap meson8b_hdmi_sys_sel = {
.name = "hdmi_sys_sel",
.ops = &clk_regmap_mux_ro_ops,
/* FIXME: all other parents are unknown */
- .parent_hws = (const struct clk_hw *[]) {
- &meson8b_xtal.hw
+ .parent_data = &(const struct clk_parent_data) {
+ .name = "xtal",
+ .index = -1,
},
.num_parents = 1,
.flags = CLK_SET_RATE_NO_REPARENT,
@@ -1767,14 +1771,14 @@ static struct clk_regmap meson8b_hdmi_sys = {
* muxed by a glitch-free switch on Meson8b and Meson8m2. Meson8 only
* has mali_0 and no glitch-free mux.
*/
-static const struct clk_hw *meson8b_mali_0_1_parent_hws[] = {
- &meson8b_xtal.hw,
- &meson8b_mpll2.hw,
- &meson8b_mpll1.hw,
- &meson8b_fclk_div7.hw,
- &meson8b_fclk_div4.hw,
- &meson8b_fclk_div3.hw,
- &meson8b_fclk_div5.hw,
+static const struct clk_parent_data meson8b_mali_0_1_parent_data[] = {
+ { .name = "xtal", .index = -1, },
+ { .hw = &meson8b_mpll2.hw, },
+ { .hw = &meson8b_mpll1.hw, },
+ { .hw = &meson8b_fclk_div7.hw, },
+ { .hw = &meson8b_fclk_div4.hw, },
+ { .hw = &meson8b_fclk_div3.hw, },
+ { .hw = &meson8b_fclk_div5.hw, },
};

static u32 meson8b_mali_0_1_mux_table[] = { 0, 2, 3, 4, 5, 6, 7 };
@@ -1789,8 +1793,8 @@ static struct clk_regmap meson8b_mali_0_sel = {
.hw.init = &(struct clk_init_data){
.name = "mali_0_sel",
.ops = &clk_regmap_mux_ops,
- .parent_hws = meson8b_mali_0_1_parent_hws,
- .num_parents = ARRAY_SIZE(meson8b_mali_0_1_parent_hws),
+ .parent_data = meson8b_mali_0_1_parent_data,
+ .num_parents = ARRAY_SIZE(meson8b_mali_0_1_parent_data),
/*
* Don't propagate rate changes up because the only changeable
* parents are mpll1 and mpll2 but we need those for audio and
@@ -1844,8 +1848,8 @@ static struct clk_regmap meson8b_mali_1_sel = {
.hw.init = &(struct clk_init_data){
.name = "mali_1_sel",
.ops = &clk_regmap_mux_ops,
- .parent_hws = meson8b_mali_0_1_parent_hws,
- .num_parents = ARRAY_SIZE(meson8b_mali_0_1_parent_hws),
+ .parent_data = meson8b_mali_0_1_parent_data,
+ .num_parents = ARRAY_SIZE(meson8b_mali_0_1_parent_data),
/*
* Don't propagate rate changes up because the only changeable
* parents are mpll1 and mpll2 but we need those for audio and
@@ -1944,8 +1948,9 @@ static struct clk_regmap meson8m2_gp_pll_dco = {
.hw.init = &(struct clk_init_data){
.name = "gp_pll_dco",
.ops = &meson_clk_pll_ops,
- .parent_hws = (const struct clk_hw *[]) {
- &meson8b_xtal.hw
+ .parent_data = &(const struct clk_parent_data) {
+ .name = "xtal",
+ .index = -1,
},
.num_parents = 1,
},
--
2.23.0

2019-10-27 18:24:37

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH v2 2/5] clk: meson: meson8b: use clk_hw_set_parent in the CPU clock notifier

Switch from clk_set_parent() to clk_hw_set_parent() now that we have a
way to configure a mux clock based on clk_hw pointers. This simplifies
the meson8b_cpu_clk_notifier_cb logic. No functional changes.

Signed-off-by: Martin Blumenstingl <[email protected]>
---
drivers/clk/meson/meson8b.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index 67e6691e080c..d376f80e806d 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -3585,7 +3585,7 @@ static const struct reset_control_ops meson8b_clk_reset_ops = {

struct meson8b_nb_data {
struct notifier_block nb;
- struct clk_hw_onecell_data *onecell_data;
+ struct clk_hw *cpu_clk;
};

static int meson8b_cpu_clk_notifier_cb(struct notifier_block *nb,
@@ -3593,30 +3593,25 @@ static int meson8b_cpu_clk_notifier_cb(struct notifier_block *nb,
{
struct meson8b_nb_data *nb_data =
container_of(nb, struct meson8b_nb_data, nb);
- struct clk_hw **hws = nb_data->onecell_data->hws;
- struct clk_hw *cpu_clk_hw, *parent_clk_hw;
- struct clk *cpu_clk, *parent_clk;
+ struct clk_hw *parent_clk;
int ret;

switch (event) {
case PRE_RATE_CHANGE:
- parent_clk_hw = hws[CLKID_XTAL];
+ /* xtal */
+ parent_clk = clk_hw_get_parent_by_index(nb_data->cpu_clk, 0);
break;

case POST_RATE_CHANGE:
- parent_clk_hw = hws[CLKID_CPU_SCALE_OUT_SEL];
+ /* cpu_scale_out_sel */
+ parent_clk = clk_hw_get_parent_by_index(nb_data->cpu_clk, 1);
break;

default:
return NOTIFY_DONE;
}

- cpu_clk_hw = hws[CLKID_CPUCLK];
- cpu_clk = __clk_lookup(clk_hw_get_name(cpu_clk_hw));
-
- parent_clk = __clk_lookup(clk_hw_get_name(parent_clk_hw));
-
- ret = clk_set_parent(cpu_clk, parent_clk);
+ ret = clk_hw_set_parent(nb_data->cpu_clk, parent_clk);
if (ret)
return notifier_from_errno(ret);

@@ -3695,7 +3690,7 @@ static void __init meson8b_clkc_init_common(struct device_node *np,
return;
}

- meson8b_cpu_nb_data.onecell_data = clk_hw_onecell_data;
+ meson8b_cpu_nb_data.cpu_clk = clk_hw_onecell_data->hws[CLKID_CPUCLK];

/*
* FIXME we shouldn't program the muxes in notifier handlers. The
--
2.23.0

2019-10-27 18:24:58

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH v2 4/5] clk: meson: meson8b: don't register the XTAL clock when provided via OF

The XTAL clock is an actual crystal on the PCB. Thus the meson8b clock
driver should not register the XTAL clock - instead it should be
provided via .dts and then passed to the clock controller.

Skip the registration of the XTAL clock if a parent clock is provided
via OF. Fall back to registering the XTAL clock if this is not the case
to keep support for old .dtbs.

Signed-off-by: Martin Blumenstingl <[email protected]>
---
drivers/clk/meson/meson8b.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index b785b67baf2b..70ac6755607e 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -3682,10 +3682,16 @@ static void __init meson8b_clkc_init_common(struct device_node *np,
meson8b_clk_regmaps[i]->map = map;

/*
- * register all clks
- * CLKID_UNUSED = 0, so skip it and start with CLKID_XTAL = 1
+ * always skip CLKID_UNUSED and also skip XTAL if the .dtb provides the
+ * XTAL clock as input.
*/
- for (i = CLKID_XTAL; i < CLK_NR_CLKS; i++) {
+ if (!IS_ERR(of_clk_get_by_name(np, "xtal")))
+ i = CLKID_PLL_FIXED;
+ else
+ i = CLKID_XTAL;
+
+ /* register all clks */
+ for (; i < CLK_NR_CLKS; i++) {
/* array might be sparse */
if (!clk_hw_onecell_data->hws[i])
continue;
--
2.23.0

2019-10-27 18:27:26

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH v2 5/5] ARM: dts: meson: provide the XTAL clock using a fixed-clock

The clock controller driver has provided the XTAL clock so far. This
does not match how the hardware actually works because the XTAL clock is
an actual crystal which is mounted on the PCB.

Add the "xtal" clock to meson.dtsi and replace all references to the
clock controller's CLKID_XTAL with the new xtal clock node.

Signed-off-by: Martin Blumenstingl <[email protected]>
---
arch/arm/boot/dts/meson.dtsi | 7 +++++++
arch/arm/boot/dts/meson6.dtsi | 7 -------
arch/arm/boot/dts/meson8.dtsi | 15 ++++++++-------
arch/arm/boot/dts/meson8b-ec100.dts | 2 +-
arch/arm/boot/dts/meson8b-mxq.dts | 2 +-
arch/arm/boot/dts/meson8b-odroidc1.dts | 2 +-
arch/arm/boot/dts/meson8b.dtsi | 15 ++++++++-------
7 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/arch/arm/boot/dts/meson.dtsi b/arch/arm/boot/dts/meson.dtsi
index c4447f6c8b2c..5d198309058a 100644
--- a/arch/arm/boot/dts/meson.dtsi
+++ b/arch/arm/boot/dts/meson.dtsi
@@ -282,4 +282,11 @@
};
};
};
+
+ xtal: xtal-clk {
+ compatible = "fixed-clock";
+ clock-frequency = <24000000>;
+ clock-output-names = "xtal";
+ #clock-cells = <0>;
+ };
}; /* end of / */
diff --git a/arch/arm/boot/dts/meson6.dtsi b/arch/arm/boot/dts/meson6.dtsi
index 2d31b7ce3f8c..4716030a48d0 100644
--- a/arch/arm/boot/dts/meson6.dtsi
+++ b/arch/arm/boot/dts/meson6.dtsi
@@ -36,13 +36,6 @@
ranges = <0x0 0xd0000000 0x40000>;
};

- xtal: xtal-clk {
- compatible = "fixed-clock";
- clock-frequency = <24000000>;
- clock-output-names = "xtal";
- #clock-cells = <0>;
- };
-
clk81: clk@0 {
#clock-cells = <0>;
compatible = "fixed-clock";
diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi
index 5a7e3e5caebe..4f59a4c8f036 100644
--- a/arch/arm/boot/dts/meson8.dtsi
+++ b/arch/arm/boot/dts/meson8.dtsi
@@ -455,6 +455,8 @@
&hhi {
clkc: clock-controller {
compatible = "amlogic,meson8-clkc";
+ clocks = <&xtal>;
+ clock-names = "xtal";
#clock-cells = <1>;
#reset-cells = <1>;
};
@@ -529,8 +531,7 @@

&saradc {
compatible = "amlogic,meson8-saradc", "amlogic,meson-saradc";
- clocks = <&clkc CLKID_XTAL>,
- <&clkc CLKID_SAR_ADC>;
+ clocks = <&xtal>, <&clkc CLKID_SAR_ADC>;
clock-names = "clkin", "core";
amlogic,hhi-sysctrl = <&hhi>;
nvmem-cells = <&temperature_calib>;
@@ -548,31 +549,31 @@
};

&timer_abcde {
- clocks = <&clkc CLKID_XTAL>, <&clkc CLKID_CLK81>;
+ clocks = <&xtal>, <&clkc CLKID_CLK81>;
clock-names = "xtal", "pclk";
};

&uart_AO {
compatible = "amlogic,meson8-uart", "amlogic,meson-uart";
- clocks = <&clkc CLKID_CLK81>, <&clkc CLKID_XTAL>, <&clkc CLKID_CLK81>;
+ clocks = <&clkc CLKID_CLK81>, <&xtal>, <&clkc CLKID_CLK81>;
clock-names = "baud", "xtal", "pclk";
};

&uart_A {
compatible = "amlogic,meson8-uart", "amlogic,meson-uart";
- clocks = <&clkc CLKID_CLK81>, <&clkc CLKID_XTAL>, <&clkc CLKID_UART0>;
+ clocks = <&clkc CLKID_CLK81>, <&xtal>, <&clkc CLKID_UART0>;
clock-names = "baud", "xtal", "pclk";
};

&uart_B {
compatible = "amlogic,meson8-uart", "amlogic,meson-uart";
- clocks = <&clkc CLKID_CLK81>, <&clkc CLKID_XTAL>, <&clkc CLKID_UART1>;
+ clocks = <&clkc CLKID_CLK81>, <&xtal>, <&clkc CLKID_UART1>;
clock-names = "baud", "xtal", "pclk";
};

&uart_C {
compatible = "amlogic,meson8-uart", "amlogic,meson-uart";
- clocks = <&clkc CLKID_CLK81>, <&clkc CLKID_XTAL>, <&clkc CLKID_UART2>;
+ clocks = <&clkc CLKID_CLK81>, <&xtal>, <&clkc CLKID_UART2>;
clock-names = "baud", "xtal", "pclk";
};

diff --git a/arch/arm/boot/dts/meson8b-ec100.dts b/arch/arm/boot/dts/meson8b-ec100.dts
index bed1dfef1985..163a200d5a7b 100644
--- a/arch/arm/boot/dts/meson8b-ec100.dts
+++ b/arch/arm/boot/dts/meson8b-ec100.dts
@@ -377,7 +377,7 @@
status = "okay";
pinctrl-0 = <&pwm_c1_pins>, <&pwm_d_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_XTAL>, <&clkc CLKID_XTAL>;
+ clocks = <&xtal>, <&xtal>;
clock-names = "clkin0", "clkin1";
};

diff --git a/arch/arm/boot/dts/meson8b-mxq.dts b/arch/arm/boot/dts/meson8b-mxq.dts
index 6e39ad52e42d..33037ef62d0a 100644
--- a/arch/arm/boot/dts/meson8b-mxq.dts
+++ b/arch/arm/boot/dts/meson8b-mxq.dts
@@ -165,7 +165,7 @@
status = "okay";
pinctrl-0 = <&pwm_c1_pins>, <&pwm_d_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_XTAL>, <&clkc CLKID_XTAL>;
+ clocks = <&xtal>, <&xtal>;
clock-names = "clkin0", "clkin1";
};

diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts b/arch/arm/boot/dts/meson8b-odroidc1.dts
index a24eccc354b9..a2a47804fc4a 100644
--- a/arch/arm/boot/dts/meson8b-odroidc1.dts
+++ b/arch/arm/boot/dts/meson8b-odroidc1.dts
@@ -340,7 +340,7 @@
status = "okay";
pinctrl-0 = <&pwm_c1_pins>, <&pwm_d_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_XTAL>, <&clkc CLKID_XTAL>;
+ clocks = <&xtal>, <&xtal>;
clock-names = "clkin0", "clkin1";
};

diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
index 099bf8e711c9..1934666ff60f 100644
--- a/arch/arm/boot/dts/meson8b.dtsi
+++ b/arch/arm/boot/dts/meson8b.dtsi
@@ -434,6 +434,8 @@
&hhi {
clkc: clock-controller {
compatible = "amlogic,meson8-clkc";
+ clocks = <&xtal>;
+ clock-names = "xtal";
#clock-cells = <1>;
#reset-cells = <1>;
};
@@ -508,8 +510,7 @@

&saradc {
compatible = "amlogic,meson8b-saradc", "amlogic,meson-saradc";
- clocks = <&clkc CLKID_XTAL>,
- <&clkc CLKID_SAR_ADC>;
+ clocks = <&xtal>, <&clkc CLKID_SAR_ADC>;
clock-names = "clkin", "core";
amlogic,hhi-sysctrl = <&hhi>;
nvmem-cells = <&temperature_calib>;
@@ -523,31 +524,31 @@
};

&timer_abcde {
- clocks = <&clkc CLKID_XTAL>, <&clkc CLKID_CLK81>;
+ clocks = <&xtal>, <&clkc CLKID_CLK81>;
clock-names = "xtal", "pclk";
};

&uart_AO {
compatible = "amlogic,meson8b-uart", "amlogic,meson-uart";
- clocks = <&clkc CLKID_CLK81>, <&clkc CLKID_XTAL>, <&clkc CLKID_CLK81>;
+ clocks = <&clkc CLKID_CLK81>, <&xtal>, <&clkc CLKID_CLK81>;
clock-names = "baud", "xtal", "pclk";
};

&uart_A {
compatible = "amlogic,meson8b-uart", "amlogic,meson-uart";
- clocks = <&clkc CLKID_CLK81>, <&clkc CLKID_XTAL>, <&clkc CLKID_UART0>;
+ clocks = <&clkc CLKID_CLK81>, <&xtal>, <&clkc CLKID_UART0>;
clock-names = "baud", "xtal", "pclk";
};

&uart_B {
compatible = "amlogic,meson8b-uart", "amlogic,meson-uart";
- clocks = <&clkc CLKID_CLK81>, <&clkc CLKID_XTAL>, <&clkc CLKID_UART1>;
+ clocks = <&clkc CLKID_CLK81>, <&xtal>, <&clkc CLKID_UART1>;
clock-names = "baud", "xtal", "pclk";
};

&uart_C {
compatible = "amlogic,meson8b-uart", "amlogic,meson-uart";
- clocks = <&clkc CLKID_CLK81>, <&clkc CLKID_XTAL>, <&clkc CLKID_UART2>;
+ clocks = <&clkc CLKID_CLK81>, <&xtal>, <&clkc CLKID_UART2>;
clock-names = "baud", "xtal", "pclk";
};

--
2.23.0

2019-11-04 08:10:04

by Jerome Brunet

[permalink] [raw]
Subject: Re: [PATCH v2 3/5] clk: meson: meson8b: change references to the XTAL clock to use the name


On Sun 27 Oct 2019 at 17:18, Martin Blumenstingl <[email protected]> wrote:

> The XTAL clock is an actual crystal which is mounted on the PCB. Thus
> the meson8b clock controller driver should not provide the XTAL clock.
>
> The meson8b clock controller driver must not use references to
> the meson8b_xtal clock anymore before we can provide the XTAL clock
> via OF. Replace the references to the meson8b_xtal.hw by using
> clk_parent_data.name = "xtal" (along with index = -1) because this works
> regardless how the XTAL clock is registered (either as fixed-clock in
> the .dtb or - if missing - when registered in the meson8b clock
> controller driver).
>
> Signed-off-by: Martin Blumenstingl <[email protected]>
> ---
> drivers/clk/meson/meson8b.c | 73 ++++++++++++++++++++-----------------
> 1 file changed, 39 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
> index d376f80e806d..b785b67baf2b 100644
> --- a/drivers/clk/meson/meson8b.c
> +++ b/drivers/clk/meson/meson8b.c
> @@ -97,8 +97,9 @@ static struct clk_regmap meson8b_fixed_pll_dco = {
> .hw.init = &(struct clk_init_data){
> .name = "fixed_pll_dco",
> .ops = &meson_clk_pll_ro_ops,
> - .parent_hws = (const struct clk_hw *[]) {
> - &meson8b_xtal.hw
> + .parent_data = &(const struct clk_parent_data) {
> + .name = "xtal",
> + .index = -1,

if I got correctly, when transitioning to DT, you can specify both
"fw_name" and "name". CCF should try to get the clock through DT and
fallback to global name matching if not available

> },
> .num_parents = 1,
> },
> @@ -162,8 +163,9 @@ static struct clk_regmap meson8b_hdmi_pll_dco = {
> /* sometimes also called "HPLL" or "HPLL PLL" */
> .name = "hdmi_pll_dco",
> .ops = &meson_clk_pll_ro_ops,
> - .parent_hws = (const struct clk_hw *[]) {
> - &meson8b_xtal.hw
> + .parent_data = &(const struct clk_parent_data) {
> + .name = "xtal",
> + .index = -1,
> },
> .num_parents = 1,
> },
> @@ -237,8 +239,9 @@ static struct clk_regmap meson8b_sys_pll_dco = {
> .hw.init = &(struct clk_init_data){
> .name = "sys_pll_dco",
> .ops = &meson_clk_pll_ops,
> - .parent_hws = (const struct clk_hw *[]) {
> - &meson8b_xtal.hw
> + .parent_data = &(const struct clk_parent_data) {
> + .name = "xtal",
> + .index = -1,
> },
> .num_parents = 1,
> },
> @@ -631,9 +634,9 @@ static struct clk_regmap meson8b_cpu_in_sel = {
> .hw.init = &(struct clk_init_data){
> .name = "cpu_in_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_hws = (const struct clk_hw *[]) {
> - &meson8b_xtal.hw,
> - &meson8b_sys_pll.hw,
> + .parent_data = (const struct clk_parent_data[]) {
> + { .name = "xtal", .index = -1, },
> + { .hw = &meson8b_sys_pll.hw, },
> },
> .num_parents = 2,
> .flags = (CLK_SET_RATE_PARENT |
> @@ -736,9 +739,9 @@ static struct clk_regmap meson8b_cpu_clk = {
> .hw.init = &(struct clk_init_data){
> .name = "cpu_clk",
> .ops = &clk_regmap_mux_ops,
> - .parent_hws = (const struct clk_hw *[]) {
> - &meson8b_xtal.hw,
> - &meson8b_cpu_scale_out_sel.hw,
> + .parent_data = (const struct clk_parent_data[]) {
> + { .name = "xtal", .index = -1, },
> + { .hw = &meson8b_cpu_scale_out_sel.hw, },
> },
> .num_parents = 2,
> .flags = (CLK_SET_RATE_PARENT |
> @@ -758,12 +761,12 @@ static struct clk_regmap meson8b_nand_clk_sel = {
> .name = "nand_clk_sel",
> .ops = &clk_regmap_mux_ops,
> /* FIXME all other parents are unknown: */
> - .parent_hws = (const struct clk_hw *[]) {
> - &meson8b_fclk_div4.hw,
> - &meson8b_fclk_div3.hw,
> - &meson8b_fclk_div5.hw,
> - &meson8b_fclk_div7.hw,
> - &meson8b_xtal.hw,
> + .parent_data = (const struct clk_parent_data[]) {
> + { .hw = &meson8b_fclk_div4.hw, },
> + { .hw = &meson8b_fclk_div3.hw, },
> + { .hw = &meson8b_fclk_div5.hw, },
> + { .hw = &meson8b_fclk_div7.hw, },
> + { .name = "xtal", .index = -1, },
> },
> .num_parents = 5,
> .flags = CLK_SET_RATE_PARENT,
> @@ -1721,8 +1724,9 @@ static struct clk_regmap meson8b_hdmi_sys_sel = {
> .name = "hdmi_sys_sel",
> .ops = &clk_regmap_mux_ro_ops,
> /* FIXME: all other parents are unknown */
> - .parent_hws = (const struct clk_hw *[]) {
> - &meson8b_xtal.hw
> + .parent_data = &(const struct clk_parent_data) {
> + .name = "xtal",
> + .index = -1,
> },
> .num_parents = 1,
> .flags = CLK_SET_RATE_NO_REPARENT,
> @@ -1767,14 +1771,14 @@ static struct clk_regmap meson8b_hdmi_sys = {
> * muxed by a glitch-free switch on Meson8b and Meson8m2. Meson8 only
> * has mali_0 and no glitch-free mux.
> */
> -static const struct clk_hw *meson8b_mali_0_1_parent_hws[] = {
> - &meson8b_xtal.hw,
> - &meson8b_mpll2.hw,
> - &meson8b_mpll1.hw,
> - &meson8b_fclk_div7.hw,
> - &meson8b_fclk_div4.hw,
> - &meson8b_fclk_div3.hw,
> - &meson8b_fclk_div5.hw,
> +static const struct clk_parent_data meson8b_mali_0_1_parent_data[] = {
> + { .name = "xtal", .index = -1, },
> + { .hw = &meson8b_mpll2.hw, },
> + { .hw = &meson8b_mpll1.hw, },
> + { .hw = &meson8b_fclk_div7.hw, },
> + { .hw = &meson8b_fclk_div4.hw, },
> + { .hw = &meson8b_fclk_div3.hw, },
> + { .hw = &meson8b_fclk_div5.hw, },
> };
>
> static u32 meson8b_mali_0_1_mux_table[] = { 0, 2, 3, 4, 5, 6, 7 };
> @@ -1789,8 +1793,8 @@ static struct clk_regmap meson8b_mali_0_sel = {
> .hw.init = &(struct clk_init_data){
> .name = "mali_0_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_hws = meson8b_mali_0_1_parent_hws,
> - .num_parents = ARRAY_SIZE(meson8b_mali_0_1_parent_hws),
> + .parent_data = meson8b_mali_0_1_parent_data,
> + .num_parents = ARRAY_SIZE(meson8b_mali_0_1_parent_data),
> /*
> * Don't propagate rate changes up because the only changeable
> * parents are mpll1 and mpll2 but we need those for audio and
> @@ -1844,8 +1848,8 @@ static struct clk_regmap meson8b_mali_1_sel = {
> .hw.init = &(struct clk_init_data){
> .name = "mali_1_sel",
> .ops = &clk_regmap_mux_ops,
> - .parent_hws = meson8b_mali_0_1_parent_hws,
> - .num_parents = ARRAY_SIZE(meson8b_mali_0_1_parent_hws),
> + .parent_data = meson8b_mali_0_1_parent_data,
> + .num_parents = ARRAY_SIZE(meson8b_mali_0_1_parent_data),
> /*
> * Don't propagate rate changes up because the only changeable
> * parents are mpll1 and mpll2 but we need those for audio and
> @@ -1944,8 +1948,9 @@ static struct clk_regmap meson8m2_gp_pll_dco = {
> .hw.init = &(struct clk_init_data){
> .name = "gp_pll_dco",
> .ops = &meson_clk_pll_ops,
> - .parent_hws = (const struct clk_hw *[]) {
> - &meson8b_xtal.hw
> + .parent_data = &(const struct clk_parent_data) {
> + .name = "xtal",
> + .index = -1,
> },
> .num_parents = 1,
> },

2019-11-04 22:32:42

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH v2 3/5] clk: meson: meson8b: change references to the XTAL clock to use the name

Hi Jerome,

On Mon, Nov 4, 2019 at 9:08 AM Jerome Brunet <[email protected]> wrote:
>
>
> On Sun 27 Oct 2019 at 17:18, Martin Blumenstingl <[email protected]> wrote:
>
> > The XTAL clock is an actual crystal which is mounted on the PCB. Thus
> > the meson8b clock controller driver should not provide the XTAL clock.
> >
> > The meson8b clock controller driver must not use references to
> > the meson8b_xtal clock anymore before we can provide the XTAL clock
> > via OF. Replace the references to the meson8b_xtal.hw by using
> > clk_parent_data.name = "xtal" (along with index = -1) because this works
> > regardless how the XTAL clock is registered (either as fixed-clock in
> > the .dtb or - if missing - when registered in the meson8b clock
> > controller driver).
> >
> > Signed-off-by: Martin Blumenstingl <[email protected]>
> > ---
> > drivers/clk/meson/meson8b.c | 73 ++++++++++++++++++++-----------------
> > 1 file changed, 39 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
> > index d376f80e806d..b785b67baf2b 100644
> > --- a/drivers/clk/meson/meson8b.c
> > +++ b/drivers/clk/meson/meson8b.c
> > @@ -97,8 +97,9 @@ static struct clk_regmap meson8b_fixed_pll_dco = {
> > .hw.init = &(struct clk_init_data){
> > .name = "fixed_pll_dco",
> > .ops = &meson_clk_pll_ro_ops,
> > - .parent_hws = (const struct clk_hw *[]) {
> > - &meson8b_xtal.hw
> > + .parent_data = &(const struct clk_parent_data) {
> > + .name = "xtal",
> > + .index = -1,
>
> if I got correctly, when transitioning to DT, you can specify both
> "fw_name" and "name". CCF should try to get the clock through DT and
> fallback to global name matching if not available
thank you for the hint - I may even get away with just setting fw_name
if I understand clk_core_get() correctly.
I'll try that during the weekend and report back


Martin