cvs head arch/parisc/kernel/drivers.c issues

parisc-linux Richard Hirst 2 participants 13 messages Jul 30, 2005 – Aug 1, 2005

Share & tools

Richard Hirst

author

2005-07-30 23:10 UTC #0 lore raw

Trying to boot cvs head on a C360; this is what happens:

do_device_inventory() calls system_map_inventory(), which calls pdc_system_map_find_mods() which returns devices in the following order:

system_map_inventory() f2003000, -1 -1 -1 -1 8 0 + 63 system_map_inventory() ffd00000, -1 -1 -1 -1 -1 8 + 16 system_map_inventory() ffd05000, -1 -1 -1 -1 8 16 + 4 system_map_inventory() ffd06000, -1 -1 -1 -1 8 16 + 5 system_map_inventory() ffd02000, -1 -1 -1 -1 8 16 + 0 system_map_inventory() ffd04000, -1 -1 -1 -1 8 16 + 1 system_map_inventory() ffd08000, -1 -1 -1 -1 8 16 + 7 system_map_inventory() ffd08100, -1 -1 -1 -1 8 16 + 8 system_map_inventory() f203f000, -1 -1 -1 -1 -1 8 + 63 system_map_inventory() f103f000, -1 -1 -1 -1 -1 10 + 63 system_map_inventory() f1000000, -1 -1 -1 -1 -1 10 + 0 system_map_inventory() f2000000, -1 -1 -1 -1 -1 8 + 0

so the first device node we try to create is 8/0/63. That causes nodes 8 and 8/0 to get created by create_parisc_device() calling alloc_tree_node() as it walks the path. Those two nodes are left with "dev->id.hw_type = HPHW_FAULTY" by create_tree_node().

Next we try to create device node 8/16. We end up in match_by_id() to check if a node for '8' exists already. Because that node is marked HPHW_FAULTY it gets ignored, and alloc_tree_node() ends up creating a second node for device '8'. We then go on to try and register this second node '8' with sysfs and things start falling apart because the node already exists.

I tried making match_by_id() treat HPHW_FAULTY nodes as valid and then the boot gets much further:

Found devices:
1. Raven U/L2 Dino RS-232 at 0xf2003000 [8/0/63] { 10, 0x0, 0x006, 0x0008c }
2. Raven+ w SE FWSCSI Core RS-232 at 0xffd05000 [8/16/4] { 10, 0x0, 0x056, 0x0008c }
3. Raven+ w SE FWSCSI Core SCSI at 0xffd06000 [8/16/5] { 10, 0x0, 0x056, 0x00082 }
4. Raven+ w SE FWSCSI Core Centronics at 0xffd02000 [8/16/0] { 10, 0x0, 0x056, 0x00074 }, additional addresses: 0xffd01000 0xffd03000
5. Raven+ w SE FWSCSI Core Audio at 0xffd04000 [8/16/1] { 10, 0x4, 0x056, 0x0007b }
6. Raven+ w SE FWSCSI Core PS/2 Port at 0xffd08000 [8/16/7] { 10, 0x0, 0x056, 0x00084 }
7. Raven+ w SE FWSCSI Core PS/2 Port at 0xffd08100 [8/16/8] { 10, 0x0, 0x056, 0x00084 }
8. Dino PCI Bridge at 0xf2000000 [8/0] { 13, 0x3, 0x680, 0x0000a }, additional addresses: 0xf2800000
9. Raven+ w SE FWSCSI Core BA at 0xffd00000 [8/16] { 11, 0x0, 0x056, 0x00081 }, additional addresses: 0xffd0c000 0xffc00000
10. U2-IOA BC GSC+ Port at 0xf203f000 [8/63] { 7, 0x1, 0x501, 0x0000c }
11. Raven U/L2 Dino PS/2 Port at 0xf2001000 [8/1] { 10, 0x0, 0x006, 0x00096 }
12. U2-IOA BC GSC+ Port at 0xf103f000 [10/63] { 7, 0x1, 0x501, 0x0000c }
13. Cujo PCI Bridge at 0xf1000000 [10/0] { 13, 0x1, 0x682, 0x0000a }, additional addresses: 0xf1800000 0xf6000000
14. Dino RS-232 at 0xf1003000 [10/3] { 10, 0x0, 0x007, 0x0008c }
15. U2-IOA BC Runway Port at 0xfff88000 [8] { 12, 0xf, 0x580, 0x0000b }
16. U2-IOA BC Runway Port at 0xfff8a000 [10] { 12, 0xf, 0x580, 0x0000b }
17. Raven W 360 (9000/780) at 0xfffa0000 [32] { 0, 0x0, 0x5c6, 0x00004 }
18. Memory at 0xfffb1000 [49] { 1, 0x0, 0x097, 0x00009 }
CPU(s): 1 x PA8500 (PCX-W) at 367.111100 MHz
Setting cache flush threshold to 720 (1 CPUs online)
Found U2 at 0xfff88000
Found U2 at 0xfff8a000
Lasi version 0 at 0xffd00000 found.
Dino version 3.1 found at 0xf2000000
DEV: registering device: ID = 'pci0000:00'
DEV: registering device: ID = '0000:00:13.0'
DEV: registering device: ID = '0000:00:14.0'
Cujo version 2.0 found at 0xf1000000
Enabling Cujo 2.0 bug workaround
DEV: registering device: ID = 'pci0000:01'
DEV: registering device: ID = '0000:01:04.0'
SCSI subsystem initialized
TC classifier action (bugs to [email protected] cc [email protected])
unwind_init: start = 0x10423e20, end = 0x10447940, entries = 9138
inotify syscall
VFS: Disk quotas dquot_6.5.1
Dquot-cache hash table entries: 1024 (order 0, 4096 bytes)
Initializing Cryptographic API
PDC Stable Storage facility v0.09

