diff mbox series

[v3,3/4] watchdog: mt7621-wdt: avoid static global declarations

Message ID 20230210173841.705783-4-sergio.paracuellos@gmail.com (mailing list archive)
State Superseded
Headers show
Series watchdog: mt7621-wdt: avoid globals and arch dependencies | expand

Commit Message

Sergio Paracuellos Feb. 10, 2023, 5:38 p.m. UTC 
Instead of using static global definitions in driver code, refactor code
introducing a new watchdog driver data structure and use it along the
code.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/watchdog/mt7621_wdt.c | 102 ++++++++++++++++++++++------------
 1 file changed, 65 insertions(+), 37 deletions(-)

Comments

Guenter Roeck Feb. 10, 2023, 6:46 p.m. UTC | #1 
On Fri, Feb 10, 2023 at 06:38:40PM +0100, Sergio Paracuellos wrote:
> Instead of using static global definitions in driver code, refactor code
> introducing a new watchdog driver data structure and use it along the
> code.
> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/mt7621_wdt.c | 102 ++++++++++++++++++++++------------
>  1 file changed, 65 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/watchdog/mt7621_wdt.c b/drivers/watchdog/mt7621_wdt.c
> index a8aa3522c..40fb2c9ba 100644
> --- a/drivers/watchdog/mt7621_wdt.c
> +++ b/drivers/watchdog/mt7621_wdt.c
> @@ -31,8 +31,11 @@
>  #define TMR1CTL_RESTART			BIT(9)
>  #define TMR1CTL_PRESCALE_SHIFT		16
>  
> -static void __iomem *mt7621_wdt_base;
> -static struct reset_control *mt7621_wdt_reset;
> +struct mt7621_wdt_data {
> +	void __iomem *base;
> +	struct reset_control *rst;
> +	struct watchdog_device wdt;
> +};
>  
>  static bool nowayout = WATCHDOG_NOWAYOUT;
>  module_param(nowayout, bool, 0);
> @@ -40,27 +43,31 @@ MODULE_PARM_DESC(nowayout,
>  		 "Watchdog cannot be stopped once started (default="
>  		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
>  
> -static inline void rt_wdt_w32(unsigned reg, u32 val)
> +static inline void rt_wdt_w32(void __iomem *base, unsigned reg, u32 val)
>  {
> -	iowrite32(val, mt7621_wdt_base + reg);
> +	iowrite32(val, base + reg);
>  }
>  
> -static inline u32 rt_wdt_r32(unsigned reg)
> +static inline u32 rt_wdt_r32(void __iomem *base, unsigned reg)
>  {
> -	return ioread32(mt7621_wdt_base + reg);
> +	return ioread32(base + reg);
>  }
>  
>  static int mt7621_wdt_ping(struct watchdog_device *w)
>  {
> -	rt_wdt_w32(TIMER_REG_TMRSTAT, TMR1CTL_RESTART);
> +	struct mt7621_wdt_data *drvdata = watchdog_get_drvdata(w);
> +
> +	rt_wdt_w32(drvdata->base, TIMER_REG_TMRSTAT, TMR1CTL_RESTART);
>  
>  	return 0;
>  }
>  
>  static int mt7621_wdt_set_timeout(struct watchdog_device *w, unsigned int t)
>  {
> +	struct mt7621_wdt_data *drvdata = watchdog_get_drvdata(w);
> +
>  	w->timeout = t;
> -	rt_wdt_w32(TIMER_REG_TMR1LOAD, t * 1000);
> +	rt_wdt_w32(drvdata->base, TIMER_REG_TMR1LOAD, t * 1000);
>  	mt7621_wdt_ping(w);
>  
>  	return 0;
> @@ -68,29 +75,31 @@ static int mt7621_wdt_set_timeout(struct watchdog_device *w, unsigned int t)
>  
>  static int mt7621_wdt_start(struct watchdog_device *w)
>  {
> +	struct mt7621_wdt_data *drvdata = watchdog_get_drvdata(w);
>  	u32 t;
>  
>  	/* set the prescaler to 1ms == 1000us */
> -	rt_wdt_w32(TIMER_REG_TMR1CTL, 1000 << TMR1CTL_PRESCALE_SHIFT);
> +	rt_wdt_w32(drvdata->base, TIMER_REG_TMR1CTL, 1000 << TMR1CTL_PRESCALE_SHIFT);
>  
>  	mt7621_wdt_set_timeout(w, w->timeout);
>  
> -	t = rt_wdt_r32(TIMER_REG_TMR1CTL);
> +	t = rt_wdt_r32(drvdata->base, TIMER_REG_TMR1CTL);
>  	t |= TMR1CTL_ENABLE;
> -	rt_wdt_w32(TIMER_REG_TMR1CTL, t);
> +	rt_wdt_w32(drvdata->base, TIMER_REG_TMR1CTL, t);
>  
>  	return 0;
>  }
>  
>  static int mt7621_wdt_stop(struct watchdog_device *w)
>  {
> +	struct mt7621_wdt_data *drvdata = watchdog_get_drvdata(w);
>  	u32 t;
>  
>  	mt7621_wdt_ping(w);
>  
> -	t = rt_wdt_r32(TIMER_REG_TMR1CTL);
> +	t = rt_wdt_r32(drvdata->base, TIMER_REG_TMR1CTL);
>  	t &= ~TMR1CTL_ENABLE;
> -	rt_wdt_w32(TIMER_REG_TMR1CTL, t);
> +	rt_wdt_w32(drvdata->base, TIMER_REG_TMR1CTL, t);
>  
>  	return 0;
>  }
> @@ -105,7 +114,9 @@ static int mt7621_wdt_bootcause(void)
>  
>  static int mt7621_wdt_is_running(struct watchdog_device *w)
>  {
> -	return !!(rt_wdt_r32(TIMER_REG_TMR1CTL) & TMR1CTL_ENABLE);
> +	struct mt7621_wdt_data *drvdata = watchdog_get_drvdata(w);
> +
> +	return !!(rt_wdt_r32(drvdata->base, TIMER_REG_TMR1CTL) & TMR1CTL_ENABLE);
>  }
>  
>  static const struct watchdog_info mt7621_wdt_info = {
> @@ -121,30 +132,39 @@ static const struct watchdog_ops mt7621_wdt_ops = {
>  	.set_timeout = mt7621_wdt_set_timeout,
>  };
>  
> -static struct watchdog_device mt7621_wdt_dev = {
> -	.info = &mt7621_wdt_info,
> -	.ops = &mt7621_wdt_ops,
> -	.min_timeout = 1,
> -	.max_timeout = 0xfffful / 1000,
> -};
> -
>  static int mt7621_wdt_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> -	mt7621_wdt_base = devm_platform_ioremap_resource(pdev, 0);
> -	if (IS_ERR(mt7621_wdt_base))
> -		return PTR_ERR(mt7621_wdt_base);
> +	struct watchdog_device *mt7621_wdt;
> +	struct mt7621_wdt_data *drvdata;
> +	int err;
> +
> +	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM;
>  
> -	mt7621_wdt_reset = devm_reset_control_get_exclusive(dev, NULL);
> -	if (!IS_ERR(mt7621_wdt_reset))
> -		reset_control_deassert(mt7621_wdt_reset);
> +	drvdata->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(drvdata->base))
> +		return PTR_ERR(drvdata->base);
>  
> -	mt7621_wdt_dev.bootstatus = mt7621_wdt_bootcause();
> +	drvdata->rst = devm_reset_control_get_exclusive(dev, NULL);
> +	if (!IS_ERR(drvdata->rst))
> +		reset_control_deassert(drvdata->rst);
>  
> -	watchdog_init_timeout(&mt7621_wdt_dev, mt7621_wdt_dev.max_timeout,
> -			      dev);
> -	watchdog_set_nowayout(&mt7621_wdt_dev, nowayout);
> -	if (mt7621_wdt_is_running(&mt7621_wdt_dev)) {
> +	mt7621_wdt = &drvdata->wdt;
> +	mt7621_wdt->info = &mt7621_wdt_info;
> +	mt7621_wdt->ops = &mt7621_wdt_ops;
> +	mt7621_wdt->min_timeout = 1;
> +	mt7621_wdt->max_timeout = 0xfffful / 1000;
> +	mt7621_wdt->parent = dev;
> +
> +	mt7621_wdt->bootstatus = mt7621_wdt_bootcause();
> +
> +	watchdog_init_timeout(mt7621_wdt, mt7621_wdt->max_timeout, dev);
> +	watchdog_set_nowayout(mt7621_wdt, nowayout);
> +	watchdog_set_drvdata(mt7621_wdt, drvdata);
> +
> +	if (mt7621_wdt_is_running(mt7621_wdt)) {
>  		/*
>  		 * Make sure to apply timeout from watchdog core, taking
>  		 * the prescaler of this driver here into account (the
> @@ -154,17 +174,25 @@ static int mt7621_wdt_probe(struct platform_device *pdev)
>  		 * we first disable the watchdog, set the new prescaler
>  		 * and timeout, and then re-enable the watchdog.
>  		 */
> -		mt7621_wdt_stop(&mt7621_wdt_dev);
> -		mt7621_wdt_start(&mt7621_wdt_dev);
> -		set_bit(WDOG_HW_RUNNING, &mt7621_wdt_dev.status);
> +		mt7621_wdt_stop(mt7621_wdt);
> +		mt7621_wdt_start(mt7621_wdt);
> +		set_bit(WDOG_HW_RUNNING, &mt7621_wdt->status);
>  	}
>  
> -	return devm_watchdog_register_device(dev, &mt7621_wdt_dev);
> +	err = devm_watchdog_register_device(dev, &drvdata->wdt);
> +	if (err)
> +		return err;
> +
> +	platform_set_drvdata(pdev, drvdata);
> +
> +	return 0;
>  }
>  
>  static void mt7621_wdt_shutdown(struct platform_device *pdev)
>  {
> -	mt7621_wdt_stop(&mt7621_wdt_dev);
> +	struct mt7621_wdt_data *drvdata = platform_get_drvdata(pdev);
> +
> +	mt7621_wdt_stop(&drvdata->wdt);
>  }
>  
>  static const struct of_device_id mt7621_wdt_match[] = {
> -- 
> 2.25.1
>
diff mbox series

Patch

diff --git a/drivers/watchdog/mt7621_wdt.c b/drivers/watchdog/mt7621_wdt.c
index a8aa3522c..40fb2c9ba 100644
--- a/drivers/watchdog/mt7621_wdt.c
+++ b/drivers/watchdog/mt7621_wdt.c
@@ -31,8 +31,11 @@ 
 #define TMR1CTL_RESTART			BIT(9)
 #define TMR1CTL_PRESCALE_SHIFT		16
 
-static void __iomem *mt7621_wdt_base;
-static struct reset_control *mt7621_wdt_reset;
+struct mt7621_wdt_data {
+	void __iomem *base;
+	struct reset_control *rst;
+	struct watchdog_device wdt;
+};
 
 static bool nowayout = WATCHDOG_NOWAYOUT;
 module_param(nowayout, bool, 0);
@@ -40,27 +43,31 @@  MODULE_PARM_DESC(nowayout,
 		 "Watchdog cannot be stopped once started (default="
 		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 
-static inline void rt_wdt_w32(unsigned reg, u32 val)
+static inline void rt_wdt_w32(void __iomem *base, unsigned reg, u32 val)
 {
-	iowrite32(val, mt7621_wdt_base + reg);
+	iowrite32(val, base + reg);
 }
 
-static inline u32 rt_wdt_r32(unsigned reg)
+static inline u32 rt_wdt_r32(void __iomem *base, unsigned reg)
 {
-	return ioread32(mt7621_wdt_base + reg);
+	return ioread32(base + reg);
 }
 
 static int mt7621_wdt_ping(struct watchdog_device *w)
 {
-	rt_wdt_w32(TIMER_REG_TMRSTAT, TMR1CTL_RESTART);
+	struct mt7621_wdt_data *drvdata = watchdog_get_drvdata(w);
+
+	rt_wdt_w32(drvdata->base, TIMER_REG_TMRSTAT, TMR1CTL_RESTART);
 
 	return 0;
 }
 
 static int mt7621_wdt_set_timeout(struct watchdog_device *w, unsigned int t)
 {
+	struct mt7621_wdt_data *drvdata = watchdog_get_drvdata(w);
+
 	w->timeout = t;
-	rt_wdt_w32(TIMER_REG_TMR1LOAD, t * 1000);
+	rt_wdt_w32(drvdata->base, TIMER_REG_TMR1LOAD, t * 1000);
 	mt7621_wdt_ping(w);
 
 	return 0;
@@ -68,29 +75,31 @@  static int mt7621_wdt_set_timeout(struct watchdog_device *w, unsigned int t)
 
 static int mt7621_wdt_start(struct watchdog_device *w)
 {
+	struct mt7621_wdt_data *drvdata = watchdog_get_drvdata(w);
 	u32 t;
 
 	/* set the prescaler to 1ms == 1000us */
-	rt_wdt_w32(TIMER_REG_TMR1CTL, 1000 << TMR1CTL_PRESCALE_SHIFT);
+	rt_wdt_w32(drvdata->base, TIMER_REG_TMR1CTL, 1000 << TMR1CTL_PRESCALE_SHIFT);
 
 	mt7621_wdt_set_timeout(w, w->timeout);
 
-	t = rt_wdt_r32(TIMER_REG_TMR1CTL);
+	t = rt_wdt_r32(drvdata->base, TIMER_REG_TMR1CTL);
 	t |= TMR1CTL_ENABLE;
-	rt_wdt_w32(TIMER_REG_TMR1CTL, t);
+	rt_wdt_w32(drvdata->base, TIMER_REG_TMR1CTL, t);
 
 	return 0;
 }
 
 static int mt7621_wdt_stop(struct watchdog_device *w)
 {
+	struct mt7621_wdt_data *drvdata = watchdog_get_drvdata(w);
 	u32 t;
 
 	mt7621_wdt_ping(w);
 
-	t = rt_wdt_r32(TIMER_REG_TMR1CTL);
+	t = rt_wdt_r32(drvdata->base, TIMER_REG_TMR1CTL);
 	t &= ~TMR1CTL_ENABLE;
-	rt_wdt_w32(TIMER_REG_TMR1CTL, t);
+	rt_wdt_w32(drvdata->base, TIMER_REG_TMR1CTL, t);
 
 	return 0;
 }
@@ -105,7 +114,9 @@  static int mt7621_wdt_bootcause(void)
 
 static int mt7621_wdt_is_running(struct watchdog_device *w)
 {
-	return !!(rt_wdt_r32(TIMER_REG_TMR1CTL) & TMR1CTL_ENABLE);
+	struct mt7621_wdt_data *drvdata = watchdog_get_drvdata(w);
+
+	return !!(rt_wdt_r32(drvdata->base, TIMER_REG_TMR1CTL) & TMR1CTL_ENABLE);
 }
 
 static const struct watchdog_info mt7621_wdt_info = {
@@ -121,30 +132,39 @@  static const struct watchdog_ops mt7621_wdt_ops = {
 	.set_timeout = mt7621_wdt_set_timeout,
 };
 
-static struct watchdog_device mt7621_wdt_dev = {
-	.info = &mt7621_wdt_info,
-	.ops = &mt7621_wdt_ops,
-	.min_timeout = 1,
-	.max_timeout = 0xfffful / 1000,
-};
-
 static int mt7621_wdt_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	mt7621_wdt_base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(mt7621_wdt_base))
-		return PTR_ERR(mt7621_wdt_base);
+	struct watchdog_device *mt7621_wdt;
+	struct mt7621_wdt_data *drvdata;
+	int err;
+
+	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
 
-	mt7621_wdt_reset = devm_reset_control_get_exclusive(dev, NULL);
-	if (!IS_ERR(mt7621_wdt_reset))
-		reset_control_deassert(mt7621_wdt_reset);
+	drvdata->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(drvdata->base))
+		return PTR_ERR(drvdata->base);
 
-	mt7621_wdt_dev.bootstatus = mt7621_wdt_bootcause();
+	drvdata->rst = devm_reset_control_get_exclusive(dev, NULL);
+	if (!IS_ERR(drvdata->rst))
+		reset_control_deassert(drvdata->rst);
 
-	watchdog_init_timeout(&mt7621_wdt_dev, mt7621_wdt_dev.max_timeout,
-			      dev);
-	watchdog_set_nowayout(&mt7621_wdt_dev, nowayout);
-	if (mt7621_wdt_is_running(&mt7621_wdt_dev)) {
+	mt7621_wdt = &drvdata->wdt;
+	mt7621_wdt->info = &mt7621_wdt_info;
+	mt7621_wdt->ops = &mt7621_wdt_ops;
+	mt7621_wdt->min_timeout = 1;
+	mt7621_wdt->max_timeout = 0xfffful / 1000;
+	mt7621_wdt->parent = dev;
+
+	mt7621_wdt->bootstatus = mt7621_wdt_bootcause();
+
+	watchdog_init_timeout(mt7621_wdt, mt7621_wdt->max_timeout, dev);
+	watchdog_set_nowayout(mt7621_wdt, nowayout);
+	watchdog_set_drvdata(mt7621_wdt, drvdata);
+
+	if (mt7621_wdt_is_running(mt7621_wdt)) {
 		/*
 		 * Make sure to apply timeout from watchdog core, taking
 		 * the prescaler of this driver here into account (the
@@ -154,17 +174,25 @@  static int mt7621_wdt_probe(struct platform_device *pdev)
 		 * we first disable the watchdog, set the new prescaler
 		 * and timeout, and then re-enable the watchdog.
 		 */
-		mt7621_wdt_stop(&mt7621_wdt_dev);
-		mt7621_wdt_start(&mt7621_wdt_dev);
-		set_bit(WDOG_HW_RUNNING, &mt7621_wdt_dev.status);
+		mt7621_wdt_stop(mt7621_wdt);
+		mt7621_wdt_start(mt7621_wdt);
+		set_bit(WDOG_HW_RUNNING, &mt7621_wdt->status);
 	}
 
-	return devm_watchdog_register_device(dev, &mt7621_wdt_dev);
+	err = devm_watchdog_register_device(dev, &drvdata->wdt);
+	if (err)
+		return err;
+
+	platform_set_drvdata(pdev, drvdata);
+
+	return 0;
 }
 
 static void mt7621_wdt_shutdown(struct platform_device *pdev)
 {
-	mt7621_wdt_stop(&mt7621_wdt_dev);
+	struct mt7621_wdt_data *drvdata = platform_get_drvdata(pdev);
+
+	mt7621_wdt_stop(&drvdata->wdt);
 }
 
 static const struct of_device_id mt7621_wdt_match[] = {