Messages in this thread |  | | | Date | Mon, 7 Nov 2011 11:47:02 +0200 | | From | Felipe Balbi <> | | Subject | Re: [PATCH V2] drivers/i2c/busses/i2c-at91.c: fix brokeness |
| |
Hi,
On Mon, Nov 07, 2011 at 10:27:52AM +0100, Nikolaus Voss wrote: > From a05fa963f819dabf985ec0d76c769b2cf4894ccf Mon Sep 17 00:00:00 2001 > From: Nikolaus Voss <n.voss@weinmann.de> > Date: Thu, 27 Oct 2011 11:12:55 +0200 > Subject: [PATCH 1/6] drivers/i2c/busses/i2c-at91.c: fix brokeness > > This patch contains the following fixes: > 1. Support for multiple interfaces (there are usually two of them). > 2. Remove busy waiting in favour of interrupt driven io. > 3. No repeated start (Sr) was possible. This implementation supports one > repeated start condition which is enough for most real-world applications > including all SMBus transfer types. > > Tested on Atmel G45 with BQ20Z80 battery SMBus client. > > Signed-off-by: Nikolaus Voss <n.voss@weinmann.de>
IMHO, you should split this patch into three or more smaller patches. You're doing lots of different things in one commit and it'll be a pain to bisect should this cause any issues to anyone.
> --- > V2: No killed tabs, patch should apply now. > > drivers/i2c/busses/Kconfig | 11 +- > drivers/i2c/busses/i2c-at91.c | 415 +++++++++++++++++++++++++++-------------- > 2 files changed, 278 insertions(+), 148 deletions(-) > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 646068e..c4b6bdc 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -286,18 +286,15 @@ comment "I2C system bus drivers (mostly embedded / system-on-chip)" > > config I2C_AT91 > tristate "Atmel AT91 I2C Two-Wire interface (TWI)" > - depends on ARCH_AT91 && EXPERIMENTAL && BROKEN > + depends on ARCH_AT91 && EXPERIMENTAL > help > This supports the use of the I2C interface on Atmel AT91 > processors. > > - This driver is BROKEN because the controller which it uses > - will easily trigger RX overrun and TX underrun errors. Using > - low I2C clock rates may partially work around those issues > - on some systems. Another serious problem is that there is no > - documented way to issue repeated START conditions, as needed > + A serious problem is that there is no documented way to issue > + repeated START conditions for more than two messages, as needed > to support combined I2C messages. Use the i2c-gpio driver > - unless your system can cope with those limitations. > + unless your system can cope with this limitation. > > config I2C_AU1550 > tristate "Au1550/Au1200 SMBus interface" > diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c > index 305c075..a2c38ff 100644 > --- a/drivers/i2c/busses/i2c-at91.c > +++ b/drivers/i2c/busses/i2c-at91.c > @@ -1,6 +1,10 @@ > -/* > +/* -*- linux-c -*-
you shouldn't add this editor hooks to linux source files.
> @@ -279,33 +400,45 @@ static int __devexit at91_i2c_remove(struct platform_device *pdev) > > /* NOTE: could save a few mA by keeping clock off outside of at91_xfer... */ > > -static int at91_i2c_suspend(struct platform_device *pdev, pm_message_t mesg) > +static int at91_i2c_suspend(struct device *dev) > { > - clk_disable(twi_clk); > + struct platform_device *pdev = to_platform_device(dev); > + struct at91_i2c_dev *i2c_dev = platform_get_drvdata(pdev); > + > + clk_disable(i2c_dev->clk); > + > return 0; > } > > -static int at91_i2c_resume(struct platform_device *pdev) > +static int at91_i2c_resume(struct device *dev) > { > - return clk_enable(twi_clk); > + struct platform_device *pdev = to_platform_device(dev); > + struct at91_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
dev_get_drvdata(dev) will do the same of two above lines. You don't need to fetch the platform_device...
> @@ -322,6 +455,6 @@ static void __exit at91_i2c_exit(void) > module_init(at91_i2c_init); > module_exit(at91_i2c_exit); > > -MODULE_AUTHOR("Rick Bronson"); > +MODULE_AUTHOR("Nikolaus Voss");
wow... are you sure ? Rick will always be the original author, no ?
-- balbi [unhandled content-type:application/pgp-signature] |  |