Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1288997 > unrolled thread
| Started by | Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> |
|---|---|
| First post | 2015-12-11 00:10 +0100 |
| Last post | 2015-12-11 15:50 +0100 |
| Articles | 5 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH] extcon-usb-gpio: add enable pin support Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-12-11 00:10 +0100
Re: [PATCH] extcon-usb-gpio: add enable pin support Rob Herring <robh@kernel.org> - 2015-12-11 05:00 +0100
Re: [PATCH] extcon-usb-gpio: add enable pin support Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-12-11 19:40 +0100
Re: [PATCH] extcon-usb-gpio: add enable pin support Chanwoo Choi <cw00.choi@samsung.com> - 2015-12-11 05:10 +0100
Re: [PATCH] extcon-usb-gpio: add enable pin support Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-12-11 15:50 +0100
| From | Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> |
|---|---|
| Date | 2015-12-11 00:10 +0100 |
| Subject | [PATCH] extcon-usb-gpio: add enable pin support |
| Message-ID | <qEivw-4mq-21@gated-at.bofh.it> |
Sometimes there's a real OTG chip behind the USB ID signal mapped to a GPIO
pin: in my case it's Maxim Integrated MAX3355E which integrates Vbus charge
pump and comparators and passes thru the ID signal from an OTG connector.
This chip also has the SHDN# pin which should be driven high for the normal
operation and low to save power; it is connected to a GPIO pin as well on,
hence we'll have to teach the driver to parse the new optional device tree
property, "enable-gpio"...
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
The patch is against the 'extcon-next' branch of the 'extcon.git' repo.
Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt | 3 +++
drivers/extcon/extcon-usb-gpio.c | 5 +++++
2 files changed, 8 insertions(+)
Index: extcon/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
===================================================================
--- extcon.orig/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
+++ extcon/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
@@ -7,6 +7,9 @@ Required properties:
- compatible: Should be "linux,extcon-usb-gpio"
- id-gpio: gpio for USB ID pin. See gpio binding.
+Optional properties:
+- enable-gpio: gpio for the enable pin. See gpio binding.
+
Example: Examples of extcon-usb-gpio node in dra7-evm.dts as listed below:
extcon_usb1 {
compatible = "linux,extcon-usb-gpio";
Index: extcon/drivers/extcon/extcon-usb-gpio.c
===================================================================
--- extcon.orig/drivers/extcon/extcon-usb-gpio.c
+++ extcon/drivers/extcon/extcon-usb-gpio.c
@@ -33,6 +33,7 @@ struct usb_extcon_info {
struct device *dev;
struct extcon_dev *edev;
+ struct gpio_desc *enable_gpiod;
struct gpio_desc *id_gpiod;
int id_irq;
@@ -99,6 +100,8 @@ static int usb_extcon_probe(struct platf
return -ENOMEM;
info->dev = dev;
+ info->enable_gpiod = devm_gpiod_get_optional(&pdev->dev, "enable",
+ GPIOD_OUT_HIGH);
info->id_gpiod = devm_gpiod_get(&pdev->dev, "id", GPIOD_IN);
if (IS_ERR(info->id_gpiod)) {
dev_err(dev, "failed to get ID GPIO\n");
@@ -155,6 +158,8 @@ static int usb_extcon_remove(struct plat
cancel_delayed_work_sync(&info->wq_detcable);
+ gpiod_set_value_cansleep(info->enable_gpiod, 0);
+
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2015-12-11 05:00 +0100 |
| Message-ID | <qEn2a-7bp-9@gated-at.bofh.it> |
| In reply to | #1288997 |
On Fri, Dec 11, 2015 at 02:07:05AM +0300, Sergei Shtylyov wrote:
> Sometimes there's a real OTG chip behind the USB ID signal mapped to a GPIO
> pin: in my case it's Maxim Integrated MAX3355E which integrates Vbus charge
> pump and comparators and passes thru the ID signal from an OTG connector.
> This chip also has the SHDN# pin which should be driven high for the normal
> operation and low to save power; it is connected to a GPIO pin as well on,
> hence we'll have to teach the driver to parse the new optional device tree
> property, "enable-gpio"...
Some wierd spacing going on here.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> The patch is against the 'extcon-next' branch of the 'extcon.git' repo.
>
> Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt | 3 +++
> drivers/extcon/extcon-usb-gpio.c | 5 +++++
> 2 files changed, 8 insertions(+)
>
> Index: extcon/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> ===================================================================
> --- extcon.orig/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> +++ extcon/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> @@ -7,6 +7,9 @@ Required properties:
> - compatible: Should be "linux,extcon-usb-gpio"
> - id-gpio: gpio for USB ID pin. See gpio binding.
>
> +Optional properties:
> +- enable-gpio: gpio for the enable pin. See gpio binding.
Use -gpios as -gpio is deprecated.
> +
> Example: Examples of extcon-usb-gpio node in dra7-evm.dts as listed below:
> extcon_usb1 {
> compatible = "linux,extcon-usb-gpio";
> Index: extcon/drivers/extcon/extcon-usb-gpio.c
> ===================================================================
> --- extcon.orig/drivers/extcon/extcon-usb-gpio.c
> +++ extcon/drivers/extcon/extcon-usb-gpio.c
> @@ -33,6 +33,7 @@ struct usb_extcon_info {
> struct device *dev;
> struct extcon_dev *edev;
>
> + struct gpio_desc *enable_gpiod;
> struct gpio_desc *id_gpiod;
> int id_irq;
>
> @@ -99,6 +100,8 @@ static int usb_extcon_probe(struct platf
> return -ENOMEM;
>
> info->dev = dev;
> + info->enable_gpiod = devm_gpiod_get_optional(&pdev->dev, "enable",
> + GPIOD_OUT_HIGH);
> info->id_gpiod = devm_gpiod_get(&pdev->dev, "id", GPIOD_IN);
> if (IS_ERR(info->id_gpiod)) {
> dev_err(dev, "failed to get ID GPIO\n");
> @@ -155,6 +158,8 @@ static int usb_extcon_remove(struct plat
>
> cancel_delayed_work_sync(&info->wq_detcable);
>
> + gpiod_set_value_cansleep(info->enable_gpiod, 0);
Shouldn't you support either polarity?
Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> |
|---|---|
| Date | 2015-12-11 19:40 +0100 |
| Message-ID | <qEALM-8a5-5@gated-at.bofh.it> |
| In reply to | #1289115 |
Hello.
On 12/11/2015 06:53 AM, Rob Herring wrote:
>> Sometimes there's a real OTG chip behind the USB ID signal mapped to a GPIO
>> pin: in my case it's Maxim Integrated MAX3355E which integrates Vbus charge
>> pump and comparators and passes thru the ID signal from an OTG connector.
>> This chip also has the SHDN# pin which should be driven high for the normal
>> operation and low to save power; it is connected to a GPIO pin as well on,
>> hence we'll have to teach the driver to parse the new optional device tree
>> property, "enable-gpio"...
>
> Some wierd spacing going on here.
I like my text properly filling up the given columns. What's the problem? :-)
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> ---
>> The patch is against the 'extcon-next' branch of the 'extcon.git' repo.
>>
>> Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt | 3 +++
>> drivers/extcon/extcon-usb-gpio.c | 5 +++++
>> 2 files changed, 8 insertions(+)
>>
>> Index: extcon/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>> ===================================================================
>> --- extcon.orig/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>> +++ extcon/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>> @@ -7,6 +7,9 @@ Required properties:
>> - compatible: Should be "linux,extcon-usb-gpio"
>> - id-gpio: gpio for USB ID pin. See gpio binding.
>>
>> +Optional properties:
>> +- enable-gpio: gpio for the enable pin. See gpio binding.
>
> Use -gpios as -gpio is deprecated.
Didn't know, thanks.
[...]
>> Index: extcon/drivers/extcon/extcon-usb-gpio.c
>> ===================================================================
>> --- extcon.orig/drivers/extcon/extcon-usb-gpio.c
>> +++ extcon/drivers/extcon/extcon-usb-gpio.c
[...]
>> @@ -99,6 +100,8 @@ static int usb_extcon_probe(struct platf
>> return -ENOMEM;
>>
>> info->dev = dev;
>> + info->enable_gpiod = devm_gpiod_get_optional(&pdev->dev, "enable",
>> + GPIOD_OUT_HIGH);
>> info->id_gpiod = devm_gpiod_get(&pdev->dev, "id", GPIOD_IN);
>> if (IS_ERR(info->id_gpiod)) {
>> dev_err(dev, "failed to get ID GPIO\n");
>> @@ -155,6 +158,8 @@ static int usb_extcon_remove(struct plat
>>
>> cancel_delayed_work_sync(&info->wq_detcable);
>>
>> + gpiod_set_value_cansleep(info->enable_gpiod, 0);
> Shouldn't you support either polarity?
The gpiolib does that for me -- devm_gpiod_get_optional() should read the
polarity from DT.
> Rob
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Chanwoo Choi <cw00.choi@samsung.com> |
|---|---|
| Date | 2015-12-11 05:10 +0100 |
| Message-ID | <qEnbP-7u0-13@gated-at.bofh.it> |
| In reply to | #1288997 |
Hi Sergei,
On 2015년 12월 11일 08:07, Sergei Shtylyov wrote:
> Sometimes there's a real OTG chip behind the USB ID signal mapped to a GPIO
> pin: in my case it's Maxim Integrated MAX3355E which integrates Vbus charge
> pump and comparators and passes thru the ID signal from an OTG connector.
s/thru/through ?
> This chip also has the SHDN# pin which should be driven high for the normal
> operation and low to save power; it is connected to a GPIO pin as well on,
> hence we'll have to teach the driver to parse the new optional device tree
> property, "enable-gpio"...
This patch description includes the double space between words. Also, I think
you need to write the patch description again for formal style.
This patch adds the specific 'enable-gpio' pin to express the SHDN#pin for MAX3355E.
I think it is not regular and standard case because maybe USB specification
don't include the SHDN#pin information. I think it not appropriate way.
Instead, you better to make the MAX3355 extcon driver to support this case.
Thanks,
Chanwoo
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> The patch is against the 'extcon-next' branch of the 'extcon.git' repo.
>
> Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt | 3 +++
> drivers/extcon/extcon-usb-gpio.c | 5 +++++
> 2 files changed, 8 insertions(+)
>
> Index: extcon/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> ===================================================================
> --- extcon.orig/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> +++ extcon/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
> @@ -7,6 +7,9 @@ Required properties:
> - compatible: Should be "linux,extcon-usb-gpio"
> - id-gpio: gpio for USB ID pin. See gpio binding.
>
> +Optional properties:
> +- enable-gpio: gpio for the enable pin. See gpio binding.
> +
> Example: Examples of extcon-usb-gpio node in dra7-evm.dts as listed below:
> extcon_usb1 {
> compatible = "linux,extcon-usb-gpio";
> Index: extcon/drivers/extcon/extcon-usb-gpio.c
> ===================================================================
> --- extcon.orig/drivers/extcon/extcon-usb-gpio.c
> +++ extcon/drivers/extcon/extcon-usb-gpio.c
> @@ -33,6 +33,7 @@ struct usb_extcon_info {
> struct device *dev;
> struct extcon_dev *edev;
>
> + struct gpio_desc *enable_gpiod;
> struct gpio_desc *id_gpiod;
> int id_irq;
>
> @@ -99,6 +100,8 @@ static int usb_extcon_probe(struct platf
> return -ENOMEM;
>
> info->dev = dev;
> + info->enable_gpiod = devm_gpiod_get_optional(&pdev->dev, "enable",
> + GPIOD_OUT_HIGH);
> info->id_gpiod = devm_gpiod_get(&pdev->dev, "id", GPIOD_IN);
> if (IS_ERR(info->id_gpiod)) {
> dev_err(dev, "failed to get ID GPIO\n");
> @@ -155,6 +158,8 @@ static int usb_extcon_remove(struct plat
>
> cancel_delayed_work_sync(&info->wq_detcable);
>
> + gpiod_set_value_cansleep(info->enable_gpiod, 0);
> +
> return 0;
> }
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> |
|---|---|
| Date | 2015-12-11 15:50 +0100 |
| Message-ID | <qExbc-5Nm-23@gated-at.bofh.it> |
| In reply to | #1289119 |
Hello.
On 12/11/2015 07:05 AM, Chanwoo Choi wrote:
>> Sometimes there's a real OTG chip behind the USB ID signal mapped to a GPIO
>> pin: in my case it's Maxim Integrated MAX3355E which integrates Vbus charge
>> pump and comparators and passes thru the ID signal from an OTG connector.
>
> s/thru/through ?
"Thru" is valid English.
>> This chip also has the SHDN# pin which should be driven high for the normal
>> operation and low to save power; it is connected to a GPIO pin as well on,
>> hence we'll have to teach the driver to parse the new optional device tree
>> property, "enable-gpio"...
> This patch description includes the double space between words. Also, I think
So what?
> you need to write the patch description again for formal style.
Not sure I understand you.
> This patch adds the specific 'enable-gpio' pin to express the SHDN#pin for MAX3355E.
> I think it is not regular and standard case because maybe USB specification
> don't include the SHDN#pin information.
Certainly, it's not a USB pin.
> I think it not appropriate way.
> Instead, you better to make the MAX3355 extcon driver to support this case.
OK, just didn't want to duplicate most of this driver there...
> Thanks,
> Chanwoo
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web