[PATCH 0/6] Cleanups to c67x00 USB host controller driver

[PATCH 0/6] Cleanups to c67x00 USB host controller driver

[Grant Likely]

Peter,

Here's the series of changes that I've made to the c67x00 driver.  Most of
them are pretty straight forward.  Most invasive is the rename of
'struct c67x00_drv' to 'struct c67x00_device' which is big and scary, but
doesn't really do much.

One thing that I haven't tackled is the layout of the lowlevel drivers.
I think it should be reworked to add a generic ops structure which each
low level driver can populate with the correct ops functions.  It should
simplify the code from the hard coded approach currently used.

I'll probably have more comments/fixes later as I work with it, but this
series is what I have now.  Please add me to the CC list when you repost
an updated driver to the list.  I'm not subscribed to linux-usb-devel.

Cheers,
g.

[PATCH 1/6] [C67x00] Add test of active flag when checking TDs

[Grant Likely]

The active flag in a TD needs to be checked to determine whether or not
the TD was processed in the frame.  Without this check, the HCD assumes
the TD completed successfully, when in reality it was not processed at all.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/usb/c67x00/c67x00-sched.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/c67x00/c67x00-sched.c b/drivers/usb/c67x00/c67x00-sched.c
index 84b40a4..952bdab 100644
--- a/drivers/usb/c67x00/c67x00-sched.c
+++ b/drivers/usb/c67x00/c67x00-sched.c
@@ -1020,8 +1020,8 @@ static void check_td_list(struct c67x00_hcd *c67x00, struct pt_regs *regs)
 			goto cont;
 		}
 
-		if ((td->status & TD_STATUSMASK_NAK) || !td_sequence_ok(td) ||
-		    !td_acked(td))
+		if ((td->status & TD_STATUSMASK_NAK) ||
+		    !td_sequence_ok(td) || !td_acked(td) || td_active(td))
 			goto cont;
 
 		/* Sequence ok and acked, don't need to fix toggle */
-- 
1.4.4.2

[PATCH 2/6] [C67x00] Fix calculation of frame bandwidth

[Grant Likely]

Use the correct formulas and values for calculating the bittime within
a frame.
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/usb/c67x00/c67x00-hcd.h   |   10 ++++++--
 drivers/usb/c67x00/c67x00-sched.c |   42 +++++++++++++++++++++++++++++++++---
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/c67x00/c67x00-hcd.h b/drivers/usb/c67x00/c67x00-hcd.h
index 8f8eff1..94ce7ec 100644
--- a/drivers/usb/c67x00/c67x00-hcd.h
+++ b/drivers/usb/c67x00/c67x00-hcd.h
@@ -46,12 +46,16 @@
  * The current implementation switches between _STD (default) and _ISO (when
  * isochronous transfers are scheduled), in order to optimize the throughput
  * in normal cicrumstances, but also provide good isochronous behaviour.
+ *
+ * Bandwidth is described in bit time so with a 12MHz USB clock and 1ms
+ * frames; there are 12000 bit times per frame.
  */
 
-#define MAX_FRAME_BW_STD	4000
-#define MAX_FRAME_BW_ISO	2400
+#define TOTAL_FRAME_BW		12000
+#define DEFAULT_EOT		2250
 
-#define DEFAULT_EOT		6500
+#define MAX_FRAME_BW_STD	(TOTAL_FRAME_BW - DEFAULT_EOT)
+#define MAX_FRAME_BW_ISO	2400
 
 /*
  * Periodic transfers may only use 90% of the full frame, but as
diff --git a/drivers/usb/c67x00/c67x00-sched.c b/drivers/usb/c67x00/c67x00-sched.c
index 952bdab..0d08af7 100644
--- a/drivers/usb/c67x00/c67x00-sched.c
+++ b/drivers/usb/c67x00/c67x00-sched.c
@@ -520,11 +520,45 @@ static inline void giveback_urb(struct c67x00_hcd *c67x00, struct urb *urb,
 
 /* -------------------------------------------------------------------------- */
 
-static int claim_frame_bw(struct c67x00_hcd *c67x00, int len, int periodic)
+static int claim_frame_bw(struct c67x00_hcd *c67x00, struct urb *urb,
+			  int len, int periodic)
 {
-	int bit_time = len * 8;
+	struct urb_priv *urbp = urb->hcpriv;
+	int bit_time;
+
+	/* According to the C67x00 BIOS user manual, page 3-18,19, the
+	 * following calculations provide the full speed bit times for
+	 * a transaction.
+	 *
+	 * FS(in)	= 112.5 +  9.36*BC + HOST_DELAY
+	 * FS(in,iso)	=  90.5 +  9.36*BC + HOST_DELAY
+	 * FS(out)	= 112.5 +  9.36*BC + HOST_DELAY
+	 * FS(out,iso)	=  78.4 +  9.36*BC + HOST_DELAY
+	 * LS(in)	= 802.4 + 75.78*BC + HOST_DELAY
+	 * LS(out)	= 802.6 + 74.67*BC + HOST_DELAY
+	 *
+	 * HOST_DELAY == 106 for the c67200 and c67300.
+	 */
+
+	/* make calculations in 1/100 bit times to maintain resolution */
+	if (urbp->ep_data->dev->speed == USB_SPEED_LOW) {
+		/* Low speed pipe */
+		if (usb_pipein(urb->pipe))
+			bit_time = 80240 + 7578*len;
+		else
+			bit_time = 80260 + 7467*len;
+	} else {
+		/* FS pipes */
+		if (usb_pipeisoc(urb->pipe))
+			bit_time = usb_pipein(urb->pipe) ? 9050 : 7840;
+		else
+			bit_time = 11250;
+		bit_time += 936*len;
+	}
 
-	/* TODO don't we need to take low speed into regards? */
+	/* Scale back down to integer bit times.  Use a host delay of 106.
+	 * (this is the only place it is used) */
+	bit_time = ((bit_time+50) / 100) + 106;
 
 	if (unlikely(bit_time + c67x00->bandwidth_allocated >=
 		     c67x00->max_frame_bw))
@@ -565,7 +599,7 @@ static int create_td(struct c67x00_hcd *c67x00,
 	__u8 cmd = 0;
 	int tt = 0;
 
-	if (claim_frame_bw(c67x00, len,
+	if (claim_frame_bw(c67x00, urb, len,
 			   usb_pipeisoc(urb->pipe) || usb_pipeint(urb->pipe)))
 		return -EMSGSIZE;	/* Not really an error, but expected */
 
-- 
1.4.4.2

[PATCH 3/6] [C67x00] Remove unnecessary references to pt_regs

[Grant Likely]

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/usb/c67x00/c67x00-sched.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/c67x00/c67x00-sched.c b/drivers/usb/c67x00/c67x00-sched.c
index 0d08af7..3a870cf 100644
--- a/drivers/usb/c67x00/c67x00-sched.c
+++ b/drivers/usb/c67x00/c67x00-sched.c
@@ -956,7 +956,7 @@ static inline void clear_pipe(struct c67x00_hcd *c67x00,
 /* -------------------------------------------------------------------------- */
 
 static void handle_successful_td(struct c67x00_hcd *c67x00,
-				 struct c67x00_td *td, struct pt_regs *regs)
+				 struct c67x00_td *td)
 {
 	struct urb *urb = td->urb;
 
@@ -1025,7 +1025,7 @@ static void handle_isoc(struct c67x00_hcd *c67x00, struct c67x00_td *td)
  * check_td_list - handle tds which have been processed by the c67x00
  * pre: current_td == 0
  */
-static void check_td_list(struct c67x00_hcd *c67x00, struct pt_regs *regs)
+static void check_td_list(struct c67x00_hcd *c67x00)
 {
 	struct c67x00_td *td, *tmp;
 	struct urb *urb;
@@ -1070,7 +1070,7 @@ static void check_td_list(struct c67x00_hcd *c67x00, struct pt_regs *regs)
 		}
 
 		clear_endpoint = 0;
-		handle_successful_td(c67x00, td, regs);
+		handle_successful_td(c67x00, td);
 
 	      cont:
 		if (clear_endpoint)
@@ -1132,7 +1132,7 @@ static void c67x00_do_work(struct c67x00_hcd *c67x00)
 	if (!all_tds_processed(c67x00))
 		goto out;
 
-	check_td_list(c67x00, NULL);
+	check_td_list(c67x00);
 
 	/* no td's are being processed (current == 0)
 	 * and all have been "checked" */
-- 
1.4.4.2

[PATCH 4/6] [C67x00] Added error handling paths to lowlevel interface code

[Grant Likely]

Fix up some of the error paths in the low level code to not go into an
endless loop.  Replace the endless loops with failout code.

This patch is just a first step.  It eliminates the endless loops, but
some of the code paths don't yet have a failure path, so instead the
driver uses BUG_ON() to die with lots of noise.  The driver needs to be
refactored to add in the failure paths so the driver can fail gracefully

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/usb/c67x00/c67x00-drv.c    |    8 ++++-
 drivers/usb/c67x00/c67x00-ll-hpi.c |   57 ++++++++++++++++++++++-------------
 drivers/usb/c67x00/c67x00-ll-hpi.h |    6 ++--
 3 files changed, 46 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/c67x00/c67x00-drv.c b/drivers/usb/c67x00/c67x00-drv.c
index 6b38248..2737344 100644
--- a/drivers/usb/c67x00/c67x00-drv.c
+++ b/drivers/usb/c67x00/c67x00-drv.c
@@ -209,7 +209,11 @@ static int __devinit usb_c67x00_drv_probe(struct platform_device *pdev)
 		goto request_irq_failed;
 	}
 
-	c67x00_ll_reset(drv);
+	ret = c67x00_ll_reset(drv);
+	if (ret) {
+		dev_err(&pdev->dev, "Device reset failed\n");
+		goto reset_failed;
+	}
 
 	for (i = 0; i < C67X00_SIES; i++)
 		probe_sie(&drv->sie[i]);
@@ -218,6 +222,8 @@ static int __devinit usb_c67x00_drv_probe(struct platform_device *pdev)
 
 	return 0;
 
+reset_failed:
+	free_irq(res2->start, drv);
 request_irq_failed:
 	iounmap(drv->hpi.base);
 map_failed:
diff --git a/drivers/usb/c67x00/c67x00-ll-hpi.c b/drivers/usb/c67x00/c67x00-ll-hpi.c
index f47ce79..868736a 100644
--- a/drivers/usb/c67x00/c67x00-ll-hpi.c
+++ b/drivers/usb/c67x00/c67x00-ll-hpi.c
@@ -243,7 +243,7 @@ static inline u16 ll_recv_msg(struct c67x00_drv *drv)
 	INIT_COMPLETION(drv->lcp.msg_received);
 	WARN_ON(!res);
 
-	return res;
+	return (res == 0) ? -EIO : 0;
 }
 
 static inline void ll_release(struct c67x00_drv *drv)
@@ -283,18 +283,22 @@ static inline void c67x00_ll_husb_sie_init(struct c67x00_sie *sie)
 {
 	struct c67x00_drv *drv = sie->drv;
 	struct lcp_int_data data;
+	int rc;
 
-	c67x00_comm_exec_int(drv, HUSB_SIE_INIT_INT(sie->sie_num), &data);
+	rc = c67x00_comm_exec_int(drv, HUSB_SIE_INIT_INT(sie->sie_num), &data);
+	BUG_ON(rc); /* No return path for error code; crash spectacularly */
 }
 
 void c67x00_ll_husb_reset(struct c67x00_sie *sie, int port)
 {
 	struct c67x00_drv *drv = sie->drv;
 	struct lcp_int_data data;
+	int rc;
 
 	data.regs[0] = 50;	/* Reset USB port for 50ms */
 	data.regs[1] = port | (sie->sie_num << 1);
-	c67x00_comm_exec_int(drv, HUSB_RESET_INT, &data);
+	rc = c67x00_comm_exec_int(drv, HUSB_RESET_INT, &data);
+	BUG_ON(rc); /* No return path for error code; crash spectacularly */
 }
 
 void c67x00_ll_husb_set_current_td(struct c67x00_sie *sie, u16 addr)
@@ -358,10 +362,12 @@ void c67x00_ll_susb_init(struct c67x00_sie *sie)
 {
 	struct c67x00_drv *drv = sie->drv;
 	struct lcp_int_data data;
+	int rc;
 
 	data.regs[1] = 1;	/* full speed */
 	data.regs[2] = sie->sie_num + 1;
-	c67x00_comm_exec_int(drv, SUSB_INIT_INT, &data);
+	rc = c67x00_comm_exec_int(drv, SUSB_INIT_INT, &data);
+	BUG_ON(rc); /* No return path for error code; crash spectacularly */
 
 	hpi_clear_bits(drv, HPI_IRQ_ROUTING_REG,
 		       SOFEOP_TO_HPI_EN(sie->sie_num));
@@ -384,45 +390,54 @@ void c67x00_ll_irq(struct c67x00_drv *drv)
 u16 c67x00_comm_read_ctrl_reg(struct c67x00_drv * drv, u16 addr)
 {
 	unsigned long msg, res;
+	int rc;
 
 	ll_start(drv);
-	do {
-		hpi_write_word(drv, COMM_CTRL_REG_ADDR, addr);
-		hpi_send_mbox(drv, COMM_READ_CTRL_REG);
-	} while (!ll_recv_msg(drv));
+	hpi_write_word(drv, COMM_CTRL_REG_ADDR, addr);
+	hpi_send_mbox(drv, COMM_READ_CTRL_REG);
+	rc = ll_recv_msg(drv);
+
+	BUG_ON(rc); /* No return path for error code; crash spectacularly */
+
 	msg = drv->lcp.last_msg;
 	if (msg != COMM_ACK) {
 		dev_warn(&drv->pdev->dev, "COMM_READ_CTRL_REG didn't ACK!\n");
 		res = 0;
-	} else
+	} else {
 		res = hpi_read_word(drv, COMM_CTRL_REG_DATA);
+	}
 	ll_release(drv);
 	return res;
 }
 
-void c67x00_comm_exec_int(struct c67x00_drv *drv, u16 nr,
+int c67x00_comm_exec_int(struct c67x00_drv *drv, u16 nr,
 			  struct lcp_int_data *data)
 {
+	int i, rc;
+
 	ll_start(drv);
-	do {
-		int i;
-		hpi_write_word(drv, COMM_INT_NUM, nr);
-		for (i = 0; i < COMM_REGS; i++)
-			hpi_write_word(drv, COMM_R(i), data->regs[i]);
-		hpi_send_mbox(drv, COMM_EXEC_INT);
-	} while (!ll_recv_msg(drv));
+	hpi_write_word(drv, COMM_INT_NUM, nr);
+	for (i = 0; i < COMM_REGS; i++)
+		hpi_write_word(drv, COMM_R(i), data->regs[i]);
+	hpi_send_mbox(drv, COMM_EXEC_INT);
+	rc = ll_recv_msg(drv);
 	ll_release(drv);
+
+	return rc;
 }
 
 /* -------------------------------------------------------------------------- */
 
-void c67x00_ll_reset(struct c67x00_drv *drv)
+int c67x00_ll_reset(struct c67x00_drv *drv)
 {
+	int rc;
+
 	ll_start(drv);
-	do {
-		hpi_send_mbox(drv, COMM_RESET);
-	} while (!ll_recv_msg(drv));
+	hpi_send_mbox(drv, COMM_RESET);
+	rc = ll_recv_msg(drv);
 	ll_release(drv);
+
+	return rc;
 }
 
 /* -------------------------------------------------------------------------- */
diff --git a/drivers/usb/c67x00/c67x00-ll-hpi.h b/drivers/usb/c67x00/c67x00-ll-hpi.h
index 118cd7d..3f84348 100644
--- a/drivers/usb/c67x00/c67x00-ll-hpi.h
+++ b/drivers/usb/c67x00/c67x00-ll-hpi.h
@@ -85,14 +85,14 @@ void c67x00_read_mem_le16(struct c67x00_drv *drv, u16 addr,
 
 u16 c67x00_comm_read_ctrl_reg(struct c67x00_drv *drv, u16 addr);
 
-void c67x00_comm_exec_int(struct c67x00_drv *drv, u16 nr,
-			  struct lcp_int_data *data);
+int c67x00_comm_exec_int(struct c67x00_drv *drv, u16 nr,
+			 struct lcp_int_data *data);
 
 /* Called by c67x00_irq to handle lcp interrupts */
 void c67x00_ll_irq(struct c67x00_drv *drv);
 
 void c67x00_ll_init(struct c67x00_drv *drv);
 void c67x00_ll_release(struct c67x00_drv *drv);
-void c67x00_ll_reset(struct c67x00_drv *drv);
+int c67x00_ll_reset(struct c67x00_drv *drv);
 
 #endif				/* _USB_C67X00_LL_HPI_H */
-- 
1.4.4.2

[PATCH 5/6] [C67x00] Change 'struct c67x00_drv' to 'struct c67x00_device'

[Grant Likely]

The structure describes per-device data, not per driver data.  Convention
seems to be to use the _device suffix for this kind of structure.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/usb/c67x00/c67x00-drv.c    |  102 +++++++-------
 drivers/usb/c67x00/c67x00-drv.h    |   10 +-
 drivers/usb/c67x00/c67x00-hcd.c    |    4 +-
 drivers/usb/c67x00/c67x00-hub.c    |    2 +-
 drivers/usb/c67x00/c67x00-ll-hpi.c |  266 ++++++++++++++++++------------------
 drivers/usb/c67x00/c67x00-ll-hpi.h |   28 ++--
 drivers/usb/c67x00/c67x00-sched.c  |   10 +-
 7 files changed, 211 insertions(+), 211 deletions(-)

diff --git a/drivers/usb/c67x00/c67x00-drv.c b/drivers/usb/c67x00/c67x00-drv.c
index 2737344..fe29734 100644
--- a/drivers/usb/c67x00/c67x00-drv.c
+++ b/drivers/usb/c67x00/c67x00-drv.c
@@ -51,7 +51,7 @@ static struct platform_driver c67x00_driver;
 /* -------------------------------------------------------------------------- */
 
 static void setup_sie(struct c67x00_sie *sie,
-		      struct c67x00_drv *drv, int sie_num)
+		      struct c67x00_device *dev, int sie_num)
 {
 	static unsigned int id = 0;
 
@@ -61,9 +61,9 @@ static void setup_sie(struct c67x00_sie *sie,
 	/* driver used in hub.c: hub_port_init */
 	sie->pdev->dev.driver = &c67x00_driver.driver;
 	spin_lock_init(&sie->lock);
-	sie->drv = drv;
+	sie->dev = dev;
 	sie->sie_num = sie_num;
-	sie->mode = c67x00_sie_config(drv->pdata->sie_config, sie_num);
+	sie->mode = c67x00_sie_config(dev->pdata->sie_config, sie_num);
 }
 
 static void teardown_sie(struct c67x00_sie *sie)
@@ -76,7 +76,7 @@ static void teardown_sie(struct c67x00_sie *sie)
 
 static void probe_sie(struct c67x00_sie *sie)
 {
-	switch (c67x00_sie_config(sie->drv->pdata->sie_config, sie->sie_num)) {
+	switch (c67x00_sie_config(sie->dev->pdata->sie_config, sie->sie_num)) {
 	case C67X00_SIE_HOST:
 		usb_hcd_c67x00_probe(sie);
 		break;
@@ -93,7 +93,7 @@ static void probe_sie(struct c67x00_sie *sie)
 	default:
 		dev_err(sie_dev(sie),
 			"Unsupported configuration: 0x%x for SIE %d\n",
-			c67x00_sie_config(sie->drv->pdata->sie_config,
+			c67x00_sie_config(sie->dev->pdata->sie_config,
 					  sie->sie_num), sie->sie_num);
 		break;
 	}
@@ -101,7 +101,7 @@ static void probe_sie(struct c67x00_sie *sie)
 
 static void remove_sie(struct c67x00_sie *sie)
 {
-	switch (c67x00_sie_config(sie->drv->pdata->sie_config, sie->sie_num)) {
+	switch (c67x00_sie_config(sie->dev->pdata->sie_config, sie->sie_num)) {
 	case C67X00_SIE_HOST:
 		usb_hcd_c67x00_remove(sie);
 		break;
@@ -117,36 +117,36 @@ static void remove_sie(struct c67x00_sie *sie)
 
 /* -------------------------------------------------------------------------- */
 
-static irqreturn_t c67x00_irq(int irq, void *__drv)
+static irqreturn_t c67x00_irq(int irq, void *__dev)
 {
-	struct c67x00_drv *drv = __drv;
+	struct c67x00_device *dev = __dev;
 	int i, count = 8;
 
-	drv->int_status = c67x00_hpi_status(drv);
-	if (!drv->int_status)
+	dev->int_status = c67x00_hpi_status(dev);
+	if (!dev->int_status)
 		return IRQ_NONE;
 
-	while (drv->int_status != 0 && (count-- >= 0)) {
-		c67x00_ll_irq(drv);
+	while (dev->int_status != 0 && (count-- >= 0)) {
+		c67x00_ll_irq(dev);
 		for (i = 0; i < C67X00_SIES; i++) {
-			spin_lock(&drv->sie[i].lock);
-			if (drv->int_status & SIEMSG_FLAG(i)) {
+			spin_lock(&dev->sie[i].lock);
+			if (dev->int_status & SIEMSG_FLAG(i)) {
 				u16 msg;
-				msg = c67x00_ll_get_siemsg(drv, i);
-				if (drv->sie[i].msg_received)
-					drv->sie[i].msg_received(&drv->sie[i],
+				msg = c67x00_ll_get_siemsg(dev, i);
+				if (dev->sie[i].msg_received)
+					dev->sie[i].msg_received(&dev->sie[i],
 								 msg);
 			}
-			if (drv->sie[i].irq)
-				drv->sie[i].irq(irq, &drv->sie[i]);
-			spin_unlock(&drv->sie[i].lock);
+			if (dev->sie[i].irq)
+				dev->sie[i].irq(irq, &dev->sie[i]);
+			spin_unlock(&dev->sie[i].lock);
 		}
-		drv->int_status = c67x00_hpi_status(drv);
+		dev->int_status = c67x00_hpi_status(dev);
 	}
 
-	if (drv->int_status)
-		dev_warn(&drv->pdev->dev, "Not all interrupts handled! "
-			 "status = 0x%04x\n", drv->int_status);
+	if (dev->int_status)
+		dev_warn(&dev->pdev->dev, "Not all interrupts handled! "
+			 "status = 0x%04x\n", dev->int_status);
 
 	return IRQ_HANDLED;
 }
@@ -155,7 +155,7 @@ static irqreturn_t c67x00_irq(int irq, void *__drv)
 
 static int __devinit usb_c67x00_drv_probe(struct platform_device *pdev)
 {
-	struct c67x00_drv *drv;
+	struct c67x00_device *dev;
 	struct c67x00_platform_data *pdata;
 	struct resource *res, *res2;
 	int ret, i;
@@ -172,8 +172,8 @@ static int __devinit usb_c67x00_drv_probe(struct platform_device *pdev)
 	if (!pdata)
 		return -ENODEV;
 
-	drv = kzalloc(sizeof(*drv), GFP_KERNEL);
-	if (!drv)
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev)
 		return -ENOMEM;
 
 	if (!request_mem_region(res->start, res->end - res->start + 1,
@@ -182,82 +182,82 @@ static int __devinit usb_c67x00_drv_probe(struct platform_device *pdev)
 		ret = -EBUSY;
 		goto request_mem_failed;
 	}
-	drv->hpi.base = ioremap(res->start, res->end - res->start + 1);
-	if (!drv->hpi.base) {
+	dev->hpi.base = ioremap(res->start, res->end - res->start + 1);
+	if (!dev->hpi.base) {
 		dev_err(&pdev->dev, "Unable to map HPI registers\n");
 		ret = -EIO;
 		goto map_failed;
 	}
 
-	spin_lock_init(&drv->hw_lock);
-	drv->hpi.regstep = pdata->hpi_regstep;
-	drv->pdata = pdev->dev.platform_data;
-	drv->pdev = pdev;
+	spin_lock_init(&dev->hw_lock);
+	dev->hpi.regstep = pdata->hpi_regstep;
+	dev->pdata = pdev->dev.platform_data;
+	dev->pdev = pdev;
 
 	for (i = 0; i < C67X00_SIES; i++)
-		setup_sie(&drv->sie[i], drv, i);
+		setup_sie(&dev->sie[i], dev, i);
 
-	c67x00_ll_init(drv);
-	c67x00_ll_hpi_reg_init(drv);
+	c67x00_ll_init(dev);
+	c67x00_ll_hpi_reg_init(dev);
 
 	dev_info(&pdev->dev, "USB OTG controller, p:0x%x, v:0x%p, irq:%i\n",
-	         res->start, drv->hpi.base, res2->start);
+	         res->start, dev->hpi.base, res2->start);
 
-	ret = request_irq(res2->start, c67x00_irq, 0, pdev->name, drv);
+	ret = request_irq(res2->start, c67x00_irq, 0, pdev->name, dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Cannot claim IRQ\n");
 		goto request_irq_failed;
 	}
 
-	ret = c67x00_ll_reset(drv);
+	ret = c67x00_ll_reset(dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Device reset failed\n");
 		goto reset_failed;
 	}
 
 	for (i = 0; i < C67X00_SIES; i++)
-		probe_sie(&drv->sie[i]);
+		probe_sie(&dev->sie[i]);
 
-	platform_set_drvdata(pdev, drv);
+	platform_set_drvdata(pdev, dev);
 
 	return 0;
 
 reset_failed:
-	free_irq(res2->start, drv);
+	free_irq(res2->start, dev);
 request_irq_failed:
-	iounmap(drv->hpi.base);
+	iounmap(dev->hpi.base);
 map_failed:
 	release_mem_region(res->start, res->end - res->start + 1);
 request_mem_failed:
-	kfree(drv);
+	kfree(dev);
 
 	return ret;
 }
 
 static int __devexit usb_c67x00_drv_remove(struct platform_device *pdev)
 {
-	struct c67x00_drv *drv = platform_get_drvdata(pdev);
+	struct c67x00_device *dev = platform_get_drvdata(pdev);
 	struct resource *res;
 	int i;
 
 	for (i = 0; i < C67X00_SIES; i++) {
-		remove_sie(&drv->sie[i]);
-		teardown_sie(&drv->sie[i]);
+		remove_sie(&dev->sie[i]);
+		teardown_sie(&dev->sie[i]);
 	}
 
-	c67x00_ll_release(drv);
+	c67x00_ll_release(dev);
 
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (res)
-		free_irq(res->start, drv);
+		free_irq(res->start, dev);
 
-	iounmap(drv->hpi.base);
+	iounmap(dev->hpi.base);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (res)
 		release_mem_region(res->start, res->end - res->start + 1);
 
-	kfree(drv);
+	kfree(dev);
 
 	return 0;
 }
diff --git a/drivers/usb/c67x00/c67x00-drv.h b/drivers/usb/c67x00/c67x00-drv.h
index 8e7b99c..d29f4ad 100644
--- a/drivers/usb/c67x00/c67x00-drv.h
+++ b/drivers/usb/c67x00/c67x00-drv.h
@@ -31,7 +31,7 @@
 #include <linux/workqueue.h>
 #include "c67x00-ll-hpi.h"
 
-struct c67x00_drv;
+struct c67x00_device;
 
 /**
  * struct c67x00_sie - Common data associated with an SIE
@@ -40,7 +40,7 @@ struct c67x00_drv;
  * @pdev: platform device associated with this SIE, created in c67x00-drv.c
  * @irq: subdriver depenent irq handler, set NULL when not used
  * @msg_received: called when an SIEmsg has been received
- * @drv: link to common driver structure
+ * @dev: link to common driver structure
  * @sie_num: SIE number on chip, starting from 0
  * @mode: SIE mode (host/peripheral/otg/not used)
  *
@@ -57,7 +57,7 @@ struct c67x00_sie {
 	void (*msg_received) (struct c67x00_sie * sie, u16 msg);
 
 	/* Read only: */
-	struct c67x00_drv *drv;
+	struct c67x00_device *dev;
 	int sie_num;
 	int mode;
 };
@@ -73,7 +73,7 @@ struct c67x00_hpi {
 #define C67X00_PORTS	2
 
 /**
- * struct c67x00_drv - Common data structure associated with a c67x00 instance
+ * struct c67x00_device - Common data structure associated with a c67x00 instance
  * @hpi: hpi addresses
  * @sie: array of sie's on this chip
  * @pdata: configuration provided by the platform
@@ -81,7 +81,7 @@ struct c67x00_hpi {
  * @int_status: interrupt status register, only valid in_interrupt()
  * @lcp: lcp dependent data
  */
-struct c67x00_drv {
+struct c67x00_device {
 	struct c67x00_hpi hpi;
 	struct c67x00_sie sie[C67X00_SIES];
 	struct platform_device *pdev;
diff --git a/drivers/usb/c67x00/c67x00-hcd.c b/drivers/usb/c67x00/c67x00-hcd.c
index 357e34f..d5458ea 100644
--- a/drivers/usb/c67x00/c67x00-hcd.c
+++ b/drivers/usb/c67x00/c67x00-hcd.c
@@ -51,9 +51,9 @@ static irqreturn_t c67x00_hcd_irq(struct usb_hcd *hcd)
 {
 	struct c67x00_hcd *c67x00 = hcd_to_c67x00_hcd(hcd);
 	struct c67x00_sie *sie = c67x00->sie;
-	struct c67x00_drv *drv = sie->drv;
+	struct c67x00_device *dev = sie->dev;
 	int host = sie->sie_num;
-	if (drv->int_status & SOFEOP_FLG(host)) {
+	if (dev->int_status & SOFEOP_FLG(host)) {
 		c67x00_ll_husb_clear_status(sie, SOF_EOP_IRQ_FLG);
 		c67x00_sched_sofeop(c67x00);
 		return IRQ_HANDLED;
diff --git a/drivers/usb/c67x00/c67x00-hub.c b/drivers/usb/c67x00/c67x00-hub.c
index 91ad898..2518106 100644
--- a/drivers/usb/c67x00/c67x00-hub.c
+++ b/drivers/usb/c67x00/c67x00-hub.c
@@ -50,7 +50,7 @@ static void reset_host_port(struct c67x00_sie *sie, int port)
 	c67x00_ll_husb_reset_port(sie, port);
 	spin_unlock_irqrestore(&c67x00->lock, flags);
 
-	c67x00_ll_set_husb_eot(sie->drv, DEFAULT_EOT);
+	c67x00_ll_set_husb_eot(sie->dev, DEFAULT_EOT);
 }
 
 /* -------------------------------------------------------------------------- */
diff --git a/drivers/usb/c67x00/c67x00-ll-hpi.c b/drivers/usb/c67x00/c67x00-ll-hpi.c
index 868736a..052f35d 100644
--- a/drivers/usb/c67x00/c67x00-ll-hpi.c
+++ b/drivers/usb/c67x00/c67x00-ll-hpi.c
@@ -65,250 +65,250 @@
 
 /* These functions could also be implemented with SPI of HSS.
  * This is currently not supported */
-static inline u16 hpi_read_reg(struct c67x00_drv *drv, int reg)
+static inline u16 hpi_read_reg(struct c67x00_device *dev, int reg)
 {
-	return __raw_readw(drv->hpi.base + reg * drv->hpi.regstep);
+	return __raw_readw(dev->hpi.base + reg * dev->hpi.regstep);
 }
 
-static inline void hpi_write_reg(struct c67x00_drv *drv, int reg, u16 value)
+static inline void hpi_write_reg(struct c67x00_device *dev, int reg, u16 value)
 {
-	__raw_writew(value, drv->hpi.base + reg * drv->hpi.regstep);
+	__raw_writew(value, dev->hpi.base + reg * dev->hpi.regstep);
 }
 
-static inline u16 hpi_read_word_nolock(struct c67x00_drv *drv, u16 reg)
+static inline u16 hpi_read_word_nolock(struct c67x00_device *dev, u16 reg)
 {
-	hpi_write_reg(drv, HPI_ADDR, reg);
-	return hpi_read_reg(drv, HPI_DATA);
+	hpi_write_reg(dev, HPI_ADDR, reg);
+	return hpi_read_reg(dev, HPI_DATA);
 }
 
-static inline u16 hpi_read_word(struct c67x00_drv *drv, u16 reg)
+static inline u16 hpi_read_word(struct c67x00_device *dev, u16 reg)
 {
 	u16 value;
 	unsigned long flags;
 
-	spin_lock_irqsave(&drv->hw_lock, flags);
-	value = hpi_read_word_nolock(drv, reg);
-	spin_unlock_irqrestore(&drv->hw_lock, flags);
+	spin_lock_irqsave(&dev->hw_lock, flags);
+	value = hpi_read_word_nolock(dev, reg);
+	spin_unlock_irqrestore(&dev->hw_lock, flags);
 
 	return value;
 }
 
-static inline void hpi_write_word_nolock(struct c67x00_drv *drv, u16 reg,
+static inline void hpi_write_word_nolock(struct c67x00_device *dev, u16 reg,
 					 u16 value)
 {
-	hpi_write_reg(drv, HPI_ADDR, reg);
-	hpi_write_reg(drv, HPI_DATA, value);
+	hpi_write_reg(dev, HPI_ADDR, reg);
+	hpi_write_reg(dev, HPI_DATA, value);
 }
 
-static inline void hpi_write_word(struct c67x00_drv *drv, u16 reg, u16 value)
+static inline void hpi_write_word(struct c67x00_device *dev, u16 reg, u16 value)
 {
 	unsigned long flags;
 
-	spin_lock_irqsave(&drv->hw_lock, flags);
-	hpi_write_word_nolock(drv, reg, value);
-	spin_unlock_irqrestore(&drv->hw_lock, flags);
+	spin_lock_irqsave(&dev->hw_lock, flags);
+	hpi_write_word_nolock(dev, reg, value);
+	spin_unlock_irqrestore(&dev->hw_lock, flags);
 }
 
 /*
  * Only data is little endian, addr has cpu endianess
  */
-static inline void hpi_write_words_le16(struct c67x00_drv *drv, u16 addr,
+static inline void hpi_write_words_le16(struct c67x00_device *dev, u16 addr,
 					u16 * data, u16 count)
 {
 	unsigned long flags;
 	int i;
 
-	spin_lock_irqsave(&drv->hw_lock, flags);
+	spin_lock_irqsave(&dev->hw_lock, flags);
 
-	hpi_write_reg(drv, HPI_ADDR, addr);
+	hpi_write_reg(dev, HPI_ADDR, addr);
 	for (i = 0; i < count; i++)
-		hpi_write_reg(drv, HPI_DATA, cpu_to_le16(*data++));
+		hpi_write_reg(dev, HPI_DATA, cpu_to_le16(*data++));
 
-	spin_unlock_irqrestore(&drv->hw_lock, flags);
+	spin_unlock_irqrestore(&dev->hw_lock, flags);
 }
 
 /*
  * Only data is little endian, addr has cpu endianess
  */
-static inline void hpi_read_words_le16(struct c67x00_drv *drv, u16 addr,
+static inline void hpi_read_words_le16(struct c67x00_device *dev, u16 addr,
 				       u16 * data, u16 count)
 {
 	unsigned long flags;
 	int i;
-	spin_lock_irqsave(&drv->hw_lock, flags);
-	hpi_write_reg(drv, HPI_ADDR, addr);
+	spin_lock_irqsave(&dev->hw_lock, flags);
+	hpi_write_reg(dev, HPI_ADDR, addr);
 	for (i = 0; i < count; i++)
-		*data++ = le16_to_cpu(hpi_read_reg(drv, HPI_DATA));
+		*data++ = le16_to_cpu(hpi_read_reg(dev, HPI_DATA));
 
-	spin_unlock_irqrestore(&drv->hw_lock, flags);
+	spin_unlock_irqrestore(&dev->hw_lock, flags);
 }
 
-static inline void hpi_set_bits(struct c67x00_drv *drv, u16 reg, u16 mask)
+static inline void hpi_set_bits(struct c67x00_device *dev, u16 reg, u16 mask)
 {
 	u16 value;
 	unsigned long flags;
 
-	spin_lock_irqsave(&drv->hw_lock, flags);
-	value = hpi_read_word_nolock(drv, reg);
-	hpi_write_word_nolock(drv, reg, value | mask);
-	spin_unlock_irqrestore(&drv->hw_lock, flags);
+	spin_lock_irqsave(&dev->hw_lock, flags);
+	value = hpi_read_word_nolock(dev, reg);
+	hpi_write_word_nolock(dev, reg, value | mask);
+	spin_unlock_irqrestore(&dev->hw_lock, flags);
 }
 
-static inline void hpi_clear_bits(struct c67x00_drv *drv, u16 reg, u16 mask)
+static inline void hpi_clear_bits(struct c67x00_device *dev, u16 reg, u16 mask)
 {
 	u16 value;
 	unsigned long flags;
 
-	spin_lock_irqsave(&drv->hw_lock, flags);
-	value = hpi_read_word_nolock(drv, reg);
-	hpi_write_word_nolock(drv, reg, value & ~mask);
-	spin_unlock_irqrestore(&drv->hw_lock, flags);
+	spin_lock_irqsave(&dev->hw_lock, flags);
+	value = hpi_read_word_nolock(dev, reg);
+	hpi_write_word_nolock(dev, reg, value & ~mask);
+	spin_unlock_irqrestore(&dev->hw_lock, flags);
 }
 
-static inline u16 hpi_recv_mbox(struct c67x00_drv *drv)
+static inline u16 hpi_recv_mbox(struct c67x00_device *dev)
 {
 	u16 value;
 	unsigned long flags;
 
-	spin_lock_irqsave(&drv->hw_lock, flags);
-	value = hpi_read_reg(drv, HPI_MAILBOX);
-	spin_unlock_irqrestore(&drv->hw_lock, flags);
+	spin_lock_irqsave(&dev->hw_lock, flags);
+	value = hpi_read_reg(dev, HPI_MAILBOX);
+	spin_unlock_irqrestore(&dev->hw_lock, flags);
 
 	return value;
 }
 
-static inline u16 hpi_send_mbox(struct c67x00_drv *drv, u16 value)
+static inline u16 hpi_send_mbox(struct c67x00_device *dev, u16 value)
 {
 	unsigned long flags;
 
-	spin_lock_irqsave(&drv->hw_lock, flags);
-	hpi_write_reg(drv, HPI_MAILBOX, value);
-	spin_unlock_irqrestore(&drv->hw_lock, flags);
+	spin_lock_irqsave(&dev->hw_lock, flags);
+	hpi_write_reg(dev, HPI_MAILBOX, value);
+	spin_unlock_irqrestore(&dev->hw_lock, flags);
 
 	return value;
 }
 
-u16 c67x00_hpi_status(struct c67x00_drv * drv)
+u16 c67x00_hpi_status(struct c67x00_device * dev)
 {
 	u16 value;
 	unsigned long flags;
 
-	spin_lock_irqsave(&drv->hw_lock, flags);
-	value = hpi_read_reg(drv, HPI_STATUS);
-	spin_unlock_irqrestore(&drv->hw_lock, flags);
+	spin_lock_irqsave(&dev->hw_lock, flags);
+	value = hpi_read_reg(dev, HPI_STATUS);
+	spin_unlock_irqrestore(&dev->hw_lock, flags);
 
 	return value;
 }
 
-void c67x00_ll_hpi_reg_init(struct c67x00_drv *drv)
+void c67x00_ll_hpi_reg_init(struct c67x00_device *dev)
 {
 	int i;
 
-	hpi_recv_mbox(drv);
-	c67x00_hpi_status(drv);
-	hpi_write_word(drv, HPI_IRQ_ROUTING_REG, 0);
+	hpi_recv_mbox(dev);
+	c67x00_hpi_status(dev);
+	hpi_write_word(dev, HPI_IRQ_ROUTING_REG, 0);
 
 	for (i=0; i<C67X00_SIES; i++) {
-		hpi_write_word(drv, SIEMSG_REG(i), 0);
-		hpi_read_word(drv, SIEMSG_REG(i));
+		hpi_write_word(dev, SIEMSG_REG(i), 0);
+		hpi_read_word(dev, SIEMSG_REG(i));
 	}
 }
 
 void c67x00_ll_hpi_enable_sofeop(struct c67x00_sie *sie)
 {
-	hpi_set_bits(sie->drv, HPI_IRQ_ROUTING_REG,
+	hpi_set_bits(sie->dev, HPI_IRQ_ROUTING_REG,
 		     SOFEOP_TO_HPI_EN(sie->sie_num));
 }
 
 void c67x00_ll_hpi_disable_sofeop(struct c67x00_sie *sie)
 {
-	hpi_clear_bits(sie->drv, HPI_IRQ_ROUTING_REG,
+	hpi_clear_bits(sie->dev, HPI_IRQ_ROUTING_REG,
 		       SOFEOP_TO_HPI_EN(sie->sie_num));
 }
 
 /* -------------------------------------------------------------------------- */
 /* Transactions */
 
-static inline void ll_start(struct c67x00_drv *drv)
+static inline void ll_start(struct c67x00_device *dev)
 {
-	INIT_COMPLETION(drv->lcp.msg_received);
-	mutex_lock(&drv->lcp.mutex);
+	INIT_COMPLETION(dev->lcp.msg_received);
+	mutex_lock(&dev->lcp.mutex);
 }
 
-static inline u16 ll_recv_msg(struct c67x00_drv *drv)
+static inline u16 ll_recv_msg(struct c67x00_device *dev)
 {
 	u16 res;
 
-	res = wait_for_completion_timeout(&drv->lcp.msg_received, 5 * HZ);
-	INIT_COMPLETION(drv->lcp.msg_received);
+	res = wait_for_completion_timeout(&dev->lcp.msg_received, 5 * HZ);
+	INIT_COMPLETION(dev->lcp.msg_received);
 	WARN_ON(!res);
 
 	return (res == 0) ? -EIO : 0;
 }
 
-static inline void ll_release(struct c67x00_drv *drv)
+static inline void ll_release(struct c67x00_device *dev)
 {
-	mutex_unlock(&drv->lcp.mutex);
+	mutex_unlock(&dev->lcp.mutex);
 }
 
 /* -------------------------------------------------------------------------- */
 /* General functions */
 
-u16 c67x00_ll_get_siemsg(struct c67x00_drv *drv, int sie)
+u16 c67x00_ll_get_siemsg(struct c67x00_device *dev, int sie)
 {
-	return hpi_read_word(drv, SIEMSG_REG(sie));
+	return hpi_read_word(dev, SIEMSG_REG(sie));
 }
 
-void c67x00_ll_set_siemsg(struct c67x00_drv *drv, int sie, u16 val)
+void c67x00_ll_set_siemsg(struct c67x00_device *dev, int sie, u16 val)
 {
-	hpi_write_word(drv, SIEMSG_REG(sie), val);
+	hpi_write_word(dev, SIEMSG_REG(sie), val);
 }
 
 u16 c67x00_ll_get_usb_ctl(struct c67x00_sie *sie)
 {
-	return hpi_read_word(sie->drv, USB_CTL_REG(sie->sie_num));
+	return hpi_read_word(sie->dev, USB_CTL_REG(sie->sie_num));
 }
 
 /* -------------------------------------------------------------------------- */
 /* Host specific functions */
 
-void c67x00_ll_set_husb_eot(struct c67x00_drv *drv, u16 value)
+void c67x00_ll_set_husb_eot(struct c67x00_device *dev, u16 value)
 {
-	ll_start(drv);
-	hpi_write_word(drv, HUSB_pEOT, value);
-	ll_release(drv);
+	ll_start(dev);
+	hpi_write_word(dev, HUSB_pEOT, value);
+	ll_release(dev);
 }
 
 static inline void c67x00_ll_husb_sie_init(struct c67x00_sie *sie)
 {
-	struct c67x00_drv *drv = sie->drv;
+	struct c67x00_device *dev = sie->dev;
 	struct lcp_int_data data;
 	int rc;
 
-	rc = c67x00_comm_exec_int(drv, HUSB_SIE_INIT_INT(sie->sie_num), &data);
+	rc = c67x00_comm_exec_int(dev, HUSB_SIE_INIT_INT(sie->sie_num), &data);
 	BUG_ON(rc); /* No return path for error code; crash spectacularly */
 }
 
 void c67x00_ll_husb_reset(struct c67x00_sie *sie, int port)
 {
-	struct c67x00_drv *drv = sie->drv;
+	struct c67x00_device *dev = sie->dev;
 	struct lcp_int_data data;
 	int rc;
 
 	data.regs[0] = 50;	/* Reset USB port for 50ms */
 	data.regs[1] = port | (sie->sie_num << 1);
-	rc = c67x00_comm_exec_int(drv, HUSB_RESET_INT, &data);
+	rc = c67x00_comm_exec_int(dev, HUSB_RESET_INT, &data);
 	BUG_ON(rc); /* No return path for error code; crash spectacularly */
 }
 
 void c67x00_ll_husb_set_current_td(struct c67x00_sie *sie, u16 addr)
 {
-	hpi_write_word(sie->drv, HUSB_SIE_pCurrentTDPtr(sie->sie_num), addr);
+	hpi_write_word(sie->dev, HUSB_SIE_pCurrentTDPtr(sie->sie_num), addr);
 }
 
 u16 c67x00_ll_husb_get_current_td(struct c67x00_sie *sie)
 {
-	return hpi_read_word(sie->drv, HUSB_SIE_pCurrentTDPtr(sie->sie_num));
+	return hpi_read_word(sie->dev, HUSB_SIE_pCurrentTDPtr(sie->sie_num));
 }
 
 /**
@@ -316,28 +316,28 @@ u16 c67x00_ll_husb_get_current_td(struct c67x00_sie *sie)
  */
 void c67x00_ll_husb_clear_status(struct c67x00_sie *sie, u16 bits)
 {
-	hpi_write_word(sie->drv, HOST_STAT_REG(sie->sie_num), bits);
+	hpi_write_word(sie->dev, HOST_STAT_REG(sie->sie_num), bits);
 }
 
 u16 c67x00_ll_husb_get_status(struct c67x00_sie *sie)
 {
-	return hpi_read_word(sie->drv, HOST_STAT_REG(sie->sie_num));
+	return hpi_read_word(sie->dev, HOST_STAT_REG(sie->sie_num));
 }
 
 u16 c67x00_ll_husb_get_frame(struct c67x00_sie * sie)
 {
-	return hpi_read_word(sie->drv, HOST_FRAME_REG(sie->sie_num));
+	return hpi_read_word(sie->dev, HOST_FRAME_REG(sie->sie_num));
 }
 
 void c67x00_ll_husb_init_host_port(struct c67x00_sie *sie)
 {
 	/* Set port into host mode */
-	hpi_set_bits(sie->drv, USB_CTL_REG(sie->sie_num), HOST_MODE);
+	hpi_set_bits(sie->dev, USB_CTL_REG(sie->sie_num), HOST_MODE);
 	c67x00_ll_husb_sie_init(sie);
 	/* Clear interrupts */
 	c67x00_ll_husb_clear_status(sie, HOST_STAT_MASK);
 	/* Check */
-	if (!(hpi_read_word(sie->drv, USB_CTL_REG(sie->sie_num)) & HOST_MODE))
+	if (!(hpi_read_word(sie->dev, USB_CTL_REG(sie->sie_num)) & HOST_MODE))
 		dev_warn(sie_dev(sie),
 			 "SIE %d not set to host mode\n", sie->sie_num);
 }
@@ -348,94 +348,94 @@ void c67x00_ll_husb_reset_port(struct c67x00_sie *sie, int port)
 	c67x00_ll_husb_clear_status(sie, PORT_CONNECT_CHANGE(port));
 
 	/* Enable interrupts */
-	hpi_set_bits(sie->drv, HPI_IRQ_ROUTING_REG,
+	hpi_set_bits(sie->dev, HPI_IRQ_ROUTING_REG,
 		     SOFEOP_TO_CPU_EN(sie->sie_num));
-	hpi_set_bits(sie->drv, HOST_IRQ_EN_REG(sie->sie_num),
+	hpi_set_bits(sie->dev, HOST_IRQ_EN_REG(sie->sie_num),
 		     SOF_EOP_IRQ_EN | DONE_IRQ_EN);
 
 	/* Enable pull down transistors */
-	hpi_set_bits(sie->drv, USB_CTL_REG(sie->sie_num), PORT_RES_EN(port));
+	hpi_set_bits(sie->dev, USB_CTL_REG(sie->sie_num), PORT_RES_EN(port));
 }
 
 /* -------------------------------------------------------------------------- */
 void c67x00_ll_susb_init(struct c67x00_sie *sie)
 {
-	struct c67x00_drv *drv = sie->drv;
+	struct c67x00_device *dev = sie->dev;
 	struct lcp_int_data data;
 	int rc;
 
 	data.regs[1] = 1;	/* full speed */
 	data.regs[2] = sie->sie_num + 1;
-	rc = c67x00_comm_exec_int(drv, SUSB_INIT_INT, &data);
+	rc = c67x00_comm_exec_int(dev, SUSB_INIT_INT, &data);
 	BUG_ON(rc); /* No return path for error code; crash spectacularly */
 
-	hpi_clear_bits(drv, HPI_IRQ_ROUTING_REG,
+	hpi_clear_bits(dev, HPI_IRQ_ROUTING_REG,
 		       SOFEOP_TO_HPI_EN(sie->sie_num));
-	hpi_set_bits(drv, HPI_IRQ_ROUTING_REG, SOFEOP_TO_CPU_EN(sie->sie_num));
+	hpi_set_bits(dev, HPI_IRQ_ROUTING_REG, SOFEOP_TO_CPU_EN(sie->sie_num));
 }
 
 /* -------------------------------------------------------------------------- */
 
-void c67x00_ll_irq(struct c67x00_drv *drv)
+void c67x00_ll_irq(struct c67x00_device *dev)
 {
-	if ((drv->int_status & MBX_OUT_FLG) == 0)
+	if ((dev->int_status & MBX_OUT_FLG) == 0)
 		return;
 
-	drv->lcp.last_msg = hpi_recv_mbox(drv);
-	complete(&drv->lcp.msg_received);
+	dev->lcp.last_msg = hpi_recv_mbox(dev);
+	complete(&dev->lcp.msg_received);
 }
 
 /* -------------------------------------------------------------------------- */
 
-u16 c67x00_comm_read_ctrl_reg(struct c67x00_drv * drv, u16 addr)
+u16 c67x00_comm_read_ctrl_reg(struct c67x00_device * dev, u16 addr)
 {
 	unsigned long msg, res;
 	int rc;
 
-	ll_start(drv);
-	hpi_write_word(drv, COMM_CTRL_REG_ADDR, addr);
-	hpi_send_mbox(drv, COMM_READ_CTRL_REG);
-	rc = ll_recv_msg(drv);
+	ll_start(dev);
+	hpi_write_word(dev, COMM_CTRL_REG_ADDR, addr);
+	hpi_send_mbox(dev, COMM_READ_CTRL_REG);
+	rc = ll_recv_msg(dev);
 
 	BUG_ON(rc); /* No return path for error code; crash spectacularly */
 
-	msg = drv->lcp.last_msg;
+	msg = dev->lcp.last_msg;
 	if (msg != COMM_ACK) {
-		dev_warn(&drv->pdev->dev, "COMM_READ_CTRL_REG didn't ACK!\n");
+		dev_warn(&dev->pdev->dev, "COMM_READ_CTRL_REG didn't ACK!\n");
 		res = 0;
 	} else {
-		res = hpi_read_word(drv, COMM_CTRL_REG_DATA);
+		res = hpi_read_word(dev, COMM_CTRL_REG_DATA);
 	}
-	ll_release(drv);
+	ll_release(dev);
 	return res;
 }
 
-int c67x00_comm_exec_int(struct c67x00_drv *drv, u16 nr,
+int c67x00_comm_exec_int(struct c67x00_device *dev, u16 nr,
 			  struct lcp_int_data *data)
 {
 	int i, rc;
 
-	ll_start(drv);
-	hpi_write_word(drv, COMM_INT_NUM, nr);
+	ll_start(dev);
+	hpi_write_word(dev, COMM_INT_NUM, nr);
 	for (i = 0; i < COMM_REGS; i++)
-		hpi_write_word(drv, COMM_R(i), data->regs[i]);
-	hpi_send_mbox(drv, COMM_EXEC_INT);
-	rc = ll_recv_msg(drv);
-	ll_release(drv);
+		hpi_write_word(dev, COMM_R(i), data->regs[i]);
+	hpi_send_mbox(dev, COMM_EXEC_INT);
+	rc = ll_recv_msg(dev);
+	ll_release(dev);
 
 	return rc;
 }
 
 /* -------------------------------------------------------------------------- */
 
-int c67x00_ll_reset(struct c67x00_drv *drv)
+int c67x00_ll_reset(struct c67x00_device *dev)
 {
 	int rc;
 
-	ll_start(drv);
-	hpi_send_mbox(drv, COMM_RESET);
-	rc = ll_recv_msg(drv);
-	ll_release(drv);
+	ll_start(dev);
+	hpi_send_mbox(dev, COMM_RESET);
+	rc = ll_recv_msg(dev);
+	ll_release(dev);
 
 	return rc;
 }
@@ -446,12 +446,12 @@ int c67x00_ll_reset(struct c67x00_drv *drv)
  * c67x00_write_mem_le16 - write into c67x00 memory
  * Only data is little endian, addr has cpu endianess.
  */
-void c67x00_write_mem_le16(struct c67x00_drv *drv, u16 addr,
+void c67x00_write_mem_le16(struct c67x00_device *dev, u16 addr,
 			   int len, char *data)
 {
 	/* Sanity check */
 	if (addr + len > 0xffff) {
-		dev_err(&drv->pdev->dev,
+		dev_err(&dev->pdev->dev,
 			"Trying to write beyond writable region!\n");
 		return;
 	}
@@ -459,23 +459,23 @@ void c67x00_write_mem_le16(struct c67x00_drv *drv, u16 addr,
 	if (addr & 0x01) {
 		/* unaligned access */
 		u16 tmp;
-		tmp = hpi_read_word(drv, addr - 1);
+		tmp = hpi_read_word(dev, addr - 1);
 		tmp = (tmp & 0x00ff) | (*data++ << 8);
-		hpi_write_word(drv, addr - 1, tmp);
+		hpi_write_word(dev, addr - 1, tmp);
 		addr++;
 		len--;
 	}
 
-	hpi_write_words_le16(drv, addr, (u16 *) data, len / 2);
+	hpi_write_words_le16(dev, addr, (u16 *) data, len / 2);
 	data += len & ~0x01;
 	addr += len & ~0x01;
 	len &= 0x01;
 
 	if (len) {
 		u16 tmp;
-		tmp = hpi_read_word(drv, addr);
+		tmp = hpi_read_word(dev, addr);
 		tmp = (tmp & 0xff00) | (*data++);
-		hpi_write_word(drv, addr, tmp);
+		hpi_write_word(dev, addr, tmp);
 		addr++;
 		len--;
 	}
@@ -486,25 +486,25 @@ void c67x00_write_mem_le16(struct c67x00_drv *drv, u16 addr,
  * c67x00_read_mem_le16 - read from c67x00 memory
  * Only data is little endian, addr has cpu endianess.
  */
-void c67x00_read_mem_le16(struct c67x00_drv *drv, u16 addr, int len, char *data)
+void c67x00_read_mem_le16(struct c67x00_device *dev, u16 addr, int len, char *data)
 {
 	if (addr & 0x01) {
 		/* unaligned access */
 		u16 tmp;
-		tmp = hpi_read_word(drv, addr - 1);
+		tmp = hpi_read_word(dev, addr - 1);
 		*data++ = (tmp >> 8) & 0x00ff;
 		addr++;
 		len--;
 	}
 
-	hpi_read_words_le16(drv, addr, (u16 *) data, len / 2);
+	hpi_read_words_le16(dev, addr, (u16 *) data, len / 2);
 	data += len & ~0x01;
 	addr += len & ~0x01;
 	len &= 0x01;
 
 	if (len) {
 		u16 tmp;
-		tmp = hpi_read_word(drv, addr);
+		tmp = hpi_read_word(dev, addr);
 		*data++ = tmp & 0x00ff;
 		addr++;
 		len--;
@@ -513,12 +513,12 @@ void c67x00_read_mem_le16(struct c67x00_drv *drv, u16 addr, int len, char *data)
 
 /* -------------------------------------------------------------------------- */
 
-void c67x00_ll_init(struct c67x00_drv *drv)
+void c67x00_ll_init(struct c67x00_device *dev)
 {
-	mutex_init(&drv->lcp.mutex);
-	init_completion(&drv->lcp.msg_received);
+	mutex_init(&dev->lcp.mutex);
+	init_completion(&dev->lcp.msg_received);
 }
 
-void c67x00_ll_release(struct c67x00_drv *drv)
+void c67x00_ll_release(struct c67x00_device *dev)
 {
 }
diff --git a/drivers/usb/c67x00/c67x00-ll-hpi.h b/drivers/usb/c67x00/c67x00-ll-hpi.h
index 3f84348..b3b8512 100644
--- a/drivers/usb/c67x00/c67x00-ll-hpi.h
+++ b/drivers/usb/c67x00/c67x00-ll-hpi.h
@@ -28,7 +28,7 @@
 #include <linux/mutex.h>
 
 struct c67x00_sie;
-struct c67x00_drv;
+struct c67x00_device;
 
 struct lcp {
 	/* Internal use only */
@@ -46,22 +46,22 @@ struct lcp_int_data {
 /* ------------------------------------------------------------------------- */
 /* HPI */
 
-u16 c67x00_hpi_status(struct c67x00_drv *drv);
-void c67x00_ll_hpi_reg_init(struct c67x00_drv *drv);
+u16 c67x00_hpi_status(struct c67x00_device *dev);
+void c67x00_ll_hpi_reg_init(struct c67x00_device *dev);
 void c67x00_ll_hpi_enable_sofeop(struct c67x00_sie *sie);
 void c67x00_ll_hpi_disable_sofeop(struct c67x00_sie *sie);
 
 /* -------------------------------------------------------------------------- */
 /* General functions */
 
-u16 c67x00_ll_get_siemsg(struct c67x00_drv *drv, int sie);
-void c67x00_ll_set_siemsg(struct c67x00_drv *drv, int sie, u16 val);
+u16 c67x00_ll_get_siemsg(struct c67x00_device *dev, int sie);
+void c67x00_ll_set_siemsg(struct c67x00_device *dev, int sie, u16 val);
 u16 c67x00_ll_get_usb_ctl(struct c67x00_sie *sie);
 
 /* ------------------------------------------------------------------------- */
 /* Host specific functions */
 
-void c67x00_ll_set_husb_eot(struct c67x00_drv *drv, u16 value);
+void c67x00_ll_set_husb_eot(struct c67x00_device *dev, u16 value);
 void c67x00_ll_husb_reset(struct c67x00_sie *sie, int port);
 void c67x00_ll_husb_set_current_td(struct c67x00_sie *sie, u16 addr);
 u16 c67x00_ll_husb_get_current_td(struct c67x00_sie *sie);
@@ -78,21 +78,21 @@ void c67x00_ll_susb_init(struct c67x00_sie *sie);
 
 /* ------------------------------------------------------------------------- */
 
-void c67x00_write_mem_le16(struct c67x00_drv *drv, u16 addr,
+void c67x00_write_mem_le16(struct c67x00_device *dev, u16 addr,
 			   int len, char *data);
-void c67x00_read_mem_le16(struct c67x00_drv *drv, u16 addr,
+void c67x00_read_mem_le16(struct c67x00_device *dev, u16 addr,
 			  int len, char *data);
 
-u16 c67x00_comm_read_ctrl_reg(struct c67x00_drv *drv, u16 addr);
+u16 c67x00_comm_read_ctrl_reg(struct c67x00_device *dev, u16 addr);
 
-int c67x00_comm_exec_int(struct c67x00_drv *drv, u16 nr,
+int c67x00_comm_exec_int(struct c67x00_device *dev, u16 nr,
 			 struct lcp_int_data *data);
 
 /* Called by c67x00_irq to handle lcp interrupts */
-void c67x00_ll_irq(struct c67x00_drv *drv);
+void c67x00_ll_irq(struct c67x00_device *dev);
 
-void c67x00_ll_init(struct c67x00_drv *drv);
-void c67x00_ll_release(struct c67x00_drv *drv);
-int c67x00_ll_reset(struct c67x00_drv *drv);
+void c67x00_ll_init(struct c67x00_device *dev);
+void c67x00_ll_release(struct c67x00_device *dev);
+int c67x00_ll_reset(struct c67x00_device *dev);
 
 #endif				/* _USB_C67X00_LL_HPI_H */
diff --git a/drivers/usb/c67x00/c67x00-sched.c b/drivers/usb/c67x00/c67x00-sched.c
index 3a870cf..d408f2f 100644
--- a/drivers/usb/c67x00/c67x00-sched.c
+++ b/drivers/usb/c67x00/c67x00-sched.c
@@ -858,18 +858,18 @@ static void send_td(struct c67x00_hcd *c67x00, struct c67x00_td *td)
 	int len = td_length(td);
 
 	if (len && ((td->pid_ep & TD_PIDEPMASK_PID) != TD_PID_IN))
-		c67x00_write_mem_le16(c67x00->sie->drv,
+		c67x00_write_mem_le16(c67x00->sie->dev,
 				      td_ly_base_addr(td), len, td->data);
 #ifdef DEBUG_PATTERN
 	else {			/* write known patterns into memories */
 		memset(td->data, NON_RECEIVED_PATTERN, len);
-		c67x00_write_mem_le16(c67x00->sie->drv,
+		c67x00_write_mem_le16(c67x00->sie->dev,
 				      td_ly_base_addr(td), len, td->data);
 		memset(td->data, UNREAD_PATTERN, len);
 	}
 #endif				/* DEBUG_PATTERN */
 
-	c67x00_write_mem_le16(c67x00->sie->drv,
+	c67x00_write_mem_le16(c67x00->sie->dev,
 			      td->td_addr, CY_TD_SIZE, (char *)td);
 }
 
@@ -878,11 +878,11 @@ static void send_td(struct c67x00_hcd *c67x00, struct c67x00_td *td)
  */
 static inline void parse_td(struct c67x00_hcd *c67x00, struct c67x00_td *td)
 {
-	c67x00_read_mem_le16(c67x00->sie->drv,
+	c67x00_read_mem_le16(c67x00->sie->dev,
 			     td->td_addr, CY_TD_SIZE, (char *)td);
 
 	if (usb_pipein(td->pipe) && td_actual_bytes(td))
-		c67x00_read_mem_le16(c67x00->sie->drv, td_ly_base_addr(td),
+		c67x00_read_mem_le16(c67x00->sie->dev, td_ly_base_addr(td),
 				     td_actual_bytes(td), td->data);
 }
 
-- 
1.4.4.2

[PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c

[Grant Likely]

Rather than c67x00-hub.c being compiled seperately, the original code had
c67x00-hub.c *included* by c67x00-hcd.c.  This is a very bad idea.
Simplest solution is to merge the two files into one and be done with it.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/usb/c67x00/c67x00-hcd.c |  183 ++++++++++++++++++++++++++++++++++-
 drivers/usb/c67x00/c67x00-hub.c |  206 ---------------------------------------
 2 files changed, 182 insertions(+), 207 deletions(-)

diff --git a/drivers/usb/c67x00/c67x00-hcd.c b/drivers/usb/c67x00/c67x00-hcd.c
index d5458ea..4e6810d 100644
--- a/drivers/usb/c67x00/c67x00-hcd.c
+++ b/drivers/usb/c67x00/c67x00-hcd.c
@@ -89,8 +89,189 @@ static int c67x00_get_frame(struct usb_hcd *hcd)
 }
 
 /* -------------------------------------------------------------------------- */
+/* Root Hub Support */
 
-#include "c67x00-hub.c"
+static __u8 root_hub_hub_des[] = {
+	0x09,			/*  __u8  bLength; */
+	0x29,			/*  __u8  bDescriptorType; Hub-descriptor */
+	0x02,			/*  __u8  bNbrPorts; */
+	0x00,			/* __u16  wHubCharacteristics; */
+	0x00,			/*   (per-port OC, no power switching) */
+	0x32,			/*  __u8  bPwrOn2pwrGood; 2ms */
+	0x00,			/*  __u8  bHubContrCurrent; 0 mA */
+	0x00,			/*  __u8  DeviceRemovable; ** 7 Ports max ** */
+	0xff,			/*  __u8  PortPwrCtrlMask; ** 7 ports max ** */
+};
+
+#define OK(x)			len = (x); break
+
+/* -------------------------------------------------------------------------- */
+
+static void c67x00_hub_reset_host_port(struct c67x00_sie *sie, int port)
+{
+	struct c67x00_hcd *c67x00 = sie->private_data;
+	unsigned long flags;
+
+	c67x00_ll_husb_reset(sie, port);
+
+	spin_lock_irqsave(&c67x00->lock, flags);
+	c67x00_ll_husb_reset_port(sie, port);
+	spin_unlock_irqrestore(&c67x00->lock, flags);
+
+	c67x00_ll_set_husb_eot(sie->dev, DEFAULT_EOT);
+}
+
+/* -------------------------------------------------------------------------- */
+
+static int c67x00_hub_status_data(struct usb_hcd *hcd, char *buf)
+{
+	struct c67x00_hcd *c67x00 = hcd_to_c67x00_hcd(hcd);
+	struct c67x00_sie *sie = c67x00->sie;
+	u16 status;
+	int i;
+
+	*buf = 0;
+	status = c67x00_ll_husb_get_status(sie);
+	for (i=0; i<C67X00_PORTS; i++)
+		if (status & PORT_CONNECT_CHANGE(i))
+			*buf |= (1 << i);
+
+	/* bit 0 denotes hub change, b1..n port change */
+	*buf <<= 1;
+
+	return !!*buf;
+}
+
+static int c67x00_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
+			      u16 wIndex, char *buf, u16 wLength)
+{
+	struct c67x00_hcd *c67x00 = hcd_to_c67x00_hcd(hcd);
+	struct c67x00_sie *sie = c67x00->sie;
+	u16 status, usb_status;
+	int retval = 0, len = 0;
+	unsigned int port = wIndex-1;
+	u16 wPortChange, wPortStatus;
+
+	switch (typeReq) {
+
+	case GetHubStatus:
+		*(__le32 *) buf = cpu_to_le32(0);
+		OK(4);		/* hub power */
+	case GetPortStatus:
+		if (wIndex > C67X00_PORTS)
+			goto err;
+
+		status = c67x00_ll_husb_get_status(sie);
+		usb_status = c67x00_ll_get_usb_ctl(sie);
+
+		wPortChange = 0;
+		if (status & PORT_CONNECT_CHANGE(port))
+			wPortChange |= USB_PORT_STAT_C_CONNECTION;
+
+		wPortStatus = USB_PORT_STAT_POWER;
+		if (!(status & PORT_SE0_STATUS(port)))
+			wPortStatus |= USB_PORT_STAT_CONNECTION;
+		if (usb_status & LOW_SPEED_PORT(port)) {
+			wPortStatus |= USB_PORT_STAT_LOW_SPEED;
+			c67x00->low_speed_ports |= (1 << port);
+		} else
+			c67x00->low_speed_ports &= ~(1 << port);
+
+		if (usb_status & SOF_EOP_EN(port))
+			wPortStatus |= USB_PORT_STAT_ENABLE;
+
+		*(__le16 *) buf = cpu_to_le16(wPortStatus);
+		*(__le16 *) (buf + 2) = cpu_to_le16(wPortChange);
+		OK(4);
+	case SetHubFeature:	/* We don't implement these */
+	case ClearHubFeature:
+		switch (wValue) {
+		case C_HUB_OVER_CURRENT:
+		case C_HUB_LOCAL_POWER:
+			OK(0);
+		default:
+			goto err;
+		}
+		break;
+	case SetPortFeature:
+		if (wIndex > C67X00_PORTS)
+			goto err;
+
+		switch (wValue) {
+		case USB_PORT_FEAT_SUSPEND:
+			dev_dbg(c67x00_dev(c67x00),
+				"SetPortFeature %d (SUSPEND)\n", port);
+			OK(0);
+		case USB_PORT_FEAT_RESET:
+			c67x00_hub_reset_host_port(sie, port);
+			OK(0);
+		case USB_PORT_FEAT_POWER:
+			/* Power always enabled */
+			OK(0);
+		default:
+			dev_dbg(c67x00_dev(c67x00),
+				"%s: SetPortFeature %d (0x%04x) Error!\n",
+				__FUNCTION__, port, wValue);
+			goto err;
+		}
+		break;
+	case ClearPortFeature:
+		if (wIndex > C67X00_PORTS)
+			goto err;
+
+		switch (wValue) {
+		case USB_PORT_FEAT_ENABLE:
+			/* Reset the port so that the c67x00 also notices the
+			 * disconnect */
+			c67x00_hub_reset_host_port(sie, port);
+			OK(0);
+		case USB_PORT_FEAT_C_ENABLE:
+			dev_dbg(c67x00_dev(c67x00),
+				"ClearPortFeature (%d): C_ENABLE\n", port);
+			OK(0);
+		case USB_PORT_FEAT_SUSPEND:
+			dev_dbg(c67x00_dev(c67x00),
+				"ClearPortFeature (%d): SUSPEND\n", port);
+			OK(0);
+		case USB_PORT_FEAT_C_SUSPEND:
+			dev_dbg(c67x00_dev(c67x00),
+				"ClearPortFeature (%d): C_SUSPEND\n", port);
+			OK(0);
+		case USB_PORT_FEAT_POWER:
+			dev_dbg(c67x00_dev(c67x00),
+				"ClearPortFeature (%d): POWER\n", port);
+			goto err;
+		case USB_PORT_FEAT_C_CONNECTION:
+			c67x00_ll_husb_clear_status(sie,
+						    PORT_CONNECT_CHANGE(port));
+			OK(0);
+		case USB_PORT_FEAT_C_OVER_CURRENT:
+			dev_dbg(c67x00_dev(c67x00),
+				"ClearPortFeature (%d): OVER_CURRENT\n", port);
+			OK(0);
+		case USB_PORT_FEAT_C_RESET:
+			dev_dbg(c67x00_dev(c67x00),
+				"ClearPortFeature (%d): C_RESET\n", port);
+			OK(0);
+		default:
+			dev_dbg(c67x00_dev(c67x00),
+				"%s: ClearPortFeature %d (0x%04x) Error!\n",
+				__FUNCTION__, port, wValue);
+			goto err;
+		}
+		break;
+	case GetHubDescriptor:
+		len = min_t(unsigned int, sizeof(root_hub_hub_des), wLength);
+		memcpy(buf, root_hub_hub_des, len);
+		OK(len);
+	default:
+		dev_dbg(c67x00_dev(c67x00), "%s: unknown\n", __FUNCTION__);
+	      err:
+		retval = -EPIPE;
+	}
+
+	return retval;
+}
 
 /* -------------------------------------------------------------------------- */
 
diff --git a/drivers/usb/c67x00/c67x00-hub.c b/drivers/usb/c67x00/c67x00-hub.c
deleted file mode 100644
index 2518106..0000000
--- a/drivers/usb/c67x00/c67x00-hub.c
+++ /dev/null
@@ -1,206 +0,0 @@
-/*
- * c67x00-hub.c: Cypress C67X00 USB Host Controller Driver - HUB functionality
- *
- * Copyright (C) 2006-2007 Barco N.V.
- *    Derived from the Cypress cy7c67200/300 ezusb linux driver and
- *    based on multiple host controller drivers inside the linux kernel.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
- * MA  02110-1301  USA.
- */
-
-/* Included in c67x00-hcd.c */
-
-static __u8 root_hub_hub_des[] = {
-	0x09,			/*  __u8  bLength; */
-	0x29,			/*  __u8  bDescriptorType; Hub-descriptor */
-	0x02,			/*  __u8  bNbrPorts; */
-	0x00,			/* __u16  wHubCharacteristics; */
-	0x00,			/*   (per-port OC, no power switching) */
-	0x32,			/*  __u8  bPwrOn2pwrGood; 2ms */
-	0x00,			/*  __u8  bHubContrCurrent; 0 mA */
-	0x00,			/*  __u8  DeviceRemovable; ** 7 Ports max ** */
-	0xff,			/*  __u8  PortPwrCtrlMask; ** 7 ports max ** */
-};
-
-#define OK(x)			len = (x); break
-
-/* -------------------------------------------------------------------------- */
-
-static void reset_host_port(struct c67x00_sie *sie, int port)
-{
-	struct c67x00_hcd *c67x00 = sie->private_data;
-	unsigned long flags;
-
-	c67x00_ll_husb_reset(sie, port);
-
-	spin_lock_irqsave(&c67x00->lock, flags);
-	c67x00_ll_husb_reset_port(sie, port);
-	spin_unlock_irqrestore(&c67x00->lock, flags);
-
-	c67x00_ll_set_husb_eot(sie->dev, DEFAULT_EOT);
-}
-
-/* -------------------------------------------------------------------------- */
-
-static int c67x00_hub_status_data(struct usb_hcd *hcd, char *buf)
-{
-	struct c67x00_hcd *c67x00 = hcd_to_c67x00_hcd(hcd);
-	struct c67x00_sie *sie = c67x00->sie;
-	u16 status;
-	int i;
-
-	*buf = 0;
-	status = c67x00_ll_husb_get_status(sie);
-	for (i=0; i<C67X00_PORTS; i++)
-		if (status & PORT_CONNECT_CHANGE(i))
-			*buf |= (1 << i);
-
-	/* bit 0 denotes hub change, b1..n port change */
-	*buf <<= 1;
-
-	return !!*buf;
-}
-
-static int c67x00_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
-			      u16 wIndex, char *buf, u16 wLength)
-{
-	struct c67x00_hcd *c67x00 = hcd_to_c67x00_hcd(hcd);
-	struct c67x00_sie *sie = c67x00->sie;
-	u16 status, usb_status;
-	int retval = 0, len = 0;
-	unsigned int port = wIndex-1;
-	u16 wPortChange, wPortStatus;
-
-	switch (typeReq) {
-
-	case GetHubStatus:
-		*(__le32 *) buf = cpu_to_le32(0);
-		OK(4);		/* hub power */
-	case GetPortStatus:
-		if (wIndex > C67X00_PORTS)
-			goto err;
-
-		status = c67x00_ll_husb_get_status(sie);
-		usb_status = c67x00_ll_get_usb_ctl(sie);
-
-		wPortChange = 0;
-		if (status & PORT_CONNECT_CHANGE(port))
-			wPortChange |= USB_PORT_STAT_C_CONNECTION;
-
-		wPortStatus = USB_PORT_STAT_POWER;
-		if (!(status & PORT_SE0_STATUS(port)))
-			wPortStatus |= USB_PORT_STAT_CONNECTION;
-		if (usb_status & LOW_SPEED_PORT(port)) {
-			wPortStatus |= USB_PORT_STAT_LOW_SPEED;
-			c67x00->low_speed_ports |= (1 << port);
-		} else
-			c67x00->low_speed_ports &= ~(1 << port);
-
-		if (usb_status & SOF_EOP_EN(port))
-			wPortStatus |= USB_PORT_STAT_ENABLE;
-
-		*(__le16 *) buf = cpu_to_le16(wPortStatus);
-		*(__le16 *) (buf + 2) = cpu_to_le16(wPortChange);
-		OK(4);
-	case SetHubFeature:	/* We don't implement these */
-	case ClearHubFeature:
-		switch (wValue) {
-		case C_HUB_OVER_CURRENT:
-		case C_HUB_LOCAL_POWER:
-			OK(0);
-		default:
-			goto err;
-		}
-		break;
-	case SetPortFeature:
-		if (wIndex > C67X00_PORTS)
-			goto err;
-
-		switch (wValue) {
-		case USB_PORT_FEAT_SUSPEND:
-			dev_dbg(c67x00_dev(c67x00),
-				"SetPortFeature %d (SUSPEND)\n", port);
-			OK(0);
-		case USB_PORT_FEAT_RESET:
-			reset_host_port(sie, port);
-			OK(0);
-		case USB_PORT_FEAT_POWER:
-			/* Power always enabled */
-			OK(0);
-		default:
-			dev_dbg(c67x00_dev(c67x00),
-				"%s: SetPortFeature %d (0x%04x) Error!\n",
-				__FUNCTION__, port, wValue);
-			goto err;
-		}
-		break;
-	case ClearPortFeature:
-		if (wIndex > C67X00_PORTS)
-			goto err;
-
-		switch (wValue) {
-		case USB_PORT_FEAT_ENABLE:
-			/* Reset the port so that the c67x00 also notices the
-			 * disconnect */
-			reset_host_port(sie, port);
-			OK(0);
-		case USB_PORT_FEAT_C_ENABLE:
-			dev_dbg(c67x00_dev(c67x00),
-				"ClearPortFeature (%d): C_ENABLE\n", port);
-			OK(0);
-		case USB_PORT_FEAT_SUSPEND:
-			dev_dbg(c67x00_dev(c67x00),
-				"ClearPortFeature (%d): SUSPEND\n", port);
-			OK(0);
-		case USB_PORT_FEAT_C_SUSPEND:
-			dev_dbg(c67x00_dev(c67x00),
-				"ClearPortFeature (%d): C_SUSPEND\n", port);
-			OK(0);
-		case USB_PORT_FEAT_POWER:
-			dev_dbg(c67x00_dev(c67x00),
-				"ClearPortFeature (%d): POWER\n", port);
-			goto err;
-		case USB_PORT_FEAT_C_CONNECTION:
-			c67x00_ll_husb_clear_status(sie,
-						    PORT_CONNECT_CHANGE(port));
-			OK(0);
-		case USB_PORT_FEAT_C_OVER_CURRENT:
-			dev_dbg(c67x00_dev(c67x00),
-				"ClearPortFeature (%d): OVER_CURRENT\n", port);
-			OK(0);
-		case USB_PORT_FEAT_C_RESET:
-			dev_dbg(c67x00_dev(c67x00),
-				"ClearPortFeature (%d): C_RESET\n", port);
-			OK(0);
-		default:
-			dev_dbg(c67x00_dev(c67x00),
-				"%s: ClearPortFeature %d (0x%04x) Error!\n",
-				__FUNCTION__, port, wValue);
-			goto err;
-		}
-		break;
-	case GetHubDescriptor:
-		len = min_t(unsigned int, sizeof(root_hub_hub_des), wLength);
-		memcpy(buf, root_hub_hub_des, len);
-		OK(len);
-	default:
-		dev_dbg(c67x00_dev(c67x00), "%s: unknown\n", __FUNCTION__);
-	      err:
-		retval = -EPIPE;
-	}
-
-	return retval;
-}
-- 
1.4.4.2

Re: [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c

[Peter Korsgaard]

>>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:

Hi,

 Grant> Rather than c67x00-hub.c being compiled seperately, the
 Grant> original code had c67x00-hub.c *included* by c67x00-hcd.c.
 Grant> This is a very bad idea.  Simplest solution is to merge the
 Grant> two files into one and be done with it.

Yeah, it isn't exactly pretty, but it's what the other hcd drivers do,
E.G.:

% grep -rs "include.*hub.c" *c 
ehci-hcd.c:#include "ehci-hub.c"
ohci-hcd.c:#include "ohci-hub.c"
uhci-hcd.c:#include "uhci-hub.c"

I don't quite know why they do it like that though ..

-- 
Bye, Peter Korsgaard

Re: [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c

[Grant Likely]

On 6/12/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> >>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
>
> Hi,
>
>  Grant> Rather than c67x00-hub.c being compiled seperately, the
>  Grant> original code had c67x00-hub.c *included* by c67x00-hcd.c.
>  Grant> This is a very bad idea.  Simplest solution is to merge the
>  Grant> two files into one and be done with it.
>
> Yeah, it isn't exactly pretty, but it's what the other hcd drivers do,
> E.G.:
>
> % grep -rs "include.*hub.c" *c
> ehci-hcd.c:#include "ehci-hub.c"
> ohci-hcd.c:#include "ohci-hub.c"
> uhci-hcd.c:#include "uhci-hub.c"
>
> I don't quite know why they do it like that though ..

True, but that doesn't mean that it's a good idea to follow the lead.
There are lots of other examples of ugly code in the kernel that is
tolerated just because nobody has cleaned it up yet, but is still
unacceptable for new code.

We know it's an ugly thing to do, and the fix is simple and easy.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [linux-usb-devel] [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c

[phil culler]

[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]

On 6/13/07, Grant Likely <grant.likely@secretlab.ca> wrote:
>
> On 6/12/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> > >>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
> >
> > Hi,
> >
> >  Grant> Rather than c67x00-hub.c being compiled seperately, the
> >  Grant> original code had c67x00-hub.c *included* by c67x00-hcd.c.
> >  Grant> This is a very bad idea.  Simplest solution is to merge the
> >  Grant> two files into one and be done with it.




Hi Guys,
> I'm currently implementing the gadget API for the c67x00 (specifically
> CY7C63200) and have several questions.
>
> 1. Are you aware of any others working on the gadget API for this driver?
> 2. I'm doing this on an embedded device (Xilinix/PPC) with no hardware DMA
> and on top of MontaVista 4.01 (2.6.10-kernel). I'd prefer to develop on a
> current kernel using something like a PCI card. Can you recommend a
> reasonably-priced development platform for the c67x00?
> 3. Any suggestions about preventing additional ugly code? I'm adding the
> bulk of the API in c67x00-udc.c
>
> Many thanks,
> -Phil Culler
> phil@pliablerhino.com
>

[-- Attachment #2: Type: text/html, Size: 1848 bytes --]

Re: [linux-usb-devel] [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c

[Grant Likely]

On 6/13/07, phil culler <phil.culler@gmail.com> wrote:
>
> On 6/13/07, Grant Likely <grant.likely@secretlab.ca> wrote:
> > On 6/12/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> > > >>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca > writes:
> > >
> > > Hi,
> > >
> > >  Grant> Rather than c67x00-hub.c being compiled seperately, the
> > >  Grant> original code had c67x00-hub.c *included* by c67x00-hcd.c.
> > >  Grant> This is a very bad idea.  Simplest solution is to merge the
> > >  Grant> two files into one and be done with it.
>
>
> > Hi Guys,
> > I'm currently implementing the gadget API for the c67x00 (specifically
> CY7C63200) and have several questions.
> >
> > 1. Are you aware of any others working on the gadget API for this driver?

Other than the skeleton that Peter wrote, No.  However, Peter has done
much of the low level work that is shared between HCD and Gadget.  You
need to look at his patchset.

http://thread.gmane.org/gmane.linux.usb.devel/53285

> > 2. I'm doing this on an embedded device (Xilinix/PPC) with no hardware DMA
> and on top of MontaVista 4.01 (2.6.10-kernel). I'd prefer to develop on a
> current kernel using something like a PCI card. Can you recommend a
> reasonably-priced development platform for the c67x00?

Hmm, I don't know.  I don't know of any.  I'm doing all my devel on an
ml403.  Check with Cypress.  I think they have an ARM based
development kit for the c67x00.

> > 3. Any suggestions about preventing additional ugly code? I'm adding the
> bulk of the API in c67x00-udc.c

Yes, post your patches to the list so so others can comment.  That's
the best way to find out if you're taking a bad approach.  Also,
follow the Linux kernel coding convention and run your code through
scripts/Lindent

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [linux-usb-devel] [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c

[Alan Stern]

On Wed, 13 Jun 2007, Grant Likely wrote:

> On 6/12/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> > >>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
> >
> > Hi,
> >
> >  Grant> Rather than c67x00-hub.c being compiled seperately, the
> >  Grant> original code had c67x00-hub.c *included* by c67x00-hcd.c.
> >  Grant> This is a very bad idea.

What's so bad about it?  It's an elegant solution to the problem of 
breaking a very long driver up into smaller, more digestible pieces 
without polluting the kernel's namespace with lots of extra global 
symbols.

> >   Simplest solution is to merge the
> >  Grant> two files into one and be done with it.
> >
> > Yeah, it isn't exactly pretty, but it's what the other hcd drivers do,
> > E.G.:
> >
> > % grep -rs "include.*hub.c" *c
> > ehci-hcd.c:#include "ehci-hub.c"
> > ohci-hcd.c:#include "ohci-hub.c"
> > uhci-hcd.c:#include "uhci-hub.c"
> >
> > I don't quite know why they do it like that though ..
> 
> True, but that doesn't mean that it's a good idea to follow the lead.

Why not?

> There are lots of other examples of ugly code in the kernel that is
> tolerated just because nobody has cleaned it up yet, but is still
> unacceptable for new code.

What's so ugly about breaking a driver up into pieces?  Leaving it in 
one giant piece would be much more ugly IMO.

> We know it's an ugly thing to do, and the fix is simple and easy.

As the Firesign Theater once said, Everything you know is wrong!  :-)

Alan Stern

Re: [linux-usb-devel] [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c

[Grant Likely]

On 6/13/07, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Wed, 13 Jun 2007, Grant Likely wrote:
>
> > On 6/12/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> > > >>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
> > >
> > > Hi,
> > >
> > >  Grant> Rather than c67x00-hub.c being compiled seperately, the
> > >  Grant> original code had c67x00-hub.c *included* by c67x00-hcd.c.
> > >  Grant> This is a very bad idea.
>
> What's so bad about it?  It's an elegant solution to the problem of
> breaking a very long driver up into smaller, more digestible pieces
> without polluting the kernel's namespace with lots of extra global
> symbols.

Primarily because it breaks convention.  Convention is that you
#include .h files, and you compile and link .c files.  Convention is
important because it reflects the common patterns we use when reading
and writing (but mostly reading) code.

Yes there are exceptions, and yes it can be done, but there better be
a damn good reason for doing so.  In this particular case, I really
don't think it is warranted.  We're not talking about a great deal of
code, and we're *already* polluting the kernel namespace with c67x00_*
function names because the driver is already in multiple pieces.

This issue has also come up on the LKML also.  See this thread:

http://thread.gmane.org/gmane.linux.kernel/498633

> What's so ugly about breaking a driver up into pieces?  Leaving it in
> one giant piece would be much more ugly IMO.

Breaking into pieces: Good, and I fully agree.
Doing it in non-standard way: Not so good as it trades one kind of
ugliness for another.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [linux-usb-devel] [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c

[Alan Stern]

On Wed, 13 Jun 2007, Grant Likely wrote:

> On 6/13/07, Alan Stern <stern@rowland.harvard.edu> wrote:
> > On Wed, 13 Jun 2007, Grant Likely wrote:
> >
> > > On 6/12/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> > > > >>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
> > > >
> > > > Hi,
> > > >
> > > >  Grant> Rather than c67x00-hub.c being compiled seperately, the
> > > >  Grant> original code had c67x00-hub.c *included* by c67x00-hcd.c.
> > > >  Grant> This is a very bad idea.
> >
> > What's so bad about it?  It's an elegant solution to the problem of
> > breaking a very long driver up into smaller, more digestible pieces
> > without polluting the kernel's namespace with lots of extra global
> > symbols.
> 
> Primarily because it breaks convention.  Convention is that you
> #include .h files, and you compile and link .c files.  Convention is
> important because it reflects the common patterns we use when reading
> and writing (but mostly reading) code.

The problem is that there are conflicting conventions.  You mentioned 
one.  But there's another: .h files contain declarations and things 
that should be shared among multiple source files, and .c files contain 
things that generate object code (executable routines, static 
definitions, and so on).  The idea is that sharing something which 
generates object code would be a mistake, since every source file which 
included it would generate a copy of those same objects.

So if you want to #include a file which generates object code, one 
convention says it should be named .h and the other says it should be 
named .c.  A possible solution would be to use yet a different suffix, 
but I think that would only make matters worse.

(Just to add to the confusion, some people feel that .c files shouldn't
include much that _doesn't_ generate object code.  Hence they put
top-level declarations in a .h file, even though it is #included in
only one .c file.  This is mainly a matter of taste...)

> Yes there are exceptions, and yes it can be done, but there better be
> a damn good reason for doing so.  In this particular case, I really
> don't think it is warranted.

The reason for doing it is the second convention.  IMO that's just as
good a reason for doing it as the first convention is for not doing it.

>  We're not talking about a great deal of
> code, and we're *already* polluting the kernel namespace with c67x00_*
> function names because the driver is already in multiple pieces.

Sorry, I don't know what you mean.  How does the fact that uhci-hcd is 
in multiple pieces create names like c67x00_*?  Besides, the fact that 
we are already doing it doesn't justify unnecessarily doing even more.

> This issue has also come up on the LKML also.  See this thread:
> 
> http://thread.gmane.org/gmane.linux.kernel/498633

I read that thread some time ago.  If you look at it carefully, you'll
find that Linus is arguing in favor of the second convention -- mine, 
not yours.

> > What's so ugly about breaking a driver up into pieces?  Leaving it in
> > one giant piece would be much more ugly IMO.
> 
> Breaking into pieces: Good, and I fully agree.
> Doing it in non-standard way: Not so good as it trades one kind of
> ugliness for another.

But what should one do when there are two conflicting standards?

Alan Stern

Re: [linux-usb-devel] [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c

[Grant Likely]

On 6/13/07, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Wed, 13 Jun 2007, Grant Likely wrote:
>
> > On 6/13/07, Alan Stern <stern@rowland.harvard.edu> wrote:
> > > On Wed, 13 Jun 2007, Grant Likely wrote:
> > >
> > > > On 6/12/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> > > > > >>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
> > > > >
> > > > > Hi,
> > > > >
> > > > >  Grant> Rather than c67x00-hub.c being compiled seperately, the
> > > > >  Grant> original code had c67x00-hub.c *included* by c67x00-hcd.c.
> > > > >  Grant> This is a very bad idea.
> > >
> > > What's so bad about it?  It's an elegant solution to the problem of
> > > breaking a very long driver up into smaller, more digestible pieces
> > > without polluting the kernel's namespace with lots of extra global
> > > symbols.
> >
> > Primarily because it breaks convention.  Convention is that you
> > #include .h files, and you compile and link .c files.  Convention is
> > important because it reflects the common patterns we use when reading
> > and writing (but mostly reading) code.
>
> The problem is that there are conflicting conventions.  You mentioned
> one.  But there's another: .h files contain declarations and things
> that should be shared among multiple source files, and .c files contain
> things that generate object code (executable routines, static
> definitions, and so on).  The idea is that sharing something which
> generates object code would be a mistake, since every source file which
> included it would generate a copy of those same objects.

I agree 100%

> So if you want to #include a file which generates object code, one
> convention says it should be named .h and the other says it should be
> named .c.  A possible solution would be to use yet a different suffix,
> but I think that would only make matters worse.

Right, so I disagree with both approaches.  If it generates object
code, it should go into a .c file which is compiled and linked on it's
own.

> (Just to add to the confusion, some people feel that .c files shouldn't
> include much that _doesn't_ generate object code.  Hence they put
> top-level declarations in a .h file, even though it is #included in
> only one .c file.  This is mainly a matter of taste...)

Heeheehee

> > Yes there are exceptions, and yes it can be done, but there better be
> > a damn good reason for doing so.  In this particular case, I really
> > don't think it is warranted.
>
> The reason for doing it is the second convention.  IMO that's just as
> good a reason for doing it as the first convention is for not doing it.

I think we're crossing wires here.  In this particular case, I think
the hub support code is sufficiently small that it doesn't need to be
split off into a separate file.  (It's only 180 lines)  I'm not
suggesting that the hub support stuff be moved to a .h file.

If the code was larger, I argue that c67x00-hub.c should be compiled
separately from c67x00-hcd.c and that 'c67x00-hub.c' be added to
'c67x00-$(CONFIG_USB_C67X00_HCD)' in the Makefile.
>
> >  We're not talking about a great deal of
> > code, and we're *already* polluting the kernel namespace with c67x00_*
> > function names because the driver is already in multiple pieces.
>
> Sorry, I don't know what you mean.  How does the fact that uhci-hcd is
> in multiple pieces create names like c67x00_*?  Besides, the fact that
> we are already doing it doesn't justify unnecessarily doing even more.

Heh, 'polluted' is too loaded a term, and it suggests something I
didn't mean.  When the driver is built into the kernel, there are a
number of c67x00_* symbols which are exported.  These symbols are not
used anywhere other than in the c67x00 driver code.  However, this is
necessary because the overall driver splits the various subsystems
into separate .c files which are linked together.  This approach is
well supported by convention in the kernel, and all the non-static
symbols use the c67x00_ prefix to avoid collisions.

For example, see ib_core-y in drivers/infiniband/core/Makefile and
pcieportdrv-y in drivers/pci/pcie/Makefile.

Since the driver already makes use of this approach, I don't think it
makes sense to use a difference approach for the root hub support
code.  (Again, I'm specifically talking about the c67x00 driver here;
I've not looked at the *hci drivers in detail).

Of course, when it is built as a module, none of those symbols show up
because they are not exported.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [linux-usb-devel] [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c

[Alan Stern]

On Wed, 13 Jun 2007, Grant Likely wrote:

> I think we're crossing wires here.  In this particular case, I think
> the hub support code is sufficiently small that it doesn't need to be
> split off into a separate file.  (It's only 180 lines)  I'm not
> suggesting that the hub support stuff be moved to a .h file.
> 
> If the code was larger, I argue that c67x00-hub.c should be compiled
> separately from c67x00-hcd.c and that 'c67x00-hub.c' be added to
> 'c67x00-$(CONFIG_USB_C67X00_HCD)' in the Makefile.

> Heh, 'polluted' is too loaded a term, and it suggests something I
> didn't mean.  When the driver is built into the kernel, there are a
> number of c67x00_* symbols which are exported.  These symbols are not
> used anywhere other than in the c67x00 driver code.  However, this is
> necessary because the overall driver splits the various subsystems
> into separate .c files which are linked together.  This approach is
> well supported by convention in the kernel, and all the non-static
> symbols use the c67x00_ prefix to avoid collisions.
> 
> For example, see ib_core-y in drivers/infiniband/core/Makefile and
> pcieportdrv-y in drivers/pci/pcie/Makefile.
> 
> Since the driver already makes use of this approach, I don't think it
> makes sense to use a difference approach for the root hub support
> code.  (Again, I'm specifically talking about the c67x00 driver here;
> I've not looked at the *hci drivers in detail).

But you did say earlier that the way other drivers were written was a
bad idea...

In any case I agree that the root-hub code in c67x00 should be written
to match the style used by the rest of the driver.

> Of course, when it is built as a module, none of those symbols show up
> because they are not exported.

True.  And true as well of the other drivers which #include *.c files.

Alan Stern

I2C interrupts on 8541

[Charles Krinke]

I'm puzzled about how to setup interrupts for the second I2C interface
in an 8541. This is the CPM interface, the one with buffer descriptors.

I see mention of its I2CER/I2CMR registers, but am having trouble
figuring out how to enable and interrupt so that when the interface is
in slave mode and a packet is received, it will vector to an interrupt
service routine.

I am using Linux-2.7.17.11 and I know you-all have gone past this a bit,
but in the midst of a project, we are constrained to finish with the
kernel we started with.

A few pointers on enabling an interrupt from this interface would be
greatly appreciated.

Charles

Re: I2C interrupts on 8541

[Kumar Gala]

On Jul 30, 2007, at 11:51 AM, Charles Krinke wrote:

> I'm puzzled about how to setup interrupts for the second I2C interface
> in an 8541. This is the CPM interface, the one with buffer  
> descriptors.
>
> I see mention of its I2CER/I2CMR registers, but am having trouble
> figuring out how to enable and interrupt so that when the interface is
> in slave mode and a packet is received, it will vector to an interrupt
> service routine.
>
> I am using Linux-2.7.17.11 and I know you-all have gone past this a  
> bit,
> but in the midst of a project, we are constrained to finish with the
> kernel we started with.
>
> A few pointers on enabling an interrupt from this interface would be
> greatly appreciated.

To get interrupts you need to use the BD rings.  Look at the section  
'I2C Buffer Descriptors' in the 8555 UM and it shows 'I' bits in both  
the TX & RX rings that should cause interrupts.

- k