Re: [U-Boot] [PATCH 1/3] watchdog: bcm6345: callback start use tick instead of ms

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