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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
