Skip to contentSkip to conversation

USB: Driver for Freescale QUICC Engine USB Host Controller

PATCH Superseded Anton Vorontsov 4 participants 15 messages Dec 23, 2008 – Dec 24, 2008 USB

lore
Share & tools
lore

Anton Vorontsov

author
#0 lore raw

PATCH 1/1 USB: Driver for Freescale QUICC Engine USB Host Controller

This patch adds support for the FHCI USB controller, as found in the Freescale MPC836x and MPC832x processors. It can support Full or Low speed modes.

Quite a lot the hardware is doing by itself (SOF generation, CRC generation and checking), though scheduling and retransmission is on software's shoulders.

This controller does not integrate the root hub, so this driver also fakes one-port hub. External hub is required to support more than one device.

Signed-off-by Anton Vorontsov <[email protected]>

---

Hi all,

Finally, all the needed patches were merged into the linux-next, thus I think the driver should be OK for -mm.

Thanks,

 drivers/usb/Makefile          |    1 +
 drivers/usb/host/Kconfig      |   17 +
 drivers/usb/host/Makefile     |    6 +
 drivers/usb/host/fhci-dbg.c   |  139 +++++++
 drivers/usb/host/fhci-hcd.c   |  817 +++++++++++++++++++++++++++++++++++++
 drivers/usb/host/fhci-hub.c   |  344 ++++++++++++++++
 drivers/usb/host/fhci-mem.c   |  113 ++++++
 drivers/usb/host/fhci-q.c     |  281 +++++++++++++
 drivers/usb/host/fhci-sched.c |  888 +++++++++++++++++++++++++++++++++++++++++
 drivers/usb/host/fhci-tds.c   |  627 +++++++++++++++++++++++++++++
 drivers/usb/host/fhci.h       |  607 ++++++++++++++++++++++++++++
 11 files changed, 3840 insertions(+), 0 deletions(-)
 create mode 100644 drivers/usb/host/fhci-dbg.c
 create mode 100644 drivers/usb/host/fhci-hcd.c
 create mode 100644 drivers/usb/host/fhci-hub.c
 create mode 100644 drivers/usb/host/fhci-mem.c
 create mode 100644 drivers/usb/host/fhci-q.c
 create mode 100644 drivers/usb/host/fhci-sched.c
 create mode 100644 drivers/usb/host/fhci-tds.c
 create mode 100644 drivers/usb/host/fhci.h
drivers/usb/Makefile +1 -0
index 8b7c419..8bcde8c 100644
@@ -13,6 +13,7 @@ obj-$(CONFIG_USB_EHCI_HCD)	+= host/
 obj-$(CONFIG_USB_ISP116X_HCD)	+= host/
 obj-$(CONFIG_USB_OHCI_HCD)	+= host/
 obj-$(CONFIG_USB_UHCI_HCD)	+= host/
+obj-$(CONFIG_USB_FHCI_HCD)	+= host/
 obj-$(CONFIG_USB_SL811_HCD)	+= host/
 obj-$(CONFIG_USB_U132_HCD)	+= host/
 obj-$(CONFIG_USB_R8A66597_HCD)	+= host/
drivers/usb/host/Kconfig +17 -0
index f3a75a9..cd7c691 100644
@@ -225,6 +225,23 @@ config USB_UHCI_HCD
 	  To compile this driver as a module, choose M here: the
 	  module will be called uhci-hcd.
 
+config USB_FHCI_HCD
+	tristate "Freescale QE USB Host Controller support"
+	depends on USB && OF_GPIO && QE_GPIO && QUICC_ENGINE
+	select FSL_GTM
+	select QE_USB
+	help
+	  This driver enables support for Freescale QE USB Host Controller
+	  (as found on MPC8360 and MPC8323 processors), the driver supports
+	  Full and Low Speed USB.
+
+config FHCI_DEBUG
+	bool "Freescale QE USB Host Controller debug support"
+	depends on USB_FHCI_HCD && DEBUG_FS
+	help
+	  Say "y" to see some FHCI debug information and statistics
+	  throught debugfs.
+
 config USB_U132_HCD
 	tristate "Elan U132 Adapter Host Controller"
 	depends on USB && USB_FTDI_ELAN
drivers/usb/host/Makefile +6 -0
index 23be222..721d373 100644
@@ -7,6 +7,11 @@ ifeq ($(CONFIG_USB_DEBUG),y)
 endif
 
 isp1760-objs := isp1760-hcd.o isp1760-if.o
+fhci-objs := fhci-hcd.o fhci-hub.o fhci-q.o fhci-mem.o \
+	     fhci-tds.o fhci-sched.o
+ifeq ($(CONFIG_FHCI_DEBUG),y)
+fhci-objs += fhci-dbg.o
+endif
 
 obj-$(CONFIG_USB_WHCI_HCD)	+= whci/
 

@@ -16,6 +21,7 @@ obj-$(CONFIG_USB_EHCI_HCD)	+= ehci-hcd.o
 obj-$(CONFIG_USB_ISP116X_HCD)	+= isp116x-hcd.o
 obj-$(CONFIG_USB_OHCI_HCD)	+= ohci-hcd.o
 obj-$(CONFIG_USB_UHCI_HCD)	+= uhci-hcd.o
+obj-$(CONFIG_USB_FHCI_HCD)	+= fhci.o
 obj-$(CONFIG_USB_SL811_HCD)	+= sl811-hcd.o
 obj-$(CONFIG_USB_SL811_CS)	+= sl811_cs.o
 obj-$(CONFIG_USB_U132_HCD)	+= u132-hcd.o
drivers/usb/host/fhci-dbg.c new file +139 -0

139 lines not on this page · raw message

drivers/usb/host/fhci-hcd.c new file +817 -0

817 lines not on this page · raw message

drivers/usb/host/fhci-hub.c new file +344 -0

344 lines not on this page · raw message

drivers/usb/host/fhci-mem.c new file +113 -0

113 lines not on this page · raw message

drivers/usb/host/fhci-q.c new file +281 -0

281 lines not on this page · raw message

drivers/usb/host/fhci-sched.c new file +888 -0

888 lines not on this page · raw message

drivers/usb/host/fhci-tds.c new file +627 -0

370 lines not on this page · raw message

drivers/usb/host/fhci.h new file +607 -0

607 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

Signature · 1 line
1.5.6.5

Part of the diff is not shown here. View the raw message.

#1 lore raw
Quoted 4 lines · On Wed, 24 Dec 2008 00:03:22 +0300 Anton Vorontsov <[email protected]>…

This patch adds support for the FHCI USB controller, as found in the Freescale MPC836x and MPC832x processors. It can support Full or Low speed modes.

Quoted 8 lines

Quite a lot the hardware is doing by itself (SOF generation, CRC generation and checking), though scheduling and retransmission is on software's shoulders.

This controller does not integrate the root hub, so this driver also fakes one-port hub. External hub is required to support more than one device.

<quick scan>

Nice-looking driver. But the namespace pollution is tremendous!

Quoted 11 lines

...

drivers/usb/host/fhci-hcd.c new file +8 -0
+static int fhci_mem_init(struct fhci_hcd *fhci)
+{
+	int i;
+	int error = 0;
+
+	fhci->hc_list = kzalloc(sizeof(*fhci->hc_list), GFP_KERNEL);
+	if (!fhci->hc_list)
+		return -ENOMEM;

drivers/usb/host/fhci-hcd.c line 200

Quoted 10 lines
drivers/usb/host/fhci-hcd.c new file +10 -0
+
+	INIT_LIST_HEAD(&fhci->hc_list->ctrl_list);
+	INIT_LIST_HEAD(&fhci->hc_list->bulk_list);
+	INIT_LIST_HEAD(&fhci->hc_list->iso_list);
+	INIT_LIST_HEAD(&fhci->hc_list->intr_list);
+	INIT_LIST_HEAD(&fhci->hc_list->done_list);
+
+	fhci->vroot_hub = kzalloc(sizeof(*fhci->vroot_hub), GFP_KERNEL);
+	if (!fhci->vroot_hub)
+		return -ENOMEM;

Did we leak fhci->hc_list?

Quoted 30 lines
drivers/usb/host/fhci-hcd.c new file +30 -0
+	INIT_LIST_HEAD(&fhci->empty_eds);
+	INIT_LIST_HEAD(&fhci->empty_tds);
+
+	/* initialize work queue to handle done list */
+	fhci_tasklet.data = (unsigned long)fhci;
+	fhci->process_done_task = &fhci_tasklet;
+
+	for (i = 0; i < MAX_TDS; i++) {
+		struct td *td = kmalloc(sizeof(*td), GFP_KERNEL);
+
+		if (!td) {
+			error = 1;
+			break;
+		}
+		recycle_empty_td(fhci, td);
+	}
+	for (i = 0; i < MAX_EDS; i++) {
+		struct ed *ed = kmalloc(sizeof(*ed), GFP_KERNEL);
+
+		if (!ed) {
+			error = 1;
+			break;
+		}
+		recycle_empty_ed(fhci, ed);
+	}
+
+	if (error) {
+		fhci_mem_free(fhci);
+		return -ENOMEM;
+	}

drivers/usb/host/fhci-hcd.c

Quoted 10 lines
drivers/usb/host/fhci-hcd.c new file +7 -0
+
+	fhci->active_urbs = 0;
+
+	return error;
+}
+
 
...
 
+void config_transceiver(struct fhci_hcd *fhci, enum fhci_port_status status)

poorly-chosen global identifier.

Quoted 17 lines
drivers/usb/host/fhci-hub.c new file +17 -0
+{
+	fhci_dbg(fhci, "-> %s: %d\n", __func__, status);
+
+	switch (status) {
+	case FHCI_PORT_POWER_OFF:
+		fhci_gpio_set_value(fhci, GPIO_POWER, false);
+		break;
+	case FHCI_PORT_DISABLED:
+	case FHCI_PORT_WAITING:
+		fhci_gpio_set_value(fhci, GPIO_POWER, true);
+		break;
+	case FHCI_PORT_LOW:
+		fhci_gpio_set_value(fhci, GPIO_SPEED, false);
+		break;
+	case FHCI_PORT_FULL:
+		fhci_gpio_set_value(fhci, GPIO_SPEED, true);
+		break;

drivers/usb/host/fhci-hub.c line 82

Quoted 10 lines
drivers/usb/host/fhci-hub.c new file +10 -0
+	default:
+		WARN_ON(1);
+		break;
+	}
+
+	fhci_dbg(fhci, "<- %s: %d\n", __func__, status);
+}
+
+/* disable the USB port by clearing the EN bit in the USBMOD register */
+void usb_port_disable(struct fhci_hcd *fhci)

another

Quoted 20 lines
drivers/usb/host/fhci-hub.c new file +20 -0
+{
+	struct fhci_usb *usb = (struct fhci_usb *)fhci->usb_lld;
+	enum fhci_port_status port_status;
+
+	fhci_dbg(fhci, "-> %s\n", __func__);
+
+	fhci_stop_sof_timer(fhci);
+
+	flush_all_transmissions(usb);
+
+	fhci_usb_disable_interrupt((struct fhci_usb *)fhci->usb_lld);
+	port_status = usb->port_status;
+	usb->port_status = FHCI_PORT_DISABLED;
+
+	/* Enable IDLE since we want to know if something comes along */
+	usb->saved_msk |= USB_E_IDLE_MASK;
+	out_be16(&usb->fhci->regs->usb_mask, usb->saved_msk);
+
+	/* check if during the disconnection process attached new device */
+	if (port_status == FHCI_PORT_WAITING)

drivers/usb/host/fhci-hub.c line 112

Quoted 10 lines
drivers/usb/host/fhci-hub.c new file +10 -0
+		device_connected_interrupt(fhci);
+	usb->vroot_hub->port.wPortStatus &= ~USB_PORT_STAT_ENABLE;
+	usb->vroot_hub->port.wPortChange |= USB_PORT_STAT_C_ENABLE;
+	fhci_usb_enable_interrupt((struct fhci_usb *)fhci->usb_lld);
+
+	fhci_dbg(fhci, "<- %s\n", __func__);
+}
+
+/* enable the USB port by setting the EN bit in the USBMOD register */
+void usb_port_enable(void *lld)

another. Please review all non-static symbols which this driver adds.

Quoted 52 lines
drivers/usb/host/fhci-hub.c new file +49 -0
+{
+	struct fhci_usb *usb = (struct fhci_usb *)lld;
+	struct fhci_hcd *fhci = usb->fhci;
+
+	fhci_dbg(fhci, "-> %s\n", __func__);
+
+	config_transceiver(fhci, usb->port_status);
+
+	if ((usb->port_status != FHCI_PORT_FULL) &&
+			(usb->port_status != FHCI_PORT_LOW))
+		fhci_start_sof_timer(fhci);
+
+	usb->vroot_hub->port.wPortStatus |= USB_PORT_STAT_ENABLE;
+	usb->vroot_hub->port.wPortChange |= USB_PORT_STAT_C_ENABLE;
+
+	fhci_dbg(fhci, "<- %s\n", __func__);
+}
+
 
...
 
+static struct td *get_empty_td(struct fhci_hcd *fhci)
+{
+	struct td *td;
+
+	if (!list_empty(&fhci->empty_tds)) {
+		td = list_entry(fhci->empty_tds.next, struct td, node);
+		list_del(fhci->empty_tds.next);
+	} else {
+		td = kmalloc(sizeof(*td), GFP_ATOMIC);
+		if (!td)
+			fhci_err(fhci, "No memory to allocate to TD\n");
+		else
+			init_td(td);
+	}
+
+	return td;
+}
+
+void recycle_empty_td(struct fhci_hcd *fhci, struct td *td)
+{
+	init_td(td);
+	list_add(&td->node, &fhci->empty_tds);
+}
+
+struct ed *get_empty_ed(struct fhci_hcd *fhci)
+{
+	struct ed *ed;
+
+	if (!list_empty(&fhci->empty_eds)) {
+		ed = list_entry(fhci->empty_eds.next, struct ed, node);
+		list_del(fhci->empty_eds.next);

drivers/usb/host/fhci-hub.c line 270

Quoted 10 lines
drivers/usb/host/fhci-hub.c new file +10 -0
+	} else {
+		ed = kmalloc(sizeof(*ed), GFP_ATOMIC);
+		if (!ed)
+			fhci_err(fhci, "No memory to allocate to ED\n");
+		else
+			init_ed(ed);
+	}
+
+	return ed;
+}

The GFP_ATOMICs here are regrettable. Are these functions ever called from a context in which a more reliable allocation mode can be used? If so, the caller should pass in the gfp_t.

drivers/usb/host/fhci-tds.c line 152

Quoted 5 lines

...

drivers/usb/host/fhci-tds.c new file +2 -0
+u32 create_endpoint(struct fhci_usb *usb, enum fhci_mem_alloc data_mem,
+			   u32 ring_len)

This is a poorly chosen global identifier. fhci_create_endpoint() would be better.

author
#2 lore raw
Quoted 11 lines · Andrew Morton
Quoted 8 lines · On Wed, 24 Dec 2008 00:03:22 +0300 Anton Vorontsov <[email protected]>…

This patch adds support for the FHCI USB controller, as found in the Freescale MPC836x and MPC832x processors. It can support Full or Low speed modes.

Quite a lot the hardware is doing by itself (SOF generation, CRC generation and checking), though scheduling and retransmission is on software's shoulders.

Quoted 8 lines
Quoted 4 lines

This controller does not integrate the root hub, so this driver also fakes one-port hub. External hub is required to support more than one device.

<quick scan>

Nice-looking driver. But the namespace pollution is tremendous!

Thanks for the review, Andrew.

Name-space pollution fixed.

[..]

Quoted 12 lines
Quoted 12 lines
drivers/usb/host/fhci-hcd.c new file +12 -0
+static int fhci_mem_init(struct fhci_hcd *fhci)
+{
+	int i;
+	int error = 0;
+
+	fhci->hc_list = kzalloc(sizeof(*fhci->hc_list), GFP_KERNEL);
+	if (!fhci->hc_list)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&fhci->hc_list->ctrl_list);
+	INIT_LIST_HEAD(&fhci->hc_list->bulk_list);
+	INIT_LIST_HEAD(&fhci->hc_list->iso_list);
Quoted 8 lines

drivers/usb/host/fhci-hcd.c line 200

Quoted 6 lines
drivers/usb/host/fhci-hcd.c new file +6 -0
+	INIT_LIST_HEAD(&fhci->hc_list->intr_list);
+	INIT_LIST_HEAD(&fhci->hc_list->done_list);
+
+	fhci->vroot_hub = kzalloc(sizeof(*fhci->vroot_hub), GFP_KERNEL);
+	if (!fhci->vroot_hub)
+		return -ENOMEM;

Did we leak fhci->hc_list?

Apparently. Fixed.

[...]

Quoted 13 lines
Quoted 13 lines
drivers/usb/host/fhci-mem.c new file +13 -0
+struct ed *get_empty_ed(struct fhci_hcd *fhci)
+{
+	struct ed *ed;
+
+	if (!list_empty(&fhci->empty_eds)) {
+		ed = list_entry(fhci->empty_eds.next, struct ed, node);
+		list_del(fhci->empty_eds.next);
+	} else {
+		ed = kmalloc(sizeof(*ed), GFP_ATOMIC);
+		if (!ed)
+			fhci_err(fhci, "No memory to allocate to ED\n");
+		else
+			init_ed(ed);
Quoted 8 lines

drivers/usb/host/fhci-mem.c line 80

Quoted 4 lines
drivers/usb/host/fhci-mem.c new file +4 -0
+	}
+
+	return ed;
+}

The GFP_ATOMICs here are regrettable. Are these functions ever called from a context in which a more reliable allocation mode can be used? If so, the caller should pass in the gfp_t.

No, these are called with a spinlock held. But we don't normally allocate things via this function, instead we pre-allocate the eds and tds in the fhci_mem_init, so the kmalloc above is the last resort when no empty tds/eds left in the appropriate lists (normally should not happen).

Thanks,

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#3 lore raw
In reply to Anton Vorontsov
Quoted 11 lines · Anton Vorontsov

This patch adds support for the FHCI USB controller, as found in the Freescale MPC836x and MPC832x processors. It can support Full or Low speed modes.

Quite a lot the hardware is doing by itself (SOF generation, CRC generation and checking), though scheduling and retransmission is on software's shoulders.

This controller does not integrate the root hub, so this driver also fakes one-port hub. External hub is required to support more than one device.

It looks like the kernel this was written for is several versions out of date. The driver is missing some critical calls to functions like usb_hcd_link_urb_to_ep().

Alan Stern

author
#4 lore raw
Quoted 10 lines · Alan Stern
Quoted 8 lines · Anton Vorontsov

This patch adds support for the FHCI USB controller, as found in the Freescale MPC836x and MPC832x processors. It can support Full or Low speed modes.

Quite a lot the hardware is doing by itself (SOF generation, CRC generation and checking), though scheduling and retransmission is on software's shoulders.

Quoted 6 lines
Quoted 3 lines

This controller does not integrate the root hub, so this driver also fakes one-port hub. External hub is required to support more than one device.

It looks like the kernel this was written for is several versions out of date.

Yeah, originally it was written for 2.6.10. Though the driver I posted is for the linux-next, and it works quite well w/o the {un,}link_urb calls. ;-)

Quoted 2 lines

The driver is missing some critical calls to functions like usb_hcd_link_urb_to_ep().

Fixed.

Thanks for review!

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
author
#5 lore raw

PATCH 1/3 USB: FHCI: Driver should be responsible for managing endpoint queues

In reply to Anton Vorontsov

Follow these changes for the FHCI driver:

commit e9df41c5c5899259541dc928872cad4d07b82076

Author: Alan Stern <[email protected]>
Date:   Wed Aug 8 11:48:02 2007 -0400

USB: make HCDs responsible for managing endpoint queues

This patch (as954) implements a suggestion of David Brownell's. Now the host controller drivers are responsible for linking and unlinking URBs to/from their endpoint queues. This eliminates the possiblity of strange situations where usbcore thinks an URB is linked but the HCD thinks it isn't. It also means HCDs no longer have to check for URBs being dequeued before they were fully enqueued.

In addition to the core changes, this requires changing every host controller driver and the root-hub URB handler. For the most part the required changes are fairly small; drivers have to call usb_hcd_link_urb_to_ep() in their urb_enqueue method, usb_hcd_check_unlink_urb() in their urb_dequeue method, and usb_hcd_unlink_urb_from_ep() before giving URBs back. A few HCDs make matters more complicated by the way they split up the flow of control.

In addition some method interfaces get changed. The endpoint argument for urb_enqueue is now redundant so it is removed. The unlink status is required by usb_hcd_check_unlink_urb(), so it has been added to urb_dequeue.

Signed-off-by Anton Vorontsov <[email protected]>
---
 drivers/usb/host/fhci-hcd.c |   24 ++++++++++++++++++++----
 drivers/usb/host/fhci-q.c   |    3 +++
 2 files changed, 23 insertions(+), 4 deletions(-)
drivers/usb/host/fhci-hcd.c +20 -4
index e47b8e9..47ac33f 100644
@@ -385,7 +385,9 @@ static int fhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
 {
 	struct fhci_hcd *fhci = hcd_to_fhci(hcd);
 	u32 pipe = urb->pipe;
-	int i, size = 0;
+	int ret;
+	int i;
+	int size = 0;
 	struct urb_priv *urb_priv;
 	unsigned long flags;
 

@@ -434,6 +436,11 @@ static int fhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
 	}
 
 	spin_lock_irqsave(&fhci->lock, flags);
+
+	ret = usb_hcd_link_urb_to_ep(hcd, urb);
+	if (ret)
+		goto err;
+
 	/* fill the private part of the URB */
 	urb_priv->num_of_tds = size;
 

@@ -443,9 +450,13 @@ static int fhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
 	urb->hcpriv = urb_priv;
 
 	queue_urb(fhci, urb);
-
+err:
+	if (ret) {
+		kfree(urb_priv->tds);
+		kfree(urb_priv);
+	}
 	spin_unlock_irqrestore(&fhci->lock, flags);
-	return 0;
+	return ret;
 }
 
 /* dequeue FHCI URB */

