| Message ID | 20200331060838.24374-1-rayagonda.kokatanur@broadcom.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | [v1,1/1] driver: watchdog: get timeout and clock from dt file | expand |
On 31.03.20 08:08, Rayagonda Kokatanur wrote: > From: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com> > > Get the watchdog default timeout value and clock from the DTS file The default timeout is already read from the DT. Please see below... > Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com> > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com> > --- > drivers/watchdog/sp805_wdt.c | 36 ++++++++++++++++++++++++++++++++++-- > 1 file changed, 34 insertions(+), 2 deletions(-) > > diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c > index ca3ccbe76c..e8f54d6804 100644 > --- a/drivers/watchdog/sp805_wdt.c > +++ b/drivers/watchdog/sp805_wdt.c > @@ -34,8 +34,31 @@ DECLARE_GLOBAL_DATA_PTR; > > struct sp805_wdt_priv { > void __iomem *reg; > + u32 timeout_msec; > + u32 clk_mhz; > }; > > +static u32 msec_to_ticks(struct udevice *dev) > +{ > + u32 timeout_msec; > + u32 msec; > + struct sp805_wdt_priv *priv = dev_get_priv(dev); > + > + timeout_msec = env_get_ulong("wdt_timeout_msec", 10, 0); > + if (timeout_msec) { > + dev_dbg(dev, "Overriding timeout :%u\n", timeout_msec); > + msec = timeout_msec; > + } else { > + msec = priv->timeout_msec; > + } Why is this overriding via environment needed? BTW: You should mention such a change in the commit text as well. > + > + timeout_msec = (msec / 2) * (priv->clk_mhz / 1000); > + > + dev_dbg(dev, "ticks :%u\n", timeout_msec); > + > + return timeout_msec; > +} > + > static int sp805_wdt_reset(struct udevice *dev) > { > struct sp805_wdt_priv *priv = dev_get_priv(dev); > @@ -63,8 +86,11 @@ static int sp805_wdt_start(struct udevice *dev, u64 timeout, ulong flags) > * set 120s, the gd->bus_clk is less than 1145MHz, the load_value will > * not overflow. > */ > - load_value = (gd->bus_clk) / > - (2 * 1000 * SYS_FSL_WDT_CLK_DIV) * load_time; > + if (gd->bus_clk) > + load_value = (gd->bus_clk) / > + (2 * 1000 * SYS_FSL_WDT_CLK_DIV) * load_time; > + else /* platform provide clk */ > + load_value = msec_to_ticks(dev); Nitpicking: Please use paranthesis on multi-line statements. > writel(UNLOCK, priv->reg + WDTLOCK); > writel(load_value, priv->reg + WDTLOAD); > @@ -110,6 +136,12 @@ static int sp805_wdt_ofdata_to_platdata(struct udevice *dev) > if (IS_ERR(priv->reg)) > return PTR_ERR(priv->reg); > > + if (dev_read_u32(dev, "timeout-msec", &priv->timeout_msec)) > + return -ENODATA; This does not seem to be an official DT binding. At least I was unable to find it in the latest Linux tree. You are aware that the WDT core code already reads the WDT timeout from the DT using the official "timeout-sec" property? Do you need a higher graned timeout value (ms vs s)? Thanks, Stefan
On Tue, Mar 31, 2020 at 12:44 PM Stefan Roese <sr@denx.de> wrote: > > On 31.03.20 08:08, Rayagonda Kokatanur wrote: > > From: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com> > > > > Get the watchdog default timeout value and clock from the DTS file > > The default timeout is already read from the DT. Please see below... Thank you, will fix this. > > > Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com> > > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com> > > --- > > drivers/watchdog/sp805_wdt.c | 36 ++++++++++++++++++++++++++++++++++-- > > 1 file changed, 34 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c > > index ca3ccbe76c..e8f54d6804 100644 > > --- a/drivers/watchdog/sp805_wdt.c > > +++ b/drivers/watchdog/sp805_wdt.c > > @@ -34,8 +34,31 @@ DECLARE_GLOBAL_DATA_PTR; > > > > struct sp805_wdt_priv { > > void __iomem *reg; > > + u32 timeout_msec; > > + u32 clk_mhz; > > }; > > > > +static u32 msec_to_ticks(struct udevice *dev) > > +{ > > + u32 timeout_msec; > > + u32 msec; > > + struct sp805_wdt_priv *priv = dev_get_priv(dev); > > + > > + timeout_msec = env_get_ulong("wdt_timeout_msec", 10, 0); > > + if (timeout_msec) { > > + dev_dbg(dev, "Overriding timeout :%u\n", timeout_msec); > > + msec = timeout_msec; > > + } else { > > + msec = priv->timeout_msec; > > + } > > Why is this overriding via environment needed? BTW: You should mention > such a change in the commit text as well. This code will be removed as default timeout is already read in include/wdt.h > > > + > > + timeout_msec = (msec / 2) * (priv->clk_mhz / 1000); > > + > > + dev_dbg(dev, "ticks :%u\n", timeout_msec); > > + > > + return timeout_msec; > > +} > > + > > static int sp805_wdt_reset(struct udevice *dev) > > { > > struct sp805_wdt_priv *priv = dev_get_priv(dev); > > @@ -63,8 +86,11 @@ static int sp805_wdt_start(struct udevice *dev, u64 timeout, ulong flags) > > * set 120s, the gd->bus_clk is less than 1145MHz, the load_value will > > * not overflow. > > */ > > - load_value = (gd->bus_clk) / > > - (2 * 1000 * SYS_FSL_WDT_CLK_DIV) * load_time; > > + if (gd->bus_clk) > > + load_value = (gd->bus_clk) / > > + (2 * 1000 * SYS_FSL_WDT_CLK_DIV) * load_time; > > + else /* platform provide clk */ > > + load_value = msec_to_ticks(dev); > > Nitpicking: Please use paranthesis on multi-line statements. Thank you fix this. > > > writel(UNLOCK, priv->reg + WDTLOCK); > > writel(load_value, priv->reg + WDTLOAD); > > @@ -110,6 +136,12 @@ static int sp805_wdt_ofdata_to_platdata(struct udevice *dev) > > if (IS_ERR(priv->reg)) > > return PTR_ERR(priv->reg); > > > > + if (dev_read_u32(dev, "timeout-msec", &priv->timeout_msec)) > > + return -ENODATA; > > This does not seem to be an official DT binding. At least I was unable > to find it in the latest Linux tree. > > You are aware that the WDT core code already reads the WDT timeout from > the DT using the official "timeout-sec" property? Do you need a higher > graned timeout value (ms vs s)? Thank you for pointing this, will update my dt file. > > Thanks, > Stefan
diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c index ca3ccbe76c..e8f54d6804 100644 --- a/drivers/watchdog/sp805_wdt.c +++ b/drivers/watchdog/sp805_wdt.c @@ -34,8 +34,31 @@ DECLARE_GLOBAL_DATA_PTR; struct sp805_wdt_priv { void __iomem *reg; + u32 timeout_msec; + u32 clk_mhz; }; +static u32 msec_to_ticks(struct udevice *dev) +{ + u32 timeout_msec; + u32 msec; + struct sp805_wdt_priv *priv = dev_get_priv(dev); + + timeout_msec = env_get_ulong("wdt_timeout_msec", 10, 0); + if (timeout_msec) { + dev_dbg(dev, "Overriding timeout :%u\n", timeout_msec); + msec = timeout_msec; + } else { + msec = priv->timeout_msec; + } + + timeout_msec = (msec / 2) * (priv->clk_mhz / 1000); + + dev_dbg(dev, "ticks :%u\n", timeout_msec); + + return timeout_msec; +} + static int sp805_wdt_reset(struct udevice *dev) { struct sp805_wdt_priv *priv = dev_get_priv(dev); @@ -63,8 +86,11 @@ static int sp805_wdt_start(struct udevice *dev, u64 timeout, ulong flags) * set 120s, the gd->bus_clk is less than 1145MHz, the load_value will * not overflow. */ - load_value = (gd->bus_clk) / - (2 * 1000 * SYS_FSL_WDT_CLK_DIV) * load_time; + if (gd->bus_clk) + load_value = (gd->bus_clk) / + (2 * 1000 * SYS_FSL_WDT_CLK_DIV) * load_time; + else /* platform provide clk */ + load_value = msec_to_ticks(dev); writel(UNLOCK, priv->reg + WDTLOCK); writel(load_value, priv->reg + WDTLOAD); @@ -110,6 +136,12 @@ static int sp805_wdt_ofdata_to_platdata(struct udevice *dev) if (IS_ERR(priv->reg)) return PTR_ERR(priv->reg); + if (dev_read_u32(dev, "timeout-msec", &priv->timeout_msec)) + return -ENODATA; + + if (dev_read_u32(dev, "clk-mhz", &priv->clk_mhz)) + return -ENODATA; + return 0; }