Re: [PATCH v2 10/37] PCI, sysfs: create rescan_bridge under/sys/.../pci/devices/... for pci bridges

[Bjorn Helgaas]

On Sat, Mar 10, 2012 at 12:00 AM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
> Current code will create rescan for every pci device under parent bus.
> that is not right. the device is already there, there is no reason to rescan it.

"The device" (not "the device").

> We could have rescan for pci bridges. less confusing.
>
> Need to move rescan attr to pci dev bridge attribute group.
> And We should rescan bridge's secondary bus instead of primary bus.

"And we" (not "And We").

I think the idea of this patch makes sense.

You might want to consider having this actually read the bridge config
again.  That way, if the user manually changed the secondary bus
number or the bridge windows using setpci or something, this rescan
could use that new config if possible.

I still hate the fact that we will have three different rescan files
(four if you count /sys/bus/pci/rescan), but I don't have a solution
to offer:
    device/rescan
    device/rescan_bridge
    bus/rescan

> -v3: Use device_type for pci dev.
> -v4: Seperate pci device type change out
> -v5: add rescan_bridge for bridge type, and still keep the old rescan.
>        may remove the old rescan later.
>
> Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>
> Cc: Randy Dunlap <rdunlap@xxxxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
>  Documentation/ABI/testing/sysfs-bus-pci |   10 ++++++++++
>  drivers/pci/pci-sysfs.c                 |   24 ++++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
> index 34f5110..95f0f37 100644
> --- a/Documentation/ABI/testing/sysfs-bus-pci
> +++ b/Documentation/ABI/testing/sysfs-bus-pci
> @@ -111,6 +111,16 @@ Description:
>                from this part of the device tree.
>                Depends on CONFIG_HOTPLUG.
>
> +What:          /sys/bus/pci/devices/.../rescan_bridge
> +Date:          February 2012
> +Contact:       Linux PCI developers <linux-pci@xxxxxxxxxxxxxxx>
> +Description:
> +               Writing a non-zero value to this attribute will
> +               force a rescan of the bridge and all child buses, and
> +               re-discover devices removed earlier from this part of
> +               the device tree.
> +               Depends on CONFIG_HOTPLUG.
> +
>  What:          /sys/bus/pci/devices/.../reset
>  Date:          July 2009
>  Contact:       Michael S. Tsirkin <mst@xxxxxxxxxx>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 39d15e6..d5c8ffb 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -325,6 +325,27 @@ dev_rescan_store(struct device *dev, struct device_attribute *attr,
>        return count;
>  }
>
> +static ssize_t
> +dev_bridge_rescan_store(struct device *dev, struct device_attribute *attr,
> +                const char *buf, size_t count)
> +{
> +       unsigned long val;
> +       struct pci_dev *pdev = to_pci_dev(dev);
> +
> +       if (kstrtoul(buf, 0, &val) < 0)
> +               return -EINVAL;
> +
> +       if (val) {
> +               mutex_lock(&pci_remove_rescan_mutex);
> +               pci_rescan_bus(pdev->subordinate);
> +               mutex_unlock(&pci_remove_rescan_mutex);
> +       }
> +       return count;
> +}
> +
> +static struct device_attribute pci_dev_bridge_rescan_attr =
> +       __ATTR(rescan_bridge, (S_IWUSR|S_IWGRP), NULL, dev_bridge_rescan_store);
> +
>  static void remove_callback(struct device *dev)
>  {
>        struct pci_dev *pdev = to_pci_dev(dev);
> @@ -1337,6 +1358,9 @@ static int __init pci_sysfs_init(void)
>  late_initcall(pci_sysfs_init);
>
>  static struct attribute *pci_dev_bridge_attrs[] = {
> +#ifdef CONFIG_HOTPLUG
> +       &pci_dev_bridge_rescan_attr.attr,
> +#endif
>        NULL,
>  };
>
> --
> 1.7.7
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/