search for: devm_release_mem_region

this patchset try to add cleanup for virtio_mmio driver, include virtio_mmio_probe and virtio_mmio_remove weiping zhang (2): virtio_mmio: add cleanup for virtio_mmio_probe virtio_mmio: add cleanup for virtio_mmio_remove drivers/virtio/virtio_mmio.c | 57 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 10 deletions(-) -- 2.9.4

this patchset try to add cleanup for virtio_mmio driver, include virtio_mmio_probe and virtio_mmio_remove weiping zhang (2): virtio_mmio: add cleanup for virtio_mmio_probe virtio_mmio: add cleanup for virtio_mmio_remove drivers/virtio/virtio_mmio.c | 57 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 10 deletions(-) -- 2.9.4

...} vm_dev->vdev.id.vendor = readl(vm_dev->base + VIRTIO_MMIO_VENDOR_ID); @@ -590,33 +583,15 @@ static int virtio_mmio_probe(struct platform_device *pdev) platform_set_drvdata(pdev, vm_dev); rc = register_virtio_device(&vm_dev->vdev); - if (rc) { - iounmap(vm_dev->base); - devm_release_mem_region(&pdev->dev, mem->start, - resource_size(mem)); + if (rc) put_device(&vm_dev->vdev.dev); - } - return rc; -unmap: - iounmap(vm_dev->base); -free_mem: - devm_release_mem_region(&pdev->dev, mem->start, - resource_size(mem)); -free_vmdev: - devm_kfree(&pdev-&g...

...} vm_dev->vdev.id.vendor = readl(vm_dev->base + VIRTIO_MMIO_VENDOR_ID); @@ -590,33 +583,15 @@ static int virtio_mmio_probe(struct platform_device *pdev) platform_set_drvdata(pdev, vm_dev); rc = register_virtio_device(&vm_dev->vdev); - if (rc) { - iounmap(vm_dev->base); - devm_release_mem_region(&pdev->dev, mem->start, - resource_size(mem)); + if (rc) put_device(&vm_dev->vdev.dev); - } - return rc; -unmap: - iounmap(vm_dev->base); -free_mem: - devm_release_mem_region(&pdev->dev, mem->start, - resource_size(mem)); -free_vmdev: - devm_kfree(&pdev-&g...

...> > @@ -590,33 +583,15 @@ static int virtio_mmio_probe(struct platform_device *pdev) > platform_set_drvdata(pdev, vm_dev); > > rc = register_virtio_device(&vm_dev->vdev); > - if (rc) { > - iounmap(vm_dev->base); > - devm_release_mem_region(&pdev->dev, mem->start, > - resource_size(mem)); > + if (rc) > put_device(&vm_dev->vdev.dev); > - } > - return rc; > -unmap: > - iounmap(vm_dev->base); > -free_mem: > -...

...8 @@ static int virtio_mmio_probe(struct platform_device *pdev) platform_set_drvdata(pdev, vm_dev); - return register_virtio_device(&vm_dev->vdev); + rc = register_virtio_device(&vm_dev->vdev); + if (rc) + goto unmap; + return 0; +unmap: + iounmap(vm_dev->base); +free_mem: + devm_release_mem_region(&pdev->dev, mem->start, + resource_size(mem)); +free_vmdev: + devm_kfree(&pdev->dev, vm_dev); + return rc; } static int virtio_mmio_remove(struct platform_device *pdev) -- 2.9.4

...8 @@ static int virtio_mmio_probe(struct platform_device *pdev) platform_set_drvdata(pdev, vm_dev); - return register_virtio_device(&vm_dev->vdev); + rc = register_virtio_device(&vm_dev->vdev); + if (rc) + goto unmap; + return 0; +unmap: + iounmap(vm_dev->base); +free_mem: + devm_release_mem_region(&pdev->dev, mem->start, + resource_size(mem)); +free_vmdev: + devm_kfree(&pdev->dev, vm_dev); + return rc; } static int virtio_mmio_remove(struct platform_device *pdev) -- 2.9.4

...} vm_dev->vdev.id.vendor = readl(vm_dev->base + VIRTIO_MMIO_VENDOR_ID); @@ -590,20 +583,9 @@ static int virtio_mmio_probe(struct platform_device *pdev) platform_set_drvdata(pdev, vm_dev); rc = register_virtio_device(&vm_dev->vdev); - if (rc) { - iounmap(vm_dev->base); - devm_release_mem_region(&pdev->dev, mem->start, - resource_size(mem)); + if (rc) put_device(&vm_dev->vdev.dev); - } - return rc; -unmap: - iounmap(vm_dev->base); -free_mem: - devm_release_mem_region(&pdev->dev, mem->start, - resource_size(mem)); -free_vmdev: - devm_kfree(&pdev-&g...

...} vm_dev->vdev.id.vendor = readl(vm_dev->base + VIRTIO_MMIO_VENDOR_ID); @@ -590,20 +583,9 @@ static int virtio_mmio_probe(struct platform_device *pdev) platform_set_drvdata(pdev, vm_dev); rc = register_virtio_device(&vm_dev->vdev); - if (rc) { - iounmap(vm_dev->base); - devm_release_mem_region(&pdev->dev, mem->start, - resource_size(mem)); + if (rc) put_device(&vm_dev->vdev.dev); - } - return rc; -unmap: - iounmap(vm_dev->base); -free_mem: - devm_release_mem_region(&pdev->dev, mem->start, - resource_size(mem)); -free_vmdev: - devm_kfree(&pdev-&g...

this patchset try to add cleanup for virtio_mmio driver, include virtio_mmio_probe and virtio_mmio_remove weiping zhang (2): virtio_mmio: add cleanup for virtio_mmio_probe virtio_mmio: add cleanup for virtio_mmio_remove drivers/virtio/virtio_mmio.c | 43 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) -- 2.9.4

