Re: [PATCH 2/3] i2c-mpc: add support for the MPC512x processors from Freescale

[Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>]

On Mon, Jan 25, 2010 at 04:15:09PM +0100, Wolfram Sang wrote:
> > >>  
> > >> -static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
> > >> -					    struct mpc_i2c *i2c,
> > >> -					    u32 clock, u32 prescaler)
> > >> +static void __devinit mpc_i2c_setup_52xx(struct device_node *node,
> > >> +					 struct mpc_i2c *i2c,
> > >> +					 u32 clock, u32 prescaler)
> > >>  {
> > >>  	int ret, fdr;
> > >>  
> > >> +	if (clock == -1) {
> > > 
> > > Could we use 0 for 'no_clock'? This would make the above statement simply
> > 
> > "0" is already used to maintain backward compatibility setting a safe
> > divider.
> 
> Ah, now I see:
> 
> 'clock == -1' means 'preserve clocks' (and is checked here in mpc_i2c_setup_52xx())
> 'clock ==  0' means 'safe divider' (and is checked in mpc_i2c_get_fdr_52xx())

hmm, sounds like a job for a  #define or similar.
 
> This is not a beauty ;)
> 
> What about adding a flags variable to the setup-functions?
> 
> Regards,
> 
>    Wolfram
> 
> -- 
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |



-- 
Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'