[PATCH] i2c-ibm_iic driver - new patch

Stephen Rothwell sfr at canb.auug.org.au
Tue Jan 8 17:36:36 EST 2008


On Tue, 08 Jan 2008 00:56:27 -0500 Sean MacLennan <smaclennan at pikatech.com> wrote:
>
> Stephen Rothwell wrote:
> >
> > On Mon, 07 Jan 2008 21:03:12 -0500 Sean MacLennan <smaclennan at pikatech.com> wrote:
> >   
> > Please don't post patches as attachments.
>  
> Ok.

Unfortunately, you are using thunderbird and so the patch is now wrapped.
There is a workaround, see Documentation/email-clients.txt.

> > Please split the assignments from the tests.  Here and elsewhere.
> >   
> I made the changes in my code. I am trying to leave the original code as 
> much as possible.

Thats all we ask.

> >> +	} 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?
> >
> >   
> I'm not really qualified to answer, but I will anyway ;) I assume the 
> original author is basically saying if he cannot delete the adapter, it 
> is unsafe to free the memory since the i2c code may still use it. If I 
> have read that right, then I agree.

OK, I can see that this is a "that should not happen" condition.

> +    if (iic_force_poll)
> +        dev->irq = NO_IRQ;
> +    else if ((dev->irq = irq_of_parse_and_map(np, 0)) == NO_IRQ)

You missed this one.

Overall looks better, except all your indentation is now 4 spaces. We use
a TAB character for each level of indentation and you should be able to
set your editor to *display* the TABs as 4 places if that is what you like.

-- 
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/3c5fd612/attachment.pgp>


More information about the Linuxppc-dev mailing list