| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAHCN7xLoW2Nydhc3y-X1ieb7-UZTm=ap1O1eSLE31LawmfRZOg@mail.gmail.com>
Date: Tue, 14 May 2024 17:08:21 -0500
From: Adam Ford <aford173@...il.com>
To: Shengjiu Wang <shengjiu.wang@....com>
Cc: ulf.hansson@...aro.org, heiko@...ech.de, u.kleine-koenig@...gutronix.de,
geert+renesas@...der.be, rafael@...nel.org, linux-pm@...r.kernel.org,
abelvesa@...nel.org, peng.fan@....com, mturquette@...libre.com,
sboyd@...nel.org, shawnguo@...nel.org, s.hauer@...gutronix.de,
kernel@...gutronix.de, festevam@...il.com, imx@...ts.linux.dev,
shengjiu.wang@...il.com, frank.li@....com, mkl@...gutronix.de,
linus.walleij@...aro.org, linux-clk@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] pmdomain: imx: gpcv2: Add delay after power up handshake
On Fri, May 10, 2024 at 10:15 PM Shengjiu Wang <shengjiu.wang@....com> 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 <francesco@...cini.it>
> Closes: https://lore.kernel.org/all/66293535.170a0220.21fe.a2e7@mx.googlecom/
> Suggested-by: Frank Li <frank.li@....com>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@....com>
Tested-by: Adam Ford <aford173@...il.com>
> ---
> 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
>
>
Powered by blists - more mailing lists