On Thu, Oct 09, 2008 at 04:01:27AM +0200, Carl-Daniel Hailfinger wrote:
Replace magic numbers with existing symbolic constants. SB600 is heavily affected. This mostly targets pci_*_config*() calls.
This is part of my quest to make existing code more readable without looking up magic numbers.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Great stuff!
Acked-by: Uwe Hermann uwe@hermann-uwe.de
Do you plan to also port this to v2? If not, I might do it.
Index: corebootv3-sb600/southbridge/amd/sb600/stage1.c
--- corebootv3-sb600/southbridge/amd/sb600/stage1.c (Revision 906) +++ corebootv3-sb600/southbridge/amd/sb600/stage1.c (Arbeitskopie) @@ -23,6 +23,7 @@ #include <msr.h> #include <legacy.h> #include <device/pci.h> +#include <device/pci_ids.h> #include <statictree.h> #include <config.h> #include <io.h> @@ -68,7 +69,7 @@ static u8 get_sb600_revision(void) { u32 dev;
- if (!pci_conf1_find_device(0x1002, 0x4385, &dev)){
- if (!pci_conf1_find_device(PCI_VENDOR_ID_ATI, 0x4385, &dev)){
That may be for another patch, but the 0x4385 should of course also be replaced by PCI_DEVICE_ID_ATI_SB600_SM (same for all other chipset devices).
Currently those #defines are all in southbridge/amd/sb600/sb600.h, but they should be moved to pci_ids.h (both in v2 and in v3).
/* get base addresss */
- sata_bar5 = (u8 *) (pci_read_config32(dev, 0x24) & ~0x3FF);
- sata_bar0 = pci_read_config16(dev, 0x10) & ~0x7;
- sata_bar1 = pci_read_config16(dev, 0x14) & ~0x7;
- sata_bar2 = pci_read_config16(dev, 0x18) & ~0x7;
- sata_bar3 = pci_read_config16(dev, 0x1C) & ~0x7;
- sata_bar4 = pci_read_config16(dev, 0x20) & ~0x7;
- sata_bar5 = (u8 *) (pci_read_config32(dev, PCI_BASE_ADDRESS_5) & ~0x3FF);
- sata_bar0 = pci_read_config16(dev, PCI_BASE_ADDRESS_0) & ~0x7;
- sata_bar1 = pci_read_config16(dev, PCI_BASE_ADDRESS_1) & ~0x7;
- sata_bar2 = pci_read_config16(dev, PCI_BASE_ADDRESS_2) & ~0x7;
- sata_bar3 = pci_read_config16(dev, PCI_BASE_ADDRESS_3) & ~0x7;
- sata_bar4 = pci_read_config16(dev, PCI_BASE_ADDRESS_4) & ~0x7;
Not related to this patch, but I'm wondering why BAR5 is read first, and then 0, 1, 2, 3, 4. Is there some technical reason for that? If yes, there should be a comment here.
Uwe.