On Sun, Jun 01, 2008 at 09:07:06AM +0200, Jean Delvare wrote:
> Hi Ben,
>
> On Thu, 29 May 2008 14:22:47 +0100, Ben Dooks wrote:
> > Allow the platform data to specify the bus bumber that the
> > new I2C bus will be given. This is to allow the use of the
> > board registration mechanism to specify the new style of
> > I2C device registration which allows boards to provide a
> > list of attached devices.
> >
> > Note, as discussed on the mailing list, we have dropped
> > backwards compatibility of adding an dynamic bus number
> > as it should not affect most boards to have the bus pinned
> > to 0 if they have either not specified platform data for
> > driver. Any board supplying platform data will automatically
> > have the bus_num field set to 0, and anyone who needs the
> > driver on a different bus number can supply platform data
> > to set bus_num.
> >
> > Signed-off-by: Ben Dooks <[EMAIL PROTECTED]>
> >
> > Index: linux-2.6.26-rc4-quilt2/drivers/i2c/busses/i2c-s3c2410.c
> > ===================================================================
> > --- linux-2.6.26-rc4-quilt2.orig/drivers/i2c/busses/i2c-s3c2410.c
> > 2008-05-28 12:25:20.000000000 +0100
> > +++ linux-2.6.26-rc4-quilt2/drivers/i2c/busses/i2c-s3c2410.c
> > 2008-05-28 12:27:38.000000000 +0100
> > @@ -751,9 +751,12 @@ static int s3c24xx_i2c_init(struct s3c24
> > static int s3c24xx_i2c_probe(struct platform_device *pdev)
> > {
> > struct s3c24xx_i2c *i2c = &s3c24xx_i2c;
> > + struct s3c2410_platform_i2c *pdata;
> > struct resource *res;
> > int ret;
> >
> > + pdata = s3c24xx_i2c_get_platformdata(&pdev->dev);
> > +
> > /* find the clock and enable it */
> >
> > i2c->dev = &pdev->dev;
> > @@ -831,7 +834,15 @@ static int s3c24xx_i2c_probe(struct plat
> > dev_dbg(&pdev->dev, "irq resource %p (%lu)\n", res,
> > (unsigned long)res->start);
> >
> > - ret = i2c_add_adapter(&i2c->adap);
> > + /* Note, previous versions of the driver used i2c_add_adapter()
> > + * to add an bus at any number. We now pass the bus number via
> > + * the platform data, so if unset it will now default to always
> > + * being bus 0.
> > + */
> > +
> > + i2c->adap.nr = pdata->bus_num;
> > + ret = i2c_add_numbered_adapter(&i2c->adap);
> > +
> > if (ret < 0) {
>
> Minor stylistic comment: in general you shouldn't have blank lines
> between a function call and the code which checks the values returned
> by the function in question.
Ok, will change this.
> > dev_err(&pdev->dev, "failed to add bus to i2c core\n");
> > goto err_irq;
> > Index: linux-2.6.26-rc4-quilt2/include/asm-arm/plat-s3c/iic.h
> > ===================================================================
> > --- linux-2.6.26-rc4-quilt2.orig/include/asm-arm/plat-s3c/iic.h
> > 2008-05-28 11:49:18.000000000 +0100
> > +++ linux-2.6.26-rc4-quilt2/include/asm-arm/plat-s3c/iic.h 2008-05-28
> > 12:28:00.000000000 +0100
> > @@ -21,6 +21,7 @@
> > */
> >
> > struct s3c2410_platform_i2c {
> > + int bus_num; /* bus number to use */
> > unsigned int flags;
> > unsigned int slave_addr; /* slave address for controller */
> > unsigned long bus_freq; /* standard bus frequency */
> >
>
> Other than that, looks good to me.
>
> Acked-by: Jean Delvare <[EMAIL PROTECTED]>
>
> Do you plan to send this patch (and the next ones in the series) to
> Linus yourself, or should I take them in my tree?
No, firstly these are enhancements for the next kernel, and aren't ready
for merging yet. Secondly they form a part of a larger series that needs to
go to Russell King to avoid any horrible merge problems as some of these
machines require other updates.
I was considering sending the first two patches as fixes as soon as
possible myself as a first test to see how it all works.
--
Ben
Q: What's a light-year?
A: One-third less calories than a regular year.
_______________________________________________
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
