Patchwork [v8,01/22] PCI, acpiphp: Add is_hotplug_bridge detection

login
register
mail settings
Submitter Yinghai Lu
Date Jan. 11, 2013, 10:40 p.m.
Message ID <1357944049-29620-2-git-send-email-yinghai@kernel.org>
Download mbox | patch
Permalink /patch/211446/
State Superseded
Headers show

Comments

Yinghai Lu - Jan. 11, 2013, 10:40 p.m.
When system support hotplug bridge with children hotplug slots, we need
to make sure that parent bridge get preallocated resource so later when
device is plugged into children slot, those children devices will get
resource allocated.

We do not meet this problem, because for pcie hotplug card, when acpiphp
is used, pci_scan_bridge will set that for us when detect hotplug bit in
slot cap.

Reported-and-tested-by: Jason Baron <jbaron@redhat.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Acked-by: Jason Baron <jbaron@redhat.com>
---
 drivers/pci/hotplug/acpiphp_glue.c |   27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)
Rafael J. Wysocki - Jan. 12, 2013, 9:35 p.m.
On Friday, January 11, 2013 02:40:28 PM Yinghai Lu wrote:
> When system support hotplug bridge with children hotplug slots, we need
> to make sure that parent bridge get preallocated resource so later when
> device is plugged into children slot, those children devices will get
> resource allocated.
> 
> We do not meet this problem, because for pcie hotplug card, when acpiphp
> is used, pci_scan_bridge will set that for us when detect hotplug bit in
> slot cap.
> 
> Reported-and-tested-by: Jason Baron <jbaron@redhat.com>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Acked-by: Jason Baron <jbaron@redhat.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  drivers/pci/hotplug/acpiphp_glue.c |   27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index 91b5ad8..1e5c5df 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -797,6 +797,29 @@ static void acpiphp_set_acpi_region(struct acpiphp_slot *slot)
>  	}
>  }
>  
> +static void check_hotplug_bridge(struct acpiphp_slot *slot, struct pci_dev *dev)
> +{
> +	struct acpiphp_func *func;
> +
> +	if (!dev->subordinate)
> +		return;
> +
> +	/* quirk, or pcie could set it already */
> +	if (dev->is_hotplug_bridge)
> +		return;
> +
> +	if (PCI_SLOT(dev->devfn) != slot->device)
> +		return;
> +
> +	list_for_each_entry(func, &slot->funcs, sibling) {
> +		if (PCI_FUNC(dev->devfn) == func->function) {
> +			/* check if this bridge has ejectable slots */
> +			if ((detect_ejectable_slots(func->handle) > 0))
> +				dev->is_hotplug_bridge = 1;
> +			break;
> +		}
> +	}
> +}
>  /**
>   * enable_device - enable, configure a slot
>   * @slot: slot to be enabled
> @@ -831,8 +854,10 @@ static int __ref enable_device(struct acpiphp_slot *slot)
>  			if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
>  			    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
>  				max = pci_scan_bridge(bus, dev, max, pass);
> -				if (pass && dev->subordinate)
> +				if (pass && dev->subordinate) {
> +					check_hotplug_bridge(slot, dev);
>  					pci_bus_size_bridges(dev->subordinate);
> +				}
>  			}
>  		}
>  	}
>
Yijing Wang - Jan. 15, 2013, 6:45 a.m.
On 2013/1/12 6:40, Yinghai Lu wrote:
> When system support hotplug bridge with children hotplug slots, we need
> to make sure that parent bridge get preallocated resource so later when
> device is plugged into children slot, those children devices will get
> resource allocated.
> 
> We do not meet this problem, because for pcie hotplug card, when acpiphp
> is used, pci_scan_bridge will set that for us when detect hotplug bit in
> slot cap.
> 
> Reported-and-tested-by: Jason Baron <jbaron@redhat.com>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Acked-by: Jason Baron <jbaron@redhat.com>
> ---
>  drivers/pci/hotplug/acpiphp_glue.c |   27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index 91b5ad8..1e5c5df 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -797,6 +797,29 @@ static void acpiphp_set_acpi_region(struct acpiphp_slot *slot)
>  	}
>  }
>  
> +static void check_hotplug_bridge(struct acpiphp_slot *slot, struct pci_dev *dev)
> +{
> +	struct acpiphp_func *func;
> +
> +	if (!dev->subordinate)
> +		return;
> +
> +	/* quirk, or pcie could set it already */
> +	if (dev->is_hotplug_bridge)
> +		return;
> +
> +	if (PCI_SLOT(dev->devfn) != slot->device)
> +		return;
> +
> +	list_for_each_entry(func, &slot->funcs, sibling) {
> +		if (PCI_FUNC(dev->devfn) == func->function) {
> +			/* check if this bridge has ejectable slots */
> +			if ((detect_ejectable_slots(func->handle) > 0))
> +				dev->is_hotplug_bridge = 1;
> +			break;

Hi Yinghai,
   Need to put the "break" in parentheses of "if"? If the first func which device number == dev->devfn has't ejectable slot,
don't check the rest funcs whether have ejectable slots?

> +		}
> +	}
> +}
>  /**
>   * enable_device - enable, configure a slot
>   * @slot: slot to be enabled
> @@ -831,8 +854,10 @@ static int __ref enable_device(struct acpiphp_slot *slot)
>  			if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
>  			    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
>  				max = pci_scan_bridge(bus, dev, max, pass);
> -				if (pass && dev->subordinate)
> +				if (pass && dev->subordinate) {
> +					check_hotplug_bridge(slot, dev);
>  					pci_bus_size_bridges(dev->subordinate);
> +				}
>  			}
>  		}
>  	}
>
Yinghai Lu - Jan. 15, 2013, 7:05 a.m.
On Mon, Jan 14, 2013 at 10:45 PM, Yijing Wang <wangyijing@huawei.com> wrote:
>> +     list_for_each_entry(func, &slot->funcs, sibling) {
>> +             if (PCI_FUNC(dev->devfn) == func->function) {
>> +                     /* check if this bridge has ejectable slots */
>> +                     if ((detect_ejectable_slots(func->handle) > 0))
>> +                             dev->is_hotplug_bridge = 1;
>> +                     break;
>
> Hi Yinghai,
>    Need to put the "break" in parentheses of "if"? If the first func which device number == dev->devfn has't ejectable slot,
> don't check the rest funcs whether have ejectable slots?

no, we should break when PCI_FUNC(dev->devfn) == func->function

that iteration is for find the func for dev.

Thanks

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 91b5ad8..1e5c5df 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -797,6 +797,29 @@  static void acpiphp_set_acpi_region(struct acpiphp_slot *slot)
 	}
 }
 
+static void check_hotplug_bridge(struct acpiphp_slot *slot, struct pci_dev *dev)
+{
+	struct acpiphp_func *func;
+
+	if (!dev->subordinate)
+		return;
+
+	/* quirk, or pcie could set it already */
+	if (dev->is_hotplug_bridge)
+		return;
+
+	if (PCI_SLOT(dev->devfn) != slot->device)
+		return;
+
+	list_for_each_entry(func, &slot->funcs, sibling) {
+		if (PCI_FUNC(dev->devfn) == func->function) {
+			/* check if this bridge has ejectable slots */
+			if ((detect_ejectable_slots(func->handle) > 0))
+				dev->is_hotplug_bridge = 1;
+			break;
+		}
+	}
+}
 /**
  * enable_device - enable, configure a slot
  * @slot: slot to be enabled
@@ -831,8 +854,10 @@  static int __ref enable_device(struct acpiphp_slot *slot)
 			if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
 			    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
 				max = pci_scan_bridge(bus, dev, max, pass);
-				if (pass && dev->subordinate)
+				if (pass && dev->subordinate) {
+					check_hotplug_bridge(slot, dev);
 					pci_bus_size_bridges(dev->subordinate);
+				}
 			}
 		}
 	}