Hello, Nori, Sekhar wrote: > On Sat, Nov 14, 2009 at 21:12:36, Andrey Porodko wrote: > >> Signed-off-by: Andrey Porodko <[email protected]> >> --- >> arch/arm/mach-davinci/board-neuros-osd2.c | 3 + >> drivers/mfd/Kconfig | 8 + >> drivers/mfd/Makefile | 1 + >> drivers/mfd/neuros_osd2_msp.c | 203 >> +++++++++++++++++++++++++++++ >> include/linux/i2c/neuros_osd2_msp.h | 41 ++++++ >> 5 files changed, 256 insertions(+), 0 deletions(-) >> create mode 100644 drivers/mfd/neuros_osd2_msp.c >> create mode 100644 include/linux/i2c/neuros_osd2_msp.h >> >> diff --git a/arch/arm/mach-davinci/board-neuros-osd2.c >> b/arch/arm/mach-davinci/board-neuros-osd2.c >> index bd9ca07..e5a4a99 100644 >> --- a/arch/arm/mach-davinci/board-neuros-osd2.c >> +++ b/arch/arm/mach-davinci/board-neuros-osd2.c >> @@ -202,6 +202,9 @@ static struct davinci_i2c_platform_data ntosd2_i2c_pdata >> = { >> }; >> >> static struct i2c_board_info __initdata ntosd2_i2c_info[] = { >> + { /* MSP430 interface >> */ >> > > Unnecessary indentation before end of comment. > > Tastes differ. OK. I can remove extra tab, for sure. >> + I2C_BOARD_INFO("neuros_osd2_msp", NTOSD2_MSP430_I2C_ADDR), >> + }, >> }; >> >> static int ntosd2_init_i2c(void) >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >> index 570be13..41ceebc 100644 >> --- a/drivers/mfd/Kconfig >> +++ b/drivers/mfd/Kconfig >> @@ -43,6 +43,14 @@ config MFD_DM355EVM_MSP >> boards. MSP430 firmware manages resets and power sequencing, >> inputs from buttons and the IR remote, LEDs, an RTC, and more. >> >> +config MFD_NEUROS_OSD2_MSP >> + bool "Neuros OSD2 open set top box microcontroller" >> + depends on I2C && MACH_NEUROS_OSD2 >> + help >> + This driver supports the MSP430 microcontroller used on these >> + boards. MSP430 firmware manages inputs/outputs from IR remote, >> + an RTC and more. >> + >> config HTC_EGPIO >> bool "HTC EGPIO support" >> depends on GENERIC_HARDIRQS && GPIOLIB && ARM >> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile >> index f3b277b..32baa62 100644 >> --- a/drivers/mfd/Makefile >> +++ b/drivers/mfd/Makefile >> @@ -9,6 +9,7 @@ obj-$(CONFIG_HTC_EGPIO) += htc-egpio.o >> obj-$(CONFIG_HTC_PASIC3) += htc-pasic3.o >> >> obj-$(CONFIG_MFD_DM355EVM_MSP) += dm355evm_msp.o >> +obj-$(CONFIG_MFD_NEUROS_OSD2_MSP) += neuros_osd2_msp.o >> >> obj-$(CONFIG_MFD_T7L66XB) += t7l66xb.o >> obj-$(CONFIG_MFD_TC6387XB) += tc6387xb.o >> 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. >> @@ -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. 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. 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. Possibly one day when mfd code is merged into basic davinci platfrom code good thing will hapen to all davinci mfd drivers.
Thank you for your review. -- Best regards Andrey A. Porodko _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
