| Message ID | 20220313190419.2207-13-ansuelsmth@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | None | expand |
Quoting Ansuel Smith (2022-03-14 05:43:20) > On Mon, Mar 14, 2022 at 11:20:21AM +0300, Dmitry Baryshkov wrote: > > On 13/03/2022 22:04, Ansuel Smith wrote: > > > Add 8064 errata workaround where the sec_src clock gating needs to be > > > > Could you please be more specific whether the errata applies only to the > > ipq8064 or to the apq8064 too? 8064 is not specific enough. > > > > That's a good question... Problem is that we really don't know the > answer. This errata comes from qsdk on an old sourcecode. I assume this > is specific to ipq8064 and apq8064 have different mux configuration. > I think it was some glitch that happened when the automatic clk gating was enabled during a switch. The automatic clk gating didn't know that software was running and switching the input so it killed the CPU and stopped the clk. That lead to hangs and super badness. I assume it was applicable to apq8064 as well because ipq8064 is basically apq8064 with the multimedia subsystem replaced by the networking subsystem. Also I wouldn't remember all these details because I worked on apq8064 but not so much on ipq8064 :)
On Tue, Mar 15, 2022 at 02:34:30PM -0700, Stephen Boyd wrote: > Quoting Ansuel Smith (2022-03-14 05:43:20) > > On Mon, Mar 14, 2022 at 11:20:21AM +0300, Dmitry Baryshkov wrote: > > > On 13/03/2022 22:04, Ansuel Smith wrote: > > > > Add 8064 errata workaround where the sec_src clock gating needs to be > > > > > > Could you please be more specific whether the errata applies only to the > > > ipq8064 or to the apq8064 too? 8064 is not specific enough. > > > > > > > That's a good question... Problem is that we really don't know the > > answer. This errata comes from qsdk on an old sourcecode. I assume this > > is specific to ipq8064 and apq8064 have different mux configuration. > > > > I think it was some glitch that happened when the automatic clk gating > was enabled during a switch. The automatic clk gating didn't know that > software was running and switching the input so it killed the CPU and > stopped the clk. That lead to hangs and super badness. I assume it was > applicable to apq8064 as well because ipq8064 is basically apq8064 with > the multimedia subsystem replaced by the networking subsystem. Also I > wouldn't remember all these details because I worked on apq8064 but not > so much on ipq8064 :) Honest question. Do you remember other glitch present on the platform? We are trying to bisect an instability problem and we still needs to find the reason. We really can't understand if it's just a power delivery problem or a scaling problem from muxes or other things. The current problem is that after some time the device kernel panics with a number of strange reason like invalid kernel paging and other strange (or the device just freze and reboots, not even a crash log) Many kernel panics reports the crash near the mux switch (like random error right before the mux switch) So I suspect there is a problem there. But due to the fact that is very random we have NO exact way to repro it. I manage sometime, while playing with the code, to repo similar kernel crash but still i'm not sure of the real cause. I know it's OT but do you have any idea about it? If you remember anything about it? (To scale the freq i'm using a dedicated cpufreq driver that works this way: - We first scale the cache to the max freq across all core, we set the voltage - We scale the cpu to the correct target. This is all done under a lock. Do you see anything wrong in this logic? To mee these random crash looks to be really related to something wrong with the mux or with the cache set to a wrong state) Thx for any suggestion about this. (also I will update this commit and mention both apq and ipq in the comments)
Quoting Ansuel Smith (2022-03-15 14:47:56) > On Tue, Mar 15, 2022 at 02:34:30PM -0700, Stephen Boyd wrote: > > Quoting Ansuel Smith (2022-03-14 05:43:20) > > > On Mon, Mar 14, 2022 at 11:20:21AM +0300, Dmitry Baryshkov wrote: > > > > On 13/03/2022 22:04, Ansuel Smith wrote: > > > > > Add 8064 errata workaround where the sec_src clock gating needs to be > > > > > > > > Could you please be more specific whether the errata applies only to the > > > > ipq8064 or to the apq8064 too? 8064 is not specific enough. > > > > > > > > > > That's a good question... Problem is that we really don't know the > > > answer. This errata comes from qsdk on an old sourcecode. I assume this > > > is specific to ipq8064 and apq8064 have different mux configuration. > > > > > > > I think it was some glitch that happened when the automatic clk gating > > was enabled during a switch. The automatic clk gating didn't know that > > software was running and switching the input so it killed the CPU and > > stopped the clk. That lead to hangs and super badness. I assume it was > > applicable to apq8064 as well because ipq8064 is basically apq8064 with > > the multimedia subsystem replaced by the networking subsystem. Also I > > wouldn't remember all these details because I worked on apq8064 but not > > so much on ipq8064 :) > > Honest question. Do you remember other glitch present on the platform? > We are trying to bisect an instability problem and we still needs to > find the reason. We really can't understand if it's just a power > delivery problem or a scaling problem from muxes or other things. > > The current problem is that after some time the device kernel panics > with a number of strange reason like invalid kernel paging and other > strange (or the device just freze and reboots, not even a crash log) > Many kernel panics reports the crash near the mux switch (like random > error right before the mux switch) So I suspect there is a problem > there. But due to the fact that is very random we have NO exact way to > repro it. I manage sometime, while playing with the code, to repo > similar kernel crash but still i'm not sure of the real cause. > > I know it's OT but do you have any idea about it? If you remember > anything about it? > (To scale the freq i'm using a dedicated cpufreq driver that works this > way: > - We first scale the cache to the max freq across all core, we set the > voltage > - We scale the cpu to the correct target. > This is all done under a lock. Do you see anything wrong in this logic? I honestly don't remember much anymore about this. It's been a decade. Scaling the cache used to be an independent clk and operation vs. the CPU. Basically the clk domain and power domain for the cache was separate from the CPU. There's also the fuse stuff that means you have to read the fuse to know what OPP table to use. Otherwise you may be overclocking the CPU or undervolting it. It may also be that cpuidle can't happen during a frequency transition. Otherwise the clk gating will be reenabled when the cpu startup code reinitializes all the cpu registers? I'd have to look through some old vendor kernels to see if anything jogs my memory. > To mee these random crash looks to be really related to something wrong > with the mux or with the cache set to a wrong state) > > Thx for any suggestion about this. > (also I will update this commit and mention both apq and ipq in the > comments)
On Tue, Mar 15, 2022 at 03:41:14PM -0700, Stephen Boyd wrote: > Quoting Ansuel Smith (2022-03-15 14:47:56) > > On Tue, Mar 15, 2022 at 02:34:30PM -0700, Stephen Boyd wrote: > > > Quoting Ansuel Smith (2022-03-14 05:43:20) > > > > On Mon, Mar 14, 2022 at 11:20:21AM +0300, Dmitry Baryshkov wrote: > > > > > On 13/03/2022 22:04, Ansuel Smith wrote: > > > > > > Add 8064 errata workaround where the sec_src clock gating needs to be > > > > > > > > > > Could you please be more specific whether the errata applies only to the > > > > > ipq8064 or to the apq8064 too? 8064 is not specific enough. > > > > > > > > > > > > > That's a good question... Problem is that we really don't know the > > > > answer. This errata comes from qsdk on an old sourcecode. I assume this > > > > is specific to ipq8064 and apq8064 have different mux configuration. > > > > > > > > > > I think it was some glitch that happened when the automatic clk gating > > > was enabled during a switch. The automatic clk gating didn't know that > > > software was running and switching the input so it killed the CPU and > > > stopped the clk. That lead to hangs and super badness. I assume it was > > > applicable to apq8064 as well because ipq8064 is basically apq8064 with > > > the multimedia subsystem replaced by the networking subsystem. Also I > > > wouldn't remember all these details because I worked on apq8064 but not > > > so much on ipq8064 :) > > > > Honest question. Do you remember other glitch present on the platform? > > We are trying to bisect an instability problem and we still needs to > > find the reason. We really can't understand if it's just a power > > delivery problem or a scaling problem from muxes or other things. > > > > The current problem is that after some time the device kernel panics > > with a number of strange reason like invalid kernel paging and other > > strange (or the device just freze and reboots, not even a crash log) > > Many kernel panics reports the crash near the mux switch (like random > > error right before the mux switch) So I suspect there is a problem > > there. But due to the fact that is very random we have NO exact way to > > repro it. I manage sometime, while playing with the code, to repo > > similar kernel crash but still i'm not sure of the real cause. > > > > I know it's OT but do you have any idea about it? If you remember > > anything about it? > > (To scale the freq i'm using a dedicated cpufreq driver that works this > > way: > > - We first scale the cache to the max freq across all core, we set the > > voltage > > - We scale the cpu to the correct target. > > This is all done under a lock. Do you see anything wrong in this logic? > > I honestly don't remember much anymore about this. It's been a decade. > Scaling the cache used to be an independent clk and operation vs. the > CPU. Basically the clk domain and power domain for the cache was > separate from the CPU. There's also the fuse stuff that means you have > to read the fuse to know what OPP table to use. Otherwise you may be > overclocking the CPU or undervolting it. It may also be that cpuidle > can't happen during a frequency transition. Otherwise the clk gating > will be reenabled when the cpu startup code reinitializes all the cpu > registers? I'd have to look through some old vendor kernels to see if > anything jogs my memory. > > > To mee these random crash looks to be really related to something wrong > > with the mux or with the cache set to a wrong state) > > > > Thx for any suggestion about this. > > (also I will update this commit and mention both apq and ipq in the > > comments) Hi, i'm checking the spm qcom idle driver and something doesn't look right to me... Aside from the different sequence used for boot cpu and the abset l2 sequence, it looks like to me that WFI is enabled anyway (even if it's not defined in the DTS or set disabled) and on top of that it looks like we overwrite the WFI logic but we actually set to enter power collapse (spc). Why? Also I think we are missing the assembly code to enter wfi on krait cpu. Am I totally confused or there are some problems in the code that nobody notice?
Quoting Ansuel Smith (2022-03-16 08:46:54) > On Tue, Mar 15, 2022 at 03:41:14PM -0700, Stephen Boyd wrote: > > Quoting Ansuel Smith (2022-03-15 14:47:56) > > > On Tue, Mar 15, 2022 at 02:34:30PM -0700, Stephen Boyd wrote: > > > > Quoting Ansuel Smith (2022-03-14 05:43:20) > > > > > On Mon, Mar 14, 2022 at 11:20:21AM +0300, Dmitry Baryshkov wrote: > > > > > > On 13/03/2022 22:04, Ansuel Smith wrote: > > > > > > > Add 8064 errata workaround where the sec_src clock gating needs to be > > > > > > > > > > > > Could you please be more specific whether the errata applies only to the > > > > > > ipq8064 or to the apq8064 too? 8064 is not specific enough. > > > > > > > > > > > > > > > > That's a good question... Problem is that we really don't know the > > > > > answer. This errata comes from qsdk on an old sourcecode. I assume this > > > > > is specific to ipq8064 and apq8064 have different mux configuration. > > > > > > > > > > > > > I think it was some glitch that happened when the automatic clk gating > > > > was enabled during a switch. The automatic clk gating didn't know that > > > > software was running and switching the input so it killed the CPU and > > > > stopped the clk. That lead to hangs and super badness. I assume it was > > > > applicable to apq8064 as well because ipq8064 is basically apq8064 with > > > > the multimedia subsystem replaced by the networking subsystem. Also I > > > > wouldn't remember all these details because I worked on apq8064 but not > > > > so much on ipq8064 :) > > > > > > Honest question. Do you remember other glitch present on the platform? > > > We are trying to bisect an instability problem and we still needs to > > > find the reason. We really can't understand if it's just a power > > > delivery problem or a scaling problem from muxes or other things. > > > > > > The current problem is that after some time the device kernel panics > > > with a number of strange reason like invalid kernel paging and other > > > strange (or the device just freze and reboots, not even a crash log) > > > Many kernel panics reports the crash near the mux switch (like random > > > error right before the mux switch) So I suspect there is a problem > > > there. But due to the fact that is very random we have NO exact way to > > > repro it. I manage sometime, while playing with the code, to repo > > > similar kernel crash but still i'm not sure of the real cause. > > > > > > I know it's OT but do you have any idea about it? If you remember > > > anything about it? > > > (To scale the freq i'm using a dedicated cpufreq driver that works this > > > way: > > > - We first scale the cache to the max freq across all core, we set the > > > voltage > > > - We scale the cpu to the correct target. > > > This is all done under a lock. Do you see anything wrong in this logic? > > > > I honestly don't remember much anymore about this. It's been a decade. > > Scaling the cache used to be an independent clk and operation vs. the > > CPU. Basically the clk domain and power domain for the cache was > > separate from the CPU. There's also the fuse stuff that means you have > > to read the fuse to know what OPP table to use. Otherwise you may be > > overclocking the CPU or undervolting it. It may also be that cpuidle > > can't happen during a frequency transition. Otherwise the clk gating > > will be reenabled when the cpu startup code reinitializes all the cpu > > registers? I'd have to look through some old vendor kernels to see if > > anything jogs my memory. > > > > > To mee these random crash looks to be really related to something wrong > > > with the mux or with the cache set to a wrong state) > > > > > > Thx for any suggestion about this. > > > (also I will update this commit and mention both apq and ipq in the > > > comments) > > Hi, i'm checking the spm qcom idle driver and something doesn't look > right to me... Aside from the different sequence used for boot cpu and > the abset l2 sequence, it looks like to me that WFI is enabled anyway > (even if it's not defined in the DTS or set disabled) and on top of that > it looks like we overwrite the WFI logic but we actually set to > enter power collapse (spc). Why? When the CPU is power collapsed they need to notify software running in the secure world that the CPU is going to be reset. The CPU comes out of reset in secure mode and it has to jump to non-secure mode. It's still a WFI, but we don't see it in the kernel because the secure world code executes the wfi and that runs the power collapse sequence to turn all the power off. On power up the secure world will restore various cpu registers (*cough* workarounds *cough*) and then switch to non-secure mode wherever linux told it to execute at on warm boot. > > Also I think we are missing the assembly code to enter wfi on krait cpu. > Am I totally confused or there are some problems in the code that nobody > notice? > I'd expect that to run through some scm_call() path into the secure world. The wfi can still be run by the kernel in non-secure mode, but that will only gate the CPU clk and not actually power collapse the core. It's a "light sleep" for the CPU. All this stuff predates PSCI but it is very similar, just a bespoke solution instead of a standard calling format.
diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c index d8af281eba0e..82fe7031e1f4 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); + + /* 8064 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); + /* 8064 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 1bdc89c097e6..533a770332be 100644 --- a/drivers/clk/qcom/krait-cc.c +++ b/drivers/clk/qcom/krait-cc.c @@ -154,6 +154,7 @@ krait_add_sec_mux(struct device *dev, struct clk *qsb, int id, mux->shift = 2; mux->parent_map = sec_mux_map; mux->hw.init = &init; + mux->disable_sec_src_gating = true; init.name = kasprintf(GFP_KERNEL, "krait%s_sec_mux", s); if (!init.name)
Add 8064 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(+)