Skip to contentSkip to conversation

USB: isp1760: Support board-specific hardware configurations

PATCH Applied Nate Case 3 participants 4 messages May 21, 2008 – May 23, 2008 USB

lore
Share & tools
lore

Nate Case

author
#0 lore raw

PATCH 2/2 USB: isp1760: Support board-specific hardware configurations

This adds support for hardware configurations that don't match the chip default register settings (e.g., 16-bit data bus, DACK and DREQ pulled down instead of up, analog overcurrent mode).

These settings are passed in via the OF device tree. The PCI interface still assumes the same default values.

Signed-off-by Nate Case <[email protected]>
---
 drivers/usb/host/isp1760-hcd.c |   68 +++++++++++++++++++++++++++++++--------
 drivers/usb/host/isp1760-hcd.h |   19 ++++++++++-
 drivers/usb/host/isp1760-if.c  |   34 +++++++++++++++++++-
 3 files changed, 103 insertions(+), 18 deletions(-)
drivers/usb/host/isp1760-hcd.c +54 -14
index 65aa5ec..679b8df 100644
@@ -38,6 +38,7 @@ struct isp1760_hcd {
 	unsigned		i_thresh;
 	unsigned long		reset_done;
 	unsigned long		next_statechange;
+	unsigned int		devflags;
 };
 
 static inline struct isp1760_hcd *hcd_to_priv(struct usb_hcd *hcd)

