grvga: Fix error handling issues

This patch fixes two problems with the error handling in the grvga_probe function and simplifies it making the code easier to read. - If the call to grvga_parse_custom on line 370 fails we use the wrong label so that release_mem_region will be called without a call to request_mem_region being made. - If the call to ioremap on line 436 fails we should not try to call iounmap in the error handling code. This patch introduces the following changes: - Converts request_mem_region into its devm_ equivalent which simplifies the otherwise messy clean up code. - Changes the labels for correct error handling and their names to make the code easier to read. Signed-off-by: Emil Goode <emilgoode@gmail.com> Signed-off-by: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>

grvga: Fix error handling issues
This patch fixes two problems with the error handling in the grvga_probe function and simplifies it making the code easier to read. - If the call to grvga_parse_custom on line 370 fails we use the wrong label so that release_mem_region will be called without a call to request_mem_region being made. - If the call to ioremap on line 436 fails we should not try to call iounmap in the error handling code. This patch introduces the following changes: - Converts request_mem_region into its devm_ equivalent which simplifies the otherwise messy clean up code. - Changes the labels for correct error handling and their names to make the code easier to read. Signed-off-by: Emil Goode <emilgoode@gmail.com> Signed-off-by: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Emil Goode · Florian Tobias Schandinat
1 parent fb18155925
Showing 1 changed file with 22 additions and 25 deletions Side-by-side Diff
drivers/video/grvga.c
@@ -354,7 +354,7 @@
 	 */
 	if (fb_get_options("grvga", &options)) {
 		retval = -ENODEV;
-		goto err;
+		goto free_fb;
 	}
  
 	if (!options || !*options)
@@ -370,7 +370,7 @@
 			if (grvga_parse_custom(this_opt, &info->var) < 0) {
 				dev_err(&dev->dev, "Failed to parse custom mode (%s).\n", this_opt);
 				retval = -EINVAL;
-				goto err1;
+				goto free_fb;
 			}
 		} else if (!strncmp(this_opt, "addr", 4))
 			grvga_fix_addr = simple_strtoul(this_opt + 5, NULL, 16);
  
@@ -387,10 +387,11 @@
 	info->flags = FBINFO_DEFAULT | FBINFO_PARTIAL_PAN_OK | FBINFO_HWACCEL_YPAN;
 	info->fix.smem_len = grvga_mem_size;
  
-	if (!request_mem_region(dev->resource[0].start, resource_size(&dev->resource[0]), "grlib-svgactrl regs")) {
+	if (!devm_request_mem_region(&dev->dev, dev->resource[0].start,
+		    resource_size(&dev->resource[0]), "grlib-svgactrl regs")) {
 		dev_err(&dev->dev, "registers already mapped\n");
 		retval = -EBUSY;
-		goto err;
+		goto free_fb;
 	}
  
 	par->regs = of_ioremap(&dev->resource[0], 0,
  
@@ -400,14 +401,14 @@
 	if (!par->regs) {
 		dev_err(&dev->dev, "failed to map registers\n");
 		retval = -ENOMEM;
-		goto err1;
+		goto free_fb;
 	}
  
 	retval = fb_alloc_cmap(&info->cmap, 256, 0);
 	if (retval < 0) {
 		dev_err(&dev->dev, "failed to allocate mem with fb_alloc_cmap\n");
 		retval = -ENOMEM;
-		goto err2;
+		goto unmap_regs;
 	}
  
 	if (mode_opt) {
@@ -415,7 +416,7 @@
 				      grvga_modedb, sizeof(grvga_modedb), &grvga_modedb[0], 8);
 		if (!retval || retval == 4) {
 			retval = -EINVAL;
-			goto err3;
+			goto dealloc_cmap;
 		}
 	}
  
  
@@ -427,10 +428,11 @@
  
 		physical_start = grvga_fix_addr;
  
-		if (!request_mem_region(physical_start, grvga_mem_size, dev->name)) {
+		if (!devm_request_mem_region(&dev->dev, physical_start,
+					     grvga_mem_size, dev->name)) {
 			dev_err(&dev->dev, "failed to request memory region\n");
 			retval = -ENOMEM;
-			goto err3;
+			goto dealloc_cmap;
 		}
  
 		virtual_start = (unsigned long) ioremap(physical_start, grvga_mem_size);
@@ -438,7 +440,7 @@
 		if (!virtual_start) {
 			dev_err(&dev->dev, "error mapping framebuffer memory\n");
 			retval = -ENOMEM;
-			goto err4;
+			goto dealloc_cmap;
 		}
 	} else {	/* Allocate frambuffer memory */
  
@@ -451,7 +453,7 @@
 				"unable to allocate framebuffer memory (%lu bytes)\n",
 				grvga_mem_size);
 			retval = -ENOMEM;
-			goto err3;
+			goto dealloc_cmap;
 		}
  
 		physical_start = dma_map_single(&dev->dev, (void *)virtual_start, grvga_mem_size, DMA_TO_DEVICE);
@@ -484,7 +486,7 @@
 	retval = register_framebuffer(info);
 	if (retval < 0) {
 		dev_err(&dev->dev, "failed to register framebuffer\n");
-		goto err4;
+		goto free_mem;
 	}
  
 	__raw_writel(physical_start, &par->regs->fb_pos);
  
  
  
  
  
@@ -493,21 +495,18 @@
  
 	return 0;
  
-err4:
+free_mem:
 	dev_set_drvdata(&dev->dev, NULL);
-	if (grvga_fix_addr) {
-		release_mem_region(physical_start, grvga_mem_size);
+	if (grvga_fix_addr)
 		iounmap((void *)virtual_start);
-	} else
+	else
 		kfree((void *)virtual_start);
-err3:
+dealloc_cmap:
 	fb_dealloc_cmap(&info->cmap);
-err2:
+unmap_regs:
 	of_iounmap(&dev->resource[0], par->regs,
 		   resource_size(&dev->resource[0]));
-err1:
-	release_mem_region(dev->resource[0].start, resource_size(&dev->resource[0]));
-err:
+free_fb:
 	framebuffer_release(info);
  
 	return retval;
  
  
@@ -524,12 +523,10 @@
  
 		of_iounmap(&device->resource[0], par->regs,
 			   resource_size(&device->resource[0]));
-		release_mem_region(device->resource[0].start, resource_size(&device->resource[0]));
  
-		if (!par->fb_alloced) {
-			release_mem_region(info->fix.smem_start, info->fix.smem_len);
+		if (!par->fb_alloced)
 			iounmap(info->screen_base);
-		} else
+		else
 			kfree((void *)info->screen_base);
  
 		framebuffer_release(info);
