On Tue, Mar 25, 2008 at 3:49 AM, Jean Delvare <[EMAIL PROTECTED]> wrote:
> On Fri, 14 Mar 2008 00:22:36 -0700, Bryan Wu wrote:
>  > From: Bryan Wu <[EMAIL PROTECTED]>
>  >
>  >  - Dynamic alloc the resource of TWI driver data according to board 
> information
>  >  - TWI register read/write accessor based on dynamic regs_base
>  >  - Support TWI0/TWI1 for BF54x
>  >
>  > Signed-off-by: Bryan Wu <[EMAIL PROTECTED]>
>  > Signed-off-by: Bryan Wu <[EMAIL PROTECTED]>
>  > ---
>  >  drivers/i2c/busses/i2c-bfin-twi.c |  281 
> +++++++++++++++++++++++--------------
>  >  1 files changed, 174 insertions(+), 107 deletions(-)
>
>  Looks good, except:
>
>
>  > diff --git a/drivers/i2c/busses/i2c-bfin-twi.c 
> b/drivers/i2c/busses/i2c-bfin-twi.c
>  > index ea15125..515fe08 100644
>  > --- a/drivers/i2c/busses/i2c-bfin-twi.c
>  > +++ b/drivers/i2c/busses/i2c-bfin-twi.c
>  > (...)
>
> > @@ -593,39 +646,55 @@ static int i2c_bfin_twi_probe(struct platform_device 
> > *dev)
>  > (...)
>
> > +     return 0;
>  > +
>  > +out_error_add_adapter:
>  > +     free_irq(iface->irq, iface);
>  > +out_error_req_irq:
>  > +out_error_no_irq:
>  > +     iounmap(iface->regs_base);
>  > +out_error_ioremap:
>  > +out_error_get_res:
>  > +     kfree(iface);
>  > +out_error_nomem:
>  >       return rc;
>  >  }
>
>  You forgot to add the same steps when removing the device in
>  i2c_bfin_twi_remove(): you must call iounmap() and kfree() there too!
>

Exactly, I will add them.

>  Also, it looks to me like this patch makes many bfin_read_TWI_* and
>  bfin_write_TWI_* macros in include/asm-blackfin/mach-bf537/cdefBF534.h
>  and include/asm-blackfin/mach-bf527/cdefBF52x_base.h unneeded, so it
>  would probably be a good idea to delete them now to avoid any confusion?
>

This bfin_write_TWI_* macros are generated by script automatically. As
we moved to use low level bfin_write/bfin_read
, those TWI macros are useless. I will remove them in other patch.

Thanks a lot
-Bryan

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Reply via email to