On Mon, Aug 09, 2010 at 01:23:45PM +0100, Alan Cox wrote:
> > I would much prefer this to be called i2c-moorsetown, we have modern
> > systems which can handle >8 character names.
>
> Moorestown. Everything else in the kernel uses 'mrst' so this would
> make the driver differ from the rest of the tree. I don't care too much
> what its called. Given its now for Medfield etc as well it probably
> should be i2c_intel_mid to match the rest of the kernel
Ok.
> > Hmm, if this is a synopsys block, then is it a standard one and if so
> > can we get some standard support?
>
> See other reply on this one.
Annoying, I'd much rather have a core driver with some bus support
added as needed.
> > > + if ((p1->flags ^ p2->flags) & I2C_M_TEN)
> > > + return 1;
> > > + return 0;
> > > +}
> >
> > would have been better to return bool.
>
> Can do but most of the kernel never uses bool so again it would be a
> quirky style. If its preferred i2c style no problem.
I thought it was introduced for this sort of thing... I've been using
it in all my new driver work.
>
> > > + for (i = 0; i < length; i++)
> > > + mrst_i2c_write(i2c->base + IC_DATA_CMD,
> > > + (uint16_t)(*(buf + i)));
> >
> > you say length in bytes, but write u16?
> > also, would be neater to have a u16 *buf?
>
> Ermm no - that would be most peculiar as you'd then only send every
> alternate byte of data. Then again the cast ought to be implied by the
> types of mrst_i2c_write - will review
This code is rather easy to mis-understand, how about making it
easier to read, ie:
(uint16_t)buf[i]
--
Ben
Q: What's a light-year?
A: One-third less calories than a regular year.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
