[PATCH v2 1/3] clk: rockchip: rk3399: make CPU clocks critical

[PATCH v2 1/3] clk: rockchip: rk3399: make CPU clocks critical

[Brian Norris]

The CPU clocks don't currently have any owner (e.g., cpufreq-dt doesn't
enable() them -- and even if it did, it's not early enough compared to
other consumers -- nor does arch/arm64/kernel/smp.c), and instead are
simply assumed to be "on" all the time.

They are also parents of a few other clocks which haven't been
previously exposed for other devices to consume. If we want to expose
those clocks, then the common clock framework may eventually choose to
disable their parents (including the CPU PLLs) -- which is no fun for
anyone.

Thus, mark the CPU clocks as critical, to prevent them from being
disabled implicitly.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---

Changes in v2:
  - New, split from the patch that requires this change

 drivers/clk/rockchip/clk-rk3399.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
index 62a4f2543960..0ac9c72c4ee8 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -1514,7 +1514,10 @@ static const char *const rk3399_cru_critical_clocks[] __initconst = {
 	"aclk_vio_noc",
 
 	/* ddrc */
-	"sclk_ddrc"
+	"sclk_ddrc",
+
+	"armclkl",
+	"armclkb",
 };
 
 static const char *const rk3399_pmucru_critical_clocks[] __initconst = {
@@ -1549,9 +1552,6 @@ static void __init rk3399_clk_init(struct device_node *np)
 	rockchip_clk_register_branches(ctx, rk3399_clk_branches,
 				  ARRAY_SIZE(rk3399_clk_branches));
 
-	rockchip_clk_protect_critical(rk3399_cru_critical_clocks,
-				      ARRAY_SIZE(rk3399_cru_critical_clocks));
-
 	rockchip_clk_register_armclk(ctx, ARMCLKL, "armclkl",
 			mux_armclkl_p, ARRAY_SIZE(mux_armclkl_p),
 			&rk3399_cpuclkl_data, rk3399_cpuclkl_rates,
@@ -1562,6 +1562,9 @@ static void __init rk3399_clk_init(struct device_node *np)
 			&rk3399_cpuclkb_data, rk3399_cpuclkb_rates,
 			ARRAY_SIZE(rk3399_cpuclkb_rates));
 
+	rockchip_clk_protect_critical(rk3399_cru_critical_clocks,
+				      ARRAY_SIZE(rk3399_cru_critical_clocks));
+
 	rockchip_register_softrst(np, 21, reg_base + RK3399_SOFTRST_CON(0),
 				  ROCKCHIP_SOFTRST_HIWORD_MASK);
 
-- 
2.33.0.153.gba50c8fa24-goog

[PATCH v2 2/3] clk: rockchip: rk3399: expose PCLK_COREDBG_{B,L}

[Brian Norris]

We have DT IDs for PCLK_COREDBG_L and PCLK_COREDBG_B, but we don't
actually expose them.

Note that this requires the previous patch (making "armclkl" and
"armclkb" into "critical" clocks) to prevent these clocks from taking
down the CPU.

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---

Changes in v2:
  - Split "CPU clocks are critical" to a new patch, to make it more
    obvious

 drivers/clk/rockchip/clk-rk3399.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
index 0ac9c72c4ee8..53ed5cca335b 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -481,7 +481,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] __initdata = {
 	COMPOSITE_NOMUX(0, "atclk_core_l", "armclkl", CLK_IGNORE_UNUSED,
 			RK3399_CLKSEL_CON(1), 0, 5, DFLAGS | CLK_DIVIDER_READ_ONLY,
 			RK3399_CLKGATE_CON(0), 5, GFLAGS),
-	COMPOSITE_NOMUX(0, "pclk_dbg_core_l", "armclkl", CLK_IGNORE_UNUSED,
+	COMPOSITE_NOMUX(PCLK_COREDBG_L, "pclk_dbg_core_l", "armclkl", CLK_IGNORE_UNUSED,
 			RK3399_CLKSEL_CON(1), 8, 5, DFLAGS | CLK_DIVIDER_READ_ONLY,
 			RK3399_CLKGATE_CON(0), 6, GFLAGS),
 
@@ -531,7 +531,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] __initdata = {
 	GATE(ACLK_GIC_ADB400_CORE_B_2_GIC, "aclk_core_adb400_core_b_2_gic", "armclkb", CLK_IGNORE_UNUSED,
 			RK3399_CLKGATE_CON(14), 4, GFLAGS),
 
-	DIV(0, "pclken_dbg_core_b", "pclk_dbg_core_b", CLK_IGNORE_UNUSED,
+	DIV(PCLK_COREDBG_B, "pclken_dbg_core_b", "pclk_dbg_core_b", CLK_IGNORE_UNUSED,
 			RK3399_CLKSEL_CON(3), 13, 2, DFLAGS | CLK_DIVIDER_READ_ONLY),
 
 	GATE(0, "pclk_dbg_cxcs_pd_core_b", "pclk_dbg_core_b", CLK_IGNORE_UNUSED,
-- 
2.33.0.153.gba50c8fa24-goog

[PATCH v2 3/3] arm64: dts: rockchip: add Coresight debug range for RK3399

[Brian Norris]

Per Documentation/devicetree/bindings/arm/coresight-cpu-debug.txt.

This IP block can be used for sampling the PC of any given CPU, which is
useful in certain panic scenarios where you can't get the CPU to stop
cleanly (e.g., hard lockup).

Reviewed-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---

Changes in v2:
- Sort properly

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 48 ++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 3871c7fd83b0..c5fe2d440114 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -361,6 +361,54 @@ usb_host1_ohci: usb@fe3e0000 {
 		status = "disabled";
 	};
 
+	debug@fe430000 {
+		compatible = "arm,coresight-cpu-debug", "arm,primecell";
+		reg = <0 0xfe430000 0 0x1000>;
+		clocks = <&cru PCLK_COREDBG_L>;
+		clock-names = "apb_pclk";
+		cpu = <&cpu_l0>;
+	};
+
+	debug@fe432000 {
+		compatible = "arm,coresight-cpu-debug", "arm,primecell";
+		reg = <0 0xfe432000 0 0x1000>;
+		clocks = <&cru PCLK_COREDBG_L>;
+		clock-names = "apb_pclk";
+		cpu = <&cpu_l1>;
+	};
+
+	debug@fe434000 {
+		compatible = "arm,coresight-cpu-debug", "arm,primecell";
+		reg = <0 0xfe434000 0 0x1000>;
+		clocks = <&cru PCLK_COREDBG_L>;
+		clock-names = "apb_pclk";
+		cpu = <&cpu_l2>;
+	};
+
+	debug@fe436000 {
+		compatible = "arm,coresight-cpu-debug", "arm,primecell";
+		reg = <0 0xfe436000 0 0x1000>;
+		clocks = <&cru PCLK_COREDBG_L>;
+		clock-names = "apb_pclk";
+		cpu = <&cpu_l3>;
+	};
+
+	debug@fe610000 {
+		compatible = "arm,coresight-cpu-debug", "arm,primecell";
+		reg = <0 0xfe610000 0 0x1000>;
+		clocks = <&cru PCLK_COREDBG_B>;
+		clock-names = "apb_pclk";
+		cpu = <&cpu_b0>;
+	};
+
+	debug@fe710000 {
+		compatible = "arm,coresight-cpu-debug", "arm,primecell";
+		reg = <0 0xfe710000 0 0x1000>;
+		clocks = <&cru PCLK_COREDBG_B>;
+		clock-names = "apb_pclk";
+		cpu = <&cpu_b1>;
+	};
+
 	usbdrd3_0: usb@fe800000 {
 		compatible = "rockchip,rk3399-dwc3";
 		#address-cells = <2>;
-- 
2.33.0.153.gba50c8fa24-goog

Re: [PATCH v2 1/3] clk: rockchip: rk3399: make CPU clocks critical

[Doug Anderson]

Hi,

On Wed, Sep 8, 2021 at 11:14 AM Brian Norris <briannorris@chromium.org> wrote:
>
> The CPU clocks don't currently have any owner (e.g., cpufreq-dt doesn't
> enable() them -- and even if it did, it's not early enough compared to
> other consumers -- nor does arch/arm64/kernel/smp.c), and instead are
> simply assumed to be "on" all the time.
>
> They are also parents of a few other clocks which haven't been
> previously exposed for other devices to consume. If we want to expose
> those clocks, then the common clock framework may eventually choose to
> disable their parents (including the CPU PLLs) -- which is no fun for
> anyone.
>
> Thus, mark the CPU clocks as critical, to prevent them from being
> disabled implicitly.
>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
>
> Changes in v2:
>   - New, split from the patch that requires this change
>
>  drivers/clk/rockchip/clk-rk3399.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v2 2/3] clk: rockchip: rk3399: expose PCLK_COREDBG_{B,L}

[Doug Anderson]

Hi,

On Wed, Sep 8, 2021 at 11:14 AM Brian Norris <briannorris@chromium.org> wrote:
>
> We have DT IDs for PCLK_COREDBG_L and PCLK_COREDBG_B, but we don't
> actually expose them.
>
> Note that this requires the previous patch (making "armclkl" and
> "armclkb" into "critical" clocks) to prevent these clocks from taking
> down the CPU.
>
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
>
> Changes in v2:
>   - Split "CPU clocks are critical" to a new patch, to make it more
>     obvious
>
>  drivers/clk/rockchip/clk-rk3399.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v2 1/3] clk: rockchip: rk3399: make CPU clocks critical

[Chen-Yu Tsai]

On Thu, Sep 9, 2021 at 3:44 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Wed, Sep 8, 2021 at 11:14 AM Brian Norris <briannorris@chromium.org> wrote:
> >
> > The CPU clocks don't currently have any owner (e.g., cpufreq-dt doesn't
> > enable() them -- and even if it did, it's not early enough compared to
> > other consumers -- nor does arch/arm64/kernel/smp.c), and instead are
> > simply assumed to be "on" all the time.
> >
> > They are also parents of a few other clocks which haven't been
> > previously exposed for other devices to consume. If we want to expose
> > those clocks, then the common clock framework may eventually choose to
> > disable their parents (including the CPU PLLs) -- which is no fun for
> > anyone.
> >
> > Thus, mark the CPU clocks as critical, to prevent them from being
> > disabled implicitly.
> >
> > Signed-off-by: Brian Norris <briannorris@chromium.org>
> > ---
> >
> > Changes in v2:
> >   - New, split from the patch that requires this change
> >
> >  drivers/clk/rockchip/clk-rk3399.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

Re: [PATCH v2 1/3] clk: rockchip: rk3399: make CPU clocks critical

[Heiko Stuebner]

On Wed, 8 Sep 2021 11:13:38 -0700, Brian Norris wrote:
> The CPU clocks don't currently have any owner (e.g., cpufreq-dt doesn't
> enable() them -- and even if it did, it's not early enough compared to
> other consumers -- nor does arch/arm64/kernel/smp.c), and instead are
> simply assumed to be "on" all the time.
> 
> They are also parents of a few other clocks which haven't been
> previously exposed for other devices to consume. If we want to expose
> those clocks, then the common clock framework may eventually choose to
> disable their parents (including the CPU PLLs) -- which is no fun for
> anyone.
> 
> [...]

Applied, thanks!

[1/3] clk: rockchip: rk3399: make CPU clocks critical
      commit: ef087b7ecf8aaeb08a17ae825f10cd94e116616e
[2/3] clk: rockchip: rk3399: expose PCLK_COREDBG_{B,L}
      commit: bd2c1f664ea647d8f66fbe083f9256511d4f2b9a
[3/3] arm64: dts: rockchip: add Coresight debug range for RK3399
      commit: 75dccea503b8e176ad044175e891d7bb291b6ba0

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>