Re: [PATCH 2/6] Blackfin I2C/TWI driver: Add platform_resource interface to support multi-port TWI controllers

[Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>]

On Fri, 14 Mar 2008 00:22:36 -0700, Bryan Wu wrote:
> From: Bryan Wu <bryan.wu-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> 
>  - 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 <bryan.wu-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Bryan Wu <cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  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!

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?

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c