All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chen-Yu Tsai <wenst@chromium.org>
To: Brian Norris <briannorris@chromium.org>
Cc: Heiko Stuebner <heiko@sntech.de>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	linux-clk@vger.kernel.org,
	Douglas Anderson <dianders@chromium.org>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-rockchip@lists.infradead.org,
	"moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [RESEND PATCH 1/2] clk: rockchip: rk3399: expose PCLK_COREDBG_{B,L}
Date: Wed, 8 Sep 2021 13:00:52 +0800	[thread overview]
Message-ID: <CAGXv+5E1Kk3PYK1GBatfBVnaEVfGCLvb5P-dN9S05LawAfimjw@mail.gmail.com> (raw)
In-Reply-To: <20210907094628.RESEND.1.If29cd838efbcee4450a62b8d84a99b23c86e0a3f@changeid>

On Wed, Sep 8, 2021 at 12:46 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.
>
> In exposing these clocks (and attaching them to the coresight debug
> driver), the AMBA bus may start to disable them. Because no CPU driver
> owns these clocks (e.g., cpufreq-dt doesn't enable() them -- and even if
> it did, it's not early enough -- nor does arch/arm64/kernel/smp.c), the
> common clock framework then feels the need to disable the parents
> (including the CPU PLLs) -- which is no fun for anyone.
>
> Thus, mark the CPU clocks as critical as well.

I think this part should be done regardless, and could be a separate patch
added before exposing the COREDBG clks.

Either way,

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

> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> Resending, because I missed the mailing lists on the first version.
>
>  drivers/clk/rockchip/clk-rk3399.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
> index 62a4f2543960..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,
> @@ -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));
> +

Looking at the bigger picture, maybe it's time to convert CLK_IGNORE_UNUSED
and rockchip_clk_protect_critical() to CLK_IS_CRITICAL?

ChenYu

>         rockchip_register_softrst(np, 21, reg_base + RK3399_SOFTRST_CON(0),
>                                   ROCKCHIP_SOFTRST_HIWORD_MASK);
>
> --
> 2.33.0.153.gba50c8fa24-goog
>

WARNING: multiple messages have this Message-ID
From: Chen-Yu Tsai <wenst@chromium.org>
To: Brian Norris <briannorris@chromium.org>
Cc: Heiko Stuebner <heiko@sntech.de>,
	Michael Turquette <mturquette@baylibre.com>,
	 Stephen Boyd <sboyd@kernel.org>,
	linux-clk@vger.kernel.org,
	 Douglas Anderson <dianders@chromium.org>,
	LKML <linux-kernel@vger.kernel.org>,
	 linux-rockchip@lists.infradead.org,
	 "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [RESEND PATCH 1/2] clk: rockchip: rk3399: expose PCLK_COREDBG_{B, L}
Date: Wed, 8 Sep 2021 13:00:52 +0800	[thread overview]
Message-ID: <CAGXv+5E1Kk3PYK1GBatfBVnaEVfGCLvb5P-dN9S05LawAfimjw@mail.gmail.com> (raw)
In-Reply-To: <20210907094628.RESEND.1.If29cd838efbcee4450a62b8d84a99b23c86e0a3f@changeid>

On Wed, Sep 8, 2021 at 12:46 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.
>
> In exposing these clocks (and attaching them to the coresight debug
> driver), the AMBA bus may start to disable them. Because no CPU driver
> owns these clocks (e.g., cpufreq-dt doesn't enable() them -- and even if
> it did, it's not early enough -- nor does arch/arm64/kernel/smp.c), the
> common clock framework then feels the need to disable the parents
> (including the CPU PLLs) -- which is no fun for anyone.
>
> Thus, mark the CPU clocks as critical as well.

I think this part should be done regardless, and could be a separate patch
added before exposing the COREDBG clks.

Either way,

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

> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> Resending, because I missed the mailing lists on the first version.
>
>  drivers/clk/rockchip/clk-rk3399.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
> index 62a4f2543960..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,
> @@ -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));
> +

Looking at the bigger picture, maybe it's time to convert CLK_IGNORE_UNUSED
and rockchip_clk_protect_critical() to CLK_IS_CRITICAL?

ChenYu

>         rockchip_register_softrst(np, 21, reg_base + RK3399_SOFTRST_CON(0),
>                                   ROCKCHIP_SOFTRST_HIWORD_MASK);
>
> --
> 2.33.0.153.gba50c8fa24-goog
>

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

