Skip to contentSkip to conversation

platinumfb: Fix resource management

PATCH In review Antonino A. Daplas 4 participants 7 messages Sep 29, 2007 – Oct 3, 2007 Framebuffer layer

lore
Share & tools
lore
#0 lore raw

PATCH 6/17 platinumfb: Fix resource management

From: Benjamin Herrenschmidt <[email protected]>

Current kernels have a non-working platinumfb due to some resource management issues. This fixes it.

---

 drivers/video/platinumfb.c |   48 ++++++++++++++++++++++++--------------------
 1 files changed, 26 insertions(+), 22 deletions(-)
drivers/video/platinumfb.c +26 -22
index 8503e73..cbe71a5 100644
@@ -17,6 +17,8 @@
  *  more details.
  */
 
+#undef DEBUG
+
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -535,33 +537,35 @@ static int __devinit platinumfb_probe(st
 	volatile __u8		*fbuffer;
 	int			bank0, bank1, bank2, bank3, rc;
 
-	printk(KERN_INFO "platinumfb: Found Apple Platinum video hardware\n");
+	dev_info(&odev->dev, "Found Apple Platinum video hardware\n");
 
 	info = framebuffer_alloc(sizeof(*pinfo), &odev->dev);
-	if (info == NULL)
+	if (info == NULL) {
+		dev_err(&odev->dev, "Failed to allocate fbdev !\n");
 		return -ENOMEM;
+	}
 	pinfo = info->par;
 
 	if (of_address_to_resource(dp, 0, &pinfo->rsrc_reg) ||
 	    of_address_to_resource(dp, 1, &pinfo->rsrc_fb)) {
-		printk(KERN_ERR "platinumfb: Can't get resources\n");
-		framebuffer_release(info);
-		return -ENXIO;
-	}
-	if (!request_mem_region(pinfo->rsrc_reg.start,
-				pinfo->rsrc_reg.start -
-				pinfo->rsrc_reg.end + 1,
-				"platinumfb registers")) {
+		dev_err(&odev->dev, "Can't get resources\n");
 		framebuffer_release(info);
 		return -ENXIO;
 	}
+	dev_dbg(&odev->dev, " registers  : 0x%llx...0x%llx\n",
+		(unsigned long long)pinfo->rsrc_reg.start,
+		(unsigned long long)pinfo->rsrc_reg.end);
+	dev_dbg(&odev->dev, " framebuffer: 0x%llx...0x%llx\n",
+		(unsigned long long)pinfo->rsrc_fb.start,
+		(unsigned long long)pinfo->rsrc_fb.end);
+
+	/* Do not try to request register space, they overlap with the
+	 * northbridge and that can fail. Only request framebuffer
+	 */
 	if (!request_mem_region(pinfo->rsrc_fb.start,
-				pinfo->rsrc_fb.start
-				- pinfo->rsrc_fb.end + 1,
+				pinfo->rsrc_fb.end - pinfo->rsrc_fb.start + 1,
 				"platinumfb framebuffer")) {
-		release_mem_region(pinfo->rsrc_reg.start,
-				   pinfo->rsrc_reg.end -
-				   pinfo->rsrc_reg.start + 1);
+		printk(KERN_ERR "platinumfb: Can't request framebuffer !\n");
 		framebuffer_release(info);
 		return -ENXIO;
 	}
@@ -600,7 +604,8 @@ static int __devinit platinumfb_probe(st
 	bank2 = fbuffer[0x200000] == 0x56;
 	bank3 = fbuffer[0x300000] == 0x78;
 	pinfo->total_vram = (bank0 + bank1 + bank2 + bank3) * 0x100000;
-	printk(KERN_INFO "platinumfb: Total VRAM = %dMB (%d%d%d%d)\n", (int) (pinfo->total_vram / 1024 / 1024),
+	printk(KERN_INFO "platinumfb: Total VRAM = %dMB (%d%d%d%d)\n",
+	       (unsigned int) (pinfo->total_vram / 1024 / 1024),
 	       bank3, bank2, bank1, bank0);
 
 	/*
@@ -644,16 +649,15 @@ static int __devexit platinumfb_remove(s
         unregister_framebuffer (info);
 	
 	/* Unmap frame buffer and registers */
+	iounmap(pinfo->frame_buffer);
+	iounmap(pinfo->platinum_regs);
+	iounmap(pinfo->cmap_regs);
+
 	release_mem_region(pinfo->rsrc_fb.start,
 			   pinfo->rsrc_fb.end -
 			   pinfo->rsrc_fb.start + 1);
-	release_mem_region(pinfo->rsrc_reg.start,
-			   pinfo->rsrc_reg.end -
-			   pinfo->rsrc_reg.start + 1);
-	iounmap(pinfo->frame_buffer);
-	iounmap(pinfo->platinum_regs);
+
 	release_mem_region(pinfo->cmap_regs_phys, 0x1000);
-	iounmap(pinfo->cmap_regs);
 
 	framebuffer_release(info);
 

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

#1 lore raw
Quoted 17 lines · Antonino A. Daplas

From: Benjamin Herrenschmidt <[email protected]>

Current kernels have a non-working platinumfb due to some resource management issues. This fixes it.

---

 drivers/video/platinumfb.c |   48 ++++++++++++++++++++++++--------------------
 1 files changed, 26 insertions(+), 22 deletions(-)
drivers/video/platinumfb.c +0 -0
index 8503e73..cbe71a5 100644

drivers/video/platinumfb.c line 24

Quoted 8 lines
drivers/video/platinumfb.c +2 -0
@@ -17,6 +17,8 @@
  *  more details.
  */
 
+#undef DEBUG
+
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>

Probably scripts/checkpatch.pl should be taught to complain about `#undef DEBUG'.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

#2 lore raw
Quoted 3 lines

Probably scripts/checkpatch.pl should be taught to complain about `#undef DEBUG'.

Why ? Whenever I have things that are #ifdef DEBUG in the file (such as dev_dbg statements), I always leave a #undef DEBUG statement at the top.

That way, I know I can enable debug in that file by flicking it.

Ben.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

#3 lore raw
Quoted 5 lines · Benjamin Herrenschmidt
Quoted 2 lines

Probably scripts/checkpatch.pl should be taught to complain about `#undef DEBUG'.

Why ? Whenever I have things that are #ifdef DEBUG in the file (such as dev_dbg statements), I always leave a #undef DEBUG statement at the top.

Because it overrides a -DDEBUG on the build command line.

Quoted 1 line

That way, I know I can enable debug in that file by flicking it.

Just add #define DEBUG at the top?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

#4 lore raw
Quoted 8 lines · Geert Uytterhoeven
Quoted 5 lines · Benjamin Herrenschmidt
Quoted 2 lines

Probably scripts/checkpatch.pl should be taught to complain about `#undef DEBUG'.

Why ? Whenever I have things that are #ifdef DEBUG in the file (such as dev_dbg statements), I always leave a #undef DEBUG statement at the top.

Because it overrides a -DDEBUG on the build command line.

Hrm... ok, I never play with the commandline :-)

Quoted 3 lines
Quoted 1 line

That way, I know I can enable debug in that file by flicking it.

Just add #define DEBUG at the top?

Yeah well, it's really just a reminder that there's DEBUG stuff in that file, but feel free to send a patch removing it :-)

Ben.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

#5 lore raw
In reply to Antonino A. Daplas
Quoted 2 lines · On Sat, 29 Sep 2007 09:29:53 +0800 "Antonino A. Daplas" <[email protected]>…

Current kernels have a non-working platinumfb due to some resource management issues. This fixes it.

Why isn't this 2.6.23 material?

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

#6 lore raw
Quoted 7 lines · Andrew Morton
Quoted 2 lines · On Sat, 29 Sep 2007 09:29:53 +0800 "Antonino A. Daplas" <[email protected]>…

Current kernels have a non-working platinumfb due to some resource management issues. This fixes it.

Why isn't this 2.6.23 material?

We decided it's been broken long enough and nobody complained so it wasn't worth pushing. I wouldn't be surprised if there weren't more than a handful of people left with a PowerMac 7200 which is I think the only machine using platinumfb.

Feel free to send it to Linus for .23 if you feel like it though.

Cheers,
Ben.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/