[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