Re: quirk_e100_interrupt() called too early

Re: quirk_e100_interrupt() called too early

[Kok, Auke]

Marian Balakowicz wrote:
> I am enabling and testing PCI on tqm5200 mpc5200 based board where I
> faced the following issue.
> 
> I am using EEPRO100 PCI card for which there is specific
> quirk_e100_interrupt that tries to disable interrupts if
> they were left enabled by the firmware. quirk_e100_interrupts() is
> called after PCI controller is initialized and before PCI bus
> enumeration is performed. On some powerpc platforms, like the one I am
> using, PCI controller configuration sets different MEM and IO windows
> than those set by firmware. That is why quirk_e100_interrupt() is
> causing kernel panic as it tries to read from device BAR0 offets which
> at this time point to a invalid PCI window (set by firmware).
> 
> The patch below delays the quirk_100_interrupt() to pci_fixup_final
> phase, which happens after bus enumeration and before device PCI enable
> and device driver initialization - so, it seem to be still a good place
> for this quirk. It works fine for me but I only tested it on a tqm5200.
> Could someone please help and verify that on other platforms?

will try to do. This sounds indeed like the proper thing to do. Unfortunately I 
don't have any NICs to repro this on (allthough I have a ppc box with said 
firmware probe method).

Bjorn orignially wrote this patch, perhaps he can comment on the fixup move?

Also, please send a signed-off-by patch so I can push it forward as usual.

Cheers,

Auke


> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 01d8f8a..7194074 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -1475,7 +1475,7 @@ static void __devinit quirk_e100_interru
> 
>         iounmap(csr);
>  }
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
> quirk_e100_interrupt);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
> quirk_e100_interrupt);
> 
>  static void __devinit fixup_rev1_53c810(struct pci_dev* dev)
>  {

Re: quirk_e100_interrupt() called too early

[Bjorn Helgaas]

On Thursday 28 June 2007 10:01:08 am Kok, Auke wrote:
> Marian Balakowicz wrote:
> > I am enabling and testing PCI on tqm5200 mpc5200 based board where I
> > faced the following issue.
> > 
> > I am using EEPRO100 PCI card for which there is specific
> > quirk_e100_interrupt that tries to disable interrupts if
> > they were left enabled by the firmware. quirk_e100_interrupts() is
> > called after PCI controller is initialized and before PCI bus
> > enumeration is performed. On some powerpc platforms, like the one I am
> > using, PCI controller configuration sets different MEM and IO windows
> > than those set by firmware. That is why quirk_e100_interrupt() is
> > causing kernel panic as it tries to read from device BAR0 offets which
> > at this time point to a invalid PCI window (set by firmware).
> > 
> > The patch below delays the quirk_100_interrupt() to pci_fixup_final
> > phase, which happens after bus enumeration and before device PCI enable
> > and device driver initialization - so, it seem to be still a good place
> > for this quirk. It works fine for me but I only tested it on a tqm5200.
> > Could someone please help and verify that on other platforms?
> 
> will try to do. This sounds indeed like the proper thing to do. Unfortunately I 
> don't have any NICs to repro this on (allthough I have a ppc box with said 
> firmware probe method).
> 
> Bjorn orignially wrote this patch, perhaps he can comment on the fixup move?

Your patch looks reasonable to me.

I don't have a machine that exhibits the original problem, so
I can't actually test this patch either.  As long as the fixup
happens before any PCI drivers claim devices, I think it should
be fine.

I copied Nils Rennebarth, who originally reported the bug:
  http://bugzilla.kernel.org/show_bug.cgi?id=5918
in case he can test your patch.

> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index 01d8f8a..7194074 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -1475,7 +1475,7 @@ static void __devinit quirk_e100_interru
> > 
> >         iounmap(csr);
> >  }
> > -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
> > quirk_e100_interrupt);
> > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
> > quirk_e100_interrupt);
> > 
> >  static void __devinit fixup_rev1_53c810(struct pci_dev* dev)
> >  {
> 
> 

Re: quirk_e100_interrupt() called too early

[Nils Rennebarth]

Bjorn Helgaas wrote:
> On Thursday 28 June 2007 10:01:08 am Kok, Auke wrote:
> I copied Nils Rennebarth, who originally reported the bug:
>   http://bugzilla.kernel.org/show_bug.cgi?id=5918
> in case he can test your patch.
Most of the embedded boards we now work with appear to exhibit the same 
behaviour: During boot the

Firmware left 0000:01:05.0 e100 interrupts enabled, disabling

message appears for all four e100 nics. (The mainboard is a em-8510A 
btw.) When I have time I could try if the quirk still works with the
mentioned patch, but I am quite busy with other things at the moment.

-- 

Mit freundlichen Grüßen / with kind regards

Nils Rennebarth, Software Developer

--
Funkwerk IP-Appliances GmbH
Mönchhaldenstraße 28
D-70191 Stuttgart

Tel: +49 711 900300 - 0
Fax: +49 711 900300 - 90

E-Mail: Nils.Rennebarth@funkwerk-ec.com

Location: GmbH Neu-Ulm, Local Court Memmingen, HRB 13043
Managing Directors: Michael Marsanu, Soenke Weisner

--------------------------------
The information contained in this e-mail has been carefully researched,
but the possibility of it being inapplicable in individual cases cannot
be ruled out. We therefore regret that we cannot accept responsibility
or liability of any kind whatsoever for the correctness of the
information given. Please notify us if you discover that information is
inapplicable.

Re: quirk_e100_interrupt() called too early

[Kok, Auke]

Nils Rennebarth wrote:
> Bjorn Helgaas wrote:
>> On Thursday 28 June 2007 10:01:08 am Kok, Auke wrote:
>> I copied Nils Rennebarth, who originally reported the bug:
>>   http://bugzilla.kernel.org/show_bug.cgi?id=5918
>> in case he can test your patch.
 >
> Most of the embedded boards we now work with appear to exhibit the same 
> behaviour: During boot the
> 
> Firmware left 0000:01:05.0 e100 interrupts enabled, disabling
> 
> message appears for all four e100 nics. (The mainboard is a em-8510A 
> btw.) When I have time I could try if the quirk still works with the
> mentioned patch, but I am quite busy with other things at the moment.

well, we're too late for 2.6.22 I think so you have some time. However I'd like 
to make sure we can get this in with 2.6.23. Alternatively, someone could always 
send me one of these boards ;)

Thanks for taking the time to test

Auke

Re: quirk_e100_interrupt() called too early

[Marian Balakowicz]

Kok, Auke wrote:
> Marian Balakowicz wrote:
>> The patch below delays the quirk_100_interrupt() to pci_fixup_final
>> phase, which happens after bus enumeration and before device PCI enable
>> and device driver initialization - so, it seem to be still a good place
>> for this quirk. It works fine for me but I only tested it on a tqm5200.
>> Could someone please help and verify that on other platforms?
> 
> will try to do. This sounds indeed like the proper thing to do. Unfortunately I 
> don't have any NICs to repro this on (allthough I have a ppc box with said 
> firmware probe method).
> 
> Bjorn orignially wrote this patch, perhaps he can comment on the fixup move?
> 
> Also, please send a signed-off-by patch so I can push it forward as usual.

Sure, will do that, just wanted someone to have a look at it in the first place.

Best Regards,
Marian

[PATCH] PCI: quirk_e100_interrupt() called too early

[Marian Balakowicz]

quirk_e100_interrupts() is called after PCI controller is initialized
and before PCI bus enumeration is performed. On some powerpc platforms
which modify PCI controller configuration and set different MEM and IO
windows than those set by firmware quirk_e100_interrupt() is causing
kernel panic as it tries to read from device BAR0 offets which at this
time points to a invalid PCI window (set by firmware).

This patch delays the quirk_100_interrupt() to pci_fixup_final phase,
which happens after bus enumeration and before PCI enable and
device driver initialization.

