All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] ohci-at91: add vbus_pin_inverted platform attribute
@ 2011-07-05  9:05 Thomas Petazzoni
  2011-07-05  9:05 ` [PATCH 2/3] at91-ohci: support overcurrent notification Thomas Petazzoni
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2011-07-05  9:05 UTC (permalink / raw)
  To: linux-arm-kernel

The existing OHCI AT91 driver made the assumption that the enable
input of the USB power switch was active low. However, some USB power
switches such as the Micrel MIC2026-1 [1] have an active high input to
enable the power. A new vbus_pin_inverted attribute is added to the
at91_usbh_data structure so that board files can tell the OHCI driver
if the vbus pin logic is active low or active high.

[1] http://www.micrel.com/page.do?page=product-info/products/mic2026.shtml

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Andrew Victor <linux@maxim.org.za>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
---
 arch/arm/mach-at91/include/mach/board.h |    1 +
 drivers/usb/host/ohci-at91.c            |    4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-at91/include/mach/board.h b/arch/arm/mach-at91/include/mach/board.h
index ed544a0..61d52dc 100644
--- a/arch/arm/mach-at91/include/mach/board.h
+++ b/arch/arm/mach-at91/include/mach/board.h
@@ -98,6 +98,7 @@ extern void __init at91_add_device_eth(struct at91_eth_data *data);
 struct at91_usbh_data {
 	u8		ports;		/* number of ports on root hub */
 	u8		vbus_pin[2];	/* port power-control pin */
+	u8              vbus_pin_inverted;
 };
 extern void __init at91_add_device_usbh(struct at91_usbh_data *data);
 extern void __init at91_add_device_usbh_ohci(struct at91_usbh_data *data);
diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 944291e..52e50ba 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -284,7 +284,7 @@ static int ohci_hcd_at91_drv_probe(struct platform_device *pdev)
 			if (pdata->vbus_pin[i] <= 0)
 				continue;
 			gpio_request(pdata->vbus_pin[i], "ohci_vbus");
-			gpio_direction_output(pdata->vbus_pin[i], 0);
+			gpio_direction_output(pdata->vbus_pin[i], 0 ^ pdata->vbus_pin_inverted);
 		}
 	}
 
@@ -301,7 +301,7 @@ static int ohci_hcd_at91_drv_remove(struct platform_device *pdev)
 		for (i = 0; i < ARRAY_SIZE(pdata->vbus_pin); i++) {
 			if (pdata->vbus_pin[i] <= 0)
 				continue;
-			gpio_direction_output(pdata->vbus_pin[i], 1);
+			gpio_direction_output(pdata->vbus_pin[i], 1 ^ pdata->vbus_pin_inverted);
 			gpio_free(pdata->vbus_pin[i]);
 		}
 	}
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/3] at91-ohci: support overcurrent notification
  2011-07-05  9:05 [PATCH 1/3] ohci-at91: add vbus_pin_inverted platform attribute Thomas Petazzoni
@ 2011-07-05  9:05 ` Thomas Petazzoni
  2011-07-05 10:12   ` Matthieu CASTET
  2011-07-05 14:23   ` Jean-Christophe PLAGNIOL-VILLARD
  2011-07-05  9:05 ` [PATCH 3/3] at91-ohci: configure overcurrent pins as GPIOs Thomas Petazzoni
  2011-07-05 11:39 ` [PATCH 1/3] ohci-at91: add vbus_pin_inverted platform attribute Sergei Shtylyov
  2 siblings, 2 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2011-07-05  9:05 UTC (permalink / raw)
  To: linux-arm-kernel

Several USB power switches (AIC1526 or MIC2026) have a digital output
that is used to notify that an overcurrent situation is taking
place. This digital outputs are typically connected to GPIO inputs of
the processor and can be used to be notified of those overcurrent
situations.

Therefore, we add a new overcurrent_pin[] array in the at91_usbh_data
structure so that boards can tell the AT91 OHCI driver which pins are
used for the overcurrent notification. The AT91 OHCI driver simply
registers an interrupt handler which will log the entry and exit of an
overcurrent situation in the kernel logs.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Andrew Victor <linux@maxim.org.za>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
---
 arch/arm/mach-at91/include/mach/board.h |    1 +
 drivers/usb/host/ohci-at91.c            |   37 +++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-at91/include/mach/board.h b/arch/arm/mach-at91/include/mach/board.h
index 61d52dc..1f90d79 100644
--- a/arch/arm/mach-at91/include/mach/board.h
+++ b/arch/arm/mach-at91/include/mach/board.h
@@ -99,6 +99,7 @@ struct at91_usbh_data {
 	u8		ports;		/* number of ports on root hub */
 	u8		vbus_pin[2];	/* port power-control pin */
 	u8              vbus_pin_inverted;
+	u8              overcurrent_pin[2]; /* over-current signal pins */
 };
 extern void __init at91_add_device_usbh(struct at91_usbh_data *data);
 extern void __init at91_add_device_usbh_ohci(struct at91_usbh_data *data);
diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 52e50ba..2801c77 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -269,6 +269,19 @@ static const struct hc_driver ohci_at91_hc_driver = {
 
 /*-------------------------------------------------------------------------*/
 
+static irqreturn_t ohci_hcd_at91_overcurrent_irq(int irq, void *data)
+{
+	struct platform_device *pdev = data;
+	int val;
+
+	val = gpio_get_value(irq_to_gpio(irq));
+	dev_err(& pdev->dev, "overcurrent situation %s\n",
+		val ? "exited" : "notified");
+	return IRQ_HANDLED;
+}
+
+/*-------------------------------------------------------------------------*/
+
 static int ohci_hcd_at91_drv_probe(struct platform_device *pdev)
 {
 	struct at91_usbh_data	*pdata = pdev->dev.platform_data;
@@ -286,6 +299,23 @@ static int ohci_hcd_at91_drv_probe(struct platform_device *pdev)
 			gpio_request(pdata->vbus_pin[i], "ohci_vbus");
 			gpio_direction_output(pdata->vbus_pin[i], 0 ^ pdata->vbus_pin_inverted);
 		}
+
+		for (i = 0; i < ARRAY_SIZE(pdata->overcurrent_pin); i++) {
+			int ret;
+
+			if (pdata->overcurrent_pin[i] <= 0)
+				continue;
+			gpio_request(pdata->overcurrent_pin[i], "ohci_overcurrent");
+			gpio_direction_input(pdata->overcurrent_pin[i]);
+
+			ret = request_irq(gpio_to_irq(pdata->overcurrent_pin[i]),
+					  ohci_hcd_at91_overcurrent_irq,
+					  IRQF_SHARED, "ohci_overcurrent", pdev);
+			if (ret) {
+				gpio_free(pdata->overcurrent_pin[i]);
+				dev_warn(& pdev->dev, "cannot get GPIO IRQ for overcurrent\n");
+			}
+		}
 	}
 
 	device_init_wakeup(&pdev->dev, 1);
@@ -304,6 +334,13 @@ static int ohci_hcd_at91_drv_remove(struct platform_device *pdev)
 			gpio_direction_output(pdata->vbus_pin[i], 1 ^ pdata->vbus_pin_inverted);
 			gpio_free(pdata->vbus_pin[i]);
 		}
