[PATCH v2] AT91 OHCI active-high vbus and overcurrent handling

[PATCH v2] AT91 OHCI active-high vbus and overcurrent handling

[Thomas Petazzoni]

Hello,

This series of patches adds two features to the AT91 OHCI driver :
 * Support for active-high vbus pin USB power switches (patch 1)
 * Support for over-current notification (patches 2 and 3)

Changes since v1 (posted July, 5th 2011) :

 * Simplify expressions in gpio_direction_output() when controlling
   the VBUS pin, as suggested by Sergei Shtylyov
   <sshtylyov@mvista.com>

 * Extend the overcurrent notification support by notifying the USB
   stack and exposing the fact that per-port power switching and
   overcurrent notification, based on ohci-da8xx.c and ohci-s3c2410.c,
   as suggested by Matthieu CASTET <matthieu.castet@parrot.com>

I haven't yet taken into account Jean-Christophe Plagniol-Villard
comment on using the named ressources, because I don't see how a board
file can define ressources with the current AT91 code structure (the
ressources are only defined at the SoC level). Input from
Jean-Christophe on this would be greatly appreciated.

Regards,

Thomas

[PATCH 1/3] ohci-at91: add vbus_pin_inverted platform attribute

[Thomas Petazzoni]

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..3612ccd 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], 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], !pdata->vbus_pin_inverted);
 			gpio_free(pdata->vbus_pin[i]);
 		}
 	}
-- 
1.7.4.1

[PATCH 1/3] ohci-at91: add vbus_pin_inverted platform attribute

[Nicolas Ferre]

Le 13/07/2011 11:29, Thomas Petazzoni :
> 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>

Signed-off-by: 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..3612ccd 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], 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], !pdata->vbus_pin_inverted);
>  			gpio_free(pdata->vbus_pin[i]);
>  		}
>  	}


-- 
Nicolas Ferre

[PATCH 2/3] at91-ohci: support overcurrent notification

[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>
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

[PATCH 2/3] at91-ohci: support overcurrent notification

[Thomas Petazzoni]

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

[PATCH 2/3] at91-ohci: support overcurrent notification

[Alan Stern]

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

[PATCH 2/3] at91-ohci: support overcurrent notification

[Thomas Petazzoni]

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

[PATCH 2/3] at91-ohci: support overcurrent notification

[Alan Stern]

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

[PATCH 2/3] at91-ohci: support overcurrent notification

[Nicolas Ferre]

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

[PATCH 3/3] at91-ohci: configure overcurrent pins as input GPIOs

[Thomas Petazzoni]

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    |    6 ++++++
 arch/arm/mach-at91/at91rm9200_devices.c  |    6 ++++++
 arch/arm/mach-at91/at91sam9260_devices.c |    6 ++++++
 arch/arm/mach-at91/at91sam9261_devices.c |    6 ++++++
 arch/arm/mach-at91/at91sam9263_devices.c |    6 ++++++
 arch/arm/mach-at91/at91sam9g45_devices.c |    6 ++++++
 6 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-at91/at91cap9_devices.c b/arch/arm/mach-at91/at91cap9_devices.c
index dba0d8d..b46615a 100644
--- a/arch/arm/mach-at91/at91cap9_devices.c
+++ b/arch/arm/mach-at91/at91cap9_devices.c
@@ -80,6 +80,12 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
 			at91_set_gpio_output(data->vbus_pin[i], 0);
 	}
 