@@ -453,6 +464,7 @@ static int fhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 {
 	struct fhci_hcd *fhci = hcd_to_fhci(hcd);
 	struct fhci_usb *usb = fhci->usb_lld;
+	int ret = -EINVAL;
 	unsigned long flags;
 
 	if (!urb || !urb->dev || !urb->dev->bus)

@@ -460,6 +472,10 @@ static int fhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 
 	spin_lock_irqsave(&fhci->lock, flags);
 
+	ret = usb_hcd_check_unlink_urb(hcd, urb, status);
+	if (ret)
+		goto out2;
+
 	if (usb->port_status != FHCI_PORT_DISABLED) {
 		struct urb_priv *urb_priv;
 

@@ -483,7 +499,7 @@ static int fhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 out2:
 	spin_unlock_irqrestore(&fhci->lock, flags);
 out:
-	return 0;
+	return ret;
 }
 
 static void fhci_endpoint_disable(struct usb_hcd *hcd,
drivers/usb/host/fhci-q.c +3 -0
index 721d07d..1173318 100644
@@ -200,6 +200,9 @@ void urb_complete_free(struct fhci_hcd *fhci, struct urb *urb)
 		else
 			urb->status = 0;
 	}
+
+	usb_hcd_unlink_urb_from_ep(fhci_to_hcd(fhci), urb);
+
 	spin_unlock(&fhci->lock);
 
 	usb_hcd_giveback_urb(fhci_to_hcd(fhci), urb, urb->status);
Signature · 1 line
1.5.6.5
#6 lore raw

Re: [PATCH -mm 1/3] USB: FHCI: Driver should be responsible for managing endpoint queues

Quoted 1 line · Anton Vorontsov

Follow these changes for the FHCI driver:

Hm, would it just make more sense to respin the whole driver? That way it's never in the tree in a "broken" state?

thanks,

greg k-h

author
#7 lore raw

Re: [PATCH -mm 1/3] USB: FHCI: Driver should be responsible for managing endpoint queues

Quoted 5 lines · Greg KH
Quoted 1 line · Anton Vorontsov

Follow these changes for the FHCI driver:

Hm, would it just make more sense to respin the whole driver? That way it's never in the tree in a "broken" state?

It's for -mm tree. If I understand -mm workflow correctly, Andrew will merge all the fixes into the final patch before sending it to you in late -rc0 phase, when various -next git trees (on which this driver depend) will be merged into the Linus tree.

Andrew, am I correct?

Thanks,

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#8 lore raw

Re: [PATCH -mm 1/3] USB: FHCI: Driver should be responsible for managing endpoint queues

Quoted 7 lines · On Wed, 24 Dec 2008 23:08:45 +0300 Anton Vorontsov <[email protected]>…
Quoted 5 lines · Greg KH
Quoted 1 line · Anton Vorontsov

Follow these changes for the FHCI driver:

Hm, would it just make more sense to respin the whole driver? That way it's never in the tree in a "broken" state?

Quoted 6 lines

It's for -mm tree. If I understand -mm workflow correctly, Andrew will merge all the fixes into the final patch before sending it to you in late -rc0 phase, when various -next git trees (on which this driver depend) will be merged into the Linus tree.

Andrew, am I correct?

Yes, that's what I do.

I prefer to see the incremental patches once a patch has had review or testing, so we can see which bits changed. But if Greg intends to merge this driver then he'd probably prefer a fresh new patch, particularly as a) version 1 was only sent yesterday and b) he hasn't merged version #1.

author
#9 lore raw

PATCH v2 1/1 USB: Driver for Freescale QUICC Engine USB Host Controller

This patch adds support for the FHCI USB controller, as found in the Freescale MPC836x and MPC832x processors. It can support Full or Low speed modes.

Quite a lot the hardware is doing by itself (SOF generation, CRC generation and checking), though scheduling and retransmission is on software's shoulders.

This controller does not integrate the root hub, so this driver also fakes one-port hub. External hub is required to support more than one device.

Signed-off-by Anton Vorontsov <[email protected]>

---

Quoted 15 lines · Andrew Morton
Quoted 7 lines · On Wed, 24 Dec 2008 23:08:45 +0300 Anton Vorontsov <[email protected]>…
Quoted 5 lines · Greg KH
Quoted 1 line · Anton Vorontsov

Follow these changes for the FHCI driver:

Hm, would it just make more sense to respin the whole driver? That way it's never in the tree in a "broken" state?

Quoted 6 lines

It's for -mm tree. If I understand -mm workflow correctly, Andrew will merge all the fixes into the final patch before sending it to you in late -rc0 phase, when various -next git trees (on which this driver depend) will be merged into the Linus tree.

Andrew, am I correct?

Quoted 7 lines

Yes, that's what I do.

I prefer to see the incremental patches once a patch has had review or testing, so we can see which bits changed. But if Greg intends to merge this driver then he'd probably prefer a fresh new patch, particularly as a) version 1 was only sent yesterday and b) he hasn't merged version #1.

OK, just in case, here is the fresh new patch, following changes incorporated:

v2:
- Driver should be responsible for managing endpoint queues
- Fix namespace pollution
- Fix memory leaks in fhci_mem_{init,free}

 drivers/usb/Makefile          |    1 +
 drivers/usb/host/Kconfig      |   17 +
 drivers/usb/host/Makefile     |    6 +
 drivers/usb/host/fhci-dbg.c   |  139 +++++++
 drivers/usb/host/fhci-hcd.c   |  836 ++++++++++++++++++++++++++++++++++++++
 drivers/usb/host/fhci-hub.c   |  345 ++++++++++++++++
 drivers/usb/host/fhci-mem.c   |  113 ++++++
 drivers/usb/host/fhci-q.c     |  284 +++++++++++++
 drivers/usb/host/fhci-sched.c |  888 +++++++++++++++++++++++++++++++++++++++++
 drivers/usb/host/fhci-tds.c   |  626 +++++++++++++++++++++++++++++
 drivers/usb/host/fhci.h       |  607 ++++++++++++++++++++++++++++
 11 files changed, 3862 insertions(+), 0 deletions(-)
 create mode 100644 drivers/usb/host/fhci-dbg.c
 create mode 100644 drivers/usb/host/fhci-hcd.c
 create mode 100644 drivers/usb/host/fhci-hub.c
 create mode 100644 drivers/usb/host/fhci-mem.c
 create mode 100644 drivers/usb/host/fhci-q.c
 create mode 100644 drivers/usb/host/fhci-sched.c
 create mode 100644 drivers/usb/host/fhci-tds.c
 create mode 100644 drivers/usb/host/fhci.h
drivers/usb/Makefile +1 -0
index 8b7c419..8bcde8c 100644
@@ -13,6 +13,7 @@ obj-$(CONFIG_USB_EHCI_HCD)	+= host/
 obj-$(CONFIG_USB_ISP116X_HCD)	+= host/
 obj-$(CONFIG_USB_OHCI_HCD)	+= host/
 obj-$(CONFIG_USB_UHCI_HCD)	+= host/
+obj-$(CONFIG_USB_FHCI_HCD)	+= host/
 obj-$(CONFIG_USB_SL811_HCD)	+= host/
 obj-$(CONFIG_USB_U132_HCD)	+= host/
 obj-$(CONFIG_USB_R8A66597_HCD)	+= host/
drivers/usb/host/Kconfig +17 -0
index f3a75a9..cd7c691 100644
@@ -225,6 +225,23 @@ config USB_UHCI_HCD
 	  To compile this driver as a module, choose M here: the
 	  module will be called uhci-hcd.
 
+config USB_FHCI_HCD
+	tristate "Freescale QE USB Host Controller support"
+	depends on USB && OF_GPIO && QE_GPIO && QUICC_ENGINE
+	select FSL_GTM
+	select QE_USB
+	help
+	  This driver enables support for Freescale QE USB Host Controller
+	  (as found on MPC8360 and MPC8323 processors), the driver supports
+	  Full and Low Speed USB.
+
+config FHCI_DEBUG
+	bool "Freescale QE USB Host Controller debug support"
+	depends on USB_FHCI_HCD && DEBUG_FS
+	help
+	  Say "y" to see some FHCI debug information and statistics
+	  throught debugfs.
+
 config USB_U132_HCD
 	tristate "Elan U132 Adapter Host Controller"
 	depends on USB && USB_FTDI_ELAN
drivers/usb/host/Makefile +6 -0
index 23be222..721d373 100644
@@ -7,6 +7,11 @@ ifeq ($(CONFIG_USB_DEBUG),y)
 endif
 
 isp1760-objs := isp1760-hcd.o isp1760-if.o
+fhci-objs := fhci-hcd.o fhci-hub.o fhci-q.o fhci-mem.o \
+	     fhci-tds.o fhci-sched.o
+ifeq ($(CONFIG_FHCI_DEBUG),y)
+fhci-objs += fhci-dbg.o
+endif
 
 obj-$(CONFIG_USB_WHCI_HCD)	+= whci/
 

@@ -16,6 +21,7 @@ obj-$(CONFIG_USB_EHCI_HCD)	+= ehci-hcd.o
 obj-$(CONFIG_USB_ISP116X_HCD)	+= isp116x-hcd.o
 obj-$(CONFIG_USB_OHCI_HCD)	+= ohci-hcd.o
 obj-$(CONFIG_USB_UHCI_HCD)	+= uhci-hcd.o
+obj-$(CONFIG_USB_FHCI_HCD)	+= fhci.o
 obj-$(CONFIG_USB_SL811_HCD)	+= sl811-hcd.o
 obj-$(CONFIG_USB_SL811_CS)	+= sl811_cs.o
 obj-$(CONFIG_USB_U132_HCD)	+= u132-hcd.o
drivers/usb/host/fhci-dbg.c new file +139 -0

139 lines not on this page · raw message

drivers/usb/host/fhci-hcd.c new file +836 -0

836 lines not on this page · raw message

drivers/usb/host/fhci-hub.c new file +345 -0

345 lines not on this page · raw message

drivers/usb/host/fhci-mem.c new file +113 -0

113 lines not on this page · raw message

drivers/usb/host/fhci-q.c new file +284 -0

284 lines not on this page · raw message

drivers/usb/host/fhci-sched.c new file +888 -0

888 lines not on this page · raw message

drivers/usb/host/fhci-tds.c new file +626 -0

347 lines not on this page · raw message

drivers/usb/host/fhci.h new file +607 -0

607 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

Signature · 1 line
1.5.6.5

Part of the diff is not shown here. View the raw message.

#10 lore raw

Re: [PATCH -mm 1/3] USB: FHCI: Driver should be responsible for managing endpoint queues

In reply to Andrew Morton
Quoted 16 lines · Andrew Morton
Quoted 7 lines · On Wed, 24 Dec 2008 23:08:45 +0300 Anton Vorontsov <[email protected]>…
Quoted 5 lines · Greg KH
Quoted 1 line · Anton Vorontsov

Follow these changes for the FHCI driver:

Hm, would it just make more sense to respin the whole driver? That way it's never in the tree in a "broken" state?

Quoted 6 lines

It's for -mm tree. If I understand -mm workflow correctly, Andrew will merge all the fixes into the final patch before sending it to you in late -rc0 phase, when various -next git trees (on which this driver depend) will be merged into the Linus tree.

Andrew, am I correct?

Quoted 7 lines

Yes, that's what I do.

I prefer to see the incremental patches once a patch has had review or testing, so we can see which bits changed. But if Greg intends to merge this driver then he'd probably prefer a fresh new patch, particularly as a) version 1 was only sent yesterday and b) he hasn't merged version #1.

I do intend to merge it, given that it's in a buildable format :)

So a merged version would be good for me.

thanks,

greg k-h

author
#11 lore raw

Re: [PATCH -mm 1/3] USB: FHCI: Driver should be responsible for managing endpoint queues

Quoted 19 lines · Greg KH
Quoted 18 lines · Andrew Morton
Quoted 7 lines · On Wed, 24 Dec 2008 23:08:45 +0300 Anton Vorontsov <[email protected]>…
Quoted 5 lines · Greg KH
Quoted 1 line · Anton Vorontsov

Follow these changes for the FHCI driver:

Hm, would it just make more sense to respin the whole driver? That way it's never in the tree in a "broken" state?

Quoted 6 lines

It's for -mm tree. If I understand -mm workflow correctly, Andrew will merge all the fixes into the final patch before sending it to you in late -rc0 phase, when various -next git trees (on which this driver depend) will be merged into the Linus tree.

Andrew, am I correct?

Yes, that's what I do.

Quoted 7 lines
Quoted 5 lines

I prefer to see the incremental patches once a patch has had review or testing, so we can see which bits changed. But if Greg intends to merge this driver then he'd probably prefer a fresh new patch, particularly as a) version 1 was only sent yesterday and b) he hasn't merged version #1.

I do intend to merge it, given that it's in a buildable format :)

The point was that it's not buildable without linux-next queue (mainly few patches from the powerpc-next git tree). So if you apply it onto the pure linux-usb tree it won't build. Though it will build in the -mm tree, as -mm bases on the linux-next.

That's exactly why I asked Andrew to merge it into the -mm tree in the first place -- he would resend it to you at appropriate time.

Thanks,

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#12 lore raw

Re: [PATCH -mm 1/3] USB: FHCI: Driver should be responsible for managing endpoint queues

In reply to Anton Vorontsov
Quoted 7 lines · Anton Vorontsov

Follow these changes for the FHCI driver:

commit e9df41c5c5899259541dc928872cad4d07b82076

Author: Alan Stern <[email protected]>
Date:   Wed Aug 8 11:48:02 2007 -0400

USB: make HCDs responsible for managing endpoint queues

On the whole this looks good.

drivers/usb/host/fhci-q.c void urb_complete_free(struct fhci_hcd *fhci, struct urb *urb) · line 208

Quoted 12 lines
drivers/usb/host/fhci-q.c +3 -0
@@ -200,6 +200,9 @@ void urb_complete_free(struct fhci_hcd *fhci, struct urb *urb)
 		else
 			urb->status = 0;
 	}
