On Tue, Sep 13, 2011 at 10:55 PM, Amos Kong <akong@xxxxxxxxxx> 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.
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.
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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