Signed-off-by: Marian Balakowicz <m8@semihalf.com>
---

 drivers/pci/quirks.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 01d8f8a..7194074 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1475,7 +1475,7 @@ static void __devinit quirk_e100_interru

 	iounmap(csr);
 }
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_ANY_ID, quirk_e100_interrupt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_ANY_ID, quirk_e100_interrupt);

 static void __devinit fixup_rev1_53c810(struct pci_dev* dev)
 {

patch pci-quirk_e100_interrupt-called-too-early.patch added to gregkh-2.6 tree

[gregkh]

This is a note to let you know that I've just added the patch titled

     Subject: [PATCH] PCI: quirk_e100_interrupt() called too early

to my gregkh-2.6 tree.  Its filename is

     pci-quirk_e100_interrupt-called-too-early.patch

This tree can be found at 
    http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/


>From owner-linux-pci@atrey.karlin.mff.cuni.cz Tue Jul  3 02:03:55 2007
From: Marian Balakowicz <m8@semihalf.com>
Date: Tue, 03 Jul 2007 11:03:18 +0200
Subject: [PATCH] PCI: quirk_e100_interrupt() called too early
To: "Kok, Auke" <auke-jan.h.kok@intel.com>
Cc: bjorn.helgaas@hp.com, linux-pci@atrey.karlin.mff.cuni.cz, len.brown@intel.com, NetDev <netdev@vger.kernel.org>
Message-ID: <468A10D6.2020900@semihalf.com>


quirk_e100_interrupts() is called after PCI controller is initialized
and before PCI bus enumeration is performed. On some powerpc platforms
which modify PCI controller configuration and set different MEM and IO
windows than those set by firmware quirk_e100_interrupt() is causing
kernel panic as it tries to read from device BAR0 offets which at this
time points to a invalid PCI window (set by firmware).

This patch delays the quirk_100_interrupt() to pci_fixup_final phase,
which happens after bus enumeration and before PCI enable and
device driver initialization.

Signed-off-by: Marian Balakowicz <m8@semihalf.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/pci/quirks.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1485,7 +1485,7 @@ static void __devinit quirk_e100_interru
 
 	iounmap(csr);
 }
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_ANY_ID, quirk_e100_interrupt);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_ANY_ID, quirk_e100_interrupt);
 
 static void __devinit fixup_rev1_53c810(struct pci_dev* dev)
 {


Patches currently in gregkh-2.6 which might be from m8@semihalf.com are

pci/pci-quirk_e100_interrupt-called-too-early.patch

Re: patch pci-quirk_e100_interrupt-called-too-early.patch added to gregkh-2.6 tree

[Kok, Auke]

gregkh@suse.de wrote:
> This is a note to let you know that I've just added the patch titled
> 
>      Subject: [PATCH] PCI: quirk_e100_interrupt() called too early
> 
> to my gregkh-2.6 tree.  Its filename is
> 
>      pci-quirk_e100_interrupt-called-too-early.patch
> 
> This tree can be found at 
>     http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/
> 
> 
> From owner-linux-pci@atrey.karlin.mff.cuni.cz Tue Jul  3 02:03:55 2007
> From: Marian Balakowicz <m8@semihalf.com>
> Date: Tue, 03 Jul 2007 11:03:18 +0200
> Subject: [PATCH] PCI: quirk_e100_interrupt() called too early
> To: "Kok, Auke" <auke-jan.h.kok@intel.com>
> Cc: bjorn.helgaas@hp.com, linux-pci@atrey.karlin.mff.cuni.cz, len.brown@intel.com, NetDev <netdev@vger.kernel.org>
> Message-ID: <468A10D6.2020900@semihalf.com>
> 
> 
> quirk_e100_interrupts() is called after PCI controller is initialized
> and before PCI bus enumeration is performed. On some powerpc platforms
> which modify PCI controller configuration and set different MEM and IO
> windows than those set by firmware quirk_e100_interrupt() is causing
> kernel panic as it tries to read from device BAR0 offets which at this
> time points to a invalid PCI window (set by firmware).
> 
> This patch delays the quirk_100_interrupt() to pci_fixup_final phase,
> which happens after bus enumeration and before PCI enable and
> device driver initialization.
> 
> Signed-off-by: Marian Balakowicz <m8@semihalf.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> 
> ---
>  drivers/pci/quirks.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -1485,7 +1485,7 @@ static void __devinit quirk_e100_interru
>  
>  	iounmap(csr);
>  }
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_ANY_ID, quirk_e100_interrupt);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_ANY_ID, quirk_e100_interrupt);
>  
>  static void __devinit fixup_rev1_53c810(struct pci_dev* dev)
>  {
> 
> 
> Patches currently in gregkh-2.6 which might be from m8@semihalf.com are
> 
> pci/pci-quirk_e100_interrupt-called-too-early.patch


Yes, that's OK. Please note that I asked the person who originally reported the 
problem to make sure that this patch doesn't break anything, but he was still 
too busy to test until now.

I assume (from what I know of the PCI subsystem now) that this is a safe patch, 
so feel free to add:

Acked-by: Auke Kok <auke-jan.h.kok@intel.com>

Cheers,


Auke