this patchset try to add cleanup for virtio_mmio driver, include virtio_mmio_probe and virtio_mmio_remove weiping zhang (2): virtio_mmio: add cleanup for virtio_mmio_probe virtio_mmio: add cleanup for virtio_mmio_remove drivers/virtio/virtio_mmio.c | 43 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) -- 2.9.4

...VIRTIO_MMIO_VENDOR_ID); @@ -573,7 +589,22 @@ static int virtio_mmio_probe(struct platform_device *pdev) platform_set_drvdata(pdev, vm_dev); - return register_virtio_device(&vm_dev->vdev); + rc = register_virtio_device(&vm_dev->vdev); + if (rc) { + iounmap(vm_dev->base); + devm_release_mem_region(&pdev->dev, mem->start, + resource_size(mem)); + put_device(&vm_dev->vdev.dev); + } + return rc; +unmap: + iounmap(vm_dev->base); +free_mem: + devm_release_mem_region(&pdev->dev, mem->start, + resource_size(mem)); +free_vmdev: + devm_kfree(&pdev->dev, vm_...

...m_dev->base + VIRTIO_MMIO_VENDOR_ID); > > @@ -590,20 +583,9 @@ static int virtio_mmio_probe(struct platform_device *pdev) > platform_set_drvdata(pdev, vm_dev); > > rc = register_virtio_device(&vm_dev->vdev); > - if (rc) { > - iounmap(vm_dev->base); > - devm_release_mem_region(&pdev->dev, mem->start, > - resource_size(mem)); > + if (rc) > put_device(&vm_dev->vdev.dev); > - } > - return rc; > -unmap: > - iounmap(vm_dev->base); > -free_mem: > - devm_release_mem_region(&pdev->dev, mem->start, > - resource_...

...t know if device_add() had already exposed the struct device to other code that might have acquired a reference). As the device has an empty release function, touching the device structure after that is not a real problem, but... > +unmap: > + iounmap(vm_dev->base); > +free_mem: > + devm_release_mem_region(&pdev->dev, mem->start, > + resource_size(mem)); > +free_vmdev: > + devm_kfree(&pdev->dev, vm_dev); ...unconditionally freeing the device here would be a problem if other code had acquired a reference above. (Unlikely, but we should try to get this right.) > + retur...

...t know if device_add() had already exposed the struct device to other code that might have acquired a reference). As the device has an empty release function, touching the device structure after that is not a real problem, but... > +unmap: > + iounmap(vm_dev->base); > +free_mem: > + devm_release_mem_region(&pdev->dev, mem->start, > + resource_size(mem)); > +free_vmdev: > + devm_kfree(&pdev->dev, vm_dev); ...unconditionally freeing the device here would be a problem if other code had acquired a reference above. (Unlikely, but we should try to get this right.) > + retur...

...ev) > > platform_set_drvdata(pdev, vm_dev); > > - return register_virtio_device(&vm_dev->vdev); > + rc = register_virtio_device(&vm_dev->vdev); > + if (rc) > + goto unmap; > + return 0; > +unmap: > + iounmap(vm_dev->base); > +free_mem: > + devm_release_mem_region(&pdev->dev, mem->start, > + resource_size(mem)); > +free_vmdev: > + devm_kfree(&pdev->dev, vm_dev); I think this is problematic as vm_dev embeds a struct device (via embedding a struct virtio_device). I think the right way to do this is - call this only if register_virt...

...tic int virtio_mmio_remove(struct platform_device *pdev) { struct virtio_mmio_device *vm_dev = platform_get_drvdata(pdev); + struct resource *mem; unregister_virtio_device(&vm_dev->vdev); + iounmap(vm_dev->base); + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (mem) + devm_release_mem_region(&pdev->dev, mem->start, + resource_size(mem)); + devm_kfree(&pdev->dev, vm_dev); return 0; } -- 2.9.4

...rtio_mmio_probe(struct platform_device *pdev) static int virtio_mmio_remove(struct platform_device *pdev) { struct virtio_mmio_device *vm_dev = platform_get_drvdata(pdev); + struct resource *mem; + iounmap(vm_dev->base); + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (mem) + devm_release_mem_region(&pdev->dev, mem->start, + resource_size(mem)); unregister_virtio_device(&vm_dev->vdev); return 0; -- 2.9.4

...of release_mem_region_adjustable() is here, can we move > iomem_resource into the function body? Actually, we don't iterate the resource > tree from any level. We always start from the root. You mean, making iomem_resource implicit? I can spot that something similar was done for #define devm_release_mem_region(dev, start, n) \ __devm_release_region(dev, &iomem_resource, (start), (n)) I'll send an addon patch for that, ok? - thanks. -- Thanks, David / dhildenb

...;>> iomem_resource into the function body? Actually, we don't iterate the resource >>> tree from any level. We always start from the root. >> >> You mean, making iomem_resource implicit? I can spot that something >> similar was done for >> >> #define devm_release_mem_region(dev, start, n) \ >> __devm_release_region(dev, &iomem_resource, (start), (n)) >> > > What I prefer is remove iomem_resource from the parameter list. Just use is in > the function body. > > For the example you listed, __release_region() would have varies of *parent*...