WARNING: multiple messages have this Message-ID
From: Chen-Yu Tsai <wenst@chromium.org>
To: Brian Norris <briannorris@chromium.org>
Cc: Heiko Stuebner <heiko@sntech.de>,
	Michael Turquette <mturquette@baylibre.com>,
	 Stephen Boyd <sboyd@kernel.org>,
	linux-clk@vger.kernel.org,
	 Douglas Anderson <dianders@chromium.org>,
	LKML <linux-kernel@vger.kernel.org>,
	 linux-rockchip@lists.infradead.org,
	 "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [RESEND PATCH 1/2] clk: rockchip: rk3399: expose PCLK_COREDBG_{B, L}
Date: Wed, 8 Sep 2021 13:00:52 +0800	[thread overview]
Message-ID: <CAGXv+5E1Kk3PYK1GBatfBVnaEVfGCLvb5P-dN9S05LawAfimjw@mail.gmail.com> (raw)
In-Reply-To: <20210907094628.RESEND.1.If29cd838efbcee4450a62b8d84a99b23c86e0a3f@changeid>

On Wed, Sep 8, 2021 at 12:46 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.
>
> In exposing these clocks (and attaching them to the coresight debug
> driver), the AMBA bus may start to disable them. Because no CPU driver
> owns these clocks (e.g., cpufreq-dt doesn't enable() them -- and even if
> it did, it's not early enough -- nor does arch/arm64/kernel/smp.c), the
> common clock framework then feels the need to disable the parents
> (including the CPU PLLs) -- which is no fun for anyone.
>
> Thus, mark the CPU clocks as critical as well.

I think this part should be done regardless, and could be a separate patch
added before exposing the COREDBG clks.

Either way,

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

> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> Resending, because I missed the mailing lists on the first version.
>
>  drivers/clk/rockchip/clk-rk3399.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
> index 62a4f2543960..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,
> @@ -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));
> +

Looking at the bigger picture, maybe it's time to convert CLK_IGNORE_UNUSED
and rockchip_clk_protect_critical() to CLK_IS_CRITICAL?

ChenYu

>         rockchip_register_softrst(np, 21, reg_base + RK3399_SOFTRST_CON(0),
>                                   ROCKCHIP_SOFTRST_HIWORD_MASK);
>
> --
> 2.33.0.153.gba50c8fa24-goog
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2021-09-08  5:01 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-07 16:46 Brian Norris
2021-09-07 16:46 ` Brian Norris
2021-09-07 16:46 ` Brian Norris
2021-09-07 16:46 ` [RESEND PATCH 2/2] arm64: dts: rockchip: add Coresight debug range for RK3399 Brian Norris
2021-09-07 16:46   ` Brian Norris
2021-09-07 16:46   ` Brian Norris
2021-09-08  2:09   ` Leo Yan
2021-09-08  2:09     ` Leo Yan
2021-09-08  2:09     ` Leo Yan
2021-09-08  5:12   ` Chen-Yu Tsai
2021-09-08  5:12     ` Chen-Yu Tsai
2021-09-08  5:12     ` Chen-Yu Tsai
2021-09-08 15:09   ` Doug Anderson
2021-09-08 15:09     ` Doug Anderson
2021-09-08 15:09     ` Doug Anderson
2021-09-08 18:09     ` Brian Norris
2021-09-08 18:09       ` Brian Norris
2021-09-08 18:09       ` Brian Norris
2021-09-08  5:00 ` Chen-Yu Tsai [this message]
2021-09-08  5:00   ` [RESEND PATCH 1/2] clk: rockchip: rk3399: expose PCLK_COREDBG_{B, L} Chen-Yu Tsai
2021-09-08  5:00   ` Chen-Yu Tsai
2021-09-08  7:32   ` [RESEND PATCH 1/2] clk: rockchip: rk3399: expose PCLK_COREDBG_{B,L} Heiko Stübner
2021-09-08  7:32     ` [RESEND PATCH 1/2] clk: rockchip: rk3399: expose PCLK_COREDBG_{B, L} Heiko Stübner
2021-09-08  7:32     ` Heiko Stübner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAGXv+5E1Kk3PYK1GBatfBVnaEVfGCLvb5P-dN9S05LawAfimjw@mail.gmail.com \
    --to=wenst@chromium.org \
    --cc=briannorris@chromium.org \
    --cc=dianders@chromium.org \
    --cc=heiko@sntech.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mturquette@baylibre.com \
    --cc=sboyd@kernel.org \
    --subject='Re: [RESEND PATCH 1/2] clk: rockchip: rk3399: expose PCLK_COREDBG_{B,L}' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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.