diff mbox

arm/tegra: clk_get should not be fatal

Message ID 1319559329-324-1-git-send-email-pdeschrijver@nvidia.com
State Superseded, archived
Headers show

Commit Message

Peter De Schrijver Oct. 25, 2011, 4:15 p.m. UTC 
From: Peter De Schrijver <pdeschrijver@nvidia.com>

The timer and rtc-timer clocks aren't gated by default, so there is no reason
to crash the system if the dummy enable call failed.

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 arch/arm/mach-tegra/timer.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

Comments

Joe Perches Oct. 25, 2011, 4:26 p.m. UTC | #1 
On Tue, 2011-10-25 at 19:15 +0300, pdeschrijver@nvidia.com wrote:
> The timer and rtc-timer clocks aren't gated by default, so there is no reason
> to crash the system if the dummy enable call failed.
[]
> diff --git a/arch/arm/mach-tegra/timer.c b/arch/arm/mach-tegra/timer.c
[]
> @@ -186,16 +186,20 @@ static void __init tegra_init_timer(void)
>  	int ret;
>  
>  	clk = clk_get_sys("timer", NULL);
> -	BUG_ON(IS_ERR(clk));
> -	clk_enable(clk);
> +	if (IS_ERR(clk))
> +		pr_warning("Unable to get timer clock");
> +	else
> +		clk_enable(clk);
>  
>  	/*
>  	 * rtc registers are used by read_persistent_clock, keep the rtc clock
>  	 * enabled
>  	 */
>  	clk = clk_get_sys("rtc-tegra", NULL);
> -	BUG_ON(IS_ERR(clk));
> -	clk_enable(clk);
> +	if (IS_ERR(clk))
> +		pr_warning("Unable to get rtc-tegra clock");
> +	else
> +		clk_enable(clk);

Are these messages are really necessary?
Maybe just:
	if (!IS_ERR(clk))
		clk_enable(clk)

If these are really necessary, please use
	pr_warn("Unable to get <foo>\n");
pr_warn and with a terminating newline.

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter De Schrijver Oct. 25, 2011, 5 p.m. UTC | #2 
On Tue, Oct 25, 2011 at 06:26:08PM +0200, Joe Perches wrote:
> On Tue, 2011-10-25 at 19:15 +0300, pdeschrijver@nvidia.com wrote:
> > The timer and rtc-timer clocks aren't gated by default, so there is no reason
> > to crash the system if the dummy enable call failed.
> []
> > diff --git a/arch/arm/mach-tegra/timer.c b/arch/arm/mach-tegra/timer.c
> []
> > @@ -186,16 +186,20 @@ static void __init tegra_init_timer(void)
> >  	int ret;
> >  
> >  	clk = clk_get_sys("timer", NULL);
> > -	BUG_ON(IS_ERR(clk));
> > -	clk_enable(clk);
> > +	if (IS_ERR(clk))
> > +		pr_warning("Unable to get timer clock");
> > +	else
> > +		clk_enable(clk);
> >  
> >  	/*
> >  	 * rtc registers are used by read_persistent_clock, keep the rtc clock
> >  	 * enabled
> >  	 */
> >  	clk = clk_get_sys("rtc-tegra", NULL);
> > -	BUG_ON(IS_ERR(clk));
> > -	clk_enable(clk);
> > +	if (IS_ERR(clk))
> > +		pr_warning("Unable to get rtc-tegra clock");
> > +	else
> > +		clk_enable(clk);
> 
> Are these messages are really necessary?

I think it's still useful to have them as not having a clock fw is a strange
situation. It's not fatal though, but worth a warning I would say.

> Maybe just:
> 	if (!IS_ERR(clk))
> 		clk_enable(clk)
> 
> If these are really necessary, please use
> 	pr_warn("Unable to get <foo>\n");
> pr_warn and with a terminating newline.
> 

Is pr_warn any different then pr_warning? Point taken about the newline.

Cheers,

Peter.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joe Perches Oct. 26, 2011, 1:15 a.m. UTC | #3 
On Tue, 2011-10-25 at 20:00 +0300, Peter De Schrijver wrote:
> Is pr_warn any different then pr_warning?

Other than being 3 characters shorter to type and it also
lets some format strings fit sub 80 chars line length?  No.

It is more like dev_warn, netdev_warn, et al.

cheers, Joe

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mach-tegra/timer.c b/arch/arm/mach-tegra/timer.c
index e2272d2..e12b064 100644
--- a/arch/arm/mach-tegra/timer.c
+++ b/arch/arm/mach-tegra/timer.c
@@ -186,16 +186,20 @@  static void __init tegra_init_timer(void)
 	int ret;
 
 	clk = clk_get_sys("timer", NULL);
-	BUG_ON(IS_ERR(clk));
-	clk_enable(clk);
+	if (IS_ERR(clk))
+		pr_warning("Unable to get timer clock");
+	else
+		clk_enable(clk);
 
 	/*
 	 * rtc registers are used by read_persistent_clock, keep the rtc clock
 	 * enabled
 	 */
 	clk = clk_get_sys("rtc-tegra", NULL);
-	BUG_ON(IS_ERR(clk));
-	clk_enable(clk);
+	if (IS_ERR(clk))
+		pr_warning("Unable to get rtc-tegra clock");
+	else
+		clk_enable(clk);
 
 #ifdef CONFIG_HAVE_ARM_TWD
 	twd_base = IO_ADDRESS(TEGRA_ARM_PERIF_BASE + 0x600);