A little more cleanup in northbridge/amd/k8/pci.c

[Myles Watson]

This patch makes northbridge/amd/k8/pci.c use pci functions.  Build tested
on Serengeti.

Signed-off-by: Myles Watson

Thanks,
Myles

[Marc Jones]

Hi Myles,

On Tue, Nov 18, 2008 at 10:06 AM, Myles Watson  wrote:
> This patch makes northbridge/amd/k8/pci.c use pci functions.  Build tested
> on Serengeti.
>
> Signed-off-by: Myles Watson

I don't think that it should ever happen, but  you might have an
endless while loop. Maybe keep the nodeid < CONFIG_MAX_PHYSICAL_CPUS
check?

Marc

[Myles Watson]

> -----Original Message-----
> From: Marc Jones [mailto:[e-mail]]
> Sent: Tuesday, November 18, 2008 10:36 AM
> To: Myles Watson
> Cc: Coreboot
> Subject: Re: [coreboot] A little more cleanup in northbridge/amd/k8/pci.c
>
> Hi Myles,
>
> On Tue, Nov 18, 2008 at 10:06 AM, Myles Watson  wrote:
> > This patch makes northbridge/amd/k8/pci.c use pci functions.  Build
>
> tested
> > on Serengeti.
> >
> > Signed-off-by: Myles Watson
>
> I don't think that it should ever happen, but  you might have an
> endless while loop. Maybe keep the nodeid < CONFIG_MAX_PHYSICAL_CPUS
> check?

-	for (nodeid = 0; !res && (nodeid < CONFIG_MAX_PHYSICAL_CPUS);
nodeid++) {
-		struct device *dev;
-		dev = __f0_dev[nodeid];
-		if (!dev)
-			continue;
+	while (!res
+	       && (dev = dev_find_pci_device(PCI_VENDOR_ID_AMD, 0x1100,
dev))) {
    	for (link = 0; !res && (link < 3); link++) {
    		res = probe_resource(dev, 0x100 + (reg | link));
    	}
+		nodeid++;
    }

Thanks for the quick review.  The reason I didn't keep it is because
dev_find_pci_device is updating dev, which is where the search starts from
each time.  You shouldn't find the same device twice using that function, or
you have a loop in the device list.

I can put it back in, or a comment explaining it if you want, though.

Thanks,
Myles

[Marc Jones]

On Tue, Nov 18, 2008 at 10:59 AM, Myles Watson  wrote:
> > -----Original Message-----
> > From: Marc Jones [mailto:[e-mail]]
> > Sent: Tuesday, November 18, 2008 10:36 AM
> > To: Myles Watson
> > Cc: Coreboot
> > Subject: Re: [coreboot] A little more cleanup in northbridge/amd/k8/pci.c
> >
> > Hi Myles,
> >
> > On Tue, Nov 18, 2008 at 10:06 AM, Myles Watson  wrote:
> > > This patch makes northbridge/amd/k8/pci.c use pci functions.  Build
> >
> > tested
> > > on Serengeti.
> > >
> > > Signed-off-by: Myles Watson
> >
> > I don't think that it should ever happen, but  you might have an
> > endless while loop. Maybe keep the nodeid < CONFIG_MAX_PHYSICAL_CPUS
> > check?
>   for (nodeid = 0; !res && (nodeid < CONFIG_MAX_PHYSICAL_CPUS);
>
> nodeid++) {
>           struct device *dev;
>           dev = __f0_dev[nodeid];
>           if (!dev)
>                   continue;
>   while (!res
>          && (dev = dev_find_pci_device(PCI_VENDOR_ID_AMD, 0x1100,
>
> dev))) {
>                for (link = 0; !res && (link < 3); link++) {
>                        res = probe_resource(dev, 0x100 + (reg | link));
>                }
>           nodeid++;
>  }
>
> Thanks for the quick review.  The reason I didn't keep it is because
> dev_find_pci_device is updating dev, which is where the search starts from
> each time.  You shouldn't find the same device twice using that function, or
> you have a loop in the device list.
>
> I can put it back in, or a comment explaining it if you want, though.

I see. You are right.
Acked-by: Marc Jones

[Myles Watson]

On Tue, Nov 18, 2008 at 11:51 AM, Marc Jones  wrote:
> On Tue, Nov 18, 2008 at 10:59 AM, Myles Watson  wrote:
> > > -----Original Message-----
> > > From: Marc Jones [mailto:[e-mail]]
> > > Sent: Tuesday, November 18, 2008 10:36 AM
> > > To: Myles Watson
> > > Cc: Coreboot
> > > Subject: Re: [coreboot] A little more cleanup in
>
> northbridge/amd/k8/pci.c
> > > Hi Myles,
> > >
> > > On Tue, Nov 18, 2008 at 10:06 AM, Myles Watson
>
> wrote:
> > > > This patch makes northbridge/amd/k8/pci.c use pci functions.  Build
> > >
> > > tested
> > > > on Serengeti.
> > > >
> > > > Signed-off-by: Myles Watson
>
> Acked-by: Marc Jones

Rev 1043.

Thanks,
Myles