[PATCH] i2c-ibm_iic driver - new patch

[Stephen Rothwell]

Hi Sean,

On Mon, 07 Jan 2008 21:03:12 -0500 Sean MacLennan <smaclennan at pikatech.com> wrote:
>

Please don't post patches as attachments.

> +static int __devinit iic_probe(struct of_device *ofdev,
> +							   const struct of_device_id *match)

Indenting could be better.

> +{

> +	if (!(dev = kzalloc(sizeof(*dev), GFP_KERNEL))) {

Please split the assignments from the tests.  Here and elsewhere.

> +		printk(KERN_CRIT "ibm-iic: failed to allocate device data\n");

I am not sure that these messages are necessary and, even if so, not KERN_CRIT.

> +	if(iic_force_poll)

Space after "if"

> +	if (dev->irq != NO_IRQ) {
	.
	.
> +	}
> +
> +	if (dev->irq == NO_IRQ)

	else instead?

> +		printk(KERN_WARNING "ibm-iic%d: using polling mode\n",
> +			   dev->idx);
> +static int __devexit iic_remove(struct of_device *ofdev)
> +{
> +	struct ibm_iic_private* dev = (struct ibm_iic_private*)dev_get_drvdata(&ofdev->dev);

Unnecessary cast.

> +	if (i2c_del_adapter(&dev->adap)){
> +		printk(KERN_CRIT "ibm-iic%d: failed to delete i2c adapter :(\n",
> +			dev->idx);

This is not a KERN_CRIT situation ...

> +		/* That's *very* bad, just shutdown IRQ ... */
> +		if (dev->irq >= 0){

What is that testing? For NO_IRQ as below?

> +		    iic_interrupt_mode(dev, 0);
> +		    free_irq(dev->irq, dev);
> +		    dev->irq = -1;

NO_IRQ?

> +		}
> +	} else {
> +		if (dev->irq != NO_IRQ){
> +		    iic_interrupt_mode(dev, 0);
> +		    free_irq(dev->irq, dev);
> +		}
> +		iounmap(dev->vaddr);
> +		kfree(dev);

Should these last two be after the below brace?

> +	}
> +
> +	return 0;
> +}
> +
> +
> +static struct of_device_id ibm_iic_match[] =

This should be const.

-- 
Cheers,
Stephen Rothwell                    sfr at canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20080108/e1ae56ce/attachment.pgp>