AGPGart / AMD K7

AGPGart / AMD K7

[Preston A. Elder]

Hi,

I have a Tyan Thunder K7x Pro (S2469) and the amd-k7-agp module does not
seem to be probing my AGP device.  I have even tried putting debugging
code into the amd-k7-agp module, and sure enough I can see it being
loaded, but the probe function is never called.  This is with kernel 2.6.19.

Here is my lspci -vvv for the relevent devices:

00:00.0 Host bridge: Advanced Micro Devices [AMD] AMD-760 MP [IGD4-2P]
System Co
ntroller (rev 20)
        Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr- Step
ping- SERR- FastB2B-
        Status: Cap+ 66MHz+ UDF- FastB2B- ParErr- DEVSEL=medium >TAbort-
<TAbort
- <MAbort+ >SERR- <PERR-
        Latency: 32
        Region 0: Memory at d0000000 (32-bit, prefetchable) [size=256M]
        Region 1: Memory at c2200000 (32-bit, prefetchable) [size=4K]
        Region 2: I/O ports at 1050 [disabled] [size=4]
        Capabilities: [a0] AGP version 2.0
                Status: RQ=16 Iso- ArqSz=0 Cal=0 SBA+ ITACoh- GART64-
HTrans- 64
bit- FW+ AGP3- Rate=x1,x2,x4
                Command: RQ=1 ArqSz=0 Cal=0 SBA+ AGP+ GART64- 64bit- FW-
Rate=<n
one>

00:01.0 PCI bridge: Advanced Micro Devices [AMD] AMD-760 MP [IGD4-2P]
AGP Bridge
 (prog-if 00 [Normal decode])
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr- Step
ping- SERR- FastB2B-
        Status: Cap- 66MHz+ UDF- FastB2B- ParErr- DEVSEL=medium >TAbort-
<TAbort
- <MAbort- >SERR- <PERR-
        Latency: 99
        Bus: primary=00, secondary=01, subordinate=01, sec-latency=64
        I/O behind bridge: 00002000-00002fff
        Memory behind bridge: c0100000-c01fffff
        Prefetchable memory behind bridge: e0000000-efffffff
        Secondary status: 66MHz+ FastB2B- ParErr- DEVSEL=medium >TAbort-
<TAbort
- <MAbort+ <SERR- <PERR-
        BridgeCtl: Parity- SERR- NoISA+ VGA+ MAbort- >Reset- FastB2B-


As you can see, the first device is indeed showing up as AGP capable and
such, its just never probed (at least the probe function in amd-k7-agp
is never called once the module is loaded).

To simplefy things, here is a pcitweak -l of the top two devices above:
PCI: 00:00:0: chip 1022,700c card 0000,0000 rev 20 class 06,00,00 hdr 00
PCI: 00:01:0: chip 1022,700d card 0000,0000 rev 00 class 06,04,00 hdr 01

In the amd-k7-agp code, this is in the device list:
    {
    .class      = (PCI_CLASS_BRIDGE_HOST << 8),
    .class_mask = ~0,
    .vendor     = PCI_VENDOR_ID_AMD,
    .device     = PCI_DEVICE_ID_AMD_FE_GATE_700C,
    .subvendor  = PCI_ANY_ID,
    .subdevice  = PCI_ANY_ID,
    },

Which matches the first device.  So I'm completely unsure as to why this
device is never probed or how to fix it.

Any help appreciated,

PreZ :)

PS - please CC me specifically on replies.  I will check the list myself
manually, but just in case I miss something .. :)

Re: AGPGart / AMD K7

[Dave Jones]

On Fri, Apr 20, 2007 at 03:10:45AM -0400, Preston A. Elder wrote:
 
 > I have a Tyan Thunder K7x Pro (S2469) and the amd-k7-agp module does not
 > seem to be probing my AGP device.  I have even tried putting debugging
 > code into the amd-k7-agp module, and sure enough I can see it being
 > loaded, but the probe function is never called.  This is with kernel 2.6.19.

This is the second report of this I've heard, and I really have no good
explanation for it.

 > As you can see, the first device is indeed showing up as AGP capable and
 > such, its just never probed (at least the probe function in amd-k7-agp
 > is never called once the module is loaded).
 > 
 > To simplefy things, here is a pcitweak -l of the top two devices above:
 > PCI: 00:00:0: chip 1022,700c card 0000,0000 rev 20 class 06,00,00 hdr 00
 > PCI: 00:01:0: chip 1022,700d card 0000,0000 rev 00 class 06,04,00 hdr 01
 > 
 > In the amd-k7-agp code, this is in the device list:
 >     {
 >     .class      = (PCI_CLASS_BRIDGE_HOST << 8),
 >     .class_mask = ~0,
 >     .vendor     = PCI_VENDOR_ID_AMD,
 >     .device     = PCI_DEVICE_ID_AMD_FE_GATE_700C,
 >     .subvendor  = PCI_ANY_ID,
 >     .subdevice  = PCI_ANY_ID,
 >     },
 > 
 > Which matches the first device.  So I'm completely unsure as to why this
 > device is never probed or how to fix it.

try adding some instrumentation to __pci_register_driver and the functions
it calls.

oh, one thought.. do you have CONFIG_PCI_MULTITHREAD_PROBE set?
I'm wondering if the probing is racing with another driver which is claiming
the same PCI ID. (Edac, or watchdog for example)

	Dave

-- 
http://www.codemonkey.org.uk

Re: AGPGart / AMD K7

[Preston A. Elder]

Dave Jones wrote:
> try adding some instrumentation to __pci_register_driver and the functions
> it calls.
>
> oh, one thought.. do you have CONFIG_PCI_MULTITHREAD_PROBE set?
> I'm wondering if the probing is racing with another driver which is claiming
> the same PCI ID. (Edac, or watchdog for example)
>   
Dave,

Multithread is not set.

Here is the output of my instrumentation:

Linux agpgart interface v0.101 (c) Dave Jones
agpgart: DEBUG 0
agpgart: DEBUG 1
__pci_register_driver: In function
__pci_register_driver: driver = agpgart-amdk7, multithread = 0
__pci_register_driver: Before Spinlock
__pci_register_driver: Before List Init
__pci_register_driver: Before Driver Register
__pci_register_driver: Error = 0
__pci_register_driver: Returning 0

The DEBUG 0 and 1 are coming from agp_amdk7_init()
There is a DEBUG 2 at the top of agp_amdk7_probe(), even before
pci_find_capability, but the function never gets called.

I assume an error of 0 means no conflict.  Any further steps to take for
this?  I would really like my AGP working :S
Especially since lspci obviously detects it, even tells me it supports
1x, 2x and 4x.

PreZ :S

Re: AGPGart / AMD K7

[Dave Jones]

On Fri, Apr 20, 2007 at 12:53:31PM -0400, Preston A. Elder wrote:

 > Linux agpgart interface v0.101 (c) Dave Jones
 > agpgart: DEBUG 0
 > agpgart: DEBUG 1
 > __pci_register_driver: In function
 > __pci_register_driver: driver = agpgart-amdk7, multithread = 0
 > __pci_register_driver: Before Spinlock
 > __pci_register_driver: Before List Init
 > __pci_register_driver: Before Driver Register
 > __pci_register_driver: Error = 0
 > __pci_register_driver: Returning 0
 > 
 > The DEBUG 0 and 1 are coming from agp_amdk7_init()
 > There is a DEBUG 2 at the top of agp_amdk7_probe(), even before
 > pci_find_capability, but the function never gets called.

bus_add_driver() returns 0 on error, but there's a few different
cases it can fail, which isn't helpful.  Add some printk's to
the error cases there, and see if that gives any more clues.

	Dave

-- 
http://www.codemonkey.org.uk

Re: AGPGart / AMD K7

[Preston A. Elder]

Dave Jones wrote:
> On Fri, Apr 20, 2007 at 12:53:31PM -0400, Preston A. Elder wrote:
>
>  > Linux agpgart interface v0.101 (c) Dave Jones
>  > agpgart: DEBUG 0
>  > agpgart: DEBUG 1
>  > __pci_register_driver: In function
>  > __pci_register_driver: driver = agpgart-amdk7, multithread = 0
>  > __pci_register_driver: Before Spinlock
>  > __pci_register_driver: Before List Init
>  > __pci_register_driver: Before Driver Register
>  > __pci_register_driver: Error = 0
>  > __pci_register_driver: Returning 0
>  > 
>  > The DEBUG 0 and 1 are coming from agp_amdk7_init()
>  > There is a DEBUG 2 at the top of agp_amdk7_probe(), even before
>  > pci_find_capability, but the function never gets called.
>
> bus_add_driver() returns 0 on error, but there's a few different
> cases it can fail, which isn't helpful.  Add some printk's to
> the error cases there, and see if that gives any more clues.
>
> 	Dave
>
>   
Here you go:

