Hi Marek On Sat, 22 Aug 2009, Marek Vasut wrote: > From 479aafc9a6540efec8a691a84aff166eb0218a72 Mon Sep 17 00:00:00 2001 > From: Marek Vasut <marek.vasut@xxxxxxxxx> > Date: Sat, 22 Aug 2009 05:14:23 +0200 > Subject: [PATCH 2/3] Add driver for OmniVision OV9640 sensor > > Signed-off-by: Marek Vasut <marek.vasut@xxxxxxxxx> Ok, I see, you rebased your patches on my soc-camera patch-stack, this is good, thanks. But you missed a couple of my comments - you still have a few static functions in the ov9640.c marked inline: ov9640_set_crop() is not used at all, ov9640_set_bus_param() gets assigned to a struct member, so, cannot be inline. ov9640_alter_regs() is indeed only called at one location, but see Chapter 15 in Documentation/CodingStyle. You left at least one multi-line comment wrongly formatted. You left some broken printk format lines, etc. You moved your header under drivers/... - that's good. But, please, address all my comments that I provided in a private review, or explain why you do not agree. Otherwise I feel like I wasted my time. Besides, your mailer has wrapped lines. Please, read Documentation/email-clients.txt on how to configure your email client to send patches properly. In the worst case, you can inline your patches while reviewing, and then attach them for a final submission. This is, however, discouraged, because it makes review and test of your intermediate patches with wrapped lines more difficult. Also, please check your patches with scripts/checkpatch.pl. [patch skipped] Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
