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
next prev 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: linkBe 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.