[coreboot] Gigabyte M57SLI-S4 Interrupt Fix

Harald Gutmann harald.gutmann at gmx.net
Thu Jun 11 01:26:33 CEST 2009


On Thursday 11 June 2009 01:06:05 Luc Verhaegen wrote:
> On Tue, Jun 09, 2009 at 09:18:58PM +0200, Harald Gutmann wrote:
> > Hello,
> >
> > after a few tries of me to fix the interrupt problems which occurred on
> > the M57SLI here is the final patch from me.
> >
> > The Patch fixes the following issues on M57SLI:
> > * Interrupt routing in the MPTable
> >
> > which affects the following hardware parts:
> > * PCI-E 16x Slots (blue and black) - both are working now like a charm
> > * PCI-E 1x Slots - all three should be fine, but I have no card to test.
> > (untested)
> > * PCI Slots - both work fine now.
> >
> > This is the first and (hopefully) the last patch on this issue which gets
> > a
> >
> > Signed-off-by: Harald Gutmann <harald.gutmann at gmx.net>
> >
> > from me.
> >
> > I'm really looking forward to commit this patch (which is my first one)
> > and mark the interrupt issues on the wiki page to the M57SLI as solved.
> > :)
> >
> >
> > Kind regards,
> > Harald Gutmann
> >
> >
> >
> > @@ -87,19 +87,17 @@
> >  			if (res) {
> >  				smp_write_ioapic(mc, apicid_mcp55, 0x11, res->base);
> >  			}
> > -
> > -			dword = 0x43c6c643;
> > +			dword = 0xc643c643;
> >  	        	pci_write_config32(dev, 0x7c, dword);
> >
> > -		        dword = 0x81001a00;
> > +		        dword = 0x8da01009;
> >  		        pci_write_config32(dev, 0x80, dword);
> >
> > -	        	dword = 0xd0001202;
> > +	        	dword = 0x200018d2;
> >  		        pci_write_config32(dev, 0x84, dword);
> > -
> >                  }
> >  	}
> > -
> > +
>
> Anal stuff: while you're here, do get rid of dword :)
I'll do that.

>
> >  	/* The PCIe slots, each on its own bus */
> > -	for(j=7; j>=2; j--) {
> > -		if(!bus_mcp55[j]) continue;
> > -	        for(i=0;i<4;i++) { /* map all functions */
> > -        	        PCI_INT(j,0,i, 16+(1+j+i)%4);
> > -        	}
> > -	}
> > +        k = 1;
> > +        for(i=0; i<=3; i++){
> > +                for(j=7; j>=2; j--){
> > +                        if(k>3) k=0;
> > +                        PCI_INT(j,0,i, 16+k);
> > +                        k++;
> > +                }
> > +                k--;
> > +        }
>
> Anal stuff: i<4 and j>1 is easier on the eye :)
Yes, sounds good.

> >  	/* On bus 1: the physical PCI bus slots...  */
> > -	for(j=0; j<2; j++) /* on a Rev 1.x board, they are devs 7 and 8 */
> > -	        for(i=0;i<4;i++) { /* map all functions */
> > -        	        PCI_INT(1,7+j,i, 16+(3+i+j)%4);
> > -	        }
> > -	/* ... and OB FireWire */
> > -	PCI_INT(1,0x0a,0, 18);
> > +        k=2;
> > +        for(i=0; i<=3; i++){
> > +                for(j=6; j<=10; j++){
> > +                        if(k>3) k=0;
> > +                        PCI_INT(1,j,i, 16+k);
> > +                        k++;
> > +                }
> > +        }
>
> This is the only bit i have a real question about: you remove the comment:
> /* on a Rev 1.x board, they are devs 7 and 8 */
Oh, removing the comment was not my intention, it seems that i just missed 
this line.

> and have gone from 7+[0,1] to [6,7,8,9,10] for the devices.
>
> While i like the for loops used this way, you are touching devices 6,9,10
> which weren't touched before. What board revision are you using? Could this
> be the difference between your board and what yinghai had? What possible
> effects are there with assigning irqs in the apic to devices which possibly
> aren't there (as i have no clue about apics)?
Okay, good that you ask this, because I'm not really sure if it is a benefit to 
add the devices 6,9,10, but when Rudolf Marek figured out the wiring from the 
vendors dsdt.asl file there appeared all devices from 6-10.
I haven't looked at the mainboard detail if there is something wired and used 
from that devices, so i can't give a good answer to your question.

But i recognized that there is at least one line which is done doubled in the 
code, and it would be for sure a good idea to drop this line:
+	PCI_INT(1,0x0a,0, 18);   /* Firewire */

For the wiring which was figured out from the dstd.asl you can have a look on 
http://www.coreboot.org/Nvidia_MCP55_Porting_Notes where I added the 
information.

Has anyone of you comments/ideas on that?
Would it be better to ignore devices 6,9,10 and add a single initialization 
for the firewire device?
Does it matter if the "unnecessary" devices get initialized?


> Please keep at least some of Yinghai's comment and expand this with your
> information.
Yes, that's a good idea, I'll keep at least a modified comment on the 
initialization for Bus 1.

> This is the only question i have with this change, if i see a good answer
> to this, you have:
>
> Acked-by: Luc Verhaegen <libv at skynet.be>
>
> Luc Verhaegen.
Thanks for reviewing my first patch which is intended to get committed.
Harald Gutmann

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20090611/80762ac7/attachment.sig>


More information about the coreboot mailing list