ohci1394 unhandled interrupts bug in 2.6.14-rc2

ohci1394 unhandled interrupts bug in 2.6.14-rc2

[jbarnes]

I've been experiencing a bug in the Fedora kernels for awhile involving
the ohci1394 driver.  If I include the driver in my initrd (causing it
to be loaded at boot time), the IRQ corresponding to my TI OHCI firewire
controller is disabled (message says "Disabling IRQ #11", I think due to
the kernel irq debug code noticing that it hasn't been handled too many
times).  Since this IRQ is shared with my pcmcia, wireless, and usb
devices (don't ask me why it's wired this way, this is a Toshiba
Satellite laptop), nothing important works after the IRQ is disabled.

I've reproduced this problem under 2.6.14-rc2, which includes Al Viro's
latest fix to initialize interrupt handler spinlock (which I was hoping
would fix the problem but didn't).

Is this a known issue?  Does the interrupt handler need to special case
initialization somehow and return IRQ_HANDLED even if there's no event
sometimes?

Thanks,
Jesse

Re: ohci1394 unhandled interrupts bug in 2.6.14-rc2

[Stefan Richter]

jbarnes@virtuousgeek.org wrote:
> I've been experiencing a bug in the Fedora kernels for awhile involving
> the ohci1394 driver.  If I include the driver in my initrd (causing it
> to be loaded at boot time), the IRQ corresponding to my TI OHCI firewire
> controller is disabled
...
> this is a Toshiba Satellite laptop)

Somebody mentioned this Linux-on-Toshiba-Satellite page recently on 
linux1394-user: http://www.janerob.com/rob/ts5100/index.shtml
The patch available from there was briefly discussed in February:
http://marc.theaimsgroup.com/?l=linux1394-devel&t=110786507900006

Does this patch correct the problem on your machine?
-- 
Stefan Richter
-=====-=-=-= =-=- -====
http://arcgraph.de/sr/

Re: ohci1394 unhandled interrupts bug in 2.6.14-rc2

[Jesse Barnes]

On Sat, Oct 15, 2005 at 09:39:06PM +0200, Stefan Richter wrote:
> Somebody mentioned this Linux-on-Toshiba-Satellite page recently on 
> linux1394-user: http://www.janerob.com/rob/ts5100/index.shtml
> The patch available from there was briefly discussed in February:
> http://marc.theaimsgroup.com/?l=linux1394-devel&t=110786507900006
> 
> Does this patch correct the problem on your machine?

Yes, it seems to help.  If I boot up and modprobe the driver with
toshiba=1, everything looks fine (I have no firewire devices to test
with).  If I modprobe it with toshiba=0, the system gets sluggish for a
second then IRQ 11 is disabled.  I had to update the patch though, as
shown below.

I'm not sure if the fix is proper though, maybe this should be handled
as a PCI quirk of this Toshiba board instead?  Either way, some kind of
fix should make it in soon, ideally to 2.6.14 or 2.6.14.1.

Thanks,
Jesse

diff -X linux-2.6.14-rc2/Documentation/dontdiff -Naur linux-2.6.14-rc2.orig/drivers/ieee1394/ohci1394.c linux-2.6.14-rc2/drivers/ieee1394/ohci1394.c
--- linux-2.6.14-rc2.orig/drivers/ieee1394/ohci1394.c	2005-09-19 20:00:41.000000000 -0700
+++ linux-2.6.14-rc2/drivers/ieee1394/ohci1394.c	2005-10-15 12:55:08.000000000 -0700
@@ -169,6 +169,10 @@
 module_param(phys_dma, int, 0644);
 MODULE_PARM_DESC(phys_dma, "Enable physical dma (default = 1).");
 
+static int toshiba __initdata = 0;
+module_param(toshiba, bool, 0);
+MODULE_PARM_DESC(toshiba, "Toshiba Legacy-Free BIOS workaround (default=0).");
+
 static void dma_trm_tasklet(unsigned long data);
 static void dma_trm_reset(struct dma_trm_ctx *d);
 
@@ -3222,14 +3226,28 @@
 	struct hpsb_host *host;
 	struct ti_ohci *ohci;	/* shortcut to currently handled device */
 	unsigned long ohci_base;
+	u16  toshiba_data;
 
 	if (version_printed++ == 0)
 		PRINT_G(KERN_INFO, "%s", version);
 
+	if (toshiba) {
+		dev->current_state = 4;
+		pci_read_config_word(dev, PCI_CACHE_LINE_SIZE, &toshiba_data);
+	}
+
         if (pci_enable_device(dev))
 		FAIL(-ENXIO, "Failed to enable OHCI hardware");
         pci_set_master(dev);
 
+	if (toshiba) {
+		mdelay(10);
+		pci_write_config_word(dev, PCI_CACHE_LINE_SIZE, toshiba_data);
+		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));
+ 	}
+
 	host = hpsb_alloc_host(&ohci1394_driver, sizeof(struct ti_ohci), &dev->dev);
 	if (!host) FAIL(-ENOMEM, "Failed to allocate host structure");
 

Re: new PCI quirk for Toshiba Satellite?

[Jesse Barnes]

On Sat, Oct 15, 2005 at 01:29:44PM -0700, Jesse Barnes wrote:
> On Sat, Oct 15, 2005 at 09:39:06PM +0200, Stefan Richter wrote:
> > Somebody mentioned this Linux-on-Toshiba-Satellite page recently on 
> > linux1394-user: http://www.janerob.com/rob/ts5100/index.shtml
> > The patch available from there was briefly discussed in February:
> > http://marc.theaimsgroup.com/?l=linux1394-devel&t=110786507900006
> > 
> > Does this patch correct the problem on your machine?
> 
> Yes, it seems to help.  If I boot up and modprobe the driver with
> toshiba=1, everything looks fine (I have no firewire devices to test
> with).  If I modprobe it with toshiba=0, the system gets sluggish for a
> second then IRQ 11 is disabled.  I had to update the patch though, as
> shown below.
> 
> I'm not sure if the fix is proper though, maybe this should be handled
> as a PCI quirk of this Toshiba board instead?  Either way, some kind of
> fix should make it in soon, ideally to 2.6.14 or 2.6.14.1.

[Forwarding on to the PCI maintainers.]

It seems that the PCI config space isn't programmed correctly on these
machines for some reason, so the fix below allows my OHCI device to work
if I pass 'toshiba=1'.  This seems like something that belongs in the
PCI layer instead though, doesn't it?

Thanks,
Jesse

