i2c: don't autograb i2c-pca-isa
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): 489C8015.8070804-cENuUygGYd//[email protected]
Hi Rene,
Please Cc the i2c list for i2c patches (added.)
Quoted 4 lines · Rene Herman
Grabbing ISA bus resources without anything or anyone telling us we should can break boot on randconfig/allyesconfig builds by keeping resources that are in fact owned by different hardware busy and does as reported by Ingo Molnar.
I don't think it makes much sense to boot randomconfig kernels. The i2c-pca-isa driver is for rare hardware, most people will not use it and certainly won't build it into the kernel. So I don't think there really is a problem in practice here. That being said...
Quoted 6 lines
Generally it's also dangerous to just poke at random I/O ports and especially those in the range where other old easily confused ISA hardware might live.
For this specialized I2C bus driver, insist that the user specifies the resources before grabbing them.
I agree that such legacy drivers should not assume default I/O port and IRQ, that's dangerous.
Quoted 6 lines
The^WA user of this driver is a one time
echo "options i2c-pca-isa base=0x330 irq=10" >>
/etc/modprobe.confaway from the old behaviour.
I am curious how many users of this driver are left. Maybe it's time to get rid of it.
Quoted 21 lines
Signed-off-by Rene Herman <[email protected]>--- drivers/i2c/busses/i2c-pca-isa.c | 20 +++++++++++++++++--- 1 files changed, 17 insertions(+), 3 deletions(-)drivers/i2c/busses/i2c-pca-isa.c +2
-2@@ -36,8 +36,8 @@#define DRIVER "i2c-pca-isa"#define IO_SIZE 4 -static unsigned long base =0x330 ; -static int irq =10 ; +static unsigned long base; +static int irq =-1 ;/* Data sheet recommends 59kHz for 100kHz operation due to variation * in the actual clock rate */
drivers/i2c/busses/i2c-pca-isa.c static struct i2c_adapter pca_isa_ops = { · line 116
Quoted 10 lines
drivers/i2c/busses/i2c-pca-isa.c +7
-0@@ -107,6 +107,19 @@ static struct i2c_adapter pca_isa_ops = { .timeout = 100, }; +static int __devinit pca_isa_match (struct device *dev,unsigned int id) +{ +int match = base !=0 ; + +if (match) { +if (irq ==-1 ) +dev_warn (dev,"using poling mode (specify irq)\n" );
Spelling: polling.
Quoted 19 lines
drivers/i2c/busses/i2c-pca-isa.c +7
-1+ }else +dev_err (dev,"please specify base\n" ); + +return match; +} +static int __devinit pca_isa_probe (struct device *dev,unsigned int id) {init_waitqueue_head (&pca_wait);@@ -153,7 +166,7 @@ static int __devexit pca_isa_remove(struct device *dev, unsigned int id) {i2c_del_adapter (&pca_isa_ops); -if (irq >0 ) { +if (irq >-1 ) {disable_irq (irq);free_irq (irq, &pca_isa_ops); }
drivers/i2c/busses/i2c-pca-isa.c static int __devexit pca_isa_remove(struct device *dev, unsigned int id) · line 182
Quoted 7 lines
drivers/i2c/busses/i2c-pca-isa.c +1
-0@@ -163,6 +176,7 @@ static int __devexit pca_isa_remove(struct device *dev, unsigned int id) }static struct isa_driver pca_isa_driver = { + .match = pca_isa_match, .probe = pca_isa_probe, .remove =__devexit_p (pca_isa_remove), .driver = {
Other than that, this looks OK. I'll queue this for 2.6.28.
Thanks,
Signature · 6 lines
Jean Delvare _______________________________________________ i2c mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/i2c
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 11 lines · Jean Delvare
Quoted 4 lines · Rene Herman
Grabbing ISA bus resources without anything or anyone telling us we should can break boot on randconfig/allyesconfig builds by keeping resources that are in fact owned by different hardware busy and does as reported by Ingo Molnar.
I don't think it makes much sense to boot randomconfig kernels. The i2c-pca-isa driver is for rare hardware, most people will not use it and certainly won't build it into the kernel. So I don't think there really is a problem in practice here. That being said...
Ingo actually hit this -- i2c-pca-isa "broke" his e1000 by grabbing IRQ10. He does randonfig builds for testing purposes (which a little while ago caught a dumb yet very old bug in an ALSA ISA driver which impressed the usefulness of such on me...)
Quoted 2 lines
I am curious how many users of this driver are left. Maybe it's time to get rid of it.
I rather expect the number is 0 but unless it's a maintenance burden I'd say to keep it around if only as a "see, this is how simple it gets" sort of thing.
(not that the other drivers in there are wildly complex it seems upon looking, so, well, what do I know)
Quoted 9 lines
Review
Quoted 7 lines
(quoted diff) +7
-0+static int __devinit pca_isa_match(struct device *dev, unsigned int id) +{ + int match = base != 0; + + if (match) { + if (irq == -1) + dev_warn(dev, "using poling mode (specify irq)\n");Spelling: polling.
Argh.
Quoted 1 line
Other than that, this looks OK. I'll queue this for 2.6.28.
Thanks.
Rene.
_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c