Re: [PATCH 1/3] PCI,acpiphp: use generic pci_hp_add_bridge()
> On Mon, Apr 15, 2013 at 6:49 PM, Yijing Wang wrote: >> Hi Yinghai, >>Any comments about this patch? I searched the code history and found >> you introduced pci_hp_add_bridge() function at commit a8e4b9c10. In your >> patchset use pci_hp_add_bridge() for all pci hotplug drivers except acpiphp. >> So I use pci_hp_add_bridge() in acpiphp instead of using pci_scan_bridge() >> directly. I would appreciate if you can give some suggestions. Hi Yinghai, Thanks for your review and comments very much! > > There is one problem in your patch. > max start value get change. before your patch max is the maximum value > of bus all children... >now it is change bus start value. Thank you for reminding. Now I noticed this detail.(: I will drop these pathes. Thanks! Yijing. > > looks like old code try to handle the case that we already some > children on that bus. > > Last time i did not change it, because i think it only is ready after > busn_res alloc patchset > is merged. > > in pci-mini summit of last year, Bjorn asked Linus about that > patchset, and Linus said it is > risky and it is not worthy to merge it . > > so assume we should hold your patch as it could cause some regression. > > Thanks > > Yinghai > > . > -- Thanks! Yijing -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] PCI,acpiphp: use generic pci_hp_add_bridge()
On Mon, Apr 15, 2013 at 6:49 PM, Yijing Wang wangyij...@huawei.com wrote: Hi Yinghai, Any comments about this patch? I searched the code history and found you introduced pci_hp_add_bridge() function at commit a8e4b9c10. In your patchset use pci_hp_add_bridge() for all pci hotplug drivers except acpiphp. So I use pci_hp_add_bridge() in acpiphp instead of using pci_scan_bridge() directly. I would appreciate if you can give some suggestions. Hi Yinghai, Thanks for your review and comments very much! There is one problem in your patch. max start value get change. before your patch max is the maximum value of bus all children... now it is change bus start value. Thank you for reminding. Now I noticed this detail.(: I will drop these pathes. Thanks! Yijing. looks like old code try to handle the case that we already some children on that bus. Last time i did not change it, because i think it only is ready after busn_res alloc patchset is merged. in pci-mini summit of last year, Bjorn asked Linus about that patchset, and Linus said it is risky and it is not worthy to merge it . so assume we should hold your patch as it could cause some regression. Thanks Yinghai . -- Thanks! Yijing -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] PCI,acpiphp: use generic pci_hp_add_bridge()
>
> On 2013/4/1 17:22, Yijing Wang wrote:
>> Use pci_hp_add_bridge() like most other hotplug drivers
>> rather than call pci_scan_bridge() directly.
>>
>> Signed-off-by: Yijing Wang
>> ---
>> drivers/pci/hotplug/acpiphp_glue.c | 24 ++--
>> 1 files changed, 10 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/pci/hotplug/acpiphp_glue.c
>> b/drivers/pci/hotplug/acpiphp_glue.c
>> index 270fdba..7d7ed3f 100644
>> --- a/drivers/pci/hotplug/acpiphp_glue.c
>> +++ b/drivers/pci/hotplug/acpiphp_glue.c
>> @@ -811,7 +811,7 @@ static int __ref enable_device(struct acpiphp_slot *slot)
>> struct pci_bus *bus = slot->bridge->pci_bus;
>> struct acpiphp_func *func;
>> int retval = 0;
>> - int num, max, pass;
>> + int num;
>> acpi_status status;
>>
>> if (slot->flags & SLOT_ENABLED)
>> @@ -827,22 +827,18 @@ static int __ref enable_device(struct acpiphp_slot
>> *slot)
>> goto err_exit;
>> }
>>
>> - max = acpiphp_max_busnr(bus);
>> - for (pass = 0; pass < 2; pass++) {
>> - list_for_each_entry(dev, >devices, bus_list) {
>> - if (PCI_SLOT(dev->devfn) != slot->device)
>> + list_for_each_entry(dev, >devices, bus_list) {
>> + if (PCI_SLOT(dev->devfn) != slot->device)
>> continue;
>> - 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) {
>> - check_hotplug_bridge(slot, dev);
>> - pci_bus_size_bridges(dev->subordinate);
>> - }
>> - }
>> + if ((dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) ||
>> + (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS))
>> + pci_hp_add_bridge(dev);
>> + if (dev->subordinate) {
>> + check_hotplug_bridge(slot, dev);
>> + pci_bus_size_bridges(dev->subordinate);
>> }
>> }
On Mon, Apr 15, 2013 at 6:49 PM, Yijing Wang wrote:
> Hi Yinghai,
>Any comments about this patch? I searched the code history and found
> you introduced pci_hp_add_bridge() function at commit a8e4b9c10. In your
> patchset use pci_hp_add_bridge() for all pci hotplug drivers except acpiphp.
> So I use pci_hp_add_bridge() in acpiphp instead of using pci_scan_bridge()
> directly. I would appreciate if you can give some suggestions.
There is one problem in your patch.
max start value get change. before your patch max is the maximum value
of bus all children...
now it is change bus start value.
looks like old code try to handle the case that we already some
children on that bus.
Last time i did not change it, because i think it only is ready after
busn_res alloc patchset
is merged.
in pci-mini summit of last year, Bjorn asked Linus about that
patchset, and Linus said it is
risky and it is not worthy to merge it .
so assume we should hold your patch as it could cause some regression.
Thanks
Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] PCI,acpiphp: use generic pci_hp_add_bridge()
Hi Yinghai,
Any comments about this patch? I searched the code history and found
you introduced pci_hp_add_bridge() function at commit a8e4b9c10. In your
patchset use pci_hp_add_bridge() for all pci hotplug drivers except acpiphp.
So I use pci_hp_add_bridge() in acpiphp instead of using pci_scan_bridge()
directly. I would appreciate if you can give some suggestions.
Thanks!
Yijing.
On 2013/4/1 17:22, Yijing Wang wrote:
> Use pci_hp_add_bridge() like most other hotplug drivers
> rather than call pci_scan_bridge() directly.
>
> Signed-off-by: Yijing Wang
> ---
> drivers/pci/hotplug/acpiphp_glue.c | 24 ++--
> 1 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c
> b/drivers/pci/hotplug/acpiphp_glue.c
> index 270fdba..7d7ed3f 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -811,7 +811,7 @@ static int __ref enable_device(struct acpiphp_slot *slot)
> struct pci_bus *bus = slot->bridge->pci_bus;
> struct acpiphp_func *func;
> int retval = 0;
> - int num, max, pass;
> + int num;
> acpi_status status;
>
> if (slot->flags & SLOT_ENABLED)
> @@ -827,22 +827,18 @@ static int __ref enable_device(struct acpiphp_slot
> *slot)
> goto err_exit;
> }
>
> - max = acpiphp_max_busnr(bus);
> - for (pass = 0; pass < 2; pass++) {
> - list_for_each_entry(dev, >devices, bus_list) {
> - if (PCI_SLOT(dev->devfn) != slot->device)
> + list_for_each_entry(dev, >devices, bus_list) {
> + if (PCI_SLOT(dev->devfn) != slot->device)
> continue;
> - 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) {
> - check_hotplug_bridge(slot, dev);
> - pci_bus_size_bridges(dev->subordinate);
> - }
> - }
> + if ((dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) ||
> + (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS))
> + pci_hp_add_bridge(dev);
> + if (dev->subordinate) {
> + check_hotplug_bridge(slot, dev);
> + pci_bus_size_bridges(dev->subordinate);
> }
> }
> -
> +
> pci_bus_assign_resources(bus);
> acpiphp_sanitize_bus(bus);
> acpiphp_set_hpp_values(bus);
>
--
Thanks!
Yijing
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] PCI,acpiphp: use generic pci_hp_add_bridge()
Hi Yinghai,
Any comments about this patch? I searched the code history and found
you introduced pci_hp_add_bridge() function at commit a8e4b9c10. In your
patchset use pci_hp_add_bridge() for all pci hotplug drivers except acpiphp.
So I use pci_hp_add_bridge() in acpiphp instead of using pci_scan_bridge()
directly. I would appreciate if you can give some suggestions.
Thanks!
Yijing.
On 2013/4/1 17:22, Yijing Wang wrote:
Use pci_hp_add_bridge() like most other hotplug drivers
rather than call pci_scan_bridge() directly.
Signed-off-by: Yijing Wang wangyij...@huawei.com
---
drivers/pci/hotplug/acpiphp_glue.c | 24 ++--
1 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/pci/hotplug/acpiphp_glue.c
b/drivers/pci/hotplug/acpiphp_glue.c
index 270fdba..7d7ed3f 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -811,7 +811,7 @@ static int __ref enable_device(struct acpiphp_slot *slot)
struct pci_bus *bus = slot-bridge-pci_bus;
struct acpiphp_func *func;
int retval = 0;
- int num, max, pass;
+ int num;
acpi_status status;
if (slot-flags SLOT_ENABLED)
@@ -827,22 +827,18 @@ static int __ref enable_device(struct acpiphp_slot
*slot)
goto err_exit;
}
- max = acpiphp_max_busnr(bus);
- for (pass = 0; pass 2; pass++) {
- list_for_each_entry(dev, bus-devices, bus_list) {
- if (PCI_SLOT(dev-devfn) != slot-device)
+ list_for_each_entry(dev, bus-devices, bus_list) {
+ if (PCI_SLOT(dev-devfn) != slot-device)
continue;
- 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) {
- check_hotplug_bridge(slot, dev);
- pci_bus_size_bridges(dev-subordinate);
- }
- }
+ if ((dev-hdr_type == PCI_HEADER_TYPE_BRIDGE) ||
+ (dev-hdr_type == PCI_HEADER_TYPE_CARDBUS))
+ pci_hp_add_bridge(dev);
+ if (dev-subordinate) {
+ check_hotplug_bridge(slot, dev);
+ pci_bus_size_bridges(dev-subordinate);
}
}
-
+
pci_bus_assign_resources(bus);
acpiphp_sanitize_bus(bus);
acpiphp_set_hpp_values(bus);
--
Thanks!
Yijing
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] PCI,acpiphp: use generic pci_hp_add_bridge()
On 2013/4/1 17:22, Yijing Wang wrote:
Use pci_hp_add_bridge() like most other hotplug drivers
rather than call pci_scan_bridge() directly.
Signed-off-by: Yijing Wang wangyij...@huawei.com
---
drivers/pci/hotplug/acpiphp_glue.c | 24 ++--
1 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/pci/hotplug/acpiphp_glue.c
b/drivers/pci/hotplug/acpiphp_glue.c
index 270fdba..7d7ed3f 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -811,7 +811,7 @@ static int __ref enable_device(struct acpiphp_slot *slot)
struct pci_bus *bus = slot-bridge-pci_bus;
struct acpiphp_func *func;
int retval = 0;
- int num, max, pass;
+ int num;
acpi_status status;
if (slot-flags SLOT_ENABLED)
@@ -827,22 +827,18 @@ static int __ref enable_device(struct acpiphp_slot
*slot)
goto err_exit;
}
- max = acpiphp_max_busnr(bus);
- for (pass = 0; pass 2; pass++) {
- list_for_each_entry(dev, bus-devices, bus_list) {
- if (PCI_SLOT(dev-devfn) != slot-device)
+ list_for_each_entry(dev, bus-devices, bus_list) {
+ if (PCI_SLOT(dev-devfn) != slot-device)
continue;
- 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) {
- check_hotplug_bridge(slot, dev);
- pci_bus_size_bridges(dev-subordinate);
- }
- }
+ if ((dev-hdr_type == PCI_HEADER_TYPE_BRIDGE) ||
+ (dev-hdr_type == PCI_HEADER_TYPE_CARDBUS))
+ pci_hp_add_bridge(dev);
+ if (dev-subordinate) {
+ check_hotplug_bridge(slot, dev);
+ pci_bus_size_bridges(dev-subordinate);
}
}
On Mon, Apr 15, 2013 at 6:49 PM, Yijing Wang wangyij...@huawei.com wrote:
Hi Yinghai,
Any comments about this patch? I searched the code history and found
you introduced pci_hp_add_bridge() function at commit a8e4b9c10. In your
patchset use pci_hp_add_bridge() for all pci hotplug drivers except acpiphp.
So I use pci_hp_add_bridge() in acpiphp instead of using pci_scan_bridge()
directly. I would appreciate if you can give some suggestions.
There is one problem in your patch.
max start value get change. before your patch max is the maximum value
of bus all children...
now it is change bus start value.
looks like old code try to handle the case that we already some
children on that bus.
Last time i did not change it, because i think it only is ready after
busn_res alloc patchset
is merged.
in pci-mini summit of last year, Bjorn asked Linus about that
patchset, and Linus said it is
risky and it is not worthy to merge it .
so assume we should hold your patch as it could cause some regression.
Thanks
Yinghai
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
