diff -ura linux-2.6.4-i2c/drivers/media/common/saa7146_i2c.c linux-2.6.4/drivers/media/common/saa7146_i2c.c
--- linux-2.6.4-i2c/drivers/media/common/saa7146_i2c.c 2004-03-21 19:58:43.000000000 +0100
+++ linux-2.6.4/drivers/media/common/saa7146_i2c.c 2004-03-21 19:56:46.000000000 +0100
@@ -413,20 +413,14 @@
if( NULL != i2c_adapter ) {
memset(i2c_adapter,0,sizeof(struct i2c_adapter));
strcpy(i2c_adapter->name, dev->name);
-#if (LINUX_VERSION_CODE < KERNEL_VERSION(2,5,0))
- i2c_adapter->data = dev;
-#else
i2c_set_adapdata(i2c_adapter,dev);
-#endif
i2c_adapter->algo = &saa7146_algo;
i2c_adapter->algo_data = NULL;
i2c_adapter->id = I2C_ALGO_SAA7146;
i2c_adapter->timeout = SAA7146_I2C_TIMEOUT;
i2c_adapter->retries = SAA7146_I2C_RETRIES;
-#if (LINUX_VERSION_CODE < KERNEL_VERSION(2,5,0))
-#else
i2c_adapter->class = I2C_ADAP_CLASS_TV_ANALOG;
-#endif
+ i2c_adapter->flags = I2C_ADAP_FLAG_CLASS_MATCH;
}
return 0;
diff -ura linux-2.6.4-i2c/drivers/media/video/saa7111.c linux-2.6.4/drivers/media/video/saa7111.c
--- linux-2.6.4-i2c/drivers/media/video/saa7111.c 2004-03-21 19:59:29.000000000 +0100
+++ linux-2.6.4/drivers/media/video/saa7111.c 2004-03-21 19:59:31.000000000 +0100
@@ -591,6 +591,7 @@
.id = I2C_DRIVERID_SAA7111A,
.flags = I2C_DF_NOTIFY,
+ .class = I2C_ADAP_CLASS_TV_ANALOG,
.attach_adapter = saa7111_attach_adapter,
.detach_client = saa7111_detach_client,
diff -ura linux-2.6.4-i2c/drivers/media/video/tda9840.c linux-2.6.4/drivers/media/video/tda9840.c
--- linux-2.6.4-i2c/drivers/media/video/tda9840.c 2004-03-21 19:59:04.000000000 +0100
+++ linux-2.6.4/drivers/media/video/tda9840.c 2004-03-21 19:51:25.000000000 +0100
@@ -263,6 +263,7 @@
.name = "tda9840 driver",
.id = I2C_DRIVERID_TDA9840,
.flags = I2C_DF_NOTIFY,
+ .class = I2C_ADAP_CLASS_TV_ANALOG,
.attach_adapter = tda9840_attach,
.detach_client = tda9840_detach,
.command = tda9840_command,
diff -ura linux-2.6.4-i2c/drivers/media/video/tea6415c.c linux-2.6.4/drivers/media/video/tea6415c.c
--- linux-2.6.4-i2c/drivers/media/video/tea6415c.c 2004-03-21 19:58:57.000000000 +0100
+++ linux-2.6.4/drivers/media/video/tea6415c.c 2004-03-21 19:18:43.000000000 +0100
@@ -212,6 +212,7 @@
.name = "tea6415c driver",
.id = I2C_DRIVERID_TEA6415C,
.flags = I2C_DF_NOTIFY,
+ .class = I2C_ADAP_CLASS_TV_ANALOG,
.attach_adapter = tea6415c_attach,
.detach_client = tea6415c_detach,
.command = tea6415c_command,
diff -ura linux-2.6.4-i2c/drivers/media/video/tea6420.c linux-2.6.4/drivers/media/video/tea6420.c
--- linux-2.6.4-i2c/drivers/media/video/tea6420.c 2004-03-21 19:58:51.000000000 +0100
+++ linux-2.6.4/drivers/media/video/tea6420.c 2004-03-21 19:51:36.000000000 +0100
@@ -192,6 +192,7 @@
.name = "tea6420 driver",
.id = I2C_DRIVERID_TEA6420,
.flags = I2C_DF_NOTIFY,
+ .class = I2C_ADAP_CLASS_TV_ANALOG,
.attach_adapter = tea6420_attach,
.detach_client = tea6420_detach,
.command = tea6420_command,
diff -ura linux-2.6.4-i2c/drivers/media/video/tuner.c linux-2.6.4/drivers/media/video/tuner.c
--- linux-2.6.4-i2c/drivers/media/video/tuner.c 2004-03-21 19:59:10.000000000 +0100
+++ linux-2.6.4/drivers/media/video/tuner.c 2004-03-21 19:51:50.000000000 +0100
@@ -1180,6 +1180,7 @@
.name = "i2c TV tuner driver",
.id = I2C_DRIVERID_TUNER,
.flags = I2C_DF_NOTIFY,
+ .class = I2C_ADAP_CLASS_TV_ANALOG,
.attach_adapter = tuner_probe,
.detach_client = tuner_detach,
.command = tuner_command,
> I've implemented the idea of an additional i2c adapter flag in order to
> be able to keep i2c drivers away from certain i2c adapters.
>
> Here is what I did:
>
> First of all I was surprised that "struct i2c_adapter" already contained
> a "flags" field. It doesn't seem to be used anywhere, though -- please
> correct me if I'm wrong.
You seem to be correct. It looks like the flags member was added to all
i2c structures, but for adapters it was left unused.
> I added one flag named "I2C_ADAP_FLAG_CLASS_MATCH" which adapters can
> set if they want the type of the i2c driver match the type of the i2c
> adapter.
Why would an adapter want to accept non-matching clients once we will have
added a clean isolation method?
> Next I added a "class" member to "struct i2c_driver". I decided to use
> the same flags as for the "struct i2c_adapter" here, although the are
> all named I2C_ADAP_CLASS_* and are a bit misleading now when used for in
> the i2c driver.
The flags could be renamed if needed. There's at least one flag that we
already planed to rename (I2C_ADAP_CLASS_SMBUS, because the "SMBUS" part
is a misnomer - a more correct term would be HWMON or SENSORS).
> There are two places in i2c-core where drivers are informed of new adapters.
>
> At both places, the flags field is checked against the newly introduced
> flags I2C_ADAP_FLAG_CLASS_MATCH. If this flag is set and the adapter
> class and the driver class don't match, the driver is not probed on the bus.
My initial approach was different. I wouldn't have introduced a new
flag. Instead I would have defined a "neutral" class I2C_ADAP_CLASS_ANY
0xFFFF, suitable for chips that can belong to any bus (such as
eeprom.c). This same value could be used for busses that don't care
about which chips are connected to it (i2c-parport comes to mind). No
flag needed.
Finding whether a chip and an adapter match is as easy as checking if
((chip->class & adapter->class) != 0).
> All "old" drivers don't have this flag set, so everything works just
> like before this change.
This is where your approach looks slightly better than mine. With my
method, backward compatibility isn't provided. However, fixing existing
drivers would be rather easy, and I share Greg KH's view that drivers
living outside the kernel tree get the pain they deserve.
My main point is that if we start defining classes for both chips and
adapters and go on using them, we better consider it a policy and do it
consistently, or it will probably look confusing to newcomers.
> Now it's only possible to load these 5 i2c drivers against the MXB
> saa7146 i2c bus -- all other i2c drivers don't get the chance to probe,
> which is exactly what I wanted.
I guess you mean "against busses of class I2C_ADAP_CLASS_TV_ANALOG,
amongst which is the MXB saa7146 bus"? Or I just lost you.
> Todo list:
> - check if "flags" member in "struct i2c_adapter" is really unused --
> can any of the i2c experts commment on this?
I just tested. You can comment it out in the definition of i2c_adapter,
and everything still compiles without a complaint. I guess it means you
get a point.
> - Should the I2C_ADAP_CLASS_* be renamed to I2C_CLASS_*, so they can be
> used by both i2c adapters and i2c drivers?
I would say yes, but I'm not the authority.
--
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/
Hello Jean,
On 03/30/04 21:34, Jean Delvare wrote:
>>First of all I was surprised that "struct i2c_adapter" already contained
>>a "flags" field. It doesn't seem to be used anywhere, though -- please
>>correct me if I'm wrong.
> You seem to be correct. It looks like the flags member was added to all
> i2c structures, but for adapters it was left unused.
Ok.
>>I added one flag named "I2C_ADAP_FLAG_CLASS_MATCH" which adapters can
>>set if they want the type of the i2c driver match the type of the i2c
>>adapter.
> Why would an adapter want to accept non-matching clients once we will have
> added a clean isolation method?
My goal was to keep all non-dvb i2c clients away from my dvb i2c adapters.
Because the "probe every device on every bus" policy is currently used,
I didn't want to touch every i2c driver and break things.
>>At both places, the flags field is checked against the newly introduced
>>flags I2C_ADAP_FLAG_CLASS_MATCH. If this flag is set and the adapter
>>class and the driver class don't match, the driver is not probed on the bus.
> My initial approach was different. I wouldn't have introduced a new
> flag. Instead I would have defined a "neutral" class I2C_ADAP_CLASS_ANY
> 0xFFFF, suitable for chips that can belong to any bus (such as
> eeprom.c). This same value could be used for busses that don't care
> about which chips are connected to it (i2c-parport comes to mind). No
> flag needed.
>
> Finding whether a chip and an adapter match is as easy as checking if
> ((chip->class & adapter->class) != 0).
Yes, agreed.
>>All "old" drivers don't have this flag set, so everything works just
>>like before this change.
> This is where your approach looks slightly better than mine. With my
> method, backward compatibility isn't provided. However, fixing existing
> drivers would be rather easy, and I share Greg KH's view that drivers
> living outside the kernel tree get the pain they deserve.
8-) If this is the way to go, I totally agree, even if that means that
some drivers will fail to work in the beginning.
> My main point is that if we start defining classes for both chips and
> adapters and go on using them, we better consider it a policy and do it
> consistently, or it will probably look confusing to newcomers.
Sure.
>>Now it's only possible to load these 5 i2c drivers against the MXB
>>saa7146 i2c bus -- all other i2c drivers don't get the chance to probe,
>>which is exactly what I wanted.
> I guess you mean "against busses of class I2C_ADAP_CLASS_TV_ANALOG,
> amongst which is the MXB saa7146 bus"? Or I just lost you.
Yes, sorry. If other drivers are in the I2C_ADAP_CLASS_TV_ANALOG class,
they will be probed, too.
Hm, perhaps we should add another method where an adapter can specify
the I2C_DRIVERID_*s from the clients it's expecting. The rationale
behind this is, that most PCI cards can be identified uniquely by their
vendor/subvendor ids, so the driver author definately knows which i2c
clients are on the card and what are expected to be present. No need to
probe every i2c client.
What do you think?
>>- Should the I2C_ADAP_CLASS_* be renamed to I2C_CLASS_*, so they can be
>>used by both i2c adapters and i2c drivers?
> I would say yes, but I'm not the authority.
If we agree that this change should be done, this sound reasonable to me.
CU
Michael.
> > This is where your approach looks slightly better than mine. With my
> > method, backward compatibility isn't provided. However, fixing
> > existing drivers would be rather easy, and I share Greg KH's view
> > that drivers living outside the kernel tree get the pain they
> > deserve.
>
> 8-) If this is the way to go, I totally agree, even if that means that
> some drivers will fail to work in the beginning.
As the movie goes: "A beginning is a very delicate time."
> Hm, perhaps we should add another method where an adapter can specify
> the I2C_DRIVERID_*s from the clients it's expecting. The rationale
> behind this is, that most PCI cards can be identified uniquely by
> their vendor/subvendor ids, so the driver author definately knows
> which i2c clients are on the card and what are expected to be present.
> No need to probe every i2c client.
>
> What do you think?
I'm a bit surprised because I thought such a function already existed.
After a quick glance I couldn't find it though. This would tend to prove
that the chip driver IDs were never used anywhere (since this is the
only use I can think of for them).
I think I remember Greg was even thinking of getting rid of them.
I'm not sure that the function you propose would be really useful. I
guess that most people don't load i2c chip drivers they don't need. The
class filter you propose, added to the different I2C addresses, should
do the rest.
On the hardware monitoring side, we usually have a detection function in
all chip drivers, which has the responsability to make sure that the
chip is actually one which the driver is supposed to support. I admit
it's not necessarily easy since there is no official ID such as for the
PCI cards. But we do it and in most cases it's efficient. Maybe you
don't have such a mechanism for the video i2c chip drivers? This would
explain why you feel the need of such a function when I do not.
I don't really see where/how such a function be, but if you want to take
a try and propose a patch, I'll take a look.
That said, it seems to be something different from the class bitfields
we were discussing so far, so it would be better to first go on with the
classes, and see the ids later.
Greg, any comment? Is the approach I suggested OK with you, or do you
prefer Michael's one (with the additional flag)?
Thanks.
--
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/
Hello,
On 03.04.2004 16:30, Jean Delvare wrote:
> Michael Hunold wrote:
>>Hm, perhaps we should add another method where an adapter can specify
>>the I2C_DRIVERID_*s from the clients it's expecting. The rationale
>>behind this is, that most PCI cards can be identified uniquely by
>>their vendor/subvendor ids, so the driver author definately knows
>>which i2c clients are on the card and what are expected to be present.
>>No need to probe every i2c client.
>>
>>What do you think?
> I'm a bit surprised because I thought such a function already existed.
> After a quick glance I couldn't find it though. This would tend to prove
> that the chip driver IDs were never used anywhere (since this is the
> only use I can think of for them).
>
> I think I remember Greg was even thinking of getting rid of them.
Ok.
> I'm not sure that the function you propose would be really useful. I
> guess that most people don't load i2c chip drivers they don't need. The
> class filter you propose, added to the different I2C addresses, should
> do the rest.
>
> On the hardware monitoring side, we usually have a detection function in
> all chip drivers, which has the responsability to make sure that the
> chip is actually one which the driver is supposed to support. I admit
> it's not necessarily easy since there is no official ID such as for the
> PCI cards. But we do it and in most cases it's efficient. Maybe you
> don't have such a mechanism for the video i2c chip drivers? This would
> explain why you feel the need of such a function when I do not.
Well, the problem is that the check is on the wrong side to solve my
problem: it's ok that the driver checks if it's on the adapter it
expects. But I'd like to have a feature that an adapter can reject a
driver if the adapter knows that the chipset isn't on the bus for sure
(for example when the adapter knows the exact type of card via pci ids
and the i2c clients are well known)
As I already have explained, there are DVB cards that have a very
delicate i2c bus. The number of i2c tuners is still increasing and all
these tuners have I2C_ADAP_CLASS_TV_DIGITAL set, so they will be probed
even on the delicate i2c busses, even if I know for sure that they are
not on this particular bus for sure.
Hm, this is running in a completly different direction now. 8-)
> I don't really see where/how such a function be, but if you want to take
> a try and propose a patch, I'll take a look.
>
> That said, it seems to be something different from the class bitfields
> we were discussing so far, so it would be better to first go on with the
> classes, and see the ids later.
Yes, agreed. Let's get the first thing straight before.
> Greg, any comment? Is the approach I suggested OK with you, or do you
> prefer Michael's one (with the additional flag)?
I have though about this and grepped through the kernel a bit. I now
have the opinion that we should fix all adapters and drivers in the
kernel to keep a consistent scheme without the akward flag is proposed.
Here are some statistics:
Ok, adapters which specify I2C_ADAP_CLASS_TV_ANALOG
./drivers/media/video/saa7134/saa7134-i2c.c
./drivers/media/video/bttv-i2c.c
Ok, adapters which specify I2C_ADAP_CLASS_TV_ANALOG (these are my
drivers and I plan to patch them accordingly)
./drivers/media/video/hexium_orion.c
./drivers/media/video/mxb.c
./drivers/media/video/hexium_gemini.c
./drivers/media/video/dpc7146.c
Ok, adapters which specify I2C_ADAP_CLASS_CAM_DIGITAL,
./drivers/usb/media/w9968cf.c
Ok, adapters which specify I2C_ADAP_CLASS_SMBUS,
./drivers/i2c/busses/i2c-nforce2.c
./drivers/i2c/busses/i2c-sis630.c
./drivers/i2c/busses/i2c-piix4.c
./drivers/i2c/busses/i2c-sis96x.c
./drivers/i2c/busses/i2c-i801.c
./drivers/i2c/busses/i2c-ali15x3.c
./drivers/i2c/busses/i2c-isa.c
./drivers/i2c/busses/i2c-viapro.c
./drivers/i2c/busses/i2c-amd8111.c
./drivers/i2c/busses/i2c-amd756.c
./drivers/i2c/busses/i2c-parport-light.c
./drivers/i2c/busses/i2c-parport.c
The following drivers implement adapters and don't have a type
specified. (6)
./drivers/i2c/busses/i2c-ali1535.c
./drivers/i2c/busses/i2c-iop3xx.c
./drivers/i2c/busses/i2c-sis5595.c
./drivers/i2c/busses/scx200_acb.c
./drivers/i2c/busses/i2c-keywest.c
./drivers/i2c/busses/i2c-ibm_iic.c
Are all of these I2C_ADAP_CLASS_SMBUS?
Ok, now come some special cases, where busses are registered through a
helper function and don't have a type set. (my guesses are in paranthesis)
=> i2c_bit_add_bus() users with no class entry (16)
./drivers/i2c/busses/i2c-frodo.c
./drivers/i2c/busses/i2c-philips-par.c
./drivers/i2c/busses/i2c-prosavage.c
./drivers/i2c/busses/scx200_i2c.c
./drivers/i2c/busses/i2c-savage4.c
./drivers/i2c/busses/i2c-via.c
./drivers/i2c/busses/i2c-elv.c
./drivers/i2c/busses/i2c-velleman.c
./drivers/i2c/busses/i2c-hydra.c
./drivers/video/aty/radeon_i2c.c
./drivers/ieee1394/pcilynx.c
./drivers/acorn/char/i2c.c
./drivers/i2c/busses/i2c-i810.c (I2C_CLASS_DDC|I2C_CLASS_TV_ANALOG?)
./drivers/i2c/busses/i2c-voodoo3.c (I2C_CLASS_DDC|I2C_CLASS_TV_ANALOG?)
./drivers/video/matrox/i2c-matroxfb.c (I2C_CLASS_DDC?)
./drivers/media/video/zoran_card.c (I2C_CLASS_TV_ANALOG?)
=> i2c_pcf_add_bus() users with no class entry
./drivers/i2c/busses/i2c-elektor.c (I2C_CLASS_ALL?)
=> i2c_iic_add_bus() users with no class entry
./drivers/i2c/busses/i2c-ite.c (I2C_CLASS_ALL?)
Now to the other side, the client drivers. As we have discussed, they
will all need to define a class type. My proposals are above each section:
I2C_CLASS_SMBUS
./drivers/i2c/chips/w83781d.c
./drivers/i2c/chips/lm75.c
./drivers/i2c/chips/adm1021.c
./drivers/i2c/chips/via686a.c
./drivers/i2c/chips/lm85.c
./drivers/i2c/chips/eeprom.c
./drivers/i2c/chips/it87.c
./drivers/i2c/chips/lm78.c
./drivers/i2c/chips/lm83.c
./drivers/i2c/chips/asb100.c
./drivers/i2c/chips/lm90.c
./drivers/i2c/chips/w83l785ts.c
./drivers/i2c/chips/fscher.c
./drivers/i2c/chips/gl518sm.c
I2C_CLASS_TV_ANALOG
./drivers/media/video/tea6420.c
./drivers/media/video/tea6415c.c
./drivers/media/video/tda9840.c
./drivers/media/video/tuner.c
./drivers/media/video/saa5249.c
./drivers/media/video/saa5246a.c
./drivers/media/video/saa7110.c
./drivers/media/video/tda9887.c
./drivers/media/video/saa7134/saa6752hs.c
./drivers/media/video/bt856.c
./drivers/media/video/saa7185.c
./drivers/media/video/bt819.c
./drivers/media/video/saa7111.c
./drivers/media/video/tuner-3036.c
./drivers/media/video/tda9875.c
./drivers/media/video/vpx3220.c
./drivers/media/video/msp3400.c
./drivers/media/video/tda7432.c
./drivers/media/video/bt832.c
./drivers/media/video/saa7114.c
./drivers/media/video/tvmixer.c
./drivers/media/video/adv7175.c
./drivers/media/video/adv7170.c
./drivers/media/video/tvaudio.c
I2C_CLASS_TV_DIGITAL
./drivers/media/dvb/bt8xx/dvb-bt8xx.c
I2C_CLASS_DDC
./drivers/video/matrox/matroxfb_maven.c
No idea about these. Do we need some new classes for them?
./drivers/i2c/i2c-dev.c
./drivers/macintosh/therm_adt7467.c
./drivers/macintosh/therm_pm72.c
./drivers/macintosh/therm_windtunnel.c
./drivers/acorn/char/pcf8583.c
./sound/oss/dmasound/dac3550a.c
./sound/oss/dmasound/tas_common.c
./sound/ppc/keywest.c
./drivers/media/video/ir-kbd-i2c.c
> Thanks.
So, to get the whole stuff consistent through the kernel, 24 adapter
drivers and 49 client drivers need to be touched.
CU
Michael.
> Here are some statistics:
>
> Ok, adapters which specify I2C_ADAP_CLASS_TV_ANALOG
> ./drivers/media/video/saa7134/saa7134-i2c.c
> ./drivers/media/video/bttv-i2c.c
>
> Ok, adapters which specify I2C_ADAP_CLASS_TV_ANALOG (these are my
> drivers and I plan to patch them accordingly)
> ./drivers/media/video/hexium_orion.c
> ./drivers/media/video/mxb.c
> ./drivers/media/video/hexium_gemini.c
> ./drivers/media/video/dpc7146.c
I'm a bit confused, it's I2C_ADAP_CLASS_TV_ANALOG both times. What's the
difference, except that the second are yours?
>
> Ok, adapters which specify I2C_ADAP_CLASS_CAM_DIGITAL,
> ./drivers/usb/media/w9968cf.c
>
> Ok, adapters which specify I2C_ADAP_CLASS_SMBUS,
> ./drivers/i2c/busses/i2c-nforce2.c
> ./drivers/i2c/busses/i2c-sis630.c
> ./drivers/i2c/busses/i2c-piix4.c
> ./drivers/i2c/busses/i2c-sis96x.c
> ./drivers/i2c/busses/i2c-i801.c
> ./drivers/i2c/busses/i2c-ali15x3.c
> ./drivers/i2c/busses/i2c-isa.c
> ./drivers/i2c/busses/i2c-viapro.c
> ./drivers/i2c/busses/i2c-amd8111.c
> ./drivers/i2c/busses/i2c-amd756.c
> ./drivers/i2c/busses/i2c-parport-light.c
> ./drivers/i2c/busses/i2c-parport.c
You missed i2c-ali1535.c, i2c-sis5595.c and i2c-via.c. I think that you
used an old 2.6 trer for your statistics :(
> The following drivers implement adapters and don't have a type
> specified. (6)
>
> ./drivers/i2c/busses/i2c-ali1535.c
> ./drivers/i2c/busses/i2c-iop3xx.c
> ./drivers/i2c/busses/i2c-sis5595.c
> ./drivers/i2c/busses/scx200_acb.c
> ./drivers/i2c/busses/i2c-keywest.c
> ./drivers/i2c/busses/i2c-ibm_iic.c
> Are all of these I2C_ADAP_CLASS_SMBUS?
i2c-ali1535.c and i2c-sis5595.c are, and are already fixed. I can't tell
for the others.
> Ok, now come some special cases, where busses are registered through
> a helper function and don't have a type set. (my guesses are in
> paranthesis)
>
> => i2c_bit_add_bus() users with no class entry (16)
> ./drivers/i2c/busses/i2c-frodo.c
> ./drivers/i2c/busses/i2c-philips-par.c
> ./drivers/i2c/busses/i2c-prosavage.c
> ./drivers/i2c/busses/scx200_i2c.c
> ./drivers/i2c/busses/i2c-savage4.c
> ./drivers/i2c/busses/i2c-via.c
> ./drivers/i2c/busses/i2c-elv.c
> ./drivers/i2c/busses/i2c-velleman.c
> ./drivers/i2c/busses/i2c-hydra.c
> ./drivers/video/aty/radeon_i2c.c
> ./drivers/ieee1394/pcilynx.c
> ./drivers/acorn/char/i2c.c
> ./drivers/i2c/busses/i2c-i810.c (I2C_CLASS_DDC|I2C_CLASS_TV_ANALOG?)
> ./drivers/i2c/busses/i2c-voodoo3.c (I2C_CLASS_DDC|I2C_CLASS_TV_ANALOG?)
> ./drivers/video/matrox/i2c-matroxfb.c (I2C_CLASS_DDC?)
> ./drivers/media/video/zoran_card.c (I2C_CLASS_TV_ANALOG?)
i2c-philips-par.c, i2c-elv.c and i2c-velleman.c are gone so you can
forget about them.
i2c-prosavage.c, i2c-savage4.c and radeon_i2c.c are video adapter
drivers, like i2c-i810.c, i2c-voodoo3.c and i2c-matroxfb.c. Most (if not
all) of them have several busses, typically one for DDC and one for
video chips, sometimes more. I don't expect us to have to OR classes in
most cases, just give the correct class to each bus. This is already
done for i2c-voodoo3.c, BTW.
> => i2c_pcf_add_bus() users with no class entry
> ./drivers/i2c/busses/i2c-elektor.c (I2C_CLASS_ALL?)
>
> => i2c_iic_add_bus() users with no class entry
> ./drivers/i2c/busses/i2c-ite.c (I2C_CLASS_ALL?)
>
> Now to the other side, the client drivers. As we have discussed, they
> will all need to define a class type. My proposals are above each
> section:
>
> I2C_CLASS_SMBUS
> ./drivers/i2c/chips/w83781d.c
> ./drivers/i2c/chips/lm75.c
> ./drivers/i2c/chips/adm1021.c
> ./drivers/i2c/chips/via686a.c
> ./drivers/i2c/chips/lm85.c
> ./drivers/i2c/chips/eeprom.c
> ./drivers/i2c/chips/it87.c
> ./drivers/i2c/chips/lm78.c
> ./drivers/i2c/chips/lm83.c
> ./drivers/i2c/chips/asb100.c
> ./drivers/i2c/chips/lm90.c
> ./drivers/i2c/chips/w83l785ts.c
> ./drivers/i2c/chips/fscher.c
> ./drivers/i2c/chips/gl518sm.c
They all do already, except lm85.c.
> I2C_CLASS_TV_ANALOG
> ./drivers/media/video/tea6420.c
> ./drivers/media/video/tea6415c.c
> ./drivers/media/video/tda9840.c
> ./drivers/media/video/tuner.c
> ./drivers/media/video/saa5249.c
> ./drivers/media/video/saa5246a.c
> ./drivers/media/video/saa7110.c
> ./drivers/media/video/tda9887.c
> ./drivers/media/video/saa7134/saa6752hs.c
> ./drivers/media/video/bt856.c
> ./drivers/media/video/saa7185.c
> ./drivers/media/video/bt819.c
> ./drivers/media/video/saa7111.c
> ./drivers/media/video/tuner-3036.c
> ./drivers/media/video/tda9875.c
> ./drivers/media/video/vpx3220.c
> ./drivers/media/video/msp3400.c
> ./drivers/media/video/tda7432.c
> ./drivers/media/video/bt832.c
> ./drivers/media/video/saa7114.c
> ./drivers/media/video/tvmixer.c
> ./drivers/media/video/adv7175.c
> ./drivers/media/video/adv7170.c
> ./drivers/media/video/tvaudio.c
>
> I2C_CLASS_TV_DIGITAL
> ./drivers/media/dvb/bt8xx/dvb-bt8xx.c
>
> I2C_CLASS_DDC
> ./drivers/video/matrox/matroxfb_maven.c
Hu, I think this one is a bus, not a chip.
BTW, there's only one chip driver that would have class I2C_CLASS_DDC,
this is ddcmon.c and it hasn't been ported to 2.6 yet.
>
> No idea about these. Do we need some new classes for them?
> ./drivers/i2c/i2c-dev.c
This one is a generic driver that allows bus manipulation from
user-space. I2C_CLASS_ALL, definitely.
> ./drivers/macintosh/therm_adt7467.c
> ./drivers/macintosh/therm_pm72.c
> ./drivers/macintosh/therm_windtunnel.c
> ./drivers/acorn/char/pcf8583.c
> ./sound/oss/dmasound/dac3550a.c
> ./sound/oss/dmasound/tas_common.c
> ./sound/ppc/keywest.c
> ./drivers/media/video/ir-kbd-i2c.c
No idea either. All I can say is that some busses and chips concern
sound, so we will have to create a new class (I2C_CLASS_SOUND or
something similar).
> So, to get the whole stuff consistent through the kernel, 24 adapter
> drivers and 49 client drivers need to be touched.
Sounds feasable. Now the question is: what will we do with busses and
chips we don't know? Two possibilities:
1* Don't set the class. This prevents the driver from being used, so we
can expect people to complain quite quickly, letting us fix the drivers
with the correct class.
2* Use I2C_CLASS_ALL. That way they keep working and people will not
complain. But most drivers will be too permissive, which is against the
plan.
Frankly I don't know.
--
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/
Hello Jean,
>>Here are some statistics:
>>
>>Ok, adapters which specify I2C_ADAP_CLASS_TV_ANALOG
>>./drivers/media/video/saa7134/saa7134-i2c.c
>>./drivers/media/video/bttv-i2c.c
>>
>>Ok, adapters which specify I2C_ADAP_CLASS_TV_ANALOG (these are my
>>drivers and I plan to patch them accordingly)
>>./drivers/media/video/hexium_orion.c
>>./drivers/media/video/mxb.c
>>./drivers/media/video/hexium_gemini.c
>>./drivers/media/video/dpc7146.c
> I'm a bit confused, it's I2C_ADAP_CLASS_TV_ANALOG both times. What's the
> difference, except that the second are yours?
I meant to say that the first ones already define
I2C_ADAP_CLASS_TV_ANALOG correctly, but the latter ones not. But because
of the fact that these are my drivers, there won't be any problems
adding the class type and verifying the correctness afterwards.
>>Ok, adapters which specify I2C_ADAP_CLASS_SMBUS,
>>./drivers/i2c/busses/i2c-nforce2.c
>>./drivers/i2c/busses/i2c-sis630.c
>>./drivers/i2c/busses/i2c-piix4.c
>>./drivers/i2c/busses/i2c-sis96x.c
>>./drivers/i2c/busses/i2c-i801.c
>>./drivers/i2c/busses/i2c-ali15x3.c
>>./drivers/i2c/busses/i2c-isa.c
>>./drivers/i2c/busses/i2c-viapro.c
>>./drivers/i2c/busses/i2c-amd8111.c
>>./drivers/i2c/busses/i2c-amd756.c
>>./drivers/i2c/busses/i2c-parport-light.c
>>./drivers/i2c/busses/i2c-parport.c
>
>
> You missed i2c-ali1535.c, i2c-sis5595.c and i2c-via.c. I think that you
> used an old 2.6 trer for your statistics :(
Hm, if you consider 2.6.4 old, then yes. Please remember that I'm only a
i2c core system user, so I simply took the most recent 2.6 version which
was available.
> i2c-prosavage.c, i2c-savage4.c and radeon_i2c.c are video adapter
> drivers, like i2c-i810.c, i2c-voodoo3.c and i2c-matroxfb.c. Most (if not
> all) of them have several busses, typically one for DDC and one for
> video chips, sometimes more. I don't expect us to have to OR classes in
> most cases, just give the correct class to each bus. This is already
> done for i2c-voodoo3.c, BTW.
Ah, ok, there were double entries when I grepped through the code, so
the drivers defined different i2c busses then.
>>./drivers/macintosh/therm_adt7467.c
>>./drivers/macintosh/therm_pm72.c
>>./drivers/macintosh/therm_windtunnel.c
>>./drivers/acorn/char/pcf8583.c
>>./sound/oss/dmasound/dac3550a.c
>>./sound/oss/dmasound/tas_common.c
>>./sound/ppc/keywest.c
>>./drivers/media/video/ir-kbd-i2c.c
>
>
> No idea either. All I can say is that some busses and chips concern
> sound, so we will have to create a new class (I2C_CLASS_SOUND or
> something similar).
Hm, ok. Because we don't know for sure, we should probably do what I
propose below.
> Sounds feasable. Now the question is: what will we do with busses and
> chips we don't know? Two possibilities:
>
> 1* Don't set the class. This prevents the driver from being used, so we
> can expect people to complain quite quickly, letting us fix the drivers
> with the correct class.
>
> 2* Use I2C_CLASS_ALL. That way they keep working and people will not
> complain. But most drivers will be too permissive, which is against the
> plan.
I like 1 best, but this would be 2.7 stuff. We cannot break working
configurations for 2.6.
So I tend to use I2C_CLASS_ALL for all drivers we definately not know
and add a big fat #warning preprocessor message, with an url explaining
the background (i.e. "this driver doesn't have the correct i2c class
set. if you are interested in having this driver fixed, have a look at
http://foo for further informations."
> Frankly I don't know.
If we have agreed on one model, we need to create a big patch and get it
into the kernel.
I think it would be best if Greg could get it in with his patchsets.
Greg, is this ok for you or do you have any other proposals?
CU
Michael.
On Sat, 2004-04-03 at 15:30, Jean Delvare wrote:
> I'm not sure that the function you propose would be really useful. I
> guess that most people don't load i2c chip drivers they don't need. The
> class filter you propose, added to the different I2C addresses, should
> do the rest.
What about using two DVB cards of different models to record off one
multiplex while watching another?
Only an explicit list of which chips should be probed on each I2C bus is
safe for this sort of system.
- Adrian Cox
Adrian Cox wrote:
>On Sat, 2004-04-03 at 15:30, Jean Delvare wrote:
>
>
>
>>I'm not sure that the function you propose would be really useful. I
>>guess that most people don't load i2c chip drivers they don't need. The
>>class filter you propose, added to the different I2C addresses, should
>>do the rest.
>>
>>
>
>What about using two DVB cards of different models to record off one
>multiplex while watching another?
>
>Only an explicit list of which chips should be probed on each I2C bus is
>safe for this sort of system.
>
>
>
You might be able to use the ignore= and force= parameters to the i2c
drivers to accomplish this. With ignore you can prevent a driver from
scanning an address (specify -1 as the bus to ignore that address on all
busses) and with force you can specify which bus/address to find the
specific chip.
I haven't read the code in detail yet, but if force doesn't already
override ignore, it probably should. I can't think of reason why you
would want ignore to override a force directive.
:v)