> diff -X linux-2.6.14-rc2/Documentation/dontdiff -Naur linux-2.6.14-rc2.orig/drivers/ieee1394/ohci1394.c linux-2.6.14-rc2/drivers/ieee1394/ohci1394.c
> --- linux-2.6.14-rc2.orig/drivers/ieee1394/ohci1394.c	2005-09-19 20:00:41.000000000 -0700
> +++ linux-2.6.14-rc2/drivers/ieee1394/ohci1394.c	2005-10-15 12:55:08.000000000 -0700
> @@ -169,6 +169,10 @@
>  module_param(phys_dma, int, 0644);
>  MODULE_PARM_DESC(phys_dma, "Enable physical dma (default = 1).");
>  
> +static int toshiba __initdata = 0;
> +module_param(toshiba, bool, 0);
> +MODULE_PARM_DESC(toshiba, "Toshiba Legacy-Free BIOS workaround (default=0).");
> +
>  static void dma_trm_tasklet(unsigned long data);
>  static void dma_trm_reset(struct dma_trm_ctx *d);
>  
> @@ -3222,14 +3226,28 @@
>  	struct hpsb_host *host;
>  	struct ti_ohci *ohci;	/* shortcut to currently handled device */
>  	unsigned long ohci_base;
> +	u16  toshiba_data;
>  
>  	if (version_printed++ == 0)
>  		PRINT_G(KERN_INFO, "%s", version);
>  
> +	if (toshiba) {
> +		dev->current_state = 4;
> +		pci_read_config_word(dev, PCI_CACHE_LINE_SIZE, &toshiba_data);
> +	}
> +
>          if (pci_enable_device(dev))
>  		FAIL(-ENXIO, "Failed to enable OHCI hardware");
>          pci_set_master(dev);
>  
> +	if (toshiba) {
> +		mdelay(10);
> +		pci_write_config_word(dev, PCI_CACHE_LINE_SIZE, toshiba_data);
> +		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));
> + 	}
> +
>  	host = hpsb_alloc_host(&ohci1394_driver, sizeof(struct ti_ohci), &dev->dev);
>  	if (!host) FAIL(-ENOMEM, "Failed to allocate host structure");
>  
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: ohci1394 unhandled interrupts bug in 2.6.14-rc2

[Stefan Richter]

Jesse Barnes wrote:
> On Sat, Oct 15, 2005 at 09:39:06PM +0200, Stefan Richter wrote:
>>Somebody mentioned this Linux-on-Toshiba-Satellite page recently on 
>>linux1394-user: http://www.janerob.com/rob/ts5100/index.shtml
>>The patch available from there was briefly discussed in February:
>>http://marc.theaimsgroup.com/?l=linux1394-devel&t=110786507900006
> 
> Yes, it seems to help.  If I boot up and modprobe the driver with
> toshiba=1, everything looks fine (I have no firewire devices to test
> with).  If I modprobe it with toshiba=0, the system gets sluggish for a
> second then IRQ 11 is disabled.  I had to update the patch though,

What about the PCI_CACHE_LINE_SIZE read/write?

Jody McIntyre wrote on 2005-02-09:
| Can you try the fix without
| pci_write_config_word(dev,PCI_CACHE_LINE_SIZE,toshiba_pcls);
| or pci_read_config_word(dev,PCI_CACHE_LINE_SIZE,&toshiba_pcls);
| and report if it still works?
|
| If it doesn't work, try leaving those lines out but adding
| pci_clear_mwi(dev);
| after the mdelay(), on the off chance that the device thinks mwi is on.
|
| The correct fix for this, if possible, is actually a pci quirk instead
| of the dmi-based approach, but if reading PCI_CACHE_LINE_SIZE before
| pci_enable_device() really is necessary, this will be rather difficult.
[ http://marc.theaimsgroup.com/?l=linux1394-devel&m=110797909807519 ]
-- 
Stefan Richter
-=====-=-=-= =-=- -====
http://arcgraph.de/sr/

Re: ohci1394 unhandled interrupts bug in 2.6.14-rc2

[Jesse Barnes]

On Sat, Oct 15, 2005 at 11:02:48PM +0200, Stefan Richter wrote:
> What about the PCI_CACHE_LINE_SIZE read/write?
> 
> Jody McIntyre wrote on 2005-02-09:
> | Can you try the fix without
> | pci_write_config_word(dev,PCI_CACHE_LINE_SIZE,toshiba_pcls);
> | or pci_read_config_word(dev,PCI_CACHE_LINE_SIZE,&toshiba_pcls);
> | and report if it still works?
> |
> | If it doesn't work, try leaving those lines out but adding
> | pci_clear_mwi(dev);
> | after the mdelay(), on the off chance that the device thinks mwi is on.
> |
> | The correct fix for this, if possible, is actually a pci quirk instead
> | of the dmi-based approach, but if reading PCI_CACHE_LINE_SIZE before
> | pci_enable_device() really is necessary, this will be rather difficult.
> [ http://marc.theaimsgroup.com/?l=linux1394-devel&m=110797909807519 ]

It looks like it is.

I removed the PCI_CACHE_LINE_SIZE read and write, and that didn't work.
I added in a pci_clear_mwi(dev) and that didn't work either.  It looks
like the whole patch that I posted earlier is required.

Thanks,
Jesse

Re: ohci1394 unhandled interrupts bug in 2.6.14-rc2

[Andrew Morton]

Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>
> diff -X linux-2.6.14-rc2/Documentation/dontdiff -Naur linux-2.6.14-rc2.orig/drivers/ieee1394/ohci1394.c linux-2.6.14-rc2/drivers/ieee1394/ohci1394.c
> --- linux-2.6.14-rc2.orig/drivers/ieee1394/ohci1394.c	2005-09-19 20:00:41.000000000 -0700
> +++ linux-2.6.14-rc2/drivers/ieee1394/ohci1394.c	2005-10-15 12:55:08.000000000 -0700
> @@ -169,6 +169,10 @@
>  module_param(phys_dma, int, 0644);
>  MODULE_PARM_DESC(phys_dma, "Enable physical dma (default = 1).");
>  
> +static int toshiba __initdata = 0;
> +module_param(toshiba, bool, 0);
> +MODULE_PARM_DESC(toshiba, "Toshiba Legacy-Free BIOS workaround (default=0).");
> +
>  static void dma_trm_tasklet(unsigned long data);
>  static void dma_trm_reset(struct dma_trm_ctx *d);
>  
> @@ -3222,14 +3226,28 @@
>  	struct hpsb_host *host;
>  	struct ti_ohci *ohci;	/* shortcut to currently handled device */
>  	unsigned long ohci_base;
> +	u16  toshiba_data;
>  
>  	if (version_printed++ == 0)
>  		PRINT_G(KERN_INFO, "%s", version);
>  
> +	if (toshiba) {
> +		dev->current_state = 4;
> +		pci_read_config_word(dev, PCI_CACHE_LINE_SIZE, &toshiba_data);
> +	}
> +
>          if (pci_enable_device(dev))
>  		FAIL(-ENXIO, "Failed to enable OHCI hardware");
>          pci_set_master(dev);
>  
> +	if (toshiba) {
> +		mdelay(10);
> +		pci_write_config_word(dev, PCI_CACHE_LINE_SIZE, toshiba_data);
> +		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));
> + 	}
> +
>  	host = hpsb_alloc_host(&ohci1394_driver, sizeof(struct ti_ohci), &dev->dev);
>  	if (!host) FAIL(-ENOMEM, "Failed to allocate host structure");

It would be really really preferable if we could find some automatic way of
doing this.  Is it possible to use DMI matching, like
arch/i386/kernel/acpi/sleep.c:acpisleep_dmi_table ?

Re: ohci1394 unhandled interrupts bug in 2.6.14-rc2

[Stefan Richter]

Andrew Morton wrote:
> Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
>>diff -X linux-2.6.14-rc2/Documentation/dontdiff -Naur linux-2.6.14-rc2.orig/drivers/ieee1394/ohci1394.c linux-2.6.14-rc2/drivers/ieee1394/ohci1394.c
>>--- linux-2.6.14-rc2.orig/drivers/ieee1394/ohci1394.c	2005-09-19 20:00:41.000000000 -0700
>>+++ linux-2.6.14-rc2/drivers/ieee1394/ohci1394.c	2005-10-15 12:55:08.000000000 -0700
[...]
>>+module_param(toshiba, bool, 0);
>>+MODULE_PARM_DESC(toshiba, "Toshiba Legacy-Free BIOS workaround (default=0).");
[...]
> It would be really really preferable if we could find some automatic way of
> doing this.

I agree.

>  Is it possible to use DMI matching, like
> arch/i386/kernel/acpi/sleep.c:acpisleep_dmi_table ?

Earlier forms of the patch do DMI matching:
http://marc.theaimsgroup.com/?l=linux1394-devel&m=110790513206094
http://www.janerob.com/rob/ts5100/tosh-1394.patch
[short-circuited by if (1) at the second URL]

Of course we don't have a complete picture of which models are affected 
though.
-- 
Stefan Richter
-=====-=-=-= =-=- =---=
http://arcgraph.de/sr/

Re: ohci1394 unhandled interrupts bug in 2.6.14-rc2

[Andrew Morton]

Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>
> Andrew Morton wrote:
> > Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > 
> >>diff -X linux-2.6.14-rc2/Documentation/dontdiff -Naur linux-2.6.14-rc2.orig/drivers/ieee1394/ohci1394.c linux-2.6.14-rc2/drivers/ieee1394/ohci1394.c
> >>--- linux-2.6.14-rc2.orig/drivers/ieee1394/ohci1394.c	2005-09-19 20:00:41.000000000 -0700
> >>+++ linux-2.6.14-rc2/drivers/ieee1394/ohci1394.c	2005-10-15 12:55:08.000000000 -0700
> [...]
> >>+module_param(toshiba, bool, 0);
> >>+MODULE_PARM_DESC(toshiba, "Toshiba Legacy-Free BIOS workaround (default=0).");
> [...]
> > It would be really really preferable if we could find some automatic way of
> > doing this.
> 
> I agree.
> 
> >  Is it possible to use DMI matching, like
> > arch/i386/kernel/acpi/sleep.c:acpisleep_dmi_table ?
> 
> Earlier forms of the patch do DMI matching:
> http://marc.theaimsgroup.com/?l=linux1394-devel&m=110790513206094
> http://www.janerob.com/rob/ts5100/tosh-1394.patch
> [short-circuited by if (1) at the second URL]

Rob, can you finish that patch off and send it?

> Of course we don't have a complete picture of which models are affected 
> though.

I suppose we could do both.  As people are found who need the module
parameter, we grab their DMI strings and add them to the table?

Re: ohci1394 unhandled interrupts bug in 2.6.14-rc2

[Stefan Richter]

Andrew Morton wrote:
> Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>>Of course we don't have a complete picture of which models are affected 
>>though.
> 
> I suppose we could do both.  As people are found who need the module
> parameter, we grab their DMI strings and add them to the table?

Jesse, what DMI_PRODUCT_NAME matches your laptop?
-- 
Stefan Richter
-=====-=-=-= =-=- =---=
http://arcgraph.de/sr/

Re: ohci1394 unhandled interrupts bug in 2.6.14-rc2

[rob]

On Mon, 17 Oct 2005 02:42:19 -0700, Andrew Morton wrote
> Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:

> > Earlier forms of the patch do DMI matching:
> > http://marc.theaimsgroup.com/?l=linux1394-devel&m=110790513206094
> > http://www.janerob.com/rob/ts5100/tosh-1394.patch
> > [short-circuited by if (1) at the second URL]
> 
> Rob, can you finish that patch off and send it?

Sorry, I was advised that this should be correctly handled as a pci-quirk
(Jody McIntyre <scjody@modernduck.com>), and subsequently my laptop
motherboard died so I have no way of taking it further.  The responses I got
indicated that the code works as is for the followiung laptops


> System Vendor: TOSHIBA
> Product Name: S5100-501
> Version: PS510E-00NV7-EN


System Vendor: TOSHIBA
Product Name: S5200-801
Version: PS520E-31P1D-GR

Manufacturer: TOSHIBA
Product Name: Satellite 5200
Version: PS520C-31P0EP

Manufacturer: TOSHIBA
Product Name: Satellite 5205
Version: PS522U-XK00YV

Manufacturer: TOSHIBA
Product Name: S5100-603
Version: PS511E-05328-GR

toshiba satellite 5005-S504

Toshiba Satellite 5105-s607


rob.


--
Open WebMail Project (http://openwebmail.org)

Re: ohci1394 unhandled interrupts bug in 2.6.14-rc2

[Stefan Richter]

rob wrote:
> On Mon, 17 Oct 2005 02:42:19 -0700, Andrew Morton wrote
>>Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
>>>Earlier forms of the patch do DMI matching:
>>>http://marc.theaimsgroup.com/?l=linux1394-devel&m=110790513206094
>>>http://www.janerob.com/rob/ts5100/tosh-1394.patch
>>>[short-circuited by if (1) at the second URL]
>>
>>Rob, can you finish that patch off and send it?
> 
> Sorry, I was advised that this should be correctly handled as a pci-quirk
> (Jody McIntyre <scjody@modernduck.com>),

Since Jesse found that we really need to read & write back the 
PCI_CACHE_LINE_SIZE, I gather the workaround has to stay in ohci1394 
(and should be triggered by dmi_check_system), doesn't it?

> and subsequently my laptop
> motherboard died so I have no way of taking it further.

We should be able to finish it.

> The responses I got
> indicated that the code works as is for the followiung laptops
> 
>>System Vendor: TOSHIBA
>>Product Name: S5100-501
>>Version: PS510E-00NV7-EN
> 
> System Vendor: TOSHIBA
> Product Name: S5200-801
> Version: PS520E-31P1D-GR
> 
> Manufacturer: TOSHIBA
> Product Name: Satellite 5200
> Version: PS520C-31P0EP
> 
> Manufacturer: TOSHIBA
> Product Name: Satellite 5205
> Version: PS522U-XK00YV
> 
> Manufacturer: TOSHIBA
> Product Name: S5100-603
> Version: PS511E-05328-GR
> 
> toshiba satellite 5005-S504
> 
> Toshiba Satellite 5105-s607

Thanks a lot for the survey. Do they all _need_ the patch, or do some of 
them need it and the others are just not harmed by the patch?

Does anybody know a DMI_PRODUCT_NAME of a Libretto L1? Something like 
PAL1060TNMM or PAL1060TNCM?
-- 
Stefan Richter
-=====-=-=-= =-=- =---=
http://arcgraph.de/sr/

Re: ohci1394 unhandled interrupts bug in 2.6.14-rc2

[Jesse Barnes]

On Monday, October 17, 2005 3:03 am, Stefan Richter wrote:
> Andrew Morton wrote:
> > Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> >>Of course we don't have a complete picture of which models are
> >> affected though.
> >
> > I suppose we could do both.  As people are found who need the module
> > parameter, we grab their DMI strings and add them to the table?
>
> Jesse, what DMI_PRODUCT_NAME matches your laptop?

I'll have to check when I get home, is the relevant info from the "System 
Information" section of the dmidecode output?

Jesse

Re: ohci1394 unhandled interrupts bug in 2.6.14-rc2

[Stefan Richter]

Jesse Barnes wrote:
>>Jesse, what DMI_PRODUCT_NAME matches your laptop?
> 
> I'll have to check when I get home, is the relevant info from the "System 
> Information" section of the dmidecode output?

AFAICT yes. Although I wonder if there is a better suited part of the 
DMI table to look at. From what Rob posted, we could simply match 
{DMI_SYS_VENDOR, "TOSHIBA"} && {DMI_PRODUCT_VERSION, "PS5"} which 
probably catches all Satellite 5xxx's. I hope this isn't too general.

I also wonder how 1394 CardBus cards would react on this patch in 
affected Toshibas...

| +	toshiba = dmi_check_system(extra_init_dmi_table);
| +	if (toshiba) {
| +	        PRINT_G(KERN_INFO, "Toshiba %s detected, enabling extra 
initialization code",
| +			dmi_get_system_info(DMI_PRODUCT_NAME));
| +	        dev->current_state = 4;
| +		pci_read_config_word(dev, PCI_CACHE_LINE_SIZE, &toshiba_pcls);
| +	}
...
|	if (pci_enable_device(dev))
|  		FAIL(-ENXIO, "Failed to enable OHCI hardware");
...
| +	if (toshiba) {
| +	        mdelay(10);
| +	        pci_write_config_word(dev, PCI_CACHE_LINE_SIZE, toshiba_pcls);
| +	        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));
| +	}
...
|	pci_set_master(dev);

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

