Re: [PATCH 0/5][RFC] Physical PCI slot objects

Kenji Kaneshige
2007-11-14T11:47:46Z

Hi,

I tried the series of patches and I encountered some problems.
Since I don't have enough time to investigate them, I can only
report them at present. My environment was ia64 machine with 64
hotplug slots (shpc & pcie). Those slots can be handled by
acpiphp too, though I have not tried it yet.

- Problem(1)
Two kind of Call Traces were displayed so many times at the
boot time. Those are attached below. I guess it relate to
the ACPI Namespace definition.

- Problem(2)
Many of the shpc slot initializations failed with the
following error messages. I guess the cause is Problem(1).

shpchp: pci_hp_register failed with error -17
shpchp: shpchp: slot initialization failed

- Problem(3)
All of the pcie slot initializations failed with the
following error messages. I guess the cause is Problem(1).

pciehp: pci_hp_register failed with error -17
pciehp: pciehp: slot initialization failed

BTW, I have some comments about the patches.

- I think your patches assume that all the hotplug slots have
ACPI _SUN method regardless of the hotplug controller type,
because pci_slot.c is the only place to call pci_slot_create().
But I think there are hotplug slots without _SUN. If the
hotplug slot doesn't have _SUN, slot initialization by the
hotplug controller driver doesn't work (does it?), because
there are no slot directory.

- I think pci_slot.o should be kernel module so that users can
select to enable or disable this functionality.

Here is the Call Trace mentioned above:

Unable to register kobject 1024<3>pci_slot: pci_create_slot returned -22
ACPI: Invalid ACPI Bus context for device <NULL>
sysfs: duplicate filename '1024' can not be created
WARNING: at fs/sysfs/dir.c:424 sysfs_add_one()

Call Trace:
[<a000000100014b60>] show_stack+0x40/0xa0
sp=e000014101befaf0 bsp=e000014101be0f60
[<a000000100014bf0>] dump_stack+0x30/0x60
sp=e000014101befcc0 bsp=e000014101be0f48
[<a0000001001d6990>] sysfs_add_one+0xb0/0x240
sp=e000014101befcc0 bsp=e000014101be0f18
[<a0000001001d7900>] create_dir+0x80/0x100
sp=e000014101befcc0 bsp=e000014101be0ee0
[<a0000001001d7a30>] sysfs_create_dir+0xb0/0x100
sp=e000014101befce0 bsp=e000014101be0ec0
[<a0000001003b4f50>] kobject_add+0x250/0x460
sp=e000014101befcf0 bsp=e000014101be0e80
[<a0000001003b52d0>] kobject_register+0x50/0xa0
sp=e000014101befcf0 bsp=e000014101be0e58
[<a0000001003d6f40>] pci_create_slot+0x140/0x260
sp=e000014101befcf0 bsp=e000014101be0e10
[<a00000010043acd0>] register_slot+0x170/0x1c0
sp=e000014101befcf0 bsp=e000014101be0dc0
[<a000000100419ef0>] acpi_ns_walk_namespace+0x2b0/0x300
sp=e000014101befd20 bsp=e000014101be0d50
[<a000000100415a50>] acpi_walk_namespace+0x90/0xc0
sp=e000014101befd20 bsp=e000014101be0d00
[<a00000010043ab20>] acpi_pci_slot_add+0x1c0/0x200
sp=e000014101befd20 bsp=e000014101be0cd8
[<a0000001004367e0>] acpi_pci_register_driver+0xe0/0x160
sp=e000014101befd30 bsp=e000014101be0ca8
[<a000000100920620>] acpi_pci_slot_init+0x20/0x40
sp=e000014101befd30 bsp=e000014101be0c90
[<a0000001008f06e0>] kernel_init+0x460/0x7e0
sp=e000014101befd30 bsp=e000014101be0c48
[<a000000100013110>] kernel_thread_helper+0x30/0x60
sp=e000014101befe30 bsp=e000014101be0c20
[<a0000001000089c0>] start_kernel_thread+0x20/0x40
sp=e000014101befe30 bsp=e000014101be0c20
kobject_add failed for 1024 with -EEXIST, don't try to register things with the
same name in the same directory.

