[PATCH] skeletonfb

[PATCH] skeletonfb

[Lucas Correia Villa Real]

[-- Attachment #1: Type: text/plain, Size: 522 bytes --]

Hi,

This patch just fixes some typos and adds retval documentation for some 
functions that were missing it.

I've also a question here: skeletonfb says that  xxxfb_setcolreg() should 
return a negative errno on error. However, if the register number being 
accessed is out of bounds, 1 is returned. Shouldn't it be better to return 
-EINVAL instead?  By looking at fbcmap.c, the retval isn't being checked 
against positive/negative values, so it doesn't make sense to return 1. 

Thanks,

-- 
Lucas
powered by /dev/dsp

[-- Attachment #2: skeletonfb.patch --]
[-- Type: text/x-diff, Size: 3618 bytes --]

--- linux-2.6.10-lucasvr/drivers/video/skeletonfb.c.orig	2005-01-25 23:02:56.000000000 -0200
+++ linux-2.6.10-lucasvr/drivers/video/skeletonfb.c	2005-01-30 19:07:32.000000000 -0200
@@ -120,7 +120,7 @@
 static struct fb_info info;
 
     /* 
-     * Each one represents the a state of the hardware. Most hardware have 
+     * Each one represents the state of the hardware. Most hardware have 
      * just one hardware state. These here represent the default state(s). 
      */
 static struct xxx_par __initdata current_par;
@@ -139,6 +139,8 @@
  *	Usually you don't need to provide this function. The case where it 
  *	is used is to change from a text mode hardware state to a graphics
  * 	mode state. 
+ * 	
+ *	Returns negative errno on error, or zero on success.
  */
 static int xxxfb_open(const struct fb_info *info, int user)
 {
@@ -156,6 +158,8 @@
  *	console system is released. Usually you don't need this function.
  *	The case where it is usually used is to go from a graphics state
  *	to a text mode state.
+ *
+ *	Returns negative errno on error, or zero on success.
  */
 static int xxxfb_release(const struct fb_info *info, int user)
 {
@@ -201,8 +205,9 @@
  *	fb_info since we are using that data. This means we depend on the
  *	data in var inside fb_info to be supported by the hardware. 
  *	xxxfb_check_var is always called before xxxfb_set_par to ensure this.
- *	Again if you can't can't the resolution you don't need this function.
- *
+ *	Again if you can't change the resolution you don't need this function.
+ *	
+ *	Returns negative errno on error, or zero on success.
  */
 static int xxxfb_set_par(struct fb_info *info)
 {
@@ -217,14 +222,14 @@
  *      @red: The red value which can be up to 16 bits wide 
  *	@green: The green value which can be up to 16 bits wide 
  *	@blue:  The blue value which can be up to 16 bits wide.
- *	@transp: If supported the alpha value which can be up to 16 bits wide.	
+ *	@transp: If supported, the alpha value which can be up to 16 bits wide.	
  *      @info: frame buffer info structure
  * 
  *  	Set a single color register. The values supplied have a 16 bit
  *  	magnitude which needs to be scaled in this function for the hardware. 
  *	Things to take into consideration are how many color registers, if
  *	any, are supported with the current color visual. With truecolor mode
- *	no color palettes are supported. Here a psuedo palette is created 
+ *	no color palettes are supported. Here a pseudo palette is created 
  *	which we store the value in pseudo_palette in struct fb_info. For
  *	pseudocolor mode we have a limited color palette. To deal with this
  *	we can program what color is displayed for a particular pixel value.
@@ -238,7 +243,7 @@
 			   const struct fb_info *info)
 {
     if (regno >= 256)  /* no. of hw registers */
-       return 1;
+       return -EINVAL;
     /*
      * Program hardware... do anything you want with transp
      */
@@ -295,7 +300,7 @@
        u32 v;
 
        if (regno >= 16)
-           return 1;
+           return -EINVAL;
 
        v = (red << info->var.red.offset) |
            (green << info->var.green.offset) |
@@ -563,7 +568,7 @@
 #endif
 
     /* 
-     * Here we set the screen_base to the vitrual memory address
+     * Here we set the screen_base to the virtual memory address
      * for the framebuffer. Usually we obtain the resource address
      * from the bus layer and then translate it to virtual memory
      * space via ioremap. Consult ioport.h. 
@@ -621,6 +626,7 @@
      */
 
     unregister_framebuffer(info);
+    fb_dealloc_cmap(&info.cmap);
     /* ... */
 }
 

Re: [PATCH] skeletonfb

[James Simmons]

Thanks. I merged it with my tree. I also will be writing newer 
documentation for people.

On Sun, 30 Jan 2005, Lucas Correia Villa Real wrote:

> Hi,
> 
> This patch just fixes some typos and adds retval documentation for some 
> functions that were missing it.
> 
> I've also a question here: skeletonfb says that  xxxfb_setcolreg() should 
> return a negative errno on error. However, if the register number being 
> accessed is out of bounds, 1 is returned. Shouldn't it be better to return 
> -EINVAL instead?  By looking at fbcmap.c, the retval isn't being checked 
> against positive/negative values, so it doesn't make sense to return 1. 
> 
> Thanks,
> 
> -- 
> Lucas
> powered by /dev/dsp
> 


-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl

Re: [PATCH] skeletonfb

[Antonino A. Daplas]

On Monday 31 January 2005 05:11, Lucas Correia Villa Real wrote:
> Hi,
>
> This patch just fixes some typos and adds retval documentation for some
> functions that were missing it.
>
> I've also a question here: skeletonfb says that  xxxfb_setcolreg() should
> return a negative errno on error. However, if the register number being
> accessed is out of bounds, 1 is returned. Shouldn't it be better to return
> -EINVAL instead?  By looking at fbcmap.c, the retval isn't being checked
> against positive/negative values, so it doesn't make sense to return 1.
>
> Thanks,

Please, add a "Signed-off-by:" line next time.

Tony




-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl