[PATCH 06/10] ARM: mx28: update clocks for dual fec support

[u.kleine-koenig@pengutronix.de (Uwe Kleine-König)]

On Wed, Dec 29, 2010 at 08:57:34AM +0200, Baruch Siach wrote:
> Hi Shawn,
> 
> On Tue, Dec 28, 2010 at 10:55:51PM +0800, Shawn Guo wrote:
> > * Change fec clock registration to use con_id only for looking up,
> >   so that dual fec driver can find the same clock for both fec0
> >   and fec1.
> > 
> > * Explicitly call clk_enable for some clocks that have been on to
> >   reflect the clock status and get usecount updated. Otherwise,
> >   clocks could be turned off by pair call of clk_enable/clk_disable
> >   in drivers, because of the incorrect initial usecount.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > ---
> >  arch/arm/mach-mxs/clock-mx28.c |    7 ++++++-
> >  1 files changed, 6 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c
> > index dd6d158..6b77aff 100644
> > --- a/arch/arm/mach-mxs/clock-mx28.c
> > +++ b/arch/arm/mach-mxs/clock-mx28.c
> > @@ -603,7 +603,7 @@ _DEFINE_CLOCK(fec_clk, ENET, DISABLE, &hbus_clk);
> >  
> >  static struct clk_lookup lookups[] = {
> >  	_REGISTER_CLOCK("uart", NULL, uart_clk)
> > -	_REGISTER_CLOCK("fec.0", NULL, fec_clk)
> > +	_REGISTER_CLOCK(NULL, "fec_clk", fec_clk)
> 
> Looks like a wrong use of the clk API. You should define define two clocks 
> "fec.0" and "fec.1".
The commit log suggests that both should enable the same (hardware)
clock.  Still I agree with Baruch that in this case clk_get() should
just return the same clock for both fec.0 and fec.1.
I think it's a bit more work than just adding
	_REGISTER_CLOCK("fec.1", NULL, fec_clk)

, maybe just define fec1_clk as a dummy clk with parent=fec_clk?

(And IMHO don't inlude "_clk" in clknames, that's implicit.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |