Signed-off-by: Marc Bertens mbertens@xs4all.nl
Hello all,
Here is the update for the Nokia IP530 system. it has now working NICs and correct PIRQ table. corrections where made to serveral files.
Currently the PCMCIA controller need some more work, it uses a PCI1225 of Texas Instruments. Any help with this is appreciated.
Marc.
On Mon, 24 May 2010 14:25:46 +0200, mbertens mbertens@xs4all.nl wrote:
Signed-off-by: Marc Bertens mbertens@xs4all.nl
Hello all,
Here is the update for the Nokia IP530 system. it has now working NICs and correct PIRQ table. corrections where made to serveral files.
Currently the PCMCIA controller need some more work, it uses a PCI1225 of Texas Instruments. Any help with this is appreciated.
Nice :-) Any reason why you are using romcc instead of CAR?
On Mon, 2010-05-24 at 10:35 -0400, Joseph Smith wrote:
On Mon, 24 May 2010 14:25:46 +0200, mbertens mbertens@xs4all.nl wrote:
Signed-off-by: Marc Bertens mbertens@xs4all.nl
Hello all,
Here is the update for the Nokia IP530 system. it has now working NICs and correct PIRQ table. corrections where made to serveral files.
Currently the PCMCIA controller need some more work, it uses a PCI1225 of Texas Instruments. Any help with this is appreciated.
Nice :-) Any reason why you are using romcc instead of CAR?
Simple because i started with the project and it was there, i was interrested in getting it working, now its time for improvements.
And what is de difference between romcc and CAR ?
Marc
Signed-off-by: Marc Bertens mbertens@xs4all.nl
Hello all,
Here is the update for the Nokia IP530 system. it has now working NICs and correct PIRQ table. corrections where made to serveral files.
Congratulations on getting it to work.
Could you send a patch against the current revision? The IP530 is already in the tree.
For your updated patch I'd suggest not including asus_p2b_irq_tables.c I'd also look into the comments that Stefan made about devicetree.cb
Currently the PCMCIA controller need some more work, it uses a PCI1225 of Texas Instruments. Any help with this is appreciated.
I think you might get more help with a more specific request. "Needs some more work" is very generic.
Thanks, Myles
On Mon, 2010-05-24 at 09:51 -0600, Myles Watson wrote:
Signed-off-by: Marc Bertens mbertens@xs4all.nl
Hello all,
Here is the update for the Nokia IP530 system. it has now working NICs and correct PIRQ table. corrections where made to serveral files.
Congratulations on getting it to work.
Could you send a patch against the current revision? The IP530 is already in the tree.
How can i do that i did the diff, and was expecting that is was agains the current revision.
For your updated patch I'd suggest not including asus_p2b_irq_tables.c I'd also look into the comments that Stefan made about devicetree.cb
i will remove it
Currently the PCMCIA controller need some more work, it uses a PCI1225 of Texas Instruments. Any help with this is appreciated.
At the moment i have no idea where to start with it, so i need someone who can help me with this. I will read the specification for PCMCIA and CardBus. But that will be later on. Currently other things to work on :-)
I think you might get more help with a more specific request. "Needs some more work" is very generic.
Thanks, Myles
Here is the update for the Nokia IP530 system. it has now working NICs and correct PIRQ table. corrections where made to serveral files.
Congratulations on getting it to work.
Could you send a patch against the current revision? The IP530 is already in the tree.
How can i do that i did the diff, and was expecting that is was agains the current revision.
svn info (you should be at revision 5583) svn up (updates to the latest) svn diff
For your updated patch I'd suggest not including asus_p2b_irq_tables.c I'd also look into the comments that Stefan made about devicetree.cb
i will remove it
Currently the PCMCIA controller need some more work, it uses a PCI1225 of Texas Instruments. Any help with this is appreciated.
At the moment i have no idea where to start with it, so i need someone who can help me with this. I will read the specification for PCMCIA and CardBus. But that will be later on. Currently other things to work on :-)
A description of how it's failing and a bootlog is probably the place to start. You could also look at Stefan's support for TI xxx12 cardbus devices.
Thanks, Myles
Signed-off Marc Bertens mbertens@xs4all.nl
On Mon, 2010-05-24 at 12:34 -0600, Myles Watson wrote:
Here is the update for the Nokia IP530 system. it has now working NICs and correct PIRQ table. corrections where made to serveral files.
Congratulations on getting it to work.
Could you send a patch against the current revision? The IP530 is already in the tree.
How can i do that i did the diff, and was expecting that is was agains the current revision.
svn info (you should be at revision 5583) svn up (updates to the latest) svn diff
Ok done that all, and here is my dff for it
For your updated patch I'd suggest not including asus_p2b_irq_tables.c I'd also look into the comments that Stefan made about devicetree.cb
i will remove it
Currently the PCMCIA controller need some more work, it uses a PCI1225 of Texas Instruments. Any help with this is appreciated.
At the moment i have no idea where to start with it, so i need someone who can help me with this. I will read the specification for PCMCIA and CardBus. But that will be later on. Currently other things to work on :-)
A description of how it's failing and a bootlog is probably the place to start. You could also look at Stefan's support for TI xxx12 cardbus devices.
Thanks, Myles
For me the comments are important at the moment, to remember who things work and why i did certain thing to the code. So for now they are still there, sorry about that. But i dont know how to strip the comments from the diff without removing then from my code.
Marc
-----Original Message----- From: mbertens [mailto:mbertens@xs4all.nl] Sent: Tuesday, May 25, 2010 10:21 AM To: Myles Watson Cc: Coreboot mailinglist Subject: Re: [coreboot] Patch for Nokia-IP530, now with working PIRQ table
Signed-off Marc Bertens mbertens@xs4all.nl
+<<<<<<< .mine +config VENDOR_NOKIA + bool "Nokia" +======= config VENDOR_WYSE bool "Wyse" +>>>>>>> .r5583
This means that there was a conflict. When your patch is ready there shouldn't be any conflicts, and it should be obvious why you're making the changes. One thing that might help you would be to check out another copy of coreboot and test patches on that unmodified repository.
Thanks, Myles
I made a new diff, please check it, if there is still something not correct please let me known, then ill fix the problem.
Marc
On Tue, 2010-05-25 at 10:34 -0600, Myles Watson wrote:
-----Original Message----- From: mbertens [mailto:mbertens@xs4all.nl] Sent: Tuesday, May 25, 2010 10:21 AM To: Myles Watson Cc: Coreboot mailinglist Subject: Re: [coreboot] Patch for Nokia-IP530, now with working PIRQ table
Signed-off Marc Bertens mbertens@xs4all.nl
+<<<<<<< .mine +config VENDOR_NOKIA
- bool "Nokia"
+======= config VENDOR_WYSE bool "Wyse" +>>>>>>> .r5583
This means that there was a conflict. When your patch is ready there shouldn't be any conflicts, and it should be obvious why you're making the changes. One thing that might help you would be to check out another copy of coreboot and test patches on that unmodified repository.
Thanks, Myles
Still wrong, i saw of later comments in the mail that i need to fix, so disregard my previous message.
On Tue, 2010-05-25 at 23:02 +0200, mbertens wrote:
I made a new diff, please check it, if there is still something not correct please let me known, then ill fix the problem.
Marc
On Tue, 2010-05-25 at 10:34 -0600, Myles Watson wrote:
-----Original Message----- From: mbertens [mailto:mbertens@xs4all.nl] Sent: Tuesday, May 25, 2010 10:21 AM To: Myles Watson Cc: Coreboot mailinglist Subject: Re: [coreboot] Patch for Nokia-IP530, now with working PIRQ table
Signed-off Marc Bertens mbertens@xs4all.nl
+<<<<<<< .mine +config VENDOR_NOKIA
- bool "Nokia"
+======= config VENDOR_WYSE bool "Wyse" +>>>>>>> .r5583
This means that there was a conflict. When your patch is ready there shouldn't be any conflicts, and it should be obvious why you're making the changes. One thing that might help you would be to check out another copy of coreboot and test patches on that unmodified repository.
Thanks, Myles
-- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Ok, new try
Please check it, if there is still something not correct please let me known, then ill fix the problem.
Thanks
Marc
On Tue, 2010-05-25 at 23:05 +0200, mbertens wrote:
Still wrong, i saw of later comments in the mail that i need to fix, so disregard my previous message.
On Tue, 2010-05-25 at 23:02 +0200, mbertens wrote:
I made a new diff, please check it, if there is still something not correct please let me known, then ill fix the problem.
Marc
On Tue, 2010-05-25 at 10:34 -0600, Myles Watson wrote:
-----Original Message----- From: mbertens [mailto:mbertens@xs4all.nl] Sent: Tuesday, May 25, 2010 10:21 AM To: Myles Watson Cc: Coreboot mailinglist Subject: Re: [coreboot] Patch for Nokia-IP530, now with working PIRQ table
Signed-off Marc Bertens mbertens@xs4all.nl
+<<<<<<< .mine +config VENDOR_NOKIA
- bool "Nokia"
+======= config VENDOR_WYSE bool "Wyse" +>>>>>>> .r5583
This means that there was a conflict. When your patch is ready there shouldn't be any conflicts, and it should be obvious why you're making the changes. One thing that might help you would be to check out another copy of coreboot and test patches on that unmodified repository.
Thanks, Myles
-- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Signed-off Marc Bertens mbertens@xs4all.nl
In my earlier mail i forgot one file to patch.
On Tue, 2010-05-25 at 23:22 +0200, mbertens wrote:
Ok, new try
Please check it, if there is still something not correct please let me known, then ill fix the problem.
Thanks
Marc
On Tue, 2010-05-25 at 23:05 +0200, mbertens wrote:
Still wrong, i saw of later comments in the mail that i need to fix, so disregard my previous message.
On Tue, 2010-05-25 at 23:02 +0200, mbertens wrote:
I made a new diff, please check it, if there is still something not correct please let me known, then ill fix the problem.
Marc
On Tue, 2010-05-25 at 10:34 -0600, Myles Watson wrote:
-----Original Message----- From: mbertens [mailto:mbertens@xs4all.nl] Sent: Tuesday, May 25, 2010 10:21 AM To: Myles Watson Cc: Coreboot mailinglist Subject: Re: [coreboot] Patch for Nokia-IP530, now with working PIRQ table
Signed-off Marc Bertens mbertens@xs4all.nl
+<<<<<<< .mine +config VENDOR_NOKIA
- bool "Nokia"
+======= config VENDOR_WYSE bool "Wyse" +>>>>>>> .r5583
This means that there was a conflict. When your patch is ready there shouldn't be any conflicts, and it should be obvious why you're making the changes. One thing that might help you would be to check out another copy of coreboot and test patches on that unmodified repository.
Thanks, Myles
-- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
-- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
+ // fix made by Marc Bertens mbertens@xs4all.nl + if (link > 0x5f) { + // This is basically for the 440BX + link -= 0x5f; + }
I'd prefer this to be guarded by #if CONFIG_NORTHBRIDGE_INTEL_440BX (or whatever the correct one is)
It would also be nice to have an explanation.
The rest of your patch touches a lot of code with little explanation. It takes a lot more time to review patches like that. For a faster review you should split it up into pieces that add functionality. For example, the heap size (which seems really large) part of the patch should have an explanation of what problem you see with a normal heap size.
Thanks, Myles
On Wed, 2010-05-26 at 11:00 -0600, Myles Watson wrote:
// fix made by Marc Bertens <mbertens@xs4all.nl>if (link > 0x5f) {// This is basically for the 440BXlink -= 0x5f;}I'd prefer this to be guarded by #if CONFIG_NORTHBRIDGE_INTEL_440BX (or whatever the correct one is)
I was thinking of it to put it that way, but i'd. But i will make the changes to the code.
It would also be nice to have an explanation.
And give more explaination why the change was made.
The rest of your patch touches a lot of code with little explanation. It takes a lot more time to review patches like that. For a faster review you should split it up into pieces that add functionality. For example, the
I will give the patches in seperate diffs, with more explainations
heap size (which seems really large) part of the patch should have an explanation of what problem you see with a normal heap size.
I was running in to problems with the heap size, therefor i increased it to such a value that it would not bother me again :-). I will decrease the value for it to see on which value it needs to be.
Thanks, Myles
This is my first attempt to develop in an open source environment. And i'm still learning things ie "the coding standard", i hope that i'm not to much trouble, i will get it right one day :-)
Marc
On Wed, May 26, 2010 at 2:15 PM, mbertens mbertens@xs4all.nl wrote:
On Wed, 2010-05-26 at 11:00 -0600, Myles Watson wrote:
- // fix made by Marc Bertens mbertens@xs4all.nl
- if (link > 0x5f) {
- // This is basically for the 440BX
- link -= 0x5f;
- }
I'd prefer this to be guarded by #if CONFIG_NORTHBRIDGE_INTEL_440BX (or whatever the correct one is)
I was thinking of it to put it that way, but i'd. But i will make the changes to the code.
It would also be nice to have an explanation.
And give more explaination why the change was made.
The rest of your patch touches a lot of code with little explanation. It takes a lot more time to review patches like that. For a faster review you should split it up into pieces that add functionality. For example, the
I will give the patches in seperate diffs, with more explainations
Great.
heap size (which seems really large) part of the patch should have an explanation of what problem you see with a normal heap size.
I was running in to problems with the heap size, therefor i increased it to such a value that it would not bother me again :-). I will decrease the value for it to see on which value it needs to be.
The reason why we care is that usually your device tree is wrong if you run out of heap size. We'd rather fix the root problem.
This is my first attempt to develop in an open source environment. And i'm still learning things ie "the coding standard", i hope that i'm not to much trouble, i will get it right one day :-)
It's not too much trouble. Thanks for contributing.
Thanks, Myles
Signed-off-by: Marc Bertens mbertens@xs4all.nl
Hi all,
Here is the patch just for the pirq_routing() function. Its made specific to the CONFIG_NORTHBRIDGE_INTEL_440BX if that is to generic please replace by CONFIG_BOARD_NOKIA_IP530.
Put the following extras in the file; - added header accordingly "Common License Header" that was missing. - corrected a printk() warning of the compiler. - added the correction for the i440BX by AND the link value with 0x5F so that always the value is kept below 0x5F. That AND value should be 0x03 i think because the link value cannot be greater than 3. But i'm not sure about that, thats why i used the current solution.
The other patches will follow to day.
Marc
On Wed, 2010-05-26 at 15:14 -0600, Myles Watson wrote:
On Wed, May 26, 2010 at 2:15 PM, mbertens mbertens@xs4all.nl wrote:
On Wed, 2010-05-26 at 11:00 -0600, Myles Watson wrote:
// fix made by Marc Bertens <mbertens@xs4all.nl>if (link > 0x5f) {// This is basically for the 440BXlink -= 0x5f;}I'd prefer this to be guarded by #if CONFIG_NORTHBRIDGE_INTEL_440BX (or whatever the correct one is)
I was thinking of it to put it that way, but i'd. But i will make the changes to the code.
Signed-off-by: Marc Bertens mbertens@xs4all.nl
Part two now the PIRQ table for the Nokia IP530.
Changes made: - rebuild the table to get th eNICs and PCMCIA controllers get there correct IRQs assigned. - Added a stub pirq_assign_irqs() due to the fact that it was missing at compilation time. This was discussed with Rudolf Marek (ruik) r.marek@assembler.cz
Marc
On Thu, 2010-05-27 at 11:56 +0200, mbertens wrote:
Hi all,
Here is the patch just for the pirq_routing() function. Its made specific to the CONFIG_NORTHBRIDGE_INTEL_440BX if that is to generic please replace by CONFIG_BOARD_NOKIA_IP530.
Put the following extras in the file;
- added header accordingly "Common License Header" that was missing.
- corrected a printk() warning of the compiler.
- added the correction for the i440BX by AND the link value with 0x5F
so that always the value is kept below 0x5F. That AND value should be 0x03 i think because the link value cannot be greater than 3. But i'm not sure about that, thats why i used the current solution.
The other patches will follow to day.
Marc
On Wed, 2010-05-26 at 15:14 -0600, Myles Watson wrote:
On Wed, May 26, 2010 at 2:15 PM, mbertens mbertens@xs4all.nl wrote:
On Wed, 2010-05-26 at 11:00 -0600, Myles Watson wrote:
// fix made by Marc Bertens <mbertens@xs4all.nl>if (link > 0x5f) {// This is basically for the 440BXlink -= 0x5f;}I'd prefer this to be guarded by #if CONFIG_NORTHBRIDGE_INTEL_440BX (or whatever the correct one is)
I was thinking of it to put it that way, but i'd. But i will make the changes to the code.
Signed-off-by: Marc Bertens mbertens@xs4all.nl
This is the patch for Kconfig, i have taken all unnessarry things out - back to default HEAP size - correct settings for the PIRQ table - with comments
Marc
On Thu, 2010-05-27 at 12:14 +0200, mbertens wrote:
Part two now the PIRQ table for the Nokia IP530.
Changes made:
- rebuild the table to get th eNICs and PCMCIA controllers get there
correct IRQs assigned.
- Added a stub pirq_assign_irqs() due to the fact that it was missing at
compilation time. This was discussed with Rudolf Marek (ruik) r.marek@assembler.cz
Marc
On Thu, 2010-05-27 at 11:56 +0200, mbertens wrote:
Hi all,
Here is the patch just for the pirq_routing() function. Its made specific to the CONFIG_NORTHBRIDGE_INTEL_440BX if that is to generic please replace by CONFIG_BOARD_NOKIA_IP530.
Put the following extras in the file;
- added header accordingly "Common License Header" that was missing.
- corrected a printk() warning of the compiler.
- added the correction for the i440BX by AND the link value with 0x5F
so that always the value is kept below 0x5F. That AND value should be 0x03 i think because the link value cannot be greater than 3. But i'm not sure about that, thats why i used the current solution.
The other patches will follow to day.
Marc
On Wed, 2010-05-26 at 15:14 -0600, Myles Watson wrote:
On Wed, May 26, 2010 at 2:15 PM, mbertens mbertens@xs4all.nl wrote:
On Wed, 2010-05-26 at 11:00 -0600, Myles Watson wrote:
// fix made by Marc Bertens <mbertens@xs4all.nl>if (link > 0x5f) {// This is basically for the 440BXlink -= 0x5f;}I'd prefer this to be guarded by #if CONFIG_NORTHBRIDGE_INTEL_440BX (or whatever the correct one is)
I was thinking of it to put it that way, but i'd. But i will make the changes to the code.
Signed-off-by: Marc Bertens mbertens@xs4all.nl
Part four for the Nokia IP530, This is the patch for hte devcetree.cb file. Following changes where made; - Disabled the floppy controller, due to the fact that its not present on the board. - Disabled tie parallel port, due to the fact that its not present on the board. - taken out the io, irq config of device 7 (PS2 keyb/mouse) it had not effect anyways, and the controller is not used anyway, but it is physical present on the board as a 4 pin sil header. - Disabled the APCI of the superio, due to the fact that currently no ACPI is supported by the current configuration of the Nokia IP530. - Disabled the ACPI device of the southbridge, as above. - Disabled the UBE device of the southbridge, as it has no connector on the board. - Set the ideX_driveY_udma33_enable to 1 to enable the UDMA33 option of the southbridge (was tested with a 80Gb-WD and a CF 1Gb).
Remark: currently the CF that i used has the problem with DMA, and therefor it takes Linux about 120 sec extra to boot. On a asus P4B (orginal bios) this problem was not encountered. So therefor there must be something be done extra in coreboot to fix that problem.
Marc
On Thu, 2010-05-27 at 12:30 +0200, mbertens wrote
This is the patch for Kconfig, i have taken all unnessarry things out
- back to default HEAP size
- correct settings for the PIRQ table
- with comments
Marc
On Thu, 2010-05-27 at 12:14 +0200, mbertens wrote:
Part two now the PIRQ table for the Nokia IP530.
Changes made:
- rebuild the table to get th eNICs and PCMCIA controllers get there
correct IRQs assigned.
- Added a stub pirq_assign_irqs() due to the fact that it was missing at
compilation time. This was discussed with Rudolf Marek (ruik) r.marek@assembler.cz
Marc
On Thu, 2010-05-27 at 11:56 +0200, mbertens wrote:
Hi all,
Here is the patch just for the pirq_routing() function. Its made specific to the CONFIG_NORTHBRIDGE_INTEL_440BX if that is to generic please replace by CONFIG_BOARD_NOKIA_IP530.
Put the following extras in the file;
- added header accordingly "Common License Header" that was missing.
- corrected a printk() warning of the compiler.
- added the correction for the i440BX by AND the link value with 0x5F
so that always the value is kept below 0x5F. That AND value should be 0x03 i think because the link value cannot be greater than 3. But i'm not sure about that, thats why i used the current solution.
The other patches will follow to day.
Marc
On Wed, 2010-05-26 at 15:14 -0600, Myles Watson wrote:
On Wed, May 26, 2010 at 2:15 PM, mbertens mbertens@xs4all.nl wrote:
On Wed, 2010-05-26 at 11:00 -0600, Myles Watson wrote:
// fix made by Marc Bertens <mbertens@xs4all.nl>if (link > 0x5f) {// This is basically for the 440BXlink -= 0x5f;}I'd prefer this to be guarded by #if CONFIG_NORTHBRIDGE_INTEL_440BX (or whatever the correct one is)
I was thinking of it to put it that way, but i'd. But i will make the changes to the code.
But now with the attached file.
On Thu, 2010-05-27 at 12:14 +0200, mbertens wrote:
Signed-off-by: Marc Bertens mbertens@xs4all.nl
Part two now the PIRQ table for the Nokia IP530.
Changes made:
- rebuild the table to get th eNICs and PCMCIA controllers get there
correct IRQs assigned.
- Added a stub pirq_assign_irqs() due to the fact that it was missing at
compilation time. This was discussed with Rudolf Marek (ruik) r.marek@assembler.cz
Marc
On Thu, 2010-05-27 at 11:56 +0200, mbertens wrote:
Hi all,
Here is the patch just for the pirq_routing() function. Its made specific to the CONFIG_NORTHBRIDGE_INTEL_440BX if that is to generic please replace by CONFIG_BOARD_NOKIA_IP530.
Put the following extras in the file;
- added header accordingly "Common License Header" that was missing.
- corrected a printk() warning of the compiler.
- added the correction for the i440BX by AND the link value with 0x5F
so that always the value is kept below 0x5F. That AND value should be 0x03 i think because the link value cannot be greater than 3. But i'm not sure about that, thats why i used the current solution.
The other patches will follow to day.
Marc
On Wed, 2010-05-26 at 15:14 -0600, Myles Watson wrote:
On Wed, May 26, 2010 at 2:15 PM, mbertens mbertens@xs4all.nl wrote:
On Wed, 2010-05-26 at 11:00 -0600, Myles Watson wrote:
// fix made by Marc Bertens <mbertens@xs4all.nl>if (link > 0x5f) {// This is basically for the 440BXlink -= 0x5f;}I'd prefer this to be guarded by #if CONFIG_NORTHBRIDGE_INTEL_440BX (or whatever the correct one is)
I was thinking of it to put it that way, but i'd. But i will make the changes to the code.
mbertens wrote:
Here is the patch just for the pirq_routing() function. Its made specific to the CONFIG_NORTHBRIDGE_INTEL_440BX if that is to generic please replace by CONFIG_BOARD_NOKIA_IP530.
_440BX is good, but there are still a few issues.
- added the correction for the i440BX by AND the link value with 0x5F
so that always the value is kept below 0x5F. That AND value should be 0x03 i think because the link value cannot be greater than 3. But i'm not sure about that, thats why i used the current solution.
This is different from my code in your last patch. Why did it change? I'd love to know if there was a problem with using subtraction, in order to understand the work you and others are doing.
My original if(link>0x5f) link-=0x5f; is a really crude hack that makes some wild assumptions in order to translate data at hand into data that is needed. I think this must not be coded into coreboot in this particular manner.
+++ src/arch/i386/boot/pirq_routing.c (working copy)
- printk(BIOS_INFO, "Verifing copy of Interrupt Routing Table at 0x%08x... ", addr);
- /**
* fix made by Marc Bertens <mbertens@xs4all.nl>** The compiler was putting out a warning that the 'addr' value* was of the unsigned int long type but the printk() was using '%08x'*/- printk(BIOS_INFO, "Verifing copy of Interrupt Routing Table at 0x%08lx... ", addr);
These comments are not ok. Please consider what the source code would look like if every commit that touched something small like a printk argument came with 6 lines of comments.
Also, don't try to document who made changes to the code *within the code* because it will not work over time and more importantly because the source control system that we are using (subversion) keeps track of this for us! You sign-off, someone commits, the commit gets a revision number, SVN logs the commit message (which includes sign-off) and SVN can also find the revision number that is responsible for every line of source code in the project, so there is already perfect traceability both to the commiter (svn username) and to the original author(s) (Signed-off-by) without trying to store that in comments in the code.
Finally, comments about changes that have been made go into the commit message and not into a comment in the source code.
Documentation in comments is not at all forbidden, but I certainly think that it's important to not have *too many* comments in the code either. Some code certainly deserves to be commented, but not all, then it is probably so weird that it should be reworked a bit to be clear also without comments.
@@ -121,7 +147,24 @@
printk(BIOS_DEBUG, "INT: %c link: %x bitmap: %x ", 'A' + j, link, bitmap);
+#if CONFIG_NORTHBRIDGE_INTEL_I440BX == 1
/*** fix made by Marc Bertens <mbertens@xs4all.nl>** this was done for the Northbridge i440BX, due to the fact* that the values in the PIRQ table needs to be 60, 61, 62* and 63. This was passed to me by Idwer Vollering <vidwer@gmail.com>* and Peter Stuge <peter@stuge.se> helped with this* fix, so that the IRQ routing is done.*/if (link > 0x5f) {/*** as if the maximum value can be 0x5F we should* AND it instead of substracting, my opinion.*/link &= 0x5F;}+#endif
NAK for this. Even if it works, it's not the right way to solve this problem. I don't think this can be fixed without some more investigation and possibly central changes in coreboot. I don't think they will be very large changes, but still.
//Peter
On Thu, 2010-05-27 at 20:32 +0200, Peter Stuge wrote:
mbertens wrote:
Here is the patch just for the pirq_routing() function. Its made specific to the CONFIG_NORTHBRIDGE_INTEL_440BX if that is to generic please replace by CONFIG_BOARD_NOKIA_IP530.
_440BX is good, but there are still a few issues.
- added the correction for the i440BX by AND the link value with 0x5F
so that always the value is kept below 0x5F. That AND value should be 0x03 i think because the link value cannot be greater than 3. But i'm not sure about that, thats why i used the current solution.
This is different from my code in your last patch. Why did it change? I'd love to know if there was a problem with using subtraction, in order to understand the work you and others are doing.
I changed it because i saw some implementations that have even higher values in the PIRQ table (above 0x63) and i think that the link value can never be above 3 anyway (0..3), so for that the the AND value should even 0x03, to enforce that.
My original if(link>0x5f) link-=0x5f; is a really crude hack that makes some wild assumptions in order to translate data at hand into data that is needed. I think this must not be coded into coreboot in this particular manner.
Correct but for now until a better solution is there i think we should put it in, other i440BX users should have the same type of problem that i've had.
+++ src/arch/i386/boot/pirq_routing.c (working copy)
- printk(BIOS_INFO, "Verifing copy of Interrupt Routing Table at 0x%08x... ", addr);
- /**
* fix made by Marc Bertens <mbertens@xs4all.nl>** The compiler was putting out a warning that the 'addr' value* was of the unsigned int long type but the printk() was using '%08x'*/- printk(BIOS_INFO, "Verifing copy of Interrupt Routing Table at 0x%08lx... ", addr);
These comments are not ok. Please consider what the source code would look like if every commit that touched something small like a printk argument came with 6 lines of comments.
Also, don't try to document who made changes to the code *within the code* because it will not work over time and more importantly because the source control system that we are using (subversion) keeps track of this for us! You sign-off, someone commits, the commit gets a revision number, SVN logs the commit message (which includes sign-off) and SVN can also find the revision number that is responsible for every line of source code in the project, so there is already perfect traceability both to the commiter (svn username) and to the original author(s) (Signed-off-by) without trying to store that in comments in the code.
Ok i did'nt know that, i was doing this kind of comments all the time in my own projects and i was just trying to explain why i did the fix. there.
Finally, comments about changes that have been made go into the commit message and not into a comment in the source code.
Documentation in comments is not at all forbidden, but I certainly think that it's important to not have *too many* comments in the code either. Some code certainly deserves to be commented, but not all, then it is probably so weird that it should be reworked a bit to be clear also without comments.
For understanding the code i think its quiet nice to have a lot of comments, what the programmer is doing. I find it conviniant to put in information about specicifations and that kind of thing. But if you think its better to keep the comments short no problem i will take it out.
@@ -121,7 +147,24 @@
printk(BIOS_DEBUG, "INT: %c link: %x bitmap: %x ", 'A' + j, link, bitmap);
+#if CONFIG_NORTHBRIDGE_INTEL_I440BX == 1
/*** fix made by Marc Bertens <mbertens@xs4all.nl>** this was done for the Northbridge i440BX, due to the fact* that the values in the PIRQ table needs to be 60, 61, 62* and 63. This was passed to me by Idwer Vollering <vidwer@gmail.com>* and Peter Stuge <peter@stuge.se> helped with this* fix, so that the IRQ routing is done.*/if (link > 0x5f) {/*** as if the maximum value can be 0x5F we should* AND it instead of substracting, my opinion.*/link &= 0x5F;}+#endif
NAK for this. Even if it works, it's not the right way to solve this problem. I don't think this can be fixed without some more investigation and possibly central changes in coreboot. I don't think they will be very large changes, but still.
Even so that this is not the way to do it correctly, i like to put it in for now. By doing it this way basically nobody should have a problem with this addition. And i like to solve it the correct way, but i started two months ago with de coreboot project (and i like it alot) and overhauling the code for the i440BX is just a little to abitieus at the moment. Certainly due to that there are many boards with this chipset.
//Peter
And finally i like to keep working on this and fixing this at a later time so that it fits all. I dont think its handy solution that i need to keep a patch local to put this every time in when the code of the pirq_routing object changes. And others cannot benefit from this and they need to keep this patch locally too.
Let me know what you think, then i'll make a new patch with less comments. And the part of adjusting the link value for the i440BX will be a TODO comment.
Marc
mbertens wrote:
This is my first attempt to develop in an open source environment.
Cool! :)
And i'm still learning things ie "the coding standard",
I guess you already saw the http://www.coreboot.org/Development_Guidelines wiki page. It's not neccessarily complete, so please point out things that you encounter that seem to be missing from the page, so that they can be added.
i hope that i'm not to much trouble, i will get it right one day :-)
I don't think it'll take very long. :)
//Peter
On Thu, 2010-05-27 at 02:31 +0200, Peter Stuge wrote:
mbertens wrote:
This is my first attempt to develop in an open source environment.
Cool! :)
And i'm still learning things ie "the coding standard",
I guess you already saw the http://www.coreboot.org/Development_Guidelines wiki page. It's not neccessarily complete, so please point out things that you encounter that seem to be missing from the page, so that they can be added.
Yes i was reading that, i have to dislearn what i'v learned :-)
i hope that i'm not to much trouble, i will get it right one day :-)
I don't think it'll take very long. :)
//Peter
Marc
mbertens wrote:
+++ src/arch/i386/boot/pirq_routing.c (working copy) @@ -121,7 +121,11 @@
printk(BIOS_DEBUG, "INT: %c link: %x bitmap: %x ", 'A' + j, link, bitmap);
// fix made by Marc Bertens <mbertens@xs4all.nl>if (link > 0x5f) {// This is basically for the 440BXlink -= 0x5f;}
Um, except that I gave you that code.
It is a bit of a hack. So far this file and function is actually only called by Geode LX mainboards, and I don't quite understand why.
Maybe something equivalent of this code is invoked in other ways in all the other boards? The gist of pirq_routing_irqs() is to call pci_assign_irqs() and pirq_assign_irqs(). How
There are various other solutions to calling pci_assign_irqs() on other boards:
northbridge/via/cx700/cx700_lpc.c northbridge/via/vx800/vx800_lpc.c southbridge/via/vt8231/vt8231_lpc.c southbridge/via/vt8235/vt8235_lpc.c southbridge/via/vt8237r/vt8237r_lpc.c mainboard/amd/serengeti/cheetah/irq_tables.c mainboard/msi/ms7135/irq_tables.c mainboard/olpc/btest/mainboard.c mainboard/tyan/s2882/irq_tables.c mainboard/iwill/dk8_htx/irq_tables.c mainboard/emulation/qemu-x86/mainboard.c mainboard/technologic/ts5300/mainboard.c mainboard/digitallogic/msm586seg/mainboard.c
I have fairly strong opinion about those call sites being so different (maybe due to ignorance) but more importantly what are the other boards doing? There seems to be a large number of non-Geode systems not in the list above where pci_assign_irqs() is never called. The doxygen for the function says:
"This function should be called for each PCI slot in your system."
What gives? Can we find a generic solution to this interrupt problem?
//Peter
Peter Stuge wrote:
I have fairly strong opinion about those call sites being so different (maybe due to ignorance)
*my* ignorance
//Peter
On Thu, 2010-05-27 at 02:27 +0200, Peter Stuge wrote:
mbertens wrote:
+++ src/arch/i386/boot/pirq_routing.c (working copy) @@ -121,7 +121,11 @@
printk(BIOS_DEBUG, "INT: %c link: %x bitmap: %x ", 'A' + j, link, bitmap);
// fix made by Marc Bertens <mbertens@xs4all.nl>if (link > 0x5f) {// This is basically for the 440BXlink -= 0x5f;}Um, except that I gave you that code.
Yes, that was already in the new comment, that it was passed to me by you.
It is a bit of a hack. So far this file and function is actually only called by Geode LX mainboards, and I don't quite understand why.
It just works fne for the i440BX, but you need a stub function pirq_assign_irqs() in the board specific codebase (my solution).
Maybe something equivalent of this code is invoked in other ways in all the other boards? The gist of pirq_routing_irqs() is to call pci_assign_irqs() and pirq_assign_irqs(). How
pirq_routing_irqs() calls as the last action pirq_assign_irqs()
There are various other solutions to calling pci_assign_irqs() on other boards:
northbridge/via/cx700/cx700_lpc.c northbridge/via/vx800/vx800_lpc.c southbridge/via/vt8231/vt8231_lpc.c southbridge/via/vt8235/vt8235_lpc.c southbridge/via/vt8237r/vt8237r_lpc.c mainboard/amd/serengeti/cheetah/irq_tables.c mainboard/msi/ms7135/irq_tables.c mainboard/olpc/btest/mainboard.c mainboard/tyan/s2882/irq_tables.c mainboard/iwill/dk8_htx/irq_tables.c mainboard/emulation/qemu-x86/mainboard.c mainboard/technologic/ts5300/mainboard.c mainboard/digitallogic/msm586seg/mainboard.c
I looked at all those you meantioned here, and soem of them are quite similar. Maybe we can find a common solution "one size fits all", i like sourcecode that can handle all the different board types.
I have fairly strong opinion about those call sites being so different (maybe due to ignorance) but more importantly what are the other boards doing? There seems to be a large number of non-Geode systems not in the list above where pci_assign_irqs() is never called. The doxygen for the function says:
"This function should be called for each PCI slot in your system."
What gives? Can we find a generic solution to this interrupt problem?
First thing is that we need to identify the different board solutions which are similar, and see what kind of solution groups we have at the moment, and maybe its just enough to have a adjuster function for a specific board, which is called start of the pirq_routing_irqs(), and when no specific adjusting is required this can be left out by Kconfig and the same applies to the pirq_assign_irqs()
//Peter
Marc
mbertens wrote:
It is a bit of a hack. So far this file and function is actually only called by Geode LX mainboards, and I don't quite understand why.
It just works fne for the i440BX, but you need a stub function pirq_assign_irqs() in the board specific codebase (my solution).
Sure, but I'm talking about the nearly 200 boards in coreboot which do not use this code but still work. I want some comments from people who did some other board ports.
How does 945 handle IRQ assignment? The other 440BX boards? And how about the 830 systems, or ServerWorks, or RS690/SB600?
//Peter
On 5/27/10 9:08 AM, Peter Stuge wrote:
mbertens wrote:
It is a bit of a hack. So far this file and function is actually only called by Geode LX mainboards, and I don't quite understand why.
It just works fne for the i440BX, but you need a stub function pirq_assign_irqs() in the board specific codebase (my solution).
Sure, but I'm talking about the nearly 200 boards in coreboot which do not use this code but still work. I want some comments from people who did some other board ports.
How does 945 handle IRQ assignment? The other 440BX boards? And how about the 830 systems, or ServerWorks, or RS690/SB600?
//Peter
Mapping of 8259 interrupts: http://tracker.coreboot.org/trac/coreboot/browser/trunk/src/mainboard/kontro... http://tracker.coreboot.org/trac/coreboot/browser/trunk/src/southbridge/inte...
PIRQ: http://tracker.coreboot.org/trac/coreboot/browser/trunk/src/mainboard/kontro...
MP: http://tracker.coreboot.org/trac/coreboot/browser/trunk/src/mainboard/kontro...
ACPI: http://tracker.coreboot.org/trac/coreboot/browser/trunk/src/mainboard/kontro... http://tracker.coreboot.org/trac/coreboot/browser/trunk/src/mainboard/kontro...
Marc,
something went very wrong with this patch. Please try again. See comments below.
On 5/25/10 6:20 PM, mbertens wrote:
Index: src/mainboard/Kconfig
--- src/mainboard/Kconfig (revision 5583) +++ src/mainboard/Kconfig (working copy) @@ -104,8 +104,13 @@ bool "VIA" config VENDOR_WINENT bool "Win Enterprises" +<<<<<<< .mine +config VENDOR_NOKIA
- bool "Nokia"
+======= config VENDOR_WYSE bool "Wyse" +>>>>>>> .r5583
Conflicts in this file... The whole changes in this file can be dropped, they have been merged already. Please be more thorough when creating patch files. This doesn't even survive "make menuconfig".
Index: src/mainboard/nokia/ip530/Kconfig
--- src/mainboard/nokia/ip530/Kconfig (revision 5583) +++ src/mainboard/nokia/ip530/Kconfig (working copy) @@ -5,7 +5,8 @@
+## Based on romstage.c from src/mainboard/asus/p2b +##
That makes no sense, how can you base Kconfig on a romstage.c file?
config BOARD_NOKIA_IP530 bool "IP530" select ARCH_X86 @@ -26,8 +28,10 @@ select SUPERIO_SMSC_SMSCSUPERIO select ROMCC select HAVE_PIRQ_TABLE
- select PIRQ_ROUTE select UDELAY_TSC
- select BOARD_ROMSIZE_KB_256
+# select HAVE_ACPI_TABLES +# select HAVE_SMI_HANDLER
I think these two should go away
config MAINBOARD_DIR string @@ -49,3 +53,27 @@ default 6 depends on BOARD_NOKIA_IP530
+config CONFIG_IRQ_SLOT_COUNT
- int
- default 1
- depends on BOARD_NOKIA_IP530
Only one entry? Also, that entry already exists. Take care that you don't add duplicate entries. Also, CONFIG_ must be omitted in Kconfig
+config CONFIG_GENERATE_PIRQ_TABLE
- int
- default 1
- depends on BOARD_NOKIA_IP530
CONFIG_ must be omitted in Kconfig. This variable is a "bool", so it should be "select"ed above
+config HEAP_SIZE
- hex
- default 0x60000
- depends on BOARD_NOKIA_IP530
Very bad. Why?
+config CONFIG_PIRQ_ROUTE
- int
- default 1
- depends on BOARD_NOKIA_IP530
CONFIG_ must be omitted in Kconfig. same variable is "select"ed above
+config CONFIG_DEBUG
- int
- default 1
- depends on BOARD_NOKIA_IP530
CONFIG_ must be omitted in Kconfig.
Index: src/mainboard/nokia/ip530/romstage.c
--- src/mainboard/nokia/ip530/romstage.c (revision 5583) +++ src/mainboard/nokia/ip530/romstage.c (working copy) @@ -18,6 +18,8 @@
- Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/
+/* Based on romstage.c from src/mainboard/asus/p2b */
This comment is not needed nor helpful
#include <stdint.h> #include <device/pci_def.h> #include <arch/io.h> @@ -25,7 +27,8 @@ #include <arch/romcc_io.h> #include <arch/hlt.h> #include <stdlib.h> -#include <console/console.h> +#include "pc80/serial.c" +#include "console/console.c"
This just backs out a fix that went in. Please be more thorough when creating patch files. This doesn't even survive "make".
#include "lib/ramtest.c" #include "southbridge/intel/i82371eb/i82371eb_enable_rom.c" #include "southbridge/intel/i82371eb/i82371eb_early_smbus.c" @@ -37,33 +40,35 @@ #include "cpu/x86/bist.h" #include "superio/smsc/smscsuperio/smscsuperio_early_serial.c"
-#define SERIAL_DEV PNP_DEV(0x3f0, SMSCSUPERIO_SP1)
+#define SERIAL_DEV PNP_DEV( 0x3f0, SMSCSUPERIO_SP1 )
Unnecessary white space changes against coding conventions.
+//----------------------------------------------------------------------------- static inline int spd_read_byte(unsigned int device, unsigned int address) { return smbus_read_byte(device, address); }
+//----------------------------------------------------------------------------- #include "northbridge/intel/i440bx/raminit.c" #include "northbridge/intel/i440bx/debug.c"
+//----------------------------------------------------------------------------- static void main(unsigned long bist) {
- if (bist == 0)
- if ( bist == 0 )
Unnecessary white space changes against coding conventions.
- { early_mtrr_init();
- smscsuperio_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE);
- }
- smscsuperio_enable_serial( SERIAL_DEV, CONFIG_TTYS0_BASE );
Unnecessary white space changes against coding conventions.
uart_init(); console_init();
- report_bist_failure(bist);
- report_bist_failure( bist );
Unnecessary white space changes against coding conventions.
/* Enable access to the full ROM chip, needed very early by CBFS. */
- i82371eb_enable_rom(PCI_DEV(0, 7, 0) ); /* ISA bridge at 00:07.0. */
- i82371eb_enable_rom( PCI_DEV( 0, 7, 0 ) ); /* ISA bridge at 00:07.0. */
Unnecessary white space changes against coding conventions.
Index: src/mainboard/nokia/ip530/devicetree.cb
--- src/mainboard/nokia/ip530/devicetree.cb (revision 5583) +++ src/mainboard/nokia/ip530/devicetree.cb (working copy) @@ -1,94 +1,114 @@ ## -## This file is part of the coreboot project. +## Based on romstage.c from src/mainboard/asus/p2b
Bogus change
## -## Copyright (C) 2010 Marc Bertens mbertens@xs4all.nl -## -## This program is free software; you can redistribute it and/or modify -## it under the terms of the GNU General Public License as published by -## the Free Software Foundation; either version 2 of the License, or -## (at your option) any later version. -## -## This program is distributed in the hope that it will be useful, -## but WITHOUT ANY WARRANTY; without even the implied warranty of -## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -## GNU General Public License for more details. -## -## You should have received a copy of the GNU General Public License -## along with this program; if not, write to the Free Software -## Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA -##
Don't remove license headers
chip northbridge/intel/i440bx # Northbridge
- device lapic_cluster 0 on # APIC cluster
- chip cpu/intel/socket_PGA370 # CPU
device lapic 0 on end # APIC- end
- end
- device pci_domain 0 on # PCI domain
- device pci 0.0 on end # Host bridge
- device pci 1.0 on end # PCI/AGP bridge
- chip southbridge/intel/i82371eb # Southbridge
device pci 7.0 on # ISA bridgechip superio/smsc/smscsuperio # Super I/O (SMSC FDC37C878)device pnp 3f0.0 on # Floppyio 0x60 = 0x3f0irq 0x70 = 6drq 0x74 = 2enddevice pnp 3f0.3 on # Parallel portio 0x60 = 0x378irq 0x70 = 7drq 0x74 = 4enddevice pnp 3f0.4 on # COM1io 0x60 = 0x3f8irq 0x70 = 4enddevice pnp 3f0.5 on # COM2 / IRio 0x60 = 0x2f8irq 0x70 = 3enddevice pnp 3f0.7 on # PS/2 keyboard / mouseio 0x60 = 0x60io 0x62 = 0x64irq 0x70 = 1 # PS/2 keyboard interruptirq 0x72 = 12 # PS/2 mouse interruptenddevice pnp 3f0.9 on # Game portio 0x60 = 0x201enddevice pnp 3f0.a on # Power-management events (PME)io 0x60 = 0x600enddevice pnp 3f0.b on # MIDI port (MPU-401)io 0x60 = 0x330irq 0x70 = 5endendenddevice pci 7.1 on end # IDEdevice pci 7.2 on end # USBdevice pci 7.3 on end # ACPIregister "ide0_enable" = "1"register "ide1_enable" = "1"register "ide_legacy_enable" = "1"# Enable UDMA/33 for higher speed if your IDE device(s) support it.register "ide0_drive0_udma33_enable" = "0"register "ide0_drive1_udma33_enable" = "0"register "ide1_drive0_udma33_enable" = "0"register "ide1_drive1_udma33_enable" = "0"- end
- device pci 0d.0 on end # NIC (DEC DECchip 21142/43)
- device pci 0e.0 on end # NIC (DEC DECchip 21142/43)
- device pci 0f.0 on end # CardBus bridge (TI PCI1225)
- device pci 0f.1 on end # CardBus bridge (TI PCI1225)
- end
- device pci_domain 1 on # PCI domain 1
- device pci 00.0 on end # PCI bridge (DEC DECchip 21150)
- end
- device pci_domain 2 on # PCI domain 2
- device pci 04.0 on end # NIC (DECchip 21142/43)
- device pci 04.0 on end # NIC (DECchip 21142/43)
- end
- device apic_cluster 0 on # APIC cluster
chip cpu/intel/socket_PGA370 # CPUdevice apic 0 on # APICendend- end
This backs out a fix.
Won't even compile anymore
- device pci_domain 0 on # PCI domain
device pci 0.0 on # Host bridgeenddevice pci 1.0 on # PCI/AGP bridgeendchip southbridge/intel/i82371eb # Southbridgedevice pci 7.0 on # ISA bridgechip superio/smsc/smscsuperio # Super I/O FDC 37C878device pnp 3f0.0 off # Floppy# LDN 0x00 (Floppy)## Register 0x30 ( Activate ) needs to be set to 0x00 (disable de FDD)enddevice pnp 3f0.3 off # Parallel port# LDN 0x03 (Parallel port)## Register 0x30 ( Activate ) needs to be set to 0x00 (disable de PP)# Registers 0x60, 0x61, 0x70 are set to 0x00end
inconsistend indentation. Please fix.
Index: src/mainboard/nokia/ip530/irq_tables.c
--- src/mainboard/nokia/ip530/irq_tables.c (revision 5583) +++ src/mainboard/nokia/ip530/irq_tables.c (working copy) @@ -19,31 +19,74 @@ */
#include <arch/pirq_routing.h> +#include <console/console.h>
+#define PIRQ_MAX_DEVICES( no ) (32 + ( 16 * no )) +//----------------------------------------------------------------------------- const struct irq_routing_table intel_irq_routing_table = {
- PIRQ_SIGNATURE, /* u32 signature */
- PIRQ_VERSION, /* u16 version */
- 32 + 16 * CONFIG_IRQ_SLOT_COUNT,/* Max. number of devices on the bus */
- 0x00, /* Interrupt router bus */
- (0x07 << 3) | 0x0, /* Interrupt router dev */
- 0, /* IRQs devoted exclusively to PCI usage */
- 0x8086, /* Vendor */
- 0x122e, /* Device */
- 0, /* Miniport */
- { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, /* u8 rfu[11] */
- 0x36, /* Checksum */
- /* u32 signature */
- PIRQ_SIGNATURE,
- /* u16 version */
- PIRQ_VERSION,
- /* Max. number of devices on the bus, set this through Kconfig */
- PIRQ_MAX_DEVICES( CONFIG_IRQ_SLOT_COUNT ),
- /* Interrupt router bus */
- 0x00,
- /* Interrupt router dev */
- (0x07 << 3) | 0x0,
- /* IRQs devoted exclusively to PCI usage */
- 0,
- /* Vendor, device */
- 0x8086, 0x122e,
- /* Miniport */
- 0,
- /* u8 rfu[11] */
- { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 },
- /* Checksum (has to be set to some value that would give 0 after the sum of all bytes
* for this structure (including checksum).*/- 0x08, {
/* bus, dev | fn, {link, bitmap}, {link, bitmap}, {link, bitmap}, {link, bitmap}, slot, rfu */{0x00, (0x07 << 3) | 0x0, {{0x00, 0x0ea8}, {0x00, 0x0ea8}, {0x00, 0x0ea8}, {0x63, 0x0ea8}}, 0x0, 0x0},{0x00, (0x0c << 3) | 0x0, {{0x61, 0x06a8}, {0x62, 0x06a8}, {0x00, 0x06a8}, {0x00, 0x06a8}}, 0x0, 0x0},{0x00, (0x0d << 3) | 0x0, {{0x60, 0x0ea8}, {0x61, 0x0ea8}, {0x00, 0x0ea8}, {0x00, 0x0ea8}}, 0x1, 0x0},{0x00, (0x09 << 3) | 0x0, {{0x62, 0x0ea8}, {0x63, 0x0ea8}, {0x60, 0x0ea8}, {0x61, 0x0ea8}}, 0x2, 0x0},{0x00, (0x0a << 3) | 0x0, {{0x63, 0x0ea8}, {0x00, 0x0ea8}, {0x00, 0x0ea8}, {0x00, 0x0ea8}}, 0x0, 0x0},{0x01, (0x00 << 3) | 0x0, {{0x60, 0x0ea8}, {0x00, 0x0ea8}, {0x00, 0x0ea8}, {0x00, 0x0ea8}}, 0x0, 0x0},
// Table by CareBear:// 0x1E20 = 0001111000100000 (IRQ5, IRQ9, IRQ10, IRQ11, IRQ12)//// Got some unexpected behaviour for the USB, orginal set 63{ 0x00, (0x07 << 3) | 0x0, {{0x00, 0x1E20}, {0x00, 0x1E20}, {0x00, 0x1E20}, {0x63, 0x1E20}}, 0x0, 0x0 },// 62{ 0x00, (0x0d << 3) | 0x0, {{0x62, 0x1E20}, {0x00, 0x0000}, {0x00, 0x0000}, {0x00, 0x0000}}, 0x0, 0x0 },// 63{ 0x00, (0x0e << 3) | 0x0, {{0x63, 0x1E20}, {0x00, 0x0000}, {0x00, 0x0000}, {0x00, 0x0000}}, 0x0, 0x0 },{ 0x02, (0x04 << 3) | 0x0, {{0x60, 0x1E20}, {0x00, 0x0000}, {0x00, 0x0000}, {0x00, 0x0000}}, 0x0, 0x0 },{ 0x02, (0x05 << 3) | 0x0, {{0x61, 0x1E20}, {0x00, 0x0000}, {0x00, 0x0000}, {0x00, 0x0000}}, 0x0, 0x0 },// TODO: fix the PCI1225.INTRTIE_BIT = 0{ 0x00, (0x0f << 3) | 0x0, {{0x60, 0x1E20}, {0x61, 0x1E20}, {0x00, 0x0000}, {0x00, 0x0000}}, 0x0, 0x0 },}; +//----------------------------------------------------------------------------- +static int calc_checksum( const struct irq_routing_table *rt ) +{
long i;
uint8_t *addr, sum = 0;
addr = (uint8_t *) rt;
for (i = 0; i < rt->size; i++)
{
sum += addr[i];}
return ( sum );
+}
unneeded.
+//----------------------------------------------------------------------------- unsigned long write_pirq_routing_table(unsigned long addr) {
- return copy_pirq_routing_table(addr);
- int iResult = calc_checksum( &intel_irq_routing_table );
- printk( BIOS_DEBUG, "PIRQ_TABLE_CHECKSUM is %x\n", 0x100 - ( ( iResult - intel_irq_routing_table.checksum ) & 0xFF ) );
- return ( copy_pirq_routing_table( addr ) );
} +//----------------------------------------------------------------------------- +void pirq_assign_irqs(const unsigned char pIntAtoD[4]) +{
- // This is just here as a placeholder until the point is solved in the
- // main code of coreboot
- return;
+} +//----------------------------------------------------------------------------- Index: src/mainboard/nokia/ip530/mainboard.c =================================================================== --- src/mainboard/nokia/ip530/mainboard.c (revision 5583) +++ src/mainboard/nokia/ip530/mainboard.c (working copy) @@ -20,7 +20,67 @@
#include <device/device.h> #include "chip.h" +#include <device/pci_def.h> +#include <arch/io.h> +#include <device/pnp_def.h> +#include <console/console.h> +#define OUTB outb +#define INB inb
Please don't
+//----------------------------------------------------------------------------- +/* +* Taken from flashrom project +* Generic Super I/O helper functions +*/ +static uint8_t sio_read(uint16_t port, uint8_t reg) +{
- OUTB( reg, port );
- return ( INB( port + 1 ) );
+} +//----------------------------------------------------------------------------- +static void sio_write(uint16_t port, uint8_t reg, uint8_t data) +{
- OUTB( reg, port );
- OUTB( data, port + 1 );
- return;
+}
Please use the coreboot functions for this, no need to copy stuff from other projects. In fact, below should go into devicetree.cb
+//----------------------------------------------------------------------------- +static void nokia_ip530_board_enable( device_t dev ) +{
- print_debug( "Setting up IP530-Super I/O devices\n");
- /*
- Register 03 (Index Address) needs to be set to 0x80:
Enable GP1, WDT_CTRL, GP5, GP6, Soft Power Enable and Status
Register access when not in configuration mode (0x80)
Sets GP1 etc. selection regis ter used when in Run mode
(not in Configuration Mode), 00 = 0xE0
- Register 22: (Power Control) need to be set to 0x30:
Serial Port 1 Power, = 1 Power on or enabled (0x10)
Serial Port 2 Power, = 1 Power on or enabled (0x20)
*Note* the rest of the devices are disabled.
- Register 24: ( OSC ) need to be set to 0x84:
Osc is on, BRG clock is on. (0x04)
IRQ8 Polarity is IRQ8 is active low (0x80)
- This is done outside the config mode
- */
- printk( BIOS_DEBUG, "--Correcting direct registers\n");
- sio_write( 0x20, 0x03, 0x80 );
- printk( BIOS_DEBUG, "--Register 0x03 = %X := 0x80\n", sio_read( 0x20, 0x03 ) );
- sio_write( 0x20, 0x22, 0x30 );
- printk( BIOS_DEBUG, "--Register 0x22 = %X := 0x30\n", sio_read( 0x20, 0x22 ) );
- sio_write( 0x20, 0x24, 0x84 );
- printk( BIOS_DEBUG, "--Register 0x24 = %X := 0x84\n", sio_read( 0x20, 0x24 ) );
- return;
+} +//-----------------------------------------------------------------------------
struct chip_operations mainboard_ops = {
- CHIP_NAME("Nokia IP530 Mainboard")
- CHIP_NAME( "NOKIA IP530 Mainboard" )
- .enable_dev = nokia_ip530_board_enable,
};
Index: src/arch/i386/boot/pirq_routing.c
--- src/arch/i386/boot/pirq_routing.c (revision 5583) +++ src/arch/i386/boot/pirq_routing.c (working copy) @@ -1,3 +1,26 @@ +/*
- This file is part of the coreboot project.
- coreboot PIRQ Table support
- Copyright (C) 2005-2010 coresystems GmbH
- Copyright (C) 2010 Marc Bertens mbertens@xs4all.nl
broken indentation
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; version 2 of
- the License.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
- MA 02110-1301 USA
- */
#include <console/console.h> #include <arch/pirq_routing.h> #include <string.h> @@ -121,7 +144,11 @@
printk(BIOS_DEBUG, "INT: %c link: %x bitmap: %x ", 'A' + j, link, bitmap);
+// fix made by Marc Bertens mbertens@xs4all.nl
if (link > 0x5f){link -= 0x5f;}
What does this fix? That would be far more interesting than who fixed it (that belongs in the copyright lines)
Stefan
On 5/24/10 2:25 PM, mbertens wrote:
Signed-off-by: Marc Bertens mbertens@xs4all.nl
Hello all,
Here is the update for the Nokia IP530 system. it has now working NICs and correct PIRQ table. corrections where made to serveral files.
Currently the PCMCIA controller need some more work, it uses a PCI1225 of Texas Instruments. Any help with this is appreciated.
Marc.
This code is identical to what Uwe checked in on 2010-04-19 except for some minor non-functional modifications Uwe made. Are you sure you sent the right patch?
Stefan
-
Joseph Smith -
mbertens -
Myles Watson -
Peter Stuge -
Stefan Reinauer -
Stefan Reinauer