Re: [PATCH] change the keypad layout and the uart port number for the Pirelli DP-L10

Hi Marco, thanks for your effort to provide another patch. On Tue, Aug 14, 2012 at 09:48:23PM +0200, Marco Schwan wrote: However, I see the following issues: * it only addresses the UART, not the keypad * it introduces a function call for something that's compile determined As discussed elsewhere in the thread, we cannot detect the model/board at runtime. That's OK, and you are note expected to change that. However, introducing a function call just to get a constant value is probably a bit overkill. A simpler way would be simply introduce a 'const uint8_t' in the board-specific C files, declare it in some common header file and use it from within the UART code. The even cleaner solution is to have something like a sercomm_init() call which each of the board init files call, specifying the UART number at that time. The general rule is: * _within_ the board-specific part, you should always use the #define and not some kind of dynamic value or global variable. * in generic code that is compiled once and used by multiple boards, all parameters such as UART numbers should be passed in at initialization time. This rule should also apply for the keypad. The keymap has to move to some board-specific 'const' data structure which gets handed into the keypad driver at the time it is initialized. As all of the Compal boards share the same mapping, this should of course not be copied N times, but all refer to one common file. Only the pirelli would provide a different mapping. Regards, Harald -- - Harald Welte <laforge(a)gnumonks.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)