Skip to contentSkip to conversation

bc47xx : fix ssb irq setup

PATCH Applied matthieu castet 2 participants 3 messages Jun 30, 2009

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

PATCH 1/1 bc47xx : fix ssb irq setup

the current ssb irq setup (in ssb_mipscore_init) have some problem :
it configure some device on some irq without checking that the irq is not taken by an other device.

For example in my case PCI host is on irq 0 and IPSEC on irq 3. The current code :
- store in dev->irq that IPSEC irq is 3+2
- do a set_irq 0->3 on PCI host

But now IPSEC irq is not routed anymore to the mips code and dev->irq is wrong. This cause problem described in [1].

This patch try to solve the problem by making set_irq configure the device we want to take the irq on the shared irq0. The previous example become :
- store in dev->irq that IPSEC irq is 3+2
- do a set_irq 0->3 on PCI host :
- irq 3 is already taken by IPSEC. do a set_irq 3->0 on IPSEC

I also added some code to print the irq configuration after irq setup to allow easier debugging. And I add extra checking in ssb_mips_irq to report device without irq or device with not routed irq.

[1] http://www.danm.de/files/src/bcm5365p/REPORTED_DEVICES

drivers/ssb/driver_mipscore.c +0 -0
Index: linux-2.6/drivers/ssb/driver_mipscore.c
===================================================================
@@ -49,29 +49,54 @@
 

static inline u32 ssb_irqflag(struct ssb_device *dev)
{
- return ssb_read32(dev, SSB_TPSFLAG) & SSB_TPSFLAG_BPFLAG;
+ u32 tpsflag = ssb_read32(dev, SSB_TPSFLAG);
+ if (tpsflag)
+ return ssb_read32(dev, SSB_TPSFLAG) & SSB_TPSFLAG_BPFLAG;
+ else
+ /* not irq supported */
+ return 0x3f;
+}
+
+static struct ssb_device *find_device(struct ssb_device *rdev, int irqflag) +{
+ struct ssb_bus *bus = rdev->bus;
+ int i;
+ for (i = 0; i < bus->nr_devices; i++) {
+ struct ssb_device *dev;
+ dev = &(bus->devices[i]);
+ if (ssb_irqflag(dev) == irqflag)
+ return dev;
+ }
+ return NULL;
}

/* Get the MIPS IRQ assignment for a specified device.
* If unassigned, 0 is returned.
+ * If disabled, 5 is returned.
+ * If not supported, 6 is returned.
*/
unsigned int ssb_mips_irq(struct ssb_device *dev)

{
	struct ssb_bus *bus = dev->bus;
+	struct ssb_device *mdev = bus->mipscore.dev;
	u32 irqflag;
	u32 ipsflag;
	u32 tmp;
	unsigned int irq;

	irqflag = ssb_irqflag(dev);
+	if (irqflag == 0x3f)
+		return 6;
	ipsflag = ssb_read32(bus->mipscore.dev, SSB_IPSFLAG);
	for (irq = 1; irq <= 4; irq++) {
		tmp = ((ipsflag & ipsflag_irq_mask[irq]) >> ipsflag_irq_shift[irq]);
		if (tmp == irqflag)
			break;
	}

- if (irq == 5)
- irq = 0;
+ if (irq == 5) {
+ if ((1 << irqflag) & ssb_read32(mdev, SSB_INTVEC))
+ irq = 0;
+ }

	return irq;
}
@@ -97,25 +122,56 @@
	struct ssb_device *mdev = bus->mipscore.dev;
	u32 irqflag = ssb_irqflag(dev);

+ BUG_ON(oldirq == 6);
+

	dev->irq = irq + 2;

- ssb_dprintk(KERN_INFO PFX
- "set_irq: core 0x%04x, irq %d => %d\n",

-		    dev->id.coreid, oldirq, irq);
	/* clear the old irq */
	if (oldirq == 0)
		ssb_write32(mdev, SSB_INTVEC, (~(1 << irqflag) & ssb_read32(mdev, SSB_INTVEC)));
-	else
+	else if (oldirq != 5)
		clear_irq(bus, oldirq);

	/* assign the new one */
	if (irq == 0) {
		ssb_write32(mdev, SSB_INTVEC, ((1 << irqflag) | ssb_read32(mdev, SSB_INTVEC)));
	} else {

+ u32 ipsflag = ssb_read32(mdev, SSB_IPSFLAG);
+ if ((ipsflag & ipsflag_irq_mask[irq]) != ipsflag_irq_mask[irq]) {
+ u32 oldipsflag = (ipsflag & ipsflag_irq_mask[irq]) >> ipsflag_irq_shift[irq];
+ struct ssb_device *olddev = find_device(dev, oldipsflag);
+ if (olddev)
+ set_irq(olddev, 0);
+ }

		irqflag <<= ipsflag_irq_shift[irq];
-		irqflag |= (ssb_read32(mdev, SSB_IPSFLAG) & ~ipsflag_irq_mask[irq]);
+		irqflag |= (ipsflag & ~ipsflag_irq_mask[irq]);
		ssb_write32(mdev, SSB_IPSFLAG, irqflag);
	}

