ohci1394 PCI fixup for Toshiba laptops

PATCH In review Jesse Barnes 3 participants 5 messages Oct 25, 2005

Share & tools

Jesse Barnes

author

2005-10-25 01:58 UTC #0 lore raw

PATCH 1/1 ohci1394 PCI fixup for Toshiba laptops

This is a fix for a bug I see on my Toshiba laptop, where the ohci1394 controller gets initialized improperly. The patch adds two PCI fixups to arch/i386/pci/fixup.c, one that happens early on to cache the value of the PCI_CACHE_LINE_SIZE config register, and another that later restores the value, along with a valid IRQ number and some BAR values. I've tested it on my laptop, and it prevents me from running into what I consider to be a major bug: IRQ 11 is disabled by the IRQ debug code, causing my wireless to break.

Thanks to Rob for the original patch to ohci1394.c and Stefan for lots of proofreading and additional information collection. I think the DMI system list is correct, but we may need to add some more PCI IDs to the PCI_FIXUP macros over time.

Thanks,
Jesse

Signed-off-by Jesse Barnes <[email protected]>

arch/i386/pci/fixup.c +59 -0

diff -Naur -X linux-2.6.14-rc5/Documentation/dontdiff linux-2.6.14-rc5.orig/arch/i386/pci/fixup.c linux-2.6.14-rc5/arch/i386/pci/fixup.c
@@ -2,6 +2,8 @@
  * Exceptions for specific devices. Usually work-arounds for fatal design flaws.
  */
 
+#include <linux/delay.h>
+#include <linux/dmi.h>
 #include <linux/pci.h>
 #include <linux/init.h>
 #include "pci.h"

@@ -384,3 +386,60 @@
 	}
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pci_fixup_video);
+
+/*
+ * Some Toshiba laptops need extra code to enable their TI TSB43AB22/A.
+ *
+ * We pretend to bring them out of full D3 state, and restore the proper
+ * IRQ, PCI cache line size, and BARs, otherwise the device won't function
+ * properly.  In some cases, the device will generate an interrupt on
+ * the wrong IRQ line, causing any devices sharing the the line it's
+ * *supposed* to use to be disabled by the kernel's IRQ debug code.
+ */
+static u16 toshiba_line_size;
+
+static struct dmi_system_id __devinit toshiba_ohci1394_dmi_table[] = {
+	{
+		.ident = "Toshiba PS5 based laptop",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_VERSION, "PS5"),
+		},
+	},
+	{
+		.ident = "Toshiba PSM4 based laptop",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_VERSION, "PSM4"),
+		},
+	},
+	{ }
+};
+
+static void __devinit pci_pre_fixup_toshiba_ohci1394(struct pci_dev *dev)
+{
+	if (!dmi_check_system(toshiba_ohci1394_dmi_table))
+		return; /* only applies to certain Toshibas (so far) */
+
+	dev->current_state = PCI_D3cold;
+	pci_read_config_word(dev, PCI_CACHE_LINE_SIZE, &toshiba_line_size);
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TI, 0x8032,
+			 pci_pre_fixup_toshiba_ohci1394);
+
+static void __devinit pci_post_fixup_toshiba_ohci1394(struct pci_dev *dev)
+{
+	if (dmi_check_system(toshiba_ohci1394_dmi_table))
+		return; /* only applies to certain Toshibas (so far) */
+
+	/* Restore config space on Toshiba laptops */
+	mdelay(10);
+	pci_write_config_word(dev, PCI_CACHE_LINE_SIZE, toshiba_line_size);
+	pci_write_config_word(dev, PCI_INTERRUPT_LINE, dev->irq);
+	pci_write_config_dword(dev, PCI_BASE_ADDRESS_0,
+			       pci_resource_start(dev, 0));
+	pci_write_config_dword(dev, PCI_BASE_ADDRESS_1,
+			       pci_resource_start(dev, 1));
+}
+DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_TI, 0x8032,
+			 pci_post_fixup_toshiba_ohci1394);

Stefan Richter

2005-10-25 07:02 UTC #1 lore raw

arch/i386/pci/fixup.c line 437

Quoted 8 lines · Jesse Barnes

arch/i386/pci/fixup.c +8 -0

