On Wed, Dec 09, 2009 at 11:48:41AM +1100, Mark Ware wrote: > Port C interrupts can be either falling edge, or either edge. > Other external interrupts are either falling edge or active low. > > Signed-Off-By: Mark Ware <mw...@elphinstone.net>
Looks correct (checked with 8272 and 8555 specs). Reviewed-by: Anton Vorontsov <avoront...@ru.mvista.com> Cosmetic nitpicks below. I tend to think they don't desire another resend, but I couldn't resist making them anyway. ;-) [...] > + if (src >= CPM2_IRQ_PORTC15 && src <= CPM2_IRQ_PORTC0) { > + if (flow_type == IRQ_TYPE_NONE) > + flow_type = IRQ_TYPE_EDGE_BOTH; > + > + if ((flow_type != IRQ_TYPE_EDGE_BOTH) && > + (flow_type != IRQ_TYPE_EDGE_FALLING)) { I'd place one more tab here. And parenthesis aren't actually needed. > + printk(KERN_ERR "CPM2 PIC: sense type 0x%x not > supported\n", > + flow_type); > + return -EINVAL; > + } > + } else { > + if (flow_type == IRQ_TYPE_NONE) > + flow_type = IRQ_TYPE_LEVEL_LOW; > + > + if (flow_type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH)) { > + printk(KERN_ERR "CPM2 PIC: sense type 0x%x not > supported\n", > + flow_type); pr_err is shorter. Also, this message is duplicated. Would be better to do something like: if (somethingwrong) goto err_sense; ... return 0; err_sense: pr_err("CPM2 PIC: sense type 0x%x not supported\n", flow_type); return -EINVAL; } Or we may don't print any errors at all. For internal interrupts we don't print them anyway, i.e. the current code has just return (flow_type & IRQ_TYPE_LEVEL_LOW) ? 0 : -EINVAL; Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev