From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C2E31ECAAD1 for ; Wed, 31 Aug 2022 07:41:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=CvCrgI0o6LDnNtG+VDgImE2cdSACmY3TKkQr1BFZkd4=; b=gjU4bA5XMyixqH xz9Ta3tycluTzi6jq85xy3wDLD6jHpFGcezHdjminJktXXCyRxVKWhR+2NYnfhXJ4DuJ8fa4/5fr9 rdUQ5j0NbKfv2rwNTpsybYB01/i/8biy2keuPFN+aLumL5fv/xXY3CIST8XIN9mK895uu8FyRL3Pt TP5BeA0B+KMjM0vExomGyzErHSb63SCk3wA31vqk9TqqHotJhexsq+ewSxeACTgYen2/fIBgJdk0E U9Taea/lRXkBJlw5DN2B4NPqBbhwoiBV9u4GpuUAmMt+6cJf1UprU31zrBXEBTA7Ld7DtaajnJvW+ 6Pzk4snjvqkMOnObXAeQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oTIKi-004e5X-Eq; Wed, 31 Aug 2022 07:40:20 +0000 Received: from mx1.tq-group.com ([93.104.207.81]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oTIKd-004e2T-Nl for linux-arm-kernel@lists.infradead.org; Wed, 31 Aug 2022 07:40:18 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1661931615; x=1693467615; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=L76+i4iOV1VqYzI6KegPl3xwtYsecfKw3SFYj2xfKNc=; b=HhU5X7Wgm5XUSLsIEveeZQXdu5fvkyi/uXoGYd9qgaRXzDKlAN1ewVaQ ixH/Kk4ZqMAV4HVieALxy54TKGVUtd1DRQhJyjaHhtia5UH4AyIhlzCVg CZMq+GL6WyOmmoX2XAZA+RlNIL4tw0rGCIfCDlmx6CpEMymaI04uqqSil XKmTSb9yMRIQKtgJpBJnlFm7fVBgMPSG3cHt/G4cqAf/qvlZyIy016+M0 UM34vZ8cadw/ikD9P20/uyMtWx4mDcqqCE5lzEYV/2Xpl2Lu/BEVoWXwb Ow0cnRkHISzf+yhtzu1X5LnAUVfaIVAh9eCk0MQNS2HaqjlUrgVw4Pcwq Q==; X-IronPort-AV: E=Sophos;i="5.93,277,1654552800"; d="scan'208";a="25901125" Received: from unknown (HELO tq-pgp-pr1.tq-net.de) ([192.168.6.15]) by mx1-pgp.tq-group.com with ESMTP; 31 Aug 2022 09:40:10 +0200 Received: from mx1.tq-group.com ([192.168.6.7]) by tq-pgp-pr1.tq-net.de (PGP Universal service); Wed, 31 Aug 2022 09:40:10 +0200 X-PGP-Universal: processed; by tq-pgp-pr1.tq-net.de on Wed, 31 Aug 2022 09:40:10 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tq-group.com; i=@tq-group.com; q=dns/txt; s=key1; t=1661931610; x=1693467610; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=L76+i4iOV1VqYzI6KegPl3xwtYsecfKw3SFYj2xfKNc=; b=FeN05n9SqCPjMLANiMDmpdSpysXK1u0tWtbz7LgSwMVsGu7Y3B59Ythu Yy1myAPypoapLHQPu++SHk8NOEN7eTa+O/TO/XIJdNGFQH0uOvoV/AF1q +N3hKhOURXjA+TVPI4FzeRWJRD8k+abwh/WOKbTQ71wsJPxQ3Sf/6IDpI HXQ7tNvu6ljKcBbZ34DKuiuuMCkEikd4CMQdICZ1Zn9qftCDE0k/Joa2A mTrTj8y5sZGFA0EQ/ZpCqpBcHH+6LKBVrk63oqGzv2+l5NM6X788VkGL7 k8AEU+K/dJjuezwagptBD0Z3lhg/OH6SExhzCM6J1lhOUMpB02V6RIw40 g==; X-IronPort-AV: E=Sophos;i="5.93,277,1654552800"; d="scan'208";a="25901124" Received: from vtuxmail01.tq-net.de ([10.115.0.20]) by mx1.tq-group.com with ESMTP; 31 Aug 2022 09:40:10 +0200 Received: from steina-w.localnet (unknown [10.123.49.11]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by vtuxmail01.tq-net.de (Postfix) with ESMTPSA id E51BA280056; Wed, 31 Aug 2022 09:40:09 +0200 (CEST) From: Alexander Stein To: Chen-Yu Tsai Cc: Michael Turquette , Stephen Boyd , AngeloGioacchino Del Regno , =?ISO-8859-1?Q?N=EDcolas_F_=2E_R_=2E_A_=2E?= Prado , linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH RESEND v2 1/2] clk: core: Honor CLK_OPS_PARENT_ENABLE for clk gate ops Date: Wed, 31 Aug 2022 09:40:07 +0200 Message-ID: <8113103.T7Z3S40VBb@steina-w> Organization: TQ-Systems GmbH In-Reply-To: References: <20220822081424.1310926-1-wenst@chromium.org> <2254919.ElGaqSPkdT@steina-w> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220831_004016_359320_EEB156A1 X-CRM114-Status: GOOD ( 43.74 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Am Dienstag, 30. August 2022, 15:40:53 CEST schrieb Chen-Yu Tsai: > On Tue, Aug 30, 2022 at 8:37 PM Alexander Stein > = > wrote: > > Hi, > > = > > Am Montag, 29. August 2022, 11:22:16 CEST schrieb Chen-Yu Tsai: > > > Hi, > > > = > > > On Fri, Aug 26, 2022 at 8:28 PM Alexander Stein > > > = > > > wrote: > > > > Hi everybody, > > > > = > > > > Am Montag, 22. August 2022, 10:14:23 CEST schrieb Chen-Yu Tsai: > > > > > In the previous commits that added CLK_OPS_PARENT_ENABLE, support > > > > > for > > > > > this flag was only added to rate change operations (rate setting = and > > > > > reparent) and disabling unused subtree. It was not added to the > > > > > clock gate related operations. Any hardware driver that needs it = for > > > > > these operations will either see bogus results, or worse, hang. > > > > > = > > > > > This has been seen on MT8192 and MT8195, where the imp_ii2_* clk > > > > > drivers set this, but dumping debugfs clk_summary would cause it > > > > > to hang. > > > > > = > > > > > Fixes: fc8726a2c021 ("clk: core: support clocks which requires > > > > > parents > > > > > enable (part 2)") Fixes: a4b3518d146f ("clk: core: support clocks > > > > > which > > > > > requires parents enable (part 1)") Signed-off-by: Chen-Yu Tsai > > > > > > > > > > Reviewed-by: N=EDcolas F. R. A. Prado > > > > > Tested-by: N=EDcolas F. R. A. Prado > > > > > --- > > > > > = > > > > > drivers/clk/clk.c | 28 ++++++++++++++++++++++++++++ > > > > > 1 file changed, 28 insertions(+) > > > > > = > > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > > > > index 7fc191c15507..9b365cd6d14b 100644 > > > > > --- a/drivers/clk/clk.c > > > > > +++ b/drivers/clk/clk.c > > > > > @@ -196,6 +196,9 @@ static bool clk_core_rate_is_protected(struct > > > > > clk_core > > > > > *core) return core->protect_count; > > > > > = > > > > > } > > > > > = > > > > > +static int clk_core_prepare_enable(struct clk_core *core); > > > > > +static void clk_core_disable_unprepare(struct clk_core *core); > > > > > + > > > > > = > > > > > static bool clk_core_is_prepared(struct clk_core *core) > > > > > { > > > > > = > > > > > bool ret =3D false; > > > > > = > > > > > @@ -208,7 +211,11 @@ static bool clk_core_is_prepared(struct > > > > > clk_core > > > > > *core) return core->prepare_count; > > > > > = > > > > > if (!clk_pm_runtime_get(core)) { > > > > > = > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > > + clk_core_prepare_enable(core->parent); > > > > > = > > > > > ret =3D core->ops->is_prepared(core->hw); > > > > > = > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > > + clk_core_disable_unprepare(core->parent); > > > > > = > > > > > clk_pm_runtime_put(core); > > > > > = > > > > > } > > > > > = > > > > > @@ -244,7 +251,13 @@ static bool clk_core_is_enabled(struct clk_c= ore > > > > > *core) > > > > > = > > > > > } > > > > > = > > > > > } > > > > > = > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > > + clk_core_prepare_enable(core->parent); > > > > > + > > > > > = > > > > > ret =3D core->ops->is_enabled(core->hw); > > > > > = > > > > > + > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > > + clk_core_disable_unprepare(core->parent); > > > > > = > > > > > done: > > > > > if (core->rpm_enabled) > > > > > = > > > > > pm_runtime_put(core->dev); > > > > > = > > > > > @@ -812,6 +825,9 @@ int clk_rate_exclusive_get(struct clk *clk) > > > > > = > > > > > } > > > > > EXPORT_SYMBOL_GPL(clk_rate_exclusive_get); > > > > > = > > > > > +static int clk_core_enable_lock(struct clk_core *core); > > > > > +static void clk_core_disable_lock(struct clk_core *core); > > > > > + > > > > > = > > > > > static void clk_core_unprepare(struct clk_core *core) > > > > > { > > > > > = > > > > > lockdep_assert_held(&prepare_lock); > > > > > = > > > > > @@ -835,6 +851,9 @@ static void clk_core_unprepare(struct clk_core > > > > > *core) > > > > > = > > > > > WARN(core->enable_count > 0, "Unpreparing enabled %s\n", co= re- > > > > > > > > > >name); > > > > > > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > > + clk_core_enable_lock(core->parent); > > > > > + > > > > > = > > > > > trace_clk_unprepare(core); > > > > > = > > > > > if (core->ops->unprepare) > > > > > = > > > > > @@ -843,6 +862,9 @@ static void clk_core_unprepare(struct clk_core > > > > > *core) > > > > > = > > > > > clk_pm_runtime_put(core); > > > > > = > > > > > trace_clk_unprepare_complete(core); > > > > > = > > > > > + > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > > + clk_core_disable_lock(core->parent); > > > > > = > > > > > clk_core_unprepare(core->parent); > > > > > = > > > > > } > > > > > = > > > > > @@ -891,6 +913,9 @@ static int clk_core_prepare(struct clk_core > > > > > *core) > > > > > = > > > > > if (ret) > > > > > = > > > > > goto runtime_put; > > > > > = > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > > + clk_core_enable_lock(core->parent); > > > > > + > > > > > = > > > > > trace_clk_prepare(core); > > > > > = > > > > > if (core->ops->prepare) > > > > > = > > > > > @@ -898,6 +923,9 @@ static int clk_core_prepare(struct clk_core > > > > > *core) > > > > > = > > > > > trace_clk_prepare_complete(core); > > > > > = > > > > > + if (core->flags & CLK_OPS_PARENT_ENABLE) > > > > > + clk_core_disable_lock(core->parent); > > > > > + > > > > > = > > > > > if (ret) > > > > > = > > > > > goto unprepare; > > > > > = > > > > > } > > > > = > > > > Unfortunately this completely locks up my i.MX8M Plus based board > > > > during > > > > early boot. > > > > I'm currently running on next-20220826 using > > > > arch/arm64/boot/dts/freescale/ > > > > imx8mp-tqma8mpql-mba8mpxl.dts > > > > Reverting this patch gets my board booting again. dmesg until hard > > > > lockup > > > > below. > > > = > > > The standard logs don't have anything to go on. Could you add some > > > printk > > > calls to the clk core around the areas this patch touchs? That would > > > help. > > > = > > > Could you also provide a dump of /sys/kernel/debug/clk/clk_summary? T= hat > > > would help to understand the clock tree. > > = > > Sure, > > = > > These are the last kernel log lines before hard lockup: > > [ 0.686357] io scheduler mq-deadline registered > > [ 0.690907] io scheduler kyber registered > > [ 0.699275] clk_core_prepare: clk: main_axi CLK_OPS_PARENT_ENABLE > > = > > main_axi is also the only debug output up to this point. This is the us= ed > > patch for debugging: > > ---8<--- > > --- a/drivers/clk/clk.c > > +++ b/drivers/clk/clk.c > > @@ -211,8 +211,10 @@ static bool clk_core_is_prepared(struct clk_core > > *core)> = > > return core->prepare_count; > > = > > if (!clk_pm_runtime_get(core)) { > > = > > - if (core->flags & CLK_OPS_PARENT_ENABLE) > > + if (core->flags & CLK_OPS_PARENT_ENABLE) { > > + pr_info("%s: clk: %s CLK_OPS_PARENT_ENABLE\n", > > __func__, core->name); > > = > > clk_core_prepare_enable(core->parent); > > = > > + } > > = > > ret =3D core->ops->is_prepared(core->hw); > > if (core->flags & CLK_OPS_PARENT_ENABLE) > > = > > clk_core_disable_unprepare(core->parent); > > = > > @@ -251,8 +253,10 @@ static bool clk_core_is_enabled(struct clk_core > > *core) > > = > > } > > = > > } > > = > > - if (core->flags & CLK_OPS_PARENT_ENABLE) > > + if (core->flags & CLK_OPS_PARENT_ENABLE) { > > + pr_info("%s: clk: %s CLK_OPS_PARENT_ENABLE\n", __func__, > > core-> = > > >name); > > > > > clk_core_prepare_enable(core->parent); > > = > > + } > > = > > ret =3D core->ops->is_enabled(core->hw); > > = > > @@ -851,8 +855,10 @@ static void clk_core_unprepare(struct clk_core *co= re) > > = > > WARN(core->enable_count > 0, "Unpreparing enabled %s\n", > > core->name); > > = > > - if (core->flags & CLK_OPS_PARENT_ENABLE) > > + if (core->flags & CLK_OPS_PARENT_ENABLE) { > > + pr_info("%s: clk: %s CLK_OPS_PARENT_ENABLE\n", __func__, > > core-> = > > >name); > > > > > clk_core_enable_lock(core->parent); > > = > > + } > > = > > trace_clk_unprepare(core); > > = > > @@ -912,8 +918,10 @@ static int clk_core_prepare(struct clk_core *core) > > = > > if (ret) > > = > > goto runtime_put; > > = > > - if (core->flags & CLK_OPS_PARENT_ENABLE) > > + if (core->flags & CLK_OPS_PARENT_ENABLE) { > > + pr_info("%s: clk: %s CLK_OPS_PARENT_ENABLE\n", > > __func__, core->name); > > = > > clk_core_enable_lock(core->parent); > > = > > + } > > = > > trace_clk_prepare(core); > > = > > ---8<--- > = > Thanks. So the part of the clock tree that's problematic is this: > = > osc_24m (fixed) > sys_pll1_ref_sel (mux) > sys_pll1 (imx pll14xx) > sys_pll1_bypass (mux) > sys_pll1_out (gate) > sys_pll1_800m (fixed factor) > main_axi (composite CLK_IS_CRITICAL) > = > Would it be possible for you to produce a stack dump as well? And also > enable lock debugging? This likely still won't catch anything if it's > a spinlock deadlock though. Sure thing, I added a dump_stack() call to all of the above debug outputs a= s = well as the name of the parent clock, just to be sure, and here is the resu= lt = output: [ 1.142386] io scheduler mq-deadline registered [ 1.146902] io scheduler kyber registered [ 1.177345] clk_core_prepare: clk: main_axi CLK_OPS_PARENT_ENABLE [ 1.180713] clk_core_prepare: clk->parent: sys_pll1_800m [ 1.186025] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.0.0-rc3- next-20220831+ #619 9c82e5b9075d735fa07e2c558603ffb720d72983 [ 1.197274] Hardware name: TQ-Systems i.MX8MPlus TQMa8MPxL on MBa8MPxL (= DT) [ 1.204275] Call trace: [ 1.206723] dump_backtrace+0xd8/0x120 [ 1.210485] show_stack+0x14/0x50 [ 1.213811] dump_stack_lvl+0x88/0xb0 [ 1.217486] dump_stack+0x14/0x2c [ 1.220811] clk_core_prepare+0x1fc/0x240 [ 1.224834] __clk_core_init+0x208/0x4dc [ 1.228772] __clk_register+0x160/0x240 [ 1.232622] clk_hw_register+0x1c/0x5c [ 1.236384] __clk_hw_register_composite+0x1e8/0x2d0 [ 1.241372] clk_hw_register_composite+0x40/0x50 [ 1.246009] __imx8m_clk_hw_composite+0x130/0x210 [ 1.250734] imx8mp_clocks_probe+0x13ac/0x3750 [ 1.255199] platform_probe+0x64/0x100 [ 1.258959] call_driver_probe+0x28/0x140 [ 1.262984] really_probe+0xc0/0x334 [ 1.266572] __driver_probe_device+0x84/0x144 [ 1.270947] driver_probe_device+0x38/0x130 [ 1.275147] __driver_attach+0xd4/0x240 [ 1.278997] bus_for_each_dev+0x6c/0xbc [ 1.282847] driver_attach+0x20/0x30 [ 1.286436] bus_add_driver+0x178/0x250 [ 1.290284] driver_register+0x74/0x120 [ 1.294134] __platform_driver_register+0x24/0x30 [ 1.298859] imx8mp_clk_driver_init+0x18/0x20 [ 1.303234] do_one_initcall+0x48/0x180 [ 1.307084] do_initcalls+0x164/0x19c [ 1.310759] kernel_init_freeable+0xf4/0x138 [ 1.315047] kernel_init+0x2c/0x140 [ 1.318549] ret_from_fork+0x10/0x20 Enabling lock debugging results in no additional output. Best regards, Alexander _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel