Kenji Kaneshige wrote:
> Alex Chiang wrote:
>> * Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx>:
>>> There is a problem that some non hot-pluggable PCIe slots are detected
>>> as hot-pluggable by pciehp on some platforms. The immediate cause of
>>> this problem is that hot-plug capable bit in the Slot Capabilities
>>> register is set even for non hot-pluggable slots on those platforms.
>>> It seems a BIOS/hardware problem, but we need workaround about that.
>>>
>>> Some of those platforms define hot-pluggable PCIe slots on ACPI
>>> namespace properly, while hot-plug capable bit in the Slot
>>> Capabilities register is set improperly. So using ACPI namespace
>>> information in pciehp to detect PCIe hot-pluggable slots would be a
>>> workaround.
>>>
>>> This patch adds 'pciehp_detect_mode' module option. When 'acpi' is
>>> specified, pciehp uses ACPI namespace information to detect PCIe
>>> hot-pluggable slots.
>>>
>>> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx>
>>>
>>> ---
>>> drivers/pci/hotplug/Makefile | 3 +
>>> drivers/pci/hotplug/pciehp.h | 15 ++++-
>>> drivers/pci/hotplug/pciehp_acpi.c | 114 ++++++++++++++++++++++++++++++++++++++
>>> drivers/pci/hotplug/pciehp_core.c | 1
>>> 4 files changed, 132 insertions(+), 1 deletion(-)
>>>
>>> Index: 20081212/drivers/pci/hotplug/pciehp.h
>>> ===================================================================
>>> --- 20081212.orig/drivers/pci/hotplug/pciehp.h
>>> +++ 20081212/drivers/pci/hotplug/pciehp.h
>>> @@ -220,11 +220,23 @@ struct hpc_ops {
>>> #include <acpi/actypes.h>
>>> #include <linux/pci-acpi.h>
>>>
>>> +extern void __init pciehp_acpi_slot_detection_init(void);
>>> +extern int pciehp_acpi_slot_detection_check(struct pci_dev *dev);
>>> +
>>> +static inline void pciehp_firmware_init(void)
>>> +{
>>> + pciehp_acpi_slot_detection_init();
>>> +}
>>> +
>>> static inline int pciehp_get_hp_hw_control_from_firmware(struct pci_dev *dev)
>>> {
>>> + int retval;
>>> u32 flags = (OSC_PCI_EXPRESS_NATIVE_HP_CONTROL |
>>> OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL);
>>> - return acpi_get_hp_hw_control_from_firmware(dev, flags);
>>> + retval = acpi_get_hp_hw_control_from_firmware(dev, flags);
>>> + if (retval)
>>> + return retval;
>>> + return pciehp_acpi_slot_detection_check(dev);
>>> }
>>>
>>> static inline int pciehp_get_hp_params_from_firmware(struct pci_dev *dev,
>>> @@ -235,6 +247,7 @@ static inline int pciehp_get_hp_params_f
>>> return 0;
>>> }
>>> #else
>>> +#define pciehp_firmware_init() do {} while (0)
>>> #define pciehp_get_hp_hw_control_from_firmware(dev) 0
>>> #define pciehp_get_hp_params_from_firmware(dev, hpp) (-ENODEV)
>>> #endif /* CONFIG_ACPI */
>>> Index: 20081212/drivers/pci/hotplug/pciehp_acpi.c
>>> ===================================================================
>>> --- /dev/null
>>> +++ 20081212/drivers/pci/hotplug/pciehp_acpi.c
>>> @@ -0,0 +1,114 @@
>>> +/*
>>> + * ACPI related functions for PCI Express Hot Plug driver.
>>> + *
>>> + * Copyright (C) 2008 Kenji Kaneshige
>>> + * Copyright (C) 2008 Fujitsu Limited.
>>> + *
>>> + * All rights reserved.
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or (at
>>> + * your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful, but
>>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
>>> + * NON INFRINGEMENT. See the GNU General Public License for more
>>> + * details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program; if not, write to the Free Software
>>> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>>> + *
>>> + */
>>> +
>>> +#include <linux/acpi.h>
>>> +#include "pciehp.h"
>>> +
>>> +#define PCIEHP_DETECT_PCIE (0)
>>> +#define PCIEHP_DETECT_ACPI (1)
>>> +#define PCIEHP_DETECT_DEFAULT PCIEHP_DETECT_PCIE
>>> +
>>> +static int slot_detection_mode;
>>> +static char *pciehp_detect_mode;
>>> +module_param(pciehp_detect_mode, charp, 0444);
>>> +MODULE_PARM_DESC(pciehp_detect_mode,
>>> + "Slot detection mode: pcie, acpi\n"
>>> + " pcie - Use PCIe based slot detection (default)\n"
>>> + " acpi - Use ACPI for slot detection\n");
>>> +
>> I would prefer that we do not add module params at all, and just
>> make the driver always autodetect the mode. You have the 'auto'
>> code in your 2nd patch, but it would be better to remove the
>> params and just always do the autodetect.
>>
>>> +static int is_ejectable(acpi_handle handle)
>>> +{
>>> + acpi_status status;
>>> + acpi_handle tmp;
>>> + unsigned long long removable;
>>> + status = acpi_get_handle(handle, "_ADR", &tmp);
>>> + if (ACPI_FAILURE(status))
>>> + return 0;
>>> + status = acpi_get_handle(handle, "_EJ0", &tmp);
>>> + if (ACPI_SUCCESS(status))
>>> + return 1;
>>> + status = acpi_evaluate_integer(handle, "_RMV", NULL, &removable);
>>> + if (ACPI_SUCCESS(status) && removable)
>>> + return 1;
>>> + return 0;
>>> +}
>>> +
>>> +static acpi_status
>>> +check_hotplug(acpi_handle handle, u32 lvl, void *context, void **rv)
>>> +{
>>> + int *found = (int *)context;
>>> + if (is_ejectable(handle)) {
>>> + *found = 1;
>>> + return AE_CTRL_TERMINATE;
>>> + }
>>> + return AE_OK;
>>> +}
>> This code is very similar to what we already have in acpiphp.
>>
>> Is there any way you could figure out how to make both pciehp and
>> acpiphp share this code?
>>
>
> Yes, we can put those functions in pcihp_acpi.c so that pciehp and
I wanted to say "acpi_pcihp.c", sorry.
Thanks,
Kenji Kaneshige
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
