Hi,
Jonathan Corbet schrieb:
On Thu, 29 Apr 2010 20:19:02 +0200
Bruno Prémont <bonbons@xxxxxxxxxxxxxxxxx> wrote:
+ 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);
Shouldn't this ioremap_nocache() have error-checking
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.
Please don't do this. The reason for not failing in viafb is that
ioremap failure of the engine is not critical it just disables the
hardware acceleration but otherwiese it works quite nice. The problem is
that ioremap failures are quite common as we really try to remap huge
amount (sometimes above 128 or even 256 MB). In fact the bug (or missing
feature) is that the first ioremap is fatal and that needs to be fixed
on the long run. Sorry but I don't want to make viafb unusable for many
people if they don't add the "vmalloc=" kernel option so failing here
would be a step in the wrong direction.
Thanks,
Florian Tobias Schandinat
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.
*/
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
