AudioMix BLK-CTRL on i.MX8MP encountered an accessing register issue
after power up.
[ 2.181035] Kernel panic - not syncing: Asynchronous SError Interrupt
[ 2.181038] CPU: 1 PID: 48 Comm: kworker/u16:2 Not tainted 6.9.0-rc5-next-20240424-00003-g21cec88845c6 #171
[ 2.181047] Hardware name: NXP i.MX8MPlus EVK board (DT)
[ 2.181050] Workqueue: events_unbound deferred_probe_work_func
[ 2.181064] Call trace:
[...]
[ 2.181142] arm64_serror_panic+0x6c/0x78
[ 2.181149] do_serror+0x3c/0x70
[ 2.181157] el1h_64_error_handler+0x30/0x48
[ 2.181164] el1h_64_error+0x64/0x68
[ 2.181171] clk_imx8mp_audiomix_runtime_resume+0x34/0x44
[ 2.181183] __genpd_runtime_resume+0x30/0x80
[ 2.181195] genpd_runtime_resume+0x110/0x244
[ 2.181205] __rpm_callback+0x48/0x1d8
[ 2.181213] rpm_callback+0x68/0x74
[ 2.181224] rpm_resume+0x468/0x6c0
[ 2.181234] __pm_runtime_resume+0x50/0x94
[ 2.181243] pm_runtime_get_suppliers+0x60/0x8c
[ 2.181258] __driver_probe_device+0x48/0x12c
[ 2.181268] driver_probe_device+0xd8/0x15c
[ 2.181278] __device_attach_driver+0xb8/0x134
[ 2.181290] bus_for_each_drv+0x84/0xe0
[ 2.181302] __device_attach+0x9c/0x188
[ 2.181312] device_initial_probe+0x14/0x20
[ 2.181323] bus_probe_device+0xac/0xb0
[ 2.181334] deferred_probe_work_func+0x88/0xc0
[ 2.181344] process_one_work+0x150/0x290
[ 2.181357] worker_thread+0x2f8/0x408
[ 2.181370] kthread+0x110/0x114
[ 2.181381] ret_from_fork+0x10/0x20
[ 2.181391] SMP: stopping secondary CPUs
According to comments in power up handshake:
/* request the ADB400 to power up */
if (domain->bits.hskreq) {
regmap_update_bits(domain->regmap, domain->regs->hsk,
domain->bits.hskreq, domain->bits.hskreq);
/*
* ret = regmap_read_poll_timeout(domain->regmap, domain->regs->hsk, reg_val,
* (reg_val & domain->bits.hskack), 0,
* USEC_PER_MSEC);
* Technically we need the commented code to wait handshake. But that needs
* the BLK-CTL module BUS clk-en bit being set.
*
* There is a separate BLK-CTL module and we will have such a driver for it,
* that driver will set the BUS clk-en bit and handshake will be triggered
* automatically there. Just add a delay and suppose the handshake finish
* after that.
*/
}
The BLK-CTL module needs to add delay to wait for a handshake request finished.
For some BLK-CTL module (eg. AudioMix on i.MX8MP) doesn't have BUS clk-en
bit, it is better to add delay in this driver, as the BLK-CTL module doesn't
need to care about how it is powered up.
regmap_read_bypassed() is to make sure the above write IO transaction already
reaches target before udelay().
Fixes: 1496dd413b2e ("clk: imx: imx8mp: Add pm_runtime support for power saving")
Reported-by: Francesco Dolcini <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]/
Suggested-by: Frank Li <[email protected]>
Signed-off-by: Shengjiu Wang <[email protected]>
---
changes in v3:
- move change to gpcv2.c, as it is more reasonable to let power driver
to handle such power issue, suggested by Frank Li
changes in v2:
- reduce size of panic log in commit message
drivers/pmdomain/imx/gpcv2.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/pmdomain/imx/gpcv2.c b/drivers/pmdomain/imx/gpcv2.c
index 4b828d74a606..856eaac0ec14 100644
--- a/drivers/pmdomain/imx/gpcv2.c
+++ b/drivers/pmdomain/imx/gpcv2.c
@@ -393,6 +393,17 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
* automatically there. Just add a delay and suppose the handshake finish
* after that.
*/
+
+ /*
+ * For some BLK-CTL module (eg. AudioMix on i.MX8MP) doesn't have BUS
+ * clk-en bit, it is better to add delay here, as the BLK-CTL module
+ * doesn't need to care about how it is powered up.
+ *
+ * regmap_read_bypassed() is to make sure the above write IO transaction
+ * already reaches target before udelay()
+ */
+ regmap_read_bypassed(domain->regmap, domain->regs->hsk, ®_val);
+ udelay(5);
}
/* Disable reset clocks for all devices in the domain */
--
2.34.1
On Fri, May 10, 2024 at 10:15 PM Shengjiu Wang <[email protected]> wrote:
>
> AudioMix BLK-CTRL on i.MX8MP encountered an accessing register issue
> after power up.
>
> [ 2.181035] Kernel panic - not syncing: Asynchronous SError Interrupt
> [ 2.181038] CPU: 1 PID: 48 Comm: kworker/u16:2 Not tainted 6.9.0-rc5-next-20240424-00003-g21cec88845c6 #171
> [ 2.181047] Hardware name: NXP i.MX8MPlus EVK board (DT)
> [ 2.181050] Workqueue: events_unbound deferred_probe_work_func
> [ 2.181064] Call trace:
> [...]
> [ 2.181142] arm64_serror_panic+0x6c/0x78
> [ 2.181149] do_serror+0x3c/0x70
> [ 2.181157] el1h_64_error_handler+0x30/0x48
> [ 2.181164] el1h_64_error+0x64/0x68
> [ 2.181171] clk_imx8mp_audiomix_runtime_resume+0x34/0x44
> [ 2.181183] __genpd_runtime_resume+0x30/0x80
> [ 2.181195] genpd_runtime_resume+0x110/0x244
> [ 2.181205] __rpm_callback+0x48/0x1d8
> [ 2.181213] rpm_callback+0x68/0x74
> [ 2.181224] rpm_resume+0x468/0x6c0
> [ 2.181234] __pm_runtime_resume+0x50/0x94
> [ 2.181243] pm_runtime_get_suppliers+0x60/0x8c
> [ 2.181258] __driver_probe_device+0x48/0x12c
> [ 2.181268] driver_probe_device+0xd8/0x15c
> [ 2.181278] __device_attach_driver+0xb8/0x134
> [ 2.181290] bus_for_each_drv+0x84/0xe0
> [ 2.181302] __device_attach+0x9c/0x188
> [ 2.181312] device_initial_probe+0x14/0x20
> [ 2.181323] bus_probe_device+0xac/0xb0
> [ 2.181334] deferred_probe_work_func+0x88/0xc0
> [ 2.181344] process_one_work+0x150/0x290
> [ 2.181357] worker_thread+0x2f8/0x408
> [ 2.181370] kthread+0x110/0x114
> [ 2.181381] ret_from_fork+0x10/0x20
> [ 2.181391] SMP: stopping secondary CPUs
>
The imx8mp-beacon board suffers from this as well, and I can confirm
the patch also fixes it. It might be a coincidence, but the etnaviv
NPU also enumerates more reliably now too.
adam
> According to comments in power up handshake:
>
> /* request the ADB400 to power up */
> if (domain->bits.hskreq) {
> regmap_update_bits(domain->regmap, domain->regs->hsk,
> domain->bits.hskreq, domain->bits.hskreq);
>
> /*
> * ret = regmap_read_poll_timeout(domain->regmap, domain->regs->hsk, reg_val,
> * (reg_val & domain->bitshskack), 0,
> * USEC_PER_MSEC);
> * Technically we need the commented code to wait handshake. But that needs
> * the BLK-CTL module BUS clk-en bit being set.
> *
> * There is a separate BLK-CTL module and we will have such a driver for it,
> * that driver will set the BUS clk-en bit and handshake will be triggered
> * automatically there. Just add a delay and suppose the handshake finish
> * after that.
> */
> }
>
> The BLK-CTL module needs to add delay to wait for a handshake request finished.
> For some BLK-CTL module (eg. AudioMix on i.MX8MP) doesn't have BUS clk-en
> bit, it is better to add delay in this driver, as the BLK-CTL module doesn't
> need to care about how it is powered up.
>
> regmap_read_bypassed() is to make sure the above write IO transaction already
> reaches target before udelay().
>
> Fixes: 1496dd413b2e ("clk: imx: imx8mp: Add pm_runtime support for power saving")
> Reported-by: Francesco Dolcini <[email protected]>
> Closes: https://lore.kernel.org/all/[email protected]/
> Suggested-by: Frank Li <[email protected]>
> Signed-off-by: Shengjiu Wang <[email protected]>
Tested-by: Adam Ford <[email protected]>
> ---
> changes in v3:
> - move change to gpcv2.c, as it is more reasonable to let power driver
> to handle such power issue, suggested by Frank Li
>
> changes in v2:
> - reduce size of panic log in commit message
>
> drivers/pmdomain/imx/gpcv2.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/pmdomain/imx/gpcv2.c b/drivers/pmdomain/imx/gpcv2.c
> index 4b828d74a606..856eaac0ec14 100644
> --- a/drivers/pmdomain/imx/gpcv2.c
> +++ b/drivers/pmdomain/imx/gpcv2.c
> @@ -393,6 +393,17 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
> * automatically there. Just add a delay and suppose the handshake finish
> * after that.
> */
> +
> + /*
> + * For some BLK-CTL module (eg. AudioMix on i.MX8MP) doesn't have BUS
> + * clk-en bit, it is better to add delay here, as the BLK-CTL module
> + * doesn't need to care about how it is powered up.
> + *
> + * regmap_read_bypassed() is to make sure the above write IO transaction
> + * already reaches target before udelay()
> + */
> + regmap_read_bypassed(domain->regmap, domain->regs->hsk, ®_val);
> + udelay(5);
> }
>
> /* Disable reset clocks for all devices in the domain */
> --
> 2.34.1
>
>
Hi,
Am Mittwoch, 15. Mai 2024, 00:08:21 CEST schrieb Adam Ford:
> On Fri, May 10, 2024 at 10:15 PM Shengjiu Wang <[email protected]> wrote:
> >
> > AudioMix BLK-CTRL on i.MX8MP encountered an accessing register issue
> > after power up.
> >
> > [ 2.181035] Kernel panic - not syncing: Asynchronous SError Interrupt
> > [ 2.181038] CPU: 1 PID: 48 Comm: kworker/u16:2 Not tainted 6.9.0-rc5-next-20240424-00003-g21cec88845c6 #171
> > [ 2.181047] Hardware name: NXP i.MX8MPlus EVK board (DT)
> > [ 2.181050] Workqueue: events_unbound deferred_probe_work_func
> > [ 2.181064] Call trace:
> > [...]
> > [ 2.181142] arm64_serror_panic+0x6c/0x78
> > [ 2.181149] do_serror+0x3c/0x70
> > [ 2.181157] el1h_64_error_handler+0x30/0x48
> > [ 2.181164] el1h_64_error+0x64/0x68
> > [ 2.181171] clk_imx8mp_audiomix_runtime_resume+0x34/0x44
> > [ 2.181183] __genpd_runtime_resume+0x30/0x80
> > [ 2.181195] genpd_runtime_resume+0x110/0x244
> > [ 2.181205] __rpm_callback+0x48/0x1d8
> > [ 2.181213] rpm_callback+0x68/0x74
> > [ 2.181224] rpm_resume+0x468/0x6c0
> > [ 2.181234] __pm_runtime_resume+0x50/0x94
> > [ 2.181243] pm_runtime_get_suppliers+0x60/0x8c
> > [ 2.181258] __driver_probe_device+0x48/0x12c
> > [ 2.181268] driver_probe_device+0xd8/0x15c
> > [ 2.181278] __device_attach_driver+0xb8/0x134
> > [ 2.181290] bus_for_each_drv+0x84/0xe0
> > [ 2.181302] __device_attach+0x9c/0x188
> > [ 2.181312] device_initial_probe+0x14/0x20
> > [ 2.181323] bus_probe_device+0xac/0xb0
> > [ 2.181334] deferred_probe_work_func+0x88/0xc0
> > [ 2.181344] process_one_work+0x150/0x290
> > [ 2.181357] worker_thread+0x2f8/0x408
> > [ 2.181370] kthread+0x110/0x114
> > [ 2.181381] ret_from_fork+0x10/0x20
> > [ 2.181391] SMP: stopping secondary CPUs
> >
>
> The imx8mp-beacon board suffers from this as well, and I can confirm
> the patch also fixes it. It might be a coincidence, but the etnaviv
> NPU also enumerates more reliably now too.
I had a similar local hack/workaround for NPU startup. This patch address
both issues.
Tested-by: Alexander Stein <[email protected]>
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
Hello Stephen and Ulf,
On Sat, May 11, 2024 at 10:55:25AM +0800, Shengjiu Wang wrote:
> AudioMix BLK-CTRL on i.MX8MP encountered an accessing register issue
> after power up.
>
> [ 2.181035] Kernel panic - not syncing: Asynchronous SError Interrupt
..
> Fixes: 1496dd413b2e ("clk: imx: imx8mp: Add pm_runtime support for power saving")
> Reported-by: Francesco Dolcini <[email protected]>
Any chances to get this into mainline with some priority?
If in your opinion the fix is not correct I can just send a revert of
the buggy commit.
I reported the bug 4 weeks ago, the same day the broken commit was
merged into -next [1], now we have a regression in mainline that is
preventing booting with a kernel panic and because of that our whole CI
is not able to test anything and therefore preventing us to look into
any other regression.
Thanks,
Francesco
[1] https://lore.kernel.org/all/20240424164725.GA18760@francesco-nb/
On Sat, 11 May 2024 at 05:15, Shengjiu Wang <[email protected]> wrote:
>
> AudioMix BLK-CTRL on i.MX8MP encountered an accessing register issue
> after power up.
>
> [ 2.181035] Kernel panic - not syncing: Asynchronous SError Interrupt
> [ 2.181038] CPU: 1 PID: 48 Comm: kworker/u16:2 Not tainted 6.9.0-rc5-next-20240424-00003-g21cec88845c6 #171
> [ 2.181047] Hardware name: NXP i.MX8MPlus EVK board (DT)
> [ 2.181050] Workqueue: events_unbound deferred_probe_work_func
> [ 2.181064] Call trace:
> [...]
> [ 2.181142] arm64_serror_panic+0x6c/0x78
> [ 2.181149] do_serror+0x3c/0x70
> [ 2.181157] el1h_64_error_handler+0x30/0x48
> [ 2.181164] el1h_64_error+0x64/0x68
> [ 2.181171] clk_imx8mp_audiomix_runtime_resume+0x34/0x44
> [ 2.181183] __genpd_runtime_resume+0x30/0x80
> [ 2.181195] genpd_runtime_resume+0x110/0x244
> [ 2.181205] __rpm_callback+0x48/0x1d8
> [ 2.181213] rpm_callback+0x68/0x74
> [ 2.181224] rpm_resume+0x468/0x6c0
> [ 2.181234] __pm_runtime_resume+0x50/0x94
> [ 2.181243] pm_runtime_get_suppliers+0x60/0x8c
> [ 2.181258] __driver_probe_device+0x48/0x12c
> [ 2.181268] driver_probe_device+0xd8/0x15c
> [ 2.181278] __device_attach_driver+0xb8/0x134
> [ 2.181290] bus_for_each_drv+0x84/0xe0
> [ 2.181302] __device_attach+0x9c/0x188
> [ 2.181312] device_initial_probe+0x14/0x20
> [ 2.181323] bus_probe_device+0xac/0xb0
> [ 2.181334] deferred_probe_work_func+0x88/0xc0
> [ 2.181344] process_one_work+0x150/0x290
> [ 2.181357] worker_thread+0x2f8/0x408
> [ 2.181370] kthread+0x110/0x114
> [ 2.181381] ret_from_fork+0x10/0x20
> [ 2.181391] SMP: stopping secondary CPUs
>
> According to comments in power up handshake:
>
> /* request the ADB400 to power up */
> if (domain->bits.hskreq) {
> regmap_update_bits(domain->regmap, domain->regs->hsk,
> domain->bits.hskreq, domain->bits.hskreq);
>
> /*
> * ret = regmap_read_poll_timeout(domain->regmap, domain->regs->hsk, reg_val,
> * (reg_val & domain->bits.hskack), 0,
> * USEC_PER_MSEC);
> * Technically we need the commented code to wait handshake. But that needs
> * the BLK-CTL module BUS clk-en bit being set.
> *
> * There is a separate BLK-CTL module and we will have such a driver for it,
> * that driver will set the BUS clk-en bit and handshake will be triggered
> * automatically there. Just add a delay and suppose the handshake finish
> * after that.
> */
> }
>
> The BLK-CTL module needs to add delay to wait for a handshake request finished.
> For some BLK-CTL module (eg. AudioMix on i.MX8MP) doesn't have BUS clk-en
> bit, it is better to add delay in this driver, as the BLK-CTL module doesn't
> need to care about how it is powered up.
>
> regmap_read_bypassed() is to make sure the above write IO transaction already
> reaches target before udelay().
>
> Fixes: 1496dd413b2e ("clk: imx: imx8mp: Add pm_runtime support for power saving")
> Reported-by: Francesco Dolcini <[email protected]>
> Closes: https://lore.kernel.org/all/[email protected]/
> Suggested-by: Frank Li <[email protected]>
> Signed-off-by: Shengjiu Wang <[email protected]>
Sorry for the delay!
Applied for fixes, thanks!
Kind regards
Uffe
> ---
> changes in v3:
> - move change to gpcv2.c, as it is more reasonable to let power driver
> to handle such power issue, suggested by Frank Li
>
> changes in v2:
> - reduce size of panic log in commit message
>
> drivers/pmdomain/imx/gpcv2.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/pmdomain/imx/gpcv2.c b/drivers/pmdomain/imx/gpcv2.c
> index 4b828d74a606..856eaac0ec14 100644
> --- a/drivers/pmdomain/imx/gpcv2.c
> +++ b/drivers/pmdomain/imx/gpcv2.c
> @@ -393,6 +393,17 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
> * automatically there. Just add a delay and suppose the handshake finish
> * after that.
> */
> +
> + /*
> + * For some BLK-CTL module (eg. AudioMix on i.MX8MP) doesn't have BUS
> + * clk-en bit, it is better to add delay here, as the BLK-CTL module
> + * doesn't need to care about how it is powered up.
> + *
> + * regmap_read_bypassed() is to make sure the above write IO transaction
> + * already reaches target before udelay()
> + */
> + regmap_read_bypassed(domain->regmap, domain->regs->hsk, ®_val);
> + udelay(5);
> }
>
> /* Disable reset clocks for all devices in the domain */
> --
> 2.34.1
>
On Tue, May 21, 2024 at 12:22:41PM +0200, Ulf Hansson wrote:
> On Sat, 11 May 2024 at 05:15, Shengjiu Wang <[email protected]> wrote:
> > The BLK-CTL module needs to add delay to wait for a handshake request finished.
> > For some BLK-CTL module (eg. AudioMix on i.MX8MP) doesn't have BUS clk-en
> > bit, it is better to add delay in this driver, as the BLK-CTL module doesn't
> > need to care about how it is powered up.
> Sorry for the delay!
> Applied for fixes, thanks!
I see this is in -next but mainline is currently broken for i.MX8
platforms - could we get it in for -rc2?
On Mon, 27 May 2024 at 14:49, Mark Brown <[email protected]> wrote:
>
> On Tue, May 21, 2024 at 12:22:41PM +0200, Ulf Hansson wrote:
> > On Sat, 11 May 2024 at 05:15, Shengjiu Wang <[email protected]> wrote:
>
> > > The BLK-CTL module needs to add delay to wait for a handshake request finished.
> > > For some BLK-CTL module (eg. AudioMix on i.MX8MP) doesn't have BUS clk-en
> > > bit, it is better to add delay in this driver, as the BLK-CTL module doesn't
> > > need to care about how it is powered up.
>
> > Sorry for the delay!
>
> > Applied for fixes, thanks!
>
> I see this is in -next but mainline is currently broken for i.MX8
> platforms - could we get it in for -rc2?
Yes, I am preparing a pull-request as of right now!
Kind regards
Uffe