Backtrace:
 [<10128170>] __wake_up+0x54/0x84
 [<1014b3b8>] __queue_work+0x60/0x80
 [<10142484>] run_timer_softirq+0x148/0x240
 [<1013c2dc>] __do_softirq+0x140/0x194
 [<10104958>] __lock_text_end+0x58/0x64
 [<1010e068>] intr_return+0x0/0x24
 [<103d7990>] klist_next+0x8/0x78
 [<102bea2c>] device_for_each_child+0x30/0x8c
 [<101122c0>] descend_children+0x20/0x50
 [<102bea50>] device_for_each_child+0x54/0x8c
 [<101122c0>] descend_children+0x20/0x50
 [<102bea50>] device_for_each_child+0x54/0x8c
 [<101122c0>] descend_children+0x20/0x50
 [<10112310>] for_each_padev+0x20/0x2c
 [<1011292c>] parse_tree_node+0x48/0x64
 [<10112a08>] check_parent+0x64/0x154

Bad Address (null pointer deref?): Code=6 regs=12ab5380 (Addr=081f0242)

Kernel panic - not syncing: Bad Address (null pointer deref?)

The devices are not printed in a logical order (e.g. 8/0/63 is before 8/0) which I think is a change for earlier kernels.

The change I made to match_by_id() was just to disable the call to check_dev().

Anyone got any thoughts or suggestions on drivers.c or the device discovery order?

Thanks,
Richard

_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Kyle McMartin

2005-07-31 06:12 UTC #1 lore raw

Quoted 3 lines · Richard Hirst

PDC Stable Storage facility v0.09
Backtrace:
 [<10128170>] __wake_up+0x54/0x84

Try disabling CONFIG_PDC_STABLE_STORAGE until this gets worked out.

Signature · 5 lines

Kyle McMartin
_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Richard Hirst

author

2005-07-31 09:00 UTC #2 lore raw

Quoted 6 lines · Kyle McMartin

Quoted 3 lines · Richard Hirst
PDC Stable Storage facility v0.09
Backtrace:
 [<10128170>] __wake_up+0x54/0x84
Try disabling CONFIG_PDC_STABLE_STORAGE until this gets worked out.

Thanks, gets a bit further now:

...
...
16. U2-IOA BC Runway Port at 0xfff8a000 [10] { 12, 0xf, 0x580, 0x0000b }
17. Raven W 360 (9000/780) at 0xfffa0000 [32] { 0, 0x0, 0x5c6, 0x00004 }
18. Memory at 0xfffb1000 [49] { 1, 0x0, 0x097, 0x00009 }
CPU(s): 1 x PA8500 (PCX-W) at 367.111100 MHz
Setting cache flush threshold to 440 (1 CPUs online)
Found U2 at 0xfff88000
Found U2 at 0xfff8a000
Lasi version 0 at 0xffd00000 found.
Dino version 3.1 found at 0xf2000000
DEV: registering device: ID = 'pci0000:00'
DEV: registering device: ID = '0000:00:13.0'
DEV: registering device: ID = '0000:00:14.0'
Cujo version 2.0 found at 0xf1000000
Enabling Cujo 2.0 bug workaround
DEV: registering device: ID = 'pci0000:01'
DEV: registering device: ID = '0000:01:04.0'
SCSI subsystem initialized
TC classifier action (bugs to [email protected] cc [email protected])
unwind_init: start = 0x10422e20, end = 0x10446880, entries = 9126 inotify syscall
VFS: Disk quotas dquot_6.5.1
Dquot-cache hash table entries: 1024 (order 0, 4096 bytes) Initializing Cryptographic API
Soft power switch enabled, polling @ 0xf0140000.
STI GSC/PCI core graphics driver Version 0.9a
STI PCI graphic ROM found at f1ff0000 (64 kB), fb at f6000000 (32 MB)
id 2d08c0a7-9a02587, conforms to spec rev. 8.0a
graphics card name: PCI_GRAFFITIX1024
sticon: Initializing STI text console.
Console: switching to colour STI console 128x48
Console: switching to colour frame buffer device 128x48
fb0: stifb 1024x768-8 frame buffer device, PCI_GRAFFITIX1024, id: 2d08c0a7, mmio : 0xf6100000
Generic RTC Driver v1.07
serio: GSC PS/2 keyboard port at 0xffd08000 irq 20 @ 8:16:7

Then it hangs. TOC shows it is in _spin_lock_irqsave() with a return address in gscps2_interrupt().

Richard

_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Kyle McMartin

2005-07-31 18:47 UTC #3 lore raw

Quoted 3 lines · Richard Hirst

Then it hangs. TOC shows it is in _spin_lock_irqsave() with a return address in gscps2_interrupt().

Are you compiling with SMP? I seem to recall some spinlock reorganization patches may have gone in that we have not yet merged...

Signature · 5 lines

Kyle McMartin
_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Richard Hirst

author

2005-08-01 20:02 UTC #4 lore raw

Quoted 7 lines · Kyle McMartin

Quoted 3 lines · Richard Hirst
Then it hangs. TOC shows it is in _spin_lock_irqsave() with a return address in gscps2_interrupt().
Are you compiling with SMP? I seem to recall some spinlock reorganization patches may have gone in that we have not yet merged...

Just for the mail list archive... turning off SMP makes this problem go away.

Richard

_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Richard Hirst

author

2005-07-31 19:32 UTC #5 lore raw

In reply to Richard Hirst

With the following patch I can boot my C360, although there is not ps/2 (obviously, given the second part of the diff):

Index: arch/parisc/kernel/drivers.c
===================================================================
RCS file: /var/cvs/linux-2.6/arch/parisc/kernel/drivers.c,v retrieving revision 1.25

parisc/kernel/drivers.c 31 Jul 2005 19:22:21 -0000 renamed from parisc/kernel/drivers.c 26 Jul 2005 23:55:47 -0000 1.25 +3 -5

diff -u -r1.25 drivers.c
@@ -448,11 +448,9 @@
        struct parisc_device * pdev = to_parisc_device(dev);
        struct match_id_data * d = data;
 
-       if (check_dev(pdev)) {
-               if (pdev->hw_path == d->id) {
-                       d->dev = pdev;
-                       return 1;
-               }
+       if (pdev->hw_path == d->id) {
+               d->dev = pdev;
+               return 1;
        }
        return 0;
 }

Index: drivers/input/serio/gscps2.c
===================================================================
RCS file: /var/cvs/linux-2.6/drivers/input/serio/gscps2.c,v retrieving revision 1.16

input/serio/gscps2.c 31 Jul 2005 19:22:29 -0000 renamed from input/serio/gscps2.c 18 Mar 2005 13:16:54 -0000 1.16 +1 -1

diff -u -r1.16 gscps2.c
@@ -334,7 +334,7 @@
        unsigned long hpa = dev->hpa;
        int ret;
 
-       if (!dev->irq)
+       //if (!dev->irq)
                return -ENODEV;
 
        /* Offset for DINO PS/2. Works with LASI even */

I think the drivers.c part is probably reasonable, although it might be masking some underlying problem with device discovery. If I enable the gscps2 driver the system hangs, possibly with an interrupt storm.

I noticed the 2.6.13 kernel is using completely different IRQ allocations from 2.6.8 (cat /proc/interrupts); is this expected?

2.6.8

           CPU00
 32:       9151      PARISC-CPU  timer
 33:          0      PARISC-CPU  IPI
 34:        512      PARISC-CPU  lasi
 35:       2053      PARISC-CPU  Dino [8/0]
 36:          0      PARISC-CPU  Cujo
 69:         31            Lasi  GSC PS2 keyboard, GSC PS2 mouse
 86:         68            Lasi  lasi700
 90:        413            Lasi  serial
 96:         30      Dino [8/0]  eth0
 99:       2023      Dino [8/0]  sym53c8xx

2.6.13

 16:       2020        GSC-ASIC  serial
 17:         54        GSC-ASIC  lasi700
 22:       2532         GSC-PCI  sym53c8xx
 23:         36         GSC-PCI  eth0
 64:      94680             CPU  timer
 65:          0             CPU  IPI
 66:       2074             CPU  lasi
 67:       2566             CPU  Dino
 68:          0             CPU  Cujo

When I enable gscps2 it is trying to use IRQ 20.

Richard

_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Kyle McMartin

2005-07-31 19:38 UTC #6 lore raw

Quoted 3 lines · Richard Hirst

I noticed the 2.6.13 kernel is using completely different IRQ allocations from 2.6.8 (cat /proc/interrupts); is this expected?

Yeah, willy rewrote our interrupt support sometime around 2.6.10.

Signature · 5 lines

Kyle McMartin
_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Richard Hirst

author

2005-07-31 20:01 UTC #7 lore raw

Quoted 6 lines · Kyle McMartin

Quoted 3 lines · Richard Hirst
I noticed the 2.6.13 kernel is using completely different IRQ allocations from 2.6.8 (cat /proc/interrupts); is this expected?
Yeah, willy rewrote our interrupt support sometime around 2.6.10.

OK; booted 2.6.12-pa2 and and got the same allocations, with ps/2 on irq 20.

Richard

_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Kyle McMartin

2005-07-31 20:13 UTC #8 lore raw

Quoted 3 lines · Richard Hirst

OK; booted 2.6.12-pa2 and and got the same allocations, with ps/2 on irq 20.

I think the problem is that check_dev isn't properly passing the "next" device if it's faulty.

http://cvs.parisc-linux.org/linux-2.6/arch/parisc/kernel/drivers.c?rev=1.9&view=markup

For how it used to look.

Cheers,

Signature · 5 lines

Kyle McMartin
_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Kyle McMartin

2005-07-31 20:23 UTC #9 lore raw

In reply to Richard Hirst

Quoted 3 lines · Richard Hirst

OK; booted 2.6.12-pa2 and and got the same allocations, with ps/2 on irq 20.

Can you try something like this?

#define check_dev(padev) (padev->id.hw_type != HPHW_FAULTY) \
? padev : next_device(padev)

static struct parisc_device *

next_device(struct parisc_device *padev) {
	struct device *dev;
	struct klist_iter i;
	struct parisc_device *next = NULL;

	if (!padev)
		return NULL;

	klist_iter_init(&padev->dev.klist_children, &i);
	
	dev = next_device(&i);
	if (dev)
		next = to_parisc_device(dev);
	klist_iter_exit(&i);

	return next;
}

Signature · 5 lines

Kyle McMartin
_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Kyle McMartin

2005-07-31 20:27 UTC #10 lore raw

Quoted 2 lines · Kyle McMartin

static struct parisc_device *
next_device(struct parisc_device *padev) {

...

Quoted 1 line

	dev = next_device(&i);

Err, oops. The function should be next_dev, and next_device should be a la the one in sba_iommu.c and gsc.c.

Oops,

Signature · 5 lines

Kyle McMartin
_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Richard Hirst

author

2005-08-01 13:48 UTC #11 lore raw

Quoted 10 lines · Kyle McMartin

Quoted 2 lines · Kyle McMartin
static struct parisc_device *
next_device(struct parisc_device *padev) {
...
Quoted 1 line
	dev = next_device(&i);
Err, oops. The function should be next_dev, and next_device should be a la the one in sba_iommu.c and gsc.c.

Not tried this yet, but I'm unclear as to which problem you are trying to fix...

a) "Found devices" display order

b) an alternative to my match_by_id() change that removed a check_dev() call

c) a fix to gscps2.c hanging my C360 (unlikely, I guess)

d) multiple of the above :-)

The old version of drivers.c had a for_each_padev() that did a depth first walk of the tree reporting parents before children. The new code is reporting children before parents which is what causes (a) above, I think.

check_dev() is used in several places now, all of which just check for null/non-null return, so I'm not convinced that making it walk the tree via next_dev() is right; take this for example:

static int print_one_device(struct device * dev, void * data)

{
        struct parisc_device * pdev = to_parisc_device(dev);

        if (check_dev(pdev))
                print_parisc_device(pdev);
        return 0;
}

if check_dev(pdev) might call next_dev() and return some other pdev, does it make sense to be calling print_parisc_device() on the original pdev?

I'm happy to poke at this some more anyway,

Richard

_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Richard Hirst

author

2005-08-01 14:53 UTC #12 lore raw

The attached patch fixes 'Found devices' order to display parents before children. It's still not ideal, as the children are not reported in numerical order but I don't see a clean way to fix that.

I included the patch to remove the check_dev() call from match_by_id() too; that still seems like the right thing to do to me.

My C360 now reports:

Found devices:
1. U2-IOA BC Runway Port at 0xfff88000 [8] { 12, 0xf, 0x580, 0x0000b }
2. Dino PCI Bridge at 0xf2000000 [8/0] { 13, 0x3, 0x680, 0x0000a }, additional
3. Raven U/L2 Dino RS-232 at 0xf2003000 [8/0/63] { 10, 0x0, 0x006, 0x0008c }
4. Raven+ w SE FWSCSI Core BA at 0xffd00000 [8/16] { 11, 0x0, 0x056, 0x00081 },
5. Raven+ w SE FWSCSI Core RS-232 at 0xffd05000 [8/16/4] { 10, 0x0, 0x056, 0x00}
6. Raven+ w SE FWSCSI Core SCSI at 0xffd06000 [8/16/5] { 10, 0x0, 0x056, 0x0008}
7. Raven+ w SE FWSCSI Core Centronics at 0xffd02000 [8/16/0] { 10, 0x0, 0x056,
8. Raven+ w SE FWSCSI Core Audio at 0xffd04000 [8/16/1] { 10, 0x4, 0x056, 0x000}
9. Raven+ w SE FWSCSI Core PS/2 Port at 0xffd08000 [8/16/7] { 10, 0x0, 0x056, 0}
10. Raven+ w SE FWSCSI Core PS/2 Port at 0xffd08100 [8/16/8] { 10, 0x0, 0x056, }
11. U2-IOA BC GSC+ Port at 0xf203f000 [8/63] { 7, 0x1, 0x501, 0x0000c }
12. Raven U/L2 Dino PS/2 Port at 0xf2001000 [8/1] { 10, 0x0, 0x006, 0x00096 }
13. U2-IOA BC Runway Port at 0xfff8a000 [10] { 12, 0xf, 0x580, 0x0000b }
14. U2-IOA BC GSC+ Port at 0xf103f000 [10/63] { 7, 0x1, 0x501, 0x0000c }
15. Cujo PCI Bridge at 0xf1000000 [10/0] { 13, 0x1, 0x682, 0x0000a }, addition
16. Dino RS-232 at 0xf1003000 [10/3] { 10, 0x0, 0x007, 0x0008c }
17. Raven W 360 (9000/780) at 0xfffa0000 [32] { 0, 0x0, 0x5c6, 0x00004 }
18. Memory at 0xfffb1000 [49] { 1, 0x0, 0x097, 0x00009 }

Richard

Index: arch/parisc/kernel/drivers.c
===================================================================
RCS file: /var/cvs/linux-2.6/arch/parisc/kernel/drivers.c,v retrieving revision 1.25

parisc/kernel/drivers.c +10 ~~-19~~

diff -u -r1.25 drivers.c
@@ -59,19 +59,11 @@
 static int descend_children(struct device * dev, void * data)
 {
 	struct recurse_struct * recurse_data = (struct recurse_struct *)data;
-	int ret;
 
-	/*
-	 * First, descend down the tree.
-	 */
-	ret = device_for_each_child(dev, recurse_data, descend_children);
-	if (ret)
-		return ret;
-
-	/*
-	 * Now, iterate over the children and call the function.
-	 */
-	return device_for_each_child(dev, recurse_data->obj, recurse_data->fn);
+	if (recurse_data->fn(dev, recurse_data->obj))
+		return 1;
+	else
+		return device_for_each_child(dev, recurse_data, descend_children);
 }
 
 /**

@@ -80,7 +72,8 @@
  *	@data:	Data to pass to the called function.
  *
  *	This performs a depth-first traversal of the tree, calling the
- *	function passed for each node.
+ *	function passed for each node.  It calls the function for parents
+ *	before children.
  */
 
 static int for_each_padev(int (*fn)(struct device *, void *), void * data)

@@ -89,7 +82,7 @@
 		.obj	= data,
 		.fn	= fn,
 	};
-	return descend_children(&root, &recurse_data);
+	return device_for_each_child(&root, &recurse_data, descend_children);
 }
 
 /**

@@ -448,11 +441,9 @@
 	struct parisc_device * pdev = to_parisc_device(dev);
 	struct match_id_data * d = data;
 
-	if (check_dev(pdev)) {
-		if (pdev->hw_path == d->id) {
-			d->dev = pdev;
-			return 1;
-		}
+	if (pdev->hw_path == d->id) {
+		d->dev = pdev;
+		return 1;
 	}
 	return 0;
 }

_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux