On Wed, Jan 6, 2010 at 4:35 AM, Samuel Ortiz <sameo@xxxxxxxxxxxxxxx> wrote: > Hi Cory, > > late review, sorry about that... > > The driver looks fine to me, but I get 23 checkpatch warnings and 5 errors for > it. > Could you please fix them, keeping in mind that I'm fine with printk/dev_* > strings being more than 80 chars. > Yup, no problem. > A couple of additional comments: > > ?cpld? ?? That should probably be rephrased to say "unknown model CPLD". I'll fix that. > Please use pr_* instead. It seems you're using it already, so let's be > consistent. Right. > This routine is fairly big, and could be more readable by calling some > subroutines. The chips initialisation part could be extracted out for example. OK. > Shouldnt it be: htcpld->chip[i].cache_in = 0; instead ? Yes, good catch. > You could have a nicer indentation here: > error = request_threaded_irq(htcpld->chained_irq, > NULL, htcpld_handler, > flags, pdev->name, htcpld); > OK. I'll make those fixes and resubmit. Thanks for reviewing it! - Cory -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
