Hi Kenji-san,
* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
Thanks for testing. :)
Hm, you are right. On my machine, if I load pciehp first and
acpiphp second (even without loading pci_slot), I will see the
following:
[root@canola slots]# ls
0016_0006 0197_0005 10 3 4 7 8 9
[root@canola slots]# lsmod | grep pci_slot
[root@canola slots]# lsmod | grep hp
acpiphp 115984 0
pciehp 140616 0
pci_hotplug 123972 2 acpiphp,pciehp
On the other hand, if I do load pci_slot first, and then pciehp,
you are right, I will see something like this:
[root@canola slots]# ls
1 10 2 3 4 5 6 7 8 9
[root@canola slots]# lsmod | grep pci_slot
pci_slot 74436 0
[root@canola slots]# lsmod | grep hp
pciehp 140616 0
pci_hotplug 123972 1 pciehp
But I do agree, people don't need to load pci_slot at all if they
don't want it, and they won't be bothered.
Now this is very curious. The relevant line in pci_slot is:
check_slot()
status = acpi_evaluate_integer(handle, "_SUN", NULL, sun);
if (ACPI_FAILURE(status))
return -1;
Why does your firmware return the error information inside sun,
instead of returning an error in status? That doesn't seem right
to me...
Applying this patch causes me to only detect populated slots in
my system, which isn't what I want -- otherwise, I could have
just enumerated the PCI bus and found the devices that way. :)
Maybe on your machine, checking existence of _STA might do the
right thing, but I don't think we should actually be looking at
any of the actual bits returned.
If we check ACPI_STA_DEVICE_PRESENT, then we will not detect
empty slots on my system. Can you try this patch to see if at
least the first call to acpi_evaluate_integer helps? If that
doesn't help, maybe the second block will help you, but it breaks
my machine...
Thanks.
/ac
diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c
index 724f4f0..63a4dc8 100644
--- a/drivers/acpi/pci_slot.c
+++ b/drivers/acpi/pci_slot.c
@@ -55,9 +65,21 @@ static struct acpi_pci_driver acpi_pci_slot_driver = {
static int
check_slot(acpi_handle handle, int *device, unsigned long *sun)
{
- unsigned long adr;
+ unsigned long adr, sta;
acpi_status status;
+ /* Doesn't seem to hurt anything on hp systems */
+ status = acpi_evaluate_integer(handle, "_STA", NULL, &sta);
+ if (ACPI_FAILURE(status))
+ return -1;
+
+ /* This code causes us to fail to detect empty slots, so
+ * commented out for now.
+ *
+ if (!(sta & ACPI_STA_DEVICE_PRESENT))
+ return -1;
+ */
+
status = acpi_evaluate_integer(handle, "_ADR", NULL, &adr);
if (ACPI_FAILURE(status))
return -1;
-
