Re: [PATCH] Driver Core patches for 2.6.9-rc1

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Thu, 2004-08-26 at 22:26, Jean Delvare wrote:

> The i2c-keywest driver doesn't define any class for any of its I2C busses. All
> hardware monitoring chips [1] do check the class, so they wont possibly probe
> any chip on the i2c-keywest busses. It happens that on the iBook2, the second
> I2C bus hosts an Analog Devices ADM1030 monitoring chip, for which a driver
> has been developped recently. Without adding the correct class bit
> (I2C_CLASS_HWMON) to the second bus of i2c-keywest, iBook2 users can't get the
> adm1031 driver to handle their ADM1030 chip.

That bus also contains other stuffs, I'd prefer the in-kernel specific
driver for this model to be used rather than some generic stuff.

> One iBook2 user came to me, wondering why he couldn't get the adm1031 driver
> to work, and we noticed the problem. I had him test a patch and it worked. I
> then sent the patch to Greg, who in turn sent it to Linus, and here we are.
> 
> Benjamin, you seem to guard the i2c-keywest driver very closely. Is there
> anything special about this driver? My patch was rather simple and
> non-intrusive, and probably not worth reverting within the hour. Much ado
> about nothing, if you want my opinion, with all due respect.

It was wrong to check on the bus number like that. i2c-keywest is used
on a variety of models to driver totally different i2c busses, and at
this point, we can't arbitrarily say "bus 1 is HW monitoring". It
totally depends on the machine model. Besides, I don't like generic
drivers playing with those thermal control stuffs, I prefer drivers that
have been tuned for those specific machine models.

> Could you please explain why my patch doesn't make sense? Similar changes were
> made to several i2c bus drivers already [2] [3], and it never caused any problem.

As I wrote earlier. Just testing the bus number and arbitrarily deciding
bus 1 is HWMON makes no sense.

> At any rate, I may redirect i2c-keywest users to you from now on, if you
> prefer to handle it yourself.
> 
> Thanks.
> 
> [1] Except lm85, but this should be fixed.
> [2] http://marc.theaimsgroup.com/?l=bk-commits-head&m=107943782219511&w=2
> [3] http://marc.theaimsgroup.com/?l=bk-commits-head&m=107943868728036&w=2
-- 
Benjamin Herrenschmidt <benh@kernel.crashing.org>