PCI, ACPI: Physical PCI slot objects
Subsystems (8)
v10 cover PCI, ACPI: Physical PCI slot objects
Hi Greg, Kenji-san,
This is v10 of my "introduce pci_slot" series, to be considered for inclusion in linux-next to get more test exposure and shake out the bugs.
* Greg KH <[email protected]>:
Quoted 6 lines
Can you work with Kenji to get his approval on this before I add this to the tree? It sounds like there are still some outstanding issues getting this to work properly on his hardware. It needs to work without relying on a module not being loaded, as the distros are never going to go for that kind of solution.
New in v10 is the addition of a DMI list to do the right thing on Fujitsu hardware, and does not rely on "don't load the module" to get the correct behavior.
Also, v10 should fix the "pci_slot module changes the sysfs name" issue that Kenji-san was seeing.
* Kenji Kaneshige <[email protected]>:
Quoted 3 lines
Alex-san, could you send me the updated patches? I'll Ack the patch after adding Fujitsu firmware versions to the DMI list and testing it.
Thanks, I'd like to know if the naming issue is fixed for you as well.
cheers,
/ac
PATCH v10 1/3 Construct one fakephp slot per pci slot
Register one slot per slot, rather than one slot per function. Change the name of the slot to fake%d instead of the pci address.
--- drivers/pci/hotplug/fakephp.c | 85 ++++++++++++++-------------------------- 1 files changed, 30 insertions(+), 55 deletions(-)
drivers/pci/hotplug/fakephp.c +30 -55
@@ -66,6 +66,7 @@ struct dummy_slot {struct pci_dev *dev;struct work_struct remove_work;unsigned long removed; +char name[8 ]; };static int debug;
@@ -100,6 +101,7 @@ static int add_slot(struct pci_dev *dev)struct dummy_slot *dslot;struct hotplug_slot *slot;int retval = -ENOMEM ; +static int count =1 ; slot =kzalloc (sizeof (struct hotplug_slot ),GFP_KERNEL );if (!slot)
@@ -113,13 +115,14 @@ static int add_slot(struct pci_dev *dev) slot->info->max_bus_speed =PCI_SPEED_UNKNOWN ; slot->info->cur_bus_speed =PCI_SPEED_UNKNOWN ; - slot->name = &dev->dev.bus_id[0 ]; -dbg ("slot->name = %s\n" , slot->name); - dslot =kzalloc (sizeof (struct dummy_slot ),GFP_KERNEL );if (!dslot) goto error_info; + slot->name = dslot->name; +snprintf (slot->name,sizeof (dslot->name),"fake%d" , count++); +dbg ("slot->name = %s\n" , slot->name); + slot->ops = &dummy_hotplug_slot_ops; slot->release = &dummy_release; slot->private = dslot;
@@ -148,17 +151,17 @@ error:static int __init pci_scan_buses (void ) {struct pci_dev *dev =NULL ; -int retval =0 ; +int lastslot =0 ;while ((dev =pci_get_device (PCI_ANY_ID ,PCI_ANY_ID , dev)) !=NULL ) { - retval =add_slot (dev); -if (retval) { -pci_dev_put (dev); -break ; - } +if (PCI_FUNC (dev->devfn) >0 && + lastslot ==PCI_SLOT (dev->devfn)) +continue ; + lastslot =PCI_SLOT (dev->devfn); +add_slot (dev); } -return retval; +return 0 ; }static void remove_slot (struct dummy_slot *dslot)
@@ -296,23 +299,9 @@ static int enable_slot(struct hotplug_slot *hotplug_slot)return -ENODEV ; } -/* find the hotplug_slot for the pci_dev */ -static struct hotplug_slot *get_slot_from_dev (struct pci_dev *dev) -{ -struct dummy_slot *dslot; - -list_for_each_entry (dslot, &slot_list, node) { -if (dslot->dev == dev) -return dslot->slot; - } -return NULL ; -} - -static int disable_slot (struct hotplug_slot *slot) {struct dummy_slot *dslot; -struct hotplug_slot *hslot;struct pci_dev *dev;int func;
@@ -322,41 +311,27 @@ static int disable_slot(struct hotplug_slot *slot)dbg ("%s - physical_slot = %s\n" , __FUNCTION__, slot->name); -/* don't disable bridged devices just yet, we can't handle them easily... */ -if (dslot->dev->subordinate) { -err ("Can't remove PCI devices with other PCI devices behind it yet.\n" ); -return -ENODEV ; - } -if (test_and_set_bit (0 , &dslot->removed)) { -dbg ("Slot already scheduled for removal\n" ); -return -ENODEV ; - } -/* search for subfunctions and disable them first */ -if (!(dslot->dev->devfn &7 )) { -for (func =1 ; func <8 ; func++) { - dev =pci_get_slot (dslot->dev->bus, - dslot->dev->devfn + func); -if (dev) { - hslot =get_slot_from_dev (dev); -if (hslot) -disable_slot (hslot); -else { -err ("Hotplug slot not found for subfunction of PCI device\n" ); -return -ENODEV ; - } -pci_dev_put (dev); - }else -dbg ("No device in slot found\n" ); +for (func =7 ; func >=0 ; func--) { + dev =pci_get_slot (dslot->dev->bus, dslot->dev->devfn + func); +if (!dev) +continue ; + +if (test_and_set_bit (0 , &dslot->removed)) { +dbg ("Slot already scheduled for removal\n" ); +return -ENODEV ; } - } -/* remove the device from the pci core */ -pci_remove_bus_device (dslot->dev); +/* queue work item to blow away this sysfs entry and other +* parts. +*/ +INIT_WORK (&dslot->remove_work, remove_slot_worker); +queue_work (dummyphp_wq, &dslot->remove_work); -/* queue work item to blow away this sysfs entry and other parts. */ -INIT_WORK (&dslot->remove_work, remove_slot_worker); -queue_work (dummyphp_wq, &dslot->remove_work); +/* blow away this sysfs entry and other parts. */ +remove_slot (dslot); +pci_dev_put (dev); + }return 0 ; }
Signature · 1 line
1.5.3.1.g1e61
Quoted 2 lines · Alex Chiang
Also, v10 should fix the "
pci_slotmodule changes the sysfs name" issue that Kenji-san was seeing.
I thought we agreed that the current names are wrong, and we shouldn't consider this 'different name' an issue.
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Hi Willy,
* Matthew Wilcox <[email protected]>:
Quoted 6 lines · Alex Chiang
Quoted 2 lines
Also, v10 should fix the "
pci_slotmodule changes the sysfs name" issue that Kenji-san was seeing.I thought we agreed that the current names are wrong, and we shouldn't consider this 'different name' an issue.
I did see some discussion around this, but wasn't actually sure what we decided on. For v10, the little adjustment I made keeps the existing userspace names for pciehp, but doesn't affect shpchp.
[The example that Kenji-san sent out earlier showed that his pciehp slot names changed depending on the order of module loading between pciehp and pci_slot; his shpchp slot names did not change.]
Injecting my thoughts into the exchange between Kenji-san and Kristen earlier on this subject...
Quoted 2 lines
Quoted 1 line
* Kenji Kaneshige <[email protected]>:
* Kristen Carlson Accardi <[email protected]>:
* Kenji Kaneshige <[email protected]>:
Quoted 10 lines
Quoted 7 lines
Quoted 4 lines
Though I don't have any specific idea about this, folliwings might be candidates.
- Override slot names with hotplug driver's slot names
I think this should be done - if the pci slot driver detects that a hotplug driver is controlling a slot, it should allow that driver to set the name of the slot.
Yes, it is what I thought. But maybe we should try unifying slots names first, I think.
This is the tweak I added in v10.
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
- Unify slot names among all hotplug drivers
I'm not sure if we can do this, since slot name might depend on what spec people are implementing.
I'm not sure too. But I think we can unify the slot names using the combination of chassis number and slot number among the controllers which is based on PCI Hot-Plug spec.
I could take this approach instead, but it will definitely change what userspace sees. I'd like to get an ACK from Kristen before starting down this path.
Actually, my real preference would be to save it for a future patch set, while I work on getting this one into the tree. ;)
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
- Stop automatic loading of ACPI PCI slot driver
I think we should definitely implement this one ^^^.
I don't think that should be happening anyway. On my Fedora Core 8 userspace, pci_slot doesn't get loaded until I modprobe it.
Maybe Kenji-san is building it into his kernel?
/ac
Hi Alex-san,
Quoted 4 lines
I don't think that should be happening anyway. On my Fedora Core 8 userspace,
pci_slotdoesn't get loaded until I modprobe it.Maybe Kenji-san is building it into his kernel?
I also noticed that pci_slot doesn't get loaded automatically on Fedra Core 8 yesterday. When I reported before, I was using other distribution (Red Hat one). I don't know why.
Thanks,
Kenji Kaneshige
Quoted 63 lines · Alex Chiang
Hi Willy,
* Matthew Wilcox <[email protected]>:
Quoted 5 lines · Alex Chiang
Quoted 2 lines
Also, v10 should fix the "
pci_slotmodule changes the sysfs name" issue that Kenji-san was seeing.I thought we agreed that the current names are wrong, and we shouldn't consider this 'different name' an issue.
I did see some discussion around this, but wasn't actually sure what we decided on. For v10, the little adjustment I made keeps the existing userspace names for pciehp, but doesn't affect shpchp.
[The example that Kenji-san sent out earlier showed that his pciehp slot names changed depending on the order of module loading between pciehp and
pci_slot; his shpchp slot names did not change.]Injecting my thoughts into the exchange between Kenji-san and Kristen earlier on this subject...
Quoted 2 lines
Quoted 1 line
* Kenji Kaneshige <[email protected]>:
* Kristen Carlson Accardi <[email protected]>:
* Kenji Kaneshige <[email protected]>:
Quoted 9 lines
Quoted 7 lines
Quoted 4 lines
Though I don't have any specific idea about this, folliwings might be candidates.
- Override slot names with hotplug driver's slot names
I think this should be done - if the pci slot driver detects that a hotplug driver is controlling a slot, it should allow that driver to set the name of the slot.
Yes, it is what I thought. But maybe we should try unifying slots names first, I think.
This is the tweak I added in v10.
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
- Unify slot names among all hotplug drivers
I'm not sure if we can do this, since slot name might depend on what spec people are implementing.
I'm not sure too. But I think we can unify the slot names using the combination of chassis number and slot number among the controllers which is based on PCI Hot-Plug spec.
I could take this approach instead, but it will definitely change what userspace sees. I'd like to get an ACK from Kristen before starting down this path.
Actually, my real preference would be to save it for a future patch set, while I work on getting this one into the tree. ;)
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
- Stop automatic loading of ACPI PCI slot driver
I think we should definitely implement this one ^^^.
I don't think that should be happening anyway. On my Fedora Core 8 userspace,
pci_slotdoesn't get loaded until I modprobe it.Maybe Kenji-san is building it into his kernel?
/ac
Quoted 7 lines · Matthew Wilcox
Quoted 2 lines · Alex Chiang
Also, v10 should fix the "
pci_slotmodule changes the sysfs name" issue that Kenji-san was seeing.I thought we agreed that the current names are wrong, and we shouldn't consider this 'different name' an issue.
I also think that the current names are wrong. But even if we fix the slot name of shpchp/pciehp, it doesn't ensure that slot names are same among pci_slot.ko and the other hotplug dirvers.
BTW, I'm planning to suggest the patch for shpchp/pciehp to use slot number as a slot name (as they used to do), and add a module option to use the combination of bus number and slot number as a slot name as a workaround for problematic platforms.
Thanks,
Kenji Kaneshige
PATCH v10 2/3 Introduce pci_slot
In reply to Alex ChiangCurrently, /sys/bus/pci/slots/ only exposes hotplug attributes when a hotplug driver is loaded, but PCI slots have attributes such as address, speed, width, etc. that are not related to hotplug at all.
Introduce pci_slot as the primary data structure and kobject model. Hotplug attributes described in hotplug_slot become a secondary structure associated with the pci_slot.
This patch only creates the infrastructure that allows the separation of PCI slot attributes and hotplug attributes. In this patch, the PCI hotplug core remains the only user of this infrastructure, and thus, /sys/bus/pci/slots/ will still only become populated when a hotplug driver is loaded.
A later patch in this series will add a second user of this new infrastructure and demonstrate splitting the task of exposing pci_slot attributes from hotplug_slot attributes.
- Make pci_slot the primary sysfs entity. hotplug_slot becomes a subsidiary structure.
o pci_create_slot() creates and registers a slot with the PCI core
o pci_slot_add_hotplug() gives it hotplug capability
- Change the prototype of pci_hp_register() to take the bus and slot number (on parent bus) as parameters.
- Remove all the ->get_address methods since this functionality is now handled by pci_slot directly.
v8 -> v9:
Fixed printk output, changed to American spelling of "initialization"
v7 -> v8:
Removed externs from C files.
Taught sn_hp_destroy and sn_hotplug_slot_register about struct pci_slot.
v6 -> v7:
Refresh to new kobject model.
v5 -> v6:
Add debugging information.
v4 -> v5:
Add refcounting for pci_slot objects.
Return -EBUSY if an hp driver attempts to register a slot that is already registered to another driver. Do not consider that to be an error condition in acpiphp and pciehp.
v3 -> v4:
Fixed bug with pciehp and rpaphp registering slots
v2 -> v3:
Separated slot creation and slot hotplug ability into two interfaces. Fixed bugs in pci_destroy_slot(), and now properly calling from pci_hp_deregister.
v1 -> v2:
No change
--- drivers/pci/Makefile | 2 +- drivers/pci/hotplug/acpiphp.h | 1 - drivers/pci/hotplug/acpiphp_core.c | 25 +--- drivers/pci/hotplug/acpiphp_glue.c | 23 +-- drivers/pci/hotplug/acpiphp_ibm.c | 6 +- drivers/pci/hotplug/cpci_hotplug_core.c | 2 +- drivers/pci/hotplug/cpqphp_core.c | 4 +- drivers/pci/hotplug/fakephp.c | 2 +- drivers/pci/hotplug/ibmphp_ebda.c | 3 +- drivers/pci/hotplug/pci_hotplug_core.c | 244 +++++++++++-------------------- drivers/pci/hotplug/pciehp_core.c | 31 ++--- drivers/pci/hotplug/rpadlpar_sysfs.c | 5 +- drivers/pci/hotplug/rpaphp_slot.c | 3 +- drivers/pci/hotplug/sgi_hotplug.c | 10 +- drivers/pci/hotplug/shpchp_core.c | 17 +-- drivers/pci/pci.h | 13 ++ drivers/pci/slot.c | 224 ++++++++++++++++++++++++++++ include/linux/pci.h | 17 ++ include/linux/pci_hotplug.h | 12 +- 19 files changed, 394 insertions(+), 250 deletions(-) create mode 100644 drivers/pci/slot.c
drivers/pci/Makefile +1 -1
@@ -2,7 +2,7 @@ # Makefile for the PCI bus specific drivers. # -obj-y += access.o bus.o probe.o remove.o pci.o quirks.o \ +obj-y += access.o bus.o probe.o remove.o pci.o quirks.o slot.o \ pci-driver.o search.o pci-sysfs.o rom.o setup-res.o obj-$(CONFIG_PROC_FS) += proc.o
drivers/pci/hotplug/acpiphp.h +0 -1
@@ -215,7 +215,6 @@ extern u8 acpiphp_get_power_status (struct acpiphp_slot *slot);extern u8 acpiphp_get_attention_status (struct acpiphp_slot *slot);extern u8 acpiphp_get_latch_status (struct acpiphp_slot *slot);extern u8 acpiphp_get_adapter_status (struct acpiphp_slot *slot); -extern u32 acpiphp_get_address (struct acpiphp_slot *slot);/* variables */ extern int acpiphp_debug;
drivers/pci/hotplug/acpiphp_core.c +5 -20
@@ -70,7 +70,6 @@ static int disable_slot (struct hotplug_slot *slot);static int set_attention_status (struct hotplug_slot *slot,u8 value);static int get_power_status (struct hotplug_slot *slot,u8 *value);static int get_attention_status (struct hotplug_slot *slot,u8 *value); -static int get_address (struct hotplug_slot *slot,u32 *value);static int get_latch_status (struct hotplug_slot *slot,u8 *value);static int get_adapter_status (struct hotplug_slot *slot,u8 *value);
@@ -83,7 +82,6 @@ static struct hotplug_slot_ops acpi_hotplug_slot_ops = { .get_attention_status = get_attention_status, .get_latch_status = get_latch_status, .get_adapter_status = get_adapter_status, - .get_address = get_address, };
@@ -274,23 +272,6 @@ static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value)return 0 ; } - -/** -* get_address - get pci address of a slot -* @hotplug_slot: slot to get status -* @value: pointer to struct pci_busdev (seg, bus, dev) -*/ -static int get_address (struct hotplug_slot *hotplug_slot,u32 *value) -{ -struct slot *slot = hotplug_slot->private; - -dbg ("%s - physical_slot = %s\n" , __FUNCTION__, hotplug_slot->name); - - *value =acpiphp_get_address (slot->acpi_slot); - -return 0 ; -} -static int __init init_acpi (void ) {int retval;
@@ -357,7 +338,11 @@ int acpiphp_register_hotplug_slot(struct acpiphp_slot *acpiphp_slot) acpiphp_slot->slot = slot;snprintf (slot->name,sizeof (slot->name),"%u" , slot->acpi_slot->sun); - retval =pci_hp_register (slot->hotplug_slot); + retval =pci_hp_register (slot->hotplug_slot, + acpiphp_slot->bridge->pci_bus, + acpiphp_slot->device); +if (retval == -EBUSY ) + goto error_hpslot;if (retval) {err ("pci_hp_register failed with error %d\n" , retval); goto error_hpslot;
drivers/pci/hotplug/acpiphp_glue.c +6 -17
@@ -258,7 +258,12 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv) bridge->pci_bus->number, slot->device); retval =acpiphp_register_hotplug_slot (slot);if (retval) { -warn ("acpiphp_register_hotplug_slot failed(err code = 0x%x)\n" , retval); +if (retval == -EBUSY ) +warn ("Slot %d already registered by another " +"hotplug driver\n" , slot->sun); +else +warn ("acpiphp_register_hotplug_slot failed " +"(err code = 0x%x)\n" , retval); goto err_exit; } }
@@ -1867,19 +1872,3 @@ u8 acpiphp_get_adapter_status(struct acpiphp_slot *slot)return (sta ==0 ) ?0 :1 ; } - - -/* -* pci address (seg/bus/dev) -*/ -u32 acpiphp_get_address (struct acpiphp_slot *slot) -{ -u32 address; -struct pci_bus *pci_bus = slot->bridge->pci_bus; - - address = (pci_domain_nr (pci_bus) <<16 ) | - (pci_bus->number <<8 ) | - slot->device; - -return address; -}
drivers/pci/hotplug/acpiphp_ibm.c +4 -2
@@ -33,8 +33,10 @@#include <linux/kobject.h> #include <asm/uaccess.h> #include <linux/moduleparam.h> +#include <linux/pci.h> #include "acpiphp.h" +#include "../pci.h" #define DRIVER_VERSION "1.0.1"#define DRIVER_AUTHOR "Irene Zubarev <[email protected]>, Vernon Mauery <[email protected]>"
@@ -430,7 +432,7 @@ static int __init ibm_acpiphp_init(void)int retval =0 ;acpi_status status;struct acpi_device *device; -struct kobject *sysdir = &pci_hotplug_slots_kset->kobj; +struct kobject *sysdir = &pci_slots_kset->kobj;dbg ("%s\n" , __FUNCTION__);
@@ -477,7 +479,7 @@ init_return: static void __exit ibm_acpiphp_exit(void) {acpi_status status; -struct kobject *sysdir = &pci_hotplug_slots_kset->kobj; +struct kobject *sysdir = &pci_slots_kset->kobj;dbg ("%s\n" , __FUNCTION__);
drivers/pci/hotplug/cpci_hotplug_core.c +1 -1
@@ -285,7 +285,7 @@ cpci_hp_register_bus(struct pci_bus *bus, u8 first, u8 last) info->attention_status =cpci_get_attention_status (slot);dbg ("registering slot %s" , slot->hotplug_slot->name); - status =pci_hp_register (slot->hotplug_slot); + status =pci_hp_register (slot->hotplug_slot, bus, i);if (status) {err ("pci_hp_register failed with error %d" , status); goto error_name;
drivers/pci/hotplug/cpqphp_core.c +3 -1
@@ -434,7 +434,9 @@ static int ctrl_slot_setup(struct controller *ctrl, slot->bus, slot->device, slot->number, ctrl->slot_device_offset, slot_number); - result =pci_hp_register (hotplug_slot); + result =pci_hp_register (hotplug_slot, + ctrl->pci_dev->subordinate, + slot->device);if (result) {err ("pci_hp_register failed with error %d\n" , result); goto error_name;
drivers/pci/hotplug/fakephp.c +1 -1
@@ -127,7 +127,7 @@ static int add_slot(struct pci_dev *dev) slot->release = &dummy_release; slot->private = dslot; - retval =pci_hp_register (slot); + retval =pci_hp_register (slot, dev->bus,PCI_SLOT (dev->devfn));if (retval) {err ("pci_hp_register failed with error %d\n" , retval); goto error_dslot;
drivers/pci/hotplug/ibmphp_ebda.c +2 -1
@@ -1001,7 +1001,8 @@ static int __init ebda_rsrc_controller (void) tmp_slot = list_entry (list, struct slot, ibm_slot_list);snprintf (tmp_slot->hotplug_slot->name,30 ,"%s" ,create_file_name (tmp_slot)); -pci_hp_register (tmp_slot->hotplug_slot); +pci_hp_register (tmp_slot->hotplug_slot, +pci_find_bus (0 , tmp_slot->bus), tmp_slot->device); }print_ebda_hpc ();
drivers/pci/hotplug/pci_hotplug_core.c +87 -157
539 lines not on this page · raw message
drivers/pci/hotplug/pciehp_core.c +13 -18
@@ -69,7 +69,6 @@ static int get_power_status (struct hotplug_slot *slot, u8 *value);static int get_attention_status (struct hotplug_slot *slot,u8 *value);static int get_latch_status (struct hotplug_slot *slot,u8 *value);static int get_adapter_status (struct hotplug_slot *slot,u8 *value); -static int get_address (struct hotplug_slot *slot,u32 *value);static int get_max_bus_speed (struct hotplug_slot *slot,enum pci_bus_speed *value);static int get_cur_bus_speed (struct hotplug_slot *slot,enum pci_bus_speed *value);
@@ -82,7 +81,6 @@ static struct hotplug_slot_ops pciehp_hotplug_slot_ops = { .get_attention_status = get_attention_status, .get_latch_status = get_latch_status, .get_adapter_status = get_adapter_status, - .get_address = get_address, .get_max_bus_speed = get_max_bus_speed, .get_cur_bus_speed = get_cur_bus_speed, };
@@ -245,14 +243,18 @@ static int init_slots(struct controller *ctrl)dbg ("Registering bus=%x dev=%x hp_slot=%x sun=%x " "slot_device_offset=%x\n" , slot->bus, slot->device, slot->hp_slot, slot->number, ctrl->slot_device_offset); - retval =pci_hp_register (hotplug_slot); + retval =pci_hp_register (hotplug_slot, + ctrl->pci_dev->subordinate, + slot->device); +if (retval == -EBUSY ) + goto error_info;if (retval) {err ("pci_hp_register failed with error %d\n" , retval); goto error_info; }/* create additional sysfs entries */ if (EMI (ctrl->ctrlcap)) { - retval =sysfs_create_file (&hotplug_slot->kobj, + retval =sysfs_create_file (&hotplug_slot->pci_slot->kobj, &hotplug_slot_attr_lock.attr);if (retval) {pci_hp_deregister (hotplug_slot);
@@ -285,7 +287,7 @@ static void cleanup_slots(struct controller *ctrl) slot = list_entry(tmp, struct slot, slot_list);list_del (&slot->slot_list);if (EMI (ctrl->ctrlcap)) -sysfs_remove_file (&slot->hotplug_slot->kobj, +sysfs_remove_file (&slot->hotplug_slot->pci_slot->kobj, &hotplug_slot_attr_lock.attr);cancel_delayed_work (&slot->work);flush_scheduled_work ();
@@ -387,18 +389,6 @@ static int get_adapter_status(struct hotplug_slot *hotplug_slot, u8 *value)return 0 ; } -static int get_address (struct hotplug_slot *hotplug_slot,u32 *value) -{ -struct slot *slot = hotplug_slot->private; -struct pci_bus *bus = slot->ctrl->pci_dev->subordinate; - -dbg ("%s - physical_slot = %s\n" , __FUNCTION__, hotplug_slot->name); - - *value = (pci_domain_nr (bus) <<16 ) | (slot->bus <<8 ) | slot->device; - -return 0 ; -} -static int get_max_bus_speed (struct hotplug_slot *hotplug_slot,enum pci_bus_speed *value) {struct slot *slot = hotplug_slot->private;
@@ -460,7 +450,12 @@ static int pciehp_probe(struct pcie_device *dev, const struct pcie_port_service_/* Setup the slot information structures */ rc =init_slots (ctrl);if (rc) { -err ("%s: slot initialization failed\n" ,PCIE_MODULE_NAME ); +if (rc == -EBUSY ) +warn ("%s: slot already registered by another " +"hotplug driver\n" ,PCIE_MODULE_NAME ); +else +err ("%s: slot initialization failed\n" , +PCIE_MODULE_NAME ); goto err_out_release_ctlr; }
drivers/pci/hotplug/rpadlpar_sysfs.c +3 -2
@@ -14,8 +14,10 @@*/ #include <linux/kobject.h> #include <linux/string.h> +#include <linux/pci.h> #include <linux/pci_hotplug.h> #include "rpadlpar.h" +#include "../pcih." #define DLPAR_KOBJ_NAME "control"#define ADD_SLOT_ATTR_NAME "add_slot"
@@ -23,7 +25,6 @@#define MAX_DRC_NAME_LEN 64 -static ssize_t add_slot_store (struct kobject *kobj,struct kobj_attribute *attr,const char *buf,size_t nbytes) {
@@ -108,7 +109,7 @@ int dlpar_sysfs_init(void)int error; dlpar_kobj =kobject_create_and_add (DLPAR_KOBJ_NAME , - &pci_hotplug_slots_kset->kobj); + &pci_slots_kset->kobj);if (!dlpar_kobj)return -EINVAL ;
drivers/pci/hotplug/rpaphp_slot.c +2 -1
@@ -162,7 +162,8 @@ int rpaphp_register_slot(struct slot *slot)return -EAGAIN ; } - retval =pci_hp_register (php_slot); + retval =pci_hp_register (php_slot, slot->bus, +PCI_SLOT (PCI_DN (slot->dn->child)->devfn));if (retval) {err ("pci_hp_register failed with error %d\n" , retval);return retval;
drivers/pci/hotplug/sgi_hotplug.c +7 -3
@@ -197,13 +197,15 @@ static int sn_hp_slot_private_alloc(struct hotplug_slot *bss_hotplug_slot,static struct hotplug_slot *sn_hp_destroy (void ) {struct slot *slot; +struct pci_slot *pci_slot;struct hotplug_slot *bss_hotplug_slot =NULL ;list_for_each_entry (slot, &sn_hp_list, hp_list) { bss_hotplug_slot = slot->hotplug_slot; + pci_slot = bss_hotplug_slot->pci_slot;list_del (&((struct slot *)bss_hotplug_slot->private)-> hp_list); -sysfs_remove_file (&bss_hotplug_slot->kobj, +sysfs_remove_file (&pci_slot->kobj, &sn_slot_path_attr.attr);break ; }
@@ -614,6 +616,7 @@ static void sn_release_slot(struct hotplug_slot *bss_hotplug_slot)static int sn_hotplug_slot_register (struct pci_bus *pci_bus) {int device; +struct pci_slot *pci_slot;struct hotplug_slot *bss_hotplug_slot;int rc =0 ;
@@ -650,11 +653,12 @@ static int sn_hotplug_slot_register(struct pci_bus *pci_bus) bss_hotplug_slot->ops = &sn_hotplug_slot_ops; bss_hotplug_slot->release = &sn_release_slot; - rc =pci_hp_register (bss_hotplug_slot); + rc =pci_hp_register (bss_hotplug_slot, pci_bus, device);if (rc) goto register_err; - rc =sysfs_create_file (&bss_hotplug_slot->kobj, + pci_slot = bss_hotplug_slot->pci_slot; + rc =sysfs_create_file (&pci_slot->kobj, &sn_slot_path_attr.attr);if (rc) goto register_err;
drivers/pci/hotplug/shpchp_core.c +2 -15
@@ -65,7 +65,6 @@ static int get_power_status (struct hotplug_slot *slot, u8 *value);static int get_attention_status (struct hotplug_slot *slot,u8 *value);static int get_latch_status (struct hotplug_slot *slot,u8 *value);static int get_adapter_status (struct hotplug_slot *slot,u8 *value); -static int get_address (struct hotplug_slot *slot,u32 *value);static int get_max_bus_speed (struct hotplug_slot *slot,enum pci_bus_speed *value);static int get_cur_bus_speed (struct hotplug_slot *slot,enum pci_bus_speed *value);
@@ -78,7 +77,6 @@ static struct hotplug_slot_ops shpchp_hotplug_slot_ops = { .get_attention_status = get_attention_status, .get_latch_status = get_latch_status, .get_adapter_status = get_adapter_status, - .get_address = get_address, .get_max_bus_speed = get_max_bus_speed, .get_cur_bus_speed = get_cur_bus_speed, };
@@ -152,7 +150,8 @@ static int init_slots(struct controller *ctrl)dbg ("Registering bus=%x dev=%x hp_slot=%x sun=%x " "slot_device_offset=%x\n" , slot->bus, slot->device, slot->hp_slot, slot->number, ctrl->slot_device_offset); - retval =pci_hp_register (slot->hotplug_slot); + retval =pci_hp_register (slot->hotplug_slot, + ctrl->pci_dev->subordinate, slot->device);if (retval) {err ("pci_hp_register failed with error %d\n" , retval); goto error_info;
@@ -277,18 +276,6 @@ static int get_adapter_status (struct hotplug_slot *hotplug_slot, u8 *value)return 0 ; } -static int get_address (struct hotplug_slot *hotplug_slot,u32 *value) -{ -struct slot *slot =get_slot (hotplug_slot); -struct pci_bus *bus = slot->ctrl->pci_dev->subordinate; - -dbg ("%s - physical_slot = %s\n" , __FUNCTION__, hotplug_slot->name); - - *value = (pci_domain_nr (bus) <<16 ) | (slot->bus <<8 ) | slot->device; - -return 0 ; -} -static int get_max_bus_speed (struct hotplug_slot *hotplug_slot,enum pci_bus_speed *value) {struct slot *slot =get_slot (hotplug_slot);
drivers/pci/pci.h +13 -0
@@ -87,3 +87,16 @@ pci_match_one_device(const struct pci_device_id *id, const struct pci_dev *dev) }struct pci_dev *pci_find_upstream_pcie_bridge (struct pci_dev *pdev); + +/* PCI slot sysfs helper code */ +#define to_pci_slot (s) container_of(s, struct pci_slot, kobj) + +extern struct kset *pci_slots_kset; + +struct pci_slot_attribute { +struct attribute attr; +ssize_t (*show)(struct pci_slot *,char *); +ssize_t (*store)(struct pci_slot *,const char *,size_t ); +}; +#define to_pci_slot_attr (s) container_of(s, struct pci_slot_attribute, attr) +
drivers/pci/slot.c new file +224 -0
224 lines not on this page · raw message
include/linux/pci.h +17 -0
@@ -128,6 +128,16 @@ struct pci_cap_saved_state {u32 data[0 ]; }; +/* pci_slot represents a physical slot */ +struct pci_slot { +struct pci_bus *bus;/* The bus this slot is on */ +struct pci_slot *next;/* Next slot on this bus */ +struct hotplug_slot *hotplug;/* Hotplug info (migrate over time) */ +unsigned char number;/* PCI_SLOT(pci_dev->devfn) */ +struct kobject kobj; +void (*release)(struct pci_slot *); +}; +/* * The pci_dev structure is used to describe PCI devices. */
@@ -139,6 +149,7 @@ struct pci_dev {void *sysdata;/* hook for sys-specific extension */ struct proc_dir_entry *procent;/* device entry in /proc/bus/pci */ +struct pci_slot *slot;/* Physical slot this device is in */ unsigned int devfn;/* encoded device & function index */ unsigned short vendor;
@@ -258,6 +269,7 @@ struct pci_bus {struct list_head children;/* list of child buses */ struct list_head devices;/* list of devices on this bus */ struct pci_dev *self;/* bridge device as seen by parent */ +struct pci_slot *slot;/* First physical slot on this bus */ struct resource *resource[PCI_BUS_NUM_RESOURCES ];/* address space routed to this bus */
@@ -481,6 +493,11 @@ struct pci_bus *pci_create_bus(struct device *parent, int bus, struct pci_ops *ops, void *sysdata);struct pci_bus *pci_add_new_bus (struct pci_bus *parent,struct pci_dev *dev,int busnr); +struct pci_slot *pci_create_slot (struct pci_bus *parent,int slot_nr, +const char *name); +int pci_slot_add_hotplug (struct pci_bus *parent,int slot_nr, +void (*release)(struct pci_slot *)); +int pci_destroy_slot (struct pci_slot *slot);int pci_scan_slot (struct pci_bus *bus,int devfn);struct pci_dev *pci_scan_single_device (struct pci_bus *bus,int devfn);void pci_device_add (struct pci_dev *dev,struct pci_bus *bus);
include/linux/pci_hotplug.h +3 -9
@@ -95,9 +95,6 @@ struct hotplug_slot_attribute {* @get_adapter_status: Called to get see if an adapter is present in the slot or not. * If this field is NULL, the value passed in the struct hotplug_slot_info * will be used when this value is requested by a user. -* @get_address: Called to get pci address of a slot. -* If this field is NULL, the value passed in the struct hotplug_slot_info -* will be used when this value is requested by a user. * @get_max_bus_speed: Called to get the max bus speed for a slot. * If this field is NULL, the value passed in the struct hotplug_slot_info * will be used when this value is requested by a user.
@@ -120,7 +117,6 @@ struct hotplug_slot_ops {int (*get_attention_status) (struct hotplug_slot *slot,u8 *value);int (*get_latch_status) (struct hotplug_slot *slot,u8 *value);int (*get_adapter_status) (struct hotplug_slot *slot,u8 *value); -int (*get_address) (struct hotplug_slot *slot,u32 *value);int (*get_max_bus_speed) (struct hotplug_slot *slot,enum pci_bus_speed *value);int (*get_cur_bus_speed) (struct hotplug_slot *slot,enum pci_bus_speed *value); };
@@ -140,7 +136,6 @@ struct hotplug_slot_info {u8 attention_status;u8 latch_status;u8 adapter_status; -u32 address;enum pci_bus_speed max_bus_speed;enum pci_bus_speed cur_bus_speed; };
@@ -166,15 +161,14 @@ struct hotplug_slot {/* Variables below this are for use only by the hotplug pci core. */ struct list_head slot_list; -struct kobject kobj; +struct pci_slot *pci_slot; };#define to_hotplug_slot (n) container_of(n, struct hotplug_slot, kobj) -extern int pci_hp_register (struct hotplug_slot *slot); -extern int pci_hp_deregister (struct hotplug_slot *slot); +extern int pci_hp_register (struct hotplug_slot *,struct pci_bus *,int nr); +extern int pci_hp_deregister (struct hotplug_slot *slot);extern int __must_check pci_hp_change_slot_info (struct hotplug_slot *slot,struct hotplug_slot_info *info); -extern struct kset *pci_hotplug_slots_kset;/* PCI Setting Record (Type 0) */ struct hpp_type0 {
Signature · 1 line
1.5.3.1.g1e61
PATCH v10 3/3 ACPI PCI slot detection driver
In reply to Alex ChiangDetect all physical PCI slots as described by ACPI, and create entries in /sys/bus/pci/slots/.
Not all physical slots are hotpluggable, and the acpiphp module does not detect them. Now we know the physical PCI geography of our system, without caring about hotplug.
v9 -> v10:
Allow an hp driver to override the pci_slot kobject name.
v8 -> v9:
Add DMI quirk for Fujitsu machines; eval _STA before _SUN
v6 -> v8:
No change
v5 -> v6:
Add debugging information.
v4 -> v5:
Convert to a tristate module.
Remove #ifdef CONFIG_ACPI_PCI_SLOT, as struct pci_slot objects are properly refcounted, and multiple calls to pci_create/destroy_slot work just fine.
Remove prior -EBUSY changes, as they have been folded into the Introduce pci_slot patch.
v3 -> v4:
Always attempt to call pci_create_slot from pcihp_core to cover cases of
a) user did not say Y to Kconfig option ACPI_PCI_SLOT
b) native PCIe hotplug driver registering on a non-ACPI system.
Return -EBUSY if an hp driver attempts to register a slot that is already registered to another driver. Do not consider that to be an error condition in acpiphp and pciehp.
v2 -> v3:
Add Kconfig option to driver, allowing users to [de]config this driver. If configured, take slightly different code paths in pci_hp_register and pci_hp_deregister.
v1 -> v2:
Now recursively discovering p2p bridges and slots underneath them. Hopefully, this will prevent us from trying to register the same slot multiple times.
--- drivers/acpi/Kconfig | 9 + drivers/acpi/Makefile | 1 + drivers/acpi/pci_slot.c | 360 ++++++++++++++++++++++++++++++++ drivers/pci/hotplug/pci_hotplug_core.c | 22 ++- drivers/pci/hotplug/pciehp_core.c | 2 +- drivers/pci/hotplug/sgi_hotplug.c | 2 +- 6 files changed, 393 insertions(+), 3 deletions(-) create mode 100644 drivers/acpi/pci_slot.c
drivers/acpi/Kconfig +9 -0
@@ -335,6 +335,15 @@ config ACPI_EC the battery and thermal drivers. If you are compiling for a mobile system, say Y. +config ACPI_PCI_SLOT + tristate "PCI slot detection driver" + default n + help + This driver will attempt to discover all PCI slots in your system, + and creates entries in /sys/bus/pci/slots/. This feature can + help you correlate PCI bus addresses with the physical geography + of your slots. If you are unsure, say N. + config ACPI_POWER bool default y
drivers/acpi/Makefile +1 -0
@@ -48,6 +48,7 @@ obj-$(CONFIG_ACPI_DOCK) += dock.o obj-$(CONFIG_ACPI_BAY) += bay.o obj-$(CONFIG_ACPI_VIDEO) += video.o obj-y += pci_root.o pci_link.o pci_irq.o pci_bind.o +obj-$(CONFIG_ACPI_PCI_SLOT) += pci_slot.o obj-$(CONFIG_ACPI_POWER) += power.o obj-$(CONFIG_ACPI_PROCESSOR) += processor.o obj-$(CONFIG_ACPI_CONTAINER) += container.o
drivers/acpi/pci_slot.c new file +360 -0
360 lines not on this page · raw message
drivers/pci/hotplug/pci_hotplug_core.c +21 -1
@@ -567,6 +567,11 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)return -EINVAL ; } +/* +* No problems if we call this interface from both ACPI_PCI_SLOT +* driver and call it here again. If we've already created the +* pci_slot, the interface will simply bump the refcount. +*/ pci_slot =pci_create_slot (bus, slot_nr, slot->name);if (IS_ERR (pci_slot))return PTR_ERR (pci_slot);
@@ -580,6 +585,17 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr) slot->pci_slot = pci_slot; pci_slot->hotplug = slot; +/* +* Allow pcihp drivers to override the ACPI_PCI_SLOT name. +*/ +if (strcmp (kobject_name (&pci_slot->kobj), slot->name)) { + result =kobject_rename (&pci_slot->kobj, slot->name); +if (result) { +pci_destroy_slot (pci_slot); +return result; + } + } +list_add (&slot->slot_list, &pci_hotplug_slot_list); result =fs_add_slot (pci_slot);
@@ -613,8 +629,12 @@ int pci_hp_deregister(struct hotplug_slot *hotplug) slot = hotplug->pci_slot;fs_remove_slot (slot); -pci_destroy_slot (slot);dbg ("Removed slot %s from the list\n" , hotplug->name); + +hotplug_release (slot); + slot->release =NULL ; +pci_destroy_slot (slot); +return 0 ; }
drivers/pci/hotplug/pciehp_core.c +1 -1
@@ -249,7 +249,7 @@ static int init_slots(struct controller *ctrl)if (retval == -EBUSY ) goto error_info;if (retval) { -err ("pci_hp_register failed with error %d\n" , retval); +err ("pci_hp_register failed with error %d\n" , retval); goto error_info; }/* create additional sysfs entries */
drivers/pci/hotplug/sgi_hotplug.c +1 -1
@@ -668,7 +668,7 @@ static int sn_hotplug_slot_register(struct pci_bus *pci_bus) register_err:dev_dbg (&pci_bus->self->dev,"bus failed to register with err = %d\n" , - rc); + rc); alloc_err:if (rc == -ENOMEM )
Signature · 1 line
1.5.3.1.g1e61
Hi Alex-san, Greg-san,
I've reviewed and tested the latest version of Alex's "introduce pci_slot" series with DMI table entry for Fujitsu PRIMEQUEST added on our PRIMEQUEST server that has both SHPC and PCI-Ex based hotplug slots. And I confirmed invalid slot ('1023') problem was solved and slot names were overridden with hotplug drivers as we want. But on the other hand, I found some bugs and some codes need to be changed. So I would like Alex-san to fix those. I made patches for each of those. Could you consider adding or merging my patches to your patch series? My patches are as follows. Please see the header of each patch for details. Those are on top of your three patches.
- [PATCH 1/16][BUG] Export kobject_rename for pci_hotplug_core (Not for mainline!) - [PATCH 2/16] ACPI pci_slot: Fix dmi table for Fujitsu PRIMEQUEST (Not for mainline!) - [PATCH 3/16][BUG] ACPI pci_slot: Fix _STA evaluation (Not for mainline!) - [PATCH 4/16][BUG] PCI slot: Add missing semaphore for slot release (Not for mainline!) - [PATCH 5/16] PCI slot: Use list_head for pci slot list (Not for mainline!) - [PATCH 6/16][BUG] ACPI pci_slot: Fix slot removal path (Not for mainline!) - [PATCH 7/16][BUG] PCI slot: Remove compiler warnings (Not for mainline!) - [PATCH 8/16][BUG] PCI slot: Fix invalid memory access (Not for mainline!) - [PATCH 9/16] PCI slot: Remove unused slot member from pci_dev (Not for mainline!)
- [PATCH 10/16] PCI slot: Replace dbg with pr_debug (Not for mainline!)
- [PATCH 11/16] PCI slot: Remove useless release handler (Not for mainline!)
- [PATCH 12/16] PCI slot: Use .default_attrs for address file (Not for mainline!)
- [PATCH 13/16] PCI slot: Fix return value of pci_create_slot() (Not for mainline!)
- [PATCH 14/16] PCI slot: Change return value of pci_destroy_slot() (Not for mainline!)
- [PATCH 15/16] PCI slot: Trivial cleanups for slot.c (Not for mainline!)
- [PATCH 16/16][BUG] PCI hotplug core: add missing lock for hotplug slot list (Not for mainline!)
BTW, I've posted other patches to fix the slot name of shpchp/pciehp to pcihpd-discuss mailing-list. Please see it also.
Thanks,
Kenji Kaneshige
Quoted 35 lines · Alex Chiang
Hi Greg, Kenji-san,
This is v10 of my "introduce
pci_slot" series, to be considered for inclusion in linux-next to get more test exposure and shake out the bugs.* Greg KH <[email protected]>:
Quoted 6 lines
Can you work with Kenji to get his approval on this before I add this to the tree? It sounds like there are still some outstanding issues getting this to work properly on his hardware. It needs to work without relying on a module not being loaded, as the distros are never going to go for that kind of solution.
New in v10 is the addition of a DMI list to do the right thing on Fujitsu hardware, and does not rely on "don't load the module" to get the correct behavior.
Also, v10 should fix the "
pci_slotmodule changes the sysfs name" issue that Kenji-san was seeing.* Kenji Kaneshige <[email protected]>:
Quoted 3 lines
Alex-san, could you send me the updated patches? I'll Ack the patch after adding Fujitsu firmware versions to the DMI list and testing it.
Thanks, I'd like to know if the naming issue is fixed for you as well.
cheers,
/ac
PATCH 1/16 Export kobject_rename for pci_hotplug_core (Not for mainline!)
Export kobject_rename() to fix the following link error. This happens when pci_hotplug_core driver is compiled as a kernel module.
ERROR: "kobject_rename" [drivers/pci/hotplug/pci_hotplug.ko] undefined! make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2
make: *** Waiting for unfinished jobs....
--- lib/kobject.c | 1 + 1 file changed, 1 insertion(+)
lib/kobject.c +1 -0
@@ -456,6 +456,7 @@ out:return error; } +EXPORT_SYMBOL_GPL (kobject_rename);/** * kobject_move - move object to another parent
Re: [PATCH 1/16][BUG] Export kobject_rename for pci_hotplug_core (Not for mainline!)
Hi Greg,
This patch looks like a candidate for the current 2.6.25 cycle. Any chance of getting it in there?
If not, maybe you can push it into linux-next without waiting for the rest of my pci_slot changes, since this is a fix for the kobject core, and not necessarily what I'm working on.
Thanks.
/ac
* Kenji Kaneshige <[email protected]>:
Quoted 9 lines
Export
kobject_rename()to fix the following link error. This happens whenpci_hotplug_coredriver is compiled as a kernel module.ERROR: "
kobject_rename" [drivers/pci/hotplug/pci_hotplug.ko] undefined! make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2
make: *** Waiting for unfinished jobs....Signed-off-by Kenji Kaneshige <[email protected]>
Quoted 19 lines
--- lib/kobject.c | 1 + 1 file changed, 1 insertion(+)Index:
linux-2.6.25-rc6/lib/kobject.c
===================================================================lib/kobject.c +1
-0@@ -456,6 +456,7 @@ out:return error; } +EXPORT_SYMBOL_GPL (kobject_rename);/** * kobject_move - move object to another parent
Re: [PATCH 1/16][BUG] Export kobject_rename for pci_hotplug_core (Not for mainline!)
Quoted 4 lines · Alex Chiang
Hi Greg,
This patch looks like a candidate for the current 2.6.25 cycle. Any chance of getting it in there?
Is this a problem today? I don't see it being one, so when you need this function exported, we can do it then. Otherwise people will just start sending me patches unexporting it as they don't see any in-kernel users of it :)
So just keep it as part of your patchset, it's easier for everyone that way.
And it will remind me to verify that you really should be calling kobject_rename and not just getting the name correctly the first time around...{hint}
thanks,
greg k-h
Re: [PATCH 1/16][BUG] Export kobject_rename for pci_hotplug_core (Not for mainline!)
* Greg KH <[email protected]>:
Quoted 6 lines · Alex Chiang
Quoted 4 lines
Hi Greg,
This patch looks like a candidate for the current 2.6.25 cycle. Any chance of getting it in there?
Quoted 7 lines
Is this a problem today? I don't see it being one, so when you need this function exported, we can do it then. Otherwise people will just start sending me patches unexporting it as they don't see any in-kernel users of it :)
So just keep it as part of your patchset, it's easier for everyone that way.
Ok, sounds fair to me.
Quoted 3 lines
And it will remind me to verify that you really should be calling
kobject_renameand not just getting the name correctly the first time around...{hint}
I'm pretty sure we really do need to call it, but I appreciate your future review. ;)
/ac
PATCH 2/16 ACPI pci_slot: Fix dmi table for Fujitsu PRIMEQUEST (Not for mainline!)
In reply to Kenji KaneshigeFix DMI table entry for Fujitsu PRIMEQUEST.
--- drivers/acpi/pci_slot.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/acpi/pci_slot.c +3 -3
@@ -332,10 +332,10 @@ static struct dmi_system_id acpi_pci_slo*/ { .callback = do_sta_before_sun, - .ident = "Fujitsu Limited Primequest", + .ident = "Fujitsu PRIMEQUEST", .matches = { - DMI_MATCH(DMI_BIOS_VENDOR, "Fujitsu"), - DMI_MATCH(DMI_BIOS_VERSION, "2.35"), + DMI_MATCH(DMI_BIOS_VENDOR, "FUJITSU LIMITED"), + DMI_MATCH(DMI_BIOS_VERSION, "PRIMEQUEST"), }, }, {}
Alex Chiang
AppliedRe: [PATCH 2/16] ACPI pci_slot: Fix dmi table for Fujitsu PRIMEQUEST (Not for mainline!)
* Kenji Kaneshige <[email protected]>:
Quoted 3 lines
Fix DMI table entry for Fujitsu PRIMEQUEST.
Signed-off-by Kenji Kaneshige <[email protected]>
Merged, thanks.
/ac
Quoted 25 lines
--- drivers/acpi/pci_slot.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)Index:
linux-2.6.25-rc6/drivers/acpi/pci_slot.c
===================================================================drivers/acpi/pci_slot.c +3
-3@@ -332,10 +332,10 @@ static struct dmi_system_id acpi_pci_slo*/ { .callback = do_sta_before_sun, - .ident = "Fujitsu Limited Primequest", + .ident = "Fujitsu PRIMEQUEST", .matches = { - DMI_MATCH(DMI_BIOS_VENDOR, "Fujitsu"), - DMI_MATCH(DMI_BIOS_VERSION, "2.35"), + DMI_MATCH(DMI_BIOS_VENDOR, "FUJITSU LIMITED"), + DMI_MATCH(DMI_BIOS_VERSION, "PRIMEQUEST"), }, }, {}
PATCH 3/16 ACPI pci_slot: Fix _STA evaluation (Not for mainline!)
In reply to Kenji KaneshigeFix the problem that pci slots that doesn't have _STA are not detected. If the device doesn't have _STA, we must assume it is always there.
--- drivers/acpi/pci_slot.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/acpi/pci_slot.c +2 -3
@@ -76,9 +76,8 @@ check_slot(acpi_handle handle, int *deviif (check_sta_before_sun) {/* If SxFy doesn't have _STA, we just assume it's there */ -acpi_evaluate_integer (handle,"_STA" ,NULL , &sta); - -if (!(sta &ACPI_STA_DEVICE_PRESENT )) { + status =acpi_evaluate_integer (handle,"_STA" ,NULL , &sta); +if (ACPI_SUCCESS (status) && !(sta &ACPI_STA_DEVICE_PRESENT )) { retval =-1 ; goto out; }
Alex Chiang
AppliedRe: [PATCH 3/16][BUG] ACPI pci_slot: Fix _STA evaluation (Not for mainline!)
* Kenji Kaneshige <[email protected]>:
Quoted 3 lines
Fix the problem that pci slots that doesn't have
_STAare not detected. If the device doesn't have_STA, we must assume it is always there.
Merged, thanks.
/ac
Quoted 24 lines
Signed-off-by Kenji Kaneshige <[email protected]>--- drivers/acpi/pci_slot.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)Index:
linux-2.6.25-rc6/drivers/acpi/pci_slot.c
===================================================================drivers/acpi/pci_slot.c +2
-3@@ -76,9 +76,8 @@ check_slot(acpi_handle handle, int *deviif (check_sta_before_sun) {/* If SxFy doesn't have _STA, we just assume it's there */ -acpi_evaluate_integer (handle,"_STA" ,NULL , &sta); - -if (!(sta &ACPI_STA_DEVICE_PRESENT )) { + status =acpi_evaluate_integer (handle,"_STA" ,NULL , &sta); +if (ACPI_SUCCESS (status) && !(sta &ACPI_STA_DEVICE_PRESENT )) { retval =-1 ; goto out; }
PATCH 4/16 PCI slot: Add missing semaphore for slot release (Not for mainline!)
In reply to Kenji KaneshigeAdd missing pci_bus sem into pci_destroy_slot(). There is possible race condition because current code doesn't have any serialization in the destroy path.
--- drivers/pci/slot.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/pci/slot.c +2 -0
@@ -196,7 +196,9 @@ EXPORT_SYMBOL_GPL(pci_create_slot);int pci_destroy_slot (struct pci_slot *slot) { +down_write (&pci_bus_sem);kobject_put (&slot->kobj); +up_write (&pci_bus_sem);dbg ("%s: decreased refcount to %d on %x:%d\n" , __func__, slot->kobj.kref.refcount, slot->bus->number, slot->number);
Alex Chiang
AppliedRe: [PATCH 4/16][BUG] PCI slot: Add missing semaphore for slot release (Not for mainline!)
* Kenji Kaneshige <[email protected]>:
Quoted 3 lines
Add missing
pci_bussem intopci_destroy_slot(). There is possible race condition because current code doesn't have any serialization in the destroy path.
Merged, thanks.
/ac
Quoted 23 lines
Signed-off-by Kenji Kaneshige <[email protected]>--- drivers/pci/slot.c | 2 ++ 1 file changed, 2 insertions(+)Index:
linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================drivers/pci/slot.c +2
-0@@ -196,7 +196,9 @@ EXPORT_SYMBOL_GPL(pci_create_slot);int pci_destroy_slot (struct pci_slot *slot) { +down_write (&pci_bus_sem);kobject_put (&slot->kobj); +up_write (&pci_bus_sem);dbg ("%s: decreased refcount to %d on %x:%d\n" , __func__, slot->kobj.kref.refcount, slot->bus->number, slot->number);
PATCH 5/16 PCI slot: Use list_head for pci slot list (Not for mainline!)
In reply to Kenji KaneshigeTo make code simple, we should use list_head for physical slot list on the bus.
--- drivers/acpi/pci_slot.c | 9 ++++++--- drivers/pci/probe.c | 1 + drivers/pci/slot.c | 17 ++++++----------- include/linux/pci.h | 4 ++-- 4 files changed, 15 insertions(+), 16 deletions(-)
drivers/pci/probe.c +1 -0
@@ -384,6 +384,7 @@ static struct pci_bus * pci_alloc_bus(voINIT_LIST_HEAD (&b->node);INIT_LIST_HEAD (&b->children);INIT_LIST_HEAD (&b->devices); +INIT_LIST_HEAD (&b->slots); }return b; }
drivers/pci/slot.c +6 -11
@@ -69,18 +69,12 @@ static int create_sysfs_files(struct pcistatic void pci_slot_release (struct kobject *kobj) { -struct pci_slot **pprev;struct pci_slot *slot =to_pci_slot (kobj);dbg ("%s: releasing pci_slot on %x:%d\n" , __func__, slot->bus->number, slot->number); -for (pprev = &slot->bus->slot; *pprev; pprev = &(*pprev)->next) { -if (*pprev == slot) { - *pprev = slot->next; -break ; - } - } +list_del (&slot->list);if (slot->release) slot->release (slot);
@@ -107,11 +101,12 @@ int pci_slot_add_hotplug(struct pci_bus/* This slot should have already been created, so look for it. If * we can't find it, return -EEXIST. */ -for (slot = parent->slot; slot; slot = slot->next) +list_for_each_entry (slot, &parent->slots, list) {if (slot->number == slot_nr) { found =1 ;break ; } + }if (!found) {dbg ("%s: slot not found\n" , __func__);
@@ -143,7 +138,7 @@ struct pci_slot *pci_create_slot(structdown_write (&pci_bus_sem);/* If we've already created this slot, bump refcount and return. */ -for (slot = parent->slot; slot; slot = slot->next) { +list_for_each_entry (slot, &parent->slots, list) {if (slot->number == slot_nr) {kobject_get (&slot->kobj); dbg("%s: bumped refcount to %d on %x:%d\n",
@@ -175,8 +170,8 @@ struct pci_slot *pci_create_slot(structif (err) goto unregister; - slot->next = parent->slot; - parent->slot = slot; +INIT_LIST_HEAD (&slot->list); +list_add (&slot->list, &parent->slots);dbg ("%s: created pci_slot on %x:%d\n" , __func__, parent->number, slot_nr);
include/linux/pci.h +2 -2
@@ -131,7 +131,7 @@ struct pci_cap_saved_state {/* pci_slot represents a physical slot */ struct pci_slot {struct pci_bus *bus;/* The bus this slot is on */ -struct pci_slot *next;/* Next slot on this bus */ +struct list_head list;/* node in list of slots on this bus */ struct hotplug_slot *hotplug;/* Hotplug info (migrate over time) */ unsigned char number;/* PCI_SLOT(pci_dev->devfn) */ struct kobject kobj;
@@ -269,7 +269,7 @@ struct pci_bus {struct list_head children;/* list of child buses */ struct list_head devices;/* list of devices on this bus */ struct pci_dev *self;/* bridge device as seen by parent */ -struct pci_slot *slot;/* First physical slot on this bus */ +struct list_head slots;/* list of slots on this bus */ struct resource *resource[PCI_BUS_NUM_RESOURCES ];/* address space routed to this bus */
drivers/acpi/pci_slot.c +6 -3
@@ -117,15 +117,18 @@ unregister_slot(acpi_handle handle, u32 {int device;unsigned long sun; -struct pci_slot *slot; +struct pci_slot *slot, *tmp;struct pci_bus *pci_bus = context;if (check_slot (handle, &device, &sun))return AE_OK ; -for (slot = pci_bus->slot; slot; slot = slot->next) { -if (slot->number == device) +/* FIXME - Need pci_bus_sem to be held */ +list_for_each_entry_safe (slot, tmp, &pci_bus->slots, list) { +if (slot->number == device) {pci_destroy_slot (slot); +break ; + } }return AE_OK ;
Alex Chiang
AppliedRe: [PATCH 5/16] PCI slot: Use list_head for pci slot list (Not for mainline!)
* Kenji Kaneshige <[email protected]>:
Quoted 2 lines
To make code simple, we should use
list_headfor physical slot list on the bus.
Merged, thanks.
/ac
Quoted 130 lines
Signed-off-by Kenji Kaneshige <[email protected]>--- drivers/acpi/pci_slot.c | 9 ++++++--- drivers/pci/probe.c | 1 + drivers/pci/slot.c | 17 ++++++----------- include/linux/pci.h | 4 ++-- 4 files changed, 15 insertions(+), 16 deletions(-)Index:
linux-2.6.25-rc6/drivers/pci/probe.c
===================================================================drivers/pci/probe.c +1
-0@@ -384,6 +384,7 @@ static struct pci_bus * pci_alloc_bus(voINIT_LIST_HEAD (&b->node);INIT_LIST_HEAD (&b->children);INIT_LIST_HEAD (&b->devices); +INIT_LIST_HEAD (&b->slots); }return b; }Index:
linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================drivers/pci/slot.c +6
-11@@ -69,18 +69,12 @@ static int create_sysfs_files(struct pcistatic void pci_slot_release (struct kobject *kobj) { -struct pci_slot **pprev;struct pci_slot *slot =to_pci_slot (kobj);dbg ("%s: releasing pci_slot on %x:%d\n" , __func__, slot->bus->number, slot->number); -for (pprev = &slot->bus->slot; *pprev; pprev = &(*pprev)->next) { -if (*pprev == slot) { - *pprev = slot->next; -break ; - } - } +list_del (&slot->list);if (slot->release) slot->release (slot);@@ -107,11 +101,12 @@ int pci_slot_add_hotplug(struct pci_bus/* This slot should have already been created, so look for it. If * we can't find it, return -EEXIST. */ -for (slot = parent->slot; slot; slot = slot->next) +list_for_each_entry (slot, &parent->slots, list) {if (slot->number == slot_nr) { found =1 ;break ; } + }if (!found) {dbg ("%s: slot not found\n" , __func__);@@ -143,7 +138,7 @@ struct pci_slot *pci_create_slot(structdown_write (&pci_bus_sem);/* If we've already created this slot, bump refcount and return. */ -for (slot = parent->slot; slot; slot = slot->next) { +list_for_each_entry (slot, &parent->slots, list) {if (slot->number == slot_nr) {kobject_get (&slot->kobj); dbg("%s: bumped refcount to %d on %x:%d\n",@@ -175,8 +170,8 @@ struct pci_slot *pci_create_slot(structif (err) goto unregister; - slot->next = parent->slot; - parent->slot = slot; +INIT_LIST_HEAD (&slot->list); +list_add (&slot->list, &parent->slots);dbg ("%s: created pci_slot on %x:%d\n" , __func__, parent->number, slot_nr);Index:
linux-2.6.25-rc6/include/linux/pci.h
===================================================================include/linux/pci.h +2
-2@@ -131,7 +131,7 @@ struct pci_cap_saved_state {/* pci_slot represents a physical slot */ struct pci_slot {struct pci_bus *bus;/* The bus this slot is on */ -struct pci_slot *next;/* Next slot on this bus */ +struct list_head list;/* node in list of slots on this bus */ struct hotplug_slot *hotplug;/* Hotplug info (migrate over time) */ unsigned char number;/* PCI_SLOT(pci_dev->devfn) */ struct kobject kobj;@@ -269,7 +269,7 @@ struct pci_bus {struct list_head children;/* list of child buses */ struct list_head devices;/* list of devices on this bus */ struct pci_dev *self;/* bridge device as seen by parent */ -struct pci_slot *slot;/* First physical slot on this bus */ +struct list_head slots;/* list of slots on this bus */ struct resource *resource[PCI_BUS_NUM_RESOURCES ];/* address space routed to this bus */ Index:
linux-2.6.25-rc6/drivers/acpi/pci_slot.c
===================================================================drivers/acpi/pci_slot.c +6
-3@@ -117,15 +117,18 @@ unregister_slot(acpi_handle handle, u32 {int device;unsigned long sun; -struct pci_slot *slot; +struct pci_slot *slot, *tmp;struct pci_bus *pci_bus = context;if (check_slot (handle, &device, &sun))return AE_OK ; -for (slot = pci_bus->slot; slot; slot = slot->next) { -if (slot->number == device) +/* FIXME - Need pci_bus_sem to be held */ +list_for_each_entry_safe (slot, tmp, &pci_bus->slots, list) { +if (slot->number == device) {pci_destroy_slot (slot); +break ; + } }return AE_OK ;
PATCH 6/16 ACPI pci_slot: Fix slot removal path (Not for mainline!)
In reply to Kenji KaneshigeCurrent ACPI pci slot driver scans ACPI namespace and slot list on pci_bus structure to find the pci_slot pointer. It is definitely redundant. In addition, it scans slot list on pci_bus without holding pci_bus_sem. This patch fixes those problems.
--- drivers/acpi/pci_slot.c | 104 +++++++++++++++++++++++++----------------------- 1 file changed, 55 insertions(+), 49 deletions(-)
drivers/acpi/pci_slot.c +55 -49
@@ -55,9 +55,17 @@ ACPI_MODULE_NAME("pci_slot"); MY_NAME , ## arg); \ } while (0) +struct acpi_pci_slot { +acpi_handle root_handle;/* handle of the root bridge */ +struct pci_slot *pci_slot;/* corresponding pci_slot */ +struct list_head list;/* node in the list of slots */ +}; +static int acpi_pci_slot_add (acpi_handle handle);static void acpi_pci_slot_remove (acpi_handle handle); +static LIST_HEAD (slot_list); +static DEFINE_MUTEX (slot_list_lock);static struct acpi_pci_driver acpi_pci_slot_driver = { .add = acpi_pci_slot_add, .remove = acpi_pci_slot_remove,
@@ -105,34 +113,11 @@ out:return retval; } -/* -* unregister_slot -* -* Called once for each SxFy object in the namespace. Each call to -* pci_destroy_slot decrements the refcount on the pci_slot, and -* eventually calls kobject_unregister at the appropriate time. -*/ -static acpi_status -unregister_slot (acpi_handle handle,u32 lvl,void *context,void **rv) -{ -int device; -unsigned long sun; -struct pci_slot *slot, *tmp; -struct pci_bus *pci_bus = context; - -if (check_slot (handle, &device, &sun)) -return AE_OK ; - -/* FIXME - Need pci_bus_sem to be held */ -list_for_each_entry_safe (slot, tmp, &pci_bus->slots, list) { -if (slot->number == device) { -pci_destroy_slot (slot); -break ; - } - } - -return AE_OK ; -} +struct callback_args { +acpi_walk_callback user_function;/* only for walk_p2p_bridge */ +struct pci_bus *pci_bus; +acpi_handle root_handle; +};/* * register_slot
@@ -150,17 +135,33 @@ register_slot(acpi_handle handle, u32 lvint device;unsigned long sun;char name[KOBJ_NAME_LEN ]; - +struct acpi_pci_slot *slot;struct pci_slot *pci_slot; -struct pci_bus *pci_bus = context; +struct callback_args *parent_context = context; +struct pci_bus *pci_bus = parent_context->pci_bus;if (check_slot (handle, &device, &sun))return AE_OK ; + slot =kmalloc (sizeof (*slot),GFP_KERNEL ); +if (!slot) { +err ("%s: cannot allocate memory\n" , __FUNCTION__); +return AE_OK ; + } +snprintf (name,sizeof (name),"%u" , (u32 )sun); pci_slot =pci_create_slot (pci_bus, device, name); -if (IS_ERR (pci_slot)) +if (IS_ERR (pci_slot)) {err ("pci_create_slot returned %ld\n" ,PTR_ERR (pci_slot)); +kfree (slot); + } + + slot->root_handle = parent_context->root_handle; + slot->pci_slot = pci_slot; +INIT_LIST_HEAD (&slot->list); +mutex_lock (&slot_list_lock); +list_add (&slot->list, &slot_list); +mutex_unlock (&slot_list_lock);dbg ("pci_slot: %#lx, pci_bus: %x, device: %d, name: %s\n" , (uint64_t )pci_slot, pci_bus->number, device, name);
@@ -168,11 +169,6 @@ register_slot(acpi_handle handle, u32 lvreturn AE_OK ; } -struct p2p_bridge_context { -acpi_walk_callback user_function; -struct pci_bus *pci_bus; -}; -/* * walk_p2p_bridge - discover and walk p2p bridges * @handle: points to an acpi_pci_root
@@ -192,8 +188,8 @@ walk_p2p_bridge(acpi_handle handle, u32struct pci_dev *dev;struct pci_bus *pci_bus; -struct p2p_bridge_context child_context; -struct p2p_bridge_context *parent_context = context; +struct callback_args child_context; +struct callback_args *parent_context = context; pci_bus = parent_context->pci_bus; user_function = parent_context->user_function;
@@ -213,14 +209,16 @@ walk_p2p_bridge(acpi_handle handle, u32if (!dev || !dev->subordinate) goto out; + child_context.pci_bus = dev->subordinate; + child_context.user_function = user_function; + child_context.root_handle = parent_context->root_handle; +dbg ("p2p bridge walk, pci_bus = %x\n" , dev->subordinate->number); status =acpi_walk_namespace (ACPI_TYPE_DEVICE , handle, (u32 )1 , - user_function, dev->subordinate,NULL ); + user_function, &child_context,NULL );if (ACPI_FAILURE (status)) goto out; - child_context.pci_bus = dev->subordinate; - child_context.user_function = user_function; status =acpi_walk_namespace (ACPI_TYPE_DEVICE , handle, (u32 )1 , walk_p2p_bridge, &child_context,NULL ); out:
@@ -246,7 +244,7 @@ walk_root_bridge(acpi_handle handle, acpacpi_status status;acpi_handle dummy_handle;struct pci_bus *pci_bus; -struct p2p_bridge_context context; +struct callback_args context;/* If the bridge doesn't have _STA, we assume it is always there */ status =acpi_get_handle (handle,"_STA" , &dummy_handle);
@@ -271,14 +269,16 @@ walk_root_bridge(acpi_handle handle, acpif (!pci_bus)return 0 ; + context.pci_bus = pci_bus; + context.user_function = user_function; + context.root_handle = handle; +dbg ("root bridge walk, pci_bus = %x\n" , pci_bus->number); status =acpi_walk_namespace (ACPI_TYPE_DEVICE , handle, (u32 )1 , - user_function, pci_bus,NULL ); + user_function, &context,NULL );if (ACPI_FAILURE (status))return status; - context.pci_bus = pci_bus; - context.user_function = user_function; status =acpi_walk_namespace (ACPI_TYPE_DEVICE , handle, (u32 )1 , walk_p2p_bridge, &context,NULL );if (ACPI_FAILURE (status))
@@ -310,11 +310,17 @@ acpi_pci_slot_add(acpi_handle handle)static void acpi_pci_slot_remove (acpi_handle handle) { -acpi_status status; +struct acpi_pci_slot *slot, *tmp; - status =walk_root_bridge (handle, unregister_slot); -if (ACPI_FAILURE (status)) -err ("%s: unregister_slot failure - %d\n" , __func__, status); +mutex_lock (&slot_list_lock); +list_for_each_entry_safe (slot, tmp, &slot_list, list) { +if (slot->root_handle == handle) { +list_del (&slot->list); +pci_destroy_slot (slot->pci_slot); +kfree (slot); + } + } +mutex_unlock (&slot_list_lock); }#ifdef CONFIG_DMI
Re: [PATCH 6/16][BUG] ACPI pci_slot: Fix slot removal path (Not for mainline!)
* Kenji Kaneshige <[email protected]>:
Quoted 4 lines
Current ACPI pci slot driver scans ACPI namespace and slot list on
pci_busstructure to find thepci_slotpointer. It is definitely redundant. In addition, it scans slot list onpci_buswithout holdingpci_bus_sem. This patch fixes those problems.
Tested and merged, thanks.
/ac
Quoted 202 lines
Signed-off-by Kenji Kaneshige <[email protected]>--- drivers/acpi/pci_slot.c | 104 +++++++++++++++++++++++++----------------------- 1 file changed, 55 insertions(+), 49 deletions(-)Index:
linux-2.6.25-rc6/drivers/acpi/pci_slot.c
===================================================================drivers/acpi/pci_slot.c +55
-49@@ -55,9 +55,17 @@ ACPI_MODULE_NAME("pci_slot"); MY_NAME , ## arg); \ } while (0) +struct acpi_pci_slot { +acpi_handle root_handle;/* handle of the root bridge */ +struct pci_slot *pci_slot;/* corresponding pci_slot */ +struct list_head list;/* node in the list of slots */ +}; +static int acpi_pci_slot_add (acpi_handle handle);static void acpi_pci_slot_remove (acpi_handle handle); +static LIST_HEAD (slot_list); +static DEFINE_MUTEX (slot_list_lock);static struct acpi_pci_driver acpi_pci_slot_driver = { .add = acpi_pci_slot_add, .remove = acpi_pci_slot_remove,@@ -105,34 +113,11 @@ out:return retval; } -/* -* unregister_slot -* -* Called once for each SxFy object in the namespace. Each call to -* pci_destroy_slot decrements the refcount on the pci_slot, and -* eventually calls kobject_unregister at the appropriate time. -*/ -static acpi_status -unregister_slot (acpi_handle handle,u32 lvl,void *context,void **rv) -{ -int device; -unsigned long sun; -struct pci_slot *slot, *tmp; -struct pci_bus *pci_bus = context; - -if (check_slot (handle, &device, &sun)) -return AE_OK ; - -/* FIXME - Need pci_bus_sem to be held */ -list_for_each_entry_safe (slot, tmp, &pci_bus->slots, list) { -if (slot->number == device) { -pci_destroy_slot (slot); -break ; - } - } - -return AE_OK ; -} +struct callback_args { +acpi_walk_callback user_function;/* only for walk_p2p_bridge */ +struct pci_bus *pci_bus; +acpi_handle root_handle; +};/* * register_slot @@ -150,17 +135,33 @@ register_slot(acpi_handle handle, u32 lvint device;unsigned long sun;char name[KOBJ_NAME_LEN ]; - +struct acpi_pci_slot *slot;struct pci_slot *pci_slot; -struct pci_bus *pci_bus = context; +struct callback_args *parent_context = context; +struct pci_bus *pci_bus = parent_context->pci_bus;if (check_slot (handle, &device, &sun))return AE_OK ; + slot =kmalloc (sizeof (*slot),GFP_KERNEL ); +if (!slot) { +err ("%s: cannot allocate memory\n" , __FUNCTION__); +return AE_OK ; + } +snprintf (name,sizeof (name),"%u" , (u32 )sun); pci_slot =pci_create_slot (pci_bus, device, name); -if (IS_ERR (pci_slot)) +if (IS_ERR (pci_slot)) {err ("pci_create_slot returned %ld\n" ,PTR_ERR (pci_slot)); +kfree (slot); + } + + slot->root_handle = parent_context->root_handle; + slot->pci_slot = pci_slot; +INIT_LIST_HEAD (&slot->list); +mutex_lock (&slot_list_lock); +list_add (&slot->list, &slot_list); +mutex_unlock (&slot_list_lock);dbg ("pci_slot: %#lx, pci_bus: %x, device: %d, name: %s\n" , (uint64_t )pci_slot, pci_bus->number, device, name);@@ -168,11 +169,6 @@ register_slot(acpi_handle handle, u32 lvreturn AE_OK ; } -struct p2p_bridge_context { -acpi_walk_callback user_function; -struct pci_bus *pci_bus; -}; -/* * walk_p2p_bridge - discover and walk p2p bridges * @handle: points to an acpi_pci_root @@ -192,8 +188,8 @@ walk_p2p_bridge(acpi_handle handle, u32struct pci_dev *dev;struct pci_bus *pci_bus; -struct p2p_bridge_context child_context; -struct p2p_bridge_context *parent_context = context; +struct callback_args child_context; +struct callback_args *parent_context = context; pci_bus = parent_context->pci_bus; user_function = parent_context->user_function;@@ -213,14 +209,16 @@ walk_p2p_bridge(acpi_handle handle, u32if (!dev || !dev->subordinate) goto out; + child_context.pci_bus = dev->subordinate; + child_context.user_function = user_function; + child_context.root_handle = parent_context->root_handle; +dbg ("p2p bridge walk, pci_bus = %x\n" , dev->subordinate->number); status =acpi_walk_namespace (ACPI_TYPE_DEVICE , handle, (u32 )1 , - user_function, dev->subordinate,NULL ); + user_function, &child_context,NULL );if (ACPI_FAILURE (status)) goto out; - child_context.pci_bus = dev->subordinate; - child_context.user_function = user_function; status =acpi_walk_namespace (ACPI_TYPE_DEVICE , handle, (u32 )1 , walk_p2p_bridge, &child_context,NULL ); out:@@ -246,7 +244,7 @@ walk_root_bridge(acpi_handle handle, acpacpi_status status;acpi_handle dummy_handle;struct pci_bus *pci_bus; -struct p2p_bridge_context context; +struct callback_args context;/* If the bridge doesn't have _STA, we assume it is always there */ status =acpi_get_handle (handle,"_STA" , &dummy_handle);@@ -271,14 +269,16 @@ walk_root_bridge(acpi_handle handle, acpif (!pci_bus)return 0 ; + context.pci_bus = pci_bus; + context.user_function = user_function; + context.root_handle = handle; +dbg ("root bridge walk, pci_bus = %x\n" , pci_bus->number); status =acpi_walk_namespace (ACPI_TYPE_DEVICE , handle, (u32 )1 , - user_function, pci_bus,NULL ); + user_function, &context,NULL );if (ACPI_FAILURE (status))return status; - context.pci_bus = pci_bus; - context.user_function = user_function; status =acpi_walk_namespace (ACPI_TYPE_DEVICE , handle, (u32 )1 , walk_p2p_bridge, &context,NULL );if (ACPI_FAILURE (status))@@ -310,11 +310,17 @@ acpi_pci_slot_add(acpi_handle handle)static void acpi_pci_slot_remove (acpi_handle handle) { -acpi_status status; +struct acpi_pci_slot *slot, *tmp; - status =walk_root_bridge (handle, unregister_slot); -if (ACPI_FAILURE (status)) -err ("%s: unregister_slot failure - %d\n" , __func__, status); +mutex_lock (&slot_list_lock); +list_for_each_entry_safe (slot, tmp, &slot_list, list) { +if (slot->root_handle == handle) { +list_del (&slot->list); +pci_destroy_slot (slot->pci_slot); +kfree (slot); + } + } +mutex_unlock (&slot_list_lock); }#ifdef CONFIG_DMI
PATCH 7/16 PCI slot: Remove compiler warnings (Not for mainline!)
In reply to Kenji KaneshigeRemove following compiler warnings.
drivers/pci/slot.c: In function 'pci_create_slot':drivers/pci/slot.c:146: warning: format '%d' expects type 'int', but argument 4 has type 'atomic_t' drivers/pci/slot.c: In function 'pci_destroy_slot':drivers/pci/slot.c:198: warning: format '%d' expects type 'int', but argument 4 has type 'atomic_t'
--- drivers/pci/slot.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/pci/slot.c +4 -3
@@ -142,8 +142,8 @@ struct pci_slot *pci_create_slot(structif (slot->number == slot_nr) {kobject_get (&slot->kobj);dbg ("%s: bumped refcount to %d on %x:%d\n" , - __func__, slot->kobj.kref.refcount, - parent->number, slot_nr); + __func__,atomic_read (&slot->kobj.kref.refcount), + parent->number, slot_nr); goto out; } }
@@ -196,7 +196,8 @@ int pci_destroy_slot(struct pci_slot *slup_write (&pci_bus_sem);dbg ("%s: decreased refcount to %d on %x:%d\n" , __func__, - slot->kobj.kref.refcount, slot->bus->number, slot->number); +atomic_read (&slot->kobj.kref.refcount), slot->bus->number, + slot->number);return 0 ; }EXPORT_SYMBOL_GPL (pci_destroy_slot);
Alex Chiang
AppliedRe: [PATCH 7/16][BUG] PCI slot: Remove compiler warnings (Not for mainline!)
* Kenji Kaneshige <[email protected]>:
Quoted 1 line
Remove following compiler warnings.
Merged, thanks.
/ac
Quoted 39 lines
drivers/pci/slot.c: In function 'pci_create_slot':drivers/pci/slot.c:146: warning: format '%d' expects type 'int', but argument 4 has type 'atomic_t'drivers/pci/slot.c: In function 'pci_destroy_slot':drivers/pci/slot.c:198: warning: format '%d' expects type 'int', but argument 4 has type 'atomic_t'Signed-off-by Kenji Kaneshige <[email protected]>--- drivers/pci/slot.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)Index:
linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================drivers/pci/slot.c +4
-3@@ -142,8 +142,8 @@ struct pci_slot *pci_create_slot(structif (slot->number == slot_nr) {kobject_get (&slot->kobj);dbg ("%s: bumped refcount to %d on %x:%d\n" , - __func__, slot->kobj.kref.refcount, - parent->number, slot_nr); + __func__,atomic_read (&slot->kobj.kref.refcount), + parent->number, slot_nr); goto out; } }@@ -196,7 +196,8 @@ int pci_destroy_slot(struct pci_slot *slup_write (&pci_bus_sem);dbg ("%s: decreased refcount to %d on %x:%d\n" , __func__, - slot->kobj.kref.refcount, slot->bus->number, slot->number); +atomic_read (&slot->kobj.kref.refcount), slot->bus->number, + slot->number);return 0 ; }EXPORT_SYMBOL_GPL (pci_destroy_slot);
PATCH 8/16 PCI slot: Fix invalid memory access (Not for mainline!)
In reply to Kenji KaneshigeFix kernel oops in pci_release_slot() when dbg() is enabled.
The dbg() in pci_release_slot() cause a kernel oops (invalid memory access) because it accesses slot after it is released. We need to access slot before calling kobject_put().
--- drivers/pci/slot.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/pci/slot.c +4 -3
@@ -191,13 +191,14 @@ EXPORT_SYMBOL_GPL(pci_create_slot);int pci_destroy_slot (struct pci_slot *slot) { +dbg ("%s: decreased refcount to %d on %x:%d\n" , __func__, +atomic_read (&slot->kobj.kref.refcount) -1 , slot->bus->number, + slot->number); +down_write (&pci_bus_sem);kobject_put (&slot->kobj);up_write (&pci_bus_sem); -dbg ("%s: decreased refcount to %d on %x:%d\n" , __func__, -atomic_read (&slot->kobj.kref.refcount), slot->bus->number, - slot->number);return 0 ; }EXPORT_SYMBOL_GPL (pci_destroy_slot);
Alex Chiang
AppliedRe: [PATCH 8/16][BUG] PCI slot: Fix invalid memory access (Not for mainline!)
* Kenji Kaneshige <[email protected]>:
Quoted 1 line
Fix kernel oops in
pci_release_slot()whendbg()is enabled.
Merged, thanks.
/ac
Quoted 35 lines
The
dbg()inpci_release_slot()cause a kernel oops (invalid memory access) because it accesses slot after it is released. We need to access slot before callingkobject_put().Signed-off-by Kenji Kaneshige <[email protected]>--- drivers/pci/slot.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)Index:
linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================drivers/pci/slot.c +4
-3@@ -191,13 +191,14 @@ EXPORT_SYMBOL_GPL(pci_create_slot);int pci_destroy_slot (struct pci_slot *slot) { +dbg ("%s: decreased refcount to %d on %x:%d\n" , __func__, +atomic_read (&slot->kobj.kref.refcount) -1 , slot->bus->number, + slot->number); +down_write (&pci_bus_sem);kobject_put (&slot->kobj);up_write (&pci_bus_sem); -dbg ("%s: decreased refcount to %d on %x:%d\n" , __func__, -atomic_read (&slot->kobj.kref.refcount), slot->bus->number, - slot->number);return 0 ; }EXPORT_SYMBOL_GPL (pci_destroy_slot);
PATCH 9/16 PCI slot: Remove unused slot member from pci_dev (Not for mainline!)
In reply to Kenji KaneshigeThe 'slot' member in struct pci_dev is not used. Remove this unused member.
--- include/linux/pci.h | 1 - 1 file changed, 1 deletion(-)
include/linux/pci.h +0 -1
@@ -149,7 +149,6 @@ struct pci_dev {void *sysdata;/* hook for sys-specific extension */ struct proc_dir_entry *procent;/* device entry in /proc/bus/pci */ -struct pci_slot *slot;/* Physical slot this device is in */ unsigned int devfn;/* encoded device & function index */ unsigned short vendor;
Re: [PATCH 9/16] PCI slot: Remove unused slot member from pci_dev (Not for mainline!)
Quoted 2 lines · Kenji Kaneshige
The 'slot' member in struct
pci_devis not used. Remove this unused member.
It *ought* to be used. If it isn't yet, I guess that part of my patch has been dropped, and it should be re-added.
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Re: [PATCH 9/16] PCI slot: Remove unused slot member from pci_dev (Not for mainline!)
Hi willy,
* Matthew Wilcox <[email protected]>:
Quoted 6 lines · Kenji Kaneshige
Quoted 2 lines
The 'slot' member in struct
pci_devis not used. Remove this unused member.It *ought* to be used. If it isn't yet, I guess that part of my patch has been dropped, and it should be re-added.
I checked through your original patch set:
http://www.parisc-linux.org/~willy/pci-slot-2b/
And although I see a bunch of uses of pci_bus->slot, I don't see any uses of pci_dev->slot.
Wanna enlighten me? :)
/ac
PATCH 10/16 PCI slot: Replace dbg with pr_debug (Not for mainline!)
In reply to Kenji KaneshigeThe dbg() macro doesn't work because the 'pci_slot_debug' flag never be set unless we manually modify drivers/pci/slots.c. So we should use pr_debug() instead. This patch replaces dbg() with pr_debug().
--- drivers/pci/slot.c | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-)
drivers/pci/slot.c +15 -23
@@ -9,15 +9,6 @@#include <linux/pci.h> #include "pci.h" -static int pci_slot_debug; -#define MY_NAME "slot" -#define dbg(format, arg...) \ - do { \ - if (pci_slot_debug) \ - printk(KERN_DEBUG "%s: " format, \ - MY_NAME , ## arg); \ - } while (0) -struct kset *pci_slots_kset;EXPORT_SYMBOL_GPL (pci_slots_kset);
@@ -71,8 +62,8 @@ static void pci_slot_release(struct kobj {struct pci_slot *slot =to_pci_slot (kobj); -dbg ("%s: releasing pci_slot on %x:%d\n" , __func__, - slot->bus->number, slot->number); +pr_debug ("%s: releasing pci_slot on %x:%d\n" , __func__, + slot->bus->number, slot->number);list_del (&slot->list);
@@ -109,19 +100,19 @@ int pci_slot_add_hotplug(struct pci_bus }if (!found) { -dbg ("%s: slot not found\n" , __func__); +pr_debug ("%s: slot not found\n" , __func__); retval = -EEXIST ; goto out; }if (slot->release) { -dbg ("%s: already claimed\n" , __func__); +pr_debug ("%s: already claimed\n" , __func__); retval = -EBUSY ; goto out; } -dbg ("%s: adding release function to %x:%d\n" , - __func__, parent->number, slot_nr); +pr_debug ("%s: adding release function to %x:%d\n" , + __func__, parent->number, slot_nr); slot->release = release; out:up_write (&pci_bus_sem);
@@ -141,9 +132,10 @@ struct pci_slot *pci_create_slot(structlist_for_each_entry (slot, &parent->slots, list) {if (slot->number == slot_nr) {kobject_get (&slot->kobj); -dbg ("%s: bumped refcount to %d on %x:%d\n" , - __func__,atomic_read (&slot->kobj.kref.refcount), - parent->number, slot_nr); +pr_debug ("%s: bumped refcount to %d on %x:%d\n" , + __func__, +atomic_read (&slot->kobj.kref.refcount), + parent->number, slot_nr); goto out; } }
@@ -173,8 +165,8 @@ struct pci_slot *pci_create_slot(structINIT_LIST_HEAD (&slot->list);list_add (&slot->list, &parent->slots); -dbg ("%s: created pci_slot on %x:%d\n" , - __func__, parent->number, slot_nr); +pr_debug ("%s: created pci_slot on %x:%d\n" , + __func__, parent->number, slot_nr); out:up_write (&pci_bus_sem);
@@ -191,9 +183,9 @@ EXPORT_SYMBOL_GPL(pci_create_slot);int pci_destroy_slot (struct pci_slot *slot) { -dbg ("%s: decreased refcount to %d on %x:%d\n" , __func__, -atomic_read (&slot->kobj.kref.refcount) -1 , slot->bus->number, - slot->number); +pr_debug ("%s: decreased refcount to %d on %x:%d\n" , __func__, +atomic_read (&slot->kobj.kref.refcount) -1 , slot->bus->number, + slot->number);down_write (&pci_bus_sem);kobject_put (&slot->kobj);
Re: [PATCH 10/16] PCI slot: Replace dbg with pr_debug (Not for mainline!)
Quoted 3 lines · Kenji Kaneshige
The
dbg()macro doesn't work because the 'pci_slot_debug' flag never be set unless we manually modifydrivers/pci/slots.c. So we should usepr_debug()instead. This patch replacesdbg()withpr_debug().
ACK.
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Alex Chiang
AppliedRe: [PATCH 10/16] PCI slot: Replace dbg with pr_debug (Not for mainline!)
In reply to Kenji Kaneshige* Kenji Kaneshige <[email protected]>:
Quoted 3 lines
The
dbg()macro doesn't work because the 'pci_slot_debug' flag never be set unless we manually modifydrivers/pci/slots.c. So we should usepr_debug()instead. This patch replacesdbg()withpr_debug().
Merged, thanks.
/ac
Quoted 102 lines
Signed-off-by Kenji Kaneshige <[email protected]>--- drivers/pci/slot.c | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-)Index:
linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================drivers/pci/slot.c +15
-23@@ -9,15 +9,6 @@#include <linux/pci.h> #include "pci.h" -static int pci_slot_debug; -#define MY_NAME "slot" -#define dbg(format, arg...) \ - do { \ - if (pci_slot_debug) \ - printk(KERN_DEBUG "%s: " format, \ - MY_NAME , ## arg); \ - } while (0) -struct kset *pci_slots_kset;EXPORT_SYMBOL_GPL (pci_slots_kset);@@ -71,8 +62,8 @@ static void pci_slot_release(struct kobj {struct pci_slot *slot =to_pci_slot (kobj); -dbg ("%s: releasing pci_slot on %x:%d\n" , __func__, - slot->bus->number, slot->number); +pr_debug ("%s: releasing pci_slot on %x:%d\n" , __func__, + slot->bus->number, slot->number);list_del (&slot->list);@@ -109,19 +100,19 @@ int pci_slot_add_hotplug(struct pci_bus }if (!found) { -dbg ("%s: slot not found\n" , __func__); +pr_debug ("%s: slot not found\n" , __func__); retval = -EEXIST ; goto out; }if (slot->release) { -dbg ("%s: already claimed\n" , __func__); +pr_debug ("%s: already claimed\n" , __func__); retval = -EBUSY ; goto out; } -dbg ("%s: adding release function to %x:%d\n" , - __func__, parent->number, slot_nr); +pr_debug ("%s: adding release function to %x:%d\n" , + __func__, parent->number, slot_nr); slot->release = release; out:up_write (&pci_bus_sem);@@ -141,9 +132,10 @@ struct pci_slot *pci_create_slot(structlist_for_each_entry (slot, &parent->slots, list) {if (slot->number == slot_nr) {kobject_get (&slot->kobj); -dbg ("%s: bumped refcount to %d on %x:%d\n" , - __func__,atomic_read (&slot->kobj.kref.refcount), - parent->number, slot_nr); +pr_debug ("%s: bumped refcount to %d on %x:%d\n" , + __func__, +atomic_read (&slot->kobj.kref.refcount), + parent->number, slot_nr); goto out; } }@@ -173,8 +165,8 @@ struct pci_slot *pci_create_slot(structINIT_LIST_HEAD (&slot->list);list_add (&slot->list, &parent->slots); -dbg ("%s: created pci_slot on %x:%d\n" , - __func__, parent->number, slot_nr); +pr_debug ("%s: created pci_slot on %x:%d\n" , + __func__, parent->number, slot_nr); out:up_write (&pci_bus_sem);@@ -191,9 +183,9 @@ EXPORT_SYMBOL_GPL(pci_create_slot);int pci_destroy_slot (struct pci_slot *slot) { -dbg ("%s: decreased refcount to %d on %x:%d\n" , __func__, -atomic_read (&slot->kobj.kref.refcount) -1 , slot->bus->number, - slot->number); +pr_debug ("%s: decreased refcount to %d on %x:%d\n" , __func__, +atomic_read (&slot->kobj.kref.refcount) -1 , slot->bus->number, + slot->number);down_write (&pci_bus_sem);kobject_put (&slot->kobj);
PATCH 11/16 PCI slot: Remove useless release handler (Not for mainline!)
In reply to Kenji KaneshigeThe release() handler of struct pci_slot never be called because pci_hp_deregister() calls pci_slot_destroy() after setting slot->release with NULL. So we don't need release() handler of struct pci_slot. In addition, we don't need pci_slot_add_hotplug().
--- drivers/pci/hotplug/pci_hotplug_core.c | 15 ----------- drivers/pci/slot.c | 44 --------------------------------- include/linux/pci.h | 3 -- 3 files changed, 1 insertion(+), 61 deletions(-)
drivers/pci/slot.c +0 -44
@@ -67,9 +67,6 @@ static void pci_slot_release(struct kobjlist_del (&slot->list); -if (slot->release) - slot->release (slot); -remove_sysfs_files (slot);kfree (slot); }
@@ -79,47 +76,6 @@ static struct kobj_type pci_slot_ktype = .release = &pci_slot_release, }; -int pci_slot_add_hotplug (struct pci_bus *parent,int slot_nr, -void (*release)(struct pci_slot *)) -{ -struct pci_slot *slot; -int retval, found; - - retval = found =0 ; - -down_write (&pci_bus_sem); - -/* This slot should have already been created, so look for it. If -* we can't find it, return -EEXIST. -*/ -list_for_each_entry (slot, &parent->slots, list) { -if (slot->number == slot_nr) { - found =1 ; -break ; - } - } - -if (!found) { -pr_debug ("%s: slot not found\n" , __func__); - retval = -EEXIST ; - goto out; - } - -if (slot->release) { -pr_debug ("%s: already claimed\n" , __func__); - retval = -EBUSY ; - goto out; - } - -pr_debug ("%s: adding release function to %x:%d\n" , - __func__, parent->number, slot_nr); - slot->release = release; - out: -up_write (&pci_bus_sem); -return retval; -} -EXPORT_SYMBOL_GPL (pci_slot_add_hotplug); -struct pci_slot *pci_create_slot (struct pci_bus *parent,int slot_nr,const char *name) {
include/linux/pci.h +0 -3
@@ -135,7 +135,6 @@ struct pci_slot {struct hotplug_slot *hotplug;/* Hotplug info (migrate over time) */ unsigned char number;/* PCI_SLOT(pci_dev->devfn) */ struct kobject kobj; -void (*release)(struct pci_slot *); };/*
@@ -494,8 +493,6 @@ struct pci_bus *pci_add_new_bus(struct p int busnr);struct pci_slot *pci_create_slot (struct pci_bus *parent,int slot_nr,const char *name); -int pci_slot_add_hotplug (struct pci_bus *parent,int slot_nr, -void (*release)(struct pci_slot *));int pci_destroy_slot (struct pci_slot *slot);int pci_scan_slot (struct pci_bus *bus,int devfn);struct pci_dev *pci_scan_single_device (struct pci_bus *bus,int devfn);
drivers/pci/hotplug/pci_hotplug_core.c +1 -14
@@ -537,12 +537,6 @@ static struct hotplug_slot *get_slot_froreturn NULL ; } -static void hotplug_release (struct pci_slot *slot) -{ -struct hotplug_slot *hotplug = slot->hotplug; - hotplug->release (hotplug); -} -/** * pci_hp_register - register a hotplug_slot with the PCI hotplug subsystem * @slot: pointer to the &struct hotplug_slot to register
@@ -576,12 +570,6 @@ int pci_hp_register(struct hotplug_slotif (IS_ERR (pci_slot))return PTR_ERR (pci_slot); - result =pci_slot_add_hotplug (bus, slot_nr, hotplug_release); -if (result) { -pci_destroy_slot (pci_slot); -return result; - } - slot->pci_slot = pci_slot; pci_slot->hotplug = slot;
@@ -631,8 +619,7 @@ int pci_hp_deregister(struct hotplug_slofs_remove_slot (slot);dbg ("Removed slot %s from the list\n" , hotplug->name); -hotplug_release (slot); - slot->release =NULL ; + hotplug->release (hotplug);pci_destroy_slot (slot);return 0 ;
Re: [PATCH 11/16] PCI slot: Remove useless release handler (Not for mainline!)
* Kenji Kaneshige <[email protected]>:
Quoted 4 lines
The
release()handler of structpci_slotnever be called becausepci_hp_deregister()callspci_slot_destroy()after settingslot->releasewith NULL. So we don't needrelease()handler of structpci_slot. In addition, we don't needpci_slot_add_hotplug().
Thanks, this was a good change. I've tested and merged it.
I did make two small changes though:
Quoted 110 lines
Signed-off-by Kenji Kaneshige <[email protected]>--- drivers/pci/hotplug/pci_hotplug_core.c | 15 ----------- drivers/pci/slot.c | 44 --------------------------------- include/linux/pci.h | 3 -- 3 files changed, 1 insertion(+), 61 deletions(-)Index:
linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================drivers/pci/slot.c +0
-44@@ -67,9 +67,6 @@ static void pci_slot_release(struct kobjlist_del (&slot->list); -if (slot->release) - slot->release (slot); -remove_sysfs_files (slot);kfree (slot); }@@ -79,47 +76,6 @@ static struct kobj_type pci_slot_ktype = .release = &pci_slot_release, }; -int pci_slot_add_hotplug (struct pci_bus *parent,int slot_nr, -void (*release)(struct pci_slot *)) -{ -struct pci_slot *slot; -int retval, found; - - retval = found =0 ; - -down_write (&pci_bus_sem); - -/* This slot should have already been created, so look for it. If -* we can't find it, return -EEXIST. -*/ -list_for_each_entry (slot, &parent->slots, list) { -if (slot->number == slot_nr) { - found =1 ; -break ; - } - } - -if (!found) { -pr_debug ("%s: slot not found\n" , __func__); - retval = -EEXIST ; - goto out; - } - -if (slot->release) { -pr_debug ("%s: already claimed\n" , __func__); - retval = -EBUSY ; - goto out; - } - -pr_debug ("%s: adding release function to %x:%d\n" , - __func__, parent->number, slot_nr); - slot->release = release; - out: -up_write (&pci_bus_sem); -return retval; -} -EXPORT_SYMBOL_GPL (pci_slot_add_hotplug); -struct pci_slot *pci_create_slot (struct pci_bus *parent,int slot_nr,const char *name) {Index:
linux-2.6.25-rc6/include/linux/pci.h
===================================================================include/linux/pci.h +0
-3@@ -135,7 +135,6 @@ struct pci_slot {struct hotplug_slot *hotplug;/* Hotplug info (migrate over time) */ unsigned char number;/* PCI_SLOT(pci_dev->devfn) */ struct kobject kobj; -void (*release)(struct pci_slot *); };/* @@ -494,8 +493,6 @@ struct pci_bus *pci_add_new_bus(struct p int busnr);struct pci_slot *pci_create_slot (struct pci_bus *parent,int slot_nr,const char *name); -int pci_slot_add_hotplug (struct pci_bus *parent,int slot_nr, -void (*release)(struct pci_slot *));int pci_destroy_slot (struct pci_slot *slot);int pci_scan_slot (struct pci_bus *bus,int devfn);struct pci_dev *pci_scan_single_device (struct pci_bus *bus,int devfn);Index:
linux-2.6.25-rc6/drivers/pci/hotplug/pci_hotplug_core.c
===================================================================drivers/pci/hotplug/pci_hotplug_core.c +0
-6@@ -537,12 +537,6 @@ static struct hotplug_slot *get_slot_froreturn NULL ; } -static void hotplug_release (struct pci_slot *slot) -{ -struct hotplug_slot *hotplug = slot->hotplug; - hotplug->release (hotplug); -} -/** * pci_hp_register - register a hotplug_slot with the PCI hotplug subsystem * @slot: pointer to the &struct hotplug_slot to register
drivers/pci/hotplug/pci_hotplug_core.c int pci_hp_register(struct hotplug_slot · line 571
Quoted 2 lines
drivers/pci/hotplug/pci_hotplug_core.c +0
-0@@ -576,12 +570,6 @@ int pci_hp_register(struct hotplug_slotif (IS_ERR (pci_slot))return PTR_ERR (pci_slot);
This prevents multiple hotplug drivers from claiming the same slot.
+ if (pci_slot->hotplug) {
+ dbg("%s: already claimed\n", __func__);
+ pci_destroy_slot(pci_slot);
+ return -EBUSY;Quoted 11 lines
drivers/pci/hotplug/pci_hotplug_core.c +0
-6- result =pci_slot_add_hotplug (bus, slot_nr, hotplug_release); -if (result) { -pci_destroy_slot (pci_slot); -return result; - } - slot->pci_slot = pci_slot; pci_slot->hotplug = slot;
drivers/pci/hotplug/pci_hotplug_core.c int pci_hp_deregister(struct hotplug_slo · line 622
Quoted 6 lines
drivers/pci/hotplug/pci_hotplug_core.c +1
-2@@ -631,8 +619,7 @@ int pci_hp_deregister(struct hotplug_slofs_remove_slot (slot);dbg ("Removed slot %s from the list\n" , hotplug->name); -hotplug_release (slot); - slot->release =NULL ; + hotplug->release (hotplug);
+ slot->hotplug = NULL;
The above is needed if pci_slot is loaded, and you wish to repeatedly load/unload acpiphp/pciehp/other hp drivers.
Thanks,
/ac
Quoted 5 lines
drivers/pci/hotplug/pci_hotplug_core.c +0
-0pci_destroy_slot (slot);return 0 ;
PATCH 12/16 PCI slot: Use .default_attrs for address file (Not for mainline!)
In reply to Kenji KaneshigeUse .default_attrs for 'address' file. This can simplify the code.
--- drivers/pci/slot.c | 36 +++++++++--------------------------- 1 file changed, 9 insertions(+), 27 deletions(-)
drivers/pci/slot.c +9 -27
@@ -39,25 +39,6 @@ static ssize_t address_read_file(struct slot->bus->number, slot->number); } -static struct pci_slot_attribute pci_slot_attr_address = { - .attr = { .name ="address" , .mode =S_IFREG |S_IRUGO }, - .show = address_read_file, -}; - -static void remove_sysfs_files (struct pci_slot *slot) -{ -sysfs_remove_file (&slot->kobj, &pci_slot_attr_address.attr); -} - -static int create_sysfs_files (struct pci_slot *slot) -{ -int result; - - result =sysfs_create_file (&slot->kobj, &pci_slot_attr_address.attr); - -return result; -} -static void pci_slot_release (struct kobject *kobj) {struct pci_slot *slot =to_pci_slot (kobj);
@@ -67,13 +48,21 @@ static void pci_slot_release(struct kobjlist_del (&slot->list); -remove_sysfs_files (slot);kfree (slot); } +static struct pci_slot_attribute pci_slot_attr_address = +__ATTR (address, (S_IFREG |S_IRUGO ), address_read_file,NULL ); + +static struct attribute *pci_slot_default_attrs[] = { + &pci_slot_attr_address.attr, +NULL , +}; +static struct kobj_type pci_slot_ktype = { .sysfs_ops = &pci_slot_sysfs_ops, .release = &pci_slot_release, + .default_attrs = pci_slot_default_attrs, }; struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
@@ -114,10 +103,6 @@ struct pci_slot *pci_create_slot(struct goto err; } - err =create_sysfs_files (slot); -if (err) - goto unregister; -INIT_LIST_HEAD (&slot->list);list_add (&slot->list, &parent->slots);
@@ -127,9 +112,6 @@ struct pci_slot *pci_create_slot(struct out:up_write (&pci_bus_sem);return slot; - - unregister: -kobject_put (&slot->kobj); err:kfree (slot); slot =ERR_PTR (err);
Re: [PATCH 12/16] PCI slot: Use .default_attrs for address file (Not for mainline!)
Quoted 1 line · Kenji Kaneshige
Use .default_attrs for 'address' file. This can simplify the code.
Seems like a good idea to me. I don't think default_attrs was available when I initially did this work.
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Re: [PATCH 12/16] PCI slot: Use .default_attrs for address file (Not for mainline!)
In reply to Kenji Kaneshige* Kenji Kaneshige <[email protected]>:
Quoted 1 line
Use .default_attrs for 'address' file. This can simplify the code.
Thanks, merged.
/ac
Quoted 83 lines
Signed-off-by Kenji Kaneshige <[email protected]>--- drivers/pci/slot.c | 36 +++++++++--------------------------- 1 file changed, 9 insertions(+), 27 deletions(-)Index:
linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================drivers/pci/slot.c +9
-27@@ -39,25 +39,6 @@ static ssize_t address_read_file(struct slot->bus->number, slot->number); } -static struct pci_slot_attribute pci_slot_attr_address = { - .attr = { .name ="address" , .mode =S_IFREG |S_IRUGO }, - .show = address_read_file, -}; - -static void remove_sysfs_files (struct pci_slot *slot) -{ -sysfs_remove_file (&slot->kobj, &pci_slot_attr_address.attr); -} - -static int create_sysfs_files (struct pci_slot *slot) -{ -int result; - - result =sysfs_create_file (&slot->kobj, &pci_slot_attr_address.attr); - -return result; -} -static void pci_slot_release (struct kobject *kobj) {struct pci_slot *slot =to_pci_slot (kobj);@@ -67,13 +48,21 @@ static void pci_slot_release(struct kobjlist_del (&slot->list); -remove_sysfs_files (slot);kfree (slot); } +static struct pci_slot_attribute pci_slot_attr_address = +__ATTR (address, (S_IFREG |S_IRUGO ), address_read_file,NULL ); + +static struct attribute *pci_slot_default_attrs[] = { + &pci_slot_attr_address.attr, +NULL , +}; +static struct kobj_type pci_slot_ktype = { .sysfs_ops = &pci_slot_sysfs_ops, .release = &pci_slot_release, + .default_attrs = pci_slot_default_attrs, }; struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,@@ -114,10 +103,6 @@ struct pci_slot *pci_create_slot(struct goto err; } - err =create_sysfs_files (slot); -if (err) - goto unregister; -INIT_LIST_HEAD (&slot->list);list_add (&slot->list, &parent->slots);@@ -127,9 +112,6 @@ struct pci_slot *pci_create_slot(struct out:up_write (&pci_bus_sem);return slot; - - unregister: -kobject_put (&slot->kobj); err:kfree (slot); slot =ERR_PTR (err);
PATCH 13/16 PCI slot: Fix return value of pci_create_slot() (Not for mainline!)
In reply to Kenji KaneshigeWe can use the error code from kobject_init_and_add() as it is.
--- drivers/pci/slot.c | 1 - 1 file changed, 1 deletion(-)
drivers/pci/slot.c +0 -1
@@ -99,7 +99,6 @@ struct pci_slot *pci_create_slot(struct "%s", name);if (err) {printk (KERN_ERR "Unable to register kobject %s\n" , name); - err = -EINVAL ; goto err; }
Re: [PATCH 13/16] PCI slot: Fix return value of pci_create_slot() (Not for mainline!)
* Kenji Kaneshige <[email protected]>:
Quoted 1 line
We can use the error code from
kobject_init_and_add()as it is.
Thanks, merged.
/ac
Quoted 21 lines
Signed-off-by Kenji Kaneshige <[email protected]>--- drivers/pci/slot.c | 1 - 1 file changed, 1 deletion(-)Index:
linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================drivers/pci/slot.c +0
-1@@ -99,7 +99,6 @@ struct pci_slot *pci_create_slot(struct "%s", name);if (err) {printk (KERN_ERR "Unable to register kobject %s\n" , name); - err = -EINVAL ; goto err; }
PATCH 14/16 PCI slot: Change return value of pci_destroy_slot() (Not for mainline!)
In reply to Kenji KaneshigeSince nobody checks the return value from pci_destroy_slot(), it should be void.
--- drivers/pci/slot.c | 4 +--- include/linux/pci.h | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-)
drivers/pci/slot.c +1 -3
@@ -118,7 +118,7 @@ struct pci_slot *pci_create_slot(struct }EXPORT_SYMBOL_GPL (pci_create_slot); -int pci_destroy_slot (struct pci_slot *slot) +void pci_destroy_slot (struct pci_slot *slot) { pr_debug("%s: decreased refcount to %d on %x:%d\n", __func__, atomic_read(&slot->kobj.kref.refcount) - 1, slot->bus->number,
@@ -127,8 +127,6 @@ int pci_destroy_slot(struct pci_slot *sldown_write (&pci_bus_sem);kobject_put (&slot->kobj);up_write (&pci_bus_sem); - -return 0 ; }EXPORT_SYMBOL_GPL (pci_destroy_slot);
include/linux/pci.h +1 -1
@@ -493,7 +493,7 @@ struct pci_bus *pci_add_new_bus(struct p int busnr);struct pci_slot *pci_create_slot (struct pci_bus *parent,int slot_nr,const char *name); -int pci_destroy_slot (struct pci_slot *slot); +void pci_destroy_slot (struct pci_slot *slot);int pci_scan_slot (struct pci_bus *bus,int devfn);struct pci_dev *pci_scan_single_device (struct pci_bus *bus,int devfn);void pci_device_add (struct pci_dev *dev,struct pci_bus *bus);
Re: [PATCH 14/16] PCI slot: Change return value of pci_destroy_slot() (Not for mainline!)
Quoted 2 lines · Kenji Kaneshige
Since nobody checks the return value from
pci_destroy_slot(), it should be void.
ACK
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Re: [PATCH 14/16] PCI slot: Change return value of pci_destroy_slot() (Not for mainline!)
In reply to Kenji Kaneshige* Kenji Kaneshige <[email protected]>:
Quoted 2 lines
Since nobody checks the return value from
pci_destroy_slot(), it should be void.
Thanks, merged.
/ac
Quoted 45 lines
Signed-off-by Kenji Kaneshige <[email protected]>--- drivers/pci/slot.c | 4 +--- include/linux/pci.h | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-)Index:
linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================drivers/pci/slot.c +1
-3@@ -118,7 +118,7 @@ struct pci_slot *pci_create_slot(struct }EXPORT_SYMBOL_GPL (pci_create_slot); -int pci_destroy_slot (struct pci_slot *slot) +void pci_destroy_slot (struct pci_slot *slot) { pr_debug("%s: decreased refcount to %d on %x:%d\n", __func__, atomic_read(&slot->kobj.kref.refcount) - 1, slot->bus->number,@@ -127,8 +127,6 @@ int pci_destroy_slot(struct pci_slot *sldown_write (&pci_bus_sem);kobject_put (&slot->kobj);up_write (&pci_bus_sem); - -return 0 ; }EXPORT_SYMBOL_GPL (pci_destroy_slot);Index:
linux-2.6.25-rc6/include/linux/pci.h
===================================================================include/linux/pci.h +1
-1@@ -493,7 +493,7 @@ struct pci_bus *pci_add_new_bus(struct p int busnr);struct pci_slot *pci_create_slot (struct pci_bus *parent,int slot_nr,const char *name); -int pci_destroy_slot (struct pci_slot *slot); +void pci_destroy_slot (struct pci_slot *slot);int pci_scan_slot (struct pci_bus *bus,int devfn);struct pci_dev *pci_scan_single_device (struct pci_bus *bus,int devfn);void pci_device_add (struct pci_dev *dev,struct pci_bus *bus);
PATCH 15/16 PCI slot: Trivial cleanups for slot.c (Not for mainline!)
In reply to Kenji KaneshigeSome trivial cleanups for drivers/pci/slot.c.
--- drivers/pci/slot.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
drivers/pci/slot.c +3 -6
@@ -132,19 +132,16 @@ EXPORT_SYMBOL_GPL(pci_destroy_slot);static int pci_slot_init (void ) { -int result =0 ;struct kset *pci_bus_kset; pci_bus_kset =bus_get_kset (&pci_bus_type); - pci_slots_kset =kset_create_and_add ("slots" ,NULL , &pci_bus_kset->kobj);if (!pci_slots_kset) { - result = -ENOMEM ; -printk (KERN_ERR "PCI: Slot initialization failure (%d)\n" , - result); +printk (KERN_ERR "PCI: Slot initialization failure\n" ); +return -ENOMEM ; } -return result; +return 0 ; }subsys_initcall (pci_slot_init);
Re: [PATCH 15/16] PCI slot: Trivial cleanups for slot.c (Not for mainline!)
Quoted 1 line · Kenji Kaneshige
Some trivial cleanups for
drivers/pci/slot.c.
ACK
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Re: [PATCH 15/16] PCI slot: Trivial cleanups for slot.c (Not for mainline!)
In reply to Kenji Kaneshige* Kenji Kaneshige <[email protected]>:
Quoted 1 line
Some trivial cleanups for
drivers/pci/slot.c.
Thanks, merged.
/ac
Quoted 36 lines
Signed-off-by Kenji Kaneshige <[email protected]>--- drivers/pci/slot.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)Index:
linux-2.6.25-rc6/drivers/pci/slot.c
===================================================================drivers/pci/slot.c +3
-6@@ -132,19 +132,16 @@ EXPORT_SYMBOL_GPL(pci_destroy_slot);static int pci_slot_init (void ) { -int result =0 ;struct kset *pci_bus_kset; pci_bus_kset =bus_get_kset (&pci_bus_type); - pci_slots_kset =kset_create_and_add ("slots" ,NULL , &pci_bus_kset->kobj);if (!pci_slots_kset) { - result = -ENOMEM ; -printk (KERN_ERR "PCI: Slot initialization failure (%d)\n" , - result); +printk (KERN_ERR "PCI: Slot initialization failure\n" ); +return -ENOMEM ; } -return result; +return 0 ; }subsys_initcall (pci_slot_init);
PATCH 16/16 PCI hotplug core: add missing lock for hotplug slot list (Not for mainline!)
In reply to Kenji KaneshigeAdd missing lock for manipulating pci_hotplug_slot_list.
--- drivers/pci/hotplug/pci_hotplug_core.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
drivers/pci/hotplug/pci_hotplug_core.c +11 -2
@@ -61,6 +61,7 @@ static int debug;////////////////////////////////////////////////////////////////// static LIST_HEAD (pci_hotplug_slot_list); +static DEFINE_SPINLOCK (pci_hotplug_slot_list_lock);/* these strings match up with the values in pci_bus_speed */ static char *pci_bus_speed_strings[] = {
@@ -529,12 +530,16 @@ static struct hotplug_slot *get_slot_frostruct hotplug_slot *slot;struct list_head *tmp; +spin_lock (&pci_hotplug_slot_list_lock); list_for_each (tmp, &pci_hotplug_slot_list) { slot = list_entry (tmp, struct hotplug_slot, slot_list);if (strcmp (slot->name, name) ==0 ) -return slot; + goto out; } -return NULL ; + slot =NULL ; +out: +spin_unlock (&pci_hotplug_slot_list_lock); +return slot; }/**
@@ -584,7 +589,9 @@ int pci_hp_register(struct hotplug_slot } } +spin_lock (&pci_hotplug_slot_list_lock);list_add (&slot->slot_list, &pci_hotplug_slot_list); +spin_unlock (&pci_hotplug_slot_list_lock); result =fs_add_slot (pci_slot);kobject_uevent (&pci_slot->kobj,KOBJ_ADD );
@@ -613,7 +620,9 @@ int pci_hp_deregister(struct hotplug_sloif (temp != hotplug)return -ENODEV ; +spin_lock (&pci_hotplug_slot_list_lock);list_del (&hotplug->slot_list); +spin_unlock (&pci_hotplug_slot_list_lock); slot = hotplug->pci_slot;fs_remove_slot (slot);
Re: [PATCH 16/16][BUG] PCI hotplug core: add missing lock for hotplug slot list (Not for mainline!)
* Kenji Kaneshige <[email protected]>:
Quoted 1 line
Add missing lock for manipulating
pci_hotplug_slot_list.
Nice work, thanks. I've merged it.
/ac
Quoted 60 lines
Signed-off-by Kenji Kaneshige <[email protected]>--- drivers/pci/hotplug/pci_hotplug_core.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)Index:
linux-2.6.25-rc6/drivers/pci/hotplug/pci_hotplug_core.c
===================================================================drivers/pci/hotplug/pci_hotplug_core.c +11
-2@@ -61,6 +61,7 @@ static int debug;////////////////////////////////////////////////////////////////// static LIST_HEAD (pci_hotplug_slot_list); +static DEFINE_SPINLOCK (pci_hotplug_slot_list_lock);/* these strings match up with the values in pci_bus_speed */ static char *pci_bus_speed_strings[] = {@@ -529,12 +530,16 @@ static struct hotplug_slot *get_slot_frostruct hotplug_slot *slot;struct list_head *tmp; +spin_lock (&pci_hotplug_slot_list_lock); list_for_each (tmp, &pci_hotplug_slot_list) { slot = list_entry (tmp, struct hotplug_slot, slot_list);if (strcmp (slot->name, name) ==0 ) -return slot; + goto out; } -return NULL ; + slot =NULL ; +out: +spin_unlock (&pci_hotplug_slot_list_lock); +return slot; }/** @@ -584,7 +589,9 @@ int pci_hp_register(struct hotplug_slot } } +spin_lock (&pci_hotplug_slot_list_lock);list_add (&slot->slot_list, &pci_hotplug_slot_list); +spin_unlock (&pci_hotplug_slot_list_lock); result =fs_add_slot (pci_slot);kobject_uevent (&pci_slot->kobj,KOBJ_ADD );@@ -613,7 +620,9 @@ int pci_hp_deregister(struct hotplug_sloif (temp != hotplug)return -ENODEV ; +spin_lock (&pci_hotplug_slot_list_lock);list_del (&hotplug->slot_list); +spin_unlock (&pci_hotplug_slot_list_lock); slot = hotplug->pci_slot;fs_remove_slot (slot);
Hi Kenji-san,
Quoted 5 lines
I've reviewed and tested the latest version of Alex's "introduce
pci_slot" seriesBut on the other hand, I found some bugs and some codes need to be changed.
Thank you very much for the review and patches!
I will get started on reviewing / merging / testing these as soon as I can, and will respond to each patch individually.
Thanks!
/ac