On Thu, 29 Apr 2010 20:19:02 +0200
Bruno Prémont <bonbons@xxxxxxxxxxxxxxxxx> wrote:
+ vdev->engine_start = pci_resource_start(vdev->pdev, 1);Shouldn't this ioremap_nocache() have error-checking
+ vdev->engine_len = pci_resource_len(vdev->pdev, 1);
+ /* If this fails, others will notice later */
+ vdev->engine_mmio = ioremap_nocache(vdev->engine_start,
+ vdev->engine_len);
as the one below (instead of relying on later code to bail out)?
It would be better, yes, I'll add a patch to fix it up.
viafb: Fix initialization error paths
Properly localize error cleanup, and make sure that the iomem regions are
unmapped if framebuffer initialization fails.
Reported-by: Bruno Prémont <bonbons@xxxxxxxxxxxxxxxxx>
Signed-off-by: Jonathan Corbet <corbet@xxxxxxx>
---
drivers/video/via/via-core.c | 26 ++++++++++++++++----------
1 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/video/via/via-core.c b/drivers/video/via/via-core.c
index 2b5773a..b8a8783 100644
--- a/drivers/video/via/via-core.c
+++ b/drivers/video/via/via-core.c
@@ -454,26 +454,32 @@ static int viafb_get_fb_size_from_pci(int chip_type)
*/
static int __devinit via_pci_setup_mmio(struct viafb_dev *vdev)
{
+ int ret;
/*
* Hook up to the device registers.
*/
vdev->engine_start = pci_resource_start(vdev->pdev, 1);
vdev->engine_len = pci_resource_len(vdev->pdev, 1);
- /* If this fails, others will notice later */
vdev->engine_mmio = ioremap_nocache(vdev->engine_start,
vdev->engine_len);
-
+ if (vdev->engine_mmio == NULL)
+ return -ENOMEM;
/*
- * Likewise with I/O memory.
+ * Likewise with framebuffer memory.
*/
vdev->fbmem_start = pci_resource_start(vdev->pdev, 0);
- vdev->fbmem_len = viafb_get_fb_size_from_pci(vdev->chip_type);
- if (vdev->fbmem_len < 0)
- return vdev->fbmem_len;
+ ret = vdev->fbmem_len = viafb_get_fb_size_from_pci(vdev->chip_type);
+ if (ret < 0)
+ goto out_unmap;
vdev->fbmem = ioremap_nocache(vdev->fbmem_start, vdev->fbmem_len);
- if (vdev->fbmem == NULL)
- return -ENOMEM;
+ if (vdev->fbmem == NULL) {
+ ret = -ENOMEM;
+ goto out_unmap;
+ }
return 0;
+out_unmap:
+ iounmap(vdev->engine_mmio);
+ return ret;
}
static void __devexit via_pci_teardown_mmio(struct viafb_dev *vdev)
@@ -566,13 +572,13 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
spin_lock_init(&global_dev.reg_lock);
ret = via_pci_setup_mmio(&global_dev);
if (ret)
- goto out_teardown;
+ return ret;
/*
* Set up the framebuffer device
*/
ret = via_fb_pci_probe(&global_dev);
if (ret)
- return ret;
+ goto out_teardown;
/*
* Create our subdevices.
*/