Messages in this thread | | | Date | Mon, 30 Nov 2009 11:37:33 +0500 | From | "Andrey A. Porodko" <> | Subject | Re: Patch for MSP430 support on Neuros OSD2 board |
| |
Samuel Ortiz wrote:
Hi Samuel,
The reason I used "ifdef" instead of refactoring code is that I don't have dm355 board to check nor I'm familiar with this hardware and I was afraid to screw up what's already done for dm355. Initially I created a completely separate driver (although based on dm355) for Neuros, but kernel people told me to combine code with existent. - Is it possible to find someone with dm355 hardware to check if didn't screw up it? - I don't quite understand how to evaluate impact on config_* files, do you mean I need to check standard kernel configuration files bundled with kernel and make necessary adjustments there?
Thank you for a quick reply. > Hi Andrey, > > On Thu, Nov 26, 2009 at 06:17:22PM +0500, Andrey A. Porodko wrote: > >> Hello, >> >> Here is a patch for MSP430 chip support for Neuros OSD2 (Davinci DM6446 >> based) board. >> Patch made against 2.6.32-rc6 kernel. >> > Thanks for the patch, here are some comments about it: > > - Renaming a file may be acceptable, but you have to delete the prvious one. > Also, as you're changing the Kconfig symbol, you should evaluate the impact on > the current users (in config_* files for example). > > - Then about the code itself: ifdefs as the one you're doing here is not > exactly nice, and leads to a lot of code replication and maintenance burden. > It seems that you're trying to have a common MSP430 driver support for 2 > different boards, which is a good idea. The main problem, if I understand it > correctly, is those 2 boards are running the same MSP430 HW running different > FWs. > What I'd really like to see here would be to have a generic MSP430 support. > You'd need to define a FW definition structure (it seems it would mostly be > GPIO settings), then have different static definitions for every known firmware > revision, and finally have a common probe routine that would go through this > firmware structure and sets thing accordingly. You would pass the firmware > revision you're using from your board definitions, unless there are some > registers on that chip that would let us know about this firmware. > > Cheers, > Samuel. > > >
-- Best regards Andrey A. Porodko
| |