Hi Sean, On Mon, 07 Jan 2008 21:03:12 -0500 Sean MacLennan <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED]
http://www.canb.auug.org.au/~sfr/
pgp9mNRmHQIRl.pgp
Description: PGP signature
_______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
