Re: [PATCH v2 2/2] extcon: add optional debounce-timeout-ms attribute

[Raveendra Padasalagi]

Thanks Chanwoo Choi for the comments. Will revise the patch.

Regards,
Raveendra
On Thu, Oct 19, 2017 at 10:52 AM, Chanwoo Choi <cw00.choi@xxxxxxxxxxx> wrote:
> On 2017ë 10ì 19ì 13:59, Chanwoo Choi wrote:
>> Hi,
>>
>> On 2017ë 10ì 19ì 12:26, Raveendra Padasalagi wrote:
>>> Add changes to capture optional dt attribute "debounce-timeout-ms"
>>> provided in extcon node and used the same value if provided otherwise
>>> default value of 20ms is used for id and vbus gpios debounce time.
>>>
>>> Signed-off-by: Raveendra Padasalagi <raveendra.padasalagi@xxxxxxxxxxxx>
>>> Reviewed-by: Ray Jui <ray.jui@xxxxxxxxxxxx>
>>> Reviewed-by: Srinath Mannam <srinath.mannam@xxxxxxxxxxxx>
>>> ---
>>>
>>> Changes in v2:
>>>  Rename gpio_debounce_timeout_ms to debounce_usecs
>>>
>>>  drivers/extcon/extcon-usb-gpio.c | 12 +++++++++---
>>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/extcon/extcon-usb-gpio.c b/drivers/extcon/extcon-usb-gpio.c
>>> index 9c925b0..76ef1da 100644
>>> --- a/drivers/extcon/extcon-usb-gpio.c
>>> +++ b/drivers/extcon/extcon-usb-gpio.c
>>> @@ -41,6 +41,7 @@ struct usb_extcon_info {
>>>
>>>      unsigned long debounce_jiffies;
>>>      struct delayed_work wq_detcable;
>>> +    unsigned int debounce_usecs;
>>>  };
>>>
>>>  static const unsigned int usb_extcon_cable[] = {
>>> @@ -133,6 +134,11 @@ static int usb_extcon_probe(struct platform_device *pdev)
>>>      if (IS_ERR(info->vbus_gpiod))
>>>              return PTR_ERR(info->vbus_gpiod);
>>>
>>> +    ret = of_property_read_u32(np, "input-debounce",
>>> +                         &info->debounce_usecs);
>>> +    if (ret)
>>> +            info->debounce_usecs = USB_GPIO_DEBOUNCE_MS;
>>
>> The USB_GPIO_DEBOUNCE_MS indicates 20 millisecond.
>> You need to redefine it as following:
>>       -#define USB_GPIO_DEBOUNCE_MS   20      /* ms */
>>       +#define USB_GPIO_DEBOUNCE_USEC 20000
>>
>>       info->debounce_usecs = USB_GPIO_DEBOUNCE_USEC;
>>
>> or
>>       info->debounce_usecs = USB_GPIO_DEBOUNCE_MS * 1000;
>>
>>
>>> +
>>>      info->edev = devm_extcon_dev_allocate(dev, usb_extcon_cable);
>>>      if (IS_ERR(info->edev)) {
>>>              dev_err(dev, "failed to allocate extcon device\n");
>>> @@ -147,13 +153,13 @@ static int usb_extcon_probe(struct platform_device *pdev)
>>>
>>>      if (info->id_gpiod)
>>>              ret = gpiod_set_debounce(info->id_gpiod,
>>> -                                     USB_GPIO_DEBOUNCE_MS * 1000);
>>> +                                     info->debounce_usecs * 1000);
>>
>> The debounce_usecs is already microsecond, You don't need to mutiply with 1000.
>>
>>
>>>      if (!ret && info->vbus_gpiod)
>>>              ret = gpiod_set_debounce(info->vbus_gpiod,
>>> -                                     USB_GPIO_DEBOUNCE_MS * 1000);
>>> +                                     info->debounce_usecs * 1000);
>
> You don't need to mutiply with 1000.
>
>>>
>>>      if (ret < 0)
>>> -            info->debounce_jiffies = msecs_to_jiffies(USB_GPIO_DEBOUNCE_MS);
>>> +            info->debounce_jiffies = msecs_to_jiffies(info->debounce_usecs);
>
> You should you the 'usecs_to_jiffies' because info->debounce_usecs indicates the usec.
>
> --
> Best Regards,
> Chanwoo Choi
> Samsung Electronics