Received-SPF: pass (google.com: domain of linux-kernel+bounces-160130-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Fri, 26 Apr 2024 10:11:50 -0400
From: Frank Li <Frank.li@nxp.com>
To: Shengjiu Wang <shengjiu.wang@gmail.com>
Cc: Shengjiu Wang <shengjiu.wang@nxp.com>, abelvesa@kernel.org,
	peng.fan@nxp.com, mturquette@baylibre.com, sboyd@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de, kernel@pengutronix.de,
	festevam@gmail.com, imx@lists.linux.dev, linux-clk@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] clk: imx: imx8mp: Add delay after power up
Message-ID: <Ziu2JjDVO/+gCESM@lizhi-Precision-Tower-5810>
References: <1714040364-17127-1-git-send-email-shengjiu.wang@nxp.com>
 <ZirB5sWBsOXucVsY@lizhi-Precision-Tower-5810>
 <CAA+D8AO4Hzg3ydc1CkidZ0T18bJ3M3d79WPSxAcWZ2xdT4=rMA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAA+D8AO4Hzg3ydc1CkidZ0T18bJ3M3d79WPSxAcWZ2xdT4=rMA@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?b0dEbnBMVTIwYnpBczNMWWtYakRiNVZmR0FDUm0zcVBkMTFzdzVuQXFMN3NJ?=
 =?utf-8?B?SEZuTWMwNlNVQ2NEQTQxbEwyZVowN2Z3QXlsWXhJTXgrVzUycjgyT2dsVDI4?=
 =?utf-8?B?UXVoNHdzWDNxK3dHMmFDeS8rSGlvU3R1U0FIMGxJMGh1RFQ2RzFleldSY0tx?=
 =?utf-8?B?L1V0cXV3LzZoQ1ZKbWF0dTNoNmpHOWVNbDBRSTZTSStkbzZGYURVQnZJb1lR?=
 =?utf-8?B?MlpoeDVlUjBXTUZRZWMwQXpNYkkxdDQ3dGtKZElaTHdtNlFNUitjaTRqa05I?=
 =?utf-8?B?U3JwTGhBekJCYThHRlRDRzN5dUJxUGo5bWkzSS9ROExSTXczNkx2dUgxRFNT?=
 =?utf-8?B?YzlGNDhORExiekxjVzZYOXFVRXhHc0FhdGtuUGJYNGlNN1AyVE1STzVvSldJ?=
 =?utf-8?B?THFjWmVVaGc1eEJJTllqU0dUN3FzZHJuS28xTEJOZXgzYk44K0JJbVhpbVp6?=
 =?utf-8?B?V09XUWJzVDUxTUY1bmg4OXRUNmJUeUdaK1k2QjRKN2hJdXpoNkdCbFp0dUd0?=
 =?utf-8?B?NGNFdmRtKzNEMVMwOG1TZmppOWpNS1NGcFRoVG10QlBIMjB2STM1UlJkQjU1?=
 =?utf-8?B?aVJCUDRDcDYrWGY0aTk4YXJSMjllR3o2b1hzWG1JK2YvcHlSTHZ2S3VnRUxW?=
 =?utf-8?B?L0FoRDIxMUtTbzlRWXZ1TEJBMlVtWlRBWFFZdm9VbW1yMVR2Z3ZGeU40cW51?=
 =?utf-8?B?aXhBdE5UNUtHUDFXdTBwNCtMWFV2UmxzSUp6MmU1MDQxaS96QTcvaXJtWWFC?=
 =?utf-8?B?WEFVNDNaZ0NLNzZrWjk4WFhMQXpFenlQMkU5aWdwYkQ3aUZpVVNpUlo5MkZn?=
 =?utf-8?B?ZG15QUVpNXRBd0pBM1BJOFZFNTRrQ0Z5dmNoU0JsOFpiSHNsdElhekcvRFVn?=
 =?utf-8?B?NU5FWHN0SnNsMW9XbldwZG1COTczaGU2MGhYeWVSUnBPRlZEdHkrQUF6WVJ4?=
 =?utf-8?B?ZS9iZEw2Z0dIMlExTGQzUkJuSHdpWGVQZzVUMWFGUE1EcmRSck5pd0VLRHcw?=
 =?utf-8?B?aVhZeGxwMFZEYThySzRKNElLRUJveVA5bVJwTVB5ZUdHd0t2aUVOZkN2c2tI?=
 =?utf-8?B?aGwraC9lM2JJZEF6UEJ1c1NSeHF0M2JlZ1B6NHdtWFFrV1dsOE50VGpWTHBw?=
 =?utf-8?B?cDEwMGhBS0hBT05mVVFrNkFWdS81RXU0QVYwVi9xclpMUmx5YVVScTFmSWtX?=
 =?utf-8?B?TlcrTFFac2hDQnRsVU9NemtVRzhtbTJIT1ZQdVExRUpuRjJadXFIRzlyR1pr?=
 =?utf-8?B?a2JaNlIvT2FVMGRhNEExeUpSeDYxbGwzNENheEZFMWtlMXFXTEtjNHlSa2hv?=
 =?utf-8?B?MHo5MjVCUkp6RVJsYUNnV0xaeUtvT0VoUWYwdys5Y1dnaHozOEhLY2ZXU2Vx?=
 =?utf-8?B?QmFaQnVtVTdpRDg1bURDLzBPZGV3Mjd2K3ZSVXdURjArVkJGdUYxMmthSE5L?=
 =?utf-8?B?ZUNDcGtLM2drTFJNNElIVHdveGFMYWsvTXpBdExhdWJXc0UvckNIMkNtdkVK?=
 =?utf-8?B?ZDdRZGZYemRyNnNIbjdRaTIxUEZXSTZoOG15UDZiRFdrL2xVOGY5aXQrR2xY?=
 =?utf-8?B?bVVhWDVRNldCVHBrSGFqdzVEemZXTHVja1ovclc2NTdPWUZ3WUJ0QU85aWE3?=
 =?utf-8?B?aTR1RnRKb0IxMjBaVklQNkVabk4vN05jd0pYempLaGdqdWZzalBPQVhFdkZ6?=
 =?utf-8?B?aTdKT0praldtZmdjRmtmWmkwdlZFbkljc1BpcWdEcHhwSS96cmpmc1ptdjN0?=
 =?utf-8?B?dEhNTFZWaDFvbmZTTXdjK2wxajhFR04zWWFRWWdUS0xDQVYxdnVSZFZTWU8z?=
 =?utf-8?B?dTN2ZlR5YzBDdm1kL1F0bHZVN0NneU54eHFHb1FKK1NOaG9BOEJwbHBYeUIv?=
 =?utf-8?B?YkkwWWhrcWNYWElMNFVwUHlkV05haW5QanV0cUZaOEZId25pSDlqY05tUEo1?=
 =?utf-8?B?RzFMck5jUkpjazEraktBWkN1SGRTRVJJQi9pVkFjTnF3NWgzUFJhOGhJMUtU?=
 =?utf-8?B?TG1QZnR0NmJmcjNLWm1MV2hKRS9Ma01tLzAzLzN6WG5OZHpveFJLeVZ6MGYz?=
 =?utf-8?B?bkMrZ2VOT0lIMHdrLzE4a1hDeUZKRC9LZDdnNTRRbkowWHRZczE1WjlzQ1pD?=
 =?utf-8?Q?KgP4=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a91a36e7-f22b-42f1-74e5-08dc65fad6dd
