Kenji Kaneshige wrote:
> Yinghai Lu wrote:
>> Kenji Kaneshige wrote:
>>> Yinghai Lu wrote:
>>>> move out bus_size_bridges and assign resources out of
>>>> pciehp_add_bridge()
>>>> and at last do them all together one time including slot bridge, to
>>>> avoid to
>>>> call assign resources several times, when there are several bridges
>>>> under the
>>>> slot bridge.
>>>>
>>>> need to introduce pci_bridge_assign_resources there.
>>>>
>>>> handle the case the slot bridge that doesn't get pre-allocated big
>>>> enough res
>>>> from FW.
>>>> for example pcie devices need 256M, but the bridge only get
>>>> preallocated 2M...
>>>>
>>>> pci_setup_bridge() will take extra check_enabled for the slot bridge,
>>>> otherwise
>>>> update res is not updated to bridge BAR. that is bridge is enabled
>>>> already for
>>>> port service.
>>>>
>>>> -v2: address Alex's concern about pci remove/rescan feature about
>>>> pci_setup_bridge changes.
>>>> -v3: Kenji pointed out that pci_config_slot need to be called before
>>>> pci_bus_add_devices()
>>>> -v4: move out pci_is_enabled checkout of pci_setup_bridge()
>>>>
>>>> Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>
>>>>
>>>> ---
>>>> drivers/pci/hotplug/pciehp_pci.c | 29 ++++++++++++---
>>>> drivers/pci/setup-bus.c | 73
>>>> ++++++++++++++++++++++++++++++++++++---
>>>> include/linux/pci.h | 1 3 files changed, 93
>>>> insertions(+), 10 deletions(-)
>>>>
>>>> Index: linux-2.6/drivers/pci/hotplug/pciehp_pci.c
>>>> ===================================================================
>>>> --- linux-2.6.orig/drivers/pci/hotplug/pciehp_pci.c
>>>> +++ linux-2.6/drivers/pci/hotplug/pciehp_pci.c
>>>> @@ -53,19 +53,18 @@ static int __ref pciehp_add_bridge(struc
>>>> busnr = pci_scan_bridge(parent, dev, busnr, pass);
>>>> if (!dev->subordinate)
>>>> return -1;
>>>> - pci_bus_size_bridges(dev->subordinate);
>>>> - pci_bus_assign_resources(parent);
>>>> - pci_enable_bridges(parent);
>>>> - pci_bus_add_devices(parent);
>>>> +
>>>> return 0;
>>>> }
>>>>
>>>> int pciehp_configure_device(struct slot *p_slot)
>>>> {
>>>> struct pci_dev *dev;
>>>> - struct pci_bus *parent = p_slot->ctrl->pcie->port->subordinate;
>>>> + struct pci_dev *bridge = p_slot->ctrl->pcie->port;
>>>> + struct pci_bus *parent = bridge->subordinate;
>>>> int num, fn;
>>>> struct controller *ctrl = p_slot->ctrl;
>>>> + int retval;
>>>>
>>>> dev = pci_get_slot(parent, PCI_DEVFN(0, 0));
>>>> if (dev) {
>>>> @@ -96,12 +95,30 @@ int pciehp_configure_device(struct slot
>>>> (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)) {
>>>> pciehp_add_bridge(dev);
>>>> }
>>>> + pci_dev_put(dev);
>>>> + }
>>>> +
>>>> + pci_bus_size_bridges(parent);
>>>> + pci_clear_master(bridge);
>>>> + pci_bridge_assign_resources(bridge);
>>>> + retval = pci_reenable_device(bridge);
>>>> + pci_set_master(bridge);
>>>> + pci_enable_bridges(parent);
>>>> +
>>>> + for (fn = 0; fn < 8; fn++) {
>>>> + dev = pci_get_slot(parent, PCI_DEVFN(0, fn));
>>>> + if (!dev)
>>>> + continue;
>>>> + if ((dev->class >> 16) == PCI_BASE_CLASS_DISPLAY) {
>>>> + pci_dev_put(dev);
>>>> + continue;
>>>> + }
>>>> pci_configure_slot(dev);
>>>> pci_dev_put(dev);
>>>> }
>>>>
>>>> - pci_bus_assign_resources(parent);
>>>> pci_bus_add_devices(parent);
>>>> +
>>>> return 0;
>>>> }
>>>>
>>>> Index: linux-2.6/drivers/pci/setup-bus.c
>>>> ===================================================================
>>>> --- linux-2.6.orig/drivers/pci/setup-bus.c
>>>> +++ linux-2.6/drivers/pci/setup-bus.c
>>>> @@ -27,6 +27,44 @@
>>>> #include <linux/slab.h>
>>>> #include "pci.h"
>>>>
>>>> +static void pdev_assign_resources_sorted(struct pci_dev *dev)
>>>> +{
>>>> + struct resource *res;
>>>> + struct resource_list head, *list, *tmp;
>>>> + int idx;
>>>> + u16 class = dev->class >> 8;
>>>> +
>>>> + head.next = NULL;
>>>> +
>>>> + /* Don't touch classless devices or host bridges or ioapics. */
>>>> + if (class == PCI_CLASS_NOT_DEFINED ||
>>>> + class == PCI_CLASS_BRIDGE_HOST)
>>>> + return;
>>>> +
>>>> + /* Don't touch ioapic devices already enabled by firmware */
>>>> + if (class == PCI_CLASS_SYSTEM_PIC) {
>>>> + u16 command;
>>>> + pci_read_config_word(dev, PCI_COMMAND, &command);
>>>> + if (command & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
>>>> + return;
>>>> + }
>>>> +
>>>> + pdev_sort_resources(dev, &head);
>>>> +
>>>> + for (list = head.next; list;) {
>>>> + res = list->res;
>>>> + idx = res - &list->dev->resource[0];
>>>> + if (pci_assign_resource(list->dev, idx)) {
>>>> + res->start = 0;
>>>> + res->end = 0;
>>>> + res->flags = 0;
>>>> + }
>>>> + tmp = list;
>>>> + list = list->next;
>>>> + kfree(tmp);
>>>> + }
>>>> +}
>>>> +
>>>> static void pbus_assign_resources_sorted(const struct pci_bus *bus)
>>>> {
>>>> struct pci_dev *dev;
>>>> @@ -144,9 +182,6 @@ static void pci_setup_bridge(struct pci_
>>>> u32 l, bu, lu, io_upper16;
>>>> int pref_mem64;
>>>>
>>>> - if (pci_is_enabled(bridge))
>>>> - return;
>>>> -
>>>> dev_info(&bridge->dev, "PCI bridge, secondary bus %04x:%02x\n",
>>>> pci_domain_nr(bus), bus->number);
>>>>
>>>> @@ -541,6 +576,35 @@ void __ref pci_bus_size_bridges(struct p
>>>> }
>>>> EXPORT_SYMBOL(pci_bus_size_bridges);
>>>>
>>>> +void __ref pci_bridge_assign_resources(const struct pci_dev *bridge)
>>>> +{
>>>> + struct pci_bus *b;
>>>> +
>>>> + pdev_assign_resources_sorted((struct pci_dev *)bridge);
>>>> +
>>>> + b = bridge->subordinate;
>>>> + if (!b)
>>>> + return;
>>>> +
>>>> + pci_bus_assign_resources(b);
>>>> +
>>>> + switch (bridge->class >> 8) {
>>>> + case PCI_CLASS_BRIDGE_PCI:
>>>> + pci_setup_bridge(b);
>>>> + break;
>>>> +
>>>> + case PCI_CLASS_BRIDGE_CARDBUS:
>>>> + pci_setup_cardbus(b);
>>>> + break;
>>>> +
>>>> + default:
>>>> + dev_info(&bridge->dev, "not setting up bridge for bus "
>>>> + "%04x:%02x\n", pci_domain_nr(b), b->number);
>>>> + break;
>>>> + }
>>>> +}
>>>> +EXPORT_SYMBOL(pci_bridge_assign_resources);
>>>> +
>>>> void __ref pci_bus_assign_resources(const struct pci_bus *bus)
>>>> {
>>>> struct pci_bus *b;
>>>> @@ -557,7 +621,8 @@ void __ref pci_bus_assign_resources(cons
>>>>
>>>> switch (dev->class >> 8) {
>>>> case PCI_CLASS_BRIDGE_PCI:
>>>> - pci_setup_bridge(b);
>>>> + if (!pci_is_enabled(dev))
>>>> + pci_setup_bridge(b);
>>>> break;
>>>>
>>>> case PCI_CLASS_BRIDGE_CARDBUS:
>>>> Index: linux-2.6/include/linux/pci.h
>>>> ===================================================================
>>>> --- linux-2.6.orig/include/linux/pci.h
>>>> +++ linux-2.6/include/linux/pci.h
>>>> @@ -756,6 +756,7 @@ ssize_t pci_write_vpd(struct pci_dev *de
>>>> int pci_vpd_truncate(struct pci_dev *dev, size_t size);
>>>>
>>>> /* Helper functions for low-level code
>>>> (drivers/pci/setup-[bus,res].c) */
>>>> +void pci_bridge_assign_resources(const struct pci_dev *bridge);
>>>> void pci_bus_assign_resources(const struct pci_bus *bus);
>>>> void pci_bus_size_bridges(struct pci_bus *bus);
>>>> int pci_claim_resource(struct pci_dev *, int);
>>>>
>>> Does this patch work without [PATCH 2/2]? I don't understand who
>>> releases resouces? Does find_free_bus_resource() still release
>>> resources?
>>
>> need to work with [2/2].
>>
>
> Ok. Could you rearrange the set of patches with the right pieces and
> with the right order? It's very difficult for me to understand and
> review the current patches.
ok
>
> By the way, is release_resource() in find_free_bus_resource() already
> removed?
YES. Jesse sent request to Linus to revert that.
YH
--
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
