diff mbox series

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

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

Commit Message

Rayagonda Kokatanur April 6, 2020, 7:15 a.m. UTC
Get the watchdog platform clock from the DTS file
using clk subsystem and use the same for calculating
ticks in msec.

Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com>
Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty at broadcom.com>
---
Changes from v2:
 -Address review comments from Stefan Roese,
  Use clk subsystem to get the base frequency of device instead
  of dt parameter "clk_mhz".

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 | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

Comments

Stefan Roese April 6, 2020, 7:47 a.m. UTC | #1
On 06.04.20 09:15, Rayagonda Kokatanur wrote:
> Get the watchdog platform clock from the DTS file
> using clk subsystem and use the same for calculating
> ticks in msec.
> 
> Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com>
> Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty at broadcom.com>
> ---
> Changes from v2:
>   -Address review comments from Stefan Roese,
>    Use clk subsystem to get the base frequency of device instead
>    of dt parameter "clk_mhz".
> 
> 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 | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
> index ca3ccbe76c..d377ae0fc5 100644
> --- a/drivers/watchdog/sp805_wdt.c
> +++ b/drivers/watchdog/sp805_wdt.c
> @@ -7,6 +7,7 @@
>   
>   #include <asm/io.h>
>   #include <common.h>
> +#include <clk.h>
>   #include <dm/device.h>
>   #include <dm/fdtaddr.h>
>   #include <dm/read.h>
> @@ -34,6 +35,7 @@ DECLARE_GLOBAL_DATA_PTR;
>   
>   struct sp805_wdt_priv {
>   	void __iomem *reg;
> +	unsigned long clk_rate;
>   };
>   
>   static int sp805_wdt_reset(struct udevice *dev)
> @@ -63,8 +65,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_rate / 1000);
> +	}
>   
>   	writel(UNLOCK, priv->reg + WDTLOCK);
>   	writel(load_value, priv->reg + WDTLOAD);
> @@ -105,11 +112,19 @@ static int sp805_wdt_probe(struct udevice *dev)
>   static int sp805_wdt_ofdata_to_platdata(struct udevice *dev)
>   {
>   	struct sp805_wdt_priv *priv = dev_get_priv(dev);
> +	struct clk clk;
> +	int ret;
>   
>   	priv->reg = (void __iomem *)dev_read_addr(dev);
>   	if (IS_ERR(priv->reg))
>   		return PTR_ERR(priv->reg);
>   
> +	ret = clk_get_by_index(dev, 0, &clk);
> +	if (!ret)
> +		priv->clk_rate = clk_get_rate(&clk);
> +	else
> +		return -EINVAL;
> +

Thanks for addresses this. Looks much better now, without all the
ad-hoc DT bindings.

Though I suspect that some boards using this WDT driver do not fully
support the CLK API for this device. That's why we still have this
"bus_clk" in global-data here. Some NXP folks might test and report
if this works for them. Otherwise sp805_wdt_ofdata_to_platdata() should
not fail if clk_get_by_index() returns with error.

Thanks,
Stefan
Rayagonda Kokatanur April 6, 2020, 7:54 a.m. UTC | #2
On Mon, Apr 6, 2020 at 1:17 PM Stefan Roese <sr at denx.de> wrote:
>
> On 06.04.20 09:15, Rayagonda Kokatanur wrote:
> > Get the watchdog platform clock from the DTS file
> > using clk subsystem and use the same for calculating
> > ticks in msec.
> >
> > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com>
> > Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty at broadcom.com>
> > ---
> > Changes from v2:
> >   -Address review comments from Stefan Roese,
> >    Use clk subsystem to get the base frequency of device instead
> >    of dt parameter "clk_mhz".
> >
> > 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 | 19 +++++++++++++++++--
> >   1 file changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
> > index ca3ccbe76c..d377ae0fc5 100644
> > --- a/drivers/watchdog/sp805_wdt.c
> > +++ b/drivers/watchdog/sp805_wdt.c
> > @@ -7,6 +7,7 @@
> >
> >   #include <asm/io.h>
> >   #include <common.h>
> > +#include <clk.h>
> >   #include <dm/device.h>
> >   #include <dm/fdtaddr.h>
> >   #include <dm/read.h>
> > @@ -34,6 +35,7 @@ DECLARE_GLOBAL_DATA_PTR;
> >
> >   struct sp805_wdt_priv {
> >       void __iomem *reg;
> > +     unsigned long clk_rate;
> >   };
> >
> >   static int sp805_wdt_reset(struct udevice *dev)
> > @@ -63,8 +65,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_rate / 1000);
> > +     }
> >
> >       writel(UNLOCK, priv->reg + WDTLOCK);
> >       writel(load_value, priv->reg + WDTLOAD);
> > @@ -105,11 +112,19 @@ static int sp805_wdt_probe(struct udevice *dev)
> >   static int sp805_wdt_ofdata_to_platdata(struct udevice *dev)
> >   {
> >       struct sp805_wdt_priv *priv = dev_get_priv(dev);
> > +     struct clk clk;
> > +     int ret;
> >
> >       priv->reg = (void __iomem *)dev_read_addr(dev);
> >       if (IS_ERR(priv->reg))
> >               return PTR_ERR(priv->reg);
> >
> > +     ret = clk_get_by_index(dev, 0, &clk);
> > +     if (!ret)
> > +             priv->clk_rate = clk_get_rate(&clk);
> > +     else
> > +             return -EINVAL;
> > +
>
> Thanks for addresses this. Looks much better now, without all the
> ad-hoc DT bindings.
>
> Though I suspect that some boards using this WDT driver do not fully
> support the CLK API for this device. That's why we still have this
> "bus_clk" in global-data here. Some NXP folks might test and report
> if this works for them. Otherwise sp805_wdt_ofdata_to_platdata() should
> not fail if clk_get_by_index() returns with error.

Okay, will fix it.

Thank you
Rayagonda

>
> Thanks,
> Stefan
diff mbox series

Patch

diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index ca3ccbe76c..d377ae0fc5 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -7,6 +7,7 @@ 
 
 #include <asm/io.h>
 #include <common.h>
+#include <clk.h>
 #include <dm/device.h>
 #include <dm/fdtaddr.h>
 #include <dm/read.h>
@@ -34,6 +35,7 @@  DECLARE_GLOBAL_DATA_PTR;
 
 struct sp805_wdt_priv {
 	void __iomem *reg;
+	unsigned long clk_rate;
 };
 
 static int sp805_wdt_reset(struct udevice *dev)
@@ -63,8 +65,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_rate / 1000);
+	}
 
 	writel(UNLOCK, priv->reg + WDTLOCK);
 	writel(load_value, priv->reg + WDTLOAD);
@@ -105,11 +112,19 @@  static int sp805_wdt_probe(struct udevice *dev)
 static int sp805_wdt_ofdata_to_platdata(struct udevice *dev)
 {
 	struct sp805_wdt_priv *priv = dev_get_priv(dev);
+	struct clk clk;
+	int ret;
 
 	priv->reg = (void __iomem *)dev_read_addr(dev);
 	if (IS_ERR(priv->reg))
 		return PTR_ERR(priv->reg);
 
+	ret = clk_get_by_index(dev, 0, &clk);
+	if (!ret)
+		priv->clk_rate = clk_get_rate(&clk);
+	else
+		return -EINVAL;
+
 	return 0;
 }