Re: [PATCH v2 1/2] Input: Add device_enable handler to DaVinci Keyscan platform data

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

Dmitry Torokhov wrote:
> On Thu, Nov 19, 2009 at 11:54:35AM -0600, Miguel Aguilar wrote:
>> Dmitry Torokhov wrote:
>>> On Thu, Nov 19, 2009 at 10:32:21AM -0600, Miguel Aguilar wrote:
>>>> Hi Dmitry,
>>>>
>>>>>>  +	if (pdata->device_enable) {
>>>>>> +		error = pdata->device_enable(dev);
>>>>>> +		if (error < 0) {
>>>>>> +			dev_dbg(dev, "device enable function failed\n");
>>>>>> +			return error;
>>>>>> +		}
>>>>>> +	}
>>>>>> +
>>>>> Hi Miguel,
>>>>>
>>>>> Does this need to live in the driver? Why can't platform code do this
>>>>> for us?
>>>>>
>>>>> Thanks.
>>>>>
>>>> The reason to invoke the device_enable function in the driver is 
>>>> because in the testing process of the key scan driver a issue was 
>>>> found when the key scan is built as a module.
>>>>
>>>> There is a specific PINMUX configuration that only should be set if 
>>>> the key scan driver is loaded in the kernel to avoid pin conflicts. 
>>>> So when the key scan is built as a module the board file (or platform 
>>>> code) doesn't know if the key scan is loaded or not, so that's why 
>>>> the driver is the one who must invoke the device_enable function in 
>>>> the probe function.
>>>>
>>> I see... What happens if PINMUX is set but there isn't a driver? Also,
>>> what happens when you unload the module? Don't you need a similar call
>>> to disable PINMUX configuration?
>>>
>> Dmitry,
>>
>> If PINMUX is set and there isn't the driver, the PINMUX configuration can 
>> overwrite the PINMUX configuration needed by other drivers which actually 
>> are loaded. This situation happens in the DM365 EVM, because the hardware 
>> design there are hardware modules that can't coexist, so handling the 
>> PINMUX configuration calling a function pointer is a safe way to ensure 
>> that the PINMUX is set only when the module is loaded.
>>
> 
> How do you ensure that only one of these drivers is loaded at a time?
> Or is the set of supported devices is board-specific (in which case the
> board code should have an idea how to properly set the configuration for
> the devices that are supported on that board)?
The DM365 EVM has other device that can't coexist with key scan, so this is a 
particular situation. There no way to ensure that one of this modules is loaded 
at time, but using device_enable handler is a proper way to ensure that a driver 
will be loaded properly, the board code can't assume which driver is going to be 
loaded.

I think Sneha, can bring you more details about this particular case.
> 
>> Disable PINMUX configuration doesn't make sense, because there isn't a 
>> thing such disable PINMUX configuration, there are a lot of possible 
>> PINMUX combinations, and there is no combination that means disable, and 
>> also change the PINMUX configuration to a different state would be risky.
>>
> 
> I see.
>