Skip to contentSkip to conversation

ohci-at91: add vbus_pin_inverted platform attribute

PATCH New Thomas Petazzoni 4 participants 9 messages Jul 5, 2011

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

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

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

Cc Andrew Victor <[email protected]>
Cc Nicolas Ferre <[email protected]>
Cc Jean-Christophe Plagniol-Villard <[email protected]>
---
 arch/arm/mach-at91/include/mach/board.h |    1 +
 drivers/usb/host/ohci-at91.c            |    4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)
arch/arm/mach-at91/include/mach/board.h +1 -0
index ed544a0..61d52dc 100644
@@ -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);
drivers/usb/host/ohci-at91.c +2 -2
index 944291e..52e50ba 100644
@@ -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]);
 		}
 	}
Signature · 1 line
1.7.4.1
#1 lore raw

PATCH 2/3 at91-ohci: support overcurrent notification

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.

Cc Andrew Victor <[email protected]>
Cc Nicolas Ferre <[email protected]>
Cc Jean-Christophe Plagniol-Villard <[email protected]>
---
 arch/arm/mach-at91/include/mach/board.h |    1 +
 drivers/usb/host/ohci-at91.c            |   37 +++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 0 deletions(-)
arch/arm/mach-at91/include/mach/board.h +1 -0
index 61d52dc..1f90d79 100644
@@ -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);
drivers/usb/host/ohci-at91.c +37 -0
index 52e50ba..2801c77 100644
@@ -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);
Signature · 1 line
1.7.4.1
#2 lore raw

PATCH 2/3 at91-ohci: support overcurrent notification

Thomas Petazzoni a ?crit :

Quoted 6 lines

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.

Quoted 6 lines

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

#3 lore raw

PATCH 2/3 at91-ohci: support overcurrent notification

Hello,

Le Tue, 5 Jul 2011 12:12:54 +0200,
Matthieu CASTET <[email protected]> a ?crit :

Quoted 2 lines

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

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#4 lore raw

PATCH 2/3 at91-ohci: support overcurrent notification

Hello,

Thomas Petazzoni a ?crit :

Quoted 10 lines

Hello,

Le Tue, 5 Jul 2011 12:12:54 +0200,
Matthieu CASTET <[email protected]> a ?crit :

Quoted 2 lines

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

#5 lore raw

PATCH 2/3 at91-ohci: support overcurrent notification

In reply to Thomas Petazzoni
Quoted 26 lines · 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. The AT91 OHCI driver simply registers an interrupt handler which will log the entry and exit of an overcurrent situation in the kernel logs.

Cc Andrew Victor <[email protected]>
Cc Nicolas Ferre <[email protected]>
Cc Jean-Christophe Plagniol-Villard <[email protected]>
---
 arch/arm/mach-at91/include/mach/board.h |    1 +
 drivers/usb/host/ohci-at91.c            |   37 +++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 0 deletions(-)
arch/arm/mach-at91/include/mach/board.h +0 -0
index 61d52dc..1f90d79 100644

arch/arm/mach-at91/include/mach/board.h struct at91_usbh_data { · line 102

Quoted 4 lines
arch/arm/mach-at91/include/mach/board.h +1 -0
@@ -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.

#6 lore raw

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

In reply to 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.

Cc Andrew Victor <[email protected]>
Cc Nicolas Ferre <[email protected]>
Cc Jean-Christophe Plagniol-Villard <[email protected]>
---
 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(-)
arch/arm/mach-at91/at91cap9_devices.c +5 -0
index dba0d8d..db7173a 100644
@@ -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);
 }
arch/arm/mach-at91/at91rm9200_devices.c +5 -0
index 7227755..6a1d564 100644
@@ -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);
 }
arch/arm/mach-at91/at91sam9260_devices.c +5 -0
index 39f81f4..3a77508 100644
@@ -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);
 }
arch/arm/mach-at91/at91sam9261_devices.c +5 -0
index 5004bf0..57b3fa7 100644
@@ -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);
 }
arch/arm/mach-at91/at91sam9263_devices.c +5 -0
index a050f41..f0fcc93 100644
@@ -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);
 }
arch/arm/mach-at91/at91sam9g45_devices.c +5 -0
index 600bffb..4f2d774 100644
@@ -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);
 }
Signature · 1 line
1.7.4.1
#7 lore raw

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

In reply to Thomas Petazzoni

Hello.

Quoted 13 lines · 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

Cc Andrew Victor<[email protected]>
Cc Nicolas Ferre<[email protected]>
Cc Jean-Christophe Plagniol-Villard<[email protected]>

[...]

drivers/usb/host/ohci-at91.c static int ohci_hcd_at91_drv_probe(struct platform_device *pdev) · line 287

Quoted 10 lines
drivers/usb/host/ohci-at91.c +1 -1
index 944291e..52e50ba 100644
@@ -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'?

drivers/usb/host/ohci-at91.c static int ohci_hcd_at91_drv_remove(struct platform_device *pdev) · line 304

Quoted 9 lines
drivers/usb/host/ohci-at91.c +1 -1
  		}
  	}
 
@@ -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

#8 lore raw

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

Le Tue, 05 Jul 2011 15:39:18 +0400,
Sergei Shtylyov <[email protected]> a ?crit :

Quoted 9 lines

drivers/usb/host/ohci-at91.c static int ohci_hcd_at91_drv_probe(struct platform_device *pdev) · line 287

Quoted 2 lines
drivers/usb/host/ohci-at91.c +1 -1
-			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'?

drivers/usb/host/ohci-at91.c static int ohci_hcd_at91_drv_remove(struct platform_device *pdev) · line 304

Quoted 2 lines
drivers/usb/host/ohci-at91.c +1 -1
-			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

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