Re: Patch for MSP430 support on Neuros OSD2 board

["Andrey A. Porodko" <andrey.porodko@gmail.com>]

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