Linux agpgart interface v0.101 (c) Dave Jones
agpgart: DEBUG 0
agpgart: DEBUG 1
__pci_register_driver: In function
__pci_register_driver: driver = agpgart-amdk7, multithread = 0
__pci_register_driver: Before Spinlock
__pci_register_driver: Before List Init
__pci_register_driver: Before Driver Register
bus_add_driver: In Function
bus_add_driver: Before kobject_set_name (agpgart-amdk7)
bus_add_driver: error = 0
bus_add_driver: Before kobject_register
bus_add_driver: error = 0
bus_add_driver: Before driver_attach
bus_add_driver: error = 0
bus_add_driver: Before klist_add_tail
bus_add_driver: Before module_add_driver
bus_add_driver: Before driver_add_attrs
bus_add_driver: error = 0
bus_add_driver: Before add_bind_files
bus_add_driver: error = 0
bus_add_driver: Returning 0
__pci_register_driver: Error = 0
__pci_register_driver: Returning 0


PreZ :)

Re: AGPGart / AMD K7

[Dave Jones]

On Fri, Apr 20, 2007 at 02:04:45PM -0400, Preston A. Elder wrote:
 > Dave Jones wrote:
 > > On Fri, Apr 20, 2007 at 12:53:31PM -0400, Preston A. Elder wrote:
 > >
 > >  > Linux agpgart interface v0.101 (c) Dave Jones
 > >  > agpgart: DEBUG 0
 > >  > agpgart: DEBUG 1
 > >  > __pci_register_driver: In function
 > >  > __pci_register_driver: driver = agpgart-amdk7, multithread = 0
 > >  > __pci_register_driver: Before Spinlock
 > >  > __pci_register_driver: Before List Init
 > >  > __pci_register_driver: Before Driver Register
 > >  > __pci_register_driver: Error = 0
 > >  > __pci_register_driver: Returning 0
 > >  > 
 > >  > The DEBUG 0 and 1 are coming from agp_amdk7_init()
 > >  > There is a DEBUG 2 at the top of agp_amdk7_probe(), even before
 > >  > pci_find_capability, but the function never gets called.
 > >
 > > bus_add_driver() returns 0 on error, but there's a few different
 > > cases it can fail, which isn't helpful.

Actually I misparsed this function, see below..

 > > Add some printk's to
 > > the error cases there, and see if that gives any more clues.
 > >
 > Here you go:

This is odd..

 > >  > __pci_register_driver: Before Driver Register
 > >  > __pci_register_driver: Error = 0
 > >  > __pci_register_driver: Returning 0

That __pci_register_driver code is (presumably with your printk's added..)

        error = driver_register(&drv->driver);
	printk("Error = %d\n", error);
        if (error) {
		printk("Returning %d\n" error);
                return error;
	}

Which doesn't make much sense.  If 'error' is 0, we shouldn't be
taking that second printk & return. What compiler version is this?

btw Greg, wtf does driver_register return a 0 as 'success' if it
completes the function, and 0 as 'failure' if !bus ?
That seems doomed to failure.

 > Linux agpgart interface v0.101 (c) Dave Jones
 > agpgart: DEBUG 0
 > agpgart: DEBUG 1
 > __pci_register_driver: In function
 > __pci_register_driver: driver = agpgart-amdk7, multithread = 0
 > __pci_register_driver: Before Spinlock
 > __pci_register_driver: Before List Init
 > __pci_register_driver: Before Driver Register
 > bus_add_driver: In Function
 > bus_add_driver: Before kobject_set_name (agpgart-amdk7)
 > bus_add_driver: error = 0
 > bus_add_driver: Before kobject_register
 > bus_add_driver: error = 0
 > bus_add_driver: Before driver_attach
 > bus_add_driver: error = 0
 > bus_add_driver: Before klist_add_tail
 > bus_add_driver: Before module_add_driver
 > bus_add_driver: Before driver_add_attrs
 > bus_add_driver: error = 0
 > bus_add_driver: Before add_bind_files
 > bus_add_driver: error = 0
 > bus_add_driver: Returning 0
 > __pci_register_driver: Error = 0
 > __pci_register_driver: Returning 0

So we completed bus_add_driver without failing, then popped back
up to __pci_register_driver and were somehow treated as
if we failed.   *scratches head*

	Dave

-- 
http://www.codemonkey.org.uk

Re: AGPGart / AMD K7

[Greg KH]

On Fri, Apr 20, 2007 at 02:20:29PM -0400, Dave Jones wrote:
> 
> btw Greg, wtf does driver_register return a 0 as 'success' if it
> completes the function, and 0 as 'failure' if !bus ?
> That seems doomed to failure.

I don't know why the code does that, we should always have a bus
assigned to a driver.  I'll change that and watch to see what breaks :)

thanks,

greg k-h

Re: AGPGart / AMD K7

[Dave Jones]

On Fri, Apr 20, 2007 at 11:29:52AM -0700, Greg Kroah-Hartman wrote:
 > On Fri, Apr 20, 2007 at 02:20:29PM -0400, Dave Jones wrote:
 > > 
 > > btw Greg, wtf does driver_register return a 0 as 'success' if it
 > > completes the function, and 0 as 'failure' if !bus ?
 > > That seems doomed to failure.
 > 
 > I don't know why the code does that, we should always have a bus
 > assigned to a driver.  I'll change that and watch to see what breaks :)

Maybe this?

We should always have a bus in bus_add_driver()
Instead of returning success when we don't, BUG().

Signed-off-by: Dave Jones <davej@redhat.com>

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 253868e..3ba8f1f 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -530,8 +530,7 @@ int bus_add_driver(struct device_driver *drv)
 	struct bus_type * bus = get_bus(drv->bus);
 	int error = 0;
 
-	if (!bus)
-		return 0;
+	BUG_ON(!bus);
 
 	pr_debug("bus %s: add driver %s\n", bus->name, drv->name);
 	error = kobject_set_name(&drv->kobj, "%s", drv->name);

-- 
http://www.codemonkey.org.uk

Re: AGPGart / AMD K7

[Greg KH]

On Fri, Apr 20, 2007 at 03:00:28PM -0400, Dave Jones wrote:
> On Fri, Apr 20, 2007 at 11:29:52AM -0700, Greg Kroah-Hartman wrote:
>  > On Fri, Apr 20, 2007 at 02:20:29PM -0400, Dave Jones wrote:
>  > > 
>  > > btw Greg, wtf does driver_register return a 0 as 'success' if it
>  > > completes the function, and 0 as 'failure' if !bus ?
>  > > That seems doomed to failure.
>  > 
>  > I don't know why the code does that, we should always have a bus
>  > assigned to a driver.  I'll change that and watch to see what breaks :)
> 
> Maybe this?
> 
> We should always have a bus in bus_add_driver()
> Instead of returning success when we don't, BUG().

Nah, I don't like adding BUG() calls to the kernel if it can be helped,
how about the version I copied you on a few hours ago, which is also
below?

thanks,

greg k-h

------
From: Greg Kroah-Hartman <gregkh@suse.de>
Subject: driver core: bus_add_driver should return an error if no bus

As pointed out by Dave Jones.

Cc: Dave Jones <davej@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/base/bus.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -601,7 +601,7 @@ int bus_add_driver(struct device_driver 
 	int error = 0;
 
 	if (!bus)
-		return 0;
+		return -EINVAL;
 
 	pr_debug("bus %s: add driver %s\n", bus->name, drv->name);
 	error = kobject_set_name(&drv->kobj, "%s", drv->name);

Re: AGPGart / AMD K7

[Dave Jones]

On Fri, Apr 20, 2007 at 04:26:51PM -0700, Greg Kroah-Hartman wrote:
 > > We should always have a bus in bus_add_driver()
 > > Instead of returning success when we don't, BUG().
 > 
 > Nah, I don't like adding BUG() calls to the kernel if it can be helped,
 > how about the version I copied you on a few hours ago, which is also
 > below?

Either works for me..

	Dave

-- 
http://www.codemonkey.org.uk

Re: AGPGart / AMD K7

[Preston A. Elder]

Dave Jones wrote:
> On Fri, Apr 20, 2007 at 02:04:45PM -0400, Preston A. Elder wrote:
>  > Dave Jones wrote:
>  > > On Fri, Apr 20, 2007 at 12:53:31PM -0400, Preston A. Elder wrote:
>  > >
>  > >  > Linux agpgart interface v0.101 (c) Dave Jones
>  > >  > agpgart: DEBUG 0
>  > >  > agpgart: DEBUG 1
>  > >  > __pci_register_driver: In function
>  > >  > __pci_register_driver: driver = agpgart-amdk7, multithread = 0
>  > >  > __pci_register_driver: Before Spinlock
>  > >  > __pci_register_driver: Before List Init
>  > >  > __pci_register_driver: Before Driver Register
>  > >  > __pci_register_driver: Error = 0
>  > >  > __pci_register_driver: Returning 0
>  > >  > 
>  > >  > The DEBUG 0 and 1 are coming from agp_amdk7_init()
>  > >  > There is a DEBUG 2 at the top of agp_amdk7_probe(), even before
>  > >  > pci_find_capability, but the function never gets called.
>  > >
>  > > bus_add_driver() returns 0 on error, but there's a few different
>  > > cases it can fail, which isn't helpful.
>
> Actually I misparsed this function, see below..
>
>  > > Add some printk's to
>  > > the error cases there, and see if that gives any more clues.
>  > >
>  > Here you go:
>
> This is odd..
>
>  > >  > __pci_register_driver: Before Driver Register
>  > >  > __pci_register_driver: Error = 0
>  > >  > __pci_register_driver: Returning 0
>
> That __pci_register_driver code is (presumably with your printk's added..)
>
>         error = driver_register(&drv->driver);
> 	printk("Error = %d\n", error);
>         if (error) {
> 		printk("Returning %d\n" error);
>                 return error;
> 	}
>
> Which doesn't make much sense.  If 'error' is 0, we shouldn't be
> taking that second printk & return. What compiler version is this?
>
> btw Greg, wtf does driver_register return a 0 as 'success' if it
> completes the function, and 0 as 'failure' if !bus ?
> That seems doomed to failure.
>
>  > Linux agpgart interface v0.101 (c) Dave Jones
>  > agpgart: DEBUG 0
>  > agpgart: DEBUG 1
>  > __pci_register_driver: In function
>  > __pci_register_driver: driver = agpgart-amdk7, multithread = 0
>  > __pci_register_driver: Before Spinlock
>  > __pci_register_driver: Before List Init
>  > __pci_register_driver: Before Driver Register
>  > bus_add_driver: In Function
>  > bus_add_driver: Before kobject_set_name (agpgart-amdk7)
>  > bus_add_driver: error = 0
>  > bus_add_driver: Before kobject_register
>  > bus_add_driver: error = 0
>  > bus_add_driver: Before driver_attach
>  > bus_add_driver: error = 0
>  > bus_add_driver: Before klist_add_tail
>  > bus_add_driver: Before module_add_driver
>  > bus_add_driver: Before driver_add_attrs
>  > bus_add_driver: error = 0
>  > bus_add_driver: Before add_bind_files
>  > bus_add_driver: error = 0
>  > bus_add_driver: Returning 0
>  > __pci_register_driver: Error = 0
>  > __pci_register_driver: Returning 0
>
> So we completed bus_add_driver without failing, then popped back
> up to __pci_register_driver and were somehow treated as
> if we failed.   *scratches head*
>
> 	Dave
>
>   
Dave,

Here is the code for __pci_register_driver:

  int __pci_register_driver(struct pci_driver *drv, struct module *owner)
  {
      int error;

  printk(KERN_INFO "__pci_register_driver: In function\n");
      /* initialize common driver fields */
      drv->driver.name = drv->name;
      drv->driver.bus = &pci_bus_type;
      drv->driver.owner = owner;
      drv->driver.kobj.ktype = &pci_driver_kobj_type;

  printk(KERN_INFO "__pci_register_driver: driver = %s, multithread = %d\n",
            drv->name, pci_multithread_probe);
      if (pci_multithread_probe)
          drv->driver.multithread_probe = pci_multithread_probe;
      else
          drv->driver.multithread_probe = drv->multithread_probe;

  printk(KERN_INFO "__pci_register_driver: Before Spinlock\n");
      spin_lock_init(&drv->dynids.lock);
  printk(KERN_INFO "__pci_register_driver: Before List Init\n");
     INIT_LIST_HEAD(&drv->dynids.list);

  printk(KERN_INFO "__pci_register_driver: Before Driver Register\n");
      /* register with core */
      error = driver_register(&drv->driver);

  printk(KERN_INFO "__pci_register_driver: Error = %d\n", error);
      if (!error)
          error = pci_create_newid_file(drv);

  printk(KERN_INFO "__pci_register_driver: Returning %d\n", error);
      return error;
  }

So in the above case, we ARE saying if driver_register returns 0 then
pci_create_newid_file.

Is it different to the code you have?  As I said, this IS 2.6.19.

PreZ :)

Re: AGPGart / AMD K7

[Dave Jones]

On Fri, Apr 20, 2007 at 02:31:01PM -0400, Preston A. Elder wrote:

 > Here is the code for __pci_register_driver:
 > ...
 > 
 > So in the above case, we ARE saying if driver_register returns 0 then
 > pci_create_newid_file.
 > 
 > Is it different to the code you have?  As I said, this IS 2.6.19.

Yes, .20 changed this in this way..

@@ -445,9 +442,12 @@ int __pci_register_driver(struct pci_driver *drv, struct module *owner)
 
        /* register with core */
        error = driver_register(&drv->driver);
+       if (error)
+               return error;
 
-       if (!error)
-               error = pci_create_newid_file(drv);
+       error = pci_create_newid_file(drv);
+       if (error)
+               driver_unregister(&drv->driver);
 
        return error;
 }


Retry your tracing with .20 (or better yet, .21rc7/todays git)

	Dave

-- 
http://www.codemonkey.org.uk

Re: AGPGart / AMD K7

[Preston A. Elder]

Dave, Greg,

Here is the trace with 2.6.20.6

I added back in my trace code, as you see.  As you can also see,
agp_amdk7_probe is still not called.

Linux agpgart interface v0.101 (c) Dave Jones
agp_amdk7_init: In function
agp_amdk7_init: Before pci_register_driver
__pci_register_driver: In Function (driver = agpgart-amdk7, multithread = 0)
__pci_register_driver: Before Spinlock
__pci_register_driver: Before Init List Head
__pci_register_driver: Before driver_register
bus_add_driver: In Function (c048e920)
bus_add_driver: Before kobject_set_name
bus_add_driver: error = 0
bus_add_driver: Before kobject_register
bus_add_driver: error = 0
bus_add_driver: Before driver_attach
bus_add_driver: error = 0
bus_add_driver: Before klist_add_tail
bus_add_driver: Before module_add_driver
bus_add_driver: Before driver_add_attrs
bus_add_driver: error = 0
bus_add_driver: Before add_bind_files
bus_add_driver: error = 0
bus_add_driver: Returning 0
__pci_register_driver: error = 0
__pci_register_driver: Before pci_create_newid_file
__pci_register_driver: error = 0
__pci_register_driver: Returning 0

Even when I start X (using the fglrx driver) I still do not see the
probe function being called.

Everything looks successful, too :(

I will try with 2.6.21rc7, but I don't hold out too much hope.

PreZ

Dave Jones wrote:
> On Fri, Apr 20, 2007 at 02:31:01PM -0400, Preston A. Elder wrote:
>
>  > Here is the code for __pci_register_driver:
>  > ...
>  > 
>  > So in the above case, we ARE saying if driver_register returns 0 then
>  > pci_create_newid_file.
>  > 
>  > Is it different to the code you have?  As I said, this IS 2.6.19.
>
> Yes, .20 changed this in this way..
>
> @@ -445,9 +442,12 @@ int __pci_register_driver(struct pci_driver *drv, struct module *owner)
>  
>         /* register with core */
>         error = driver_register(&drv->driver);
> +       if (error)
> +               return error;
>  
> -       if (!error)
> -               error = pci_create_newid_file(drv);
> +       error = pci_create_newid_file(drv);
> +       if (error)
> +               driver_unregister(&drv->driver);
>  
>         return error;
>  }
>
>
> Retry your tracing with .20 (or better yet, .21rc7/todays git)
>
> 	Dave
>
>   

Re: AGPGart / AMD K7

[Dave Jones]

On Fri, Apr 20, 2007 at 04:22:06PM -0400, Preston A. Elder wrote:
 > Dave, Greg,
 > 
 > Here is the trace with 2.6.20.6
 > 
 > I added back in my trace code, as you see.  As you can also see,
 > agp_amdk7_probe is still not called.

Try looking down in __driver_attach()
The fact that we're not calling the ->probe function is quite bizarre.

It could be this in __driver_attach

        if (!dev->driver)
                driver_probe_device(drv, dev);

Though that'd be odd.

Putting a #define DEBUG 1 in drivers/base/dd.c may also yield some clues.

	Dave

-- 
http://www.codemonkey.org.uk

Re: AGPGart / AMD K7

[Preston A. Elder]

Dave Jones wrote:
> On Fri, Apr 20, 2007 at 04:22:06PM -0400, Preston A. Elder wrote:
>  > Dave, Greg,
>  > 
>  > Here is the trace with 2.6.20.6
>  > 
>  > I added back in my trace code, as you see.  As you can also see,
>  > agp_amdk7_probe is still not called.
>
> Try looking down in __driver_attach()
> The fact that we're not calling the ->probe function is quite bizarre.
>
> It could be this in __driver_attach
>
>         if (!dev->driver)
>                 driver_probe_device(drv, dev);
>
> Though that'd be odd.
>
> Putting a #define DEBUG 1 in drivers/base/dd.c may also yield some clues.
>
> 	Dave
>
>   
OK, I found it!

Here is more trace:
Linux agpgart interface v0.101 (c) Dave Jones
agp_amdk7_init: In function
agp_amdk7_init: Before pci_register_driver
__pci_register_driver: In Function (driver = agpgart-amdk7, multithread = 0)
__pci_register_driver: Before Spinlock
__pci_register_driver: Before Init List Head
__pci_register_driver: Before driver_register
bus_add_driver: In Function (c0492920)
bus_add_driver: Before kobject_set_name
bus_add_driver: error = 0
bus_add_driver: Before kobject_register
bus_add_driver: error = 0
bus_add_driver: Before driver_attach
__driver_attach (0000:00:00.0,1): Before Down (parent) (c21c8600)
__driver_attach (0000:00:00.0, 1): Before Down
__driver_attach (0000:00:00.0, 1): Before Probe Device (c049fe54)
__driver_attach (0000:00:00.0, 1): alreay registered with driver amd76x_edac
__driver_attach (0000:00:00.0, 1): Before Up
__driver_attach (0000:00:00.0, 1): Before Up (parent) (c21c8600)
__driver_attach (0000:00:00.0, 1): Returning 0
bus_add_driver: error = 0
bus_add_driver: Before klist_add_tail
bus_add_driver: Before module_add_driver
bus_add_driver: Before driver_add_attrs
bus_add_driver: error = 0
bus_add_driver: Before add_bind_files
bus_add_driver: error = 0
bus_add_driver: Returning 0
__pci_register_driver: error = 0
__pci_register_driver: Before pci_create_newid_file
__pci_register_driver: error = 0
__pci_register_driver: Returning 0

I snipped some since __driver_attach is called many times.

But the long and short is that 00:00:00 is already associated with the
'amd76x_edac' driver, and as such will not call the agp probe call. 
What is this edac, btw?

PreZ :)

Re: AGPGart / AMD K7

[Preston A. Elder]

Final followup,

If I compile EDAC out of the kernel completely, everything works now.

This should be resolved though.
1) dd.c should produce some kind of warning when it wants to assign a
driver to a device, but it can't because a driver is already assigned to
a device

ie. change:
      if (!dev->driver)
          driver_probe_device(drv, dev);
to:
      if (!dev->driver)
          driver_probe_device(drv, dev);
      else
          printk(KERN_WARNING "__driver_attach (%s): alreay registered
with driver %s\n",
             dev->bus_id, dev->driver->name);

2) Possibly a device should be able to have more than one driver
associated with it - so the AGP driver and EDAC could both use the
device in question here (though this would probably be a sizable change).

Either way, at least I found the culprit :)

PreZ :)

Preston A. Elder wrote:
> Dave Jones wrote:
>   
>> On Fri, Apr 20, 2007 at 04:22:06PM -0400, Preston A. Elder wrote:
>>  > Dave, Greg,
>>  > 
>>  > Here is the trace with 2.6.20.6
>>  > 
>>  > I added back in my trace code, as you see.  As you can also see,
>>  > agp_amdk7_probe is still not called.
>>
>> Try looking down in __driver_attach()
>> The fact that we're not calling the ->probe function is quite bizarre.
>>
>> It could be this in __driver_attach
>>
>>         if (!dev->driver)
>>                 driver_probe_device(drv, dev);
>>
>> Though that'd be odd.
>>
>> Putting a #define DEBUG 1 in drivers/base/dd.c may also yield some clues.
>>
>> 	Dave
>>
>>   
>>     
> OK, I found it!
>
> Here is more trace:
> Linux agpgart interface v0.101 (c) Dave Jones
> agp_amdk7_init: In function
> agp_amdk7_init: Before pci_register_driver
> __pci_register_driver: In Function (driver = agpgart-amdk7, multithread = 0)
> __pci_register_driver: Before Spinlock
> __pci_register_driver: Before Init List Head
> __pci_register_driver: Before driver_register
> bus_add_driver: In Function (c0492920)
> bus_add_driver: Before kobject_set_name
> bus_add_driver: error = 0
> bus_add_driver: Before kobject_register
> bus_add_driver: error = 0
> bus_add_driver: Before driver_attach
> __driver_attach (0000:00:00.0,1): Before Down (parent) (c21c8600)
> __driver_attach (0000:00:00.0, 1): Before Down
> __driver_attach (0000:00:00.0, 1): Before Probe Device (c049fe54)
> __driver_attach (0000:00:00.0, 1): alreay registered with driver amd76x_edac
> __driver_attach (0000:00:00.0, 1): Before Up
> __driver_attach (0000:00:00.0, 1): Before Up (parent) (c21c8600)
> __driver_attach (0000:00:00.0, 1): Returning 0
> bus_add_driver: error = 0
> bus_add_driver: Before klist_add_tail
> bus_add_driver: Before module_add_driver
> bus_add_driver: Before driver_add_attrs
> bus_add_driver: error = 0
> bus_add_driver: Before add_bind_files
> bus_add_driver: error = 0
> bus_add_driver: Returning 0
> __pci_register_driver: error = 0
> __pci_register_driver: Before pci_create_newid_file
> __pci_register_driver: error = 0
> __pci_register_driver: Returning 0
>
> I snipped some since __driver_attach is called many times.
>
> But the long and short is that 00:00:00 is already associated with the
> 'amd76x_edac' driver, and as such will not call the agp probe call. 
> What is this edac, btw?
>
> PreZ :)
>
>
>
>   

Re: AGPGart / AMD K7

[Greg KH]

On Fri, Apr 20, 2007 at 07:42:33PM -0400, Preston A. Elder wrote:
> Final followup,
> 
> If I compile EDAC out of the kernel completely, everything works now.
> 
> This should be resolved though.
> 1) dd.c should produce some kind of warning when it wants to assign a
> driver to a device, but it can't because a driver is already assigned to
> a device
> 
> ie. change:
>       if (!dev->driver)
>           driver_probe_device(drv, dev);
> to:
>       if (!dev->driver)
>           driver_probe_device(drv, dev);
>       else
>           printk(KERN_WARNING "__driver_attach (%s): alreay registered
> with driver %s\n",
>              dev->bus_id, dev->driver->name);
> 
> 2) Possibly a device should be able to have more than one driver
> associated with it - so the AGP driver and EDAC could both use the
> device in question here (though this would probably be a sizable change).

I'm working on this change for PCI devices right now, but it's slow
going due to some other external things (OLS paper that I am woefully
behind on, etc...)

thanks,

greg k-h

Re: AGPGart / AMD K7

[Greg KH]

On Fri, Apr 20, 2007 at 04:33:42PM -0400, Dave Jones wrote:
> On Fri, Apr 20, 2007 at 04:22:06PM -0400, Preston A. Elder wrote:
>  > Dave, Greg,
>  > 
>  > Here is the trace with 2.6.20.6
>  > 
>  > I added back in my trace code, as you see.  As you can also see,
>  > agp_amdk7_probe is still not called.
> 
> Try looking down in __driver_attach()
> The fact that we're not calling the ->probe function is quite bizarre.
> 
> It could be this in __driver_attach
> 
>         if (!dev->driver)
>                 driver_probe_device(drv, dev);
> 
> Though that'd be odd.
> 
> Putting a #define DEBUG 1 in drivers/base/dd.c may also yield some clues.

Setting CONFIG_DEBUG_DRIVER automatically enables this also and might
provide some more hints.

thanks,

greg k-h

Re: AGPGart / AMD K7

[John Sigler]

Preston A. Elder wrote:

> What is this edac, btw?

AFAIK, EDAC stands for Error Detection and Correction.
http://bluesmoke.sourceforge.net/