+	/* Enable overcurrent notification */
+	for (i = 0; i < data->ports; i++) {
+		if (data->overcurrent_pin[i])
+			at91_set_gpio_input(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..b2202b6 100644
--- a/arch/arm/mach-at91/at91rm9200_devices.c
+++ b/arch/arm/mach-at91/at91rm9200_devices.c
@@ -63,6 +63,12 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
 	if (!data)
 		return;
 
+	/* Enable overcurrent notification */
+	for (i = 0; i < data->ports; i++) {
+		if (data->overcurrent_pin[i])
+			at91_set_gpio_input(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..ad64fc3 100644
--- a/arch/arm/mach-at91/at91sam9260_devices.c
+++ b/arch/arm/mach-at91/at91sam9260_devices.c
@@ -64,6 +64,12 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
 	if (!data)
 		return;
 
+	/* Enable overcurrent notification */
+	for (i = 0; i < data->ports; i++) {
+		if (data->overcurrent_pin[i])
+			at91_set_gpio_input(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..e394e86 100644
--- a/arch/arm/mach-at91/at91sam9261_devices.c
+++ b/arch/arm/mach-at91/at91sam9261_devices.c
@@ -67,6 +67,12 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
 	if (!data)
 		return;
 
+	/* Enable overcurrent notification */
+	for (i = 0; i < data->ports; i++) {
+		if (data->overcurrent_pin[i])
+			at91_set_gpio_input(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..d4aef76 100644
--- a/arch/arm/mach-at91/at91sam9263_devices.c
+++ b/arch/arm/mach-at91/at91sam9263_devices.c
@@ -74,6 +74,12 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
 			at91_set_gpio_output(data->vbus_pin[i], 0);
 	}
 
+	/* Enable overcurrent notification */
+	for (i = 0; i < data->ports; i++) {
+		if (data->overcurrent_pin[i])
+			at91_set_gpio_input(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..e4a9857 100644
--- a/arch/arm/mach-at91/at91sam9g45_devices.c
+++ b/arch/arm/mach-at91/at91sam9g45_devices.c
@@ -124,6 +124,12 @@ void __init at91_add_device_usbh_ohci(struct at91_usbh_data *data)
 			at91_set_gpio_output(data->vbus_pin[i], 0);
 	}
 
+	/* Enable overcurrent notification */
+	for (i = 0; i < data->ports; i++) {
+		if (data->overcurrent_pin[i])
+			at91_set_gpio_input(data->overcurrent_pin[i], 1);
+	}
+
 	usbh_ohci_data = *data;
 	platform_device_register(&at91_usbh_ohci_device);
 }
-- 
1.7.4.1

[PATCH 3/3] at91-ohci: configure overcurrent pins as input GPIOs

[Nicolas Ferre]

Le 13/07/2011 11:29, Thomas Petazzoni :
> 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>

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

I will include this patch series in at91-l2 branch on:
git://github.com/at91linux/linux-at91.git
(mind the removal of -2.6-)

But there are several changes to ohci-at91 which also may fit in an USB
git tree...
According to the nature of changes, I think it is better to send them
upstream through the AT91 - arm-soc flow.

Thanks Thomas, best regards,

> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> ---
>  arch/arm/mach-at91/at91cap9_devices.c    |    6 ++++++
>  arch/arm/mach-at91/at91rm9200_devices.c  |    6 ++++++
>  arch/arm/mach-at91/at91sam9260_devices.c |    6 ++++++
>  arch/arm/mach-at91/at91sam9261_devices.c |    6 ++++++
>  arch/arm/mach-at91/at91sam9263_devices.c |    6 ++++++
>  arch/arm/mach-at91/at91sam9g45_devices.c |    6 ++++++
>  6 files changed, 36 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/at91cap9_devices.c b/arch/arm/mach-at91/at91cap9_devices.c
> index dba0d8d..b46615a 100644
> --- a/arch/arm/mach-at91/at91cap9_devices.c
> +++ b/arch/arm/mach-at91/at91cap9_devices.c
> @@ -80,6 +80,12 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
>  			at91_set_gpio_output(data->vbus_pin[i], 0);
>  	}
>  
> +	/* Enable overcurrent notification */
> +	for (i = 0; i < data->ports; i++) {
> +		if (data->overcurrent_pin[i])
> +			at91_set_gpio_input(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..b2202b6 100644
> --- a/arch/arm/mach-at91/at91rm9200_devices.c
> +++ b/arch/arm/mach-at91/at91rm9200_devices.c
> @@ -63,6 +63,12 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
>  	if (!data)
>  		return;
>  
> +	/* Enable overcurrent notification */
> +	for (i = 0; i < data->ports; i++) {
> +		if (data->overcurrent_pin[i])
> +			at91_set_gpio_input(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..ad64fc3 100644
> --- a/arch/arm/mach-at91/at91sam9260_devices.c
> +++ b/arch/arm/mach-at91/at91sam9260_devices.c
> @@ -64,6 +64,12 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
>  	if (!data)
>  		return;
>  
> +	/* Enable overcurrent notification */
> +	for (i = 0; i < data->ports; i++) {
> +		if (data->overcurrent_pin[i])
> +			at91_set_gpio_input(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..e394e86 100644
> --- a/arch/arm/mach-at91/at91sam9261_devices.c
> +++ b/arch/arm/mach-at91/at91sam9261_devices.c
> @@ -67,6 +67,12 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
>  	if (!data)
>  		return;
>  
> +	/* Enable overcurrent notification */
> +	for (i = 0; i < data->ports; i++) {
> +		if (data->overcurrent_pin[i])
> +			at91_set_gpio_input(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..d4aef76 100644
> --- a/arch/arm/mach-at91/at91sam9263_devices.c
> +++ b/arch/arm/mach-at91/at91sam9263_devices.c
> @@ -74,6 +74,12 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
>  			at91_set_gpio_output(data->vbus_pin[i], 0);
>  	}
>  
> +	/* Enable overcurrent notification */
> +	for (i = 0; i < data->ports; i++) {
> +		if (data->overcurrent_pin[i])
> +			at91_set_gpio_input(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..e4a9857 100644
> --- a/arch/arm/mach-at91/at91sam9g45_devices.c
> +++ b/arch/arm/mach-at91/at91sam9g45_devices.c
> @@ -124,6 +124,12 @@ void __init at91_add_device_usbh_ohci(struct at91_usbh_data *data)
>  			at91_set_gpio_output(data->vbus_pin[i], 0);
>  	}
>  
> +	/* Enable overcurrent notification */
> +	for (i = 0; i < data->ports; i++) {
> +		if (data->overcurrent_pin[i])
> +			at91_set_gpio_input(data->overcurrent_pin[i], 1);
> +	}
> +
>  	usbh_ohci_data = *data;
>  	platform_device_register(&at91_usbh_ohci_device);
>  }


-- 
Nicolas Ferre

[PATCH 3/3] at91-ohci: configure overcurrent pins as input GPIOs

[Nicolas Ferre]

Le 07/09/2011 12:51, Nicolas Ferre :
> Le 13/07/2011 11:29, Thomas Petazzoni :
>> 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>
> 
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>

but with...

> I will include this patch series in at91-l2 branch on:
> git://github.com/at91linux/linux-at91.git
> (mind the removal of -2.6-)
> 
> But there are several changes to ohci-at91 which also may fit in an USB
> git tree...
> According to the nature of changes, I think it is better to send them
> upstream through the AT91 - arm-soc flow.
> 
> Thanks Thomas, best regards,
> 
>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>> ---
>>  arch/arm/mach-at91/at91cap9_devices.c    |    6 ++++++
>>  arch/arm/mach-at91/at91rm9200_devices.c  |    6 ++++++
>>  arch/arm/mach-at91/at91sam9260_devices.c |    6 ++++++
>>  arch/arm/mach-at91/at91sam9261_devices.c |    6 ++++++
>>  arch/arm/mach-at91/at91sam9263_devices.c |    6 ++++++
>>  arch/arm/mach-at91/at91sam9g45_devices.c |    6 ++++++
>>  6 files changed, 36 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-at91/at91cap9_devices.c b/arch/arm/mach-at91/at91cap9_devices.c
>> index dba0d8d..b46615a 100644
>> --- a/arch/arm/mach-at91/at91cap9_devices.c
>> +++ b/arch/arm/mach-at91/at91cap9_devices.c
>> @@ -80,6 +80,12 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
>>  			at91_set_gpio_output(data->vbus_pin[i], 0);
>>  	}
>>  
>> +	/* Enable overcurrent notification */
>> +	for (i = 0; i < data->ports; i++) {
>> +		if (data->overcurrent_pin[i])
>> +			at91_set_gpio_input(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..b2202b6 100644
>> --- a/arch/arm/mach-at91/at91rm9200_devices.c
>> +++ b/arch/arm/mach-at91/at91rm9200_devices.c
>> @@ -63,6 +63,12 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
>>  	if (!data)
>>  		return;
>>  
>> +	/* Enable overcurrent notification */
>> +	for (i = 0; i < data->ports; i++) {

Yes, but... Missing "i" declaration... I have added it.

>> +		if (data->overcurrent_pin[i])
>> +			at91_set_gpio_input(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..ad64fc3 100644
>> --- a/arch/arm/mach-at91/at91sam9260_devices.c
>> +++ b/arch/arm/mach-at91/at91sam9260_devices.c
>> @@ -64,6 +64,12 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
>>  	if (!data)
>>  		return;
>>  
>> +	/* Enable overcurrent notification */
>> +	for (i = 0; i < data->ports; i++) {


Ditto.

>> +		if (data->overcurrent_pin[i])
>> +			at91_set_gpio_input(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..e394e86 100644
>> --- a/arch/arm/mach-at91/at91sam9261_devices.c
>> +++ b/arch/arm/mach-at91/at91sam9261_devices.c
>> @@ -67,6 +67,12 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
>>  	if (!data)
>>  		return;
>>  
>> +	/* Enable overcurrent notification */
>> +	for (i = 0; i < data->ports; i++) {

Ditto.


>> +		if (data->overcurrent_pin[i])
>> +			at91_set_gpio_input(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..d4aef76 100644
>> --- a/arch/arm/mach-at91/at91sam9263_devices.c
>> +++ b/arch/arm/mach-at91/at91sam9263_devices.c
>> @@ -74,6 +74,12 @@ void __init at91_add_device_usbh(struct at91_usbh_data *data)
>>  			at91_set_gpio_output(data->vbus_pin[i], 0);
>>  	}
>>  
>> +	/* Enable overcurrent notification */
>> +	for (i = 0; i < data->ports; i++) {

Ok here as we already have an "i"...

>> +		if (data->overcurrent_pin[i])
>> +			at91_set_gpio_input(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..e4a9857 100644
>> --- a/arch/arm/mach-at91/at91sam9g45_devices.c
>> +++ b/arch/arm/mach-at91/at91sam9g45_devices.c
>> @@ -124,6 +124,12 @@ void __init at91_add_device_usbh_ohci(struct at91_usbh_data *data)
>>  			at91_set_gpio_output(data->vbus_pin[i], 0);
>>  	}
>>  
>> +	/* Enable overcurrent notification */
>> +	for (i = 0; i < data->ports; i++) {
>> +		if (data->overcurrent_pin[i])
>> +			at91_set_gpio_input(data->overcurrent_pin[i], 1);
>> +	}
>> +
>>  	usbh_ohci_data = *data;
>>  	platform_device_register(&at91_usbh_ohci_device);
>>  }
> 
> 

Best regards,
-- 
Nicolas Ferre