| Message ID | 1556905388-19249-2-git-send-email-philippe.reynes@softathome.com |
|---|---|
| State | Accepted |
| Commit | c411adbdd0f2ff0e1dd35342c843edd3afa7c15b |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | fix bcm6345 watchdog on broadcom board | expand |
On 03.05.19 19:43, Philippe Reynes wrote: > The function bcm6345_wdt_start use the argument timeout > as tick but it should be used as milliseconds. > > A clock is added as requirement for this driver. > The frequency of the clock is then used to convert the > millisecond to ticks in the function bcm6345_wdt_start. > > Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com> You might want to emphasize, that this is a pretty severe fix to this driver, as without this patch, boards using this driver will end in a endless reboot loop. So it should be applied to this release. Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan > --- > drivers/watchdog/bcm6345_wdt.c | 21 ++++++++++++++++----- > 1 file changed, 16 insertions(+), 5 deletions(-) > > diff --git a/drivers/watchdog/bcm6345_wdt.c b/drivers/watchdog/bcm6345_wdt.c > index 44f5662..9f14e7d 100644 > --- a/drivers/watchdog/bcm6345_wdt.c > +++ b/drivers/watchdog/bcm6345_wdt.c > @@ -10,6 +10,7 @@ > #include <common.h> > #include <dm.h> > #include <wdt.h> > +#include <clk.h> > #include <asm/io.h> > > /* WDT Value register */ > @@ -26,6 +27,7 @@ > > struct bcm6345_wdt_priv { > void __iomem *regs; > + unsigned long clk_rate; > }; > > static int bcm6345_wdt_reset(struct udevice *dev) > @@ -41,16 +43,17 @@ static int bcm6345_wdt_reset(struct udevice *dev) > static int bcm6345_wdt_start(struct udevice *dev, u64 timeout, ulong flags) > { > struct bcm6345_wdt_priv *priv = dev_get_priv(dev); > + u32 val = priv->clk_rate / 1000 * timeout; > > - if (timeout < WDT_VAL_MIN) { > + if (val < WDT_VAL_MIN) { > debug("watchdog won't fire with less than 2 ticks\n"); > - timeout = WDT_VAL_MIN; > - } else if (timeout > WDT_VAL_MAX) { > + val = WDT_VAL_MIN; > + } else if (val > WDT_VAL_MAX) { > debug("maximum watchdog timeout exceeded\n"); > - timeout = WDT_VAL_MAX; > + val = WDT_VAL_MAX; > } > > - writel(timeout, priv->regs + WDT_VAL_REG); > + writel(val, priv->regs + WDT_VAL_REG); > > return bcm6345_wdt_reset(dev); > } > @@ -85,11 +88,19 @@ static const struct udevice_id bcm6345_wdt_ids[] = { > static int bcm6345_wdt_probe(struct udevice *dev) > { > struct bcm6345_wdt_priv *priv = dev_get_priv(dev); > + struct clk clk; > + int ret; > > priv->regs = dev_remap_addr(dev); > if (!priv->regs) > return -EINVAL; > > + ret = clk_get_by_index(dev, 0, &clk); > + if (!ret) > + priv->clk_rate = clk_get_rate(&clk); > + else > + return -EINVAL; > + > bcm6345_wdt_stop(dev); > > return 0; > Viele Grüße, Stefan
On Fri, May 03, 2019 at 07:43:06PM +0200, Philippe Reynes wrote: > The function bcm6345_wdt_start use the argument timeout > as tick but it should be used as milliseconds. > > A clock is added as requirement for this driver. > The frequency of the clock is then used to convert the > millisecond to ticks in the function bcm6345_wdt_start. > > Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com> > Reviewed-by: Stefan Roese <sr@denx.de> Applied to u-boot/master, thanks!
diff --git a/drivers/watchdog/bcm6345_wdt.c b/drivers/watchdog/bcm6345_wdt.c index 44f5662..9f14e7d 100644 --- a/drivers/watchdog/bcm6345_wdt.c +++ b/drivers/watchdog/bcm6345_wdt.c @@ -10,6 +10,7 @@ #include <common.h> #include <dm.h> #include <wdt.h> +#include <clk.h> #include <asm/io.h> /* WDT Value register */ @@ -26,6 +27,7 @@ struct bcm6345_wdt_priv { void __iomem *regs; + unsigned long clk_rate; }; static int bcm6345_wdt_reset(struct udevice *dev) @@ -41,16 +43,17 @@ static int bcm6345_wdt_reset(struct udevice *dev) static int bcm6345_wdt_start(struct udevice *dev, u64 timeout, ulong flags) { struct bcm6345_wdt_priv *priv = dev_get_priv(dev); + u32 val = priv->clk_rate / 1000 * timeout; - if (timeout < WDT_VAL_MIN) { + if (val < WDT_VAL_MIN) { debug("watchdog won't fire with less than 2 ticks\n"); - timeout = WDT_VAL_MIN; - } else if (timeout > WDT_VAL_MAX) { + val = WDT_VAL_MIN; + } else if (val > WDT_VAL_MAX) { debug("maximum watchdog timeout exceeded\n"); - timeout = WDT_VAL_MAX; + val = WDT_VAL_MAX; } - writel(timeout, priv->regs + WDT_VAL_REG); + writel(val, priv->regs + WDT_VAL_REG); return bcm6345_wdt_reset(dev); } @@ -85,11 +88,19 @@ static const struct udevice_id bcm6345_wdt_ids[] = { static int bcm6345_wdt_probe(struct udevice *dev) { struct bcm6345_wdt_priv *priv = dev_get_priv(dev); + struct clk clk; + int ret; priv->regs = dev_remap_addr(dev); if (!priv->regs) return -EINVAL; + ret = clk_get_by_index(dev, 0, &clk); + if (!ret) + priv->clk_rate = clk_get_rate(&clk); + else + return -EINVAL; + bcm6345_wdt_stop(dev); return 0;
The function bcm6345_wdt_start use the argument timeout as tick but it should be used as milliseconds. A clock is added as requirement for this driver. The frequency of the clock is then used to convert the millisecond to ticks in the function bcm6345_wdt_start. Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com> --- drivers/watchdog/bcm6345_wdt.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)