[i2c] [PATCH 8/12] drivers: PMC MSP71xx TWI driver

[Jean Delvare]

On Fri, 29 Jun 2007 13:26:18 -0700, Marc St-Jean wrote:
> >  > diff --git a/drivers/i2c/busses/i2c-pmcmsp.c b/drivers/i2c/busses/i2c-pmcmsp.c
> >  > new file mode 100644
> >  > index 0000000..68018df
> >  > --- /dev/null
> >  > +++ b/drivers/i2c/busses/i2c-pmcmsp.c
> >  > (...)
> >  > +/* IO Operation macros */
> >  > +#define pmcmsptwi_readl              __raw_readl
> >  > +#define pmcmsptwi_writel     __raw_writel
> > 
> > Where are these defined?
> 
> These are defined in include/asm-mips/io.h macros.

Then you should #include <asm/io.h>.

> >  > +
> >  > +     if (msg->len == 0) {
> >  > +             if (msg->flags & I2C_M_RD) {
> >  > +                     dev_dbg(&adap->dev,
> >  > +                             "Read of 0 bytes unsupported\n");
> >  > +                     return -EINVAL;
> >  > +             } else {
> >  > +                     dev_dbg(&adap->dev, "Probing for slave at 0x%02x\n",
> >  > +                             msg->addr & 0xff);
> >  > +                     probe = true;
> >  > +
> >  > +                     /* Probe is a special write of 1 byte */
> >  > +                     cmd.write_len = 1;
> >  > +                     cmd.write_data = probe_buf;
> >  > +             }
> >  > +     }
> > 
> > This should go away. As explained in my other post, new-style i2c
> > drivers don't need probing (and if you really want probing, it no
> > longer requires zero-byte messages.)
> 
> I understand but is that not dependent on a the chip drivers?

It is, to some extent.

> We can't assume our customers only use new style drivers to get
> at devices on their boards.

Old-style drivers which were using the standard I2C_CLIENT_INSMOD*
macros can be loaded with a force parameter which will bypass the
probe, so they can work even without zero-byte transaction support.

The point is that your driver will not be accepted upstream as long as
it includes this hack. If you really want to keep it, you'll have to
maintain it as a separate patch for your customers. But I'd rather add
missing I2C_CLIENT_INSMOD* calls to old-style drivers which lack them.

> > You'll have to change this. I2C_FUNC_SMBUS_EMUL includes
> > I2C_FUNC_SMBUS_QUICK and it's important that you don't pretend to
> > support that when you don't.
> 
> OK, changed to:
> 	return I2C_FUNC_I2C | | I2C_FUNC_10BIT_ADDR |
> 		I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_BYTE_DATA |
> 		I2C_FUNC_SMBUS_WORD_DATA | I2C_FUNC_SMBUS_PROC_CALLL;

Looks OK, assuming that the doubled "|" and the tripled "L" are
unintended ;)

-- 
Jean Delvare