| Message ID | 1508383560-15567-1-git-send-email-raveendra.padasalagi@broadcom.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v2,1/2] Documentation: dt: extcon: add optional debounce-timeout-ms attribute | expand |
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"; >
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>
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); > >
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.
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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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";