Re: [PATCH 1/2] OMAP2/3: hwmod: fix the i2c-reset timeout during bootup

["Avinash.H.M." <avinashhm@ti.com>]

Hi Paul ,

Thanks for the review. 

> > +/* In register I2C_CON, Bit 15 is the I2C enable bit */
> > +#define I2C_EN				BIT(15)
> > +#define I2C_CON_OFFSET			0x24
> 
> This stuff, along with omap_i2c_reset(), doesn't belong in omap_hwmod.c, 
> which is common code that is not I2C-specific.  Please put it in 
> mach-omap2/i2c.c instead.

I agree. I ll move these I2C specific things from omap_hwmod.c to
mach-omap2/i2c.c.


[...]


> > + */
> > +int omap_i2c_reset(struct omap_hwmod *oh)
> > +{
> > +	u32 v = 0;
> 
> no need to initialize this to 0
> 
> > +	int ret = 0, c = 0;
> 
> no need to initialize ret to 0

OK . I ll remove initialization of 'v'.

Since now i am moving the function to i2c.c, i can't call _set_softreset
and _write_sysconfig(static functions in omap_hwmod.c). Instead, i ll
set the SOFTRESET bit and write into SYSC register directly. So i do not
need 'ret'. I ll remove it.

Just curios. I understand there is no use of initializing 'v' to 0
here. But by programming practice, i usually initialize local variables
to '0'. Is there anything wrong in doing this ? Any negative impact ?


[...]


> >  int omap_hwmod_register(struct omap_hwmod **ohs);
> > +int omap_i2c_reset(struct omap_hwmod *oh);
> 
> This should go into plat-omap/include/plat/i2c.h

Sure. I will move this change to plat/i2c.h. 

I ll send out the v2 of this shortly after testing.

thanks ,

- avinash

> 
> > +
> >  struct omap_hwmod *omap_hwmod_lookup(const char *name);
> >  int omap_hwmod_for_each(int (*fn)(struct omap_hwmod *oh, void *data),
> >  			void *data);
> > -- 
> > 1.7.1
> > 
> 
> 
> - Paul