+ ssb_dprintk(KERN_INFO PFX
+ "set_irq: core 0x%04x, irq %d => %d\n",
+ dev->id.coreid, oldirq+2, irq+2);
+}
+
+static void print_irq(struct ssb_device *dev, unsigned int irq) +{
+ int i;
+ static const char *irq_name[] = {"2(S)", "3", "4", "5", "6", "D", "I"};
+ ssb_dprintk(KERN_INFO PFX
+ "core 0x%04x, irq :", dev->id.coreid);
+ for (i = 0; i <= 6; i++) {
+ ssb_dprintk(" %s%s", irq_name[i], i==irq?"*":" ");
+ }
+ ssb_dprintk("\n");
+}
+
+static void dump_irq(struct ssb_bus *bus)
+{
+ int i;
+ for (i = 0; i < bus->nr_devices; i++) {
+ struct ssb_device *dev;
+ dev = &(bus->devices[i]);
+ print_irq(dev, ssb_mips_irq(dev));
+ }
}

static void ssb_mips_serial_init(struct ssb_mipscore *mcore) @@ -197,16 +253,23 @@

	/* Assign IRQs to all cores on the bus, start with irq line 2, because serial usually takes 1 */
	for (irq = 2, i = 0; i < bus->nr_devices; i++) {
+		int mips_irq;
		dev = &(bus->devices[i]);

- dev->irq = ssb_mips_irq(dev) + 2;
+ mips_irq = ssb_mips_irq(dev);
+ if (mips_irq > 4)
+ dev->irq = 0;
+ else
+ dev->irq = mips_irq + 2;
+ if (dev->irq > 5)

+			continue;
		switch (dev->id.coreid) {
		case SSB_DEV_USB11_HOST:
			/* shouldn't need a separate irq line for non-4710, most of them have a proper
			 * external usb controller on the pci */
			if ((bus->chip_id == 0x4710) && (irq <= 4)) {
				set_irq(dev, irq++);
-				break;
			}
+			break;
			/* fallthrough */
		case SSB_DEV_PCI:
		case SSB_DEV_ETHERNET:
@@ -220,6 +283,8 @@
			}
		}
	}

+ ssb_dprintk(KERN_INFO PFX "after irq reconfiguration\n");
+ dump_irq(bus);

	ssb_mips_serial_init(mcore);
	ssb_mips_flash_detect(mcore);
author Acked-by Michael Buesch Tested-by Florian Fainelli on v1 1/1
#1 lore raw

Update : the previous patch was broken by my mailler

the current ssb irq setup (in ssb_mipscore_init) have some problem :
it configure some device on some irq without checking that the irq is not taken by an other device.

For example in my case PCI host is on irq 0 and IPSEC on irq 3. The current code :
- store in dev->irq that IPSEC irq is 3+2
- do a set_irq 0->3 on PCI host

But now IPSEC irq is not routed anymore to the mips code and dev->irq is wrong. This cause problem described in [1].

This patch try to solve the problem by making set_irq configure the device we want to take the irq on the shared irq0. The previous example become :
- store in dev->irq that IPSEC irq is 3+2
- do a set_irq 0->3 on PCI host :
- irq 3 is already taken by IPSEC. do a set_irq 3->0 on IPSEC

I also added some code to print the irq configuration after irq setup to allow easier debugging. And I add extra checking in ssb_mips_irq to report device without irq or device with not routed irq.

[1] http://www.danm.de/files/src/bcm5365p/REPORTED_DEVICES

drivers/ssb/driver_mipscore.c +0 -0
Index: linux-2.6/drivers/ssb/driver_mipscore.c
===================================================================
@@ -49,29 +49,54 @@
 

static inline u32 ssb_irqflag(struct ssb_device *dev)
{
- return ssb_read32(dev, SSB_TPSFLAG) & SSB_TPSFLAG_BPFLAG;
+ u32 tpsflag = ssb_read32(dev, SSB_TPSFLAG);
+ if (tpsflag)
+ return ssb_read32(dev, SSB_TPSFLAG) & SSB_TPSFLAG_BPFLAG;
+ else
+ /* not irq supported */
+ return 0x3f;
+}
+
+static struct ssb_device *find_device(struct ssb_device *rdev, int irqflag) +{
+ struct ssb_bus *bus = rdev->bus;
+ int i;
+ for (i = 0; i < bus->nr_devices; i++) {
+ struct ssb_device *dev;
+ dev = &(bus->devices[i]);
+ if (ssb_irqflag(dev) == irqflag)
+ return dev;
+ }
+ return NULL;
}

/* Get the MIPS IRQ assignment for a specified device.
* If unassigned, 0 is returned.
+ * If disabled, 5 is returned.
+ * If not supported, 6 is returned.
*/
unsigned int ssb_mips_irq(struct ssb_device *dev)

