* [PATCH 1/4] clk: qcom: clk-hfpll: use poll_timeout macro
2022-04-29 12:01 [PATCH 0/4] Small fixes/improvement for hfpll and krait Ansuel Smith
@ 2022-04-29 12:01 ` Ansuel Smith
2022-04-29 14:42 ` Dmitry Baryshkov
2022-04-29 12:01 ` [PATCH 2/4] clk: qcom: clk-krait: unlock spin after mux completion Ansuel Smith
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Ansuel Smith @ 2022-04-29 12:01 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Sricharan R, linux-arm-msm, linux-clk, linux-kernel
Cc: Ansuel Smith
Use regmap_read_poll_timeout macro instead of do-while structure to tidy
things up. Also set a timeout to prevent any sort of system stall.
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
drivers/clk/qcom/clk-hfpll.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/clk/qcom/clk-hfpll.c b/drivers/clk/qcom/clk-hfpll.c
index e847d586a73a..7dd17c184b69 100644
--- a/drivers/clk/qcom/clk-hfpll.c
+++ b/drivers/clk/qcom/clk-hfpll.c
@@ -72,13 +72,16 @@ static void __clk_hfpll_enable(struct clk_hw *hw)
regmap_update_bits(regmap, hd->mode_reg, PLL_RESET_N, PLL_RESET_N);
/* Wait for PLL to lock. */
- if (hd->status_reg) {
- do {
- regmap_read(regmap, hd->status_reg, &val);
- } while (!(val & BIT(hd->lock_bit)));
- } else {
+ if (hd->status_reg)
+ /*
+ * Busy wait. Should never timeout, we add a timeout to
+ * prevent any sort of stall.
+ */
+ regmap_read_poll_timeout(regmap, hd->status_reg, val,
+ !(val & BIT(hd->lock_bit)), 0,
+ 100 * USEC_PER_MSEC);
+ else
udelay(60);
- }
/* Enable PLL output. */
regmap_update_bits(regmap, hd->mode_reg, PLL_OUTCTRL, PLL_OUTCTRL);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] clk: qcom: clk-hfpll: use poll_timeout macro
2022-04-29 12:01 ` [PATCH 1/4] clk: qcom: clk-hfpll: use poll_timeout macro Ansuel Smith
@ 2022-04-29 14:42 ` Dmitry Baryshkov
0 siblings, 0 replies; 13+ messages in thread
From: Dmitry Baryshkov @ 2022-04-29 14:42 UTC (permalink / raw)
To: Ansuel Smith, Andy Gross, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Sricharan R, linux-arm-msm, linux-clk,
linux-kernel
On 29/04/2022 15:01, Ansuel Smith wrote:
> Use regmap_read_poll_timeout macro instead of do-while structure to tidy
> things up. Also set a timeout to prevent any sort of system stall.
>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/clk/qcom/clk-hfpll.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clk/qcom/clk-hfpll.c b/drivers/clk/qcom/clk-hfpll.c
> index e847d586a73a..7dd17c184b69 100644
> --- a/drivers/clk/qcom/clk-hfpll.c
> +++ b/drivers/clk/qcom/clk-hfpll.c
> @@ -72,13 +72,16 @@ static void __clk_hfpll_enable(struct clk_hw *hw)
> regmap_update_bits(regmap, hd->mode_reg, PLL_RESET_N, PLL_RESET_N);
>
> /* Wait for PLL to lock. */
> - if (hd->status_reg) {
> - do {
> - regmap_read(regmap, hd->status_reg, &val);
> - } while (!(val & BIT(hd->lock_bit)));
> - } else {
> + if (hd->status_reg)
> + /*
> + * Busy wait. Should never timeout, we add a timeout to
> + * prevent any sort of stall.
> + */
> + regmap_read_poll_timeout(regmap, hd->status_reg, val,
> + !(val & BIT(hd->lock_bit)), 0,
> + 100 * USEC_PER_MSEC);
> + else
> udelay(60);
> - }
>
> /* Enable PLL output. */
> regmap_update_bits(regmap, hd->mode_reg, PLL_OUTCTRL, PLL_OUTCTRL);
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] clk: qcom: clk-krait: unlock spin after mux completion
2022-04-29 12:01 [PATCH 0/4] Small fixes/improvement for hfpll and krait Ansuel Smith
2022-04-29 12:01 ` [PATCH 1/4] clk: qcom: clk-hfpll: use poll_timeout macro Ansuel Smith
@ 2022-04-29 12:01 ` Ansuel Smith
2022-04-29 14:41 ` Dmitry Baryshkov
2022-04-29 12:01 ` [PATCH 3/4] clk: qcom: clk-krait: add hw_parent check for div2_round_rate Ansuel Smith
2022-04-29 12:01 ` [PATCH 4/4] clk: qcom: clk-krait: add apq/ipq8064 errata workaround Ansuel Smith
3 siblings, 1 reply; 13+ messages in thread
From: Ansuel Smith @ 2022-04-29 12:01 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Sricharan R, linux-arm-msm, linux-clk, linux-kernel
Cc: Ansuel Smith
Unlock spinlock after the mux switch is completed to prevent any corner
case of mux request while the switch still needs to be done.
Fixes: 4d7dc77babfe ("clk: qcom: Add support for Krait clocks")
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
drivers/clk/qcom/clk-krait.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
index 59f1af415b58..90046428693c 100644
--- a/drivers/clk/qcom/clk-krait.c
+++ b/drivers/clk/qcom/clk-krait.c
@@ -32,11 +32,16 @@ static void __krait_mux_set_sel(struct krait_mux_clk *mux, int sel)
regval |= (sel & mux->mask) << (mux->shift + LPL_SHIFT);
}
krait_set_l2_indirect_reg(mux->offset, regval);
- spin_unlock_irqrestore(&krait_clock_reg_lock, flags);
/* Wait for switch to complete. */
mb();
udelay(1);
+
+ /*
+ * Unlock now to make sure the mux register is not
+ * modified while switching to the new parent.
+ */
+ spin_unlock_irqrestore(&krait_clock_reg_lock, flags);
}
static int krait_mux_set_parent(struct clk_hw *hw, u8 index)
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] clk: qcom: clk-krait: unlock spin after mux completion
2022-04-29 12:01 ` [PATCH 2/4] clk: qcom: clk-krait: unlock spin after mux completion Ansuel Smith
@ 2022-04-29 14:41 ` Dmitry Baryshkov
0 siblings, 0 replies; 13+ messages in thread
From: Dmitry Baryshkov @ 2022-04-29 14:41 UTC (permalink / raw)
To: Ansuel Smith, Andy Gross, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Sricharan R, linux-arm-msm, linux-clk,
linux-kernel
On 29/04/2022 15:01, Ansuel Smith wrote:
> Unlock spinlock after the mux switch is completed to prevent any corner
> case of mux request while the switch still needs to be done.
>
> Fixes: 4d7dc77babfe ("clk: qcom: Add support for Krait clocks")
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/clk/qcom/clk-krait.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
> index 59f1af415b58..90046428693c 100644
> --- a/drivers/clk/qcom/clk-krait.c
> +++ b/drivers/clk/qcom/clk-krait.c
> @@ -32,11 +32,16 @@ static void __krait_mux_set_sel(struct krait_mux_clk *mux, int sel)
> regval |= (sel & mux->mask) << (mux->shift + LPL_SHIFT);
> }
> krait_set_l2_indirect_reg(mux->offset, regval);
> - spin_unlock_irqrestore(&krait_clock_reg_lock, flags);
>
> /* Wait for switch to complete. */
> mb();
> udelay(1);
> +
> + /*
> + * Unlock now to make sure the mux register is not
> + * modified while switching to the new parent.
> + */
> + spin_unlock_irqrestore(&krait_clock_reg_lock, flags);
> }
>
> static int krait_mux_set_parent(struct clk_hw *hw, u8 index)
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] clk: qcom: clk-krait: add hw_parent check for div2_round_rate
2022-04-29 12:01 [PATCH 0/4] Small fixes/improvement for hfpll and krait Ansuel Smith
2022-04-29 12:01 ` [PATCH 1/4] clk: qcom: clk-hfpll: use poll_timeout macro Ansuel Smith
2022-04-29 12:01 ` [PATCH 2/4] clk: qcom: clk-krait: unlock spin after mux completion Ansuel Smith
@ 2022-04-29 12:01 ` Ansuel Smith
2022-04-29 14:53 ` Dmitry Baryshkov
2022-04-29 12:01 ` [PATCH 4/4] clk: qcom: clk-krait: add apq/ipq8064 errata workaround Ansuel Smith
3 siblings, 1 reply; 13+ messages in thread
From: Ansuel Smith @ 2022-04-29 12:01 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Sricharan R, linux-arm-msm, linux-clk, linux-kernel
Cc: Ansuel Smith
Check if hw_parent is present before calculating the round_rate to
prevent kernel panic. On error -EINVAL is reported.
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
drivers/clk/qcom/clk-krait.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
index 90046428693c..6c367ad6506a 100644
--- a/drivers/clk/qcom/clk-krait.c
+++ b/drivers/clk/qcom/clk-krait.c
@@ -84,7 +84,12 @@ EXPORT_SYMBOL_GPL(krait_mux_clk_ops);
static long krait_div2_round_rate(struct clk_hw *hw, unsigned long rate,
unsigned long *parent_rate)
{
- *parent_rate = clk_hw_round_rate(clk_hw_get_parent(hw), rate * 2);
+ struct clk_hw *hw_parent = clk_hw_get_parent(hw);
+
+ if (!hw_parent)
+ return -EINVAL;
+
+ *parent_rate = clk_hw_round_rate(hw_parent, rate * 2);
return DIV_ROUND_UP(*parent_rate, 2);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] clk: qcom: clk-krait: add hw_parent check for div2_round_rate
2022-04-29 12:01 ` [PATCH 3/4] clk: qcom: clk-krait: add hw_parent check for div2_round_rate Ansuel Smith
@ 2022-04-29 14:53 ` Dmitry Baryshkov
2022-04-29 15:06 ` Ansuel Smith
0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Baryshkov @ 2022-04-29 14:53 UTC (permalink / raw)
To: Ansuel Smith, Andy Gross, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Sricharan R, linux-arm-msm, linux-clk,
linux-kernel
On 29/04/2022 15:01, Ansuel Smith wrote:
> Check if hw_parent is present before calculating the round_rate to
> prevent kernel panic. On error -EINVAL is reported.
>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
I see that other clock drivers do not perform this check. Which path
leads to this oops?
> ---
> drivers/clk/qcom/clk-krait.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
> index 90046428693c..6c367ad6506a 100644
> --- a/drivers/clk/qcom/clk-krait.c
> +++ b/drivers/clk/qcom/clk-krait.c
> @@ -84,7 +84,12 @@ EXPORT_SYMBOL_GPL(krait_mux_clk_ops);
> static long krait_div2_round_rate(struct clk_hw *hw, unsigned long rate,
> unsigned long *parent_rate)
> {
> - *parent_rate = clk_hw_round_rate(clk_hw_get_parent(hw), rate * 2);
> + struct clk_hw *hw_parent = clk_hw_get_parent(hw);
> +
> + if (!hw_parent)
> + return -EINVAL;
> +
> + *parent_rate = clk_hw_round_rate(hw_parent, rate * 2);
> return DIV_ROUND_UP(*parent_rate, 2);
> }
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] clk: qcom: clk-krait: add hw_parent check for div2_round_rate
2022-04-29 14:53 ` Dmitry Baryshkov
@ 2022-04-29 15:06 ` Ansuel Smith
2022-04-29 15:22 ` Dmitry Baryshkov
0 siblings, 1 reply; 13+ messages in thread
From: Ansuel Smith @ 2022-04-29 15:06 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Andy Gross, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Sricharan R, linux-arm-msm, linux-clk, linux-kernel
On Fri, Apr 29, 2022 at 05:53:32PM +0300, Dmitry Baryshkov wrote:
> On 29/04/2022 15:01, Ansuel Smith wrote:
> > Check if hw_parent is present before calculating the round_rate to
> > prevent kernel panic. On error -EINVAL is reported.
> >
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
>
> I see that other clock drivers do not perform this check. Which path leads
> to this oops?
>
This comes from qsdk patches so I apologize in advance about this.
Anyway I'm checking the code and krait-cc is the only user of
krait_div2_clk_ops. That user have as parent only hfpll_something that
is declared by gcc. Now hfpll can also be declared in dts with a
dedicated driver so I wonder if the problem is there in the case when
hfpll is declared in dts and is probed after krait-cc. This is not the
case for ipq8064 but I wonder if qsdk have other krait based device that
have a configuration with hfpll declared in dts.
In short you are right and in our current code the check is uselss and
I'm positive about dropping this patch but I do wonder if downstream
there is an actual use of this. Don't know how to proceed. Any hint?
> > ---
> > drivers/clk/qcom/clk-krait.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
> > index 90046428693c..6c367ad6506a 100644
> > --- a/drivers/clk/qcom/clk-krait.c
> > +++ b/drivers/clk/qcom/clk-krait.c
> > @@ -84,7 +84,12 @@ EXPORT_SYMBOL_GPL(krait_mux_clk_ops);
> > static long krait_div2_round_rate(struct clk_hw *hw, unsigned long rate,
> > unsigned long *parent_rate)
> > {
> > - *parent_rate = clk_hw_round_rate(clk_hw_get_parent(hw), rate * 2);
> > + struct clk_hw *hw_parent = clk_hw_get_parent(hw);
> > +
> > + if (!hw_parent)
> > + return -EINVAL;
> > +
> > + *parent_rate = clk_hw_round_rate(hw_parent, rate * 2);
> > return DIV_ROUND_UP(*parent_rate, 2);
> > }
>
>
> --
> With best wishes
> Dmitry
--
Ansuel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] clk: qcom: clk-krait: add hw_parent check for div2_round_rate
2022-04-29 15:06 ` Ansuel Smith
@ 2022-04-29 15:22 ` Dmitry Baryshkov
0 siblings, 0 replies; 13+ messages in thread
From: Dmitry Baryshkov @ 2022-04-29 15:22 UTC (permalink / raw)
To: Ansuel Smith
Cc: Andy Gross, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Sricharan R, linux-arm-msm, linux-clk, linux-kernel
On Fri, 29 Apr 2022 at 18:08, Ansuel Smith <ansuelsmth@gmail.com> wrote:
>
> On Fri, Apr 29, 2022 at 05:53:32PM +0300, Dmitry Baryshkov wrote:
> > On 29/04/2022 15:01, Ansuel Smith wrote:
> > > Check if hw_parent is present before calculating the round_rate to
> > > prevent kernel panic. On error -EINVAL is reported.
> > >
> > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> >
> > I see that other clock drivers do not perform this check. Which path leads
> > to this oops?
> >
>
> This comes from qsdk patches so I apologize in advance about this.
Ugh. If it comes from the code authored by somebody else, it'd be
better to note this (by using the From or Co-developed-by tags).
At the very least (if the author is unknown) you can mention the
origin of the patch (qsdk) in the commit message.
>
> Anyway I'm checking the code and krait-cc is the only user of
> krait_div2_clk_ops. That user have as parent only hfpll_something that
> is declared by gcc. Now hfpll can also be declared in dts with a
> dedicated driver so I wonder if the problem is there in the case when
> hfpll is declared in dts and is probed after krait-cc. This is not the
> case for ipq8064 but I wonder if qsdk have other krait based device that
> have a configuration with hfpll declared in dts.
On msm8974 (and maybe others) the hfpll should be driven by the
separate hfpll driver.
>
> In short you are right and in our current code the check is uselss and
> I'm positive about dropping this patch but I do wonder if downstream
> there is an actual use of this. Don't know how to proceed. Any hint?
I'd say, let's drop it for now unless Stephen or Bjorn tell us that
it's a valid check.
>
> > > ---
> > > drivers/clk/qcom/clk-krait.c | 7 ++++++-
> > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
> > > index 90046428693c..6c367ad6506a 100644
> > > --- a/drivers/clk/qcom/clk-krait.c
> > > +++ b/drivers/clk/qcom/clk-krait.c
> > > @@ -84,7 +84,12 @@ EXPORT_SYMBOL_GPL(krait_mux_clk_ops);
> > > static long krait_div2_round_rate(struct clk_hw *hw, unsigned long rate,
> > > unsigned long *parent_rate)
> > > {
> > > - *parent_rate = clk_hw_round_rate(clk_hw_get_parent(hw), rate * 2);
> > > + struct clk_hw *hw_parent = clk_hw_get_parent(hw);
> > > +
> > > + if (!hw_parent)
> > > + return -EINVAL;
> > > +
> > > + *parent_rate = clk_hw_round_rate(hw_parent, rate * 2);
> > > return DIV_ROUND_UP(*parent_rate, 2);
> > > }
> >
> >
> > --
> > With best wishes
> > Dmitry
>
> --
> Ansuel
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] clk: qcom: clk-krait: add apq/ipq8064 errata workaround
2022-04-29 12:01 [PATCH 0/4] Small fixes/improvement for hfpll and krait Ansuel Smith
` (2 preceding siblings ...)
2022-04-29 12:01 ` [PATCH 3/4] clk: qcom: clk-krait: add hw_parent check for div2_round_rate Ansuel Smith
@ 2022-04-29 12:01 ` Ansuel Smith
2022-04-29 15:00 ` Dmitry Baryshkov
3 siblings, 1 reply; 13+ messages in thread
From: Ansuel Smith @ 2022-04-29 12:01 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Sricharan R, linux-arm-msm, linux-clk, linux-kernel
Cc: Ansuel Smith
Add apq/ipq8064 errata workaround where the sec_src clock gating needs to
be disabled during switching. To enable this set disable_sec_src_gating
in the mux struct.
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
drivers/clk/qcom/clk-krait.c | 16 ++++++++++++++++
drivers/clk/qcom/clk-krait.h | 1 +
drivers/clk/qcom/krait-cc.c | 1 +
3 files changed, 18 insertions(+)
diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
index 6c367ad6506a..4a9b3296c45b 100644
--- a/drivers/clk/qcom/clk-krait.c
+++ b/drivers/clk/qcom/clk-krait.c
@@ -18,13 +18,23 @@
static DEFINE_SPINLOCK(krait_clock_reg_lock);
#define LPL_SHIFT 8
+#define SECCLKAGD BIT(4)
+
static void __krait_mux_set_sel(struct krait_mux_clk *mux, int sel)
{
unsigned long flags;
u32 regval;
spin_lock_irqsave(&krait_clock_reg_lock, flags);
+
regval = krait_get_l2_indirect_reg(mux->offset);
+
+ /* apq/ipq8064 Errata: disable sec_src clock gating during switch. */
+ if (mux->disable_sec_src_gating) {
+ regval |= SECCLKAGD;
+ krait_set_l2_indirect_reg(mux->offset, regval);
+ }
+
regval &= ~(mux->mask << mux->shift);
regval |= (sel & mux->mask) << mux->shift;
if (mux->lpl) {
@@ -33,6 +43,12 @@ static void __krait_mux_set_sel(struct krait_mux_clk *mux, int sel)
}
krait_set_l2_indirect_reg(mux->offset, regval);
+ /* apq/ipq8064 Errata: re-enabled sec_src clock gating. */
+ if (mux->disable_sec_src_gating) {
+ regval &= ~SECCLKAGD;
+ krait_set_l2_indirect_reg(mux->offset, regval);
+ }
+
/* Wait for switch to complete. */
mb();
udelay(1);
diff --git a/drivers/clk/qcom/clk-krait.h b/drivers/clk/qcom/clk-krait.h
index 9120bd2f5297..f930538c539e 100644
--- a/drivers/clk/qcom/clk-krait.h
+++ b/drivers/clk/qcom/clk-krait.h
@@ -15,6 +15,7 @@ struct krait_mux_clk {
u8 safe_sel;
u8 old_index;
bool reparent;
+ bool disable_sec_src_gating;
struct clk_hw hw;
struct notifier_block clk_nb;
diff --git a/drivers/clk/qcom/krait-cc.c b/drivers/clk/qcom/krait-cc.c
index 4d4b657d33c3..0f88bf41ec6e 100644
--- a/drivers/clk/qcom/krait-cc.c
+++ b/drivers/clk/qcom/krait-cc.c
@@ -138,6 +138,7 @@ krait_add_sec_mux(struct device *dev, int id, const char *s,
mux->parent_map = sec_mux_map;
mux->hw.init = &init;
mux->safe_sel = 0;
+ mux->disable_sec_src_gating = true;
init.name = kasprintf(GFP_KERNEL, "krait%s_sec_mux", s);
if (!init.name)
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] clk: qcom: clk-krait: add apq/ipq8064 errata workaround
2022-04-29 12:01 ` [PATCH 4/4] clk: qcom: clk-krait: add apq/ipq8064 errata workaround Ansuel Smith
@ 2022-04-29 15:00 ` Dmitry Baryshkov
2022-04-29 15:09 ` Ansuel Smith
0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Baryshkov @ 2022-04-29 15:00 UTC (permalink / raw)
To: Ansuel Smith, Andy Gross, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Sricharan R, linux-arm-msm, linux-clk,
linux-kernel
On 29/04/2022 15:01, Ansuel Smith wrote:
> Add apq/ipq8064 errata workaround where the sec_src clock gating needs to
> be disabled during switching. To enable this set disable_sec_src_gating
> in the mux struct.
>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
> drivers/clk/qcom/clk-krait.c | 16 ++++++++++++++++
> drivers/clk/qcom/clk-krait.h | 1 +
> drivers/clk/qcom/krait-cc.c | 1 +
> 3 files changed, 18 insertions(+)
>
> diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
> index 6c367ad6506a..4a9b3296c45b 100644
> --- a/drivers/clk/qcom/clk-krait.c
> +++ b/drivers/clk/qcom/clk-krait.c
> @@ -18,13 +18,23 @@
> static DEFINE_SPINLOCK(krait_clock_reg_lock);
>
> #define LPL_SHIFT 8
> +#define SECCLKAGD BIT(4)
> +
> static void __krait_mux_set_sel(struct krait_mux_clk *mux, int sel)
> {
> unsigned long flags;
> u32 regval;
>
> spin_lock_irqsave(&krait_clock_reg_lock, flags);
> +
> regval = krait_get_l2_indirect_reg(mux->offset);
> +
> + /* apq/ipq8064 Errata: disable sec_src clock gating during switch. */
> + if (mux->disable_sec_src_gating) {
> + regval |= SECCLKAGD;
> + krait_set_l2_indirect_reg(mux->offset, regval);
> + }
> +
> regval &= ~(mux->mask << mux->shift);
> regval |= (sel & mux->mask) << mux->shift;
> if (mux->lpl) {
> @@ -33,6 +43,12 @@ static void __krait_mux_set_sel(struct krait_mux_clk *mux, int sel)
> }
> krait_set_l2_indirect_reg(mux->offset, regval);
>
> + /* apq/ipq8064 Errata: re-enabled sec_src clock gating. */
> + if (mux->disable_sec_src_gating) {
> + regval &= ~SECCLKAGD;
> + krait_set_l2_indirect_reg(mux->offset, regval);
> + }
> +
> /* Wait for switch to complete. */
> mb();
> udelay(1);
> diff --git a/drivers/clk/qcom/clk-krait.h b/drivers/clk/qcom/clk-krait.h
> index 9120bd2f5297..f930538c539e 100644
> --- a/drivers/clk/qcom/clk-krait.h
> +++ b/drivers/clk/qcom/clk-krait.h
> @@ -15,6 +15,7 @@ struct krait_mux_clk {
> u8 safe_sel;
> u8 old_index;
> bool reparent;
> + bool disable_sec_src_gating;
>
> struct clk_hw hw;
> struct notifier_block clk_nb;
> diff --git a/drivers/clk/qcom/krait-cc.c b/drivers/clk/qcom/krait-cc.c
> index 4d4b657d33c3..0f88bf41ec6e 100644
> --- a/drivers/clk/qcom/krait-cc.c
> +++ b/drivers/clk/qcom/krait-cc.c
> @@ -138,6 +138,7 @@ krait_add_sec_mux(struct device *dev, int id, const char *s,
> mux->parent_map = sec_mux_map;
> mux->hw.init = &init;
> mux->safe_sel = 0;
> + mux->disable_sec_src_gating = true;
This has to be guarded with the of_compatible checks. Otherwise you'd
apply this errata to all Krait CPUs, not only apq/ipq8064.
At least this should be limited to krait-cc-v1 with the note that there
is no way to distinguish between platforms.
>
> init.name = kasprintf(GFP_KERNEL, "krait%s_sec_mux", s);
> if (!init.name)
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] clk: qcom: clk-krait: add apq/ipq8064 errata workaround
2022-04-29 15:00 ` Dmitry Baryshkov
@ 2022-04-29 15:09 ` Ansuel Smith
2022-04-29 15:13 ` Dmitry Baryshkov
0 siblings, 1 reply; 13+ messages in thread
From: Ansuel Smith @ 2022-04-29 15:09 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Andy Gross, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Sricharan R, linux-arm-msm, linux-clk, linux-kernel
On Fri, Apr 29, 2022 at 06:00:45PM +0300, Dmitry Baryshkov wrote:
> On 29/04/2022 15:01, Ansuel Smith wrote:
> > Add apq/ipq8064 errata workaround where the sec_src clock gating needs to
> > be disabled during switching. To enable this set disable_sec_src_gating
> > in the mux struct.
> >
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> > drivers/clk/qcom/clk-krait.c | 16 ++++++++++++++++
> > drivers/clk/qcom/clk-krait.h | 1 +
> > drivers/clk/qcom/krait-cc.c | 1 +
> > 3 files changed, 18 insertions(+)
> >
> > diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
> > index 6c367ad6506a..4a9b3296c45b 100644
> > --- a/drivers/clk/qcom/clk-krait.c
> > +++ b/drivers/clk/qcom/clk-krait.c
> > @@ -18,13 +18,23 @@
> > static DEFINE_SPINLOCK(krait_clock_reg_lock);
> > #define LPL_SHIFT 8
> > +#define SECCLKAGD BIT(4)
> > +
> > static void __krait_mux_set_sel(struct krait_mux_clk *mux, int sel)
> > {
> > unsigned long flags;
> > u32 regval;
> > spin_lock_irqsave(&krait_clock_reg_lock, flags);
> > +
> > regval = krait_get_l2_indirect_reg(mux->offset);
> > +
> > + /* apq/ipq8064 Errata: disable sec_src clock gating during switch. */
> > + if (mux->disable_sec_src_gating) {
> > + regval |= SECCLKAGD;
> > + krait_set_l2_indirect_reg(mux->offset, regval);
> > + }
> > +
> > regval &= ~(mux->mask << mux->shift);
> > regval |= (sel & mux->mask) << mux->shift;
> > if (mux->lpl) {
> > @@ -33,6 +43,12 @@ static void __krait_mux_set_sel(struct krait_mux_clk *mux, int sel)
> > }
> > krait_set_l2_indirect_reg(mux->offset, regval);
> > + /* apq/ipq8064 Errata: re-enabled sec_src clock gating. */
> > + if (mux->disable_sec_src_gating) {
> > + regval &= ~SECCLKAGD;
> > + krait_set_l2_indirect_reg(mux->offset, regval);
> > + }
> > +
> > /* Wait for switch to complete. */
> > mb();
> > udelay(1);
> > diff --git a/drivers/clk/qcom/clk-krait.h b/drivers/clk/qcom/clk-krait.h
> > index 9120bd2f5297..f930538c539e 100644
> > --- a/drivers/clk/qcom/clk-krait.h
> > +++ b/drivers/clk/qcom/clk-krait.h
> > @@ -15,6 +15,7 @@ struct krait_mux_clk {
> > u8 safe_sel;
> > u8 old_index;
> > bool reparent;
> > + bool disable_sec_src_gating;
> > struct clk_hw hw;
> > struct notifier_block clk_nb;
> > diff --git a/drivers/clk/qcom/krait-cc.c b/drivers/clk/qcom/krait-cc.c
> > index 4d4b657d33c3..0f88bf41ec6e 100644
> > --- a/drivers/clk/qcom/krait-cc.c
> > +++ b/drivers/clk/qcom/krait-cc.c
> > @@ -138,6 +138,7 @@ krait_add_sec_mux(struct device *dev, int id, const char *s,
> > mux->parent_map = sec_mux_map;
> > mux->hw.init = &init;
> > mux->safe_sel = 0;
> > + mux->disable_sec_src_gating = true;
>
> This has to be guarded with the of_compatible checks. Otherwise you'd apply
> this errata to all Krait CPUs, not only apq/ipq8064.
>
> At least this should be limited to krait-cc-v1 with the note that there is
> no way to distinguish between platforms.
>
Mhh can't i check the machine compatible directly to limit this to
apq/ipq8064?
> > init.name = kasprintf(GFP_KERNEL, "krait%s_sec_mux", s);
> > if (!init.name)
>
>
> --
> With best wishes
> Dmitry
--
Ansuel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] clk: qcom: clk-krait: add apq/ipq8064 errata workaround
2022-04-29 15:09 ` Ansuel Smith
@ 2022-04-29 15:13 ` Dmitry Baryshkov
0 siblings, 0 replies; 13+ messages in thread
From: Dmitry Baryshkov @ 2022-04-29 15:13 UTC (permalink / raw)
To: Ansuel Smith
Cc: Andy Gross, Bjorn Andersson, Michael Turquette, Stephen Boyd,
linux-arm-msm, linux-clk, linux-kernel
On Fri, 29 Apr 2022 at 18:11, Ansuel Smith <ansuelsmth@gmail.com> wrote:
>
> On Fri, Apr 29, 2022 at 06:00:45PM +0300, Dmitry Baryshkov wrote:
> > On 29/04/2022 15:01, Ansuel Smith wrote:
> > > Add apq/ipq8064 errata workaround where the sec_src clock gating needs to
> > > be disabled during switching. To enable this set disable_sec_src_gating
> > > in the mux struct.
> > >
> > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > > ---
> > > drivers/clk/qcom/clk-krait.c | 16 ++++++++++++++++
> > > drivers/clk/qcom/clk-krait.h | 1 +
> > > drivers/clk/qcom/krait-cc.c | 1 +
> > > 3 files changed, 18 insertions(+)
> > >
> > > diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
> > > index 6c367ad6506a..4a9b3296c45b 100644
> > > --- a/drivers/clk/qcom/clk-krait.c
> > > +++ b/drivers/clk/qcom/clk-krait.c
> > > @@ -18,13 +18,23 @@
> > > static DEFINE_SPINLOCK(krait_clock_reg_lock);
> > > #define LPL_SHIFT 8
> > > +#define SECCLKAGD BIT(4)
> > > +
> > > static void __krait_mux_set_sel(struct krait_mux_clk *mux, int sel)
> > > {
> > > unsigned long flags;
> > > u32 regval;
> > > spin_lock_irqsave(&krait_clock_reg_lock, flags);
> > > +
> > > regval = krait_get_l2_indirect_reg(mux->offset);
> > > +
> > > + /* apq/ipq8064 Errata: disable sec_src clock gating during switch. */
> > > + if (mux->disable_sec_src_gating) {
> > > + regval |= SECCLKAGD;
> > > + krait_set_l2_indirect_reg(mux->offset, regval);
> > > + }
> > > +
> > > regval &= ~(mux->mask << mux->shift);
> > > regval |= (sel & mux->mask) << mux->shift;
> > > if (mux->lpl) {
> > > @@ -33,6 +43,12 @@ static void __krait_mux_set_sel(struct krait_mux_clk *mux, int sel)
> > > }
> > > krait_set_l2_indirect_reg(mux->offset, regval);
> > > + /* apq/ipq8064 Errata: re-enabled sec_src clock gating. */
> > > + if (mux->disable_sec_src_gating) {
> > > + regval &= ~SECCLKAGD;
> > > + krait_set_l2_indirect_reg(mux->offset, regval);
> > > + }
> > > +
> > > /* Wait for switch to complete. */
> > > mb();
> > > udelay(1);
> > > diff --git a/drivers/clk/qcom/clk-krait.h b/drivers/clk/qcom/clk-krait.h
> > > index 9120bd2f5297..f930538c539e 100644
> > > --- a/drivers/clk/qcom/clk-krait.h
> > > +++ b/drivers/clk/qcom/clk-krait.h
> > > @@ -15,6 +15,7 @@ struct krait_mux_clk {
> > > u8 safe_sel;
> > > u8 old_index;
> > > bool reparent;
> > > + bool disable_sec_src_gating;
> > > struct clk_hw hw;
> > > struct notifier_block clk_nb;
> > > diff --git a/drivers/clk/qcom/krait-cc.c b/drivers/clk/qcom/krait-cc.c
> > > index 4d4b657d33c3..0f88bf41ec6e 100644
> > > --- a/drivers/clk/qcom/krait-cc.c
> > > +++ b/drivers/clk/qcom/krait-cc.c
> > > @@ -138,6 +138,7 @@ krait_add_sec_mux(struct device *dev, int id, const char *s,
> > > mux->parent_map = sec_mux_map;
> > > mux->hw.init = &init;
> > > mux->safe_sel = 0;
> > > + mux->disable_sec_src_gating = true;
> >
> > This has to be guarded with the of_compatible checks. Otherwise you'd apply
> > this errata to all Krait CPUs, not only apq/ipq8064.
> >
> > At least this should be limited to krait-cc-v1 with the note that there is
> > no way to distinguish between platforms.
> >
>
> Mhh can't i check the machine compatible directly to limit this to
> apq/ipq8064?
Yes, use of_machine_is_compatible(). Add a comment that you have to do
this because of the lack of soc-specific device compats.
> > > init.name = kasprintf(GFP_KERNEL, "krait%s_sec_mux", s);
> > > if (!init.name)
> >
> >
> > --
> > With best wishes
> > Dmitry
>
> --
> Ansuel
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 13+ messages in thread