X-MS-Exchange-CrossTenant-AuthSource: PAXPR04MB9642.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Apr 2024 14:12:00.2135
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: nPSb0ShPUaK6K/bV5cU6jYX8eP3QyqWyhidWVLtyh2S89G6hFpJoT+qGO5J0UOpTkdXtAWTLdMNp0EGJRiRRTw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXPR04MB9021

On Fri, Apr 26, 2024 at 05:40:52PM +0800, Shengjiu Wang wrote:
> On Fri, Apr 26, 2024 at 4:49 AM Frank Li <Frank.li@nxp.com> wrote:
> >
> > On Thu, Apr 25, 2024 at 06:19:24PM +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->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
> > > before accessing registers, which is just after the enabling of power domain.
> > >
> > > Otherwise there is error:
> > >
> > > [    2.180834] SError Interrupt on CPU1, code 0x00000000bf000002 -- SError
> > > [    2.180849] CPU: 1 PID: 48 Comm: kworker/u16:2 Not tainted 6.9.0-rc5-next-20240424-00003-g21cec88845c6 #171
> > > [    2.180856] Hardware name: NXP i.MX8MPlus EVK board (DT)
> > > [    2.180861] Workqueue: events_unbound deferred_probe_work_func
> > > [    2.180878] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > [    2.180885] pc : clk_imx8mp_audiomix_runtime_resume+0x34/0x44
> > > [    2.180900] lr : pm_generic_runtime_resume+0x2c/0x44
> > > [    2.180910] sp : ffff800083423a20
> > > [    2.180913] x29: ffff800083423a20 x28: 0000000000000000 x27: 0000000000000000
> > > [    2.180922] x26: ffff0000c0e4e410 x25: 0000000000000000 x24: 0000000000000000
> > > [    2.180935] x23: 0000000000000000 x22: ffff0000c02afd20 x21: 0000000000000000
> > > [    2.180945] x20: ffff0000c162a000 x19: ffff0000c0e52810 x18: ffffffffffffffff
> > > [    2.180959] x17: 0000000000000000 x16: 0000000000000100 x15: ffff8000834239f0
> > > [    2.180970] x14: ffff0000c03d0a1c x13: ffff0000c0a03440 x12: 00000000000003c7
> > > [    2.180982] x11: 0000000000000000 x10: ffff8000825010ba x9 : 0000000000000008
> > > [    2.180993] x8 : 0000000000000008 x7 : 0000000000000000 x6 : 0000000000000000
> > > [    2.181005] x5 : ffff8000838b0000 x4 : ffff0000c0e66088 x3 : ffff8000813852c0
> > > [    2.181018] x2 : 0000000000000000 x1 : 0000000000000004 x0 : ffff8000838b0300
> > > [    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.181066]  dump_backtrace+0x90/0xe8
> > > [    2.181080]  show_stack+0x18/0x24
> > > [    2.181091]  dump_stack_lvl+0x34/0x8c
> > > [    2.181104]  dump_stack+0x18/0x24
> > > [    2.181117]  panic+0x39c/0x3d0
> > > [    2.181129]  nmi_panic+0x48/0x94
> > > [    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
> > > [    2.181400] Kernel Offset: disabled
> > > [    2.181403] CPU features: 0x0,00000040,00100000,4200421b
> > > [    2.181407] Memory Limit: none
> > > [    2.463040] ---[ end Kernel panic - not syncing: Asynchronous SError Interrupt ]---
> > >
> > > Fixes: 1496dd413b2e ("clk: imx: imx8mp: Add pm_runtime support for power saving")
> > > Reported-by: Francesco Dolcini <francesco@dolcini.it>
> > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > ---
> > >  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 e4231e9c8f05..cb44c460548e 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>
> > > @@ -362,6 +363,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);
> > > +
> >
> > https://lore.kernel.org/imx/20230727152503.2199550-1-shenwei.wang@nxp.com/T/#m2dc5111e9628235f031c0bad2a137222b0205a61
> >
> > supposed clk_imx8mp_audiomix_save_restore() is that write save data into
> > some registers.
> >
> > See above link for detail, it may not delay 5us before write to register.
> > You need
> >         readl();
> >         udelay(5);
> >         writel();
> 
> Here we need to wait for the handshake to be ready,  we have nothing
> to be read.

It should power_on by writel() at somewhere, maybe gpcv2.c. 
1. writel() //power on

2. udelay(5) // in your code,

3. writel() // in your driver to init  audiomix controller. 

1,2,3 just distribute at difference file, but basic work flow should be
the same.

If here nothing to read, you put udelay at power_on code, maybe gpcv2.

Other driver may meet similar problem, you met this problem just this
driver running close to gpc's v2 by driver probe order or compiler linker
driver order.

> 
> but I don't really understand why the code is commented.
> 
>                /*
>                 * ret = regmap_read_poll_timeout(domain->regmap,
> domain->regs->hsk, reg_val,
>                 *                                (reg_val &
> domain->bits.hskack), 0,
>                 *                                USEC_PER_MSEC);
> 
> If we uncomment the code,  it can also fix the issue.

dose git blame give some clue or ask original driver owner? 

> 
> best regards
> Shengjiu Wang
> 
> >
> > >       clk_imx8mp_audiomix_save_restore(dev, false);
> > >
> > >       return 0;
> > > --
> > > 2.34.1
> > >