Hello, On Tue, Nov 17, 2009 at 11:15:58, A. Porodko wrote: > Hello, > > Nori, Sekhar wrote: > > On Sat, Nov 14, 2009 at 21:12:36, Andrey Porodko wrote: > > > >> Signed-off-by: Andrey Porodko <[email protected]>
[...] > >> diff --git a/drivers/mfd/neuros_osd2_msp.c b/drivers/mfd/neuros_osd2_msp.c > >> new file mode 100644 > >> index 0000000..6cf5ca6 > >> --- /dev/null > >> +++ b/drivers/mfd/neuros_osd2_msp.c > >> > > > > This should actually merge through MFD subsystem. > > I take it you are only looking for a review here. > > > > > Sorry. I don't quite understand what you mean here. I thought I made it > a MFD driver. The path for this driver to reach Linus's kernel is not through Kevin's tree (DaVinci GIT tree), but through MFD subsystem tree. Have a look at MAINTANIERS file for information about who maintains the mfd tree. Kevin only sends arch/arm/mach-davinci/* changes to Linus. > >> @@ -0,0 +1,203 @@ > >> +/* > >> + * neuros_osd2_msp.c - driver for MSP430 firmware on Neuros OSD2 board > >> + * > >> + * Based on code of dm355evm.c by David Brownell > >> > > > > The file actually referenced is dm355evm_msp.c. On the whole > > this one looks like a cut down version of the one for DM355 EVM. > > Have you considered ways of re-using the original file? The > > msp_has_xxx() macros should help adding/removing features specific > > to the two boards. > > > > Thanks, > > Sekhar > > > > > Seems some time ago the file was dm355evm, obviously it was renamed later. > You are right this driver is mostly a cut and paste of dm355 evm, but > the thing is > 1. This neuros osd2 board is a descendant of dm6446 evm board not dm355 > and I don't know how good it is to mix both in one file. IMO, as long as the code still makes sense, we can mix them in one file. > 2. dm355 evm has too much code and features which neuros doesn't have. > Look at the size of file it is more then twice shorter than the original > one. You should compare object sizes instead. All the if(0)'s will be optimized out by gcc. > 3. Also having routines named as dm355evm_msp in board which is actually > OSD2 can confuse, don't you agree? Otherwise the whole dm355evm_msp code > would be an entire #ifdef on each line. So I think it would be better to > keep code separately. Having routine names which match board names is nice but not necessary for functionality. This happens a lot of times when it is impossible to guess what the file will end up getting used into when it is first written. You could also first update the code to use some generic naming before adding osd2 support. > Possibly one day when mfd code is merged into basic davinci platfrom > code good thing will hapen to all davinci mfd drivers. Er, mfd driver code is already present in Linus's tree. Not sure what you mean here. Thanks, Sekhar _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
