diff mbox series

[3/3,RFC] soc: imx: imx8m-blk-ctrl: Split clock prepare from clock enable in the domain

Message ID 20221108013517.749665-3-marex@denx.de (mailing list archive)
State RFC, archived
Headers show
Series [1/3,RFC] PM: domains: Introduce .power_pre/post_on/off callbacks | expand

Commit Message

Marek Vasut Nov. 8, 2022, 1:35 a.m. UTC 
It is possible for clk_disable_unused() to trigger lockdep warning
regarding lock ordering in this driver. This happens in case of the
following conditions:

A) clock core clk_disable_unused() triggers the following sequence in a
   driver which also uses blkctrl domain:
   - clk_prepare_lock() -> obtains clock core prepare_lock
   - pm_runtime_get*() -> obtains &blk_ctrl_genpd_lock_class

B) driver powers up a power domain and triggers the following sequence
   in blkctrl:
   - pm_runtime_get_sync() -> obtains &blk_ctrl_genpd_lock_class
   - clk_bulk_prepare_enable() -> obtains clock core prepare_lock

This can lead to a deadlock in case A and B runs on separate CPUs.

To avoid the deadlock, split clk_*prepare() from clk_*enable() and call
the former in power_pre_on() callback, before pm_runtime_get_sync(). The
reverse is implemented in the power_off_post() callback in the same way.
This way, the blkctrl driver always claims the prepare_lock before
blk_ctrl_genpd_lock_class and the deadlock is avoided.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Adam Ford <aford173@gmail.com>
Cc: Fabio Estevam <festevam@denx.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jacky Bai <ping.bai@nxp.com>
Cc: Kevin Hilman <khilman@kernel.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Len Brown <len.brown@intel.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Marek Vasut <marex@denx.de>
Cc: Mark Brown <broonie@kernel.org>
Cc: Martin Kepplinger <martink@posteo.de>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Shengjiu Wang <shengjiu.wang@nxp.com>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: linux-clk@vger.kernel.org
Cc: linux-imx@nxp.com
Cc: linux-pm@vger.kernel.org
To: linux-arm-kernel@lists.infradead.org
---
 drivers/soc/imx/imx8mp-blk-ctrl.c | 38 +++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 5 deletions(-)
diff mbox series

Patch

diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-blk-ctrl.c
index ca4366e264783..844039d4e6bd2 100644
--- a/drivers/soc/imx/imx8mp-blk-ctrl.c
+++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
@@ -408,6 +408,30 @@  static const struct imx8mp_blk_ctrl_data imx8mp_hdmi_blk_ctl_dev_data = {
 	.num_domains = ARRAY_SIZE(imx8mp_hdmi_domain_data),
 };
 
+static int imx8mp_blk_ctrl_power_pre_on(struct generic_pm_domain *genpd)
+{
+	struct imx8mp_blk_ctrl_domain *domain = to_imx8mp_blk_ctrl_domain(genpd);
+	const struct imx8mp_blk_ctrl_domain_data *data = domain->data;
+	struct imx8mp_blk_ctrl *bc = domain->bc;
+	int ret;
+
+	ret = clk_bulk_prepare(data->num_clks, domain->clks);
+	if (ret)
+		dev_err(bc->dev, "failed to enable clocks\n");
+
+	return ret;
+}
+
+static int imx8mp_blk_ctrl_power_off_post(struct generic_pm_domain *genpd)
+{
+	struct imx8mp_blk_ctrl_domain *domain = to_imx8mp_blk_ctrl_domain(genpd);
+	const struct imx8mp_blk_ctrl_domain_data *data = domain->data;
+
+	clk_bulk_unprepare(data->num_clks, domain->clks);
+
+	return 0;
+}
+
 static int imx8mp_blk_ctrl_power_on(struct generic_pm_domain *genpd)
 {
 	struct imx8mp_blk_ctrl_domain *domain = to_imx8mp_blk_ctrl_domain(genpd);
@@ -423,7 +447,7 @@  static int imx8mp_blk_ctrl_power_on(struct generic_pm_domain *genpd)
 	}
 
 	/* enable upstream clocks */
-	ret = clk_bulk_prepare_enable(data->num_clks, domain->clks);
+	ret = clk_bulk_enable(data->num_clks, domain->clks);
 	if (ret) {
 		dev_err(bc->dev, "failed to enable clocks\n");
 		goto bus_put;
@@ -443,12 +467,12 @@  static int imx8mp_blk_ctrl_power_on(struct generic_pm_domain *genpd)
 	if (ret)
 		dev_err(bc->dev, "failed to set icc bw\n");
 
-	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
+	clk_bulk_disable(data->num_clks, domain->clks);
 
 	return 0;
 
 clk_disable:
-	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
+	clk_bulk_disable(data->num_clks, domain->clks);
 bus_put:
 	pm_runtime_put(bc->bus_power_dev);
 
@@ -462,7 +486,7 @@  static int imx8mp_blk_ctrl_power_off(struct generic_pm_domain *genpd)
 	struct imx8mp_blk_ctrl *bc = domain->bc;
 	int ret;
 
-	ret = clk_bulk_prepare_enable(data->num_clks, domain->clks);
+	ret = clk_bulk_enable(data->num_clks, domain->clks);
 	if (ret) {
 		dev_err(bc->dev, "failed to enable clocks\n");
 		return ret;
@@ -471,7 +495,7 @@  static int imx8mp_blk_ctrl_power_off(struct generic_pm_domain *genpd)
 	/* domain specific blk-ctrl manipulation */
 	bc->power_off(bc, domain);
 
-	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
+	clk_bulk_disable(data->num_clks, domain->clks);
 
 	/* power down upstream GPC domain */
 	pm_runtime_put(domain->power_dev);
@@ -585,8 +609,12 @@  static int imx8mp_blk_ctrl_probe(struct platform_device *pdev)
 		dev_set_name(domain->power_dev, "%s", data->name);
 
 		domain->genpd.name = data->name;
+		domain->genpd.power_pre_on = imx8mp_blk_ctrl_power_pre_on;
 		domain->genpd.power_on = imx8mp_blk_ctrl_power_on;
+		domain->genpd.power_post_on = imx8mp_blk_ctrl_power_off_post;
+		domain->genpd.power_off_pre = imx8mp_blk_ctrl_power_pre_on;
 		domain->genpd.power_off = imx8mp_blk_ctrl_power_off;
+		domain->genpd.power_off_post = imx8mp_blk_ctrl_power_off_post;
 		domain->bc = bc;
 		domain->id = i;