+
+		for (i = 0; i < ARRAY_SIZE(pdata->overcurrent_pin); i++) {
+			if (pdata->overcurrent_pin[i] <= 0)
+				continue;
+			free_irq(gpio_to_irq(pdata->overcurrent_pin[i]), pdev);
+			gpio_free(pdata->overcurrent_pin[i]);
+		}
 	}
 
 	device_init_wakeup(&pdev->dev, 0);
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/3] at91-ohci: configure overcurrent pins as GPIOs
  2011-07-05  9:05 [PATCH 1/3] ohci-at91: add vbus_pin_inverted platform attribute Thomas Petazzoni
  2011-07-05  9:05 ` [PATCH 2/3] at91-ohci: support overcurrent notification Thomas Petazzoni
@ 2011-07-05  9:05 ` Thomas Petazzoni
  2011-07-05 11:39 ` [PATCH 1/3] ohci-at91: add vbus_pin_inverted platform attribute Sergei Shtylyov
  2 siblings, 0 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2011-07-05  9:05 UTC (permalink / raw)
  To: linux-arm-kernel

As a new overcurrent_pin[] array has been added to the at91_usbh_data
structure, those pins must be muxed to work properly. This commit
implements this muxing for all AT91 SoCs that support the AT91 OHCI.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Andrew Victor <linux@maxim.org.za>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
---
 arch/arm/mach-at91/at91cap9_devices.c    |    5 +++++
 arch/arm/mach-at91/at91rm9200_devices.c  |    5 +++++
 arch/arm/mach-at91/at91sam9260_devices.c |    5 +++++
 arch/arm/mach-at91/at91sam9261_devices.c |    5 +++++
 arch/arm/mach-at91/at91sam9263_devices.c |    5 +++++
 arch/arm/mach-at91/at91sam9g45_devices.c |    5 +++++
 6 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-at91/at91cap9_devices.c b/arch/arm/mach-at91/at91cap9_devices.c
index dba0d8d..db7173a 100644
--- a/arch/arm/mach-at91/at91cap9_devices.c
+++ b/arch/arm/mach-at91/at91cap9_devices.c
@@ -80,6 +80,11 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
 			at91_set_gpio_output(data->vbus_pin[i], 0);
 	}
 
+	for (i = 0; i < data->ports; i++) {
+		if (data->overcurrent_pin[i])
+			at91_set_GPIO_periph(data->overcurrent_pin[i], 1);
+	}
+
 	usbh_data = *data;
 	platform_device_register(&at91_usbh_device);
 }
diff --git a/arch/arm/mach-at91/at91rm9200_devices.c b/arch/arm/mach-at91/at91rm9200_devices.c
index 7227755..6a1d564 100644
--- a/arch/arm/mach-at91/at91rm9200_devices.c
+++ b/arch/arm/mach-at91/at91rm9200_devices.c
@@ -63,6 +63,11 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
 	if (!data)
 		return;
 
+	for (i = 0; i < data->ports; i++) {
+		if (data->overcurrent_pin[i])
+			at91_set_GPIO_periph(data->overcurrent_pin[i], 1);
+	}
+
 	usbh_data = *data;
 	platform_device_register(&at91rm9200_usbh_device);
 }
diff --git a/arch/arm/mach-at91/at91sam9260_devices.c b/arch/arm/mach-at91/at91sam9260_devices.c
index 39f81f4..3a77508 100644
--- a/arch/arm/mach-at91/at91sam9260_devices.c
+++ b/arch/arm/mach-at91/at91sam9260_devices.c
@@ -64,6 +64,11 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
 	if (!data)
 		return;
 
+	for (i = 0; i < data->ports; i++) {
+		if (data->overcurrent_pin[i])
+			at91_set_GPIO_periph(data->overcurrent_pin[i], 1);
+	}
+
 	usbh_data = *data;
 	platform_device_register(&at91_usbh_device);
 }
diff --git a/arch/arm/mach-at91/at91sam9261_devices.c b/arch/arm/mach-at91/at91sam9261_devices.c
index 5004bf0..57b3fa7 100644
--- a/arch/arm/mach-at91/at91sam9261_devices.c
+++ b/arch/arm/mach-at91/at91sam9261_devices.c
@@ -67,6 +67,11 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
 	if (!data)
 		return;
 
+	for (i = 0; i < data->ports; i++) {
+		if (data->overcurrent_pin[i])
+			at91_set_GPIO_periph(data->overcurrent_pin[i], 1);
+	}
+
 	usbh_data = *data;
 	platform_device_register(&at91sam9261_usbh_device);
 }
diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
index a050f41..f0fcc93 100644
--- a/arch/arm/mach-at91/at91sam9263_devices.c
+++ b/arch/arm/mach-at91/at91sam9263_devices.c
@@ -74,6 +74,11 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
 			at91_set_gpio_output(data->vbus_pin[i], 0);
 	}
 
+	for (i = 0; i < data->ports; i++) {
+		if (data->overcurrent_pin[i])
+			at91_set_GPIO_periph(data->overcurrent_pin[i], 1);
+	}
+
 	usbh_data = *data;
 	platform_device_register(&at91_usbh_device);
 }
diff --git a/arch/arm/mach-at91/at91sam9g45_devices.c b/arch/arm/mach-at91/at91sam9g45_devices.c
index 600bffb..4f2d774 100644
--- a/arch/arm/mach-at91/at91sam9g45_devices.c
+++ b/arch/arm/mach-at91/at91sam9g45_devices.c
@@ -124,6 +124,11 @@ void __init at91_add_device_usbh_ohci(struct at91_usbh_data *data)
 			at91_set_gpio_output(data->vbus_pin[i], 0);
 	}
 
+	for (i = 0; i < data->ports; i++) {
+		if (data->overcurrent_pin[i])
+			at91_set_GPIO_periph(data->overcurrent_pin[i], 1);
+	}
+
 	usbh_ohci_data = *data;
 	platform_device_register(&at91_usbh_ohci_device);
 }
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/3] at91-ohci: support overcurrent notification
  2011-07-05  9:05 ` [PATCH 2/3] at91-ohci: support overcurrent notification Thomas Petazzoni
@ 2011-07-05 10:12   ` Matthieu CASTET
  2011-07-05 10:18     ` Thomas Petazzoni
  2011-07-05 14:23   ` Jean-Christophe PLAGNIOL-VILLARD
  1 sibling, 1 reply; 15+ messages in thread
From: Matthieu CASTET @ 2011-07-05 10:12 UTC (permalink / raw)
  To: linux-arm-kernel

Thomas Petazzoni a ?crit :
> Several USB power switches (AIC1526 or MIC2026) have a digital output
> that is used to notify that an overcurrent situation is taking
> place. This digital outputs are typically connected to GPIO inputs of
> the processor and can be used to be notified of those overcurrent
> situations.
> 
> Therefore, we add a new overcurrent_pin[] array in the at91_usbh_data
> structure so that boards can tell the AT91 OHCI driver which pins are
> used for the overcurrent notification. The AT91 OHCI driver simply
> registers an interrupt handler which will log the entry and exit of an
> overcurrent situation in the kernel logs.
>
Why don't you forward the overcurrent notification to the usb stack ?
A printk is quite useless.



Matthieu

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/3] at91-ohci: support overcurrent notification
  2011-07-05 10:12   ` Matthieu CASTET
@ 2011-07-05 10:18     ` Thomas Petazzoni
  2011-07-05 12:18       ` Matthieu CASTET
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Petazzoni @ 2011-07-05 10:18 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

Le Tue, 5 Jul 2011 12:12:54 +0200,
Matthieu CASTET <matthieu.castet@parrot.com> a ?crit :

> Why don't you forward the overcurrent notification to the usb stack ?
> A printk is quite useless.

Do you have a pointer to the appropriate functions/mechanisms of the
USB stack about this ?

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/3] ohci-at91: add vbus_pin_inverted platform attribute
  2011-07-05  9:05 [PATCH 1/3] ohci-at91: add vbus_pin_inverted platform attribute Thomas Petazzoni
  2011-07-05  9:05 ` [PATCH 2/3] at91-ohci: support overcurrent notification Thomas Petazzoni
  2011-07-05  9:05 ` [PATCH 3/3] at91-ohci: configure overcurrent pins as GPIOs Thomas Petazzoni
@ 2011-07-05 11:39 ` Sergei Shtylyov
  2011-07-05 11:54   ` Thomas Petazzoni
  2 siblings, 1 reply; 15+ messages in thread
From: Sergei Shtylyov @ 2011-07-05 11:39 UTC (permalink / raw)
  To: linux-arm-kernel

Hello.

On 05-07-2011 13:05, Thomas Petazzoni wrote:

> The existing OHCI AT91 driver made the assumption that the enable
> input of the USB power switch was active low. However, some USB power
> switches such as the Micrel MIC2026-1 [1] have an active high input to
> enable the power. A new vbus_pin_inverted attribute is added to the
> at91_usbh_data structure so that board files can tell the OHCI driver
> if the vbus pin logic is active low or active high.

> [1] http://www.micrel.com/page.do?page=product-info/products/mic2026.shtml

> Signed-off-by: Thomas Petazzoni<thomas.petazzoni@free-electrons.com>
> Cc: Andrew Victor<linux@maxim.org.za>
> Cc: Nicolas Ferre<nicolas.ferre@atmel.com>
> Cc: Jean-Christophe Plagniol-Villard<plagnioj@jcrosoft.com>
[...]

> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> index 944291e..52e50ba 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -284,7 +284,7 @@ static int ohci_hcd_at91_drv_probe(struct platform_device *pdev)
>   			if (pdata->vbus_pin[i]<= 0)
>   				continue;
>   			gpio_request(pdata->vbus_pin[i], "ohci_vbus");
> -			gpio_direction_output(pdata->vbus_pin[i], 0);
> +			gpio_direction_output(pdata->vbus_pin[i], 0 ^ pdata->vbus_pin_inverted);

    Why not simply 'pdata->vbus_pin_inverted'?

>   		}
>   	}
>
> @@ -301,7 +301,7 @@ static int ohci_hcd_at91_drv_remove(struct platform_device *pdev)
>   		for (i = 0; i<  ARRAY_SIZE(pdata->vbus_pin); i++) {
>   			if (pdata->vbus_pin[i]<= 0)
>   				continue;
> -			gpio_direction_output(pdata->vbus_pin[i], 1);
> +			gpio_direction_output(pdata->vbus_pin[i], 1 ^ pdata->vbus_pin_inverted);

    Why not simply '!pdata->vbus_pin_inverted'?

WBR, Sergei

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/3] ohci-at91: add vbus_pin_inverted platform attribute
  2011-07-05 11:39 ` [PATCH 1/3] ohci-at91: add vbus_pin_inverted platform attribute Sergei Shtylyov
@ 2011-07-05 11:54   ` Thomas Petazzoni
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2011-07-05 11:54 UTC (permalink / raw)
  To: linux-arm-kernel

Le Tue, 05 Jul 2011 15:39:18 +0400,
Sergei Shtylyov <sshtylyov@mvista.com> a ?crit :

> > -			gpio_direction_output(pdata->vbus_pin[i], 0);
> > +			gpio_direction_output(pdata->vbus_pin[i], 0 ^ pdata->vbus_pin_inverted);
> 
>     Why not simply 'pdata->vbus_pin_inverted'?

> > -			gpio_direction_output(pdata->vbus_pin[i], 1);
> > +			gpio_direction_output(pdata->vbus_pin[i], 1 ^ pdata->vbus_pin_inverted);
> 
>     Why not simply '!pdata->vbus_pin_inverted'?

Ah, correct. The 0 and 1 were present before, and I just wanted to xor
them with vbus_pin_inverted, but obviously, there's a simpler way of
writing things, as you suggest.

I'll update my patch and send a new version after gathering comments
from other reviewers.

Thanks!

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/3] at91-ohci: support overcurrent notification
  2011-07-05 10:18     ` Thomas Petazzoni
@ 2011-07-05 12:18       ` Matthieu CASTET
  0 siblings, 0 replies; 15+ messages in thread
From: Matthieu CASTET @ 2011-07-05 12:18 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

Thomas Petazzoni a ?crit :
> Hello,
> 
> Le Tue, 5 Jul 2011 12:12:54 +0200,
> Matthieu CASTET <matthieu.castet@parrot.com> a ?crit :
> 
>> Why don't you forward the overcurrent notification to the usb stack ?
>> A printk is quite useless.
> 
> Do you have a pointer to the appropriate functions/mechanisms of the
> USB stack about this ?

For ohci you have to override hub_status_data and hub_control callback.

You can look how it is done in ohci-da8xx.c / ohci-s3c2410.c driver.

May be (or not) it could be useful to introduce a generic function to do all the
parsing stuff.


Matthieu

PS : Add linux-usb to CC

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/3] at91-ohci: support overcurrent notification
  2011-07-05  9:05 ` [PATCH 2/3] at91-ohci: support overcurrent notification Thomas Petazzoni
  2011-07-05 10:12   ` Matthieu CASTET
@ 2011-07-05 14:23   ` Jean-Christophe PLAGNIOL-VILLARD
  1 sibling, 0 replies; 15+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2011-07-05 14:23 UTC (permalink / raw)
  To: linux-arm-kernel

On 11:05 Tue 05 Jul     , Thomas Petazzoni wrote:
> Several USB power switches (AIC1526 or MIC2026) have a digital output
> that is used to notify that an overcurrent situation is taking
> place. This digital outputs are typically connected to GPIO inputs of
> the processor and can be used to be notified of those overcurrent
> situations.
> 
> Therefore, we add a new overcurrent_pin[] array in the at91_usbh_data
> structure so that boards can tell the AT91 OHCI driver which pins are
> used for the overcurrent notification. The AT91 OHCI driver simply
> registers an interrupt handler which will log the entry and exit of an
> overcurrent situation in the kernel logs.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Andrew Victor <linux@maxim.org.za>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> ---
>  arch/arm/mach-at91/include/mach/board.h |    1 +
>  drivers/usb/host/ohci-at91.c            |   37 +++++++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/include/mach/board.h b/arch/arm/mach-at91/include/mach/board.h
> index 61d52dc..1f90d79 100644
> --- a/arch/arm/mach-at91/include/mach/board.h
> +++ b/arch/arm/mach-at91/include/mach/board.h
> @@ -99,6 +99,7 @@ struct at91_usbh_data {
>  	u8		ports;		/* number of ports on root hub */
>  	u8		vbus_pin[2];	/* port power-control pin */
>  	u8              vbus_pin_inverted;
> +	u8              overcurrent_pin[2]; /* over-current signal pins */
please use named resourcees and provide the irq number

Best Regards,
J.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/3] at91-ohci: support overcurrent notification
  2011-07-13  9:29 ` [PATCH 2/3] at91-ohci: support overcurrent notification Thomas Petazzoni
  2011-07-13 14:28   ` Thomas Petazzoni
@ 2011-09-07 10:47   ` Nicolas Ferre
  1 sibling, 0 replies; 15+ messages in thread
From: Nicolas Ferre @ 2011-09-07 10:47 UTC (permalink / raw)
  To: linux-arm-kernel

