Patchwork [3/3] PCI, acpiphp: Use res->flags to determine whether the resouce is valid

login
register
mail settings
Submitter Yijing Wang
Date Aug. 16, 2012, 12:12 p.m.
Message ID <1345119142-5896-3-git-send-email-wangyijing@huawei.com>
Download mbox | patch
Permalink /patch/177970/
State Rejected
Headers show

Comments

Yijing Wang - Aug. 16, 2012, 12:12 p.m.
When we hot plug pci devices, system will allocate resources to these new add
devices, pci_bus_assign_resources() will be called.If the pci devices was assigned
resource fail, the resource struct will reset to zero.So I think use res->flags here
to determine whether the resource is valid is reliable.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/hotplug/acpiphp_glue.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)
Yijing Wang - Aug. 17, 2012, 12:58 a.m.
Hi Bjorn,
   Please ignore these three patches, I'm very sorry I still have some confusion about allocating resources
to pci devices.So this patch is not appropriate. I will provide new version patches when I find a better
solution for this problem.

> When we hot plug pci devices, system will allocate resources to these new add
> devices, pci_bus_assign_resources() will be called.If the pci devices was assigned
> resource fail, the resource struct will reset to zero.So I think use res->flags here
> to determine whether the resource is valid is reliable.
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
>  drivers/pci/hotplug/acpiphp_glue.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index 7bbd6bf..2161902 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -1084,13 +1084,11 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus)
>  {
>  	struct pci_dev *dev, *tmp;
>  	int i;
> -	unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM;
>  
>  	list_for_each_entry_safe(dev, tmp, &bus->devices, bus_list) {
>  		for (i=0; i<PCI_BRIDGE_RESOURCES; i++) {
>  			struct resource *res = &dev->resource[i];
> -			if ((res->flags & type_mask) && !res->start &&
> -					res->end) {
> +			if (!res->flags) {
>  				/* Could not assign a required resources
>  				 * for this device, remove it */
>  				pci_stop_and_remove_bus_device(dev);
>
Bjorn Helgaas - Aug. 17, 2012, 2:34 a.m.
On Thu, Aug 16, 2012 at 6:58 PM, Yijing Wang <wangyijing@huawei.com> wrote:
> Hi Bjorn,
>    Please ignore these three patches, I'm very sorry I still have some confusion about allocating resources
> to pci devices.So this patch is not appropriate. I will provide new version patches when I find a better
> solution for this problem.

We don't have a uniform way of detecting unassigned BARs and handling
resource allocation failures yet.  When we make that all consistent, I
think it will make it more obvious how to do these patches.

I think patch 2/3 (the list_for_each_safe() one) is probably still a
good bug fix, so with your permission, I'll keep that one.

>> When we hot plug pci devices, system will allocate resources to these new add
>> devices, pci_bus_assign_resources() will be called.If the pci devices was assigned
>> resource fail, the resource struct will reset to zero.So I think use res->flags here
>> to determine whether the resource is valid is reliable.
>>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>> ---
>>  drivers/pci/hotplug/acpiphp_glue.c |    4 +---
>>  1 files changed, 1 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
>> index 7bbd6bf..2161902 100644
>> --- a/drivers/pci/hotplug/acpiphp_glue.c
>> +++ b/drivers/pci/hotplug/acpiphp_glue.c
>> @@ -1084,13 +1084,11 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus)
>>  {
>>       struct pci_dev *dev, *tmp;
>>       int i;
>> -     unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM;
>>
>>       list_for_each_entry_safe(dev, tmp, &bus->devices, bus_list) {
>>               for (i=0; i<PCI_BRIDGE_RESOURCES; i++) {
>>                       struct resource *res = &dev->resource[i];
>> -                     if ((res->flags & type_mask) && !res->start &&
>> -                                     res->end) {
>> +                     if (!res->flags) {
>>                               /* Could not assign a required resources
>>                                * for this device, remove it */
>>                               pci_stop_and_remove_bus_device(dev);
>>
>
>
> --
> Thanks!
> Yijing
>
--
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
Yijing Wang - Aug. 17, 2012, 4:59 a.m.
On 2012/8/17 10:34, Bjorn Helgaas wrote:
> On Thu, Aug 16, 2012 at 6:58 PM, Yijing Wang <wangyijing@huawei.com> wrote:
>> Hi Bjorn,
>>    Please ignore these three patches, I'm very sorry I still have some confusion about allocating resources
>> to pci devices.So this patch is not appropriate. I will provide new version patches when I find a better
>> solution for this problem.
> 
> We don't have a uniform way of detecting unassigned BARs and handling
> resource allocation failures yet.  When we make that all consistent, I
> think it will make it more obvious how to do these patches.
Yes, I also think maybe we need one uniform way to detecting unassigned BARs and resources
that be assigned failure.Then if necessary, some codes which want to correct or re-assign resoruces
for pci dev could do that more clearly. I will try to analyse that, and propose some patches as soon.
Thanks!
> I think patch 2/3 (the list_for_each_safe() one) is probably still a
> good bug fix, so with your permission, I'll keep that one.
Sure,that's ok.
>>> When we hot plug pci devices, system will allocate resources to these new add
>>> devices, pci_bus_assign_resources() will be called.If the pci devices was assigned
>>> resource fail, the resource struct will reset to zero.So I think use res->flags here
>>> to determine whether the resource is valid is reliable.

Patch

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 7bbd6bf..2161902 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -1084,13 +1084,11 @@  static void acpiphp_sanitize_bus(struct pci_bus *bus)
 {
 	struct pci_dev *dev, *tmp;
 	int i;
-	unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM;
 
 	list_for_each_entry_safe(dev, tmp, &bus->devices, bus_list) {
 		for (i=0; i<PCI_BRIDGE_RESOURCES; i++) {
 			struct resource *res = &dev->resource[i];
-			if ((res->flags & type_mask) && !res->start &&
-					res->end) {
+			if (!res->flags) {
 				/* Could not assign a required resources
 				 * for this device, remove it */
 				pci_stop_and_remove_bus_device(dev);