| Message ID | 20200401123128.1547-1-rayagonda.kokatanur@broadcom.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | [v2,1/1] driver: watchdog: get platform clock from dt file | expand |
On 01.04.20 14:31, Rayagonda Kokatanur wrote: > From: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com> > > Get the watchdog platform clock from the DTS file > and use the same for calculating ticks in msec. > > Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com> > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com> > --- > Changes from v1: > -Address review comments from Stefan Roese, > Remove default timeout reading in probe() as it is read in include/wdt.h. > Add paranthesis on multi-line statements. Looks much better, thanks. One comment still below... > drivers/watchdog/sp805_wdt.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c > index ca3ccbe76c..b9e4aee7a5 100644 > --- a/drivers/watchdog/sp805_wdt.c > +++ b/drivers/watchdog/sp805_wdt.c > @@ -34,6 +34,7 @@ DECLARE_GLOBAL_DATA_PTR; > > struct sp805_wdt_priv { > void __iomem *reg; > + u32 clk_mhz; > }; > > static int sp805_wdt_reset(struct udevice *dev) > @@ -63,8 +64,13 @@ 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 = (timeout / 2) * (priv->clk_mhz / 1000); > + } > > writel(UNLOCK, priv->reg + WDTLOCK); > writel(load_value, priv->reg + WDTLOAD); > @@ -110,6 +116,9 @@ 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, "clk-mhz", &priv->clk_mhz)) > + return -ENODATA; > + "clk-mhz" doesn't seem to be an official DT binding. Can't you use the clk subsystem to get the base frequency of this device? On which platform do you use the WDT driver btw? Thanks, Stefan
On Thu, Apr 2, 2020 at 11:58 AM Stefan Roese <sr@denx.de> wrote: > > On 01.04.20 14:31, Rayagonda Kokatanur wrote: > > From: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com> > > > > Get the watchdog platform clock from the DTS file > > and use the same for calculating ticks in msec. > > > > Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com> > > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com> > > --- > > Changes from v1: > > -Address review comments from Stefan Roese, > > Remove default timeout reading in probe() as it is read in include/wdt.h. > > Add paranthesis on multi-line statements. > > Looks much better, thanks. One comment still below... thank you for reviewing. > > > drivers/watchdog/sp805_wdt.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c > > index ca3ccbe76c..b9e4aee7a5 100644 > > --- a/drivers/watchdog/sp805_wdt.c > > +++ b/drivers/watchdog/sp805_wdt.c > > @@ -34,6 +34,7 @@ DECLARE_GLOBAL_DATA_PTR; > > > > struct sp805_wdt_priv { > > void __iomem *reg; > > + u32 clk_mhz; > > }; > > > > static int sp805_wdt_reset(struct udevice *dev) > > @@ -63,8 +64,13 @@ 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 = (timeout / 2) * (priv->clk_mhz / 1000); > > + } > > > > writel(UNLOCK, priv->reg + WDTLOCK); > > writel(load_value, priv->reg + WDTLOAD); > > @@ -110,6 +116,9 @@ 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, "clk-mhz", &priv->clk_mhz)) > > + return -ENODATA; > > + > > "clk-mhz" doesn't seem to be an official DT binding. Can't you use the > clk subsystem to get the base frequency of this device? On which > platform do you use the WDT driver btw? I will check the clk subsystem, thank you. It will be used in broadcom ns3 platform. I will send the platform patch as well, started with driver files first. > > Thanks, > Stefan
diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c index ca3ccbe76c..b9e4aee7a5 100644 --- a/drivers/watchdog/sp805_wdt.c +++ b/drivers/watchdog/sp805_wdt.c @@ -34,6 +34,7 @@ DECLARE_GLOBAL_DATA_PTR; struct sp805_wdt_priv { void __iomem *reg; + u32 clk_mhz; }; static int sp805_wdt_reset(struct udevice *dev) @@ -63,8 +64,13 @@ 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 = (timeout / 2) * (priv->clk_mhz / 1000); + } writel(UNLOCK, priv->reg + WDTLOCK); writel(load_value, priv->reg + WDTLOAD); @@ -110,6 +116,9 @@ 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, "clk-mhz", &priv->clk_mhz)) + return -ENODATA; + return 0; }