Change in coreboot[master]: nb/amd/pi/00730F01/northbridge.c: Use generic allocation functions
Attention is currently required from: Krystian Hebel. Hello Krystian Hebel,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/52926
to review the following change.
Change subject: nb/amd/pi/00730F01/northbridge.c: Use generic allocation functions ......................................................................
nb/amd/pi/00730F01/northbridge.c: Use generic allocation functions
Remove obsolete resource assigning functions. Use generic PCI and PCI domain resource allocation functions wherever possible.
TEST=boot Debian with Linux 4.14 on apu2 4GB ECC and apu3 2GB no ECC
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Signed-off-by: Krystian Hebel krystian.hebel@3mdeb.com Change-Id: I8dd5e40bce513c5ba7f1d42a06e7ab0846666942 --- M src/northbridge/amd/pi/00730F01/northbridge.c 1 file changed, 10 insertions(+), 150 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/52926/1
diff --git a/src/northbridge/amd/pi/00730F01/northbridge.c b/src/northbridge/amd/pi/00730F01/northbridge.c index e1ff95a..91b1278 100644 --- a/src/northbridge/amd/pi/00730F01/northbridge.c +++ b/src/northbridge/amd/pi/00730F01/northbridge.c @@ -37,33 +37,6 @@ static struct device *__f4_dev[MAX_NODE_NUMS]; static unsigned int fx_devs = 0;
-static void set_io_addr_reg(struct device *dev, u32 nodeid, u32 linkn, u32 reg, - u32 io_min, u32 io_max) -{ - u32 i; - u32 tempreg; - /* io range allocation */ - tempreg = (nodeid&0xf) | ((nodeid & 0x30)<<(8-4)) | (linkn<<4) | ((io_max&0xf0)<<(12-4)); //limit - for (i = 0; i < node_nums; i++) - pci_write_config32(__f1_dev[i], reg+4, tempreg); - tempreg = 3 /*| (3<<4)*/ | ((io_min&0xf0)<<(12-4)); //base :ISA and VGA ? - for (i = 0; i < node_nums; i++) - pci_write_config32(__f1_dev[i], reg, tempreg); -} - -static void set_mmio_addr_reg(u32 nodeid, u32 linkn, u32 reg, u32 index, u32 mmio_min, u32 mmio_max, u32 nodes) -{ - u32 i; - u32 tempreg; - /* io range allocation */ - tempreg = (nodeid&0xf) | (linkn<<4) | (mmio_max&0xffffff00); //limit - for (i = 0; i < nodes; i++) - pci_write_config32(__f1_dev[i], reg+4, tempreg); - tempreg = 3 | (nodeid & 0x30) | (mmio_min&0xffffff00); - for (i = 0; i < node_nums; i++) - pci_write_config32(__f1_dev[i], reg, tempreg); -} - static struct device *get_node_pci(u32 nodeid, u32 fn) { return pcidev_on_root(DEV_CDB + nodeid, fn); @@ -86,13 +59,6 @@ printk(BIOS_DEBUG, "fx_devs = 0x%x\n", fx_devs); }
-static u32 f1_read_config32(unsigned int reg) -{ - if (fx_devs == 0) - get_fx_devs(); - return pci_read_config32(__f1_dev[0], reg); -} - static void f1_write_config32(unsigned int reg, u32 value) { int i; @@ -282,7 +248,7 @@
}
-static void read_resources(struct device *dev) +static void nb_read_resources(struct device *dev) { u32 nodeid; struct bus *link; @@ -309,57 +275,6 @@ res->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; }
-static void set_resource(struct device *dev, struct resource *resource, u32 nodeid) -{ - resource_t rbase, rend; - unsigned int reg, link_num; - char buf[50]; - - /* Make certain the resource has actually been set */ - if (!(resource->flags & IORESOURCE_ASSIGNED)) { - return; - } - - /* If I have already stored this resource don't worry about it */ - if (resource->flags & IORESOURCE_STORED) { - return; - } - - /* Only handle PCI memory and IO resources */ - if (!(resource->flags & (IORESOURCE_MEM | IORESOURCE_IO))) - return; - - /* Ensure I am actually looking at a resource of function 1 */ - if ((resource->index & 0xffff) < 0x1000) { - return; - } - /* Get the base address */ - rbase = resource->base; - - /* Get the limit (rounded up) */ - rend = resource_end(resource); - - /* Get the register and link */ - reg = resource->index & 0xfff; // 4k - link_num = IOINDEX_LINK(resource->index); - - if (resource->flags & IORESOURCE_IO) { - set_io_addr_reg(dev, nodeid, link_num, reg, rbase>>8, rend>>8); - } - else if (resource->flags & IORESOURCE_MEM) { - set_mmio_addr_reg(nodeid, link_num, reg, (resource->index >>24), rbase>>8, rend>>8, node_nums); // [39:8] - } - resource->flags |= IORESOURCE_STORED; - snprintf(buf, sizeof(buf), " <node %x link %x>", - nodeid, link_num); - report_resource_stored(dev, resource, buf); -} - -/** - * I tried to reuse the resource allocation code in set_resource() - * but it is too difficult to deal with the resource allocation magic. - */ - static void create_vga_resource(struct device *dev, unsigned int nodeid) { struct bus *link; @@ -388,27 +303,16 @@ set_vga_enable_reg(nodeid, sblink); }
-static void set_resources(struct device *dev) +static void nb_set_resources(struct device *dev) { unsigned int nodeid; - struct bus *bus; - struct resource *res;
/* Find the nodeid */ nodeid = amdfam16_nodeid(dev);
create_vga_resource(dev, nodeid); //TODO: do we need this?
- /* Set each resource we have found */ - for (res = dev->resource_list; res; res = res->next) { - set_resource(dev, res, nodeid); - } - - for (bus = dev->link_list; bus; bus = bus->next) { - if (bus->children) { - assign_resources(bus); - } - } + pci_dev_set_resources(dev); }
static void northbridge_init(struct device *dev) @@ -850,10 +754,11 @@ }
static struct device_operations northbridge_operations = { - .read_resources = read_resources, - .set_resources = set_resources, + .read_resources = nb_read_resources, + .set_resources = nb_set_resources, .enable_resources = pci_dev_enable_resources, .init = northbridge_init, + .ops_pci = &pci_dev_ops_pci, .acpi_fill_ssdt = northbridge_fill_ssdt_generator, .write_acpi_tables = agesa_write_acpi_tables, }; @@ -922,46 +827,6 @@ .final = fam16_finalize, };
-static void domain_read_resources(struct device *dev) -{ - unsigned int reg; - - /* Find the already assigned resource pairs */ - get_fx_devs(); - for (reg = 0x80; reg <= 0xd8; reg+= 0x08) { - u32 base, limit; - base = f1_read_config32(reg); - limit = f1_read_config32(reg + 0x04); - /* Is this register allocated? */ - if ((base & 3) != 0) { - unsigned int nodeid, reg_link; - struct device *reg_dev; - if (reg < 0xc0) { // mmio - nodeid = (limit & 0xf) + (base&0x30); - } else { // io - nodeid = (limit & 0xf) + ((base>>4)&0x30); - } - reg_link = (limit >> 4) & 7; - reg_dev = __f0_dev[nodeid]; - if (reg_dev) { - /* Reserve the resource */ - struct resource *res; - res = new_resource(reg_dev, IOINDEX(0x1000 + reg, reg_link)); - if (res) { - res->flags = 1; - } - } - } - } - /* FIXME: do we need to check extend conf space? - I don't believe that much preset value */ - pci_domain_read_resources(dev); -} - -static void domain_enable_resources(struct device *dev) -{ -} - #if CONFIG_HW_MEM_HOLE_SIZEK != 0 struct hw_mem_hole_info { unsigned int hole_startk; @@ -1008,7 +873,7 @@ } #endif
-static void domain_set_resources(struct device *dev) +static void domain_read_resources(struct device *dev) { unsigned long mmio_basek; u32 pci_tolm; @@ -1018,6 +883,8 @@ struct hw_mem_hole_info mem_hole; #endif
+ pci_domain_read_resources(dev); + pci_tolm = 0xffffffffUL; for (link = dev->link_list; link; link = link->next) { pci_tolm = find_pci_tolm(link); @@ -1098,12 +965,6 @@ }
add_uma_resource_below_tolm(dev, 7); - - for (link = dev->link_list; link; link = link->next) { - if (link->children) { - assign_resources(link); - } - } }
static const char *domain_acpi_name(const struct device *dev) @@ -1116,8 +977,7 @@
static struct device_operations pci_domain_ops = { .read_resources = domain_read_resources, - .set_resources = domain_set_resources, - .enable_resources = domain_enable_resources, + .set_resources = pci_domain_set_resources, .scan_bus = pci_domain_scan_bus, .acpi_name = domain_acpi_name, };
-
Michał Żygowski (Code Review)