public inbox for linux-clk@vger.kernel.org
 help / color / mirror / Atom feed
From: Marek Vasut <marex@denx.de>
To: linux-pm@vger.kernel.org
Cc: Marek Vasut <marex@denx.de>, Adam Ford <aford173@gmail.com>,
	Fabio Estevam <festevam@denx.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jacky Bai <ping.bai@nxp.com>, Kevin Hilman <khilman@kernel.org>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Len Brown <len.brown@intel.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Lucas Stach <l.stach@pengutronix.de>,
	Mark Brown <broonie@kernel.org>,
	Martin Kepplinger <martink@posteo.de>,
	Pavel Machek <pavel@ucw.cz>, Peng Fan <peng.fan@nxp.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Shawn Guo <shawnguo@kernel.org>,
	Shengjiu Wang <shengjiu.wang@nxp.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	linux-clk@vger.kernel.org, linux-imx@nxp.com
Subject: [PATCH 3/3] [RFC] soc: imx: imx8m-blk-ctrl: Split clock prepare from clock enable in the domain
Date: Tue,  8 Nov 2022 02:35:17 +0100	[thread overview]
Message-ID: <20221108013517.749665-3-marex@denx.de> (raw)
In-Reply-To: <20221108013517.749665-1-marex@denx.de>

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 --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;
 
-- 
2.35.1


  parent reply	other threads:[~2022-11-08  1:36 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-08  1:35 [PATCH 1/3] [RFC] PM: domains: Introduce .power_pre/post_on/off callbacks Marek Vasut
2022-11-08  1:35 ` [PATCH 2/3] [RFC] soc: imx: gpcv2: Split clock prepare from clock enable in the domain Marek Vasut
2022-11-11  8:27   ` Peng Fan
2022-11-08  1:35 ` Marek Vasut [this message]
2022-11-09 13:19 ` [PATCH 1/3] [RFC] PM: domains: Introduce .power_pre/post_on/off callbacks Laurent Pinchart
2022-11-09 13:25   ` Marek Vasut
2022-11-14 19:40 ` Ulf Hansson
2022-11-14 20:32   ` Marek Vasut
2022-11-16 12:41     ` Ulf Hansson
2022-11-16 13:25       ` Lucas Stach
2022-11-16 16:30         ` Ulf Hansson
2023-01-04  8:37           ` Peng Fan
2023-01-18 12:55             ` Ulf Hansson
2023-01-18 13:07               ` Marek Vasut
2023-02-16  1:47               ` Peng Fan
2023-02-16 10:48                 ` Ulf Hansson
2023-03-01  0:52                   ` Peng Fan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221108013517.749665-3-marex@denx.de \
    --to=marex@denx.de \
    --cc=aford173@gmail.com \
    --cc=broonie@kernel.org \
    --cc=festevam@denx.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel@pengutronix.de \
    --cc=khilman@kernel.org \
    --cc=l.stach@pengutronix.de \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=len.brown@intel.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-pm@vger.kernel.org \
    --cc=martink@posteo.de \
    --cc=p.zabel@pengutronix.de \
    --cc=pavel@ucw.cz \
    --cc=peng.fan@nxp.com \
    --cc=ping.bai@nxp.com \
    --cc=rafael@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=sboyd@kernel.org \
    --cc=shawnguo@kernel.org \
    --cc=shengjiu.wang@nxp.com \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox