[coreboot] [PATCH] asrock e350m1: configure sb800 gpp ports to support onboard pcie nic

Scott Duplichan scott at notabs.org
Mon Jun 20 23:43:24 CEST 2011


Marc Jones wrote:

]Hi Scott,
]
]On Fri, Jun 17, 2011 at 10:26 PM, Scott Duplichan <scott at notabs.org> wrote:
]> The attached patch allows the asrock e350m1 onboard nic to work.
]> 1) Update the asrock e350m1 devicetree.cb to match the hardware.
]> 2) Change the way the sb800 cimx wrapper code works. The original
]> cimx code calls sb800 cimx function sbBeforePciInit() once. When
]> ported to coreboot, the gpp component of this function was called
]> once for each gpp port, as the gpp port's enable/disable state
]> became known. A 05/15/2011 change makes the early gpp code run
]> only once, triggered by processing the 4th gpp port. This method
]> is not general enough because the 4th gpp port is not enabled on
]> all boards. With the current change, the early gpp code runs when
]> the first gpp port is processed. If any gpp ports are enabled, the
]> first must be enabled. Tested with Win7 and linux on asrock e350m1.
]> This change will also affect amd inagua, and has not been tested
]> on that board.
]>
]> Signed-off-by: Scott Duplichan <scott at notabs.org>
]
]-		sb_config->PORTCONFIG[0].PortCfg.PortPresent = dev->enabled;
]-		return;
]-	case (0x15 << 3) | 1: /* 0:15:1 PCIe PortB */
]-		sb_config->PORTCONFIG[1].PortCfg.PortPresent = dev->enabled;
]-		return;
]-	case (0x15 << 3) | 2: /* 0:15:2 PCIe PortC */
]-		sb_config->PORTCONFIG[2].PortCfg.PortPresent = dev->enabled;
]-		return;
]-	case (0x15 << 3) | 3: /* 0:15:3 PCIe PortD */
]-		sb_config->PORTCONFIG[3].PortCfg.PortPresent = dev->enabled;
]+		{
]+		device_t device;
]+		for (device = dev; device; device = device->next) {
]+			if (dev->path.type != DEVICE_PATH_PCI) continue;
]+			if ((device->path.pci.devfn & ~7) !=
PCI_DEVFN(0x15,0)) ]break;
]+			sb_config->PORTCONFIG[device->path.pci.devfn &
]3].PortCfg.PortPresent = device->enabled;
]+		}
]
]The allocator is going to loop through and call this function for each
]device in the devicetree.cb. Is there a reason to change this to a
]loop here? It looks like the real fix is moving the SB_BEFORE_PCI_INIT
]call to the last device, and to not run for each device. Did I miss
]something more subtle in this patch?
]
]Marc

Hello Marc,

I wanted to call the standard cimx entry point only once, the way it is done
in the reference BIOS. This is difficult to do with coreboot, because all
the portPresent values need to be known before making the call. All of the
dev->enable values are not available in parallel from coreboot. The previous
code captured the dev->enable values for functions 0, 1, and 2 in the local
sb_config struct, then returned. When function 3 was processed all 4 values
were ready. I think that is similar to what you suggest. The difference is
that the prevoius code called only the sbPcieGppEarlyInit part of
SB_BEFORE_PCI_INIT.

I think your suggestion works for boards that use the sb800 option for 4 x1
ports, as asrock e350m1 does. But what about a board that uses one of the
other sb800 pcie options such as single x4 port? My concern was that
functions 1, 2, and 3 might not even be visible. Even if they are visible,
the person writing devicetree might not choose to include them since they
are not present in a booted system. Because function 0 is visible in all
cases, I thought that is the best place to trigger the cimx call. Because
function 0 is processed first, getting the dev->enable values is not so
easy. The new code scans the devicetree.cb nodes following device 0x15
function 0. Scanning stops on the first pci node not for device 0x15, so
the loop will run a maximum of 3 times. That lets the code gather the needed
dev->enable values.

Thanks,
Scott





More information about the coreboot mailing list