| Message ID | 20190521155313.19326-2-paul@crapouillou.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/4] watchdog: jz4740: Use register names from <linux/mfd/ingenic-tcu.h> | expand |
On Tue, May 21, 2019 at 05:53:11PM +0200, Paul Cercueil wrote: > Previously the jz4740_wdt_set_timeout() function was starting the timer > unconditionally, even if it was stopped when that function was entered. > > Now, the timer will be restarted only if it was already running before > this function is called. > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> Comment inline, but not worth a respin (and I don't have a good idea how to improve it). Reviewed-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/watchdog/jz4740_wdt.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/watchdog/jz4740_wdt.c b/drivers/watchdog/jz4740_wdt.c > index 51be321c775a..f970a7a53084 100644 > --- a/drivers/watchdog/jz4740_wdt.c > +++ b/drivers/watchdog/jz4740_wdt.c > @@ -77,6 +77,7 @@ static int jz4740_wdt_set_timeout(struct watchdog_device *wdt_dev, > unsigned int rtc_clk_rate; > unsigned int timeout_value; > unsigned short clock_div = JZ_WDT_CLOCK_DIV_1; > + u8 tcer; > > rtc_clk_rate = clk_get_rate(drvdata->rtc_clk); > > @@ -92,6 +93,7 @@ static int jz4740_wdt_set_timeout(struct watchdog_device *wdt_dev, > clock_div += (1 << TCU_TCSR_PRESCALE_LSB); > } > > + tcer = readb(drvdata->base + TCU_REG_WDT_TCER); > writeb(0x0, drvdata->base + TCU_REG_WDT_TCER); > writew(clock_div, drvdata->base + TCU_REG_WDT_TCSR); > > @@ -99,7 +101,7 @@ static int jz4740_wdt_set_timeout(struct watchdog_device *wdt_dev, > writew(0x0, drvdata->base + TCU_REG_WDT_TCNT); > writew(clock_div | JZ_WDT_CLOCK_RTC, drvdata->base + TCU_REG_WDT_TCSR); > > - writeb(0x1, drvdata->base + TCU_REG_WDT_TCER); > + writeb(tcer & TCU_WDT_TCER_TCEN, drvdata->base + TCU_REG_WDT_TCER); This unnecessarily writes 0 if the timer is not running. > > wdt_dev->timeout = new_timeout; > return 0; > @@ -107,8 +109,11 @@ static int jz4740_wdt_set_timeout(struct watchdog_device *wdt_dev, > > static int jz4740_wdt_start(struct watchdog_device *wdt_dev) > { > + struct jz4740_wdt_drvdata *drvdata = watchdog_get_drvdata(wdt_dev); > + > jz4740_timer_enable_watchdog(); > jz4740_wdt_set_timeout(wdt_dev, wdt_dev->timeout); > + writeb(TCU_WDT_TCER_TCEN, drvdata->base + TCU_REG_WDT_TCER); This unnecessarily enables the timer even if it is already enabled (that should not happen, but does if the watchdog is already running at boot). > > return 0; > } > -- > 2.21.0.593.g511ec345e18 >
diff --git a/drivers/watchdog/jz4740_wdt.c b/drivers/watchdog/jz4740_wdt.c index 51be321c775a..f970a7a53084 100644 --- a/drivers/watchdog/jz4740_wdt.c +++ b/drivers/watchdog/jz4740_wdt.c @@ -77,6 +77,7 @@ static int jz4740_wdt_set_timeout(struct watchdog_device *wdt_dev, unsigned int rtc_clk_rate; unsigned int timeout_value; unsigned short clock_div = JZ_WDT_CLOCK_DIV_1; + u8 tcer; rtc_clk_rate = clk_get_rate(drvdata->rtc_clk); @@ -92,6 +93,7 @@ static int jz4740_wdt_set_timeout(struct watchdog_device *wdt_dev, clock_div += (1 << TCU_TCSR_PRESCALE_LSB); } + tcer = readb(drvdata->base + TCU_REG_WDT_TCER); writeb(0x0, drvdata->base + TCU_REG_WDT_TCER); writew(clock_div, drvdata->base + TCU_REG_WDT_TCSR); @@ -99,7 +101,7 @@ static int jz4740_wdt_set_timeout(struct watchdog_device *wdt_dev, writew(0x0, drvdata->base + TCU_REG_WDT_TCNT); writew(clock_div | JZ_WDT_CLOCK_RTC, drvdata->base + TCU_REG_WDT_TCSR); - writeb(0x1, drvdata->base + TCU_REG_WDT_TCER); + writeb(tcer & TCU_WDT_TCER_TCEN, drvdata->base + TCU_REG_WDT_TCER); wdt_dev->timeout = new_timeout; return 0; @@ -107,8 +109,11 @@ static int jz4740_wdt_set_timeout(struct watchdog_device *wdt_dev, static int jz4740_wdt_start(struct watchdog_device *wdt_dev) { + struct jz4740_wdt_drvdata *drvdata = watchdog_get_drvdata(wdt_dev); + jz4740_timer_enable_watchdog(); jz4740_wdt_set_timeout(wdt_dev, wdt_dev->timeout); + writeb(TCU_WDT_TCER_TCEN, drvdata->base + TCU_REG_WDT_TCER); return 0; }
Previously the jz4740_wdt_set_timeout() function was starting the timer unconditionally, even if it was stopped when that function was entered. Now, the timer will be restarted only if it was already running before this function is called. Signed-off-by: Paul Cercueil <paul@crapouillou.net> --- drivers/watchdog/jz4740_wdt.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)