Re: ohci1394 unhandled interrupts bug in 2.6.14-rc2

[rob]

On Mon, 17 Oct 2005 17:58:10 +0200, Stefan Richter wrote
...
> > Toshiba Satellite 5105-s607
> 
> Thanks a lot for the survey. Do they all _need_ the patch, or do 
> some of them need it and the others are just not harmed by the patch?

they all need the patch to modprobe without errors, not all had firewire
devices to test with.

> Does anybody know a DMI_PRODUCT_NAME of a Libretto L1? Something 
> like PAL1060TNMM or PAL1060TNCM?

Might try John Belmonte <john at neggie.net>, he has an L5 and runs a website
for it.

rob.


--
Open WebMail Project (http://openwebmail.org)

Re: ohci1394 unhandled interrupts bug in 2.6.14-rc2

[Jesse Barnes]

On Monday, October 17, 2005 11:50 am, Stefan Richter wrote:
> Jesse Barnes wrote:
> >>Jesse, what DMI_PRODUCT_NAME matches your laptop?
> >
> > I'll have to check when I get home, is the relevant info from the
> > "System Information" section of the dmidecode output?
>
> AFAICT yes. Although I wonder if there is a better suited part of the
> DMI table to look at. From what Rob posted, we could simply match
> {DMI_SYS_VENDOR, "TOSHIBA"} && {DMI_PRODUCT_VERSION, "PS5"} which
> probably catches all Satellite 5xxx's. I hope this isn't too general.
>
> I also wonder how 1394 CardBus cards would react on this patch in
> affected Toshibas...

It should be harmless, right?  I don't have any way of testing since I 
don't have such a card myself.  Here's the dmidecode output from my 
machine.

Thanks,
Jesse

# dmidecode 2.7
SMBIOS 2.3 present.
45 structures occupying 1419 bytes.
Table at 0x000EB160.

Handle 0x0000, DMI type 0, 20 bytes.
BIOS Information
        Vendor: TOSHIBA
        Version: Version 1.20
        Release Date: 01/24/2005
        Address: 0xEB000
        Runtime Size: 84 kB
        ROM Size: 512 kB
        Characteristics:
                ISA is supported
                PCI is supported
                PC Card (PCMCIA) is supported
                PNP is supported
                APM is supported
                BIOS is upgradeable
                BIOS shadowing is allowed
                Boot from CD is supported
                BIOS ROM is socketed
                EDD is supported
                Japanese floppy for NEC 9800 1.2 MB is supported (int 
13h)
                3.5"/720 KB floppy services are supported (int 13h)
                3.5"/2.88 MB floppy services are supported (int 13h)
                Print screen service is supported (int 5h)
                8042 keyboard services are supported (int 9h)
                Serial services are supported (int 14h)
                Printer services are supported (int 17h)
                CGA/mono video services are supported (int 10h)
                ACPI is supported
                USB legacy is supported
                LS-120 boot is supported
                ATAPI Zip drive boot is supported
                Smart battery is supported
                Function key-initiated network boot is supported

Handle 0x0001, DMI type 1, 25 bytes.
System Information
        Manufacturer: TOSHIBA
        Product Name: Satellite M45
        Version: PSM40U-01X001
        Serial Number: 15106102Q
        UUID: 60178840-7169-11D9-A70E-00A0D1BB8852
        Wake-up Type: Power Switch

Handle 0x0002, DMI type 2, 17 bytes.
Base Board Information
        Manufacturer: TOSHIBA
        Product Name: Portable PC
        Version: Version A0
        Serial Number: 0000000000
        Asset Tag: Not Specified
        Features: None
        Location In Chassis: Not Specified
        Chassis Handle: 0x0000
        Type: Other
        Contained Object Handles: 0

Handle 0x0003, DMI type 3, 17 bytes.
Chassis Information
        Manufacturer: TOSHIBA
        Type: Portable
        Lock: Not Present
        Version: Version 1.0
        Serial Number: 00000000
        Asset Tag: 0000000000
        Boot-up State: Safe
        Power Supply State: Safe
        Thermal State: Safe
        Security Status: None
        OEM Information: 0x00000000

Handle 0x0004, DMI type 4, 32 bytes.
Processor Information
        Socket Designation: mFCPGA
        Type: Central Processor
        Family: Pentium M
        Manufacturer: GenuineIntelation
        ID: FF F9 E9 AF 00 00 00 00
        Signature: Type 3, Family 4073, Model 159, Stepping 15
        Flags: None
        Version: Pentium(R) M
        Voltage: 2.9 V
        External Clock: 133 MHz
        Max Speed: 1733 MHz
        Current Speed: 1733 MHz
        Status: Populated, Enabled
        Upgrade: None
        L1 Cache Handle: 0x0005
        L2 Cache Handle: 0x0006
        L3 Cache Handle: Not Provided

Handle 0x0005, DMI type 7, 19 bytes.
Cache Information
        Socket Designation: L1 Cache
        Configuration: Enabled, Not Socketed, Level 1
        Operational Mode: Write Back
        Location: Internal
        Installed Size: 32 KB
        Maximum Size: 32 KB
        Supported SRAM Types:
                Burst
                Pipeline Burst
                Asynchronous
        Installed SRAM Type: Burst
        Speed: Unknown
        Error Correction Type: None
        System Type: Unknown
        Associativity: Unknown

Handle 0x0006, DMI type 7, 19 bytes.
Cache Information
        Socket Designation: L2 Cache
        Configuration: Enabled, Not Socketed, Level 2
        Operational Mode: Write Back
        Location: External
        Installed Size: 2048 KB
        Maximum Size: 2048 KB
        Supported SRAM Types:
                Burst
                Pipeline Burst
                Asynchronous
        Installed SRAM Type: Burst
        Speed: Unknown
        Error Correction Type: None
        System Type: Unknown
        Associativity: Unknown

Handle 0x0007, DMI type 8, 9 bytes.
Port Connector Information
        Internal Reference Designator: Serial Port
        Internal Connector Type: None
        External Reference Designator: J16
        External Connector Type: DB-9 male
        Port Type: Serial Port 16550A Compatible

Handle 0x0008, DMI type 8, 9 bytes.
Port Connector Information
        Internal Reference Designator: Parallel Port
        Internal Connector Type: None
        External Reference Designator: J20
        External Connector Type: DB-25 male
        Port Type: Parallel Port ECP/EPP

Handle 0x0009, DMI type 8, 9 bytes.
Port Connector Information
        Internal Reference Designator: Kbd Connector
        Internal Connector Type: None
        External Reference Designator: J8
        External Connector Type: PS/2
        Port Type: Keyboard Port

Handle 0x000A, DMI type 8, 9 bytes.
Port Connector Information
        Internal Reference Designator: Mouse Conn.
        Internal Connector Type: None
        External Reference Designator: J9
        External Connector Type: PS/2
        Port Type: Mouse Port

Handle 0x000B, DMI type 8, 9 bytes.
Port Connector Information
        Internal Reference Designator: USB Conn.
        Internal Connector Type: None
        External Reference Designator: J12
        External Connector Type: Other
        Port Type: USB

Handle 0x000C, DMI type 8, 9 bytes.
Port Connector Information
        Internal Reference Designator: Dock Conn.
        Internal Connector Type: None
        External Reference Designator: J6
        External Connector Type: Other
        Port Type: Other

Handle 0x000D, DMI type 8, 9 bytes.
Port Connector Information
        Internal Reference Designator: VGA Conn.
        Internal Connector Type: None
        External Reference Designator: J7
        External Connector Type: Other
        Port Type: Other

Handle 0x000E, DMI type 8, 9 bytes.
Port Connector Information
        Internal Reference Designator: DC Input
        Internal Connector Type: None
        External Reference Designator: J35
        External Connector Type: Other
        Port Type: Other

Handle 0x000F, DMI type 8, 9 bytes.
Port Connector Information
        Internal Reference Designator: IrDA Port
        Internal Connector Type: None
        External Reference Designator: U1
        External Connector Type: Infrared
        Port Type: Other

Handle 0x0010, DMI type 8, 9 bytes.
Port Connector Information
        Internal Reference Designator: CardBus Conn.
        Internal Connector Type: None
        External Reference Designator: J26
        External Connector Type: Other
        Port Type: Cardbus

Handle 0x0011, DMI type 9, 13 bytes.
System Slot Information
        Designation: ISA Slot
        Type: 16-bit ISA
        Current Usage: Available
        Length: Long
        Characteristics: Unknown

Handle 0x0012, DMI type 9, 13 bytes.
System Slot Information
        Designation: PCI Slot
        Type: 32-bit PCI
        Current Usage: Available
        Length: Long
        ID: 4
        Characteristics: Unknown

Handle 0x0013, DMI type 10, 16 bytes.
On Board Device 1 Information
        Type: Other
        Status: Disabled
        Description: ECP Port
On Board Device 2 Information
        Type: Other
        Status: Disabled
        Description: 16550 UART
On Board Device 3 Information
        Type: Other
        Status: Disabled
        Description: IrDA Port
On Board Device 4 Information
        Type: Other
        Status: Enabled
        Description: CardBus Bridge
On Board Device 5 Information
        Type: Other
        Status: Enabled
        Description: IDE Controller
On Board Device 6 Information
        Type: Video
        Status: Enabled
        Description: Itl

Handle 0x0014, DMI type 11, 5 bytes.
OEM Strings
        String 1: PSM40U-01X001,SQ003520,11V

Handle 0x0015, DMI type 12, 5 bytes.
System Configuration Options
        Option 1: TOSHIBA

Handle 0x0016, DMI type 13, 22 bytes.
BIOS Language Information
        Installable Languages: 1
                en|US|iso8859-1
        Currently Installed Language: en|US|iso8859-1

Handle 0x0017, DMI type 16, 15 bytes.
Physical Memory Array
        Location: System Board Or Motherboard
        Use: System Memory
        Error Correction Type: None
        Maximum Capacity: 2 GB
        Error Information Handle: 0x001D
        Number Of Devices: 2

Handle 0x0018, DMI type 16, 15 bytes.
Physical Memory Array
        Location: System Board Or Motherboard
        Use: Video Memory
        Error Correction Type: None
        Maximum Capacity: 64 MB
        Error Information Handle: Not Provided
        Number Of Devices: 1

Handle 0x0019, DMI type 16, 15 bytes.
Physical Memory Array
        Location: System Board Or Motherboard
        Use: Flash Memory
        Error Correction Type: None
        Maximum Capacity: 512 kB
        Error Information Handle: Not Provided
        Number Of Devices: 1

Handle 0x001A, DMI type 16, 15 bytes.
Physical Memory Array
        Location: System Board Or Motherboard
        Use: Cache Memory
        Error Correction Type: None
        Maximum Capacity: 2 MB
        Error Information Handle: Not Provided
        Number Of Devices: 1

Handle 0x001B, DMI type 17, 23 bytes.
Memory Device
        Array Handle: 0x0017
        Error Information Handle: Not Provided
        Total Width: 64 bits
        Data Width: 64 bits
        Size: 512 MB
        Form Factor: SODIMM
        Set: Unknown
        Locator: DRAM Slot 0
        Bank Locator: Banks 0/1
        Type: DDR
        Type Detail: EDO
        Speed: 333 MHz (3.0 ns)

Handle 0x001C, DMI type 17, 23 bytes.
Memory Device
        Array Handle: 0x0017
        Error Information Handle: Not Provided
        Total Width: 64 bits
        Data Width: 64 bits
        Size: No Module Installed
        Form Factor: SODIMM
        Set: Unknown
        Locator: DRAM Slot 1
        Bank Locator: Banks 2/3
        Type: DDR
        Type Detail: EDO
        Speed: 333 MHz (3.0 ns)

Handle 0x001D, DMI type 18, 23 bytes.
32-bit Memory Error Information
        Type: Unknown
        Granularity: Memory Partition Level
        Operation: Unknown
        Vendor Syndrome: Unknown
        Memory Array Address: Unknown
        Device Address: Unknown
        Resolution: Unknown

Handle 0x001E, DMI type 19, 15 bytes.
Memory Array Mapped Address
        Starting Address: 0x00000000000
        Ending Address: 0x0001FFFFFFF
        Range Size: 512 MB
        Physical Array Handle: 0x0017
        Partition Width: 0

Handle 0x001F, DMI type 20, 19 bytes.
Memory Device Mapped Address
        Starting Address: 0x00000000000
        Ending Address: 0x00003FFFFFF
        Range Size: 64 MB
        Physical Device Handle: 0x001B
        Memory Array Mapped Address Handle: 0x0003
        Partition Row Position: <OUT OF SPEC>

Handle 0x0020, DMI type 20, 19 bytes.
Memory Device Mapped Address
        Starting Address: 0x00000000000
        Ending Address: 0x00003FFFFFF
        Range Size: 64 MB
        Physical Device Handle: 0x001C
        Memory Array Mapped Address Handle: 0x0003
        Partition Row Position: <OUT OF SPEC>

Handle 0x0021, DMI type 21, 7 bytes.
Built-in Pointing Device
        Type: Mouse
        Interface: PS/2
        Buttons: 2

Handle 0x0022, DMI type 22, 26 bytes.
Portable Battery
        Location: Smart Battery Conn J37
        Manufacturer: Duracell
        Manufacture Date: 01/06/97
        Serial Number: 0042
        Name: DR36
        Design Capacity: Unknown
        Design Voltage: 12 mV
        SBDS Version: V1.0
        Maximum Error: Unknown
        SBDS Chemistry: Not Specified
        OEM-specific Information: 0x00000000

Handle 0x0023, DMI type 23, 13 bytes.
System Reset
        Status: Disabled
        Watchdog Timer: Not Present

Handle 0x0024, DMI type 24, 5 bytes.
Hardware Security
        Power-On Password Status: Disabled
        Keyboard Password Status: Not Implemented
        Administrator Password Status: Disabled
        Front Panel Reset Status: Not Implemented

Handle 0x0025, DMI type 25, 9 bytes.
        System Power Controls
        Next Scheduled Power-on: *-* 00:00:00

Handle 0x0026, DMI type 26, 22 bytes.
Voltage Probe
        Description: Voltage Probe Description
        Location: Unknown
        Status: Unknown
        Maximum Value: Unknown
        Minimum Value: Unknown
        Resolution: Unknown
        Tolerance: Unknown
        Accuracy: Unknown
        OEM-specific Information: 0x00000000
        Nominal Value: 0.000 V

Handle 0x0027, DMI type 27, 14 bytes.
Cooling Device
        Temperature Probe Handle: 0x0028
        Type: Unknown
        Status: Unknown
        OEM-specific Information: 0x00000000
        Nominal Speed: 0 rpm

Handle 0x0028, DMI type 28, 22 bytes.
Temperature Probe
        Description: Temperature Probe Description
        Location: Unknown
        Status: Unknown
        Maximum Value: Unknown
        Minimum Value Unknown
        Resolution: Unknown
        Tolerance: Unknown
        Accuracy: Unknown
        OEM-specific Information: 0x00000000
        Nominal Value: 0.0 deg C

Handle 0x0029, DMI type 29, 22 bytes.
Electrical Current Probe
        Description: Electrical Probe Description
        Location: Unknown
        Status: Unknown
        Maximum Value: Unknown
        Minimum Value: Unknown
        Resolution: Unknown
        Tolerance: Unknown
        Accuracy: Unknown
        OEM-specific Information: 0x00000000
        Nominal Value: 0.000 A

Handle 0x002A, DMI type 30, 6 bytes.
Out-of-band Remote Access
        Manufacturer Name: Manufacturer Name
        Inbound Connection: Disabled
        Outbound Connection: Disabled

Handle 0x002B, DMI type 32, 11 bytes.
System Boot Information
        Status: No errors detected

Handle 0x002C, DMI type 127, 4 bytes.
End Of Table

Re: new PCI quirk for Toshiba Satellite?

[Greg KH]

On Sat, Oct 15, 2005 at 01:40:40PM -0700, Jesse Barnes wrote:
> On Sat, Oct 15, 2005 at 01:29:44PM -0700, Jesse Barnes wrote:
> > On Sat, Oct 15, 2005 at 09:39:06PM +0200, Stefan Richter wrote:
> > > Somebody mentioned this Linux-on-Toshiba-Satellite page recently on 
> > > linux1394-user: http://www.janerob.com/rob/ts5100/index.shtml
> > > The patch available from there was briefly discussed in February:
> > > http://marc.theaimsgroup.com/?l=linux1394-devel&t=110786507900006
> > > 
> > > Does this patch correct the problem on your machine?
> > 
> > Yes, it seems to help.  If I boot up and modprobe the driver with
> > toshiba=1, everything looks fine (I have no firewire devices to test
> > with).  If I modprobe it with toshiba=0, the system gets sluggish for a
> > second then IRQ 11 is disabled.  I had to update the patch though, as
> > shown below.
> > 
> > I'm not sure if the fix is proper though, maybe this should be handled
> > as a PCI quirk of this Toshiba board instead?  Either way, some kind of
> > fix should make it in soon, ideally to 2.6.14 or 2.6.14.1.
> 
> [Forwarding on to the PCI maintainers.]
> 
> It seems that the PCI config space isn't programmed correctly on these
> machines for some reason, so the fix below allows my OHCI device to work
> if I pass 'toshiba=1'.  This seems like something that belongs in the
> PCI layer instead though, doesn't it?

Yes, looks like it should be a pci quirk instead.

thanks,

greg k-h

Re: new PCI quirk for Toshiba Satellite?

[Stefan Richter]

Greg KH wrote:
> On Sat, Oct 15, 2005 at 01:40:40PM -0700, Jesse Barnes wrote:
>>>On Sat, Oct 15, 2005 at 09:39:06PM +0200, Stefan Richter wrote:
>>>>Somebody mentioned this Linux-on-Toshiba-Satellite page recently on 
>>>>linux1394-user: http://www.janerob.com/rob/ts5100/index.shtml
>>>>The patch available from there was briefly discussed in February:
>>>>http://marc.theaimsgroup.com/?l=linux1394-devel&t=110786507900006
...
>>It seems that the PCI config space isn't programmed correctly on these
>>machines for some reason, so the fix below allows my OHCI device to work
>>if I pass 'toshiba=1'.  This seems like something that belongs in the
>>PCI layer instead though, doesn't it?
> 
> Yes, looks like it should be a pci quirk instead.

I gather from DMI info provided by Rob and Jesse that affected machines 
could be determined by dmi_check_system() using these two checks:
   - DMI_SYS_VENDOR contains "TOSHIBA" && DMI_PRODUCT_VERSION contains
     "PS5", i.e. most of Satellite 5xxx, as well as
   - DMI_SYS_VENDOR contains "TOSHIBA" && DMI_PRODUCT_VERSION contains
     "PSM4", i.e. all Satellite M4x.
A few more series are probably affected too.

http://marc.theaimsgroup.com/?l=linux-kernel&m=112955338318326
http://marc.theaimsgroup.com/?l=linux-kernel&m=112974457930368
http://linux.toshiba-dme.co.jp/linux/eng/speclist.php3

BTW, the author of the initial incarnation of the patch is this person:
http://nemaru.at.infoseek.co.jp/1394.html
-- 
Stefan Richter
-=====-=-=-= =-=- =-=--
http://arcgraph.de/sr/

Re: new PCI quirk for Toshiba Satellite?

[Jesse Barnes]

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

On Thursday, October 20, 2005 11:32 am, Stefan Richter wrote:
> Greg KH wrote:
> > On Sat, Oct 15, 2005 at 01:40:40PM -0700, Jesse Barnes wrote:
> >> It seems that the PCI config space isn't programmed correctly on
> >> these machines for some reason, so the fix below allows my OHCI
> >> device to work if I pass 'toshiba=1'.  This seems like something
> >> that belongs in the PCI layer instead though, doesn't it?
> >
> > Yes, looks like it should be a pci quirk instead.
>
> I gather from DMI info provided by Rob and Jesse that affected
> machines could be determined by dmi_check_system() using these two
> checks: - DMI_SYS_VENDOR contains "TOSHIBA" && DMI_PRODUCT_VERSION
> contains "PS5", i.e. most of Satellite 5xxx, as well as
>    - DMI_SYS_VENDOR contains "TOSHIBA" && DMI_PRODUCT_VERSION contains
>      "PSM4", i.e. all Satellite M4x.
> A few more series are probably affected too.
>
> http://marc.theaimsgroup.com/?l=linux-kernel&m=112955338318326
> http://marc.theaimsgroup.com/?l=linux-kernel&m=112974457930368
> http://linux.toshiba-dme.co.jp/linux/eng/speclist.php3
>
> BTW, the author of the initial incarnation of the patch is this
> person: http://nemaru.at.infoseek.co.jp/1394.html

Stefan, is a PCI quirk addition possible or do we have to use 
dmi_check_system in the ohci driver itself (since we have to reprogram 
the cache line size in addition to the other registers)?

Something like this is what you had in mind (please forgive the word 
wrapping)?  In ohci1394.c somewhere:

> struct tosh_config_data {
> 	struct pci_dev *dev;
> 	u16 cache_line_size;
> };
>
> int ohci1394_toshiba_reprogram_config(struct dmi_system_id *d)
> {
> 	struct tosh_config_data *data = d->driver_data;
>
> 	mdelay(10);
> 	pci_write_config_word(data->dev, PCI_CACHE_LINE_SIZE,
> 			      data->cache_line_size);
> 	pci_write_config_word(data->dev, PCI_INTERRUPT_LINE, data->dev->irq);
> 	pci_write_config_dword(data->dev, PCI_BASE_ADDRESS_0,
> 			       pci_resource_start(data->dev, 0));
> 	pci_write_config_dword(data->dev, PCI_BASE_ADDRESS_1,
> 			       pci_resource_start(data->dev, 1));
> }

And then in ohci1394_pci_probe():

> struct tosh_config_data tosh_data;
> tosh_data.dev = dev;
> pci_read_config_word(dev, PCI_CACHE_LINE_SIZE,
> 		     &tosh_data.cache_line_size);
> static struct dmi_system_id __initdata toshiba_ohci1394_dmi_table[] =
> { {	/* Handle lack of config space programming on Toshiba laptops */
> .callback = ohci1394_toshiba_reprogram_config,
> 		.ident = "Toshiba PS5 based laptop",
> 		.matches = {
> 			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
> 			DMI_MATCH(DMI_PRODUCT_VERSION, "PS5"),
> 		},
> 		.driver_data = &tosh_data;
> 	},
> 	{
> 		.callback = ohci1394_toshiba_reprogram_config,
> 		.ident = "Toshiba PSM4 based laptop",
> 		.matches = {
> 			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
> 			DMI_MATCH(DMI_PRODUCT_VERSION, "PSM4"),
> 		},
> 		.driver_data = &tosh_data;
> 	},
> }
>
> dmi_check_system(toshiba_ohci1394_dmi_table);

But then what about the dev->current_state = 4?  Is that necessary?

And is anyone going to put together a patch for this, or should I do it?  
It would be great to have fixed either in 2.6.14 or 2.6.14.1 given that 
it breaks my laptop everytime I upgrade my kernel and forget to delete 
the ieee1394 devices from my initrd.

Jody or Ben, any comment?  I've attached the patch I most recently tested 
for reference.

Thanks,
Jesse

[-- Attachment #2: ohci-toshiba-fix-2.patch --]
[-- Type: text/x-diff, Size: 1649 bytes --]

diff -X linux-2.6.14-rc2/Documentation/dontdiff -Naur linux-2.6.14-rc2.orig/drivers/ieee1394/ohci1394.c linux-2.6.14-rc2/drivers/ieee1394/ohci1394.c
--- linux-2.6.14-rc2.orig/drivers/ieee1394/ohci1394.c	2005-09-19 20:00:41.000000000 -0700
+++ linux-2.6.14-rc2/drivers/ieee1394/ohci1394.c	2005-10-15 12:55:08.000000000 -0700
@@ -169,6 +169,10 @@
 module_param(phys_dma, int, 0644);
 MODULE_PARM_DESC(phys_dma, "Enable physical dma (default = 1).");
 
+static int toshiba __initdata = 0;
+module_param(toshiba, bool, 0);
+MODULE_PARM_DESC(toshiba, "Toshiba Legacy-Free BIOS workaround (default=0).");
+
 static void dma_trm_tasklet(unsigned long data);
 static void dma_trm_reset(struct dma_trm_ctx *d);
 
@@ -3222,14 +3226,28 @@
 	struct hpsb_host *host;
 	struct ti_ohci *ohci;	/* shortcut to currently handled device */
 	unsigned long ohci_base;
+	u16  toshiba_data;
 
 	if (version_printed++ == 0)
 		PRINT_G(KERN_INFO, "%s", version);
 
+	if (toshiba) {
+		dev->current_state = 4;
+		pci_read_config_word(dev, PCI_CACHE_LINE_SIZE, &toshiba_data);
+	}
+
         if (pci_enable_device(dev))
 		FAIL(-ENXIO, "Failed to enable OHCI hardware");
         pci_set_master(dev);
 
+	if (toshiba) {
+		mdelay(10);
+		pci_write_config_word(dev, PCI_CACHE_LINE_SIZE, toshiba_data);
+		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));
+ 	}
+
 	host = hpsb_alloc_host(&ohci1394_driver, sizeof(struct ti_ohci), &dev->dev);
 	if (!host) FAIL(-ENOMEM, "Failed to allocate host structure");
 

