Dirk Behme <[EMAIL PROTECTED]> writes: > steven.zhang wrote: >> On Mon, 2008-03-24 at 13:49 +0800, steven.zhang wrote: >> >>>This patch provides the Video Resizer support for DaVinci on kernel >>>2.6.10. >>>sign-off-by: [EMAIL PROTECTED] >>>_______________________________________________ >>>Davinci-linux-open-source mailing list >>>[email protected] >>>http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source >> What about the status of these two patch ? Thanks. > > Do you refer to > > http://linux.omap.com/pipermail/davinci-linux-open-source/2008-March/005828.html > http://linux.omap.com/pipermail/davinci-linux-open-source/2008-March/005829.html > > ? > > - As already mentioned several times, patches should be checkpatch > clean. Seems to me that you introduced additional checkpatch warnings > to patch 02. Some older patches were cleaner. > > Patch 01: > > total: 1 errors, 46 warnings, 2238 lines checked > > Patch 02: > > total: 0 errors, 32 warnings, 365 lines checked > > What is the problem to run checkpatch *before* sending the patches to ML? > > - As already mentioned several times, still strip level 2. > > - A minor thing, but the correct string is "Signed-off-by" >
In addition to Dirk's comments, I might add that patches against the master branch of the DaVinci git tree are preferred. If patches are not against the master branch, it is more work to apply and test them. After working past the problems Dirk mentioned, I applied your patches and they do apply to the master branch, do not compile against the latest kernel due to changes in the interrupt API. Also, I understand this is a forward port of a driver from 2.6.10, so most of the errors/warnings come from the original TI driver, and not from your work. However, for me to merge this code, at a minimum all the checkpatch errors/warnings should be fixed, and there are a handful of other whitespace issues to be cleaned up for readability as well. That being said, I did compare the original driver from the TI/MV 2.6.10 kernel to your forward port, and you seem to have made lots of changes that are not scrictly forward port changes. When doing this, I prefer to see separate patches for these kinds of changes. In other words, I'd prefer to see one patch which is just a forward port, followed by cleanup patches with detailed descriptions. At first glance, the only change strictly needed for foward port is the devfs removal. In addition to this, you've done several changes to the debug prints, as well as added braces around single statement while/if blocks, which is not needed, and is frowned upon by upstream maintainers. Also, you explicitly add a NULL assignment to static variables, which is redundant as these end up in the .bss section which is explicitly zeroed. (specifically: static struct class_simple *rsz_class = NULL;) Kevin _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
