Messages in this thread | | | Date | Wed, 16 Jan 2013 18:57:54 +0200 | From | Felipe Balbi <> | Subject | Re: [PATCH 02/10] goldfish: add the goldfish virtual bus |
| |
Hi,
On Wed, Jan 16, 2013 at 04:58:51PM +0000, Alan Cox wrote: > +static irqreturn_t goldfish_pdev_bus_interrupt(int irq, void *dev_id) > +{ > + irqreturn_t ret = IRQ_NONE; > + while (1) { > + u32 op = readl(pdev_bus_base + PDEV_BUS_OP); > + switch (op) { > + case PDEV_BUS_OP_DONE: > + return IRQ_NONE; > + > + case PDEV_BUS_OP_REMOVE_DEV: > + goldfish_pdev_remove(); > + break; > + > + case PDEV_BUS_OP_ADD_DEV: > + goldfish_new_pdev(); > + break; > + } > + ret = IRQ_HANDLED; > + }
missing a return ret here ?
> +} > + > +static int goldfish_pdev_bus_probe(struct platform_device *pdev) > +{ > + int ret; > + struct resource *r; > + > + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (r == NULL) > + return -EINVAL; > + > + pdev_bus_addr = r->start; > + pdev_bus_len = resource_size(r); > + > + if (request_region(pdev_bus_addr, pdev_bus_len, "goldfish")) { > + pr_err("unable to reserve Goldfish MMIO.\n"); > + return -EBUSY; > + } > + > + pdev_bus_base = ioremap(pdev_bus_addr, pdev_bus_len);
how about converting the two above to devm_request_and_ioremap() ?
> + if (pdev_bus_base == NULL) { > + ret = -ENOMEM; > + pr_err("unable to map Goldfish MMIO.\n"); > + goto free_resources; > + } > + > + r = platform_get_resource(pdev, IORESOURCE_IRQ, 0); > + if (r == NULL) { > + ret = -ENOENT; > + goto free_map; > + } > + > + pdev_bus_irq = r->start; > + > + ret = request_irq(pdev_bus_irq, goldfish_pdev_bus_interrupt,
and here to devm_request_irq() ...
> + IRQF_SHARED, "goldfish_pdev_bus", pdev); > + if (ret) > + goto free_map; > + > + writel(PDEV_BUS_OP_INIT, pdev_bus_base + PDEV_BUS_OP); > + return 0; > + > +free_map: > + iounmap(pdev_bus_base); > +free_resources: > + release_region(pdev_bus_addr, pdev_bus_len); > + return ret; > +} > + > +static int goldfish_pdev_bus_remove(struct platform_device *pdev) > +{ > + iounmap(pdev_bus_base); > + free_irq(pdev_bus_irq, pdev); > + release_region(pdev_bus_addr, pdev_bus_len);
your remove would, then, be an empty function.
-- balbi [unhandled content-type:application/pgp-signature] | |