Re: new PCI quirk for Toshiba Satellite?

[Stefan Richter]

Jesse Barnes wrote:
> Stefan, is a PCI quirk addition possible or do we have to use 
> dmi_check_system in the ohci driver itself (since we have to reprogram 
> the cache line size in addition to the other registers)?

I am not familiar with the PCI subsystem, thus cannot advise how to 
handle it best nor wanted to post a patch myself (yet).

[...]
>>		.callback = ohci1394_toshiba_reprogram_config,
>>		.ident = "Toshiba PSM4 based laptop",
>>		.matches = {
>>			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
>>			DMI_MATCH(DMI_PRODUCT_VERSION, "PSM4"),
>>		},
>>		.driver_data = &tosh_data;

It seems to me, using the .callback and .driver_data doesn't make it 
cleaner and leaner.

> But then what about the dev->current_state = 4?  Is that necessary?

It is necessary; at least if the workaround resides in ohci1394. 
Otherwise the controller won't come back after a suspend/ resume cycle. 
(See Rob's post from February, 
http://marc.theaimsgroup.com/?m=110786495210243 ) Maybe there is another 
way to do that if the workaround was moved to pci/quirks.c.

[...]
> +	if (toshiba) {
> +		dev->current_state = 4;
> +		pci_read_config_word(dev, PCI_CACHE_LINE_SIZE, &toshiba_data);
> +	}
> +
>          if (pci_enable_device(dev))
>  		FAIL(-ENXIO, "Failed to enable OHCI hardware");
>          pci_set_master(dev);
>  
> +	if (toshiba) {
> +		mdelay(10);
> +		pci_write_config_word(dev, PCI_CACHE_LINE_SIZE, toshiba_data);
[...]

pci_set_master(dev) can be moved below the second part of the Toshiba 
workaround. That means AFAIU, the 2nd part of the Toshiba workaround can 
be moved out of ohci1394 into pci_fixup_device() which is called from 
pci_enable_device(), to be called as a DECLARE_PCI_FIXUP_ENABLE hook.

The first part of the workaround, i.e. caching the cache line size, for 
example by means of a static variable, would have to go into an 
_FIXUP_EARLY, _FIXUP_HEADER, or _FIXUP_FINAL hook. I am not sure yet 
about which type of hook to use.

Furthermore, everything which belongs to the workaround should IMO be 
enclosed by #ifdef SOME_SENSIBLE_MACRO. This avoids kernel bloat for any 
target which is surely not a Toshiba laptop. Rob used an #if 
defined(__i386__).
-- 
Stefan Richter
-=====-=-=-= =-=- =-=-=
http://arcgraph.de/sr/

Re: new PCI quirk for Toshiba Satellite?

[Jesse Barnes]

On Friday, October 21, 2005 1:13 pm, Stefan Richter wrote:
> Jesse Barnes wrote:
> > Stefan, is a PCI quirk addition possible or do we have to use
> > dmi_check_system in the ohci driver itself (since we have to
> > reprogram the cache line size in addition to the other registers)?
>
> I am not familiar with the PCI subsystem, thus cannot advise how to
> handle it best nor wanted to post a patch myself (yet).

Ok, I'll update Rob's patch to the latest tree and send it on (I hadn't 
seen it yet when I wrote my version).

> It seems to me, using the .callback and .driver_data doesn't make it
> cleaner and leaner.

I agree, I thought it was necessary since I hadn't looked at 
dmi_check_system and didn't know if it had a return value I could check.  
Rob's patch looks much simpler and nicer.

> > But then what about the dev->current_state = 4?  Is that necessary?
>
> It is necessary; at least if the workaround resides in ohci1394.
> Otherwise the controller won't come back after a suspend/ resume
> cycle. (See Rob's post from February,
> http://marc.theaimsgroup.com/?m=110786495210243 ) Maybe there is
> another way to do that if the workaround was moved to pci/quirks.c.

Having it all in the driver probably makes the most sense if we have to 
have code there anyway.  Otherwise future users may have to check both 
places if things break again in another configuration.

> Furthermore, everything which belongs to the workaround should IMO be
> enclosed by #ifdef SOME_SENSIBLE_MACRO. This avoids kernel bloat for
> any target which is surely not a Toshiba laptop. Rob used an #if
> defined(__i386__).

Checks against the compiler defined arch are usually wrong since users 
could be cross compiling, and I'd like to avoid an ifdef altogether.  I 
think we can make the code collapse entirely by fixing linux/dmi.h.  If 
we remove the !defined(CONFIG_X86_64) check around the extern of 
dmi_check_system, all other arches will have it defined to simply return 
0, causing gcc to remove the dead conditional in ohci1394.c.

Anyway, I'll refresh that patch, test it, and send it on to Andrew, 
hopefully tonight.

Thanks,
Jesse

Re: new PCI quirk for Toshiba Satellite?

[Jesse Barnes]

On Monday, October 24, 2005 10:45 am, Jesse Barnes wrote:
> Checks against the compiler defined arch are usually wrong since users
> could be cross compiling, and I'd like to avoid an ifdef altogether. 
> I think we can make the code collapse entirely by fixing linux/dmi.h. 
> If we remove the !defined(CONFIG_X86_64) check around the extern of
> dmi_check_system, all other arches will have it defined to simply
> return 0, causing gcc to remove the dead conditional in ohci1394.c.

Duh, I don't even think we have to do anything to dmi.h, it should work 
as described above as it stands now.  All we have to do is 
unconditionally add dmi.h to ohci1394.c and use the dmi stuff there; on 
x86 it'll do something, on other arches it should compile out just fine.

Jesse

Re: new PCI quirk for Toshiba Satellite?

[Stefan Richter]

Jesse Barnes wrote:
> On Friday, October 21, 2005 1:13 pm, Stefan Richter wrote:
>>Jesse Barnes wrote:
>>>But then what about the dev->current_state = 4?  Is that necessary?
>>
>>It is necessary; at least if the workaround resides in ohci1394.
>>Otherwise the controller won't come back after a suspend/ resume
>>cycle. (See Rob's post from February,
>>http://marc.theaimsgroup.com/?m=110786495210243 ) Maybe there is
>>another way to do that if the workaround was moved to pci/quirks.c.
> 
> Having it all in the driver probably makes the most sense if we have to 
> have code there anyway.  Otherwise future users may have to check both 
> places if things break again in another configuration.

No, better move it to pci/quirks.c. I may be wrong (again) but I think
everything of the workaround can be done there.

>>Furthermore, everything which belongs to the workaround should IMO be
>>enclosed by #ifdef SOME_SENSIBLE_MACRO. This avoids kernel bloat for
>>any target which is surely not a Toshiba laptop. Rob used an #if
>>defined(__i386__).
> 
> Checks against the compiler defined arch are usually wrong since users 
> could be cross compiling, and I'd like to avoid an ifdef altogether.

Once the workaround is in pci/quirks.c, a single #ifdef would suffice
(if it is still of any benefit there).
-- 
Stefan Richter
-=====-=-=-= =-=- ==---
http://arcgraph.de/sr/

Re: new PCI quirk for Toshiba Satellite?

[Ivan Kokshaysky]

On Mon, Oct 24, 2005 at 08:21:06PM +0200, Stefan Richter wrote:
> Once the workaround is in pci/quirks.c, a single #ifdef would suffice
> (if it is still of any benefit there).

Such an obviously i386-specific quirk should go into
arch/i386/pci/fixup.c.

Ivan.