* [PATCH v2 1/2] clk: mediatek: Introduce need_pm_runtime to mtk_clk_desc
@ 2024-01-02 8:12 Pin-yen Lin
2024-01-02 8:12 ` [PATCH v2 2/2] clk: mediatek: mt8183: Enable need_runtime_pm on mt8183-mfgcfg Pin-yen Lin
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Pin-yen Lin @ 2024-01-02 8:12 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Matthias Brugger,
AngeloGioacchino Del Regno
Cc: Pin-yen Lin, Chen-Yu Tsai, linux-arm-kernel, linux-clk,
linux-kernel, Weiyi Lu, linux-mediatek
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(+)
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
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 2/2] clk: mediatek: mt8183: Enable need_runtime_pm on mt8183-mfgcfg 2024-01-02 8:12 [PATCH v2 1/2] clk: mediatek: Introduce need_pm_runtime to mtk_clk_desc Pin-yen Lin @ 2024-01-02 8:12 ` Pin-yen Lin 2024-01-03 7:45 ` [PATCH v2 1/2] clk: mediatek: Introduce need_pm_runtime to mtk_clk_desc Chen-Yu Tsai 2024-01-03 12:19 ` AngeloGioacchino Del Regno 2 siblings, 0 replies; 7+ messages in thread From: Pin-yen Lin @ 2024-01-02 8:12 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Matthias Brugger, AngeloGioacchino Del Regno Cc: Pin-yen Lin, Chen-Yu Tsai, linux-arm-kernel, linux-clk, linux-kernel, Weiyi Lu, linux-mediatek mt8183-mfgcfg has a mutual dependency with genpd during the probing stage, so enable need_runtim_pm to prevent a deadlock in the following call stack: CPU0: genpd_lock --> clk_prepare_lock genpd_power_off_work_fn() genpd_lock() generic_pm_domain::power_off() clk_unprepare() clk_prepare_lock() CPU1: clk_prepare_lock --> genpd_lock clk_register() __clk_core_init() clk_prepare_lock() clk_pm_runtime_get() genpd_lock() Do a runtime PM get at the probe function to make sure clk_register() won't acquire the genpd lock. Fixes: acddfc2c261b ("clk: mediatek: Add MT8183 clock support") Signed-off-by: Pin-yen Lin <treapking@chromium.org> --- (no changes since v1) drivers/clk/mediatek/clk-mt8183-mfgcfg.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/clk/mediatek/clk-mt8183-mfgcfg.c b/drivers/clk/mediatek/clk-mt8183-mfgcfg.c index ba504e19d420..62d876e150e1 100644 --- a/drivers/clk/mediatek/clk-mt8183-mfgcfg.c +++ b/drivers/clk/mediatek/clk-mt8183-mfgcfg.c @@ -29,6 +29,7 @@ static const struct mtk_gate mfg_clks[] = { static const struct mtk_clk_desc mfg_desc = { .clks = mfg_clks, .num_clks = ARRAY_SIZE(mfg_clks), + .need_runtime_pm = true, }; static const struct of_device_id of_match_clk_mt8183_mfg[] = { -- 2.43.0.472.g3155946c3a-goog ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] clk: mediatek: Introduce need_pm_runtime to mtk_clk_desc 2024-01-02 8:12 [PATCH v2 1/2] clk: mediatek: Introduce need_pm_runtime to mtk_clk_desc Pin-yen Lin 2024-01-02 8:12 ` [PATCH v2 2/2] clk: mediatek: mt8183: Enable need_runtime_pm on mt8183-mfgcfg Pin-yen Lin @ 2024-01-03 7:45 ` Chen-Yu Tsai 2024-01-03 12:19 ` AngeloGioacchino Del Regno 2 siblings, 0 replies; 7+ messages in thread From: Chen-Yu Tsai @ 2024-01-03 7:45 UTC (permalink / raw) To: Pin-yen Lin Cc: Michael Turquette, Stephen Boyd, Matthias Brugger, AngeloGioacchino Del Regno, linux-arm-kernel, linux-clk, linux-kernel, Weiyi Lu, linux-mediatek 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] clk: mediatek: Introduce need_pm_runtime to mtk_clk_desc 2024-01-02 8:12 [PATCH v2 1/2] clk: mediatek: Introduce need_pm_runtime to mtk_clk_desc Pin-yen Lin 2024-01-02 8:12 ` [PATCH v2 2/2] clk: mediatek: mt8183: Enable need_runtime_pm on mt8183-mfgcfg Pin-yen Lin 2024-01-03 7:45 ` [PATCH v2 1/2] clk: mediatek: Introduce need_pm_runtime to mtk_clk_desc Chen-Yu Tsai @ 2024-01-03 12:19 ` AngeloGioacchino Del Regno 2024-01-03 13:20 ` Eugen Hristev 2 siblings, 1 reply; 7+ messages in thread From: AngeloGioacchino Del Regno @ 2024-01-03 12:19 UTC (permalink / raw) To: Pin-yen Lin, Michael Turquette, Stephen Boyd, Matthias Brugger Cc: Chen-Yu Tsai, linux-arm-kernel, linux-clk, linux-kernel, Weiyi Lu, linux-mediatek, Eugen Hristev 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); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] clk: mediatek: Introduce need_pm_runtime to mtk_clk_desc 2024-01-03 12:19 ` AngeloGioacchino Del Regno @ 2024-01-03 13:20 ` Eugen Hristev 2024-01-04 14:51 ` Pin-yen Lin 0 siblings, 1 reply; 7+ messages in thread From: Eugen Hristev @ 2024-01-03 13:20 UTC (permalink / raw) To: AngeloGioacchino Del Regno, Pin-yen Lin, Michael Turquette, Stephen Boyd, Matthias Brugger Cc: Chen-Yu Tsai, linux-arm-kernel, linux-clk, linux-kernel, Weiyi Lu, linux-mediatek 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); > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] clk: mediatek: Introduce need_pm_runtime to mtk_clk_desc 2024-01-03 13:20 ` Eugen Hristev @ 2024-01-04 14:51 ` Pin-yen Lin 2024-01-05 8:39 ` Pin-yen Lin 0 siblings, 1 reply; 7+ messages in thread From: Pin-yen Lin @ 2024-01-04 14:51 UTC (permalink / raw) To: Eugen Hristev, AngeloGioacchino Del Regno Cc: Michael Turquette, Stephen Boyd, Matthias Brugger, Chen-Yu Tsai, linux-arm-kernel, linux-clk, linux-kernel, Weiyi Lu, linux-mediatek 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); > > > > > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] clk: mediatek: Introduce need_pm_runtime to mtk_clk_desc 2024-01-04 14:51 ` Pin-yen Lin @ 2024-01-05 8:39 ` Pin-yen Lin 0 siblings, 0 replies; 7+ messages in thread From: Pin-yen Lin @ 2024-01-05 8:39 UTC (permalink / raw) To: Eugen Hristev, AngeloGioacchino Del Regno Cc: Michael Turquette, Stephen Boyd, Matthias Brugger, Chen-Yu Tsai, linux-arm-kernel, linux-clk, linux-kernel, Weiyi Lu, linux-mediatek 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); > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-01-05 8:40 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-01-02 8:12 [PATCH v2 1/2] clk: mediatek: Introduce need_pm_runtime to mtk_clk_desc Pin-yen Lin 2024-01-02 8:12 ` [PATCH v2 2/2] clk: mediatek: mt8183: Enable need_runtime_pm on mt8183-mfgcfg Pin-yen Lin 2024-01-03 7:45 ` [PATCH v2 1/2] clk: mediatek: Introduce need_pm_runtime to mtk_clk_desc Chen-Yu Tsai 2024-01-03 12:19 ` AngeloGioacchino Del Regno 2024-01-03 13:20 ` Eugen Hristev 2024-01-04 14:51 ` Pin-yen Lin 2024-01-05 8:39 ` Pin-yen Lin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®