+static void __devinit pci_post_fixup_toshiba_ohci1394(struct pci_dev *dev)
+{
+	if (dmi_check_system(toshiba_ohci1394_dmi_table))
+		return; /* only applies to certain Toshibas (so far) */
+
+	/* Restore config space on Toshiba laptops */
+	mdelay(10);
+	pci_write_config_word(dev, PCI_CACHE_LINE_SIZE, toshiba_line_size);

Shouldn't this read

        if (!dmi_check_system(toshiba_ohci1394_dmi_table))
                return;
            ^ ?

dmi_check_system returns the number of matches.

Signature · 3 lines

Stefan Richter
-=====-=-=-= =-=- ==--=
http://arcgraph.de/sr/

Jesse Barnes

author

2005-10-25 17:19 UTC #2 lore raw

Quoted 13 lines · Stefan Richter

Quoted 9 lines · Jesse Barnes
+static void __devinit pci_post_fixup_toshiba_ohci1394(struct pci_dev *dev) +{
(quoted diff) +6 -0
+	if (dmi_check_system(toshiba_ohci1394_dmi_table))
+		return; /* only applies to certain Toshibas (so far) */
+
+	/* Restore config space on Toshiba laptops */
+	mdelay(10);
+	pci_write_config_word(dev, PCI_CACHE_LINE_SIZE,
toshiba_line_size);
Shouldn't this read

Quoted 5 lines

         if (!dmi_check_system(toshiba_ohci1394_dmi_table))
                 return;
             ^ ?

dmi_check_system returns the number of matches.

Doh! Yes, it should be (I tested an earlier version, then decided to reverse the logic and return, oops).

Here's an updated version.

Signed-off-by Jesse Barnes <[email protected]>

arch/i386/pci/fixup.c +59 -0

diff -Naur -X linux-2.6.14-rc5/Documentation/dontdiff linux-2.6.14-rc5.orig/arch/i386/pci/fixup.c linux-2.6.14-rc5/arch/i386/pci/fixup.c
@@ -2,6 +2,8 @@
  * Exceptions for specific devices. Usually work-arounds for fatal design flaws.
  */
 
+#include <linux/delay.h>
+#include <linux/dmi.h>
 #include <linux/pci.h>
 #include <linux/init.h>
 #include "pci.h"

@@ -384,3 +386,60 @@
 	}
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pci_fixup_video);
+
+/*
+ * Some Toshiba laptops need extra code to enable their TI TSB43AB22/A.
+ *
+ * We pretend to bring them out of full D3 state, and restore the proper
+ * IRQ, PCI cache line size, and BARs, otherwise the device won't function
+ * properly.  In some cases, the device will generate an interrupt on
+ * the wrong IRQ line, causing any devices sharing the the line it's
+ * *supposed* to use to be disabled by the kernel's IRQ debug code.
+ */
+static u16 toshiba_line_size;
+
+static struct dmi_system_id __devinit toshiba_ohci1394_dmi_table[] = {
+	{
+		.ident = "Toshiba PS5 based laptop",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_VERSION, "PS5"),
+		},
+	},
+	{
+		.ident = "Toshiba PSM4 based laptop",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_VERSION, "PSM4"),
+		},
+	},
+	{ }
+};
+
+static void __devinit pci_pre_fixup_toshiba_ohci1394(struct pci_dev *dev)
+{
+	if (!dmi_check_system(toshiba_ohci1394_dmi_table))
+		return; /* only applies to certain Toshibas (so far) */
+
+	dev->current_state = PCI_D3cold;
+	pci_read_config_word(dev, PCI_CACHE_LINE_SIZE, &toshiba_line_size);
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TI, 0x8032,
+			 pci_pre_fixup_toshiba_ohci1394);
+
+static void __devinit pci_post_fixup_toshiba_ohci1394(struct pci_dev *dev)
+{
+	if (!dmi_check_system(toshiba_ohci1394_dmi_table))
+		return; /* only applies to certain Toshibas (so far) */
+
+	/* Restore config space on Toshiba laptops */
+	mdelay(10);
+	pci_write_config_word(dev, PCI_CACHE_LINE_SIZE, toshiba_line_size);
+	pci_write_config_word(dev, PCI_INTERRUPT_LINE, dev->irq);
+	pci_write_config_dword(dev, PCI_BASE_ADDRESS_0,
+			       pci_resource_start(dev, 0));
+	pci_write_config_dword(dev, PCI_BASE_ADDRESS_1,
+			       pci_resource_start(dev, 1));
+}
+DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_TI, 0x8032,
+			 pci_post_fixup_toshiba_ohci1394);

Greg KH

2005-10-25 17:23 UTC #3 lore raw

Quoted 18 lines · Jesse Barnes

