From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Bryan Wu" Subject: Re: [PATCH 2/6] Blackfin I2C/TWI driver: Add platform_resource interface to support multi-port TWI controllers Date: Tue, 25 Mar 2008 11:16:45 +0800 Message-ID: <386072610803242016h2aa2bf79q1483456495e5da6f@mail.gmail.com> References: <1205479360-25240-3-git-send-email-cooloney@kernel.org> <20080324204938.31350213@hyperion.delvare> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20080324204938.31350213-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org> Content-Disposition: inline List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: i2c-bounces-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org Errors-To: i2c-bounces-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org To: Jean Delvare Cc: i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org List-Id: linux-i2c@vger.kernel.org On Tue, Mar 25, 2008 at 3:49 AM, Jean Delvare wrote: > On Fri, 14 Mar 2008 00:22:36 -0700, Bryan Wu wrote: > > From: Bryan Wu > > > > - 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 > > Signed-off-by: Bryan Wu > > --- > > 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 i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org http://lists.lm-sensors.org/mailman/listinfo/i2c