diff mbox series

[v2,1/1] driver: watchdog: get platform clock from dt file

Message ID 20200401123128.1547-1-rayagonda.kokatanur@broadcom.com
State New
Headers show
Series [v2,1/1] driver: watchdog: get platform clock from dt file | expand

Commit Message

Rayagonda Kokatanur April 1, 2020, 12:31 p.m. UTC
From: Bharat Kumar Reddy Gooty <bharat.gooty at 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 at broadcom.com>
Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at 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.

 drivers/watchdog/sp805_wdt.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Stefan Roese April 2, 2020, 6:28 a.m. UTC | #1
On 01.04.20 14:31, Rayagonda Kokatanur wrote:
> From: Bharat Kumar Reddy Gooty <bharat.gooty at 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 at broadcom.com>
> Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at 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
Rayagonda Kokatanur April 2, 2020, 6:35 a.m. UTC | #2
On Thu, Apr 2, 2020 at 11:58 AM Stefan Roese <sr at denx.de> wrote:
>
> On 01.04.20 14:31, Rayagonda Kokatanur wrote:
> > From: Bharat Kumar Reddy Gooty <bharat.gooty at 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 at broadcom.com>
> > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at 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 mbox series

Patch

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;
 }