All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Heiko Stübner" <heiko@sntech.de>
To: Brian Norris <briannorris@chromium.org>,
	Chen-Yu Tsai <wenst@chromium.org>
Cc: 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, 08 Sep 2021 09:32:29 +0200	[thread overview]
Message-ID: <4746858.5FAZtBfuxM@diego> (raw)
In-Reply-To: <CAGXv+5E1Kk3PYK1GBatfBVnaEVfGCLvb5P-dN9S05LawAfimjw@mail.gmail.com>

Am Mittwoch, 8. September 2021, 07:00:52 CEST schrieb Chen-Yu Tsai:
> 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.

yep ... especially as this is sort of hidden right now, with it not
being part of the patch subject but only appearing as the last
line of the patch message ;-)

So I'd also vote for a separate patch.


Heiko

> 
> 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: "Heiko Stübner" <heiko@sntech.de>
To: Brian Norris <briannorris@chromium.org>,
	Chen-Yu Tsai <wenst@chromium.org>
Cc: 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, 08 Sep 2021 09:32:29 +0200	[thread overview]
Message-ID: <4746858.5FAZtBfuxM@diego> (raw)
In-Reply-To: <CAGXv+5E1Kk3PYK1GBatfBVnaEVfGCLvb5P-dN9S05LawAfimjw@mail.gmail.com>

Am Mittwoch, 8. September 2021, 07:00:52 CEST schrieb Chen-Yu Tsai:
> 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.

yep ... especially as this is sort of hidden right now, with it not
being part of the patch subject but only appearing as the last
line of the patch message ;-)

So I'd also vote for a separate patch.


Heiko

> 
> 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: "Heiko Stübner" <heiko@sntech.de>
To: Brian Norris <briannorris@chromium.org>,
	Chen-Yu Tsai <wenst@chromium.org>
Cc: 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, 08 Sep 2021 09:32:29 +0200	[thread overview]
Message-ID: <4746858.5FAZtBfuxM@diego> (raw)
In-Reply-To: <CAGXv+5E1Kk3PYK1GBatfBVnaEVfGCLvb5P-dN9S05LawAfimjw@mail.gmail.com>

Am Mittwoch, 8. September 2021, 07:00:52 CEST schrieb Chen-Yu Tsai:
> 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.

yep ... especially as this is sort of hidden right now, with it not
being part of the patch subject but only appearing as the last
line of the patch message ;-)

So I'd also vote for a separate patch.


Heiko

> 
> 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

  reply	other threads:[~2021-09-08  7:32 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 ` [RESEND PATCH 1/2] clk: rockchip: rk3399: expose PCLK_COREDBG_{B,L} Chen-Yu Tsai
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   ` Heiko Stübner [this message]
2021-09-08  7:32     ` 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=4746858.5FAZtBfuxM@diego \
    --to=heiko@sntech.de \
    --cc=briannorris@chromium.org \
    --cc=dianders@chromium.org \
    --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 \
    --cc=wenst@chromium.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.