[SeaBIOS] [PATCH] pci: clean all funcs when hot-removing multifunc device
Kenji Kaneshige
kaneshige.kenji at jp.fujitsu.com
Fri Sep 16 01:56:32 CEST 2011
(2011/09/16 4:03), Bjorn Helgaas wrote:
> On Tue, Sep 13, 2011 at 10:55 PM, Amos Kong<akong at redhat.com> wrote:
>> 'slot->funcs' is initialized in acpiphp_glue.c:register_slot() before
>> hotpluging device, and only one entry(func 0) is added to it,
>> no new entry will be added to the list when hotpluging devices to the slot.
>> When we release the whole device, there is only one entry in the list,
>> this causes func1~7 could not be released.
>> I try to add entries for all hotpluged device in enable_device(), but
>> it doesn't work, because 'slot->funcs' is used in many place which we only
>> need to process func 0. This patch just try to clean all funcs in
>> disable_device().
> ...
>> Hotpluging multifunc of WinXp is fine.
>
> I'm going to ignore this patch for now. Please consider these
> questions, then repost it if you still want it:
>
> I assume you mean that Linux and WinXP are both running on top of the
> same SeaBIOS, and hot-remove of a multifunction device works in WinXP
> and fails in Linux. That sounds like Linux is broken, and we should
> fix it. We might want to make a SeaBIOS change for other reasons, but
> it'd still be good to fix Linux in case there are other similar
> BIOSes.
No objection about fixing Linux.
>
> Why do we need pci_scan_single_device()? The device should have been
> scanned already when it was added, and I would think that should have
> set pdev->multifunction.
It should be pci_get_slot() instead. Note that it needs
corresponding pci_dev_put().
Regards,
Kenji Kaneshige
>
> Your patch needs spaces around the operators in the "for" loop.
>
> In the changelog, it would be nice to have the URL of a bugzilla where
> the dmesg and DSDT are attached.
>
> Bjorn
>
>> Signed-off-by: Amos Kong<akong at redhat.com>
>> ---
>> drivers/pci/hotplug/acpiphp_glue.c | 27 ++++++++++++++++++---------
>> 1 files changed, 18 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
>> index a70fa89..3b86d1a 100644
>> --- a/drivers/pci/hotplug/acpiphp_glue.c
>> +++ b/drivers/pci/hotplug/acpiphp_glue.c
>> @@ -880,6 +880,8 @@ static int disable_device(struct acpiphp_slot *slot)
>> {
>> struct acpiphp_func *func;
>> struct pci_dev *pdev;
>> + struct pci_bus *bus = slot->bridge->pci_bus;
>> + int i, num = 1;
>>
>> /* is this slot already disabled? */
>> if (!(slot->flags& SLOT_ENABLED))
>> @@ -893,16 +895,23 @@ static int disable_device(struct acpiphp_slot *slot)
>> func->bridge = NULL;
>> }
>>
>> - pdev = pci_get_slot(slot->bridge->pci_bus,
>> - PCI_DEVFN(slot->device, func->function));
>> - if (pdev) {
>> - pci_stop_bus_device(pdev);
>> - if (pdev->subordinate) {
>> - disable_bridges(pdev->subordinate);
>> - pci_disable_device(pdev);
>> + pdev = pci_scan_single_device(bus,
>> + PCI_DEVFN(slot->device, 0));
>> + if (!pdev)
>> + goto err_exit;
>> + if (pdev->multifunction == 1)
>> + num = 8;
>> + for (i=0; i<num; i++) {
>> + pdev = pci_get_slot(bus, PCI_DEVFN(slot->device, i));
>> + if (pdev) {
>> + pci_stop_bus_device(pdev);
>> + if (pdev->subordinate) {
>> + disable_bridges(pdev->subordinate);
>> + pci_disable_device(pdev);
>> + }
>> + pci_remove_bus_device(pdev);
>> + pci_dev_put(pdev);
>> }
>> - pci_remove_bus_device(pdev);
>> - pci_dev_put(pdev);
>> }
>> }
>>
>> --
>> 1.7.6.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
>
More information about the SeaBIOS
mailing list