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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 19 Oct 2017 15:03:45 +0530
From:   Raveendra Padasalagi <raveendra.padasalagi@...adcom.com>
To:     Chanwoo Choi <cw00.choi@...sung.com>
Cc:     MyungJoo Ham <myungjoo.ham@...sung.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        bcm-kernel-feedback-list <bcm-kernel-feedback-list@...adcom.com>
Subject: Re: [PATCH v2 2/2] extcon: add optional debounce-timeout-ms attribute

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@...sung.com> 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@...adcom.com>
>>> Reviewed-by: Ray Jui <ray.jui@...adcom.com>
>>> Reviewed-by: Srinath Mannam <srinath.mannam@...adcom.com>
>>> ---
>>>
>>> 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