Bjorn Helgaas wrote: > These patches clean up and unify the PCI slot configuration across > the acpiphp, pciehp, and shpchp drivers. When these drivers detect > a newly-added device, they configure the bus. On ACPI systems, this > uses PCI bus settings from _HPP or _HPX methods. > > Previously, each driver handled this separately with similar but not > quite identical code. This series adds a single pci_configure_slot() > function and changes each driver to use it. > > Changes from initial post to v2: > - use static inline empty function rather than #define for non-ACPI (Eike) > - export pci_configure_slot() > - add comment about why we don't program default PCI settings for PCIe > - tidy up PCIe setting check for PCIe device > - move pci_configure_slot() and related functions to acpi_pcihp.c (Kenji) > - make acpi_get_hp_params() static > > --- > > Bjorn Helgaas (9): > PCI hotplug: acpiphp: remove superfluous _HPP/_HPX evaluation > PCI hotplug: acpiphp: don't cache hotplug_params in acpiphp_bridge > PCI hotplug: rename acpi_get_hp_params_from_firmware() to acpi_get_hp_params() > PCI hotplug: add pci_configure_slot() > PCI hotplug: pciehp: use generic pci_configure_slot() > PCI hotplug: shpchp: use generic pci_configure_slot() > PCI hotplug: acpiphp: use generic pci_configure_slot() > PCI hotplug: clean up acpi_run_hpp() > PCI hotplug: make acpi_get_hp_params() static > > > drivers/pci/hotplug/acpi_pcihp.c | 269 ++++++++++++++++++++++++++---------- > drivers/pci/hotplug/acpiphp.h | 3 > drivers/pci/hotplug/acpiphp_glue.c | 95 +------------ > drivers/pci/hotplug/pciehp.h | 9 - > drivers/pci/hotplug/pciehp_pci.c | 132 ------------------ > drivers/pci/hotplug/pcihp_slot.c | 0 > drivers/pci/hotplug/shpchp.h | 9 - > drivers/pci/hotplug/shpchp_pci.c | 62 -------- > include/linux/pci_hotplug.h | 5 - > 9 files changed, 205 insertions(+), 379 deletions(-) > create mode 100644 drivers/pci/hotplug/pcihp_slot.c > Hi Bjorn-san, I tried the set of patches on both SHPC and PCI-E hotplug slots using shpchp driver and pciehp driver respectively and confirmed it works well (I also confirmed there are no difference on hot-plugging in PCI/PCIE configuration space between before and after applying set of patches). Unfortunately, I didn't test acpiphp due to the lack of testing environment. So I can do only reviewing for acpiphp. In my understanding, your patches are not only for cleanup but it also enables acpiphp to program _HPX type2 parameters on PCI-E hotplug slots. If I must say something about acpiphp, it would be good if there is an description about it. By the way, I think we can do further cleanup regarding hotplug parameters. I think struct hpp_type? and struct hotplug_params in include/linux/pci_hotplug.h can be moved to acpi_pcihp.c like attached below. Anyway, your patches looks very good to me. Reviewed-by: Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx> Tested-by: Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx> Thanks, Kenji Kaneshige --- drivers/pci/hotplug/acpi_pcihp.c | 47 +++++++++++++++++++++++++++++++++++++++ include/linux/pci_hotplug.h | 47 --------------------------------------- 2 files changed, 47 insertions(+), 47 deletions(-) Index: 20090827/drivers/pci/hotplug/acpi_pcihp.c =================================================================== --- 20090827.orig/drivers/pci/hotplug/acpi_pcihp.c +++ 20090827/drivers/pci/hotplug/acpi_pcihp.c @@ -46,6 +46,53 @@ #define METHOD_NAME__SUN "_SUN" #define METHOD_NAME_OSHP "OSHP" +/* PCI Setting Record (Type 0) */ +struct hpp_type0 { + u32 revision; + u8 cache_line_size; + u8 latency_timer; + u8 enable_serr; + u8 enable_perr; +}; + +/* PCI-X Setting Record (Type 1) */ +struct hpp_type1 { + u32 revision; + u8 max_mem_read; + u8 avg_max_split; + u16 tot_max_split; +}; + +/* PCI Express Setting Record (Type 2) */ +struct hpp_type2 { + u32 revision; + u32 unc_err_mask_and; + u32 unc_err_mask_or; + u32 unc_err_sever_and; + u32 unc_err_sever_or; + u32 cor_err_mask_and; + u32 cor_err_mask_or; + u32 adv_err_cap_and; + u32 adv_err_cap_or; + u16 pci_exp_devctl_and; + u16 pci_exp_devctl_or; + u16 pci_exp_lnkctl_and; + u16 pci_exp_lnkctl_or; + u32 sec_unc_err_sever_and; + u32 sec_unc_err_sever_or; + u32 sec_unc_err_mask_and; + u32 sec_unc_err_mask_or; +}; + +struct hotplug_params { + struct hpp_type0 *t0; /* Type0: NULL if not available */ + struct hpp_type1 *t1; /* Type1: NULL if not available */ + struct hpp_type2 *t2; /* Type2: NULL if not available */ + struct hpp_type0 type0_data; + struct hpp_type1 type1_data; + struct hpp_type2 type2_data; +}; + static int debug_acpi; static acpi_status Index: 20090827/include/linux/pci_hotplug.h =================================================================== --- 20090827.orig/include/linux/pci_hotplug.h +++ 20090827/include/linux/pci_hotplug.h @@ -177,53 +177,6 @@ static inline int pci_hp_register(struct THIS_MODULE, KBUILD_MODNAME); } -/* PCI Setting Record (Type 0) */ -struct hpp_type0 { - u32 revision; - u8 cache_line_size; - u8 latency_timer; - u8 enable_serr; - u8 enable_perr; -}; - -/* PCI-X Setting Record (Type 1) */ -struct hpp_type1 { - u32 revision; - u8 max_mem_read; - u8 avg_max_split; - u16 tot_max_split; -}; - -/* PCI Express Setting Record (Type 2) */ -struct hpp_type2 { - u32 revision; - u32 unc_err_mask_and; - u32 unc_err_mask_or; - u32 unc_err_sever_and; - u32 unc_err_sever_or; - u32 cor_err_mask_and; - u32 cor_err_mask_or; - u32 adv_err_cap_and; - u32 adv_err_cap_or; - u16 pci_exp_devctl_and; - u16 pci_exp_devctl_or; - u16 pci_exp_lnkctl_and; - u16 pci_exp_lnkctl_or; - u32 sec_unc_err_sever_and; - u32 sec_unc_err_sever_or; - u32 sec_unc_err_mask_and; - u32 sec_unc_err_mask_or; -}; - -struct hotplug_params { - struct hpp_type0 *t0; /* Type0: NULL if not available */ - struct hpp_type1 *t1; /* Type1: NULL if not available */ - struct hpp_type2 *t2; /* Type2: NULL if not available */ - struct hpp_type0 type0_data; - struct hpp_type1 type1_data; - struct hpp_type2 type2_data; -}; - #ifdef CONFIG_ACPI #include <acpi/acpi.h> #include <acpi/acpi_bus.h> -- 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