diff mbox series

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

Message ID 20230210121735.639089-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, 12:17 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 | 104 ++++++++++++++++++++++------------
 1 file changed, 67 insertions(+), 37 deletions(-)

Comments

Guenter Roeck Feb. 10, 2023, 3 p.m. UTC | #1 
On 2/10/23 04:17, 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>
> ---
>   drivers/watchdog/mt7621_wdt.c | 104 ++++++++++++++++++++++------------
>   1 file changed, 67 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/watchdog/mt7621_wdt.c b/drivers/watchdog/mt7621_wdt.c
> index a8aa3522c..f1c702971 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;
> +
> +	drvdata->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(drvdata->base))
> +		return PTR_ERR(drvdata->base);
>   
> -	mt7621_wdt_reset = devm_reset_control_get_exclusive(dev, NULL);
> -	if (!IS_ERR(mt7621_wdt_reset))
> -		reset_control_deassert(mt7621_wdt_reset);
> +	drvdata->rst = devm_reset_control_get_exclusive(dev, NULL);
> +	if (!IS_ERR(drvdata->rst))
> +		reset_control_deassert(drvdata->rst);
>   
> -	mt7621_wdt_dev.bootstatus = mt7621_wdt_bootcause();
> +	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;
>   
> -	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->bootstatus = mt7621_wdt_bootcause();
> +
> +	watchdog_init_timeout(mt7621_wdt, mt7621_wdt->max_timeout, dev);

FWIW, setting ->timeout would have been sufficient. watchdog_init_timeout()
is only really valuable if there is a module parameter, or if the timeout
is set from devicetree. That won't work here, though, because the passed
value takes precedence. Changing that would change functionality and thus would
have to be done in a separate patch, I just wanted to mention it.

> +	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,27 @@ 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);
> +	}
> +
> +	err = devm_watchdog_register_device(dev, &drvdata->wdt);
> +	if (err) {
> +		dev_err(dev, "Error registering watchdog device\n");
> +		return err;
>   	}

This is a functional change. If you want to add an error message,
do it in a separate patch and provide a rationale for it.

>   
> -	return devm_watchdog_register_device(dev, &mt7621_wdt_dev);
> +	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);
>   }

Also FWIW, we have watchdog_stop_on_reboot() for that purpose.
Changing that would be a separate patch, though.

>   static const struct of_device_id mt7621_wdt_match[] = {
Sergio Paracuellos Feb. 10, 2023, 3:43 p.m. UTC | #2 
Hi Guenter,

Thanks for reviewing this.

On Fri, Feb 10, 2023 at 4:00 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 2/10/23 04:17, 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>
> > ---
> >   drivers/watchdog/mt7621_wdt.c | 104 ++++++++++++++++++++++------------
> >   1 file changed, 67 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/watchdog/mt7621_wdt.c b/drivers/watchdog/mt7621_wdt.c
> > index a8aa3522c..f1c702971 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;
> > +
> > +     drvdata->base = devm_platform_ioremap_resource(pdev, 0);
> > +     if (IS_ERR(drvdata->base))
> > +             return PTR_ERR(drvdata->base);
> >
> > -     mt7621_wdt_reset = devm_reset_control_get_exclusive(dev, NULL);
> > -     if (!IS_ERR(mt7621_wdt_reset))
> > -             reset_control_deassert(mt7621_wdt_reset);
> > +     drvdata->rst = devm_reset_control_get_exclusive(dev, NULL);
> > +     if (!IS_ERR(drvdata->rst))
> > +             reset_control_deassert(drvdata->rst);
> >
> > -     mt7621_wdt_dev.bootstatus = mt7621_wdt_bootcause();
> > +     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;
> >
> > -     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->bootstatus = mt7621_wdt_bootcause();
> > +
> > +     watchdog_init_timeout(mt7621_wdt, mt7621_wdt->max_timeout, dev);
>
> FWIW, setting ->timeout would have been sufficient. watchdog_init_timeout()
> is only really valuable if there is a module parameter, or if the timeout
> is set from devicetree. That won't work here, though, because the passed
> value takes precedence. Changing that would change functionality and thus would
> have to be done in a separate patch, I just wanted to mention it.

Thanks for letting me know. By now I only want to maintain the current
functionality and if it really makes sense I will send functionality
changes in new series after this one is applied.

>
> > +     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,27 @@ 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);
> > +     }
> > +
> > +     err = devm_watchdog_register_device(dev, &drvdata->wdt);
> > +     if (err) {
> > +             dev_err(dev, "Error registering watchdog device\n");
> > +             return err;
> >       }
>
> This is a functional change. If you want to add an error message,
> do it in a separate patch and provide a rationale for it.

True, will drop this error message in v3 to maintain current
functionality without any extra changes.

>
> >
> > -     return devm_watchdog_register_device(dev, &mt7621_wdt_dev);
> > +     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);
> >   }
>
> Also FWIW, we have watchdog_stop_on_reboot() for that purpose.
> Changing that would be a separate patch, though.

Thanks. As I have just said above, let's focus only on maintaining
current functionality in this series.

>
> >   static const struct of_device_id mt7621_wdt_match[] = {
>

Thanks,
    Sergio Paracuellos
diff mbox series

Patch

diff --git a/drivers/watchdog/mt7621_wdt.c b/drivers/watchdog/mt7621_wdt.c
index a8aa3522c..f1c702971 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;
+
+	drvdata->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(drvdata->base))
+		return PTR_ERR(drvdata->base);
 
-	mt7621_wdt_reset = devm_reset_control_get_exclusive(dev, NULL);
-	if (!IS_ERR(mt7621_wdt_reset))
-		reset_control_deassert(mt7621_wdt_reset);
+	drvdata->rst = devm_reset_control_get_exclusive(dev, NULL);
+	if (!IS_ERR(drvdata->rst))
+		reset_control_deassert(drvdata->rst);
 
-	mt7621_wdt_dev.bootstatus = mt7621_wdt_bootcause();
+	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;
 
-	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->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,27 @@  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);
+	}
+
+	err = devm_watchdog_register_device(dev, &drvdata->wdt);
+	if (err) {
+		dev_err(dev, "Error registering watchdog device\n");
+		return err;
 	}
 
-	return devm_watchdog_register_device(dev, &mt7621_wdt_dev);
+	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[] = {