@@ -378,9 +379,31 @@ static int isp1760_hc_setup(struct usb_hcd *hcd)
 {
 	struct isp1760_hcd *priv = hcd_to_priv(hcd);
 	int result;
-	u32 scratch;
+	u32 scratch, hwmode;
+
+	/* Setup HW Mode Control: This assumes a level active-low interrupt */
+	hwmode = HW_DATA_BUS_32BIT;
+
+	if (priv->devflags & ISP1760_FLAG_BUS_WIDTH_16)
+		hwmode &= ~HW_DATA_BUS_32BIT;
+	if (priv->devflags & ISP1760_FLAG_ANALOG_OC)
+		hwmode |= HW_ANA_DIGI_OC;
+	if (priv->devflags & ISP1760_FLAG_DACK_POL_HIGH)
+		hwmode |= HW_DACK_POL_HIGH;
+	if (priv->devflags & ISP1760_FLAG_DREQ_POL_HIGH)
+		hwmode |= HW_DREQ_POL_HIGH;
+
+	/*
+	 * We have to set this first in case we're in 16-bit mode.
+	 * Write it twice to ensure correct upper bits if switching
+	 * to 16-bit mode.
+	 */
+	isp1760_writel(hwmode, hcd->regs + HC_HW_MODE_CTRL);
+	isp1760_writel(hwmode, hcd->regs + HC_HW_MODE_CTRL);
 
 	isp1760_writel(0xdeadbabe, hcd->regs + HC_SCRATCH_REG);
+	/* Change bus pattern */
+	scratch = isp1760_readl(hcd->regs + HC_CHIP_ID_REG);
 	scratch = isp1760_readl(hcd->regs + HC_SCRATCH_REG);
 	if (scratch != 0xdeadbabe) {
 		printk(KERN_ERR "ISP1760: Scratch test failed.\n");

@@ -403,17 +426,30 @@ static int isp1760_hc_setup(struct usb_hcd *hcd)
 
 	/* Step 11 passed */
 
-	isp1760_writel(INTERRUPT_ENABLE_MASK, hcd->regs + HC_INTERRUPT_REG);
-	isp1760_writel(INTERRUPT_ENABLE_MASK, hcd->regs + HC_INTERRUPT_ENABLE);
+	isp1760_info(priv, "bus width: %d, oc: %s\n",
+			   (priv->devflags & ISP1760_FLAG_BUS_WIDTH_16) ?
+			   16 : 32, (priv->devflags & ISP1760_FLAG_ANALOG_OC) ?
+			   "analog" : "digital");
 
 	/* ATL reset */
-	scratch = isp1760_readl(hcd->regs + HC_HW_MODE_CTRL);
-	isp1760_writel(scratch | ALL_ATX_RESET, hcd->regs + HC_HW_MODE_CTRL);
+	isp1760_writel(hwmode | ALL_ATX_RESET, hcd->regs + HC_HW_MODE_CTRL);
 	mdelay(10);
-	isp1760_writel(scratch, hcd->regs + HC_HW_MODE_CTRL);
+	isp1760_writel(hwmode, hcd->regs + HC_HW_MODE_CTRL);
 
-	isp1760_writel(PORT1_POWER | PORT1_INIT2, hcd->regs + HC_PORT1_CTRL);
-	mdelay(10);
+	isp1760_writel(INTERRUPT_ENABLE_MASK, hcd->regs + HC_INTERRUPT_REG);
+	isp1760_writel(INTERRUPT_ENABLE_MASK, hcd->regs + HC_INTERRUPT_ENABLE);
+
+	/*
+	 * PORT 1 Control register of the ISP1760 is the OTG control
+	 * register on ISP1761.
+	 */
+	scratch = isp1760_readl(hcd->regs + HC_CHIP_ID_REG);
+	if (((scratch & 0xffff) == 0x1760) &&
+	    !(priv->devflags & ISP1760_FLAG_PORT1_DIS)) {
+		isp1760_writel(PORT1_POWER | PORT1_INIT2,
+			       hcd->regs + HC_PORT1_CTRL);
+		mdelay(10);
+	}
 
 	priv->hcs_params = isp1760_readl(hcd->regs + HC_HCSPARAMS);
 

@@ -453,8 +489,7 @@ static int isp1760_run(struct usb_hcd *hcd)
 	hcd->state = HC_STATE_RUNNING;
 	isp1760_enable_interrupts(hcd);
 	temp = isp1760_readl(hcd->regs + HC_HW_MODE_CTRL);
-	temp |= FINAL_HW_CONFIG;
-	isp1760_writel(temp, hcd->regs + HC_HW_MODE_CTRL);
+	isp1760_writel(temp | HW_GLOBAL_INTR_EN, hcd->regs + HC_HW_MODE_CTRL);
 
 	command = isp1760_readl(hcd->regs + HC_USBCMD);
 	command &= ~(CMD_LRESET|CMD_RESET);

@@ -2112,6 +2147,7 @@ static int isp1760_get_frame(struct usb_hcd *hcd)
 static void isp1760_stop(struct usb_hcd *hcd)
 {
 	struct isp1760_hcd *priv = hcd_to_priv(hcd);
+	u32 temp;
 
 	isp1760_hub_control(hcd, ClearPortFeature, USB_PORT_FEAT_POWER,	1,
 			NULL, 0);

@@ -2120,7 +2156,8 @@ static void isp1760_stop(struct usb_hcd *hcd)
 	spin_lock_irq(&priv->lock);
 	ehci_reset(priv);
 	/* Disable IRQ */
-	isp1760_writel(HW_DATA_BUS_32BIT, hcd->regs + HC_HW_MODE_CTRL);
+	temp = isp1760_readl(hcd->regs + HC_HW_MODE_CTRL);
+	isp1760_writel(temp &= ~HW_GLOBAL_INTR_EN, hcd->regs + HC_HW_MODE_CTRL);
 	spin_unlock_irq(&priv->lock);
 
 	isp1760_writel(0, hcd->regs + HC_CONFIGFLAG);

@@ -2128,10 +2165,11 @@ static void isp1760_stop(struct usb_hcd *hcd)
 
 static void isp1760_shutdown(struct usb_hcd *hcd)
 {
-	u32 command;
+	u32 command, temp;
 
 	isp1760_stop(hcd);
-	isp1760_writel(HW_DATA_BUS_32BIT, hcd->regs + HC_HW_MODE_CTRL);
+	temp = isp1760_readl(hcd->regs + HC_HW_MODE_CTRL);
+	isp1760_writel(temp &= ~HW_GLOBAL_INTR_EN, hcd->regs + HC_HW_MODE_CTRL);
 
 	command = isp1760_readl(hcd->regs + HC_USBCMD);
 	command &= ~CMD_RUN;

@@ -2183,7 +2221,8 @@ void deinit_kmem_cache(void)
 }
 
 struct usb_hcd *isp1760_register(u64 res_start, u64 res_len, int irq,
-		u64 irqflags, struct device *dev, const char *busname)
+		u64 irqflags, struct device *dev, const char *busname,
+		unsigned int devflags)
 {
 	struct usb_hcd *hcd;
 	struct isp1760_hcd *priv;

@@ -2200,6 +2239,7 @@ struct usb_hcd *isp1760_register(u64 res_start, u64 res_len, int irq,
 		return ERR_PTR(-ENOMEM);
 
 	priv = hcd_to_priv(hcd);
+	priv->devflags = devflags;
 	init_memory(priv);
 	hcd->regs = ioremap(res_start, res_len);
 	if (!hcd->regs) {
drivers/usb/host/isp1760-hcd.h +17 -2
index 3d86d0f..732fb2a 100644
@@ -3,7 +3,8 @@
 
 /* exports for if */
 struct usb_hcd *isp1760_register(u64 res_start, u64 res_len, int irq,
-		u64 irqflags, struct device *dev, const char *busname);
+		u64 irqflags, struct device *dev, const char *busname,
+		unsigned int devflags);
 int init_kmem_once(void);
 void deinit_kmem_cache(void);
 

@@ -31,6 +32,7 @@ void deinit_kmem_cache(void);
 /* Configuration Register */
 #define HC_HW_MODE_CTRL		0x300
 #define ALL_ATX_RESET		(1 << 31)
+#define HW_ANA_DIGI_OC		(1 << 15)
 #define HW_DATA_BUS_32BIT	(1 << 8)
 #define HW_DACK_POL_HIGH	(1 << 6)
 #define HW_DREQ_POL_HIGH	(1 << 5)

@@ -56,13 +58,14 @@ void deinit_kmem_cache(void);
 #define PORT1_POWER		(3 << 3)
 #define PORT1_INIT1		(1 << 7)
 #define PORT1_INIT2		(1 << 23)
+#define HW_OTG_CTRL_SET		0x374
+#define HW_OTG_CTRL_CLR		0x376
 
 /* Interrupt Register */
 #define HC_INTERRUPT_REG	0x310
 
 #define HC_INTERRUPT_ENABLE	0x314
 #define INTERRUPT_ENABLE_MASK	(HC_INTL_INT | HC_ATL_INT | HC_EOT_INT)
-#define FINAL_HW_CONFIG	(HW_GLOBAL_INTR_EN | HW_DATA_BUS_32BIT)
 
 #define HC_ISO_INT		(1 << 9)
 #define HC_ATL_INT		(1 << 8)

@@ -122,6 +125,18 @@ typedef void (packet_enqueue)(struct usb_hcd *hcd, struct isp1760_qh *qh,
 #define isp1760_err(priv, fmt, args...) \
 	dev_err(priv_to_hcd(priv)->self.controller, fmt, ##args)
 
+/*
+ * Device flags that can vary from board to board.  All of these
+ * indicate the most "atypical" case, so that a devflags of 0 is
+ * a sane default configuration.
+ */
+#define ISP1760_FLAG_PORT1_DIS		0x00000001 /* Port 1 disabled */
+#define ISP1760_FLAG_BUS_WIDTH_16	0x00000002 /* 16-bit data bus width */
+#define ISP1760_FLAG_OTG_EN		0x00000004 /* Port 1 supports OTG */
+#define ISP1760_FLAG_ANALOG_OC		0x00000008 /* Analog overcurrent */
+#define ISP1760_FLAG_DACK_POL_HIGH	0x00000010 /* DACK active high */
+#define ISP1760_FLAG_DREQ_POL_HIGH	0x00000020 /* DREQ active high */
+
 /* chip memory management */
 struct memory_chunk {
 	unsigned int start;
drivers/usb/host/isp1760-if.c +32 -2
index 440bf94..c50c3f5 100644
@@ -35,6 +35,8 @@ static int of_isp1760_probe(struct of_device *dev,
 	int virq;
 	u64 res_len;
 	int ret;
+	const unsigned int *prop;
+	unsigned int devflags = 0;
 
 	ret = of_address_to_resource(dp, 0, &memory);
 	if (ret)

@@ -55,8 +57,34 @@ static int of_isp1760_probe(struct of_device *dev,
 	virq = irq_create_of_mapping(oirq.controller, oirq.specifier,
 			oirq.size);
 
+	prop = of_get_property(dp, "port1-disable", NULL);
+	if (prop && *prop != 0)
+		devflags |= ISP1760_FLAG_PORT1_DIS;
+
+	/* Some systems wire up only 16 of the 32 data lines */
+	prop = of_get_property(dp, "bus-width", NULL);
+	if (prop && *prop == 16)
+		devflags |= ISP1760_FLAG_BUS_WIDTH_16;
+
+	prop = of_get_property(dp, "port1-otg", NULL);
+	if (prop && *prop != 0)
+		devflags |= ISP1760_FLAG_OTG_EN;
+
+	prop = of_get_property(dp, "analog-oc", NULL);
+	if (prop && *prop != 0)
+		devflags |= ISP1760_FLAG_ANALOG_OC;
+
+	prop = of_get_property(dp, "dack-polarity", NULL);
+	if (prop && *prop != 0)
+		devflags |= ISP1760_FLAG_DACK_POL_HIGH;
+
+	prop = of_get_property(dp, "dreq-polarity", NULL);
+	if (prop && *prop != 0)
+		devflags |= ISP1760_FLAG_DREQ_POL_HIGH;
+
 	hcd = isp1760_register(memory.start, res_len, virq,
-		IRQF_SHARED | IRQF_DISABLED, &dev->dev, dev->dev.bus_id);
+		IRQF_SHARED | IRQF_DISABLED, &dev->dev, dev->dev.bus_id,
+		devflags);
 	if (IS_ERR(hcd)) {
 		ret = PTR_ERR(hcd);
 		goto release_reg;

@@ -116,6 +144,7 @@ static int __devinit isp1761_pci_probe(struct pci_dev *dev,
 	int length;
 	int status = 1;
 	struct usb_hcd *hcd;
+	unsigned int devflags = 0;
 
 	if (usb_disabled())
 		return -ENODEV;

@@ -200,7 +229,8 @@ static int __devinit isp1761_pci_probe(struct pci_dev *dev,
 
 	dev->dev.dma_mask = NULL;
 	hcd = isp1760_register(pci_mem_phy0, length, dev->irq,
-		IRQF_SHARED | IRQF_DISABLED, &dev->dev, dev->dev.bus_id);
+		IRQF_SHARED | IRQF_DISABLED, &dev->dev, dev->dev.bus_id,
+		devflags);
 	pci_set_drvdata(dev, hcd);
 	if (!hcd)
 		return 0;
Signature · 1 line
1.5.4.4
#1 lore raw
Quoted 6 lines · Nate Case

This adds support for hardware configurations that don't match the chip default register settings (e.g., 16-bit data bus, DACK and DREQ pulled down instead of up, analog overcurrent mode).

These settings are passed in via the OF device tree. The PCI interface still assumes the same default values.

Nice! I'll give this a spin on Electra as well, it should make it work there.

A couple of comments on the OF interface:

drivers/usb/host/isp1760-if.c static int of_isp1760_probe(struct of_device *dev, · line 62

Quoted 7 lines
drivers/usb/host/isp1760-if.c +3 -0
@@ -55,8 +57,34 @@ static int of_isp1760_probe(struct of_device *dev,
 	virq = irq_create_of_mapping(oirq.controller, oirq.specifier,
 			oirq.size);
 
+	prop = of_get_property(dp, "port1-disable", NULL);
+	if (prop && *prop != 0)
+		devflags |= ISP1760_FLAG_PORT1_DIS;

It should be sufficient to add the property without a value, and just check for the presence of it. The cpu nodes have properties like this if you want something to compare with.

drivers/usb/host/isp1760-if.c static int of_isp1760_probe(struct of_device *dev, · line 67

Quoted 5 lines
drivers/usb/host/isp1760-if.c +5 -0
+
+	/* Some systems wire up only 16 of the 32 data lines */
+	prop = of_get_property(dp, "bus-width", NULL);
+	if (prop && *prop == 16)
+		devflags |= ISP1760_FLAG_BUS_WIDTH_16;

This is the obvious exception, here you'll have to check the value.

I'm not sure if the DT police will complain of the overloaded bus-width property name and would prefer a custom one.

-Olof

#2 lore raw
In reply to Nate Case
Quoted 3 lines · * Nate Case | 2008-05-21 16:28:51 [-0500]:

This adds support for hardware configurations that don't match the chip default register settings (e.g., 16-bit data bus, DACK and DREQ pulled down instead of up, analog overcurrent mode).

Nice patch. A few comments inline.

Quoted 2 lines

These settings are passed in via the OF device tree. The PCI interface still assumes the same default values.

The default values should be okay for PCI . It should only be required for the eval kit from NXP. There was an ISP1761 behind a PLB bridge. I had all three ports functional there (no OTG) in my first shot after enabling or disabling some resistors (don't remember the details anymore). Than I moved to my powerpc and returned the eval kit.

Quoted 21 lines
Signed-off-by Nate Case <[email protected]>

---

drivers/usb/host/isp1760-hcd.c |   68 +++++++++++++++++++++++++++++++--------
drivers/usb/host/isp1760-hcd.h |   19 ++++++++++-
drivers/usb/host/isp1760-if.c  |   34 +++++++++++++++++++-
3 files changed, 103 insertions(+), 18 deletions(-)
drivers/usb/host/isp1760-hcd.c +1 -0
index 65aa5ec..679b8df 100644
@@ -38,6 +38,7 @@ struct isp1760_hcd {
 	unsigned		i_thresh;
 	unsigned long		reset_done;
 	unsigned long		next_statechange;
+	unsigned int		devflags;
 };
 
 static inline struct isp1760_hcd *hcd_to_priv(struct usb_hcd *hcd)

@@ -378,9 +379,31 @@ static int isp1760_hc_setup(struct usb_hcd *hcd)
 {

drivers/usb/host/isp1760-hcd.c static int isp1760_hc_setup(struct usb_hcd *hcd) · line 388

Quoted 10 lines
drivers/usb/host/isp1760-hcd.c +7 -1
 	struct isp1760_hcd *priv = hcd_to_priv(hcd);
 	int result;
-	u32 scratch;
+	u32 scratch, hwmode;
+
+	/* Setup HW Mode Control: This assumes a level active-low interrupt */
+	hwmode = HW_DATA_BUS_32BIT;
+
+	if (priv->devflags & ISP1760_FLAG_BUS_WIDTH_16)
+		hwmode &= ~HW_DATA_BUS_32BIT;

If you don't mind, I would prefer having ISP1760_FLAG_BUS_WIDTH_32 and or conditionally HW_DATA_BUS_32BIT.

Quoted 9 lines
drivers/usb/host/isp1760-hcd.c +9 -0
+	if (priv->devflags & ISP1760_FLAG_ANALOG_OC)
+		hwmode |= HW_ANA_DIGI_OC;
+	if (priv->devflags & ISP1760_FLAG_DACK_POL_HIGH)
+		hwmode |= HW_DACK_POL_HIGH;
+	if (priv->devflags & ISP1760_FLAG_DREQ_POL_HIGH)
+		hwmode |= HW_DREQ_POL_HIGH;
+
+	/*
+	 * We have to set this first in case we're in 16-bit mode.

drivers/usb/host/isp1760-hcd.c static int isp1760_hc_setup(struct usb_hcd *hcd) · line 407

Quoted 10 lines
drivers/usb/host/isp1760-hcd.c +7 -0
+	 * Write it twice to ensure correct upper bits if switching
+	 * to 16-bit mode.
+	 */
+	isp1760_writel(hwmode, hcd->regs + HC_HW_MODE_CTRL);
+	isp1760_writel(hwmode, hcd->regs + HC_HW_MODE_CTRL);
 
 	isp1760_writel(0xdeadbabe, hcd->regs + HC_SCRATCH_REG);
+	/* Change bus pattern */
+	scratch = isp1760_readl(hcd->regs + HC_CHIP_ID_REG);
 	scratch = isp1760_readl(hcd->regs + HC_SCRATCH_REG);

Why do you have to read the chip id here? Is this a dummy read to ensure something?

Quoted 27 lines
drivers/usb/host/isp1760-hcd.c +10 -7
 	if (scratch != 0xdeadbabe) {
 		printk(KERN_ERR "ISP1760: Scratch test failed.\n");

@@ -403,17 +426,30 @@ static int isp1760_hc_setup(struct usb_hcd *hcd)
 
 	/* Step 11 passed */
 
-	isp1760_writel(INTERRUPT_ENABLE_MASK, hcd->regs + HC_INTERRUPT_REG);
-	isp1760_writel(INTERRUPT_ENABLE_MASK, hcd->regs + HC_INTERRUPT_ENABLE);
+	isp1760_info(priv, "bus width: %d, oc: %s\n",
+			   (priv->devflags & ISP1760_FLAG_BUS_WIDTH_16) ?
+			   16 : 32, (priv->devflags & ISP1760_FLAG_ANALOG_OC) ?
+			   "analog" : "digital");
 
 	/* ATL reset */
-	scratch = isp1760_readl(hcd->regs + HC_HW_MODE_CTRL);
-	isp1760_writel(scratch | ALL_ATX_RESET, hcd->regs + HC_HW_MODE_CTRL);
+	isp1760_writel(hwmode | ALL_ATX_RESET, hcd->regs + HC_HW_MODE_CTRL);
 	mdelay(10);
-	isp1760_writel(scratch, hcd->regs + HC_HW_MODE_CTRL);
+	isp1760_writel(hwmode, hcd->regs + HC_HW_MODE_CTRL);
 
-	isp1760_writel(PORT1_POWER | PORT1_INIT2, hcd->regs + HC_PORT1_CTRL);
-	mdelay(10);
+	isp1760_writel(INTERRUPT_ENABLE_MASK, hcd->regs + HC_INTERRUPT_REG);
+	isp1760_writel(INTERRUPT_ENABLE_MASK, hcd->regs + HC_INTERRUPT_ENABLE);
+
+	/*

drivers/usb/host/isp1760-hcd.c static int isp1760_hc_setup(struct usb_hcd *hcd) · line 452

Quoted 10 lines
drivers/usb/host/isp1760-hcd.c +10 -0
+	 * PORT 1 Control register of the ISP1760 is the OTG control
+	 * register on ISP1761.
+	 */
+	scratch = isp1760_readl(hcd->regs + HC_CHIP_ID_REG);
+	if (((scratch & 0xffff) == 0x1760) &&
+	    !(priv->devflags & ISP1760_FLAG_PORT1_DIS)) {
+		isp1760_writel(PORT1_POWER | PORT1_INIT2,
+			       hcd->regs + HC_PORT1_CTRL);
+		mdelay(10);
+	}

I'm sorry, you can't solve this way. My ISP1760 claims to be an ISP1761 this way :) So I end up with one functional port... The only way to distinguish between 1760 & 1761 would be at the probing level.

Sebastian

author
#3 lore raw
Quoted 2 lines · Sebastian Siewior

If you don't mind, I would prefer having ISP1760_FLAG_BUS_WIDTH_32 and or conditionally HW_DATA_BUS_32BIT.

It doesn't matter much to me -- I just reasoned that a 32-bit bus width was the norm since it's the hardware default and so having a flags=0 would be desirable for that case. Did you have something else in mind?

Quoted 2 lines

Why do you have to read the chip id here? Is this a dummy read to ensure something?

The idea is to force the data bus lines to change states to protect against a false success. E.g., you read back 0xdeadbabe but it was due to the line states from the previous write rather than coming from the chip itself (I've seen this when you get the bus chip select or timing configuration wrong).

Quoted 3 lines

I'm sorry, you can't solve this way. My ISP1760 claims to be an ISP1761 this way :) So I end up with one functional port... The only way to distinguish between 1760 & 1761 would be at the probing level.

Yikes -- you're right. I would have never guessed that the 1760 would have returned 0x1761, but sure enough that's what the datasheet says.

Thanks for the feedback.

--
Nate Case <[email protected]>