+
+	usb_hcd_unlink_urb_from_ep(fhci_to_hcd(fhci), urb);
+
 	spin_unlock(&fhci->lock);
 
 	usb_hcd_giveback_urb(fhci_to_hcd(fhci), urb, urb->status);

For the future you might think about not using urb->status at all. The intention is eventually to remove that field.

Alan Stern

author
#13 lore raw

PATCH 2/3 USB: FHCI: Fix namespace pollution

In reply to Anton Vorontsov

Prepend fhci_ prefixes to the external function names.

Signed-off-by Anton Vorontsov <[email protected]>
---
 drivers/usb/host/fhci-hcd.c   |   36 ++++++++--------
 drivers/usb/host/fhci-hub.c   |   29 +++++++------
 drivers/usb/host/fhci-mem.c   |   14 +++---
 drivers/usb/host/fhci-q.c     |   28 ++++++------
 drivers/usb/host/fhci-sched.c |   94 ++++++++++++++++++++--------------------
 drivers/usb/host/fhci-tds.c   |   51 +++++++++++-----------
 drivers/usb/host/fhci.h       |   90 +++++++++++++++++++-------------------
 7 files changed, 171 insertions(+), 171 deletions(-)
drivers/usb/host/fhci-hcd.c +18 -18
index 47ac33f..269af3c 100644
@@ -55,7 +55,7 @@ void fhci_stop_sof_timer(struct fhci_hcd *fhci)
 	fhci_dbg(fhci, "<- %s\n", __func__);
 }
 
-u16 get_sof_timer_count(struct fhci_usb *usb)
+u16 fhci_get_sof_timer_count(struct fhci_usb *usb)
 {
 	return be16_to_cpu(in_be16(&usb->fhci->regs->usb_sof_tmr) / 12);
 }

@@ -67,12 +67,12 @@ static u32 endpoint_zero_init(struct fhci_usb *usb,
 {
 	u32 rc;
 
-	rc = create_endpoint(usb, data_mem, ring_len);
+	rc = fhci_create_ep(usb, data_mem, ring_len);
 	if (rc)
 		return rc;
 
 	/* inilialize endpoint registers */
-	init_endpoint_registers(usb, usb->ep0, data_mem);
+	fhci_init_ep_registers(usb, usb->ep0, data_mem);
 
 	return 0;
 }

@@ -133,12 +133,12 @@ static u32 fhci_usb_disable(struct fhci_hcd *fhci)
 	struct fhci_usb *usb = fhci->usb_lld;
 
 	fhci_usb_disable_interrupt(usb);
-	usb_port_disable(fhci);
+	fhci_port_disable(fhci);
 
 	/* disable the usb controller */
 	if (usb->port_status == FHCI_PORT_FULL ||
 			usb->port_status == FHCI_PORT_LOW)
-		device_disconnected_interrupt(fhci);
+		fhci_device_disconnected_interrupt(fhci);
 
 	clrbits8(&usb->fhci->regs->usb_mod, USB_MODE_EN);
 

@@ -213,7 +213,7 @@ static int fhci_mem_init(struct fhci_hcd *fhci)
 			error = 1;
 			break;
 		}
-		recycle_empty_td(fhci, td);
+		fhci_recycle_empty_td(fhci, td);
 	}
 	for (i = 0; i < MAX_EDS; i++) {
 		struct ed *ed = kmalloc(sizeof(*ed), GFP_KERNEL);

@@ -222,7 +222,7 @@ static int fhci_mem_init(struct fhci_hcd *fhci)
 			error = 1;
 			break;
 		}
-		recycle_empty_ed(fhci, ed);
+		fhci_recycle_empty_ed(fhci, ed);
 	}
 
 	if (error) {

@@ -242,8 +242,8 @@ static void fhci_usb_free(void *lld)
 	struct fhci_hcd *fhci = usb->fhci;
 
 	if (usb) {
-		config_transceiver(fhci, FHCI_PORT_POWER_OFF);
-		endpoint_zero_free(usb);
+		fhci_config_transceiver(fhci, FHCI_PORT_POWER_OFF);
+		fhci_ep0_free(usb);
 		kfree(usb->actual_frame);
 		kfree(usb);
 	}

@@ -312,7 +312,7 @@ static struct fhci_usb *fhci_create_lld(struct fhci_hcd *fhci)
 	usb->hc_list = fhci->hc_list;
 	usb->vroot_hub = fhci->vroot_hub;
 
-	usb->transfer_confirm = transfer_confirm_callback;
+	usb->transfer_confirm = fhci_transfer_confirm_callback;
 
 	return usb;
 }

@@ -449,7 +449,7 @@ static int fhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
 	urb->error_count = 0;
 	urb->hcpriv = urb_priv;
 
-	queue_urb(fhci, urb);
+	fhci_queue_urb(fhci, urb);
 err:
 	if (ret) {
 		kfree(urb_priv->tds);

@@ -493,7 +493,7 @@ static int fhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 		/* already pending? */
 		urb_priv->ed->state = FHCI_ED_URB_DEL;
 	} else {
-		urb_complete_free(fhci, urb);
+		fhci_urb_complete_free(fhci, urb);
 	}
 
 out2:

@@ -514,10 +514,10 @@ static void fhci_endpoint_disable(struct usb_hcd *hcd,
 	ed = ep->hcpriv;
 	if (ed) {
 		while (ed->td_head != NULL) {
-			struct td *td = remove_td_from_ed(ed);
-			urb_complete_free(fhci, td->urb);
+			struct td *td = fhci_remove_td_from_ed(ed);
+			fhci_urb_complete_free(fhci, td->urb);
 		}
-		recycle_empty_ed(fhci, ed);
+		fhci_recycle_empty_ed(fhci, ed);
 		ep->hcpriv = NULL;
 	}
 	spin_unlock_irqrestore(&fhci->lock, flags);

@@ -726,14 +726,14 @@ static int __devinit of_fhci_probe(struct of_device *ofdev,
 
 	dev_info(dev, "at 0x%p, irq %d\n", hcd->regs, usb_irq);
 
-	config_transceiver(fhci, FHCI_PORT_POWER_OFF);
+	fhci_config_transceiver(fhci, FHCI_PORT_POWER_OFF);
 
 	/* Start with full-speed, if possible. */
 	if (fhci->fullspeed_clk != QE_CLK_NONE) {
-		config_transceiver(fhci, FHCI_PORT_FULL);
+		fhci_config_transceiver(fhci, FHCI_PORT_FULL);
 		qe_usb_clock_set(fhci->fullspeed_clk, USB_CLOCK);
 	} else {
-		config_transceiver(fhci, FHCI_PORT_LOW);
+		fhci_config_transceiver(fhci, FHCI_PORT_LOW);
 		qe_usb_clock_set(fhci->lowspeed_clk, USB_CLOCK >> 3);
 	}
drivers/usb/host/fhci-hub.c +15 -14
index 540e165..0cfaedc 100644
@@ -52,7 +52,8 @@ static void fhci_gpio_set_value(struct fhci_hcd *fhci, int gpio_nr, bool on)
 	mdelay(5);
 }
 
-void config_transceiver(struct fhci_hcd *fhci, enum fhci_port_status status)
+void fhci_config_transceiver(struct fhci_hcd *fhci,
+			     enum fhci_port_status status)
 {
 	fhci_dbg(fhci, "-> %s: %d\n", __func__, status);
 

@@ -79,7 +80,7 @@ void config_transceiver(struct fhci_hcd *fhci, enum fhci_port_status status)
 }
 
 /* disable the USB port by clearing the EN bit in the USBMOD register */
-void usb_port_disable(struct fhci_hcd *fhci)
+void fhci_port_disable(struct fhci_hcd *fhci)
 {
 	struct fhci_usb *usb = (struct fhci_usb *)fhci->usb_lld;
 	enum fhci_port_status port_status;

@@ -88,7 +89,7 @@ void usb_port_disable(struct fhci_hcd *fhci)
 
 	fhci_stop_sof_timer(fhci);
 
-	flush_all_transmissions(usb);
+	fhci_flush_all_transmissions(usb);
 
 	fhci_usb_disable_interrupt((struct fhci_usb *)fhci->usb_lld);
 	port_status = usb->port_status;

@@ -100,7 +101,7 @@ void usb_port_disable(struct fhci_hcd *fhci)
 
 	/* check if during the disconnection process attached new device */
 	if (port_status == FHCI_PORT_WAITING)
-		device_connected_interrupt(fhci);
+		fhci_device_connected_interrupt(fhci);
 	usb->vroot_hub->port.wPortStatus &= ~USB_PORT_STAT_ENABLE;
 	usb->vroot_hub->port.wPortChange |= USB_PORT_STAT_C_ENABLE;
 	fhci_usb_enable_interrupt((struct fhci_usb *)fhci->usb_lld);

@@ -109,14 +110,14 @@ void usb_port_disable(struct fhci_hcd *fhci)
 }
 
 /* enable the USB port by setting the EN bit in the USBMOD register */
-void usb_port_enable(void *lld)
+void fhci_port_enable(void *lld)
 {
 	struct fhci_usb *usb = (struct fhci_usb *)lld;
 	struct fhci_hcd *fhci = usb->fhci;
 
 	fhci_dbg(fhci, "-> %s\n", __func__);
 
-	config_transceiver(fhci, usb->port_status);
+	fhci_config_transceiver(fhci, usb->port_status);
 
 	if ((usb->port_status != FHCI_PORT_FULL) &&
 			(usb->port_status != FHCI_PORT_LOW))

@@ -128,7 +129,7 @@ void usb_port_enable(void *lld)
 	fhci_dbg(fhci, "<- %s\n", __func__);
 }
 
-void io_port_generate_reset(struct fhci_hcd *fhci)
+void fhci_io_port_generate_reset(struct fhci_hcd *fhci)
 {
 	fhci_dbg(fhci, "-> %s\n", __func__);
 

@@ -146,7 +147,7 @@ void io_port_generate_reset(struct fhci_hcd *fhci)
 }
 
 /* generate the RESET condition on the bus */
-void usb_port_reset(void *lld)
+void fhci_port_reset(void *lld)
 {
 	struct fhci_usb *usb = (struct fhci_usb *)lld;
 	struct fhci_hcd *fhci = usb->fhci;

@@ -164,7 +165,7 @@ void usb_port_reset(void *lld)
 	mask = in_be16(&fhci->regs->usb_mask);
 	out_be16(&fhci->regs->usb_mask, mask & (~USB_E_IDLE_MASK));
 
-	io_port_generate_reset(fhci);
+	fhci_io_port_generate_reset(fhci);
 
 	/* enable interrupt on this endpoint */
 	out_be16(&fhci->regs->usb_mask, mask);

@@ -233,7 +234,7 @@ int fhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 		case USB_PORT_FEAT_ENABLE:
 			fhci->vroot_hub->port.wPortStatus &=
 			    ~USB_PORT_STAT_ENABLE;
-			usb_port_disable(fhci);
+			fhci_port_disable(fhci);
 			break;
 		case USB_PORT_FEAT_C_ENABLE:
 			fhci->vroot_hub->port.wPortChange &=

@@ -251,7 +252,7 @@ int fhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 		case USB_PORT_FEAT_POWER:
 			fhci->vroot_hub->port.wPortStatus &=
 			    ~USB_PORT_STAT_POWER;
-			config_transceiver(fhci, FHCI_PORT_POWER_OFF);
+			fhci_config_transceiver(fhci, FHCI_PORT_POWER_OFF);
 			break;
 		case USB_PORT_FEAT_C_CONNECTION:
 			fhci->vroot_hub->port.wPortChange &=

@@ -306,7 +307,7 @@ int fhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 		case USB_PORT_FEAT_ENABLE:
 			fhci->vroot_hub->port.wPortStatus |=
 			    USB_PORT_STAT_ENABLE;
-			usb_port_enable(fhci->usb_lld);
+			fhci_port_enable(fhci->usb_lld);
 			break;
 		case USB_PORT_FEAT_SUSPEND:
 			fhci->vroot_hub->port.wPortStatus |=

@@ -316,7 +317,7 @@ int fhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 		case USB_PORT_FEAT_RESET:
 			fhci->vroot_hub->port.wPortStatus |=
 			    USB_PORT_STAT_RESET;
-			usb_port_reset(fhci->usb_lld);
+			fhci_port_reset(fhci->usb_lld);
 			fhci->vroot_hub->port.wPortStatus |=
 			    USB_PORT_STAT_ENABLE;
 			fhci->vroot_hub->port.wPortStatus &=

@@ -325,7 +326,7 @@ int fhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 		case USB_PORT_FEAT_POWER:
 			fhci->vroot_hub->port.wPortStatus |=
 			    USB_PORT_STAT_POWER;
-			config_transceiver(fhci, FHCI_PORT_WAITING);
+			fhci_config_transceiver(fhci, FHCI_PORT_WAITING);
 			break;
 		default:
 			goto error;
drivers/usb/host/fhci-mem.c +7 -7
index c5ba0b0..2c0736c 100644
@@ -55,13 +55,13 @@ static struct td *get_empty_td(struct fhci_hcd *fhci)
 	return td;
 }
 
-void recycle_empty_td(struct fhci_hcd *fhci, struct td *td)
+void fhci_recycle_empty_td(struct fhci_hcd *fhci, struct td *td)
 {
 	init_td(td);
 	list_add(&td->node, &fhci->empty_tds);
 }
 
-struct ed *get_empty_ed(struct fhci_hcd *fhci)
+struct ed *fhci_get_empty_ed(struct fhci_hcd *fhci)
 {
 	struct ed *ed;
 

@@ -79,16 +79,16 @@ struct ed *get_empty_ed(struct fhci_hcd *fhci)
 	return ed;
 }
 
-void recycle_empty_ed(struct fhci_hcd *fhci, struct ed *ed)
+void fhci_recycle_empty_ed(struct fhci_hcd *fhci, struct ed *ed)
 {
 	init_ed(ed);
 	list_add(&ed->node, &fhci->empty_eds);
 }
 
-struct td *td_fill(struct fhci_hcd *fhci, struct urb *urb,
-		struct urb_priv *urb_priv, struct ed *ed, u16 index,
-		enum fhci_ta_type type, int toggle, u8 *data, u32 len,
-		u16 interval, u16 start_frame, bool ioc)
+struct td *fhci_td_fill(struct fhci_hcd *fhci, struct urb *urb,
+			struct urb_priv *urb_priv, struct ed *ed, u16 index,
+			enum fhci_ta_type type, int toggle, u8 *data, u32 len,
+			u16 interval, u16 start_frame, bool ioc)
 {
 	struct td *td = get_empty_td(fhci);
drivers/usb/host/fhci-q.c +14 -14
index 1173318..b0a1446 100644
@@ -53,12 +53,12 @@ static int status_to_error(u32 status)
 		return -EINVAL;
 }
 
-void add_td_to_frame(struct fhci_time_frame *frame, struct td *td)
+void fhci_add_td_to_frame(struct fhci_time_frame *frame, struct td *td)
 {
 	list_add_tail(&td->frame_lh, &frame->tds_list);
 }
 
-void add_tds_to_ed(struct ed *ed, struct td **td_list, int number)
+void fhci_add_tds_to_ed(struct ed *ed, struct td **td_list, int number)
 {
 	int i;
 

@@ -82,7 +82,7 @@ static struct td *peek_td_from_ed(struct ed *ed)
 	return td;
 }
 
-struct td *remove_td_from_frame(struct fhci_time_frame *frame)
+struct td *fhci_remove_td_from_frame(struct fhci_time_frame *frame)
 {
 	struct td *td;
 

@@ -95,7 +95,7 @@ struct td *remove_td_from_frame(struct fhci_time_frame *frame)
 	return td;
 }
 
-struct td *peek_td_from_frame(struct fhci_time_frame *frame)
+struct td *fhci_peek_td_from_frame(struct fhci_time_frame *frame)
 {
 	struct td *td;
 

@@ -107,7 +107,7 @@ struct td *peek_td_from_frame(struct fhci_time_frame *frame)
 	return td;
 }
 
-struct td *remove_td_from_ed(struct ed *ed)
+struct td *fhci_remove_td_from_ed(struct ed *ed)
 {
 	struct td *td;
 

@@ -127,7 +127,7 @@ struct td *remove_td_from_ed(struct ed *ed)
 	return td;
 }
 
-struct td *remove_td_from_done_list(struct fhci_controller_list *p_list)
+struct td *fhci_remove_td_from_done_list(struct fhci_controller_list *p_list)
 {
 	struct td *td;
 

@@ -140,7 +140,7 @@ struct td *remove_td_from_done_list(struct fhci_controller_list *p_list)
 	return td;
 }
 
-void move_td_from_ed_to_done_list(struct fhci_usb *usb, struct ed *ed)
+void fhci_move_td_from_ed_to_done_list(struct fhci_usb *usb, struct ed *ed)
 {
 	struct td *td;
 

@@ -169,7 +169,7 @@ static void free_urb_priv(struct fhci_hcd *fhci, struct urb *urb)
 
 	for (i = 0; i < urb_priv->num_of_tds; i++) {
 		list_del_init(&urb_priv->tds[i]->node);
-		recycle_empty_td(fhci, urb_priv->tds[i]);
+		fhci_recycle_empty_td(fhci, urb_priv->tds[i]);
 	}
 
 	/* if this TD was the ED's head,find the next TD */

@@ -189,7 +189,7 @@ static void free_urb_priv(struct fhci_hcd *fhci, struct urb *urb)
 }
 
 /* this routine called to complete and free done URB */
-void urb_complete_free(struct fhci_hcd *fhci, struct urb *urb)
+void fhci_urb_complete_free(struct fhci_hcd *fhci, struct urb *urb)
 {
 	free_urb_priv(fhci, urb);
 

@@ -214,7 +214,7 @@ void urb_complete_free(struct fhci_hcd *fhci, struct urb *urb)
  * caculate transfer length/stats and update the urb
  * Precondition: irqsafe(only for urb-?status locking)
  */
-void done_td(struct urb *urb, struct td *td)
+void fhci_done_td(struct urb *urb, struct td *td)
 {
 	struct ed *ed = td->ed;
 	u32 cc = td->status;

@@ -265,20 +265,20 @@ void done_td(struct urb *urb, struct td *td)
 }
 
 /* there are some pedning request to unlink */
-void del_ed_list(struct fhci_hcd *fhci, struct ed *ed)
+void fhci_del_ed_list(struct fhci_hcd *fhci, struct ed *ed)
 {
 	struct td *td = peek_td_from_ed(ed);
 	struct urb *urb = td->urb;
 	struct urb_priv *urb_priv = urb->hcpriv;
 
 	if (urb_priv->state == URB_DEL) {
-		td = remove_td_from_ed(ed);
+		td = fhci_remove_td_from_ed(ed);
 		/* HC may have partly processed this TD */
 		if (td->status != USB_TD_INPROGRESS)
-			done_td(urb, td);
+			fhci_done_td(urb, td);
 
 		/* URB is done;clean up */
 		if (++(urb_priv->tds_cnt) == urb_priv->num_of_tds)
-			urb_complete_free(fhci, urb);
+			fhci_urb_complete_free(fhci, urb);
 	}
 }
drivers/usb/host/fhci-sched.c +47 -47

334 lines not on this page · raw message

drivers/usb/host/fhci-tds.c +25 -26

140 lines not on this page · raw message

drivers/usb/host/fhci.h +45 -45

116 lines not on this page · raw message

Signature · 1 line
1.5.6.5
author
#14 lore raw

PATCH 3/3 USB: FHCI: Fix memory leaks in fhci_mem_{init,free}

In reply to Anton Vorontsov

This patch fixes few memory leaks. Particulary:

- On errors fhci_mem_init() leaks fhci->hc_list;
- fhci_mem_free() doesn't free the allocated eds/tds.

Signed-off-by Anton Vorontsov <[email protected]>
---
 drivers/usb/host/fhci-hcd.c |   57 ++++++++++++++++++++++--------------------
 1 files changed, 30 insertions(+), 27 deletions(-)
drivers/usb/host/fhci-hcd.c +29 -26
index 269af3c..ba622cc 100644
@@ -167,27 +167,35 @@ int fhci_ioports_check_bus_state(struct fhci_hcd *fhci)
 
 static void fhci_mem_free(struct fhci_hcd *fhci)
 {
-	struct list_head *lh;
-	struct list_head *next_lh;
+	struct ed *ed;
+	struct ed *next_ed;
+	struct td *td;
+	struct td *next_td;
 
-	list_for_each_safe(lh, next_lh, &fhci->empty_eds)
-		list_del(lh);
+	list_for_each_entry_safe(ed, next_ed, &fhci->empty_eds, node) {
+		list_del(&ed->node);
+		kfree(ed);
+	}
 
-	list_for_each_safe(lh, next_lh, &fhci->empty_tds)
-		list_del(lh);
+	list_for_each_entry_safe(td, next_td, &fhci->empty_tds, node) {
+		list_del(&td->node);
+		kfree(td);
+	}
 
 	kfree(fhci->vroot_hub);
+	fhci->vroot_hub = NULL;
+
 	kfree(fhci->hc_list);
+	fhci->hc_list = NULL;
 }
 
 static int fhci_mem_init(struct fhci_hcd *fhci)
 {
 	int i;
-	int error = 0;
 
 	fhci->hc_list = kzalloc(sizeof(*fhci->hc_list), GFP_KERNEL);
 	if (!fhci->hc_list)
-		return -ENOMEM;
+		goto err;
 
 	INIT_LIST_HEAD(&fhci->hc_list->ctrl_list);
 	INIT_LIST_HEAD(&fhci->hc_list->bulk_list);

@@ -197,7 +205,7 @@ static int fhci_mem_init(struct fhci_hcd *fhci)
 
 	fhci->vroot_hub = kzalloc(sizeof(*fhci->vroot_hub), GFP_KERNEL);
 	if (!fhci->vroot_hub)
-		return -ENOMEM;
+		goto err;
 
 	INIT_LIST_HEAD(&fhci->empty_eds);
 	INIT_LIST_HEAD(&fhci->empty_tds);

@@ -207,32 +215,27 @@ static int fhci_mem_init(struct fhci_hcd *fhci)
 	fhci->process_done_task = &fhci_tasklet;
 
 	for (i = 0; i < MAX_TDS; i++) {
-		struct td *td = kmalloc(sizeof(*td), GFP_KERNEL);
+		struct td *td;
 
-		if (!td) {
-			error = 1;
-			break;
-		}
+		td = kmalloc(sizeof(*td), GFP_KERNEL);
+		if (!td)
+			goto err;
 		fhci_recycle_empty_td(fhci, td);
 	}
 	for (i = 0; i < MAX_EDS; i++) {
-		struct ed *ed = kmalloc(sizeof(*ed), GFP_KERNEL);
+		struct ed *ed;
 
-		if (!ed) {
-			error = 1;
-			break;
-		}
+		ed = kmalloc(sizeof(*ed), GFP_KERNEL);
+		if (!ed)
+			goto err;
 		fhci_recycle_empty_ed(fhci, ed);
 	}
 
-	if (error) {
-		fhci_mem_free(fhci);
-		return -ENOMEM;
-	}
-
 	fhci->active_urbs = 0;
-
-	return error;
+	return 0;
+err:
+	fhci_mem_free(fhci);
+	return -ENOMEM;
 }
 
 /* destroy the fhci_usb structure */
Signature · 1 line
1.5.6.5