linux-kernel - Re: [PATCH v2] clk: imx: imx8mp: Add delay after power up

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
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 7 May 2024 09:44:19 +0800
From: Shengjiu Wang <shengjiu.wang@...il.com>
To: Frank Li <Frank.li@....com>
Cc: Shengjiu Wang <shengjiu.wang@....com>, 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, linux-clk@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] clk: imx: imx8mp: Add delay after power up

On Tue, May 7, 2024 at 2:22 AM Frank Li <Frank.li@....com> wrote:
>
> On Mon, May 06, 2024 at 11:35:02AM +0800, Shengjiu Wang wrote:
> > According to comments in drivers/pmdomain/imx/gpcv2.c:
> >
> >       /* 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
> > before accessing registers, which is just after the enabling of the power domain.
> >
> > Otherwise there is error:
> >
> > [    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
> >
> > Fixes: 1496dd413b2e ("clk: imx: imx8mp: Add pm_runtime support for power saving")
> > Reported-by: Francesco Dolcini <francesco@...cini.it>
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@....com>
> > Revewied-by: Peng Fan <peng.fan@....com>
> > ---
> > changes in v2:
> > - reduce size of panic log in commit message
> >
> >  drivers/clk/imx/clk-imx8mp-audiomix.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/clk/imx/clk-imx8mp-audiomix.c b/drivers/clk/imx/clk-imx8mp-audiomix.c
> > index b381d6f784c8..ae2c0f254225 100644
> > --- a/drivers/clk/imx/clk-imx8mp-audiomix.c
> > +++ b/drivers/clk/imx/clk-imx8mp-audiomix.c
> > @@ -6,6 +6,7 @@
> >   */
> >
> >  #include <linux/clk-provider.h>
> > +#include <linux/delay.h>
> >  #include <linux/device.h>
> >  #include <linux/io.h>
> >  #include <linux/mod_devicetable.h>
> > @@ -360,6 +361,12 @@ static int clk_imx8mp_audiomix_runtime_suspend(struct device *dev)
> >
> >  static int clk_imx8mp_audiomix_runtime_resume(struct device *dev)
> >  {
> > +     /*
> > +      * According to the drivers/pmdomain/imx/gpcv2.c
> > +      * need to wait for handshake request to propagate
> > +      */
> > +     udelay(5);
> > +
>
> Did you address the issue I comments at v1?
> It should not fix at here, I think it should be gpcv2.c to delay 5us.

Other BLK CTRL drivers already delay 5us in its own drivers, if
add delay in gpcv2.c, for these drivers, it will delay 10us totally.

Best regards
Shengjiu Wang



>
> Frank
>
> >       clk_imx8mp_audiomix_save_restore(dev, false);
> >
> >       return 0;
> > --
> > 2.34.1
> >