[PATCH v2 1/2] Documentation: dt: extcon: add optional debounce-timeout-ms attribute

[PATCH v2 1/2] Documentation: dt: extcon: add optional debounce-timeout-ms attribute

[Raveendra Padasalagi]

Add documentation on optional dt attribute "debounce-timeout-ms"
in extcon node to capture user specified timeout value for id
and vbus gpio detection.

Signed-off-by: Raveendra Padasalagi <raveendra.padasalagi@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
---

Changes in v2:
 Rename debounce-timeout-ms to input-debounce

 Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
index dfc14f7..d115900 100644
--- a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
+++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
@@ -10,6 +10,9 @@ Either one of id-gpio or vbus-gpio must be present. Both can be present as well.
 - id-gpio: gpio for USB ID pin. See gpio binding.
 - vbus-gpio: gpio for USB VBUS pin.
 
+Optional properties:
+- input-debounce: debounce timeout value for id and vbus gpio in microseconds.
+
 Example: Examples of extcon-usb-gpio node in dra7-evm.dts as listed below:
 	extcon_usb1 {
 		compatible = "linux,extcon-usb-gpio";
-- 
1.9.1

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

[Raveendra Padasalagi]

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@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.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;
+
 	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);
 	if (!ret && info->vbus_gpiod)
 		ret = gpiod_set_debounce(info->vbus_gpiod,
-					 USB_GPIO_DEBOUNCE_MS * 1000);
+					 info->debounce_usecs * 1000);
 
 	if (ret < 0)
-		info->debounce_jiffies = msecs_to_jiffies(USB_GPIO_DEBOUNCE_MS);
+		info->debounce_jiffies = msecs_to_jiffies(info->debounce_usecs);
 
 	INIT_DELAYED_WORK(&info->wq_detcable, usb_extcon_detect_cable);
 
-- 
1.9.1

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

[Chanwoo Choi]

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@broadcom.com>
> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.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);
>  
>  	if (ret < 0)
> -		info->debounce_jiffies = msecs_to_jiffies(USB_GPIO_DEBOUNCE_MS);
> +		info->debounce_jiffies = msecs_to_jiffies(info->debounce_usecs);

ditto.

>  
>  	INIT_DELAYED_WORK(&info->wq_detcable, usb_extcon_detect_cable);
>  
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

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

[Chanwoo Choi]

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

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

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

[Chanwoo Choi]

On 2017년 10월 19일 12:25, Raveendra Padasalagi wrote:
> Add documentation on optional dt attribute "debounce-timeout-ms"
> in extcon node to capture user specified timeout value for id
> and vbus gpio detection.
> 
> Signed-off-by: Raveendra Padasalagi <raveendra.padasalagi@broadcom.com>
> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
> ---
> 
> Changes in v2:
>  Rename debounce-timeout-ms to input-debounce
> 
>  Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> index dfc14f7..d115900 100644
> --- a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> @@ -10,6 +10,9 @@ Either one of id-gpio or vbus-gpio must be present. Both can be present as well.
>  - id-gpio: gpio for USB ID pin. See gpio binding.
>  - vbus-gpio: gpio for USB VBUS pin.
>  
> +Optional properties:
> +- input-debounce: debounce timeout value for id and vbus gpio in microseconds.

The pinctrl-bindings.txt[1] specify the unit of 'input-debounce'.
The unit is usec instead of 'microsecond'. You need to modify the description of unit.
[1] Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

> +
>  Example: Examples of extcon-usb-gpio node in dra7-evm.dts as listed below:
>  	extcon_usb1 {
>  		compatible = "linux,extcon-usb-gpio";
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

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

[Chanwoo Choi]

Hi,

On 2017년 10월 19일 13:47, Chanwoo Choi wrote:
> On 2017년 10월 19일 12:25, Raveendra Padasalagi wrote:
>> Add documentation on optional dt attribute "debounce-timeout-ms"
>> in extcon node to capture user specified timeout value for id
>> and vbus gpio detection.
>>
>> Signed-off-by: Raveendra Padasalagi <raveendra.padasalagi@broadcom.com>
>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>> Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
>> ---
>>
>> Changes in v2:
>>  Rename debounce-timeout-ms to input-debounce
>>
>>  Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>> index dfc14f7..d115900 100644
>> --- a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>> +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>> @@ -10,6 +10,9 @@ Either one of id-gpio or vbus-gpio must be present. Both can be present as well.
>>  - id-gpio: gpio for USB ID pin. See gpio binding.
>>  - vbus-gpio: gpio for USB VBUS pin.
>>  
>> +Optional properties:
>> +- input-debounce: debounce timeout value for id and vbus gpio in microseconds.
> 
> The pinctrl-bindings.txt[1] specify the unit of 'input-debounce'.
> The unit is usec instead of 'microsecond'. You need to modify the description of unit.
> [1] Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

Sorry. My mistake. I confused the 'nsec' with 'usec'. This patch is right.
Please ignore my comment.

> 
>> +
>>  Example: Examples of extcon-usb-gpio node in dra7-evm.dts as listed below:
>>  	extcon_usb1 {
>>  		compatible = "linux,extcon-usb-gpio";
>>
> 
> 

Looks good to me.
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

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

[Rob Herring]

On Thu, Oct 19, 2017 at 08:55:59AM +0530, Raveendra Padasalagi wrote:
> Add documentation on optional dt attribute "debounce-timeout-ms"
> in extcon node to capture user specified timeout value for id
> and vbus gpio detection.
> 
> Signed-off-by: Raveendra Padasalagi <raveendra.padasalagi@broadcom.com>
> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
> ---
> 
> Changes in v2:
>  Rename debounce-timeout-ms to input-debounce
> 
>  Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt | 3 +++
>  1 file changed, 3 insertions(+)

Sorry, but extcon bindings should be scrapped IMO and I don't want to 
see additions. "extcon" is a Linuxism and is the first clue.

What's needed is a full USB connector binding, not just some incomplete 
handling of ID and Vbus pins. There's some work happening on that[1].

Rob

[1] https://www.spinics.net/lists/linux-usb/msg161106.html

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

[Chanwoo Choi]

Hi Rob,

On 2017년 10월 25일 23:53, Rob Herring wrote:
> On Thu, Oct 19, 2017 at 08:55:59AM +0530, Raveendra Padasalagi wrote:
>> Add documentation on optional dt attribute "debounce-timeout-ms"
>> in extcon node to capture user specified timeout value for id
>> and vbus gpio detection.
>>
>> Signed-off-by: Raveendra Padasalagi <raveendra.padasalagi@broadcom.com>
>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>> Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
>> ---
>>
>> Changes in v2:
>>  Rename debounce-timeout-ms to input-debounce
>>
>>  Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt | 3 +++
>>  1 file changed, 3 insertions(+)
> 
> Sorry, but extcon bindings should be scrapped IMO and I don't want to 
> see additions. "extcon" is a Linuxism and is the first clue.

As you commented on other mailing list, compatible name had to decide
the type of connector. So, The compatible name will be changed
from 'extcon-usb-gpio' to 'gpio-usb-connectors' in order to
remove the 'extcon' from compatible name.

> 
> What's needed is a full USB connector binding, not just some incomplete 
> handling of ID and Vbus pins. There's some work happening on that[1].

These patch set[1] are related to connect/bind between provider driver
and consumer driver through OF graph.

Before, the provider driver should probe their job on the device driver.
This patch just add the debounce property of gpio during the probe time.

> 
> Rob
> 
> [1] https://www.spinics.net/lists/linux-usb/msg161106.html

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics