All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: linux-pwm@vger.kernel.org, Shawn Guo <shawnguo@kernel.org>,
	NXP Linux Team <linux-imx@nxp.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/3] pwm: imx27: Ensure clocks being on iff the PWM is on
Date: Mon, 10 Feb 2020 22:22:40 +0100	[thread overview]
Message-ID: <20200210212240.25513-4-u.kleine-koenig@pengutronix.de> (raw)
In-Reply-To: <20200210212240.25513-1-u.kleine-koenig@pengutronix.de>

Up to now the .probe function didn't enable clks and relied on the core
to call the .get_state() callback to have the clk running. The latter
enabled the needed clocks and kept them running if the PWM is enabled.

This only works correctly if the .get_state() callback is called exactly
once and this single call happens before unused clocks are disabled by
the clk core.

The former wasn't true for a short period while commit 01ccf903edd6
("pwm: Let pwm_get_state() return the last implemented state") applied
and not not reverted yet and might become wrong in the future.

The latter isn't true any more since commit cfc4c189bc70 ("pwm: Read
initial hardware state at request time") which results in a running PWM
being stopped at boot time if for example the consumer lives in a kernel
module that is only loaded after the clk core disabled unused clocks.

So ensure .probe() is left with the clocks on if the PWM is running and
.get_state() disables everything it enabled.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-imx27.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c
index fb142813d455..e83c077bb7cc 100644
--- a/drivers/pwm/pwm-imx27.c
+++ b/drivers/pwm/pwm-imx27.c
@@ -171,8 +171,7 @@ static void pwm_imx27_get_state(struct pwm_chip *chip,
 	tmp = NSEC_PER_SEC * (u64)(val);
 	state->duty_cycle = DIV_ROUND_CLOSEST_ULL(tmp, pwm_clk);
 
-	if (!state->enabled)
-		pwm_imx27_clk_disable_unprepare(imx);
+	pwm_imx27_clk_disable_unprepare(imx);
 }
 
 static void pwm_imx27_sw_reset(struct pwm_chip *chip)
@@ -307,6 +306,8 @@ MODULE_DEVICE_TABLE(of, pwm_imx27_dt_ids);
 static int pwm_imx27_probe(struct platform_device *pdev)
 {
 	struct pwm_imx27_chip *imx;
+	int ret;
+	u32 pwmcr;
 
 	imx = devm_kzalloc(&pdev->dev, sizeof(*imx), GFP_KERNEL);
 	if (imx == NULL)
@@ -349,6 +350,15 @@ static int pwm_imx27_probe(struct platform_device *pdev)
 	if (IS_ERR(imx->mmio_base))
 		return PTR_ERR(imx->mmio_base);
 
+	ret = pwm_imx27_clk_prepare_enable(imx);
+	if (ret)
+		return ret;
+
+	/* keep clks on if pwm is running */
+	pwmcr = readl(imx->mmio_base + MX3_PWMCR);
+	if (!(pwmcr & MX3_PWMCR_EN))
+		pwm_imx27_clk_disable_unprepare(imx);
+
 	return pwmchip_add(&imx->chip);
 }
 
-- 
2.24.0

WARNING: multiple messages have this Message-ID (diff)
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: linux-pwm@vger.kernel.org, Shawn Guo <shawnguo@kernel.org>,
	NXP Linux Team <linux-imx@nxp.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/3] pwm: imx27: Ensure clocks being on iff the PWM is on
Date: Mon, 10 Feb 2020 22:22:40 +0100	[thread overview]
Message-ID: <20200210212240.25513-4-u.kleine-koenig@pengutronix.de> (raw)
In-Reply-To: <20200210212240.25513-1-u.kleine-koenig@pengutronix.de>

Up to now the .probe function didn't enable clks and relied on the core
to call the .get_state() callback to have the clk running. The latter
enabled the needed clocks and kept them running if the PWM is enabled.

This only works correctly if the .get_state() callback is called exactly
once and this single call happens before unused clocks are disabled by
the clk core.

The former wasn't true for a short period while commit 01ccf903edd6
("pwm: Let pwm_get_state() return the last implemented state") applied
and not not reverted yet and might become wrong in the future.

The latter isn't true any more since commit cfc4c189bc70 ("pwm: Read
initial hardware state at request time") which results in a running PWM
being stopped at boot time if for example the consumer lives in a kernel
module that is only loaded after the clk core disabled unused clocks.

So ensure .probe() is left with the clocks on if the PWM is running and
.get_state() disables everything it enabled.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-imx27.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c
index fb142813d455..e83c077bb7cc 100644
--- a/drivers/pwm/pwm-imx27.c
+++ b/drivers/pwm/pwm-imx27.c
@@ -171,8 +171,7 @@ static void pwm_imx27_get_state(struct pwm_chip *chip,
 	tmp = NSEC_PER_SEC * (u64)(val);
 	state->duty_cycle = DIV_ROUND_CLOSEST_ULL(tmp, pwm_clk);
 
-	if (!state->enabled)
-		pwm_imx27_clk_disable_unprepare(imx);
+	pwm_imx27_clk_disable_unprepare(imx);
 }
 
 static void pwm_imx27_sw_reset(struct pwm_chip *chip)
@@ -307,6 +306,8 @@ MODULE_DEVICE_TABLE(of, pwm_imx27_dt_ids);
 static int pwm_imx27_probe(struct platform_device *pdev)
 {
 	struct pwm_imx27_chip *imx;
+	int ret;
+	u32 pwmcr;
 
 	imx = devm_kzalloc(&pdev->dev, sizeof(*imx), GFP_KERNEL);
 	if (imx == NULL)
@@ -349,6 +350,15 @@ static int pwm_imx27_probe(struct platform_device *pdev)
 	if (IS_ERR(imx->mmio_base))
 		return PTR_ERR(imx->mmio_base);
 
+	ret = pwm_imx27_clk_prepare_enable(imx);
+	if (ret)
+		return ret;
+
+	/* keep clks on if pwm is running */
+	pwmcr = readl(imx->mmio_base + MX3_PWMCR);
+	if (!(pwmcr & MX3_PWMCR_EN))
+		pwm_imx27_clk_disable_unprepare(imx);
+
 	return pwmchip_add(&imx->chip);
 }
 
-- 
2.24.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2020-02-10 21:23 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-09 21:31 [PATCH] pwm: imx27: fix clk handling in pwm_imx27_apply() Uwe Kleine-König
2020-02-09 21:31 ` Uwe Kleine-König
2020-02-10 21:22 ` [PATCH 0/3] pwm: imx27: fix clk handling Uwe Kleine-König
2020-02-10 21:22   ` Uwe Kleine-König
2020-02-10 21:22   ` [PATCH 1/3] pwm: imx27: Simplify helper function to enable and disable clocks Uwe Kleine-König
2020-02-10 21:22     ` Uwe Kleine-König
2020-02-10 21:22   ` [PATCH 2/3] pwm: imx27: Don't disable clks at device remove time Uwe Kleine-König
2020-02-10 21:22     ` Uwe Kleine-König
2020-02-10 21:22   ` Uwe Kleine-König [this message]
2020-02-10 21:22     ` [PATCH 3/3] pwm: imx27: Ensure clocks being on iff the PWM is on Uwe Kleine-König

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=20200210212240.25513-4-u.kleine-koenig@pengutronix.de \
    --to=u.kleine-koenig@pengutronix.de \
    --cc=festevam@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-pwm@vger.kernel.org \
    --cc=shawnguo@kernel.org \
    --cc=thierry.reding@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.