Re: [PATCH V3 2/4] drivers/i2c/busses/i2c-at91.c: add new driver

Russell King - ARM Linux
2011-11-08T18:55:46Z

On Tue, Nov 08, 2011 at 08:44:48PM +0200, Felipe Balbi wrote:
> hi,
>
> On Tue, Nov 08, 2011 at 06:29:55PM +0000, Russell King - ARM Linux wrote:
> > On Tue, Nov 08, 2011 at 05:23:46PM +0200, Felipe Balbi wrote:
> > > On Tue, Nov 08, 2011 at 04:15:10PM +0100, Nicolas Ferre wrote:
> > > > -----BEGIN PGP SIGNED MESSAGE-----
> > > > Hash: SHA1
> > > >
> > > > On 11/08/2011 03:41 PM, Felipe Balbi :
> > > >
> > > > >> + if (cpu_is_at91rm9200()) { /* AT91RM9200 Errata #22 */
> > > > >
> > > > > I don't think you should be using cpu_is_* on drivers.
> > > >
> > > > It is a common pattern in at91 drivers and has worked for ages.
> > > > Do you think it is related to the need to be able to compile the
> > > > driver for any SoC in the case of multi-SoC zImage support?
> > >
> > > we have drivers compiling on multiple OMAP versions without those hacks.
> > > Generally, we check the IP revision for that. Don't you have a Revision
> > > register of some sort ?
> >
> > I'm not sure what your objection is - this is no different from what
> > happens with OMAP when detecting OMAP1,2,3,4 etc. E.g.:
>
> so ? Instead of saying this to me, you should contact the
> authors/maintainers of those drivers and ask them to clean that up.

Oh for god sake, I was just asking you to clarify your statement in
light of what is currently being done.

Now, let me set something straight. I've been saying that machine_is_xxx()
should not be used in drivers. That's a platform thing and platform
specifics should not be in drivers - it should be passed in via DT or
platform data. That's enforced by the way DT works (Grant's decision
not mine) - with DT you don't have any kind of testable machine ID for
machine_is_xxx() to use.

I've never said that cpu_is_xxx() should not - that's something *other*
people are saying (and quite rightly so) because if we're going to start
sharing drivers between different SoCs (or even architectures - eg, PXA
IP appearing on x86) then it doesn't make sense for the type of SoC to
be tested. It makes more sense for the revision of the IP implementation
to be checked IFF such information is available. If not, some other way
of controlling the 'features' needs to be sought.

As far as the use of asm/*.h includes, I've NEVER made any statement
about the use of those in drivers. In fact, I don't see any reason to
avoid them _provided_ they're standard cross-arch includes.

As for mach/*.h includes, I don't think that I've made any statement
about those either, but at this point - given that we're working towards
a single zImage on ARM - it is _sensible_ to avoid such includes in
drivers.

So, I think your reaction to my statement is way off mark, and you're
attributing statements (that it seems you personally don't agree with)
to me.