Re: [PATCH 2/2] DaVinci: DM365: Add the device_enable for the DaVinci Keyscan

[Miguel Aguilar <miguel.aguilar@ridgerun.com>]

Nori, Sekhar wrote:
> On Fri, Nov 13, 2009 at 22:49:46, miguel.aguilar@ridgerun.com wrote:
>> From: Miguel Aguilar <miguel.aguilar@ridgerun.com>
>>
>> Adds the device_enable function to the DaVinci Keyscan platform data
>> to setup the PINMUX configuration.
>>
>> It also removes #ifdef from the DM365 EVM board in order to load it
>> properly as a module.
>>
>> Signed-off-by: Miguel Aguilar <miguel.aguilar@ridgerun.com>
>> ---
>>  arch/arm/mach-davinci/board-dm365-evm.c |   11 ++++++-----
>>  arch/arm/mach-davinci/dm365.c           |    1 -
>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/board-dm365-evm.c b/arch/arm/mach-davinci/board-dm365-evm.c
>> index 8d23972..5abf89d 100644
>> --- a/arch/arm/mach-davinci/board-dm365-evm.c
>> +++ b/arch/arm/mach-davinci/board-dm365-evm.c
>> @@ -195,7 +195,11 @@ static struct davinci_i2c_platform_data i2c_pdata = {
>>       .bus_delay      = 0     /* usec */,
>>  };
>>
>> -#ifdef CONFIG_KEYBOARD_DAVINCI
>> +void dm365evm_keyscan_enable(struct device *dev)
>> +{
>> +     davinci_cfg_reg(DM365_KEYSCAN);
> 
> This could probably return a useful error (later)
> so, its better to make a provision to propagate
> the error.
Yes, that would be a good idea.
> 
>> +}
>> +
>>  static unsigned short dm365evm_keymap[] = {
>>       KEY_KP2,
>>       KEY_LEFT,
>> @@ -217,6 +221,7 @@ static unsigned short dm365evm_keymap[] = {
>>  };
>>
>>  static struct davinci_ks_platform_data dm365evm_ks_data = {
>> +     .device_enable  = dm365evm_keyscan_enable,
> 
> Maybe it's a personal choice, but simply calling it
> 'init' instead of 'device_enable' should be sufficient.
The idea is to avoid confusion with other init functions, due to the specific 
intention of this function, which is perform board specific actions only when 
the device is enabled, mainly when the driver is built as a module.
> 
> Thanks,
> Sekhar
> 

Thanks,

Miguel Aguilar