Quoted 17 lines · Stefan Richter
Quoted 9 lines · Jesse Barnes
+static void __devinit pci_post_fixup_toshiba_ohci1394(struct pci_dev *dev) +{
arch/i386/pci/fixup.c +6 -0
+	if (dmi_check_system(toshiba_ohci1394_dmi_table))
+		return; /* only applies to certain Toshibas (so far) */
+
+	/* Restore config space on Toshiba laptops */
+	mdelay(10);
+	pci_write_config_word(dev, PCI_CACHE_LINE_SIZE,
toshiba_line_size);
Shouldn't this read
         if (!dmi_check_system(toshiba_ohci1394_dmi_table))
                 return;
             ^ ?

Quoted 8 lines

Quoted 1 line
dmi_check_system returns the number of matches.
Doh! Yes, it should be (I tested an earlier version, then decided to reverse the logic and return, oops).
Here's an updated version.
Signed-off-by Jesse Barnes <[email protected]>

Care to resend with the full changelog information too, so I can apply it?

thanks,

greg k-h

Jesse Barnes

author

2005-10-25 17:28 UTC #4 lore raw

Quoted 2 lines · Greg KH

Care to resend with the full changelog information too, so I can apply it?

Thanks to Rob for the original patch to ohci1394.c and Stefan for lots of proofreading (and a last minute bug caught in review!) and additional information collection. I think the DMI system list is correct, but we may need to add some more PCI IDs to the PCI_FIXUP macros over time.

Thanks,
Jesse

Signed-off-by Jesse Barnes <[email protected]>

arch/i386/pci/fixup.c +59 -0

diff -Naur -X linux-2.6.14-rc5/Documentation/dontdiff linux-2.6.14-rc5.orig/arch/i386/pci/fixup.c linux-2.6.14-rc5/arch/i386/pci/fixup.c
@@ -2,6 +2,8 @@
  * Exceptions for specific devices. Usually work-arounds for fatal design flaws.
  */
 
+#include <linux/delay.h>
+#include <linux/dmi.h>
 #include <linux/pci.h>
 #include <linux/init.h>
 #include "pci.h"

@@ -384,3 +386,60 @@
 	}
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pci_fixup_video);
+
+/*
+ * Some Toshiba laptops need extra code to enable their TI TSB43AB22/A.
+ *
+ * We pretend to bring them out of full D3 state, and restore the proper
+ * IRQ, PCI cache line size, and BARs, otherwise the device won't function
+ * properly.  In some cases, the device will generate an interrupt on
+ * the wrong IRQ line, causing any devices sharing the the line it's
+ * *supposed* to use to be disabled by the kernel's IRQ debug code.
+ */
+static u16 toshiba_line_size;
+
+static struct dmi_system_id __devinit toshiba_ohci1394_dmi_table[] = {
+	{
+		.ident = "Toshiba PS5 based laptop",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_VERSION, "PS5"),
+		},
+	},
+	{
+		.ident = "Toshiba PSM4 based laptop",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_VERSION, "PSM4"),
+		},
+	},
+	{ }
+};
+
+static void __devinit pci_pre_fixup_toshiba_ohci1394(struct pci_dev *dev)
+{
+	if (!dmi_check_system(toshiba_ohci1394_dmi_table))
+		return; /* only applies to certain Toshibas (so far) */
+
+	dev->current_state = PCI_D3cold;
+	pci_read_config_word(dev, PCI_CACHE_LINE_SIZE, &toshiba_line_size);
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_TI, 0x8032,
+			 pci_pre_fixup_toshiba_ohci1394);
+
+static void __devinit pci_post_fixup_toshiba_ohci1394(struct pci_dev *dev)
+{
+	if (!dmi_check_system(toshiba_ohci1394_dmi_table))
+		return; /* only applies to certain Toshibas (so far) */
+
+	/* Restore config space on Toshiba laptops */
+	mdelay(10);
+	pci_write_config_word(dev, PCI_CACHE_LINE_SIZE, toshiba_line_size);
+	pci_write_config_word(dev, PCI_INTERRUPT_LINE, dev->irq);
+	pci_write_config_dword(dev, PCI_BASE_ADDRESS_0,
+			       pci_resource_start(dev, 0));
+	pci_write_config_dword(dev, PCI_BASE_ADDRESS_1,
+			       pci_resource_start(dev, 1));
+}
+DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_TI, 0x8032,
+			 pci_post_fixup_toshiba_ohci1394);