Le 13/07/2011 11:29, Thomas Petazzoni :
> Several USB power switches (AIC1526 or MIC2026) have a digital output
> that is used to notify that an overcurrent situation is taking
> place. This digital outputs are typically connected to GPIO inputs of
> the processor and can be used to be notified of those overcurrent
> situations.
> 
> Therefore, we add a new overcurrent_pin[] array in the at91_usbh_data
> structure so that boards can tell the AT91 OHCI driver which pins are
> used for the overcurrent notification, and an overcurrent_supported
> boolean to tell the driver whether overcurrent is supported or not.
> 
> The code has been largely borrowed from ohci-da8xx.c and
> ohci-s3c2410.c.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Andrew Victor <linux@maxim.org.za>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Matthieu CASTET <matthieu.castet@parrot.com>
> ---
>  arch/arm/mach-at91/include/mach/board.h |    4 +
>  drivers/usb/host/ohci-at91.c            |  224 +++++++++++++++++++++++++++++--
>  2 files changed, 219 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/include/mach/board.h b/arch/arm/mach-at91/include/mach/board.h
> index 61d52dc..d07767f 100644
> --- a/arch/arm/mach-at91/include/mach/board.h
> +++ b/arch/arm/mach-at91/include/mach/board.h
> @@ -99,6 +99,10 @@ struct at91_usbh_data {
>  	u8		ports;		/* number of ports on root hub */
>  	u8		vbus_pin[2];	/* port power-control pin */
>  	u8              vbus_pin_inverted;
> +	u8              overcurrent_supported;
> +	u8              overcurrent_pin[2];
> +	u8              overcurrent_status[2];
> +	u8              overcurrent_changed[2];
>  };
>  extern void __init at91_add_device_usbh(struct at91_usbh_data *data);
>  extern void __init at91_add_device_usbh_ohci(struct at91_usbh_data *data);
> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> index 3612ccd..331909f 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -223,6 +223,156 @@ ohci_at91_start (struct usb_hcd *hcd)
>  	return 0;
>  }
>  
> +static void ohci_at91_usb_set_power(struct at91_usbh_data *pdata, int port, int enable)
> +{
> +	if (port < 0 || port >= 2)
> +		return;
> +
> +	gpio_set_value(pdata->vbus_pin[port], !pdata->vbus_pin_inverted ^ enable);
> +}
> +
> +static int ohci_at91_usb_get_power(struct at91_usbh_data *pdata, int port)
> +{
> +	if (port < 0 || port >= 2)
> +		return -EINVAL;
> +
> +	return gpio_get_value(pdata->vbus_pin[port]) ^ !pdata->vbus_pin_inverted;
> +}
> +
> +/*
> + * Update the status data from the hub with the over-current indicator change.
> + */
> +static int ohci_at91_hub_status_data(struct usb_hcd *hcd, char *buf)
> +{
> +	struct at91_usbh_data *pdata = hcd->self.controller->platform_data;
> +	int length = ohci_hub_status_data(hcd, buf);
> +	int port;
> +
> +	for (port = 0; port < ARRAY_SIZE(pdata->overcurrent_pin); port++) {
> +		if (pdata->overcurrent_changed[port]) {
> +			if (! length)
> +				length = 1;
> +			buf[0] |= 1 << (port + 1);
> +		}
> +	}
> +
> +	return length;
> +}
> +
> +/*
> + * Look at the control requests to the root hub and see if we need to override.
> + */
> +static int ohci_at91_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
> +				 u16 wIndex, char *buf, u16 wLength)
> +{
> +	struct at91_usbh_data *pdata = hcd->self.controller->platform_data;
> +	struct usb_hub_descriptor *desc;
> +	int ret = -EINVAL;
> +	u32 *data = (u32 *)buf;
> +
> +	dev_dbg(hcd->self.controller,
> +		"ohci_at91_hub_control(%p,0x%04x,0x%04x,0x%04x,%p,%04x)\n",
> +		hcd, typeReq, wValue, wIndex, buf, wLength);
> +
> +	switch (typeReq) {
> +	case SetPortFeature:
> +		if (wValue == USB_PORT_FEAT_POWER) {
> +			dev_dbg(hcd->self.controller, "SetPortFeat: POWER\n");
> +			ohci_at91_usb_set_power(pdata, wIndex - 1, 1);
> +			goto out;
> +		}
> +		break;
> +
> +	case ClearPortFeature:
> +		switch (wValue) {
> +		case USB_PORT_FEAT_C_OVER_CURRENT:
> +			dev_dbg(hcd->self.controller,
> +				"ClearPortFeature: C_OVER_CURRENT\n");
> +
> +			if (wIndex == 1 || wIndex == 2) {
> +				pdata->overcurrent_changed[wIndex-1] = 0;
> +				pdata->overcurrent_status[wIndex-1] = 0;
> +			}
> +
> +			goto out;
> +
> +		case USB_PORT_FEAT_OVER_CURRENT:
> +			dev_dbg(hcd->self.controller,
> +				"ClearPortFeature: OVER_CURRENT\n");
> +
> +			if (wIndex == 1 || wIndex == 2) {
> +				pdata->overcurrent_status[wIndex-1] = 0;
> +			}
> +
> +			goto out;
> +
> +		case USB_PORT_FEAT_POWER:
> +			dev_dbg(hcd->self.controller,
> +				"ClearPortFeature: POWER\n");
> +
> +			if (wIndex == 1 || wIndex == 2) {
> +				ohci_at91_usb_set_power(pdata, wIndex - 1, 0);
> +				return 0;
> +			}
> +		}
> +		break;
> +	}
> +
> +	ret = ohci_hub_control(hcd, typeReq, wValue, wIndex, buf, wLength);
> +	if (ret)
> +		goto out;
> +
> +	switch (typeReq) {
> +	case GetHubDescriptor:
> +
> +		/* update the hub's descriptor */
> +
> +		desc = (struct usb_hub_descriptor *)buf;
> +
> +		dev_dbg(hcd->self.controller, "wHubCharacteristics 0x%04x\n",
> +			desc->wHubCharacteristics);
> +
> +		/* remove the old configurations for power-switching, and
> +		 * over-current protection, and insert our new configuration
> +		 */
> +
> +		desc->wHubCharacteristics &= ~cpu_to_le16(HUB_CHAR_LPSM);
> +		desc->wHubCharacteristics |= cpu_to_le16(0x0001);
> +
> +		if (pdata->overcurrent_supported) {
> +			desc->wHubCharacteristics &= ~cpu_to_le16(HUB_CHAR_OCPM);
> +			desc->wHubCharacteristics |=  cpu_to_le16(0x0008|0x0001);
> +		}
> +
> +		dev_dbg(hcd->self.controller, "wHubCharacteristics after 0x%04x\n",
> +			desc->wHubCharacteristics);
> +
> +		return ret;
> +
> +	case GetPortStatus:
> +		/* check port status */
> +
> +		dev_dbg(hcd->self.controller, "GetPortStatus(%d)\n", wIndex);
> +
> +		if (wIndex == 1 || wIndex == 2) {
> +			if (! ohci_at91_usb_get_power(pdata, wIndex-1)) {
> +				*data &= ~cpu_to_le32(RH_PS_PPS);
> +			}
> +
> +			if (pdata->overcurrent_changed[wIndex-1]) {
> +				*data |= cpu_to_le32(RH_PS_OCIC);
> +			}
> +
> +			if (pdata->overcurrent_status[wIndex-1]) {
> +				*data |= cpu_to_le32(RH_PS_POCI);
> +			}
> +		}
> +	}
> +
> + out:
> +	return ret;
> +}
> +
>  /*-------------------------------------------------------------------------*/
>  
>  static const struct hc_driver ohci_at91_hc_driver = {
> @@ -258,8 +408,8 @@ static const struct hc_driver ohci_at91_hc_driver = {
>  	/*
>  	 * root hub support
>  	 */
> -	.hub_status_data =	ohci_hub_status_data,
> -	.hub_control =		ohci_hub_control,
> +	.hub_status_data =	ohci_at91_hub_status_data,
> +	.hub_control =		ohci_at91_hub_control,
>  #ifdef CONFIG_PM
>  	.bus_suspend =		ohci_bus_suspend,
>  	.bus_resume =		ohci_bus_resume,
> @@ -269,22 +419,71 @@ static const struct hc_driver ohci_at91_hc_driver = {
>  
>  /*-------------------------------------------------------------------------*/
>  
> +static irqreturn_t ohci_hcd_at91_overcurrent_irq(int irq, void *data)
> +{
> +	struct platform_device *pdev = data;
> +	struct at91_usbh_data *pdata = pdev->dev.platform_data;
> +	int val, gpio, port;
> +
> +	/* From the GPIO notifying the over-current situation, find
> +	 * out the corresponding port */
> +	gpio = irq_to_gpio(irq);
> +	for (port = 0; port < ARRAY_SIZE(pdata->overcurrent_pin); port++) {
> +		if (pdata->overcurrent_pin[port] == gpio)
> +			break;
> +	}
> +
> +	if (port == ARRAY_SIZE(pdata->overcurrent_pin)) {
> +		dev_err(& pdev->dev, "overcurrent interrupt from unknown GPIO\n");
> +		return IRQ_HANDLED;
> +	}
> +
> +	val = gpio_get_value(gpio);
> +
> +	/* When notified of an over-current situation, disable power
> +	   on the corresponding port, and mark this port in
> +	   over-current. */
> +	if (! val) {
> +		ohci_at91_usb_set_power(pdata, port, 0);
> +		pdata->overcurrent_status[port]  = 1;
> +		pdata->overcurrent_changed[port] = 1;
> +	}
> +
> +	dev_dbg(& pdev->dev, "overcurrent situation %s\n",
> +		val ? "exited" : "notified");
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/*-------------------------------------------------------------------------*/
> +
>  static int ohci_hcd_at91_drv_probe(struct platform_device *pdev)
>  {
>  	struct at91_usbh_data	*pdata = pdev->dev.platform_data;
>  	int			i;
>  
>  	if (pdata) {
> -		/* REVISIT make the driver support per-port power switching,
> -		 * and also overcurrent detection.  Here we assume the ports
> -		 * are always powered while this driver is active, and use
> -		 * active-low power switches.
> -		 */
>  		for (i = 0; i < ARRAY_SIZE(pdata->vbus_pin); i++) {
>  			if (pdata->vbus_pin[i] <= 0)
>  				continue;
>  			gpio_request(pdata->vbus_pin[i], "ohci_vbus");
> -			gpio_direction_output(pdata->vbus_pin[i], pdata->vbus_pin_inverted);
> +			ohci_at91_usb_set_power(pdata, i, 1);
> +		}
> +
> +		for (i = 0; i < ARRAY_SIZE(pdata->overcurrent_pin); i++) {
> +			int ret;
> +
> +			if (pdata->overcurrent_pin[i] <= 0)
> +				continue;
> +			gpio_request(pdata->overcurrent_pin[i], "ohci_overcurrent");
> +
> +			ret = request_irq(gpio_to_irq(pdata->overcurrent_pin[i]),
> +					  ohci_hcd_at91_overcurrent_irq,
> +					  IRQF_SHARED, "ohci_overcurrent", pdev);
> +			if (ret) {
> +				gpio_free(pdata->overcurrent_pin[i]);
> +				dev_warn(& pdev->dev, "cannot get GPIO IRQ for overcurrent\n");
> +			}
>  		}
>  	}
>  
> @@ -301,9 +500,16 @@ static int ohci_hcd_at91_drv_remove(struct platform_device *pdev)
>  		for (i = 0; i < ARRAY_SIZE(pdata->vbus_pin); i++) {
>  			if (pdata->vbus_pin[i] <= 0)
>  				continue;
> -			gpio_direction_output(pdata->vbus_pin[i], !pdata->vbus_pin_inverted);
> +			ohci_at91_usb_set_power(pdata, i, 0);
>  			gpio_free(pdata->vbus_pin[i]);
>  		}
> +
> +		for (i = 0; i < ARRAY_SIZE(pdata->overcurrent_pin); i++) {
> +			if (pdata->overcurrent_pin[i] <= 0)
> +				continue;
> +			free_irq(gpio_to_irq(pdata->overcurrent_pin[i]), pdev);
> +			gpio_free(pdata->overcurrent_pin[i]);
> +		}
>  	}
>  
>  	device_init_wakeup(&pdev->dev, 0);


-- 
Nicolas Ferre

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/3] at91-ohci: support overcurrent notification
  2011-07-13 16:43       ` Thomas Petazzoni
@ 2011-07-13 17:17         ` Alan Stern
  0 siblings, 0 replies; 15+ messages in thread
From: Alan Stern @ 2011-07-13 17:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 13 Jul 2011, Thomas Petazzoni wrote:

> Le Wed, 13 Jul 2011 11:54:15 -0400 (EDT),
> Alan Stern <stern@rowland.harvard.edu> a ?crit :
> 
> > > So, according to step 3), I would have expected the USB core to wait
> > > for the over-currrent status to be cleared, that is, wait until I
> > > release the button.
> > 
> > If there is no power to the port, the attached device can't draw any
> > current.  Right?  Therefore the port's over-current status isn't
> > meaningful until the power is restored.
> 
> Right, but as you highlighted below, as soon as you power on the port
> again, if the same device is still plugged into the port, then you might
> end up in the same over-current situation, and this will loop forever.

Potentially yes, that could happen.  Nobody has ever complained about 
seeing it, though.

In general our handling of this situation probably is not adequate.  
However it has never received proper testing.  You can review the git
history of the hub.c file; there have been some recent changes to this
code.

> > > Below is the log of what happens between the moment I press the button
> > > (message "overcurrent situation notified" from the GPIO interrupt
> > > handler) and the moment I release the button (message "overcurrent
> > > situation exited" from the GPIO interrupt handler).
> > > 
> > > [   41.750000] at91_ohci at91_ohci: overcurrent situation notified
> > > [   41.790000] hub 1-0:1.0: state 7 ports 2 chg 0000 evt 0006
> > > [   41.790000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0001,c7861e48,0004)
> > > [   41.790000] at91_ohci at91_ohci: GetPortStatus(1)
> > > [   41.790000] hub 1-0:1.0: over-current change on port 1
> > > [   41.790000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2301,0x0013,0x0001,c7861e70,0000)
> > > [   41.790000] at91_ohci at91_ohci: ClearPortFeature: C_OVER_CURRENT
> > > [   41.790000] at91_ohci at91_ohci: CTRL: TypeReq=0x2301 val=0x13 idx=0x1 len=0 ==> -22
> > > [   41.900000] hub 1-0:1.0: enabling power on all ports
> > > [   41.900000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2303,0x0008,0x0001,c7861e58,0000)
> > > [   41.900000] at91_ohci at91_ohci: SetPortFeat: POWER
> > > [   41.900000] at91_ohci at91_ohci: CTRL: TypeReq=0x2303 val=0x8 idx=0x1 len=0 ==> -22
> > > [   41.900000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2303,0x0008,0x0002,c7861e58,0000)
> > > [   41.900000] at91_ohci at91_ohci: SetPortFeat: POWER
> > > [   41.900000] at91_ohci at91_ohci: CTRL: TypeReq=0x2303 val=0x8 idx=0x2 len=0 ==> -22
> > > [   42.010000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0001,c7861e48,0004)
> > > [   42.010000] at91_ohci at91_ohci: GetPortStatus(1)
> > > [   42.010000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861e48,0004)
> > > [   42.010000] at91_ohci at91_ohci: GetStatus roothub.portstatus [1] = 0x00030301 PESC CSC LSDA PPS CCS
> > 
> > At this point the "over-current condition on port 1" error message
> > should have appeared, and the power to port 1 should have been turned
> > off again by the hardware.
> 
> I'm not sure to follow you here. If the message did not appear, it
> indicates that the USB_PORT_STAT_OVERCURRENT flag wasn't set, for some
> reason.

Right.  Why wasn't the flag set?  Was it because the port power had
already been turned back off?  That's not what the log indicates.

> Regarding the power being turned off, it's already done by the GPIO
> interrupt handler that notifies of the beginning of an over-current
> situation :
> 
> static irqreturn_t ohci_hcd_at91_overcurrent_irq(int irq, void *data)
> {
> [...]
>         val = gpio_get_value(gpio);
> 
>         /* When notified of an over-current situation, disable power
>            on the corresponding port, and mark this port in
>            over-current. */
>         if (! val) {
>                 ohci_at91_usb_set_power(pdata, port, 0);
>                 pdata->overcurrent_status[port]  = 1;
>                 pdata->overcurrent_changed[port] = 1;
>         }
> [...]
> 
> Isn't this correct ?

I have no idea; I don't know how the AT91 works.  When does the 
overcurrent_status value get cleared?  Under what conditions does that 
IRQ fire?

> > > Could you enlighten me on how this over-current mechanism is supposed
> > > to work ?
> > 
> > We don't have any good way of waiting for the over-current status to
> > clear, because the hub driver is single-threaded.  It wouldn't be able
> > to respond to any other USB events while waiting.
> 
> Ok. I guess it would be possible to regularly poll for the over-current
> flag and see whether things are improving (and between the polls,
> handle all other USB events). But as you said above, as soon as you
> power off the port, the over-current situation should have disappeared.
> What worries me is that the over-current situation will likely take
> place again as soon as you power on the port again.
> 
> In the end, is the behavior that I see the normal behavior of
> over-current management as supported currently by the kernel?

More or less, yes.  We are always willing to consider suggestions for
improvements, especially when accompanied by patches.

Alan Stern

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/3] at91-ohci: support overcurrent notification
  2011-07-13 15:54     ` Alan Stern
@ 2011-07-13 16:43       ` Thomas Petazzoni
  2011-07-13 17:17         ` Alan Stern
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Petazzoni @ 2011-07-13 16:43 UTC (permalink / raw)
  To: linux-arm-kernel

Le Wed, 13 Jul 2011 11:54:15 -0400 (EDT),
Alan Stern <stern@rowland.harvard.edu> a ?crit :

> > So, according to step 3), I would have expected the USB core to wait
> > for the over-currrent status to be cleared, that is, wait until I
> > release the button.
> 
> If there is no power to the port, the attached device can't draw any
> current.  Right?  Therefore the port's over-current status isn't
> meaningful until the power is restored.

Right, but as you highlighted below, as soon as you power on the port
again, if the same device is still plugged into the port, then you might
end up in the same over-current situation, and this will loop forever.

> > Below is the log of what happens between the moment I press the button
> > (message "overcurrent situation notified" from the GPIO interrupt
> > handler) and the moment I release the button (message "overcurrent
> > situation exited" from the GPIO interrupt handler).
> > 
> > [   41.750000] at91_ohci at91_ohci: overcurrent situation notified
> > [   41.790000] hub 1-0:1.0: state 7 ports 2 chg 0000 evt 0006
> > [   41.790000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0001,c7861e48,0004)
> > [   41.790000] at91_ohci at91_ohci: GetPortStatus(1)
> > [   41.790000] hub 1-0:1.0: over-current change on port 1
> > [   41.790000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2301,0x0013,0x0001,c7861e70,0000)
> > [   41.790000] at91_ohci at91_ohci: ClearPortFeature: C_OVER_CURRENT
> > [   41.790000] at91_ohci at91_ohci: CTRL: TypeReq=0x2301 val=0x13 idx=0x1 len=0 ==> -22
> > [   41.900000] hub 1-0:1.0: enabling power on all ports
> > [   41.900000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2303,0x0008,0x0001,c7861e58,0000)
> > [   41.900000] at91_ohci at91_ohci: SetPortFeat: POWER
> > [   41.900000] at91_ohci at91_ohci: CTRL: TypeReq=0x2303 val=0x8 idx=0x1 len=0 ==> -22
> > [   41.900000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2303,0x0008,0x0002,c7861e58,0000)
> > [   41.900000] at91_ohci at91_ohci: SetPortFeat: POWER
> > [   41.900000] at91_ohci at91_ohci: CTRL: TypeReq=0x2303 val=0x8 idx=0x2 len=0 ==> -22
> > [   42.010000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0001,c7861e48,0004)
> > [   42.010000] at91_ohci at91_ohci: GetPortStatus(1)
> > [   42.010000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861e48,0004)
> > [   42.010000] at91_ohci at91_ohci: GetStatus roothub.portstatus [1] = 0x00030301 PESC CSC LSDA PPS CCS
> 
> At this point the "over-current condition on port 1" error message
> should have appeared, and the power to port 1 should have been turned
> off again by the hardware.

I'm not sure to follow you here. If the message did not appear, it
indicates that the USB_PORT_STAT_OVERCURRENT flag wasn't set, for some
reason.

Regarding the power being turned off, it's already done by the GPIO
interrupt handler that notifies of the beginning of an over-current
situation :

static irqreturn_t ohci_hcd_at91_overcurrent_irq(int irq, void *data)
{
[...]
        val = gpio_get_value(gpio);

        /* When notified of an over-current situation, disable power
           on the corresponding port, and mark this port in
           over-current. */
        if (! val) {
                ohci_at91_usb_set_power(pdata, port, 0);
                pdata->overcurrent_status[port]  = 1;
                pdata->overcurrent_changed[port] = 1;
        }
[...]

Isn't this correct ?

> > Could you enlighten me on how this over-current mechanism is supposed
> > to work ?
> 
> We don't have any good way of waiting for the over-current status to
> clear, because the hub driver is single-threaded.  It wouldn't be able
> to respond to any other USB events while waiting.

Ok. I guess it would be possible to regularly poll for the over-current
flag and see whether things are improving (and between the polls,
handle all other USB events). But as you said above, as soon as you
power off the port, the over-current situation should have disappeared.
What worries me is that the over-current situation will likely take
place again as soon as you power on the port again.

In the end, is the behavior that I see the normal behavior of
over-current management as supported currently by the kernel?

Regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/3] at91-ohci: support overcurrent notification
  2011-07-13 14:28   ` Thomas Petazzoni
@ 2011-07-13 15:54     ` Alan Stern
  2011-07-13 16:43       ` Thomas Petazzoni
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Stern @ 2011-07-13 15:54 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 13 Jul 2011, Thomas Petazzoni wrote:

> Now, I have a question about the behavior I observe with this code in
> place. In order to easily trigger the over-current situation, I have
> told in my board code that a button is the GPIO for the overcurrent_pin.
> 
> When I push the button, I enter the over-current situation, when I
> release the button, I exit the over-current situation.
> 
> With the code above in place, when I push the button, the power is shut
> off on the corresponding USB port and the device goes away. But
> immediately after that, the power is switched on at the USB port by the
> USB core, and the device is detected again. Is this normal behaviour ?
> I would have expected the USB core to wait for the over-current
> situation to disappear (i.e from me releasing the button). Maybe I am
> misunderstanding how over-current management works ?
> 
> Apparently, the behaviour I observe is implemented by the following
> piece of code in drivers/usb/core/hub.c :
> 
>                         if (portchange & USB_PORT_STAT_C_OVERCURRENT) {
>                                 u16 status = 0;
>                                 u16 unused;
> 
>                                 dev_dbg(hub_dev, "over-current change on port "
>                                         "%d\n", i);
>                                 clear_port_feature(hdev, i,
>                                         USB_PORT_FEAT_C_OVER_CURRENT);
>                                 msleep(100);    /* Cool down */
>                                 hub_power_on(hub, true);
>                                 hub_port_status(hub, i, &status, &unused);
>                                 if (status & USB_PORT_STAT_OVERCURRENT)
>                                         dev_err(hub_dev, "over-current "
>                                                 "condition on port %d\n", i);
>                         }
> 
> So I don't see where it would wait for the over-current situation to be
> cleared.

That's what the "msleep(100)" is for.

>  However, the USB 2.0 specification says, in section 11.12.5 :
> 
> """
> Host recovery actions for an over-current event should include the
> following:
> 1. Host gets change notification from hub with over-current
>    event.
> 2. Host extracts appropriate hub or port change information
>    (depending on the information in the change bitmap).
> 3. Host waits for over-current status bit to be cleared to 0.
> 4. Host cycles power on to all of the necessary ports (e.g., issues a
>    SetPortFeature(PORT_POWER) request for each port).
> 5. Host re-enumerates all affected ports
> """
> 
> So, according to step 3), I would have expected the USB core to wait
> for the over-currrent status to be cleared, that is, wait until I
> release the button.

If there is no power to the port, the attached device can't draw any
current.  Right?  Therefore the port's over-current status isn't
meaningful until the power is restored.

> Below is the log of what happens between the moment I press the button
> (message "overcurrent situation notified" from the GPIO interrupt
> handler) and the moment I release the button (message "overcurrent
> situation exited" from the GPIO interrupt handler).
> 
> [   41.750000] at91_ohci at91_ohci: overcurrent situation notified
> [   41.790000] hub 1-0:1.0: state 7 ports 2 chg 0000 evt 0006
> [   41.790000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0001,c7861e48,0004)
> [   41.790000] at91_ohci at91_ohci: GetPortStatus(1)
> [   41.790000] hub 1-0:1.0: over-current change on port 1
> [   41.790000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2301,0x0013,0x0001,c7861e70,0000)
> [   41.790000] at91_ohci at91_ohci: ClearPortFeature: C_OVER_CURRENT
> [   41.790000] at91_ohci at91_ohci: CTRL: TypeReq=0x2301 val=0x13 idx=0x1 len=0 ==> -22
> [   41.900000] hub 1-0:1.0: enabling power on all ports
> [   41.900000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2303,0x0008,0x0001,c7861e58,0000)
> [   41.900000] at91_ohci at91_ohci: SetPortFeat: POWER
> [   41.900000] at91_ohci at91_ohci: CTRL: TypeReq=0x2303 val=0x8 idx=0x1 len=0 ==> -22
> [   41.900000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2303,0x0008,0x0002,c7861e58,0000)
> [   41.900000] at91_ohci at91_ohci: SetPortFeat: POWER
> [   41.900000] at91_ohci at91_ohci: CTRL: TypeReq=0x2303 val=0x8 idx=0x2 len=0 ==> -22
> [   42.010000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0001,c7861e48,0004)
> [   42.010000] at91_ohci at91_ohci: GetPortStatus(1)
> [   42.010000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861e48,0004)
> [   42.010000] at91_ohci at91_ohci: GetStatus roothub.portstatus [1] = 0x00030301 PESC CSC LSDA PPS CCS

At this point the "over-current condition on port 1" error message
should have appeared, and the power to port 1 should have been turned
off again by the hardware.

> [   42.010000] at91_ohci at91_ohci: GetPortStatus(2)
> [   42.010000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2301,0x0010,0x0002,c7861e70,0000)
> [   42.010000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2301,0x0011,0x0002,c7861e70,0000)
> [   42.010000] hub 1-0:1.0: port 2, status 0301, change 0003, 1.5 Mb/s
> [   42.010000] usb 1-2: USB disconnect, device number 2
> [   42.010000] usb 1-2: unregistering device
> [   42.010000] usb 1-2: unregistering interface 1-2:1.0
> [   42.010000] usb 1-2: usb_disable_device nuking all URBs
> [   42.010000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861e48,0004)
> [   42.010000] at91_ohci at91_ohci: GetPortStatus(2)
> [   42.050000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861e48,0004)
> [   42.050000] at91_ohci at91_ohci: GetPortStatus(2)
> [   42.090000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861e48,0004)
> [   42.090000] at91_ohci at91_ohci: GetPortStatus(2)
> [   42.130000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861e48,0004)
> [   42.130000] at91_ohci at91_ohci: GetPortStatus(2)
> [   42.170000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861e48,0004)
> [   42.170000] at91_ohci at91_ohci: GetPortStatus(2)
> [   42.170000] hub 1-0:1.0: debounce: port 2: total 100ms stable 100ms status 0x301
> [   42.170000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2303,0x0004,0x0002,c7861de0,0000)
> [   42.290000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861db8,0004)
> [   42.290000] at91_ohci at91_ohci: GetStatus roothub.portstatus [1] = 0x00100303 PRSC LSDA PPS PES CCS
> [   42.290000] at91_ohci at91_ohci: GetPortStatus(2)
> [   42.350000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2301,0x0014,0x0002,c7861de0,0000)
> [   42.350000] usb 1-2: new low speed USB device number 3 using at91_ohci
> [   42.350000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2303,0x0004,0x0002,c7861de0,0000)
> [   42.470000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861db8,0004)
> [   42.470000] at91_ohci at91_ohci: GetStatus roothub.portstatus [1] = 0x00100303 PRSC LSDA PPS PES CCS
> [   42.470000] at91_ohci at91_ohci: GetPortStatus(2)
> [   42.530000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2301,0x0014,0x0002,c7861de0,0000)
> [   42.560000] usb 1-2: skipped 1 descriptor after interface
> [   42.560000] usb 1-2: default language 0x0409
> [   42.570000] usb 1-2: udev 3, busnum 1, minor = 2
> [   42.570000] usb 1-2: New USB device found, idVendor=0a81, idProduct=0701
> [   42.570000] usb 1-2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
> [   42.580000] usb 1-2: Product: USB Missile Launcher v1.0
> [   42.590000] usb 1-2: Manufacturer: Dream Link
> [   42.590000] usb 1-2: usb_probe_device
> [   42.590000] usb 1-2: configuration #1 chosen from 1 choice
> [   42.590000] usb 1-2: adding 1-2:1.0 (config #1, interface 0)
> [   42.600000] usbhid 1-2:1.0: usb_probe_interface
> [   42.600000] usbhid 1-2:1.0: usb_probe_interface - got id
> [   42.610000] generic-usb 0003:0A81:0701.0002: claimed by neither input, hiddev nor hidraw
> [   42.610000] /home/thomas/projets/linux-2.6/drivers/usb/core/inode.c: creating file '003'
> [   42.610000] hub 1-0:1.0: state 7 ports 2 chg 0000 evt 0004
> [   42.610000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861e48,0004)
> [   42.610000] at91_ohci at91_ohci: GetPortStatus(2)
> [   43.850000] at91_ohci at91_ohci: overcurrent situation exited
> 
> Could you enlighten me on how this over-current mechanism is supposed
> to work ?

We don't have any good way of waiting for the over-current status to
clear, because the hub driver is single-threaded.  It wouldn't be able
to respond to any other USB events while waiting.

If the port is still over-current when the power is turned back on, the 
hub is expected to turn the port power off again and signal another 
over-current status change.

Alan Stern

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/3] at91-ohci: support overcurrent notification
  2011-07-13  9:29 ` [PATCH 2/3] at91-ohci: support overcurrent notification Thomas Petazzoni
@ 2011-07-13 14:28   ` Thomas Petazzoni
  2011-07-13 15:54     ` Alan Stern
  2011-09-07 10:47   ` Nicolas Ferre
  1 sibling, 1 reply; 15+ messages in thread
From: Thomas Petazzoni @ 2011-07-13 14:28 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

Le Wed, 13 Jul 2011 11:29:17 +0200,
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> a ?crit :

> Several USB power switches (AIC1526 or MIC2026) have a digital output
> that is used to notify that an overcurrent situation is taking
> place. This digital outputs are typically connected to GPIO inputs of
> the processor and can be used to be notified of those overcurrent
> situations.
> 
> Therefore, we add a new overcurrent_pin[] array in the at91_usbh_data
> structure so that boards can tell the AT91 OHCI driver which pins are
> used for the overcurrent notification, and an overcurrent_supported
> boolean to tell the driver whether overcurrent is supported or not.
> 
> The code has been largely borrowed from ohci-da8xx.c and
> ohci-s3c2410.c.

Now, I have a question about the behavior I observe with this code in
place. In order to easily trigger the over-current situation, I have
told in my board code that a button is the GPIO for the overcurrent_pin.

When I push the button, I enter the over-current situation, when I
release the button, I exit the over-current situation.

With the code above in place, when I push the button, the power is shut
off on the corresponding USB port and the device goes away. But
immediately after that, the power is switched on at the USB port by the
USB core, and the device is detected again. Is this normal behaviour ?
I would have expected the USB core to wait for the over-current
situation to disappear (i.e from me releasing the button). Maybe I am
misunderstanding how over-current management works ?

Apparently, the behaviour I observe is implemented by the following
piece of code in drivers/usb/core/hub.c :

                        if (portchange & USB_PORT_STAT_C_OVERCURRENT) {
                                u16 status = 0;
                                u16 unused;

                                dev_dbg(hub_dev, "over-current change on port "
                                        "%d\n", i);
                                clear_port_feature(hdev, i,
                                        USB_PORT_FEAT_C_OVER_CURRENT);
                                msleep(100);    /* Cool down */
                                hub_power_on(hub, true);
                                hub_port_status(hub, i, &status, &unused);
                                if (status & USB_PORT_STAT_OVERCURRENT)
                                        dev_err(hub_dev, "over-current "
                                                "condition on port %d\n", i);
                        }

So I don't see where it would wait for the over-current situation to be
cleared. However, the USB 2.0 specification says, in section 11.12.5 :

"""
Host recovery actions for an over-current event should include the
following:
1. Host gets change notification from hub with over-current
   event.
2. Host extracts appropriate hub or port change information
   (depending on the information in the change bitmap).
3. Host waits for over-current status bit to be cleared to 0.
4. Host cycles power on to all of the necessary ports (e.g., issues a
   SetPortFeature(PORT_POWER) request for each port).
5. Host re-enumerates all affected ports
"""

So, according to step 3), I would have expected the USB core to wait
for the over-currrent status to be cleared, that is, wait until I
release the button.

Below is the log of what happens between the moment I press the button
(message "overcurrent situation notified" from the GPIO interrupt
handler) and the moment I release the button (message "overcurrent
situation exited" from the GPIO interrupt handler).

[   41.750000] at91_ohci at91_ohci: overcurrent situation notified
[   41.790000] hub 1-0:1.0: state 7 ports 2 chg 0000 evt 0006
[   41.790000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0001,c7861e48,0004)
[   41.790000] at91_ohci at91_ohci: GetPortStatus(1)
[   41.790000] hub 1-0:1.0: over-current change on port 1
[   41.790000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2301,0x0013,0x0001,c7861e70,0000)
[   41.790000] at91_ohci at91_ohci: ClearPortFeature: C_OVER_CURRENT
[   41.790000] at91_ohci at91_ohci: CTRL: TypeReq=0x2301 val=0x13 idx=0x1 len=0 ==> -22
[   41.900000] hub 1-0:1.0: enabling power on all ports
[   41.900000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2303,0x0008,0x0001,c7861e58,0000)
[   41.900000] at91_ohci at91_ohci: SetPortFeat: POWER
[   41.900000] at91_ohci at91_ohci: CTRL: TypeReq=0x2303 val=0x8 idx=0x1 len=0 ==> -22
[   41.900000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2303,0x0008,0x0002,c7861e58,0000)
[   41.900000] at91_ohci at91_ohci: SetPortFeat: POWER
[   41.900000] at91_ohci at91_ohci: CTRL: TypeReq=0x2303 val=0x8 idx=0x2 len=0 ==> -22
[   42.010000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0001,c7861e48,0004)
[   42.010000] at91_ohci at91_ohci: GetPortStatus(1)
[   42.010000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861e48,0004)
[   42.010000] at91_ohci at91_ohci: GetStatus roothub.portstatus [1] = 0x00030301 PESC CSC LSDA PPS CCS
[   42.010000] at91_ohci at91_ohci: GetPortStatus(2)
[   42.010000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2301,0x0010,0x0002,c7861e70,0000)
[   42.010000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2301,0x0011,0x0002,c7861e70,0000)
[   42.010000] hub 1-0:1.0: port 2, status 0301, change 0003, 1.5 Mb/s
[   42.010000] usb 1-2: USB disconnect, device number 2
[   42.010000] usb 1-2: unregistering device
[   42.010000] usb 1-2: unregistering interface 1-2:1.0
[   42.010000] usb 1-2: usb_disable_device nuking all URBs
[   42.010000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861e48,0004)
[   42.010000] at91_ohci at91_ohci: GetPortStatus(2)
[   42.050000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861e48,0004)
[   42.050000] at91_ohci at91_ohci: GetPortStatus(2)
[   42.090000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861e48,0004)
[   42.090000] at91_ohci at91_ohci: GetPortStatus(2)
[   42.130000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861e48,0004)
[   42.130000] at91_ohci at91_ohci: GetPortStatus(2)
[   42.170000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861e48,0004)
[   42.170000] at91_ohci at91_ohci: GetPortStatus(2)
[   42.170000] hub 1-0:1.0: debounce: port 2: total 100ms stable 100ms status 0x301
[   42.170000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2303,0x0004,0x0002,c7861de0,0000)
[   42.290000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861db8,0004)
[   42.290000] at91_ohci at91_ohci: GetStatus roothub.portstatus [1] = 0x00100303 PRSC LSDA PPS PES CCS
[   42.290000] at91_ohci at91_ohci: GetPortStatus(2)
[   42.350000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2301,0x0014,0x0002,c7861de0,0000)
[   42.350000] usb 1-2: new low speed USB device number 3 using at91_ohci
[   42.350000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2303,0x0004,0x0002,c7861de0,0000)
[   42.470000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861db8,0004)
[   42.470000] at91_ohci at91_ohci: GetStatus roothub.portstatus [1] = 0x00100303 PRSC LSDA PPS PES CCS
[   42.470000] at91_ohci at91_ohci: GetPortStatus(2)
[   42.530000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0x2301,0x0014,0x0002,c7861de0,0000)
[   42.560000] usb 1-2: skipped 1 descriptor after interface
[   42.560000] usb 1-2: default language 0x0409
[   42.570000] usb 1-2: udev 3, busnum 1, minor = 2
[   42.570000] usb 1-2: New USB device found, idVendor=0a81, idProduct=0701
[   42.570000] usb 1-2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[   42.580000] usb 1-2: Product: USB Missile Launcher v1.0
[   42.590000] usb 1-2: Manufacturer: Dream Link
[   42.590000] usb 1-2: usb_probe_device
[   42.590000] usb 1-2: configuration #1 chosen from 1 choice
[   42.590000] usb 1-2: adding 1-2:1.0 (config #1, interface 0)
[   42.600000] usbhid 1-2:1.0: usb_probe_interface
[   42.600000] usbhid 1-2:1.0: usb_probe_interface - got id
[   42.610000] generic-usb 0003:0A81:0701.0002: claimed by neither input, hiddev nor hidraw
[   42.610000] /home/thomas/projets/linux-2.6/drivers/usb/core/inode.c: creating file '003'
[   42.610000] hub 1-0:1.0: state 7 ports 2 chg 0000 evt 0004
[   42.610000] at91_ohci at91_ohci: ohci_at91_hub_control(c79ccc00,0xa300,0x0000,0x0002,c7861e48,0004)
[   42.610000] at91_ohci at91_ohci: GetPortStatus(2)
[   43.850000] at91_ohci at91_ohci: overcurrent situation exited

Could you enlighten me on how this over-current mechanism is supposed
to work ?

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/3] at91-ohci: support overcurrent notification
  2011-07-13  9:29 [PATCH v2] AT91 OHCI active-high vbus and overcurrent handling Thomas Petazzoni
@ 2011-07-13  9:29 ` Thomas Petazzoni
  2011-07-13 14:28   ` Thomas Petazzoni
  2011-09-07 10:47   ` Nicolas Ferre
  0 siblings, 2 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2011-07-13  9:29 UTC (permalink / raw)
  To: linux-arm-kernel

Several USB power switches (AIC1526 or MIC2026) have a digital output
that is used to notify that an overcurrent situation is taking
place. This digital outputs are typically connected to GPIO inputs of
the processor and can be used to be notified of those overcurrent
situations.

Therefore, we add a new overcurrent_pin[] array in the at91_usbh_data
structure so that boards can tell the AT91 OHCI driver which pins are
used for the overcurrent notification, and an overcurrent_supported
boolean to tell the driver whether overcurrent is supported or not.

The code has been largely borrowed from ohci-da8xx.c and
ohci-s3c2410.c.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Andrew Victor <linux@maxim.org.za>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Matthieu CASTET <matthieu.castet@parrot.com>
---
 arch/arm/mach-at91/include/mach/board.h |    4 +
 drivers/usb/host/ohci-at91.c            |  224 +++++++++++++++++++++++++++++--
 2 files changed, 219 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-at91/include/mach/board.h b/arch/arm/mach-at91/include/mach/board.h
index 61d52dc..d07767f 100644
--- a/arch/arm/mach-at91/include/mach/board.h
+++ b/arch/arm/mach-at91/include/mach/board.h
@@ -99,6 +99,10 @@ struct at91_usbh_data {
 	u8		ports;		/* number of ports on root hub */
 	u8		vbus_pin[2];	/* port power-control pin */
 	u8              vbus_pin_inverted;
+	u8              overcurrent_supported;
+	u8              overcurrent_pin[2];
+	u8              overcurrent_status[2];
+	u8              overcurrent_changed[2];
 };
 extern void __init at91_add_device_usbh(struct at91_usbh_data *data);
 extern void __init at91_add_device_usbh_ohci(struct at91_usbh_data *data);
diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 3612ccd..331909f 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -223,6 +223,156 @@ ohci_at91_start (struct usb_hcd *hcd)
 	return 0;
 }
 
+static void ohci_at91_usb_set_power(struct at91_usbh_data *pdata, int port, int enable)
+{
+	if (port < 0 || port >= 2)
+		return;
+
+	gpio_set_value(pdata->vbus_pin[port], !pdata->vbus_pin_inverted ^ enable);
+}
+
+static int ohci_at91_usb_get_power(struct at91_usbh_data *pdata, int port)
+{
+	if (port < 0 || port >= 2)
+		return -EINVAL;
+
+	return gpio_get_value(pdata->vbus_pin[port]) ^ !pdata->vbus_pin_inverted;
+}
+
+/*
+ * Update the status data from the hub with the over-current indicator change.
+ */
+static int ohci_at91_hub_status_data(struct usb_hcd *hcd, char *buf)
+{
+	struct at91_usbh_data *pdata = hcd->self.controller->platform_data;
+	int length = ohci_hub_status_data(hcd, buf);
+	int port;
+
+	for (port = 0; port < ARRAY_SIZE(pdata->overcurrent_pin); port++) {
+		if (pdata->overcurrent_changed[port]) {
+			if (! length)
+				length = 1;
+			buf[0] |= 1 << (port + 1);
+		}
+	}
+
+	return length;
+}
+
+/*
+ * Look@the control requests to the root hub and see if we need to override.
+ */
+static int ohci_at91_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
+				 u16 wIndex, char *buf, u16 wLength)
+{
+	struct at91_usbh_data *pdata = hcd->self.controller->platform_data;
+	struct usb_hub_descriptor *desc;
+	int ret = -EINVAL;
+	u32 *data = (u32 *)buf;
+
+	dev_dbg(hcd->self.controller,
+		"ohci_at91_hub_control(%p,0x%04x,0x%04x,0x%04x,%p,%04x)\n",
+		hcd, typeReq, wValue, wIndex, buf, wLength);
+
+	switch (typeReq) {
+	case SetPortFeature:
+		if (wValue == USB_PORT_FEAT_POWER) {
+			dev_dbg(hcd->self.controller, "SetPortFeat: POWER\n");
+			ohci_at91_usb_set_power(pdata, wIndex - 1, 1);
+			goto out;
+		}
+		break;
+
+	case ClearPortFeature:
+		switch (wValue) {
+		case USB_PORT_FEAT_C_OVER_CURRENT:
+			dev_dbg(hcd->self.controller,
+				"ClearPortFeature: C_OVER_CURRENT\n");
+
+			if (wIndex == 1 || wIndex == 2) {
+				pdata->overcurrent_changed[wIndex-1] = 0;
+				pdata->overcurrent_status[wIndex-1] = 0;
+			}
+
+			goto out;
+
+		case USB_PORT_FEAT_OVER_CURRENT:
+			dev_dbg(hcd->self.controller,
+				"ClearPortFeature: OVER_CURRENT\n");
+
+			if (wIndex == 1 || wIndex == 2) {
+				pdata->overcurrent_status[wIndex-1] = 0;
+			}
+
+			goto out;
+
+		case USB_PORT_FEAT_POWER:
+			dev_dbg(hcd->self.controller,
+				"ClearPortFeature: POWER\n");
+
+			if (wIndex == 1 || wIndex == 2) {
+				ohci_at91_usb_set_power(pdata, wIndex - 1, 0);
+				return 0;
+			}
+		}
+		break;
+	}
+
+	ret = ohci_hub_control(hcd, typeReq, wValue, wIndex, buf, wLength);
+	if (ret)
+		goto out;
+
+	switch (typeReq) {
+	case GetHubDescriptor:
+
+		/* update the hub's descriptor */
+
+		desc = (struct usb_hub_descriptor *)buf;
+
+		dev_dbg(hcd->self.controller, "wHubCharacteristics 0x%04x\n",
+			desc->wHubCharacteristics);
+
+		/* remove the old configurations for power-switching, and
+		 * over-current protection, and insert our new configuration
+		 */
+
+		desc->wHubCharacteristics &= ~cpu_to_le16(HUB_CHAR_LPSM);
+		desc->wHubCharacteristics |= cpu_to_le16(0x0001);
+
+		if (pdata->overcurrent_supported) {
+			desc->wHubCharacteristics &= ~cpu_to_le16(HUB_CHAR_OCPM);
+			desc->wHubCharacteristics |=  cpu_to_le16(0x0008|0x0001);
+		}
+
+		dev_dbg(hcd->self.controller, "wHubCharacteristics after 0x%04x\n",
+			desc->wHubCharacteristics);
+
+		return ret;
+
+	case GetPortStatus:
+		/* check port status */
+
+		dev_dbg(hcd->self.controller, "GetPortStatus(%d)\n", wIndex);
+
+		if (wIndex == 1 || wIndex == 2) {
+			if (! ohci_at91_usb_get_power(pdata, wIndex-1)) {
+				*data &= ~cpu_to_le32(RH_PS_PPS);
+			}
+
+			if (pdata->overcurrent_changed[wIndex-1]) {
+				*data |= cpu_to_le32(RH_PS_OCIC);
+			}
+
+			if (pdata->overcurrent_status[wIndex-1]) {
+				*data |= cpu_to_le32(RH_PS_POCI);
+			}
+		}
+	}
+
+ out:
+	return ret;
+}
+
 /*-------------------------------------------------------------------------*/
 
 static const struct hc_driver ohci_at91_hc_driver = {
@@ -258,8 +408,8 @@ static const struct hc_driver ohci_at91_hc_driver = {
 	/*
 	 * root hub support
 	 */
-	.hub_status_data =	ohci_hub_status_data,
-	.hub_control =		ohci_hub_control,
+	.hub_status_data =	ohci_at91_hub_status_data,
+	.hub_control =		ohci_at91_hub_control,
 #ifdef CONFIG_PM
 	.bus_suspend =		ohci_bus_suspend,
 	.bus_resume =		ohci_bus_resume,
@@ -269,22 +419,71 @@ static const struct hc_driver ohci_at91_hc_driver = {
 
 /*-------------------------------------------------------------------------*/
 
+static irqreturn_t ohci_hcd_at91_overcurrent_irq(int irq, void *data)
+{
+	struct platform_device *pdev = data;
+	struct at91_usbh_data *pdata = pdev->dev.platform_data;
+	int val, gpio, port;
+
+	/* From the GPIO notifying the over-current situation, find
+	 * out the corresponding port */
+	gpio = irq_to_gpio(irq);
+	for (port = 0; port < ARRAY_SIZE(pdata->overcurrent_pin); port++) {
+		if (pdata->overcurrent_pin[port] == gpio)
+			break;
+	}
+
+	if (port == ARRAY_SIZE(pdata->overcurrent_pin)) {
+		dev_err(& pdev->dev, "overcurrent interrupt from unknown GPIO\n");
+		return IRQ_HANDLED;
+	}
+
+	val = gpio_get_value(gpio);
+
+	/* When notified of an over-current situation, disable power
+	   on the corresponding port, and mark this port in
+	   over-current. */
+	if (! val) {
+		ohci_at91_usb_set_power(pdata, port, 0);
+		pdata->overcurrent_status[port]  = 1;
+		pdata->overcurrent_changed[port] = 1;
+	}
+
+	dev_dbg(& pdev->dev, "overcurrent situation %s\n",
+		val ? "exited" : "notified");
+
+	return IRQ_HANDLED;
+}
+
+/*-------------------------------------------------------------------------*/
+
 static int ohci_hcd_at91_drv_probe(struct platform_device *pdev)
 {
 	struct at91_usbh_data	*pdata = pdev->dev.platform_data;
 	int			i;
 
 	if (pdata) {
-		/* REVISIT make the driver support per-port power switching,
-		 * and also overcurrent detection.  Here we assume the ports
-		 * are always powered while this driver is active, and use
-		 * active-low power switches.
-		 */
 		for (i = 0; i < ARRAY_SIZE(pdata->vbus_pin); i++) {
 			if (pdata->vbus_pin[i] <= 0)
 				continue;
 			gpio_request(pdata->vbus_pin[i], "ohci_vbus");
-			gpio_direction_output(pdata->vbus_pin[i], pdata->vbus_pin_inverted);
+			ohci_at91_usb_set_power(pdata, i, 1);
+		}
+
+		for (i = 0; i < ARRAY_SIZE(pdata->overcurrent_pin); i++) {
+			int ret;
+
+			if (pdata->overcurrent_pin[i] <= 0)
+				continue;
+			gpio_request(pdata->overcurrent_pin[i], "ohci_overcurrent");
+
+			ret = request_irq(gpio_to_irq(pdata->overcurrent_pin[i]),
+					  ohci_hcd_at91_overcurrent_irq,
+					  IRQF_SHARED, "ohci_overcurrent", pdev);
+			if (ret) {
+				gpio_free(pdata->overcurrent_pin[i]);
+				dev_warn(& pdev->dev, "cannot get GPIO IRQ for overcurrent\n");
+			}
 		}
 	}
 
@@ -301,9 +500,16 @@ static int ohci_hcd_at91_drv_remove(struct platform_device *pdev)
 		for (i = 0; i < ARRAY_SIZE(pdata->vbus_pin); i++) {
 			if (pdata->vbus_pin[i] <= 0)
 				continue;
-			gpio_direction_output(pdata->vbus_pin[i], !pdata->vbus_pin_inverted);
+			ohci_at91_usb_set_power(pdata, i, 0);
 			gpio_free(pdata->vbus_pin[i]);
 		}
+
+		for (i = 0; i < ARRAY_SIZE(pdata->overcurrent_pin); i++) {
+			if (pdata->overcurrent_pin[i] <= 0)
+				continue;
+			free_irq(gpio_to_irq(pdata->overcurrent_pin[i]), pdev);
+			gpio_free(pdata->overcurrent_pin[i]);
+		}
 	}
 
 	device_init_wakeup(&pdev->dev, 0);
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2011-09-07 10:47 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-05  9:05 [PATCH 1/3] ohci-at91: add vbus_pin_inverted platform attribute Thomas Petazzoni
2011-07-05  9:05 ` [PATCH 2/3] at91-ohci: support overcurrent notification Thomas Petazzoni
2011-07-05 10:12   ` Matthieu CASTET
2011-07-05 10:18     ` Thomas Petazzoni
2011-07-05 12:18       ` Matthieu CASTET
2011-07-05 14:23   ` Jean-Christophe PLAGNIOL-VILLARD
2011-07-05  9:05 ` [PATCH 3/3] at91-ohci: configure overcurrent pins as GPIOs Thomas Petazzoni
2011-07-05 11:39 ` [PATCH 1/3] ohci-at91: add vbus_pin_inverted platform attribute Sergei Shtylyov
2011-07-05 11:54   ` Thomas Petazzoni
2011-07-13  9:29 [PATCH v2] AT91 OHCI active-high vbus and overcurrent handling Thomas Petazzoni
2011-07-13  9:29 ` [PATCH 2/3] at91-ohci: support overcurrent notification Thomas Petazzoni
2011-07-13 14:28   ` Thomas Petazzoni
2011-07-13 15:54     ` Alan Stern
2011-07-13 16:43       ` Thomas Petazzoni
2011-07-13 17:17         ` Alan Stern
2011-09-07 10:47   ` Nicolas Ferre

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.