Re: [coreboot] [PATCH] Factor out a few commonly duplicated functions from northbridge.c

[Uwe Hermann]

On Sun, Oct 24, 2010 at 03:27:33PM +0200, Uwe Hermann wrote:
> On Mon, Oct 11, 2010 at 09:24:07PM +0200, Peter Stuge wrote:
> > Uwe Hermann wrote:
> > > Factor out a few commonly duplicated functions from northbridge.c.
> > >
> > > The following functions are moved to devices/device_util.c:
> > >
> > > ram_resource()
> > >
> > > tolm_test()
> > >
> > > find_pci_tolm()
> > >
> > > There are only two tolm_test() / find_pci_tolm() which differ from the
> > > defaults,
> >
> > How do they differ?
>
> This is the common implementation now factored out:
>
> void tolm_test(void *gp, struct device *dev, struct resource *new)
> {
>         struct resource **best_p = gp;
>         struct resource *best;
>         best = *best_p;
>         if (!best || (best->base > new->base))
>                 best = new;
>         *best_p = best;
> }
>
> u32 find_pci_tolm(struct bus *bus)
> {
>         struct resource *min = NULL;
>         u32 tolm;
>         search_bus_resources(bus, IORESOURCE_MEM, IORESOURCE_MEM, tolm_test, &min);
>         tolm = 0xffffffffUL;
>         if (min && tolm > min->base)
>                 tolm = min->base;
>         return tolm;
> }
>
> The diff to that in the K8 version is this (in tolm_test()):
>    if (!best || (best->base > new->base))
>    /* Skip VGA. */
>    if (!best || (best->base > new->base && new->base > 0xa0000)) {
>
> Small fix which could also be moved into the global tolm_test(). Depending on
> whether or not we want _all_ northbridges to skip that VGA range, we
> could add a parameter to the funtion, or just always skip it.

Any comments on this? Why does K8 have an exception for 0xa0000, but the
other implementations don't? This doesn't sound like it's K8 specific to
me. Should all northbridges do this?