| Message ID | 1439044022-14481-5-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
On Sat, Aug 8, 2015 at 10:26 PM, Simon Glass <sjg@chromium.org> wrote: > This code could use a little tightening up. There is some repetition and > an odd use of fdtdec_get_int_array(). > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > Changes in v2: > - Fix handling of duplicate entries > > arch/x86/cpu/irq.c | 41 +++++++++++++---------------------------- > 1 file changed, 13 insertions(+), 28 deletions(-) > > diff --git a/arch/x86/cpu/irq.c b/arch/x86/cpu/irq.c > index 97dd000..6be2f81 100644 > --- a/arch/x86/cpu/irq.c > +++ b/arch/x86/cpu/irq.c > @@ -125,10 +125,10 @@ static int create_pirq_routing_table(void) > return -EINVAL; > } > > - ret = fdtdec_get_int_array(blob, node, "intel,pirq-link", > - &irq_router.link_base, 1); > - if (ret) > + ret = fdtdec_get_int(blob, node, "intel,pirq-link", -1); > + if (ret == -1) > return ret; > + irq_router.link_base = ret; > > irq_router.irq_mask = fdtdec_get_int(blob, node, > "intel,pirq-mask", PIRQ_BITMAP); > @@ -156,18 +156,13 @@ static int create_pirq_routing_table(void) > } > > cell = fdt_getprop(blob, node, "intel,pirq-routing", &len); > - if (!cell) > - return -EINVAL; > - > - if ((len % sizeof(struct pirq_routing)) == 0) > - count = len / sizeof(struct pirq_routing); > - else > + if (!cell || len % sizeof(struct pirq_routing)) > return -EINVAL; > + count = len / sizeof(struct pirq_routing); > > - rt = malloc(sizeof(struct irq_routing_table)); > + rt = calloc(1, sizeof(struct irq_routing_table)); > if (!rt) > return -ENOMEM; > - memset((char *)rt, 0, sizeof(struct irq_routing_table)); > > /* Populate the PIRQ table fields */ > rt->signature = PIRQ_SIGNATURE; > @@ -181,7 +176,8 @@ static int create_pirq_routing_table(void) > slot_base = rt->slots; > > /* Now fill in the irq_info entries in the PIRQ table */ > - for (i = 0; i < count; i++) { > + for (i = 0; i < count; > + i++, cell += sizeof(struct pirq_routing) / sizeof(u32)) { > struct pirq_routing pr; > > pr.bdf = fdt_addr_to_cpu(cell[0]); > @@ -212,25 +208,14 @@ static int create_pirq_routing_table(void) > if (slot->irq[pr.pin - 1].link != > LINK_N2V(pr.pirq, irq_router.link_base)) > debug("WARNING: Inconsistent PIRQ routing information\n"); > - > - cell += sizeof(struct pirq_routing) / > - sizeof(u32); > - continue; > - } else { > - debug("writing INT%c\n", 'A' + pr.pin - 1); > - fill_irq_info(slot, PCI_BUS(pr.bdf), > - PCI_DEV(pr.bdf), pr.pin, pr.pirq); > - cell += sizeof(struct pirq_routing) / > - sizeof(u32); > continue; > } > + } else { > + slot = slot_base + irq_entries++; > } > - > - slot = slot_base + irq_entries; > - fill_irq_info(slot, PCI_BUS(pr.bdf), PCI_DEV(pr.bdf), > - pr.pin, pr.pirq); > - irq_entries++; > - cell += sizeof(struct pirq_routing) / sizeof(u32); > + debug("writing INT%c\n", 'A' + pr.pin - 1); > + fill_irq_info(slot, PCI_BUS(pr.bdf), PCI_DEV(pr.bdf), pr.pin, > + pr.pirq); > } > > rt->size = irq_entries * sizeof(struct irq_info) + 32; > -- Reviewed-by: Bin Meng <bmeng.cn@gmail.com> Tested-by: Bin Meng <bmeng.cn@gmail.com>
diff --git a/arch/x86/cpu/irq.c b/arch/x86/cpu/irq.c index 97dd000..6be2f81 100644 --- a/arch/x86/cpu/irq.c +++ b/arch/x86/cpu/irq.c @@ -125,10 +125,10 @@ static int create_pirq_routing_table(void) return -EINVAL; } - ret = fdtdec_get_int_array(blob, node, "intel,pirq-link", - &irq_router.link_base, 1); - if (ret) + ret = fdtdec_get_int(blob, node, "intel,pirq-link", -1); + if (ret == -1) return ret; + irq_router.link_base = ret; irq_router.irq_mask = fdtdec_get_int(blob, node, "intel,pirq-mask", PIRQ_BITMAP); @@ -156,18 +156,13 @@ static int create_pirq_routing_table(void) } cell = fdt_getprop(blob, node, "intel,pirq-routing", &len); - if (!cell) - return -EINVAL; - - if ((len % sizeof(struct pirq_routing)) == 0) - count = len / sizeof(struct pirq_routing); - else + if (!cell || len % sizeof(struct pirq_routing)) return -EINVAL; + count = len / sizeof(struct pirq_routing); - rt = malloc(sizeof(struct irq_routing_table)); + rt = calloc(1, sizeof(struct irq_routing_table)); if (!rt) return -ENOMEM; - memset((char *)rt, 0, sizeof(struct irq_routing_table)); /* Populate the PIRQ table fields */ rt->signature = PIRQ_SIGNATURE; @@ -181,7 +176,8 @@ static int create_pirq_routing_table(void) slot_base = rt->slots; /* Now fill in the irq_info entries in the PIRQ table */ - for (i = 0; i < count; i++) { + for (i = 0; i < count; + i++, cell += sizeof(struct pirq_routing) / sizeof(u32)) { struct pirq_routing pr; pr.bdf = fdt_addr_to_cpu(cell[0]); @@ -212,25 +208,14 @@ static int create_pirq_routing_table(void) if (slot->irq[pr.pin - 1].link != LINK_N2V(pr.pirq, irq_router.link_base)) debug("WARNING: Inconsistent PIRQ routing information\n"); - - cell += sizeof(struct pirq_routing) / - sizeof(u32); - continue; - } else { - debug("writing INT%c\n", 'A' + pr.pin - 1); - fill_irq_info(slot, PCI_BUS(pr.bdf), - PCI_DEV(pr.bdf), pr.pin, pr.pirq); - cell += sizeof(struct pirq_routing) / - sizeof(u32); continue; } + } else { + slot = slot_base + irq_entries++; } - - slot = slot_base + irq_entries; - fill_irq_info(slot, PCI_BUS(pr.bdf), PCI_DEV(pr.bdf), - pr.pin, pr.pirq); - irq_entries++; - cell += sizeof(struct pirq_routing) / sizeof(u32); + debug("writing INT%c\n", 'A' + pr.pin - 1); + fill_irq_info(slot, PCI_BUS(pr.bdf), PCI_DEV(pr.bdf), pr.pin, + pr.pirq); } rt->size = irq_entries * sizeof(struct irq_info) + 32;
This code could use a little tightening up. There is some repetition and an odd use of fdtdec_get_int_array(). Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v2: - Fix handling of duplicate entries arch/x86/cpu/irq.c | 41 +++++++++++++---------------------------- 1 file changed, 13 insertions(+), 28 deletions(-)