diff mbox

[1/2] ARM: i.MX system: Simplify handling watchdog clock

Message ID 1402644373-8408-1-git-send-email-shc_work@mail.ru (mailing list archive)
State New, archived
Headers show

Commit Message

Alexander Shiyan June 13, 2014, 7:26 a.m. UTC
This patch simplifies handling watchdog clock a bit.
As an additional change, now we properly check WDT clock in a reset
function.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 arch/arm/mach-imx/system.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

Comments

Shawn Guo June 20, 2014, 4:26 a.m. UTC | #1
On Fri, Jun 13, 2014 at 11:26:12AM +0400, Alexander Shiyan wrote:
> This patch simplifies handling watchdog clock a bit.
> As an additional change, now we properly check WDT clock in a reset
> function.
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  arch/arm/mach-imx/system.c | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/system.c b/arch/arm/mach-imx/system.c
> index 3b0733e..ae521f3 100644
> --- a/arch/arm/mach-imx/system.c
> +++ b/arch/arm/mach-imx/system.c
> @@ -42,7 +42,7 @@ void mxc_restart(enum reboot_mode mode, const char *cmd)
>  {
>  	unsigned int wcr_enable;
>  
> -	if (wdog_clk)
> +	if (!IS_ERR_OR_NULL(wdog_clk))

I think this should just be if (!IS_ERR(wdog_clk)).

Shawn

>  		clk_enable(wdog_clk);
>  
>  	if (cpu_is_mx1())
> @@ -79,13 +79,10 @@ void __init mxc_arch_reset_init(void __iomem *base)
>  	wdog_base = base;
>  
>  	wdog_clk = clk_get_sys("imx2-wdt.0", NULL);
> -	if (IS_ERR(wdog_clk)) {
> +	if (IS_ERR(wdog_clk))
>  		pr_warn("%s: failed to get wdog clock\n", __func__);
> -		wdog_clk = NULL;
> -		return;
> -	}
> -
> -	clk_prepare(wdog_clk);
> +	else
> +		clk_prepare(wdog_clk);
>  }
>  
>  void __init mxc_arch_reset_init_dt(void)
> @@ -97,13 +94,10 @@ void __init mxc_arch_reset_init_dt(void)
>  	WARN_ON(!wdog_base);
>  
>  	wdog_clk = of_clk_get(np, 0);
> -	if (IS_ERR(wdog_clk)) {
> +	if (IS_ERR(wdog_clk))
>  		pr_warn("%s: failed to get wdog clock\n", __func__);
> -		wdog_clk = NULL;
> -		return;
> -	}
> -
> -	clk_prepare(wdog_clk);
> +	else
> +		clk_prepare(wdog_clk);
>  }
>  
>  #ifdef CONFIG_CACHE_L2X0
> -- 
> 1.8.5.5
>
Alexander Shiyan June 20, 2014, 4:36 a.m. UTC | #2
Fri, 20 Jun 2014 12:26:52 +0800 ?? Shawn Guo <shawn.guo@freescale.com>:
> On Fri, Jun 13, 2014 at 11:26:12AM +0400, Alexander Shiyan wrote:
> > This patch simplifies handling watchdog clock a bit.
> > As an additional change, now we properly check WDT clock in a reset
> > function.
> > 
> > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > ---
> >  arch/arm/mach-imx/system.c | 20 +++++++-------------
> >  1 file changed, 7 insertions(+), 13 deletions(-)
> > 
> > diff --git a/arch/arm/mach-imx/system.c b/arch/arm/mach-imx/system.c
> > index 3b0733e..ae521f3 100644
> > --- a/arch/arm/mach-imx/system.c
> > +++ b/arch/arm/mach-imx/system.c
> > @@ -42,7 +42,7 @@ void mxc_restart(enum reboot_mode mode, const char *cmd)
> >  {
> >  	unsigned int wcr_enable;
> >  
> > -	if (wdog_clk)
> > +	if (!IS_ERR_OR_NULL(wdog_clk))
> 
> I think this should just be if (!IS_ERR(wdog_clk)).

No, this is a part of commit log.
wdog_clk can be NULL if mxc_arch_reset_init() is not called, 
so we just protect mxc_restart() for such cases.

---
Shawn Guo June 20, 2014, 7:50 a.m. UTC | #3
On Fri, Jun 20, 2014 at 08:36:46AM +0400, Alexander Shiyan wrote:
> Fri, 20 Jun 2014 12:26:52 +0800 ?? Shawn Guo <shawn.guo@freescale.com>:
> > On Fri, Jun 13, 2014 at 11:26:12AM +0400, Alexander Shiyan wrote:
> > > This patch simplifies handling watchdog clock a bit.
> > > As an additional change, now we properly check WDT clock in a reset
> > > function.
> > > 
> > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > > ---
> > >  arch/arm/mach-imx/system.c | 20 +++++++-------------
> > >  1 file changed, 7 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/arch/arm/mach-imx/system.c b/arch/arm/mach-imx/system.c
> > > index 3b0733e..ae521f3 100644
> > > --- a/arch/arm/mach-imx/system.c
> > > +++ b/arch/arm/mach-imx/system.c
> > > @@ -42,7 +42,7 @@ void mxc_restart(enum reboot_mode mode, const char *cmd)
> > >  {
> > >  	unsigned int wcr_enable;
> > >  
> > > -	if (wdog_clk)
> > > +	if (!IS_ERR_OR_NULL(wdog_clk))
> > 
> > I think this should just be if (!IS_ERR(wdog_clk)).
> 
> No, this is a part of commit log.
> wdog_clk can be NULL if mxc_arch_reset_init() is not called, 
> so we just protect mxc_restart() for such cases.

I think clk API can take NULL as input.

Shawn
Alexander Shiyan June 20, 2014, 7:57 a.m. UTC | #4
Fri, 20 Jun 2014 15:50:34 +0800 ?? Shawn Guo <shawn.guo@freescale.com>:
> On Fri, Jun 20, 2014 at 08:36:46AM +0400, Alexander Shiyan wrote:
> > Fri, 20 Jun 2014 12:26:52 +0800 ?? Shawn Guo <shawn.guo@freescale.com>:
> > > On Fri, Jun 13, 2014 at 11:26:12AM +0400, Alexander Shiyan wrote:
> > > > This patch simplifies handling watchdog clock a bit.
> > > > As an additional change, now we properly check WDT clock in a reset
> > > > function.
> > > > 
> > > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > > > ---
> > > >  arch/arm/mach-imx/system.c | 20 +++++++-------------
> > > >  1 file changed, 7 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/mach-imx/system.c b/arch/arm/mach-imx/system.c
> > > > index 3b0733e..ae521f3 100644
> > > > --- a/arch/arm/mach-imx/system.c
> > > > +++ b/arch/arm/mach-imx/system.c
> > > > @@ -42,7 +42,7 @@ void mxc_restart(enum reboot_mode mode, const char *cmd)
> > > >  {
> > > >  	unsigned int wcr_enable;
> > > >  
> > > > -	if (wdog_clk)
> > > > +	if (!IS_ERR_OR_NULL(wdog_clk))
> > > 
> > > I think this should just be if (!IS_ERR(wdog_clk)).
> > 
> > No, this is a part of commit log.
> > wdog_clk can be NULL if mxc_arch_reset_init() is not called, 
> > so we just protect mxc_restart() for such cases.
> 
> I think clk API can take NULL as input.

You are right.
If this is the only issue on this series, could you fix it during the applying?

---
Shawn Guo June 20, 2014, 8:52 a.m. UTC | #5
On Fri, Jun 20, 2014 at 11:57:45AM +0400, Alexander Shiyan wrote:
> You are right.
> If this is the only issue on this series, could you fix it during the applying?

Okay.  Fixed it and applied both patches.

Shawn
diff mbox

Patch

diff --git a/arch/arm/mach-imx/system.c b/arch/arm/mach-imx/system.c
index 3b0733e..ae521f3 100644
--- a/arch/arm/mach-imx/system.c
+++ b/arch/arm/mach-imx/system.c
@@ -42,7 +42,7 @@  void mxc_restart(enum reboot_mode mode, const char *cmd)
 {
 	unsigned int wcr_enable;
 
-	if (wdog_clk)
+	if (!IS_ERR_OR_NULL(wdog_clk))
 		clk_enable(wdog_clk);
 
 	if (cpu_is_mx1())
@@ -79,13 +79,10 @@  void __init mxc_arch_reset_init(void __iomem *base)
 	wdog_base = base;
 
 	wdog_clk = clk_get_sys("imx2-wdt.0", NULL);
-	if (IS_ERR(wdog_clk)) {
+	if (IS_ERR(wdog_clk))
 		pr_warn("%s: failed to get wdog clock\n", __func__);
-		wdog_clk = NULL;
-		return;
-	}
-
-	clk_prepare(wdog_clk);
+	else
+		clk_prepare(wdog_clk);
 }
 
 void __init mxc_arch_reset_init_dt(void)
@@ -97,13 +94,10 @@  void __init mxc_arch_reset_init_dt(void)
 	WARN_ON(!wdog_base);
 
 	wdog_clk = of_clk_get(np, 0);
-	if (IS_ERR(wdog_clk)) {
+	if (IS_ERR(wdog_clk))
 		pr_warn("%s: failed to get wdog clock\n", __func__);
-		wdog_clk = NULL;
-		return;
-	}
-
-	clk_prepare(wdog_clk);
+	else
+		clk_prepare(wdog_clk);
 }
 
 #ifdef CONFIG_CACHE_L2X0