Message ID | 20240102081402.1226795-1-treapking@chromium.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-14133-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7301:6f82:b0:100:9c79:88ff with SMTP id tb2csp4325388dyb; Tue, 2 Jan 2024 00:14:33 -0800 (PST) X-Google-Smtp-Source: AGHT+IFvTjOaTFNLvjpe6vyuKSOanXOIzzvKLWQnJ/PAQlw+pDHLgpkTFA4vpXw3w68CY2IoCOrp X-Received: by 2002:a17:907:35c3:b0:a28:26e9:a13d with SMTP id ap3-20020a17090735c300b00a2826e9a13dmr1338113ejc.55.1704183273545; Tue, 02 Jan 2024 00:14:33 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1704183273; cv=none; d=google.com; s=arc-20160816; b=HYTa5emleZDqIiWJDf14Gyw5w/i/+NUZA1e3Lb/EjijK58KCZhxwiRHG1lXOjMxAtx iG68UA//WwArQlSNY7qpsxokiu+6nllZGDi5i1hcJWYQ5TwzS4U3gsUBM+QYvoulHXmS A3XJYhM6v3/pXL8/Dfr2W4UthbMfBSep/XPq6FV+RyUgBu8ZnulpCBMBD02o8QA39uM5 xpN3QYzi8uKoqgHOn5MZk+An6j0GxPH0BcCvYP5plPhVuzkUJK0ApYBUvhkyDBF77WT5 U2wV5HDxlv39N1q37mZJoy8Dhda53wEhMAeMZXAMye1Jl+nKezAK3j88JwrNPWRy1k3b EW8A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=Ewhpw6ZAnNzCIn1VG/yRZaJv/xa5pFyIuSfBXvMExEo=; fh=sqemWYpjZHqP659AZCdqxOutV8GyP3DXzvREPyu7M/o=; b=TSE/9Kgcxt/a+78JYGPnVCyD1GEm30U4H13tqfa0J5kSYLEkl+qzHeBmtvDjDZK45V CfOPTVxJojImoiqkJ9xglc1BA62ANaBQMaUSqvvFe/o/GWQz+kjBfyLIHn6jtasHjUXF WZhwNn6345QXTFBCVF+RhnuqfxQpuLUaqV1NWdC2M19YPqIWplQfDvvdTrBBUBqA9Qt2 R2UfcqsbKMmEdSlHF6aFRf6LYEyFFEudg0iqjm5/BZRBbdhFtJEpwGg1G6pch+RzQZhP cmqU6YxTYwkt3EmYsGH4sKDSnVha5qIHuw9Zeo6vvHNMg79i8ctvua++e7NNoY395S5m cN0w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=UWfqcvH2; spf=pass (google.com: domain of linux-kernel+bounces-14133-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-14133-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [2604:1380:4601:e00::3]) by mx.google.com with ESMTPS id h8-20020a17090791c800b00a281f20bf13si910316ejz.918.2024.01.02.00.14.33 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Jan 2024 00:14:33 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-14133-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=UWfqcvH2; spf=pass (google.com: domain of linux-kernel+bounces-14133-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-14133-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id 1DB761F21D4A for <ouuuleilei@gmail.com>; Tue, 2 Jan 2024 08:14:33 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id D3B58539D; Tue, 2 Jan 2024 08:14:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="UWfqcvH2" X-Original-To: linux-kernel@vger.kernel.org Received: from mail-pj1-f41.google.com (mail-pj1-f41.google.com [209.85.216.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D4462441F for <linux-kernel@vger.kernel.org>; Tue, 2 Jan 2024 08:14:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Received: by mail-pj1-f41.google.com with SMTP id 98e67ed59e1d1-28c7c422ad9so4097925a91.3 for <linux-kernel@vger.kernel.org>; Tue, 02 Jan 2024 00:14:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1704183252; x=1704788052; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=Ewhpw6ZAnNzCIn1VG/yRZaJv/xa5pFyIuSfBXvMExEo=; b=UWfqcvH2rXPgDdlsxakoo+g8+UPCMBhJqtK4bcTHu/HiM7wpnyy5dvgaaehQYQyRVB jKWcv4Nz5O3yn+YrsxXBKe3g5GC5IrfWbyBwluJClCxHa6F1iEvm+klSG0zBCtBRFX4a 9xrBUESVc3ukYP1afY1/LuEcdfpc7QRKU90Vc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704183252; x=1704788052; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Ewhpw6ZAnNzCIn1VG/yRZaJv/xa5pFyIuSfBXvMExEo=; b=EcGFwEl/Ah0KICPiWHIyyp82gPfeSC/bU7GZcVksubqB2pCD3SKtFuFREReMm7ph1+ EOYENfdLjoRfWhxHZSfZctQmxnUnJyNMED+pe10miD4duOpYIFG5oYJJbygJ0Z8xXrJC C9MOHKUGnrBbYvMYbG6FVDurotacky3bMy+gP3hgFfQAQMdPld6/LQYDuQmauoXVmM7J dExNyor7+x2lwk6krzCkGy+CwcCUYOi8k0+X4sr1qEeYiuTXVvz1eV6yzoYYNUTYzQCS 85/qBA6t0VuGTKiSeD5jupe3Pd9nhuNI79y4bzzf7l7vBE3hG5TzRCrTILjE0i7o6wgB nNRw== X-Gm-Message-State: AOJu0Ywtwcr/7wAVagQ2tFgOhhBpbxSgmEMWabd5G7PYwRYP5AvaEAuy 4uLbhrQOnfJ4xnBYyf5ht+wa7Fs1bk31 X-Received: by 2002:a17:90a:7023:b0:28b:96ba:806a with SMTP id f32-20020a17090a702300b0028b96ba806amr7610238pjk.88.1704183252177; Tue, 02 Jan 2024 00:14:12 -0800 (PST) Received: from treapking.tpe.corp.google.com ([2401:fa00:1:10:232c:f04:85bb:a34c]) by smtp.gmail.com with ESMTPSA id f3-20020a17090a638300b0028c8149ac6esm12640074pjj.42.2024.01.02.00.14.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Jan 2024 00:14:11 -0800 (PST) From: Pin-yen Lin <treapking@chromium.org> To: Michael Turquette <mturquette@baylibre.com>, Stephen Boyd <sboyd@kernel.org>, Matthias Brugger <matthias.bgg@gmail.com>, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Cc: Pin-yen Lin <treapking@chromium.org>, Chen-Yu Tsai <wenst@chromium.org>, linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, Weiyi Lu <weiyi.lu@mediatek.com>, linux-mediatek@lists.infradead.org Subject: [PATCH v2 1/2] clk: mediatek: Introduce need_pm_runtime to mtk_clk_desc Date: Tue, 2 Jan 2024 16:12:52 +0800 Message-ID: <20240102081402.1226795-1-treapking@chromium.org> X-Mailer: git-send-email 2.43.0.472.g3155946c3a-goog Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1786965680514909674 X-GMAIL-MSGID: 1786965680514909674 |
Series |
[v2,1/2] clk: mediatek: Introduce need_pm_runtime to mtk_clk_desc
|
|
Commit Message
Pin-yen Lin
Jan. 2, 2024, 8:12 a.m. UTC
Introduce a new need_pm_runtime variable to mtk_clk_desc to indicate
this clock needs a runtime PM get on the clock controller during the
probing stage.
Signed-off-by: Pin-yen Lin <treapking@chromium.org>
---
Changes in v2:
- Fix the order of error handling
- Update the commit message and add a comment before the runtime PM call
drivers/clk/mediatek/clk-mtk.c | 15 +++++++++++++++
drivers/clk/mediatek/clk-mtk.h | 2 ++
2 files changed, 17 insertions(+)
Comments
On Tue, Jan 2, 2024 at 4:14 PM Pin-yen Lin <treapking@chromium.org> wrote: > > Introduce a new need_pm_runtime variable to mtk_clk_desc to indicate > this clock needs a runtime PM get on the clock controller during the > probing stage. No. The flag indicates that the clock controller needs runtime PM for its operation, likely because it needs some power domain enabled. The runtime PM get during the probe phase is a workaround to prevent a deadlock in clk_register. These are two separate things. The second part also should be documented in the code with a comment, i.e. a comment should be placed before pm_runtime_resume_and_get(). ChenYu > Signed-off-by: Pin-yen Lin <treapking@chromium.org> > --- > > Changes in v2: > - Fix the order of error handling > - Update the commit message and add a comment before the runtime PM call > > drivers/clk/mediatek/clk-mtk.c | 15 +++++++++++++++ > drivers/clk/mediatek/clk-mtk.h | 2 ++ > 2 files changed, 17 insertions(+) > > diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c > index 2e55368dc4d8..c31e535909c8 100644 > --- a/drivers/clk/mediatek/clk-mtk.c > +++ b/drivers/clk/mediatek/clk-mtk.c > @@ -13,6 +13,7 @@ > #include <linux/of.h> > #include <linux/of_address.h> > #include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > #include <linux/slab.h> > > #include "clk-mtk.h" > @@ -494,6 +495,14 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, > return IS_ERR(base) ? PTR_ERR(base) : -ENOMEM; > } > > + > + if (mcd->need_runtime_pm) { > + devm_pm_runtime_enable(&pdev->dev); > + r = pm_runtime_resume_and_get(&pdev->dev); > + if (r) > + return r; > + } > + > /* Calculate how many clk_hw_onecell_data entries to allocate */ > num_clks = mcd->num_clks + mcd->num_composite_clks; > num_clks += mcd->num_fixed_clks + mcd->num_factor_clks; > @@ -574,6 +583,9 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, > goto unregister_clks; > } > > + if (mcd->need_runtime_pm) > + pm_runtime_put(&pdev->dev); > + > return r; > > unregister_clks: > @@ -604,6 +616,9 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, > free_base: > if (mcd->shared_io && base) > iounmap(base); > + > + if (mcd->need_runtime_pm) > + pm_runtime_put(&pdev->dev); > return r; > } > > diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h > index 22096501a60a..c17fe1c2d732 100644 > --- a/drivers/clk/mediatek/clk-mtk.h > +++ b/drivers/clk/mediatek/clk-mtk.h > @@ -237,6 +237,8 @@ struct mtk_clk_desc { > > int (*clk_notifier_func)(struct device *dev, struct clk *clk); > unsigned int mfg_clk_idx; > + > + bool need_runtime_pm; > }; > > int mtk_clk_pdev_probe(struct platform_device *pdev); > -- > 2.43.0.472.g3155946c3a-goog >
Il 02/01/24 09:12, Pin-yen Lin ha scritto: > Introduce a new need_pm_runtime variable to mtk_clk_desc to indicate > this clock needs a runtime PM get on the clock controller during the > probing stage. > > Signed-off-by: Pin-yen Lin <treapking@chromium.org> Hello Pin-yen, We have experienced something similar, but it was really hard to reproduce after some changes. In an effort to try to solve this issue (but again, reproducing is really hard), Eugen has sent a commit in the hope that someone else found a way to easily reproduce. Please look at [1]. I'm also adding Eugen to the Cc's of this email. Cheers, Angelo [1]: https://patchwork.kernel.org/project/linux-pm/patch/20231225133615.78993-1-eugen.hristev@collabora.com/ > --- > > Changes in v2: > - Fix the order of error handling > - Update the commit message and add a comment before the runtime PM call > > drivers/clk/mediatek/clk-mtk.c | 15 +++++++++++++++ > drivers/clk/mediatek/clk-mtk.h | 2 ++ > 2 files changed, 17 insertions(+) > > diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c > index 2e55368dc4d8..c31e535909c8 100644 > --- a/drivers/clk/mediatek/clk-mtk.c > +++ b/drivers/clk/mediatek/clk-mtk.c > @@ -13,6 +13,7 @@ > #include <linux/of.h> > #include <linux/of_address.h> > #include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > #include <linux/slab.h> > > #include "clk-mtk.h" > @@ -494,6 +495,14 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, > return IS_ERR(base) ? PTR_ERR(base) : -ENOMEM; > } > > + > + if (mcd->need_runtime_pm) { > + devm_pm_runtime_enable(&pdev->dev); > + r = pm_runtime_resume_and_get(&pdev->dev); > + if (r) > + return r; > + } > + > /* Calculate how many clk_hw_onecell_data entries to allocate */ > num_clks = mcd->num_clks + mcd->num_composite_clks; > num_clks += mcd->num_fixed_clks + mcd->num_factor_clks; > @@ -574,6 +583,9 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, > goto unregister_clks; > } > > + if (mcd->need_runtime_pm) > + pm_runtime_put(&pdev->dev); > + > return r; > > unregister_clks: > @@ -604,6 +616,9 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, > free_base: > if (mcd->shared_io && base) > iounmap(base); > + > + if (mcd->need_runtime_pm) > + pm_runtime_put(&pdev->dev); > return r; > } > > diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h > index 22096501a60a..c17fe1c2d732 100644 > --- a/drivers/clk/mediatek/clk-mtk.h > +++ b/drivers/clk/mediatek/clk-mtk.h > @@ -237,6 +237,8 @@ struct mtk_clk_desc { > > int (*clk_notifier_func)(struct device *dev, struct clk *clk); > unsigned int mfg_clk_idx; > + > + bool need_runtime_pm; > }; > > int mtk_clk_pdev_probe(struct platform_device *pdev);
On 1/3/24 14:19, AngeloGioacchino Del Regno wrote: > Il 02/01/24 09:12, Pin-yen Lin ha scritto: >> Introduce a new need_pm_runtime variable to mtk_clk_desc to indicate >> this clock needs a runtime PM get on the clock controller during the >> probing stage. >> >> Signed-off-by: Pin-yen Lin <treapking@chromium.org> > > Hello Pin-yen, > > We have experienced something similar, but it was really hard to reproduce after > some changes. > > In an effort to try to solve this issue (but again, reproducing is really hard), > Eugen has sent a commit in the hope that someone else found a way to easily > reproduce. Please look at [1]. > > I'm also adding Eugen to the Cc's of this email. > > Cheers, > Angelo > > [1]: > https://patchwork.kernel.org/project/linux-pm/patch/20231225133615.78993-1-eugen.hristev@collabora.com/ Hello Pin-yen, Can you try my patch and let me know if this changes anything for you ? If it does not change anything, can you also try this one as well ? It's another attempt to fix the synchronization with genpd. https://lore.kernel.org/linux-arm-kernel/20231129113120.4907-1-eugen.hristev@collabora.com/ Thanks, Eugen > >> --- >> >> Changes in v2: >> - Fix the order of error handling >> - Update the commit message and add a comment before the runtime PM call >> >> drivers/clk/mediatek/clk-mtk.c | 15 +++++++++++++++ >> drivers/clk/mediatek/clk-mtk.h | 2 ++ >> 2 files changed, 17 insertions(+) >> >> diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c >> index 2e55368dc4d8..c31e535909c8 100644 >> --- a/drivers/clk/mediatek/clk-mtk.c >> +++ b/drivers/clk/mediatek/clk-mtk.c >> @@ -13,6 +13,7 @@ >> #include <linux/of.h> >> #include <linux/of_address.h> >> #include <linux/platform_device.h> >> +#include <linux/pm_runtime.h> >> #include <linux/slab.h> >> >> #include "clk-mtk.h" >> @@ -494,6 +495,14 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, >> return IS_ERR(base) ? PTR_ERR(base) : -ENOMEM; >> } >> >> + >> + if (mcd->need_runtime_pm) { >> + devm_pm_runtime_enable(&pdev->dev); >> + r = pm_runtime_resume_and_get(&pdev->dev); >> + if (r) >> + return r; >> + } >> + >> /* Calculate how many clk_hw_onecell_data entries to allocate */ >> num_clks = mcd->num_clks + mcd->num_composite_clks; >> num_clks += mcd->num_fixed_clks + mcd->num_factor_clks; >> @@ -574,6 +583,9 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, >> goto unregister_clks; >> } >> >> + if (mcd->need_runtime_pm) >> + pm_runtime_put(&pdev->dev); >> + >> return r; >> >> unregister_clks: >> @@ -604,6 +616,9 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, >> free_base: >> if (mcd->shared_io && base) >> iounmap(base); >> + >> + if (mcd->need_runtime_pm) >> + pm_runtime_put(&pdev->dev); >> return r; >> } >> >> diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h >> index 22096501a60a..c17fe1c2d732 100644 >> --- a/drivers/clk/mediatek/clk-mtk.h >> +++ b/drivers/clk/mediatek/clk-mtk.h >> @@ -237,6 +237,8 @@ struct mtk_clk_desc { >> >> int (*clk_notifier_func)(struct device *dev, struct clk *clk); >> unsigned int mfg_clk_idx; >> + >> + bool need_runtime_pm; >> }; >> >> int mtk_clk_pdev_probe(struct platform_device *pdev); > > > >
Hi Eugen and Angelo, Unfortunately, I don't have a way to reliably reproduce this either. We notice this issue from the automatic crash reports sent from the users, but we are still not able to reproduce this locally. So our plan is to ship this patch to the users and see if the crash rate goes down after a month or so. Regards, Pin-yen On Wed, Jan 3, 2024 at 9:20 PM Eugen Hristev <eugen.hristev@collabora.com> wrote: > > On 1/3/24 14:19, AngeloGioacchino Del Regno wrote: > > Il 02/01/24 09:12, Pin-yen Lin ha scritto: > >> Introduce a new need_pm_runtime variable to mtk_clk_desc to indicate > >> this clock needs a runtime PM get on the clock controller during the > >> probing stage. > >> > >> Signed-off-by: Pin-yen Lin <treapking@chromium.org> > > > > Hello Pin-yen, > > > > We have experienced something similar, but it was really hard to reproduce after > > some changes. > > > > In an effort to try to solve this issue (but again, reproducing is really hard), > > Eugen has sent a commit in the hope that someone else found a way to easily > > reproduce. Please look at [1]. > > > > I'm also adding Eugen to the Cc's of this email. > > > > Cheers, > > Angelo > > > > [1]: > > https://patchwork.kernel.org/project/linux-pm/patch/20231225133615.78993-1-eugen.hristev@collabora.com/ > > Hello Pin-yen, > > Can you try my patch and let me know if this changes anything for you ? > > If it does not change anything, can you also try this one as well ? It's another > attempt to fix the synchronization with genpd. > > https://lore.kernel.org/linux-arm-kernel/20231129113120.4907-1-eugen.hristev@collabora.com/ > > Thanks, > Eugen > > > > >> --- > >> > >> Changes in v2: > >> - Fix the order of error handling > >> - Update the commit message and add a comment before the runtime PM call > >> > >> drivers/clk/mediatek/clk-mtk.c | 15 +++++++++++++++ > >> drivers/clk/mediatek/clk-mtk.h | 2 ++ > >> 2 files changed, 17 insertions(+) > >> > >> diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c > >> index 2e55368dc4d8..c31e535909c8 100644 > >> --- a/drivers/clk/mediatek/clk-mtk.c > >> +++ b/drivers/clk/mediatek/clk-mtk.c > >> @@ -13,6 +13,7 @@ > >> #include <linux/of.h> > >> #include <linux/of_address.h> > >> #include <linux/platform_device.h> > >> +#include <linux/pm_runtime.h> > >> #include <linux/slab.h> > >> > >> #include "clk-mtk.h" > >> @@ -494,6 +495,14 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, > >> return IS_ERR(base) ? PTR_ERR(base) : -ENOMEM; > >> } > >> > >> + > >> + if (mcd->need_runtime_pm) { > >> + devm_pm_runtime_enable(&pdev->dev); > >> + r = pm_runtime_resume_and_get(&pdev->dev); > >> + if (r) > >> + return r; > >> + } > >> + > >> /* Calculate how many clk_hw_onecell_data entries to allocate */ > >> num_clks = mcd->num_clks + mcd->num_composite_clks; > >> num_clks += mcd->num_fixed_clks + mcd->num_factor_clks; > >> @@ -574,6 +583,9 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, > >> goto unregister_clks; > >> } > >> > >> + if (mcd->need_runtime_pm) > >> + pm_runtime_put(&pdev->dev); > >> + > >> return r; > >> > >> unregister_clks: > >> @@ -604,6 +616,9 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, > >> free_base: > >> if (mcd->shared_io && base) > >> iounmap(base); > >> + > >> + if (mcd->need_runtime_pm) > >> + pm_runtime_put(&pdev->dev); > >> return r; > >> } > >> > >> diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h > >> index 22096501a60a..c17fe1c2d732 100644 > >> --- a/drivers/clk/mediatek/clk-mtk.h > >> +++ b/drivers/clk/mediatek/clk-mtk.h > >> @@ -237,6 +237,8 @@ struct mtk_clk_desc { > >> > >> int (*clk_notifier_func)(struct device *dev, struct clk *clk); > >> unsigned int mfg_clk_idx; > >> + > >> + bool need_runtime_pm; > >> }; > >> > >> int mtk_clk_pdev_probe(struct platform_device *pdev); > > > > > > > > >
On Thu, Jan 4, 2024 at 10:51 PM Pin-yen Lin <treapking@chromium.org> wrote: > > Hi Eugen and Angelo, > > Unfortunately, I don't have a way to reliably reproduce this either. > > We notice this issue from the automatic crash reports sent from the > users, but we are still not able to reproduce this locally. So our > plan is to ship this patch to the users and see if the crash rate goes > down after a month or so. > > Regards, > Pin-yen > > On Wed, Jan 3, 2024 at 9:20 PM Eugen Hristev > <eugen.hristev@collabora.com> wrote: > > > > On 1/3/24 14:19, AngeloGioacchino Del Regno wrote: > > > Il 02/01/24 09:12, Pin-yen Lin ha scritto: > > >> Introduce a new need_pm_runtime variable to mtk_clk_desc to indicate > > >> this clock needs a runtime PM get on the clock controller during the > > >> probing stage. > > >> > > >> Signed-off-by: Pin-yen Lin <treapking@chromium.org> > > > > > > Hello Pin-yen, > > > > > > We have experienced something similar, but it was really hard to reproduce after > > > some changes. > > > > > > In an effort to try to solve this issue (but again, reproducing is really hard), > > > Eugen has sent a commit in the hope that someone else found a way to easily > > > reproduce. Please look at [1]. > > > > > > I'm also adding Eugen to the Cc's of this email. > > > > > > Cheers, > > > Angelo > > > > > > [1]: > > > https://patchwork.kernel.org/project/linux-pm/patch/20231225133615.78993-1-eugen.hristev@collabora.com/ > > > > Hello Pin-yen, > > > > Can you try my patch and let me know if this changes anything for you ? > > > > If it does not change anything, can you also try this one as well ? It's another > > attempt to fix the synchronization with genpd. > > > > https://lore.kernel.org/linux-arm-kernel/20231129113120.4907-1-eugen.hristev@collabora.com/ > > > > Thanks, > > Eugen Hi Eugen and Angelo, After the offline discussion with Chen-yu, we think this series is solving a different issue from the patches you mentioned. This one is trying to resolve a deadlock in the probe stage, and more details can be found in the commit message of the next patch. The referenced patches seem to be fixing other race conditions on powering on/off the power domain. Sorry for adding the wrong commit message and maybe leading to incorrect understanding on this series. By the way, sorry for the top posting in the previous mail. Regards, Pin-yen > > > > > > > >> --- > > >> > > >> Changes in v2: > > >> - Fix the order of error handling > > >> - Update the commit message and add a comment before the runtime PM call > > >> > > >> drivers/clk/mediatek/clk-mtk.c | 15 +++++++++++++++ > > >> drivers/clk/mediatek/clk-mtk.h | 2 ++ > > >> 2 files changed, 17 insertions(+) > > >> > > >> diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c > > >> index 2e55368dc4d8..c31e535909c8 100644 > > >> --- a/drivers/clk/mediatek/clk-mtk.c > > >> +++ b/drivers/clk/mediatek/clk-mtk.c > > >> @@ -13,6 +13,7 @@ > > >> #include <linux/of.h> > > >> #include <linux/of_address.h> > > >> #include <linux/platform_device.h> > > >> +#include <linux/pm_runtime.h> > > >> #include <linux/slab.h> > > >> > > >> #include "clk-mtk.h" > > >> @@ -494,6 +495,14 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, > > >> return IS_ERR(base) ? PTR_ERR(base) : -ENOMEM; > > >> } > > >> > > >> + > > >> + if (mcd->need_runtime_pm) { > > >> + devm_pm_runtime_enable(&pdev->dev); > > >> + r = pm_runtime_resume_and_get(&pdev->dev); > > >> + if (r) > > >> + return r; > > >> + } > > >> + > > >> /* Calculate how many clk_hw_onecell_data entries to allocate */ > > >> num_clks = mcd->num_clks + mcd->num_composite_clks; > > >> num_clks += mcd->num_fixed_clks + mcd->num_factor_clks; > > >> @@ -574,6 +583,9 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, > > >> goto unregister_clks; > > >> } > > >> > > >> + if (mcd->need_runtime_pm) > > >> + pm_runtime_put(&pdev->dev); > > >> + > > >> return r; > > >> > > >> unregister_clks: > > >> @@ -604,6 +616,9 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, > > >> free_base: > > >> if (mcd->shared_io && base) > > >> iounmap(base); > > >> + > > >> + if (mcd->need_runtime_pm) > > >> + pm_runtime_put(&pdev->dev); > > >> return r; > > >> } > > >> > > >> diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h > > >> index 22096501a60a..c17fe1c2d732 100644 > > >> --- a/drivers/clk/mediatek/clk-mtk.h > > >> +++ b/drivers/clk/mediatek/clk-mtk.h > > >> @@ -237,6 +237,8 @@ struct mtk_clk_desc { > > >> > > >> int (*clk_notifier_func)(struct device *dev, struct clk *clk); > > >> unsigned int mfg_clk_idx; > > >> + > > >> + bool need_runtime_pm; > > >> }; > > >> > > >> int mtk_clk_pdev_probe(struct platform_device *pdev); > > > > > > > > > > > > > >
diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c index 2e55368dc4d8..c31e535909c8 100644 --- a/drivers/clk/mediatek/clk-mtk.c +++ b/drivers/clk/mediatek/clk-mtk.c @@ -13,6 +13,7 @@ #include <linux/of.h> #include <linux/of_address.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/slab.h> #include "clk-mtk.h" @@ -494,6 +495,14 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, return IS_ERR(base) ? PTR_ERR(base) : -ENOMEM; } + + if (mcd->need_runtime_pm) { + devm_pm_runtime_enable(&pdev->dev); + r = pm_runtime_resume_and_get(&pdev->dev); + if (r) + return r; + } + /* Calculate how many clk_hw_onecell_data entries to allocate */ num_clks = mcd->num_clks + mcd->num_composite_clks; num_clks += mcd->num_fixed_clks + mcd->num_factor_clks; @@ -574,6 +583,9 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, goto unregister_clks; } + if (mcd->need_runtime_pm) + pm_runtime_put(&pdev->dev); + return r; unregister_clks: @@ -604,6 +616,9 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev, free_base: if (mcd->shared_io && base) iounmap(base); + + if (mcd->need_runtime_pm) + pm_runtime_put(&pdev->dev); return r; } diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h index 22096501a60a..c17fe1c2d732 100644 --- a/drivers/clk/mediatek/clk-mtk.h +++ b/drivers/clk/mediatek/clk-mtk.h @@ -237,6 +237,8 @@ struct mtk_clk_desc { int (*clk_notifier_func)(struct device *dev, struct clk *clk); unsigned int mfg_clk_idx; + + bool need_runtime_pm; }; int mtk_clk_pdev_probe(struct platform_device *pdev);