On Sat, 2008-04-12 at 06:32 +0800, Michael Gao wrote: > Kevin, > > Please see comments below. > > On Tue, 2008-04-01 at 16:45 -0700, Kevin Hilman wrote: > > In addition to Dirk's comments, I might add that patches against the > > master branch of the DaVinci git tree are preferred. > With the newly created git-tree and such, I think Steven can fix the > patches according to comments from both Dirk and you, except for this > one. > > It is an unfortunate fact that Neuros must stick a certain kernel > version (2.6.23 ATM) for well-known reasons, I am afraid Steven will not > have the opportunity to go further to make sure the patch apply to > master branch cleanly. > > I take your 'might' above as an ok to this. ;-) > > > 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. > Yes, they should all be fixed. Yes, they are all fixed . > > 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. > I believe this was the case already with the patches Steven provided, > first there was a strict forward porting, then a patch following up to > remove the devfs, I'll let Steven speak for himself in the up-comping > new patch series. The porting patch is rather simple, remove the devfs, replace the struct class_simple with struct class . However, the debug compile error is still not fixed. > > 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. > It was me who brutally removed some of the debug printfs due to > compiling error if keeping them, I didn't know how to fix it and I still > don't. :-( > > Steven, can you help bring them back and post the error messages here to > ask for help if you can't fix them yourself? Thanks Michael. I will send out a mail soon ask for help. > > 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;) > I thought Steven has fixed this, but Steven can speak better for > himself. > > Thanks, It is fixed now. thanks.
_______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
