Re: [PATCH] Input: pmic8xxx-pwrkey - Set sane default for debounce time

[Stephen Boyd]

On 03/31/14 11:23, Josh Cartwright wrote:
> On Mon, Mar 31, 2014 at 11:14:24AM -0700, Stephen Boyd wrote:
>> If the debounce time is 0 our usage of ilog2() later on in this
>> driver will cause undefined behavior. If CONFIG_OF=n this fact is
>> evident to the compiler, and it emits a call to ____ilog2_NaN()
>> which doesn't exist. Fix this by setting a sane default for
>> debounce.
>>
>> Reported-by: Arnd Bergmann <arnd@xxxxxxxx>
>> Signed-off-by: Stephen Boyd <sboyd@xxxxxxxxxxxxxx>
>> ---
>>  drivers/input/misc/pmic8xxx-pwrkey.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/misc/pmic8xxx-pwrkey.c b/drivers/input/misc/pmic8xxx-pwrkey.c
>> index 1cb8fda7a166..27add04676e1 100644
>> --- a/drivers/input/misc/pmic8xxx-pwrkey.c
>> +++ b/drivers/input/misc/pmic8xxx-pwrkey.c
>> @@ -92,7 +92,7 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
>>  	bool pull_up;
>>  
>>  	if (of_property_read_u32(pdev->dev.of_node, "debounce", &kpd_delay))
>> -		kpd_delay = 0;
>> +		kpd_delay = 15625;
> Should "debounce" even be optional?  I'm wondering if we should just
> make it required...

Hmm maybe. It's not like things won't work without it set though.

>
> At the very least this default value should be documented in the DT
> binding.
>

I never understood this. The default value is something that is entirely
software driven. It could change at any time. If DT implementers aren't
specifying it they're saying "I don't care what the value is, let the
driver decide if they care". They're not saying, "Oh, that default value
in the binding is good enough for me so I don't need to specify this".

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/