...	...	@@ -354,7 +354,7 @@
354	354	*/
355	355	if (fb_get_options("grvga", &options)) {
356	356	retval = -ENODEV;
357		- goto err;
	357	+ goto free_fb;
358	358	}
359	359
360	360	if (!options \|\| !*options)
...	...	@@ -370,7 +370,7 @@
370	370	if (grvga_parse_custom(this_opt, &info->var) < 0) {
371	371	dev_err(&dev->dev, "Failed to parse custom mode (%s).\n", this_opt);
372	372	retval = -EINVAL;
373		- goto err1;
	373	+ goto free_fb;
374	374	}
375	375	} else if (!strncmp(this_opt, "addr", 4))
376	376	grvga_fix_addr = simple_strtoul(this_opt + 5, NULL, 16);
377	377
...	...	@@ -387,10 +387,11 @@
387	387	info->flags = FBINFO_DEFAULT \| FBINFO_PARTIAL_PAN_OK \| FBINFO_HWACCEL_YPAN;
388	388	info->fix.smem_len = grvga_mem_size;
389	389
390		- if (!request_mem_region(dev->resource[0].start, resource_size(&dev->resource[0]), "grlib-svgactrl regs")) {
	390	+ if (!devm_request_mem_region(&dev->dev, dev->resource[0].start,
	391	+ resource_size(&dev->resource[0]), "grlib-svgactrl regs")) {
391	392	dev_err(&dev->dev, "registers already mapped\n");
392	393	retval = -EBUSY;
393		- goto err;
	394	+ goto free_fb;
394	395	}
395	396
396	397	par->regs = of_ioremap(&dev->resource[0], 0,
397	398
...	...	@@ -400,14 +401,14 @@
400	401	if (!par->regs) {
401	402	dev_err(&dev->dev, "failed to map registers\n");
402	403	retval = -ENOMEM;
403		- goto err1;
	404	+ goto free_fb;
404	405	}
405	406
406	407	retval = fb_alloc_cmap(&info->cmap, 256, 0);
407	408	if (retval < 0) {
408	409	dev_err(&dev->dev, "failed to allocate mem with fb_alloc_cmap\n");
409	410	retval = -ENOMEM;
410		- goto err2;
	411	+ goto unmap_regs;
411	412	}
412	413
413	414	if (mode_opt) {
...	...	@@ -415,7 +416,7 @@
415	416	grvga_modedb, sizeof(grvga_modedb), &grvga_modedb[0], 8);
416	417	if (!retval \|\| retval == 4) {
417	418	retval = -EINVAL;
418		- goto err3;
	419	+ goto dealloc_cmap;
419	420	}
420	421	}
421	422
422	423
...	...	@@ -427,10 +428,11 @@
427	428
428	429	physical_start = grvga_fix_addr;
429	430
430		- if (!request_mem_region(physical_start, grvga_mem_size, dev->name)) {
	431	+ if (!devm_request_mem_region(&dev->dev, physical_start,
	432	+ grvga_mem_size, dev->name)) {
431	433	dev_err(&dev->dev, "failed to request memory region\n");
432	434	retval = -ENOMEM;
433		- goto err3;
	435	+ goto dealloc_cmap;
434	436	}
435	437
436	438	virtual_start = (unsigned long) ioremap(physical_start, grvga_mem_size);
...	...	@@ -438,7 +440,7 @@
438	440	if (!virtual_start) {
439	441	dev_err(&dev->dev, "error mapping framebuffer memory\n");
440	442	retval = -ENOMEM;
441		- goto err4;
	443	+ goto dealloc_cmap;
442	444	}
443	445	} else { /* Allocate frambuffer memory */
444	446
...	...	@@ -451,7 +453,7 @@
451	453	"unable to allocate framebuffer memory (%lu bytes)\n",
452	454	grvga_mem_size);
453	455	retval = -ENOMEM;
454		- goto err3;
	456	+ goto dealloc_cmap;
455	457	}
456	458
457	459	physical_start = dma_map_single(&dev->dev, (void *)virtual_start, grvga_mem_size, DMA_TO_DEVICE);
...	...	@@ -484,7 +486,7 @@
484	486	retval = register_framebuffer(info);
485	487	if (retval < 0) {
486	488	dev_err(&dev->dev, "failed to register framebuffer\n");
487		- goto err4;
	489	+ goto free_mem;
488	490	}
489	491
490	492	__raw_writel(physical_start, &par->regs->fb_pos);
491	493
492	494
493	495
494	496
495	497
...	...	@@ -493,21 +495,18 @@
493	495
494	496	return 0;
495	497
496		-err4:
	498	+free_mem:
497	499	dev_set_drvdata(&dev->dev, NULL);
498		- if (grvga_fix_addr) {
499		- release_mem_region(physical_start, grvga_mem_size);
	500	+ if (grvga_fix_addr)
500	501	iounmap((void *)virtual_start);
501		- } else
	502	+ else
502	503	kfree((void *)virtual_start);
503		-err3:
	504	+dealloc_cmap:
504	505	fb_dealloc_cmap(&info->cmap);
505		-err2:
	506	+unmap_regs:
506	507	of_iounmap(&dev->resource[0], par->regs,
507	508	resource_size(&dev->resource[0]));
508		-err1:
509		- release_mem_region(dev->resource[0].start, resource_size(&dev->resource[0]));
510		-err:
	509	+free_fb:
511	510	framebuffer_release(info);
512	511
513	512	return retval;
514	513
515	514
...	...	@@ -524,12 +523,10 @@
524	523
525	524	of_iounmap(&device->resource[0], par->regs,
526	525	resource_size(&device->resource[0]));
527		- release_mem_region(device->resource[0].start, resource_size(&device->resource[0]));
528	526
529		- if (!par->fb_alloced) {
530		- release_mem_region(info->fix.smem_start, info->fix.smem_len);
	527	+ if (!par->fb_alloced)
531	528	iounmap(info->screen_base);
532		- } else
	529	+ else
533	530	kfree((void *)info->screen_base);
534	531
535	532	framebuffer_release(info);