Re: [PATCH 5/17] ARM: OMAP2: Change 24xx to use new register access

[Tony Lindgren <tony@atomide.com>]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [080407 16:49]:
> On Tue, Mar 18, 2008 at 04:02:03PM +0200, Tony Lindgren wrote:
> > From: Paul Walmsley <paul@pwsan.com>
> > 
> > This patch changes 24xx to use new register access, except for clock
> > framework. Clock framework register access will get updates in the
> > next patch.
> 
> Hmm...
> 
> > @@ -187,13 +189,40 @@ static inline void __init apollon_init_smc91x(void)
> >  {
> >  	unsigned long base;
> >  
> > +	unsigned int rate;
> > +	struct clk *l3ck;
> > +	int eth_cs;
> > +
> > +	l3ck = clk_get(NULL, "core_l3_ck");
> > +	if (IS_ERR(l3ck))
> > +		rate = 100000000;
> > +	else
> > +		rate = clk_get_rate(l3ck);
> 
> Now read:
> 
> /**
>  * clk_get_rate - obtain the current clock rate (in Hz) for a clock source.
>  *                This is only valid once the clock source has been enabled.
>  * @clk: clock source
>  */
> unsigned long clk_get_rate(struct clk *clk);
> 
> And now tell me what's wrong with the above code.
> 
> > +	int eth_cs;
> > +	unsigned long cs_mem_base;
> > +	unsigned int muxed, rate;
> > +	struct clk *l3ck;
> > +
> > +	eth_cs	= H4_SMC91X_CS;
> > +
> > +	l3ck = clk_get(NULL, "core_l3_ck");
> > +	if (IS_ERR(l3ck))
> > +		rate = 100000000;
> > +	else
> > +		rate = clk_get_rate(l3ck);
> 
> Ditto.

This has been fixed with following changes:

- Use gpmc_fck instead of core_l3_ck as core_l3_ck is not the right
  clock to use (although the rates are the same as it's the parent).

- Mark gpmc_fck with ENABLE_ON_INIT so it's always on and autoidled

- Remove fixed rate setting on clk_get() error, it's not needed

Here's how it now looks:

@@ -302,15 +288,17 @@
 	int eth_cs;
 	unsigned long cs_mem_base;
 	unsigned int muxed, rate;
-	struct clk *l3ck;
+	struct clk *gpmc_fck;
 
 	eth_cs	= H4_SMC91X_CS;
 
-	l3ck = clk_get(NULL, "core_l3_ck");
-	if (IS_ERR(l3ck))
-		rate = 100000000;
-	else
-		rate = clk_get_rate(l3ck);
+	gpmc_fck = clk_get(NULL, "gpmc_fck");	/* Always on ENABLE_ON_INIT */
+	if (IS_ERR(gpmc_fck)) {
+		WARN_ON(1);
+		return;
+	}
+
+	rate = clk_get_rate(gpmc_fck);
 
 	if (is_gpmc_muxed())
 		muxed = 0x200;

Will repost the whole series.

Tony