| Message ID | 20190217213054.8223-3-judge.packham@gmail.com |
|---|---|
| State | Accepted |
| Commit | 8e427ba35170a6b5966c28e59192d0561f989f65 |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | x530: Enable watchdog | expand |
On Mon, Feb 18, 2019 at 10:31 AM Chris Packham <judge.packham@gmail.com> wrote: > > The generic wdt_start API expects to be called with the timeout in > milliseconds. Update the orion_wdt driver to accept a timeout in > milliseconds and use the clock rate specified in the dts to convert the > timeout to an appropriate value for the timer reload register. > > Signed-off-by: Chris Packham <judge.packham@gmail.com> > --- > It turned out to be easy enough to make orion_wdt conform to the > expected wdt APIs. It looks like the turris_mox already assumed a > timeout in ms, so only the turris_omnia needed updating. > > Changes in v4: > - use DIV_ROUND_UP to avoid setting the timeout to 0. > > Changes in v3: > - new > > Changes in v2: None > > board/CZ.NIC/turris_omnia/turris_omnia.c | 2 +- > drivers/watchdog/Kconfig | 1 + > drivers/watchdog/orion_wdt.c | 23 +++++++++++++++++++---- > 3 files changed, 21 insertions(+), 5 deletions(-) > > diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c > index 1f7650cb3610..5e0b686c597d 100644 > --- a/board/CZ.NIC/turris_omnia/turris_omnia.c > +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c > @@ -379,7 +379,7 @@ int board_init(void) > puts("Cannot find Armada 385 watchdog!\n"); > } else { > puts("Enabling Armada 385 watchdog.\n"); > - wdt_start(watchdog_dev, (u32) 25000000 * 120, 0); > + wdt_start(watchdog_dev, 120000, 0); > } > # endif > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index 115fc4551ffd..7f7bc4bbc494 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -97,6 +97,7 @@ config WDT_BCM6345 > config WDT_ORION > bool "Orion watchdog timer support" > depends on WDT > + select CLK Actually I just realised I can't do this yet. The armada-38x lacks a clock driver. So while the fixed-rate clock for the watchdog works fine. Enabling CONFIG_CLK causes the uart to not work because it can't look up it's clock. This is not initially noticeable because the SPL sets up the hardware but if you try to change the baudrate things get upset. I'm running out of u-boot hacking time to attempt to port the armada-38x clock code from Linux. For now I'll send a quick v5 that removes this select. > help > Select this to enable Orion watchdog timer, which can be found on some > Marvell Armada chips. > diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c > index c1add3e7c121..885821d562ea 100644 > --- a/drivers/watchdog/orion_wdt.c > +++ b/drivers/watchdog/orion_wdt.c > @@ -14,7 +14,9 @@ > > #include <common.h> > #include <dm.h> > +#include <clk.h> > #include <wdt.h> > +#include <linux/kernel.h> > #include <asm/io.h> > #include <asm/arch/cpu.h> > #include <asm/arch/soc.h> > @@ -27,6 +29,8 @@ struct orion_wdt_priv { > void __iomem *rstout; > void __iomem *rstout_mask; > u32 timeout; > + unsigned long clk_rate; > + struct clk clk; > }; > > #define RSTOUT_ENABLE_BIT BIT(8) > @@ -44,17 +48,18 @@ static int orion_wdt_reset(struct udevice *dev) > struct orion_wdt_priv *priv = dev_get_priv(dev); > > /* Reload watchdog duration */ > - writel(priv->timeout, priv->reg + priv->wdt_counter_offset); > + writel(priv->clk_rate * priv->timeout, > + priv->reg + priv->wdt_counter_offset); > > return 0; > } > > -static int orion_wdt_start(struct udevice *dev, u64 timeout, ulong flags) > +static int orion_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags) > { > struct orion_wdt_priv *priv = dev_get_priv(dev); > u32 reg; > > - priv->timeout = (u32) timeout; > + priv->timeout = DIV_ROUND_UP(timeout_ms, 1000); > > /* Enable the fixed watchdog clock input */ > reg = readl(priv->reg + TIMER_CTRL); > @@ -62,7 +67,8 @@ static int orion_wdt_start(struct udevice *dev, u64 timeout, ulong flags) > writel(reg, priv->reg + TIMER_CTRL); > > /* Set watchdog duration */ > - writel(priv->timeout, priv->reg + priv->wdt_counter_offset); > + writel(priv->clk_rate * priv->timeout, > + priv->reg + priv->wdt_counter_offset); > > /* Clear the watchdog expiration bit */ > reg = readl(priv->reg + TIMER_A370_STATUS); > @@ -147,9 +153,18 @@ err: > > static int orion_wdt_probe(struct udevice *dev) > { > + struct orion_wdt_priv *priv = dev_get_priv(dev); > + int ret; > + > debug("%s: Probing wdt%u\n", __func__, dev->seq); > orion_wdt_stop(dev); > > + ret = clk_get_by_name(dev, "fixed", &priv->clk); > + if (!ret) > + priv->clk_rate = clk_get_rate(&priv->clk); > + else > + priv->clk_rate = 25000000; > + > return 0; > } > > -- > 2.20.1 >
On Mon, Feb 18, 2019 at 10:41 AM Chris Packham <judge.packham@gmail.com> wrote: > > On Mon, Feb 18, 2019 at 10:31 AM Chris Packham <judge.packham@gmail.com> wrote: > > > > The generic wdt_start API expects to be called with the timeout in > > milliseconds. Update the orion_wdt driver to accept a timeout in > > milliseconds and use the clock rate specified in the dts to convert the > > timeout to an appropriate value for the timer reload register. > > > > Signed-off-by: Chris Packham <judge.packham@gmail.com> > > --- > > It turned out to be easy enough to make orion_wdt conform to the > > expected wdt APIs. It looks like the turris_mox already assumed a > > timeout in ms, so only the turris_omnia needed updating. > > > > Changes in v4: > > - use DIV_ROUND_UP to avoid setting the timeout to 0. > > > > Changes in v3: > > - new > > > > Changes in v2: None > > > > board/CZ.NIC/turris_omnia/turris_omnia.c | 2 +- > > drivers/watchdog/Kconfig | 1 + > > drivers/watchdog/orion_wdt.c | 23 +++++++++++++++++++---- > > 3 files changed, 21 insertions(+), 5 deletions(-) > > > > diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c > > index 1f7650cb3610..5e0b686c597d 100644 > > --- a/board/CZ.NIC/turris_omnia/turris_omnia.c > > +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c > > @@ -379,7 +379,7 @@ int board_init(void) > > puts("Cannot find Armada 385 watchdog!\n"); > > } else { > > puts("Enabling Armada 385 watchdog.\n"); > > - wdt_start(watchdog_dev, (u32) 25000000 * 120, 0); > > + wdt_start(watchdog_dev, 120000, 0); > > } > > # endif > > > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > > index 115fc4551ffd..7f7bc4bbc494 100644 > > --- a/drivers/watchdog/Kconfig > > +++ b/drivers/watchdog/Kconfig > > @@ -97,6 +97,7 @@ config WDT_BCM6345 > > config WDT_ORION > > bool "Orion watchdog timer support" > > depends on WDT > > + select CLK > > Actually I just realised I can't do this yet. The armada-38x lacks a > clock driver. So while the fixed-rate clock for the watchdog works > fine. Enabling CONFIG_CLK causes the uart to not work because it can't > look up it's clock. This is not initially noticeable because the SPL > sets up the hardware but if you try to change the baudrate things get > upset. > > I'm running out of u-boot hacking time to attempt to port the > armada-38x clock code from Linux. For now I'll send a quick v5 that > removes this select. *sigh* there is something about Mondays. To those outside of TZ +1300 sorry for the spam. I thought I'd killed the serial port due to the clocking but this all seems to work fine. The problem was that on my older setup I needed to set CONFIG_BOARD_EARLY_INIT_F (it's selected automatically for MVEBU in u-boot#master). So v4 should be good to go (barring any more review comments). > > > help > > Select this to enable Orion watchdog timer, which can be found on some > > Marvell Armada chips. > > diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c > > index c1add3e7c121..885821d562ea 100644 > > --- a/drivers/watchdog/orion_wdt.c > > +++ b/drivers/watchdog/orion_wdt.c > > @@ -14,7 +14,9 @@ > > > > #include <common.h> > > #include <dm.h> > > +#include <clk.h> > > #include <wdt.h> > > +#include <linux/kernel.h> > > #include <asm/io.h> > > #include <asm/arch/cpu.h> > > #include <asm/arch/soc.h> > > @@ -27,6 +29,8 @@ struct orion_wdt_priv { > > void __iomem *rstout; > > void __iomem *rstout_mask; > > u32 timeout; > > + unsigned long clk_rate; > > + struct clk clk; > > }; > > > > #define RSTOUT_ENABLE_BIT BIT(8) > > @@ -44,17 +48,18 @@ static int orion_wdt_reset(struct udevice *dev) > > struct orion_wdt_priv *priv = dev_get_priv(dev); > > > > /* Reload watchdog duration */ > > - writel(priv->timeout, priv->reg + priv->wdt_counter_offset); > > + writel(priv->clk_rate * priv->timeout, > > + priv->reg + priv->wdt_counter_offset); > > > > return 0; > > } > > > > -static int orion_wdt_start(struct udevice *dev, u64 timeout, ulong flags) > > +static int orion_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags) > > { > > struct orion_wdt_priv *priv = dev_get_priv(dev); > > u32 reg; > > > > - priv->timeout = (u32) timeout; > > + priv->timeout = DIV_ROUND_UP(timeout_ms, 1000); > > > > /* Enable the fixed watchdog clock input */ > > reg = readl(priv->reg + TIMER_CTRL); > > @@ -62,7 +67,8 @@ static int orion_wdt_start(struct udevice *dev, u64 timeout, ulong flags) > > writel(reg, priv->reg + TIMER_CTRL); > > > > /* Set watchdog duration */ > > - writel(priv->timeout, priv->reg + priv->wdt_counter_offset); > > + writel(priv->clk_rate * priv->timeout, > > + priv->reg + priv->wdt_counter_offset); > > > > /* Clear the watchdog expiration bit */ > > reg = readl(priv->reg + TIMER_A370_STATUS); > > @@ -147,9 +153,18 @@ err: > > > > static int orion_wdt_probe(struct udevice *dev) > > { > > + struct orion_wdt_priv *priv = dev_get_priv(dev); > > + int ret; > > + > > debug("%s: Probing wdt%u\n", __func__, dev->seq); > > orion_wdt_stop(dev); > > > > + ret = clk_get_by_name(dev, "fixed", &priv->clk); > > + if (!ret) > > + priv->clk_rate = clk_get_rate(&priv->clk); > > + else > > + priv->clk_rate = 25000000; > > + > > return 0; > > } > > > > -- > > 2.20.1 > >
On 17.02.19 22:30, Chris Packham wrote: > The generic wdt_start API expects to be called with the timeout in > milliseconds. Update the orion_wdt driver to accept a timeout in > milliseconds and use the clock rate specified in the dts to convert the > timeout to an appropriate value for the timer reload register. > > Signed-off-by: Chris Packham <judge.packham@gmail.com> > --- > It turned out to be easy enough to make orion_wdt conform to the > expected wdt APIs. It looks like the turris_mox already assumed a > timeout in ms, so only the turris_omnia needed updating. > > Changes in v4: > - use DIV_ROUND_UP to avoid setting the timeout to 0. > > Changes in v3: > - new > > Changes in v2: None Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan
On 17.02.19 22:30, Chris Packham wrote: > The generic wdt_start API expects to be called with the timeout in > milliseconds. Update the orion_wdt driver to accept a timeout in > milliseconds and use the clock rate specified in the dts to convert the > timeout to an appropriate value for the timer reload register. > > Signed-off-by: Chris Packham <judge.packham@gmail.com> > --- > It turned out to be easy enough to make orion_wdt conform to the > expected wdt APIs. It looks like the turris_mox already assumed a > timeout in ms, so only the turris_omnia needed updating. > > Changes in v4: > - use DIV_ROUND_UP to avoid setting the timeout to 0. > > Changes in v3: > - new > > Changes in v2: None Applied to u-boot-marvell/master. Thanks, Stefan
diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c index 1f7650cb3610..5e0b686c597d 100644 --- a/board/CZ.NIC/turris_omnia/turris_omnia.c +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c @@ -379,7 +379,7 @@ int board_init(void) puts("Cannot find Armada 385 watchdog!\n"); } else { puts("Enabling Armada 385 watchdog.\n"); - wdt_start(watchdog_dev, (u32) 25000000 * 120, 0); + wdt_start(watchdog_dev, 120000, 0); } # endif diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 115fc4551ffd..7f7bc4bbc494 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -97,6 +97,7 @@ config WDT_BCM6345 config WDT_ORION bool "Orion watchdog timer support" depends on WDT + select CLK help Select this to enable Orion watchdog timer, which can be found on some Marvell Armada chips. diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c index c1add3e7c121..885821d562ea 100644 --- a/drivers/watchdog/orion_wdt.c +++ b/drivers/watchdog/orion_wdt.c @@ -14,7 +14,9 @@ #include <common.h> #include <dm.h> +#include <clk.h> #include <wdt.h> +#include <linux/kernel.h> #include <asm/io.h> #include <asm/arch/cpu.h> #include <asm/arch/soc.h> @@ -27,6 +29,8 @@ struct orion_wdt_priv { void __iomem *rstout; void __iomem *rstout_mask; u32 timeout; + unsigned long clk_rate; + struct clk clk; }; #define RSTOUT_ENABLE_BIT BIT(8) @@ -44,17 +48,18 @@ static int orion_wdt_reset(struct udevice *dev) struct orion_wdt_priv *priv = dev_get_priv(dev); /* Reload watchdog duration */ - writel(priv->timeout, priv->reg + priv->wdt_counter_offset); + writel(priv->clk_rate * priv->timeout, + priv->reg + priv->wdt_counter_offset); return 0; } -static int orion_wdt_start(struct udevice *dev, u64 timeout, ulong flags) +static int orion_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags) { struct orion_wdt_priv *priv = dev_get_priv(dev); u32 reg; - priv->timeout = (u32) timeout; + priv->timeout = DIV_ROUND_UP(timeout_ms, 1000); /* Enable the fixed watchdog clock input */ reg = readl(priv->reg + TIMER_CTRL); @@ -62,7 +67,8 @@ static int orion_wdt_start(struct udevice *dev, u64 timeout, ulong flags) writel(reg, priv->reg + TIMER_CTRL); /* Set watchdog duration */ - writel(priv->timeout, priv->reg + priv->wdt_counter_offset); + writel(priv->clk_rate * priv->timeout, + priv->reg + priv->wdt_counter_offset); /* Clear the watchdog expiration bit */ reg = readl(priv->reg + TIMER_A370_STATUS); @@ -147,9 +153,18 @@ err: static int orion_wdt_probe(struct udevice *dev) { + struct orion_wdt_priv *priv = dev_get_priv(dev); + int ret; + debug("%s: Probing wdt%u\n", __func__, dev->seq); orion_wdt_stop(dev); + ret = clk_get_by_name(dev, "fixed", &priv->clk); + if (!ret) + priv->clk_rate = clk_get_rate(&priv->clk); + else + priv->clk_rate = 25000000; + return 0; }
The generic wdt_start API expects to be called with the timeout in milliseconds. Update the orion_wdt driver to accept a timeout in milliseconds and use the clock rate specified in the dts to convert the timeout to an appropriate value for the timer reload register. Signed-off-by: Chris Packham <judge.packham@gmail.com> --- It turned out to be easy enough to make orion_wdt conform to the expected wdt APIs. It looks like the turris_mox already assumed a timeout in ms, so only the turris_omnia needed updating. Changes in v4: - use DIV_ROUND_UP to avoid setting the timeout to 0. Changes in v3: - new Changes in v2: None board/CZ.NIC/turris_omnia/turris_omnia.c | 2 +- drivers/watchdog/Kconfig | 1 + drivers/watchdog/orion_wdt.c | 23 +++++++++++++++++++---- 3 files changed, 21 insertions(+), 5 deletions(-)