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

[Uwe Hermann]

See patch.

Uwe.

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

[Peter Stuge]

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?
> Signed-off-by: Uwe Hermann

Acked-by: Peter Stuge

[Uwe Hermann]

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.

The version on Fam10h is this, not sure if this is worth the hassle:

static u32 my_find_pci_tolm(struct bus *bus, u32 tolm)
{
        struct resource *min;
        min = 0;
        search_bus_resources(bus, IORESOURCE_MEM, IORESOURCE_MEM, tolm_test, &min);
        if (min && tolm > min->base) {
                tolm = min->base;
        }
        return tolm;
}

And then later:

pci_tolm = 0xffffffffUL;
        for(link = dev->link_list; link; link = link->next) {
                pci_tolm = my_find_pci_tolm(link, pci_tolm);
        }

Uwe.

[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?

[Myles Watson]

> > 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?

My guess is that most northbridges don't need to specify the "VGA
compatibility" range.  K8 requires that range to be set up explicitly,
so we've got a resource for it.  If you just look for the "lowest"
resource to find the top of RAM, it won't allow you to have any RAM
above 0xa0000.  I don't think it would hurt any other northbridges to
add this, but it will break K8 if you remove it.

Thanks,
Myles