On Fri, Oct 28, 2011 at 10:50:30AM -0500, Kumar Gala wrote:
A few remarks below.
> +static void __init dpa_uio_portal_init(struct dpa_uio_portal *p,
> + const struct dpa_uio_class *c)
This can't be "void". You have to return apropiate errors.
> +{
> + struct dpa_uio_info *info;
> + const struct resource *res;
> + u32 index;
> + int irq, ret;
> +
> + /* allocate 'info' */
> + info = kzalloc(sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return;
return -ENOMEM (more similar cases below)
> + atomic_set(&info->ref, 1);
> + if (p->type == dpa_uio_portal_bman) {
> + res = &p->bm_cfg->addr_phys[0];
> + index = p->bm_cfg->public_cfg.index;
> + irq = p->bm_cfg->public_cfg.irq;
> + } else {
> + res = &p->qm_cfg->addr_phys[0];
> + index = p->qm_cfg->public_cfg.index;
> + irq = p->qm_cfg->public_cfg.irq;
> + }
> + /* We need to map the cache-inhibited region in the kernel for
> + * interrupt-handling purposes. */
> + info->addr_ci = ioremap_prot(res[DPA_PORTAL_CI].start,
> + resource_size(&res[DPA_PORTAL_CI]),
> + _PAGE_GUARDED | _PAGE_NO_CACHE);
> + /* Name the UIO device according to the cell-index. It's supposed to be
> + * unique for each device class (Qman/Bman), and is also a convenient
> + * way for user-space to find the UIO device that corresponds to a given
> + * portal device-tree node. */
> + sprintf(info->name, "%s%x", c->dev_prefix, index);
> + info->pdev = platform_device_alloc(info->name, -1);
> + if (!info->pdev) {
> + iounmap(info->addr_ci);
> + kfree(info);
> + pr_err("dpa_uio_portal: platform_device_alloc() failed\n");
> + return;
> + }
> + ret = platform_device_add(info->pdev);
> + if (ret) {
> + platform_device_put(info->pdev);
> + iounmap(info->addr_ci);
> + kfree(info);
> + pr_err("dpa_uio_portal: platform_device_add() failed\n");
> + return;
> + }
> + info->uio.name = info->name;
> + info->uio.version = dpa_uio_version;
> + info->uio.mem[DPA_PORTAL_CE].name = "cena";
> + info->uio.mem[DPA_PORTAL_CE].addr = res[DPA_PORTAL_CE].start;
> + info->uio.mem[DPA_PORTAL_CE].size = resource_size(&res[DPA_PORTAL_CE]);
> + info->uio.mem[DPA_PORTAL_CE].memtype = UIO_MEM_PHYS;
> + info->uio.mem[DPA_PORTAL_CI].name = "cinh";
> + info->uio.mem[DPA_PORTAL_CI].addr = res[DPA_PORTAL_CI].start;
> + info->uio.mem[DPA_PORTAL_CI].size = resource_size(&res[DPA_PORTAL_CI]);
> + info->uio.mem[DPA_PORTAL_CI].memtype = UIO_MEM_PHYS;
> + info->uio.irq = irq;
> + info->uio.handler = dpa_uio_irq_handler;
> + info->uio.set_pgprot = dpa_uio_pgprot;
> + info->uio.open = dpa_uio_open;
> + info->uio.release = dpa_uio_release;
> + ret = uio_register_device(&info->pdev->dev, &info->uio);
This should be the last thing you do before return. You can have
interrupts even before uio_register_device returns.
> + if (ret) {
> + platform_device_del(info->pdev);
> + platform_device_put(info->pdev);
> + iounmap(info->addr_ci);
> + kfree(info);
> + pr_err("dpa_uio_portal: UIO registration failed\n");
> + return;
> + }
> + list_add_tail(&info->node, &uio_portal_list);
That should probably be done prior to uio_register_device.
> + pr_info("USDPAA portal initialised, %s\n", info->name);
> +}
> +
> +static int __init dpa_uio_init(void)
> +{
> + const struct dpa_uio_class *classes[3], **c = classes;
> + classes[0] = dpa_uio_bman();
> + classes[1] = dpa_uio_qman();
> + classes[2] = NULL;
> + while (*c) {
> + struct dpa_uio_portal *p;
> + list_for_each_entry(p, &(*c)->list, node)
> + dpa_uio_portal_init(p, *c);
> + c++;
> + }
> + pr_info("USDPAA portal layer loaded\n");
> + return 0;
You can't just return OK here if dpa_uio_portal_init() fails.
> +}
> +
> +static void __exit dpa_uio_exit(void)
> +{
> + struct dpa_uio_info *info, *tmp;
> + list_for_each_entry_safe(info, tmp, &uio_portal_list, node) {
> + list_del(&info->node);
> + uio_unregister_device(&info->uio);
> + platform_device_del(info->pdev);
> + platform_device_put(info->pdev);
> + iounmap(info->addr_ci);
> + pr_info("USDPAA portal removed, %s\n", info->name);
> + kfree(info);
> + }
> + pr_info("USDPAA portal layer unloaded\n");
> +}
> +
> +
> +module_init(dpa_uio_init)
> +module_exit(dpa_uio_exit)
> +MODULE_LICENSE("GPL");
> +
> --
> 1.7.3.4
>
>
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