Call Trace:
[<a000000100014b60>] show_stack+0x40/0xa0
sp=e000014101befb20 bsp=e000014101be0ed8
[<a000000100014bf0>] dump_stack+0x30/0x60
sp=e000014101befcf0 bsp=e000014101be0ec0
[<a0000001003b50e0>] kobject_add+0x3e0/0x460
sp=e000014101befcf0 bsp=e000014101be0e80
[<a0000001003b52d0>] kobject_register+0x50/0xa0
sp=e000014101befcf0 bsp=e000014101be0e58
[<a0000001003d6f40>] pci_create_slot+0x140/0x260
sp=e000014101befcf0 bsp=e000014101be0e10
[<a00000010043acd0>] register_slot+0x170/0x1c0
sp=e000014101befcf0 bsp=e000014101be0dc0
[<a000000100419ef0>] acpi_ns_walk_namespace+0x2b0/0x300
sp=e000014101befd20 bsp=e000014101be0d50
[<a000000100415a50>] acpi_walk_namespace+0x90/0xc0
sp=e000014101befd20 bsp=e000014101be0d00
[<a00000010043ab20>] acpi_pci_slot_add+0x1c0/0x200
sp=e000014101befd20 bsp=e000014101be0cd8
[<a0000001004367e0>] acpi_pci_register_driver+0xe0/0x160
sp=e000014101befd30 bsp=e000014101be0ca8
[<a000000100920620>] acpi_pci_slot_init+0x20/0x40
sp=e000014101befd30 bsp=e000014101be0c90
[<a0000001008f06e0>] kernel_init+0x460/0x7e0
sp=e000014101befd30 bsp=e000014101be0c48
[<a000000100013110>] kernel_thread_helper+0x30/0x60
sp=e000014101befe30 bsp=e000014101be0c20
[<a0000001000089c0>] start_kernel_thread+0x20/0x40
sp=e000014101befe30 bsp=e000014101be0c20
Unable to register kobject 1024<3>pci_slot: pci_create_slot returned -22

Thanks,
Kenji Kaneshige

Alex Chiang ????????:
> Hello,
>
> [this patch series touches a few subsystems; hopefully I got all
> the right maintainers]
>
> Recently, Matthew Wilcox sent out the following mail about PCI
> slots:
>
> http://marc.info/?l=linux-pci&m=119432330418980&w=2
>
> The following patch series is a rough first cut at implementing
> the ideas he outlined, namely, that PCI slots are physical
> objects that we care about, independent of their hotplug
> capabilities.
>
> We introduce a struct pci_slot as a first-class citizen, and turn
> hotplug_slot into a subsidiary structure. A brief glimpse at the
> potential for cleanup is given, as we modify the existing hotplug
> drivers and remove the multiple get_address() methods. Certainly
> more cleanup can be done with this new structure.
>
> Additionally, we introduce an ACPI PCI slot driver, which will
> detect all physical PCI slots in the system (as described by
> ACPI). De-coupling the notion of a physical slot from its hotplug
> capability allows users to understand the physical geography of
> their machines, whether their slots are hotpluggable or not.
>
> This patch series builds heavily on Willy's prior work (with a
> bit of the typical refresh needed when aiming at the moving
> target of the kernel). The ACPI PCI slot driver is new.
>
> I have tested this series on both ia64 (hp rx6600) and x86 (hp
> dl380g). On ia64, system firmware provides _SUN methods for the
> PCI slots and we get entries in /sys/bus/pci/slots/. On my x86
> machine, firmware didn't seem to provide a _SUN, so we don't get
> any entries in /sys/bus/pci/slots/, but nothing really bad
> happens either. ;)
>
> Comments/feedback are appreciated.
>
> Thanks.
>
> /ac
>
>
>