From: Lucas Stach <l.stach@pengutronix.de>
To: Shawn Guo <shawnguo@kernel.org>, Rob Herring <robh+dt@kernel.org>
Cc: NXP Linux Team <linux-imx@nxp.com>,
Fabio Estevam <festevam@gmail.com>,
Frieder Schrempf <frieder.schrempf@kontron.de>,
Marek Vasut <marex@denx.de>,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
kernel@pengutronix.de, patchwork-lst@pengutronix.de
Subject: [PATCH 03/11] soc: imx: gpcv2: split power up and power down sequence control
Date: Wed, 30 Sep 2020 17:49:58 +0200 [thread overview]
Message-ID: <20200930155006.535712-4-l.stach@pengutronix.de> (raw)
In-Reply-To: <20200930155006.535712-1-l.stach@pengutronix.de>
The current mixed function to control both power up and power down
sequences is very hard to follow and already contains some sequence
errors like triggering the ADB400 handshake at the wrong time due to
this. Split the function into two, which results in slightly more
code, but is way easier to get right.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/soc/imx/gpcv2.c | 149 +++++++++++++++++++++++++---------------
1 file changed, 95 insertions(+), 54 deletions(-)
diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index dc420734b16c..f91063c9fb92 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -127,20 +127,19 @@ struct imx_pgc_domain_data {
const struct regmap_access_table *reg_access_table;
};
-static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
- bool on)
+static inline struct imx_pgc_domain *
+to_imx_pgc_domain(struct generic_pm_domain *genpd)
{
- struct imx_pgc_domain *domain = container_of(genpd,
- struct imx_pgc_domain,
- genpd);
- unsigned int offset = on ?
- GPC_PU_PGC_SW_PUP_REQ : GPC_PU_PGC_SW_PDN_REQ;
- const bool enable_power_control = !on;
- const bool has_regulator = !IS_ERR(domain->regulator);
- int i, ret = 0;
- u32 pxx_req;
-
- if (has_regulator && on) {
+ return container_of(genpd, struct imx_pgc_domain, genpd);
+}
+
+static int imx_pgc_power_up(struct generic_pm_domain *genpd)
+{
+ struct imx_pgc_domain *domain = to_imx_pgc_domain(genpd);
+ u32 reg_val;
+ int i, ret;
+
+ if (!IS_ERR(domain->regulator)) {
ret = regulator_enable(domain->regulator);
if (ret) {
dev_err(domain->dev, "failed to enable regulator\n");
@@ -149,69 +148,111 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
}
/* Enable reset clocks for all devices in the domain */
+ for (i = 0; i < domain->num_clks; i++) {
+ ret = clk_prepare_enable(domain->clk[i]);
+ if (ret) {
+ dev_err(domain->dev, "failed to enable clock %d\n", i);
+ goto out_clk_disable;
+ }
+ }
+
+ /* request the domain to power up */
+ regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
+ domain->bits.pxx, domain->bits.pxx);
+ /*
+ * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
+ * for PUP_REQ/PDN_REQ bit to be cleared
+ */
+ ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
+ reg_val, !(reg_val & domain->bits.pxx),
+ 0, USEC_PER_MSEC);
+ if (ret) {
+ dev_err(domain->dev, "failed to command PGC\n");
+ goto out_clk_disable;
+ }
+
+ /* disable power control */
+ regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
+ GPC_PGC_CTRL_PCR, 0);
+
+ /* request the ADB400 to power up */
+ if (domain->bits.hsk)
+ regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
+ domain->bits.hsk, domain->bits.hsk);
+
+ /* Disable reset clocks for all devices in the domain */
for (i = 0; i < domain->num_clks; i++)
- clk_prepare_enable(domain->clk[i]);
+ clk_disable_unprepare(domain->clk[i]);
+
+ return 0;
- if (enable_power_control)
- regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
- GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
+out_clk_disable:
+ for (i--; i >= 0; i--)
+ clk_disable_unprepare(domain->clk[i]);
+ if (!IS_ERR(domain->regulator))
+ regulator_disable(domain->regulator);
+
+ return ret;
+}
+
+static int imx_pgc_power_down(struct generic_pm_domain *genpd)
+{
+ struct imx_pgc_domain *domain = to_imx_pgc_domain(genpd);
+ u32 reg_val;
+ int i, ret;
+ /* Enable reset clocks for all devices in the domain */
+ for (i = 0; i < domain->num_clks; i++) {
+ ret = clk_prepare_enable(domain->clk[i]);
+ if (ret) {
+ dev_err(domain->dev, "failed to enable clock %d\n", i);
+ goto out_clk_disable;
+ }
+ }
+
+ /* request the ADB400 to power down */
if (domain->bits.hsk)
regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
- domain->bits.hsk, on ? domain->bits.hsk : 0);
+ domain->bits.hsk, 0);
- regmap_update_bits(domain->regmap, offset,
- domain->bits.pxx, domain->bits.pxx);
+ /* enable power control */
+ regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
+ GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
+ /* request the domain to power down */
+ regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PDN_REQ,
+ domain->bits.pxx, domain->bits.pxx);
/*
* As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
* for PUP_REQ/PDN_REQ bit to be cleared
*/
- ret = regmap_read_poll_timeout(domain->regmap, offset, pxx_req,
- !(pxx_req & domain->bits.pxx),
+ ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PGC_SW_PDN_REQ,
+ reg_val, !(reg_val & domain->bits.pxx),
0, USEC_PER_MSEC);
if (ret) {
dev_err(domain->dev, "failed to command PGC\n");
- /*
- * If we were in a process of enabling a
- * domain and failed we might as well disable
- * the regulator we just enabled. And if it
- * was the opposite situation and we failed to
- * power down -- keep the regulator on
- */
- on = !on;
+ goto out_clk_disable;
}
- if (enable_power_control)
- regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
- GPC_PGC_CTRL_PCR, 0);
-
/* Disable reset clocks for all devices in the domain */
for (i = 0; i < domain->num_clks; i++)
clk_disable_unprepare(domain->clk[i]);
- if (has_regulator && !on) {
- int err;
-
- err = regulator_disable(domain->regulator);
- if (err)
- dev_err(domain->dev,
- "failed to disable regulator: %d\n", err);
- /* Preserve earlier error code */
- ret = ret ?: err;
+ if (!IS_ERR(domain->regulator)) {
+ ret = regulator_disable(domain->regulator);
+ if (ret) {
+ dev_err(domain->dev, "failed to disable regulator\n");
+ return ret;
+ }
}
- return ret;
-}
+ return 0;
-static int imx_gpc_pu_pgc_sw_pup_req(struct generic_pm_domain *genpd)
-{
- return imx_gpc_pu_pgc_sw_pxx_req(genpd, true);
-}
+out_clk_disable:
+ for (i--; i >= 0; i--)
+ clk_disable_unprepare(domain->clk[i]);
-static int imx_gpc_pu_pgc_sw_pdn_req(struct generic_pm_domain *genpd)
-{
- return imx_gpc_pu_pgc_sw_pxx_req(genpd, false);
+ return 0;
}
static const struct imx_pgc_domain imx7_pgc_domains[] = {
@@ -631,8 +672,8 @@ static int imx_gpcv2_probe(struct platform_device *pdev)
domain = pd_pdev->dev.platform_data;
domain->regmap = regmap;
- domain->genpd.power_on = imx_gpc_pu_pgc_sw_pup_req;
- domain->genpd.power_off = imx_gpc_pu_pgc_sw_pdn_req;
+ domain->genpd.power_on = imx_pgc_power_up;
+ domain->genpd.power_off = imx_pgc_power_down;
pd_pdev->dev.parent = dev;
pd_pdev->dev.of_node = np;
--
2.20.1
next prev parent reply other threads:[~2020-09-30 15:50 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-30 15:49 [PATCH 00/11] i.MX8MM power domain support Lucas Stach
2020-09-30 15:49 ` [PATCH 01/11] soc: imx: gpcv2: move to more ideomatic error handling in probe Lucas Stach
2020-09-30 16:04 ` Marek Vasut
2020-09-30 15:49 ` [PATCH 02/11] soc: imx: gpcv2: move domain mapping to domain driver probe Lucas Stach
2020-09-30 16:07 ` Marek Vasut
2020-09-30 15:49 ` Lucas Stach [this message]
2020-09-30 16:10 ` [PATCH 03/11] soc: imx: gpcv2: split power up and power down sequence control Marek Vasut
2020-09-30 15:49 ` [PATCH 04/11] soc: imx: gpcv2: wait for ADB400 handshake Lucas Stach
2020-09-30 16:11 ` Marek Vasut
2020-09-30 16:19 ` Lucas Stach
2020-09-30 16:23 ` Marek Vasut
2020-10-09 3:05 ` Jacky Bai
2020-10-09 7:27 ` Marek Vasut
2020-10-09 7:51 ` Jacky Bai
2020-10-09 8:19 ` Marek Vasut
2020-09-30 15:50 ` [PATCH 05/11] soc: imx: gpcv2: add runtime PM support for power-domains Lucas Stach
2020-09-30 16:14 ` Marek Vasut
2020-09-30 16:20 ` Lucas Stach
2020-09-30 15:50 ` [PATCH 06/11] soc: imx: gpcv2: allow domains without power-sequence control Lucas Stach
2020-10-09 7:54 ` Jacky Bai
2020-10-09 7:57 ` Jacky Bai
2020-09-30 15:50 ` [PATCH 07/11] soc: imx: gpcv2: add support for optional resets Lucas Stach
2020-09-30 16:15 ` Marek Vasut
2020-09-30 16:23 ` Lucas Stach
2020-09-30 16:30 ` Marek Vasut
2020-09-30 16:34 ` Lucas Stach
2020-09-30 16:38 ` Marek Vasut
2020-10-01 8:59 ` Krzysztof Kozlowski
2020-10-06 19:42 ` Rob Herring
2020-09-30 15:50 ` [PATCH 08/11] dt-bindings: add defines for i.MX8MM power domains Lucas Stach
2020-10-01 8:54 ` Krzysztof Kozlowski
2020-10-06 19:47 ` Rob Herring
2020-09-30 15:50 ` [PATCH 09/11] soc: imx: gpcv2: add support " Lucas Stach
2020-09-30 16:18 ` Marek Vasut
2020-09-30 15:50 ` [PATCH 10/11] arm64: dts: imx8mm: add GPC node and " Lucas Stach
2020-09-30 16:20 ` Marek Vasut
2020-10-01 8:51 ` Krzysztof Kozlowski
2020-10-23 13:22 ` Adam Ford
2020-10-23 14:39 ` Jacky Bai
2020-10-26 10:56 ` Abel Vesa
2020-10-26 11:01 ` Abel Vesa
2020-10-26 11:13 ` Adam Ford
2020-10-26 11:02 ` Lucas Stach
2020-09-30 15:50 ` [PATCH 11/11] arm64: dts: imx8mm: put USB controllers into power-domains Lucas Stach
2020-10-01 7:46 ` [PATCH 00/11] i.MX8MM power domain support Frieder Schrempf
2020-10-03 18:03 ` Adam Ford
[not found] ` <CAHCN7xKjWEwQr9y0QLrR6KVT=ut=v=coqt4beAvrz1kQSGbX1g@mail.gmail.com>
2020-10-03 18:08 ` Marek Vasut
2020-10-03 18:11 ` Adam Ford
2020-10-08 20:47 ` Adam Ford
2020-10-09 3:00 ` Jacky Bai
2020-10-09 11:12 ` Lucas Stach
2020-10-09 12:57 ` Adam Ford
2020-10-10 2:16 ` Jacky Bai
2020-10-13 18:26 ` Lucas Stach
2020-10-14 1:23 ` Peng Fan
2020-10-22 8:24 ` Lucas Stach
2020-10-22 16:36 ` Fabio Estevam
2020-10-28 13:50 ` Peng Fan
2020-10-31 13:56 ` Adam Ford
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=20200930155006.535712-4-l.stach@pengutronix.de \
--to=l.stach@pengutronix.de \
--cc=devicetree@vger.kernel.org \
--cc=festevam@gmail.com \
--cc=frieder.schrempf@kontron.de \
--cc=kernel@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-imx@nxp.com \
--cc=marex@denx.de \
--cc=patchwork-lst@pengutronix.de \
--cc=robh+dt@kernel.org \
--cc=shawnguo@kernel.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;
as well as URLs for NNTP newsgroup(s).