* Re: [PATCH] i2c: don't autograb i2c-pca-isa
[not found] ` <489C8015.8070804-cENuUygGYd//D1n+0JDH9g@public.gmane.org>
@ 2008-08-09 13:28 ` Jean Delvare
[not found] ` <20080809152845.45a9722c-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
0 siblings, 1 reply; 2+ messages in thread
From: Jean Delvare @ 2008-08-09 13:28 UTC (permalink / raw)
To: Rene Herman; +Cc: Andrew Morton, Ingo Molnar, Linux I2C
Hi Rene,
Please Cc the i2c list for i2c patches (added.)
On Fri, 08 Aug 2008 19:19:17 +0200, Rene Herman wrote:
> 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...
> 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.
>
> The^WA user of this driver is a one time
>
> echo "options i2c-pca-isa base=0x330 irq=10" >> /etc/modprobe.conf
>
> away from the old behaviour.
I am curious how many users of this driver are left. Maybe it's time to
get rid of it.
> Signed-off-by: Rene Herman <rene.herman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> drivers/i2c/busses/i2c-pca-isa.c | 20 +++++++++++++++++---
> 1 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-pca-isa.c b/drivers/i2c/busses/i2c-pca-isa.c
> index a119784..2579169 100644
> --- a/drivers/i2c/busses/i2c-pca-isa.c
> +++ b/drivers/i2c/busses/i2c-pca-isa.c
> @@ -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 */
> @@ -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.
> + } 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);
> }
> @@ -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,
--
Jean Delvare
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
^ permalink raw reply [flat|nested] 2+ messages in thread