Re: [PATCH 2/2] I2C: ISP1301_OMAP: New-style i2c driver updates, part 2

[Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>]

On Sat, 23 Feb 2008 21:08:04 +0200, Felipe Balbi wrote:
> Sorry replying to myself, but I have a better with most of comments
> from Jean applied.
> Missing irq flags, mostly.
> 
> It's attached

This update adds this bug:

> +	if (client->irq > 0)
> +		status = request_irq(client->irq, isp1301_irq,
> +				IRQF_SAMPLE_RANDOM | IRQF_TRIGGER_FALLING,
> +				DRIVER_NAME, isp);
>  	if (status < 0) {

This test should be moved inside the "if (client->irq > 0)".

> -		dev_dbg(&i2c->dev, "can't get IRQ %d, err %d\n",
> -				isp->irq, status);
> +		dev_dbg(&client->dev, "can't get IRQ %d, err %d\n",
> +				client->irq, status);
>  #ifdef	CONFIG_USB_OTG
>  fail2:
>  #endif
> -		i2c_detach_client(i2c);
> +		i2c_detach_client(client);

Also note (I missed it in my previous review): don't call
i2c_detach_client() for a new-syle driver!

>  		goto fail1;
>  	}

You addressed some of the issues I raised (thanks) but not all, so I
still can't include this patch into my i2c tree for now.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c