{
	struct ssb_bus *bus = dev->bus;
+	struct ssb_device *mdev = bus->mipscore.dev;
	u32 irqflag;
	u32 ipsflag;
	u32 tmp;
	unsigned int irq;

	irqflag = ssb_irqflag(dev);
+	if (irqflag == 0x3f)
+		return 6;
	ipsflag = ssb_read32(bus->mipscore.dev, SSB_IPSFLAG);
	for (irq = 1; irq <= 4; irq++) {
		tmp = ((ipsflag & ipsflag_irq_mask[irq]) >> ipsflag_irq_shift[irq]);
		if (tmp == irqflag)
			break;
	}

- if (irq == 5)
- irq = 0;
+ if (irq == 5) {
+ if ((1 << irqflag) & ssb_read32(mdev, SSB_INTVEC))
+ irq = 0;
+ }

	return irq;
}
@@ -97,25 +122,56 @@
	struct ssb_device *mdev = bus->mipscore.dev;
	u32 irqflag = ssb_irqflag(dev);

+ BUG_ON(oldirq == 6);
+

	dev->irq = irq + 2;

- ssb_dprintk(KERN_INFO PFX
- "set_irq: core 0x%04x, irq %d => %d\n",

-		    dev->id.coreid, oldirq, irq);
	/* clear the old irq */
	if (oldirq == 0)
		ssb_write32(mdev, SSB_INTVEC, (~(1 << irqflag) & ssb_read32(mdev, SSB_INTVEC)));
-	else
+	else if (oldirq != 5)
		clear_irq(bus, oldirq);

	/* assign the new one */
	if (irq == 0) {
		ssb_write32(mdev, SSB_INTVEC, ((1 << irqflag) | ssb_read32(mdev, SSB_INTVEC)));
	} else {

+ u32 ipsflag = ssb_read32(mdev, SSB_IPSFLAG);
+ if ((ipsflag & ipsflag_irq_mask[irq]) != ipsflag_irq_mask[irq]) {
+ u32 oldipsflag = (ipsflag & ipsflag_irq_mask[irq]) >> ipsflag_irq_shift[irq];
+ struct ssb_device *olddev = find_device(dev, oldipsflag);
+ if (olddev)
+ set_irq(olddev, 0);
+ }

		irqflag <<= ipsflag_irq_shift[irq];
-		irqflag |= (ssb_read32(mdev, SSB_IPSFLAG) & ~ipsflag_irq_mask[irq]);
+		irqflag |= (ipsflag & ~ipsflag_irq_mask[irq]);
		ssb_write32(mdev, SSB_IPSFLAG, irqflag);
	}

+ ssb_dprintk(KERN_INFO PFX
+ "set_irq: core 0x%04x, irq %d => %d\n",
+ dev->id.coreid, oldirq+2, irq+2);
+}
+
+static void print_irq(struct ssb_device *dev, unsigned int irq) +{
+ int i;
+ static const char *irq_name[] = {"2(S)", "3", "4", "5", "6", "D", "I"};
+ ssb_dprintk(KERN_INFO PFX
+ "core 0x%04x, irq :", dev->id.coreid);
+ for (i = 0; i <= 6; i++) {
+ ssb_dprintk(" %s%s", irq_name[i], i==irq?"*":" ");
+ }
+ ssb_dprintk("\n");
+}
+
+static void dump_irq(struct ssb_bus *bus)
+{
+ int i;
+ for (i = 0; i < bus->nr_devices; i++) {
+ struct ssb_device *dev;
+ dev = &(bus->devices[i]);
+ print_irq(dev, ssb_mips_irq(dev));
+ }
}

static void ssb_mips_serial_init(struct ssb_mipscore *mcore) @@ -197,16 +253,23 @@

	/* Assign IRQs to all cores on the bus, start with irq line 2, because serial usually takes 1 */
	for (irq = 2, i = 0; i < bus->nr_devices; i++) {
+		int mips_irq;
		dev = &(bus->devices[i]);

- dev->irq = ssb_mips_irq(dev) + 2;
+ mips_irq = ssb_mips_irq(dev);
+ if (mips_irq > 4)
+ dev->irq = 0;
+ else
+ dev->irq = mips_irq + 2;
+ if (dev->irq > 5)

+			continue;
		switch (dev->id.coreid) {
		case SSB_DEV_USB11_HOST:
			/* shouldn't need a separate irq line for non-4710, most of them have a proper
			 * external usb controller on the pci */
			if ((bus->chip_id == 0x4710) && (irq <= 4)) {
				set_irq(dev, irq++);
-				break;
			}
+			break;
			/* fallthrough */
		case SSB_DEV_PCI:
		case SSB_DEV_ETHERNET:
@@ -220,6 +283,8 @@
			}
		}
	}

+ ssb_dprintk(KERN_INFO PFX "after irq reconfiguration\n");
+ dump_irq(bus);

	ssb_mips_serial_init(mcore);
	ssb_mips_flash_detect(mcore);
Applied
#2 lore raw
Quoted 1 line · matthieu castet

Update : the previous patch was broken by my mailler

I can confirm that :-)

Applied. Thanks,

Ralf