Re: [PATCH 1/2] pciehp: add ACPI based slot detection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



* 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?

Thanks.

/ac

--
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

[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux