1. Patchew
  2. linux
  3. View series

[PATCH] drivers/clocksource/timer-ti-dm: Don't call clk_get_rate() in stop function

Ivaylo Dimitrov posted 1 patch 1 year, 2 months ago
There is a newer version of this series
drivers/clocksource/timer-ti-dm.c | 36 ++++++++++++++++++++++++++++--------
1 file changed, 28 insertions(+), 8 deletions(-)
[PATCH] drivers/clocksource/timer-ti-dm: Don't call clk_get_rate() in stop function
Posted by Ivaylo Dimitrov 1 year, 2 months ago 
clk_get_rate() might sleep, and that prevents dm-timer based PWM from being
used from atomic context.

Fix that by getting fclk rate in probe() and using a notifier in case rate
changes.

Fixes: af04aa856e93 ("ARM: OMAP: Move dmtimer driver out of plat-omap to drivers under clocksource")
Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
---
 drivers/clocksource/timer-ti-dm.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/clocksource/timer-ti-dm.c b/drivers/clocksource/timer-ti-dm.c
index 09ab29c..5f60f6b 100644
--- a/drivers/clocksource/timer-ti-dm.c
+++ b/drivers/clocksource/timer-ti-dm.c
@@ -140,6 +140,8 @@ struct dmtimer {
 	struct platform_device *pdev;
 	struct list_head node;
 	struct notifier_block nb;
+	struct notifier_block fclk_nb;
+	unsigned long fclk_rate;
 };
 
 static u32 omap_reserved_systimers;
@@ -253,8 +255,7 @@ static inline void __omap_dm_timer_enable_posted(struct dmtimer *timer)
 	timer->posted = OMAP_TIMER_POSTED;
 }
 
-static inline void __omap_dm_timer_stop(struct dmtimer *timer,
-					unsigned long rate)
+static inline void __omap_dm_timer_stop(struct dmtimer *timer)
 {
 	u32 l;
 
@@ -269,7 +270,7 @@ static inline void __omap_dm_timer_stop(struct dmtimer *timer,
 		 * Wait for functional clock period x 3.5 to make sure that
 		 * timer is stopped
 		 */
-		udelay(3500000 / rate + 1);
+		udelay(3500000 / timer->fclk_rate + 1);
 #endif
 	}
 
@@ -348,6 +349,21 @@ static int omap_timer_context_notifier(struct notifier_block *nb,
 	return NOTIFY_OK;
 }
 
+static int omap_timer_fclk_notifier(struct notifier_block *nb,
+				    unsigned long event, void *data)
+{
+	struct clk_notifier_data *clk_data = data;
+	struct dmtimer *timer = container_of(nb, struct dmtimer, fclk_nb);
+
+	switch (event) {
+	case POST_RATE_CHANGE:
+		timer->fclk_rate = clk_data->new_rate;
+		return NOTIFY_OK;
+	default:
+		return NOTIFY_DONE;
+	}
+}
+
 static int omap_dm_timer_reset(struct dmtimer *timer)
 {
 	u32 l, timeout = 100000;
@@ -754,7 +770,6 @@ static int omap_dm_timer_stop(struct omap_dm_timer *cookie)
 {
 	struct dmtimer *timer;
 	struct device *dev;
-	unsigned long rate = 0;
 
 	timer = to_dmtimer(cookie);
 	if (unlikely(!timer))
@@ -762,10 +777,7 @@ static int omap_dm_timer_stop(struct omap_dm_timer *cookie)
 
 	dev = &timer->pdev->dev;
 
-	if (!timer->omap1)
-		rate = clk_get_rate(timer->fclk);
-
-	__omap_dm_timer_stop(timer, rate);
+	__omap_dm_timer_stop(timer);
 
 	pm_runtime_put_sync(dev);
 
@@ -1124,6 +1136,14 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
 		timer->fclk = devm_clk_get(dev, "fck");
 		if (IS_ERR(timer->fclk))
 			return PTR_ERR(timer->fclk);
+
+		timer->fclk_nb.notifier_call = omap_timer_fclk_notifier;
+		ret = devm_clk_notifier_register(dev, timer->fclk,
+						 &timer->fclk_nb);
+		if (ret)
+			return ret;
+
+		timer->fclk_rate = clk_get_rate(timer->fclk);
 	} else {
 		timer->fclk = ERR_PTR(-ENODEV);
 	}
-- 
1.9.1
Re: [PATCH] drivers/clocksource/timer-ti-dm: Don't call clk_get_rate() in stop function
Posted by Sean Young 11 months, 2 weeks ago 
Hi Ivaylo,

On Tue, Oct 03, 2023 at 08:50:20AM +0300, Ivaylo Dimitrov wrote:
> clk_get_rate() might sleep, and that prevents dm-timer based PWM from being
> used from atomic context.

Now that this is merged, pwm-ir-tx can only use the pwm in atomic context
if pwm-omap-timer.c sets the atomic field like the rpi pwm does here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pwm/pwm-bcm2835.c#n175

see pwm-ir-tx here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/rc/pwm-ir-tx.c#n170

It would be nice to have this tested and working properly for the n900.

Thanks,

Sean


> 
> Fix that by getting fclk rate in probe() and using a notifier in case rate
> changes.
> 
> Fixes: af04aa856e93 ("ARM: OMAP: Move dmtimer driver out of plat-omap to drivers under clocksource")
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> ---
>  drivers/clocksource/timer-ti-dm.c | 36 ++++++++++++++++++++++++++++--------
>  1 file changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/clocksource/timer-ti-dm.c b/drivers/clocksource/timer-ti-dm.c
> index 09ab29c..5f60f6b 100644
> --- a/drivers/clocksource/timer-ti-dm.c
> +++ b/drivers/clocksource/timer-ti-dm.c
> @@ -140,6 +140,8 @@ struct dmtimer {
>  	struct platform_device *pdev;
>  	struct list_head node;
>  	struct notifier_block nb;
> +	struct notifier_block fclk_nb;
> +	unsigned long fclk_rate;
>  };
>  
>  static u32 omap_reserved_systimers;
> @@ -253,8 +255,7 @@ static inline void __omap_dm_timer_enable_posted(struct dmtimer *timer)
>  	timer->posted = OMAP_TIMER_POSTED;
>  }
>  
> -static inline void __omap_dm_timer_stop(struct dmtimer *timer,
> -					unsigned long rate)
> +static inline void __omap_dm_timer_stop(struct dmtimer *timer)
>  {
>  	u32 l;
>  
> @@ -269,7 +270,7 @@ static inline void __omap_dm_timer_stop(struct dmtimer *timer,
>  		 * Wait for functional clock period x 3.5 to make sure that
>  		 * timer is stopped
>  		 */
> -		udelay(3500000 / rate + 1);
> +		udelay(3500000 / timer->fclk_rate + 1);
>  #endif
>  	}
>  
> @@ -348,6 +349,21 @@ static int omap_timer_context_notifier(struct notifier_block *nb,
>  	return NOTIFY_OK;
>  }
>  
> +static int omap_timer_fclk_notifier(struct notifier_block *nb,
> +				    unsigned long event, void *data)
> +{
> +	struct clk_notifier_data *clk_data = data;
> +	struct dmtimer *timer = container_of(nb, struct dmtimer, fclk_nb);
> +
> +	switch (event) {
> +	case POST_RATE_CHANGE:
> +		timer->fclk_rate = clk_data->new_rate;
> +		return NOTIFY_OK;
> +	default:
> +		return NOTIFY_DONE;
> +	}
> +}
> +
>  static int omap_dm_timer_reset(struct dmtimer *timer)
>  {
>  	u32 l, timeout = 100000;
> @@ -754,7 +770,6 @@ static int omap_dm_timer_stop(struct omap_dm_timer *cookie)
>  {
>  	struct dmtimer *timer;
>  	struct device *dev;
> -	unsigned long rate = 0;
>  
>  	timer = to_dmtimer(cookie);
>  	if (unlikely(!timer))
> @@ -762,10 +777,7 @@ static int omap_dm_timer_stop(struct omap_dm_timer *cookie)
>  
>  	dev = &timer->pdev->dev;
>  
> -	if (!timer->omap1)
> -		rate = clk_get_rate(timer->fclk);
> -
> -	__omap_dm_timer_stop(timer, rate);
> +	__omap_dm_timer_stop(timer);
>  
>  	pm_runtime_put_sync(dev);
>  
> @@ -1124,6 +1136,14 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
>  		timer->fclk = devm_clk_get(dev, "fck");
>  		if (IS_ERR(timer->fclk))
>  			return PTR_ERR(timer->fclk);
> +
> +		timer->fclk_nb.notifier_call = omap_timer_fclk_notifier;
> +		ret = devm_clk_notifier_register(dev, timer->fclk,
> +						 &timer->fclk_nb);
> +		if (ret)
> +			return ret;
> +
> +		timer->fclk_rate = clk_get_rate(timer->fclk);
>  	} else {
>  		timer->fclk = ERR_PTR(-ENODEV);
>  	}
> -- 
> 1.9.1
Re: [PATCH] drivers/clocksource/timer-ti-dm: Don't call clk_get_rate() in stop function
Posted by Tony Lindgren 1 year, 2 months ago 
* Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [231003 05:50]:
> clk_get_rate() might sleep, and that prevents dm-timer based PWM from being
> used from atomic context.
> 
> Fix that by getting fclk rate in probe() and using a notifier in case rate
> changes.

Makes sense to me:

Reviewed-by: Tony Lindgren <tony@atomide.com>
[tip: timers/core] drivers/clocksource/timer-ti-dm: Don't call clk_get_rate() in stop function
Posted by tip-bot2 for Ivaylo Dimitrov 1 year, 1 month ago 
The following commit has been merged into the timers/core branch of tip:

Commit-ID:     12590d4d0e331d3cb9e6b3494515cd61c8a6624e
Gitweb:        https://git.kernel.org/tip/12590d4d0e331d3cb9e6b3494515cd61c8a6624e
Author:        Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
AuthorDate:    Tue, 03 Oct 2023 08:50:20 +03:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Wed, 11 Oct 2023 10:14:53 +02:00

drivers/clocksource/timer-ti-dm: Don't call clk_get_rate() in stop function

clk_get_rate() might sleep, and that prevents dm-timer based PWM from being
used from atomic context.

Fix that by getting fclk rate in probe() and using a notifier in case rate
changes.

Fixes: af04aa856e93 ("ARM: OMAP: Move dmtimer driver out of plat-omap to drivers under clocksource")
Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Reviewed-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Link: https://lore.kernel.org/r/1696312220-11550-1-git-send-email-ivo.g.dimitrov.75@gmail.com
---
 drivers/clocksource/timer-ti-dm.c | 36 +++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/clocksource/timer-ti-dm.c b/drivers/clocksource/timer-ti-dm.c
index 09ab29c..5f60f6b 100644
--- a/drivers/clocksource/timer-ti-dm.c
+++ b/drivers/clocksource/timer-ti-dm.c
@@ -140,6 +140,8 @@ struct dmtimer {
 	struct platform_device *pdev;
 	struct list_head node;
 	struct notifier_block nb;
+	struct notifier_block fclk_nb;
+	unsigned long fclk_rate;
 };
 
 static u32 omap_reserved_systimers;
@@ -253,8 +255,7 @@ static inline void __omap_dm_timer_enable_posted(struct dmtimer *timer)
 	timer->posted = OMAP_TIMER_POSTED;
 }
 
-static inline void __omap_dm_timer_stop(struct dmtimer *timer,
-					unsigned long rate)
+static inline void __omap_dm_timer_stop(struct dmtimer *timer)
 {
 	u32 l;
 
@@ -269,7 +270,7 @@ static inline void __omap_dm_timer_stop(struct dmtimer *timer,
 		 * Wait for functional clock period x 3.5 to make sure that
 		 * timer is stopped
 		 */
-		udelay(3500000 / rate + 1);
+		udelay(3500000 / timer->fclk_rate + 1);
 #endif
 	}
 
@@ -348,6 +349,21 @@ static int omap_timer_context_notifier(struct notifier_block *nb,
 	return NOTIFY_OK;
 }
 
+static int omap_timer_fclk_notifier(struct notifier_block *nb,
+				    unsigned long event, void *data)
+{
+	struct clk_notifier_data *clk_data = data;
+	struct dmtimer *timer = container_of(nb, struct dmtimer, fclk_nb);
+
+	switch (event) {
+	case POST_RATE_CHANGE:
+		timer->fclk_rate = clk_data->new_rate;
+		return NOTIFY_OK;
+	default:
+		return NOTIFY_DONE;
+	}
+}
+
 static int omap_dm_timer_reset(struct dmtimer *timer)
 {
 	u32 l, timeout = 100000;
@@ -754,7 +770,6 @@ static int omap_dm_timer_stop(struct omap_dm_timer *cookie)
 {
 	struct dmtimer *timer;
 	struct device *dev;
-	unsigned long rate = 0;
 
 	timer = to_dmtimer(cookie);
 	if (unlikely(!timer))
@@ -762,10 +777,7 @@ static int omap_dm_timer_stop(struct omap_dm_timer *cookie)
 
 	dev = &timer->pdev->dev;
 
-	if (!timer->omap1)
-		rate = clk_get_rate(timer->fclk);
-
-	__omap_dm_timer_stop(timer, rate);
+	__omap_dm_timer_stop(timer);
 
 	pm_runtime_put_sync(dev);
 
@@ -1124,6 +1136,14 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
 		timer->fclk = devm_clk_get(dev, "fck");
 		if (IS_ERR(timer->fclk))
 			return PTR_ERR(timer->fclk);
+
+		timer->fclk_nb.notifier_call = omap_timer_fclk_notifier;
+		ret = devm_clk_notifier_register(dev, timer->fclk,
+						 &timer->fclk_nb);
+		if (ret)
+			return ret;
+
+		timer->fclk_rate = clk_get_rate(timer->fclk);
 	} else {
 		timer->fclk = ERR_PTR(-ENODEV);
 	}

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.