diff mbox

[4/6] staging: sm750fb: correct incompatible pointer type

Message ID 1426007817-1884-5-git-send-email-sudipm.mukherjee@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sudip Mukherjee March 10, 2015, 5:16 p.m. UTC
we were getting build warnings about assignment of incompatible
pointer types. some of the function definitions were having wrong
return type or arguments.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/sm750fb/sm750.h       | 11 ++++++-----
 drivers/staging/sm750fb/sm750_accel.c |  4 ++--
 drivers/staging/sm750fb/sm750_accel.h |  4 ++--
 3 files changed, 10 insertions(+), 9 deletions(-)

Comments

Greg KH March 10, 2015, 8:11 p.m. UTC | #1
On Tue, Mar 10, 2015 at 10:46:55PM +0530, Sudip Mukherjee wrote:
> we were getting build warnings about assignment of incompatible
> pointer types. some of the function definitions were having wrong
> return type or arguments.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
>  drivers/staging/sm750fb/sm750.h       | 11 ++++++-----
>  drivers/staging/sm750fb/sm750_accel.c |  4 ++--
>  drivers/staging/sm750fb/sm750_accel.h |  4 ++--
>  3 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
> index d39968c..edb955a 100644
> --- a/drivers/staging/sm750fb/sm750.h
> +++ b/drivers/staging/sm750fb/sm750.h
> @@ -23,7 +23,7 @@ struct lynx_accel{
>  	volatile unsigned char __iomem * dpPortBase;
>  
>  	/* function fointers */
> -	int (*de_init)(struct lynx_accel *);
> +	void (*de_init)(struct lynx_accel *);

That's ok, but:

>  
>  	int (*de_wait)(void);/* see if hardware ready to work */
>  
> @@ -34,8 +34,8 @@ struct lynx_accel{
>  						u32,u32,u32,u32,
>  						u32,u32,u32,u32);
>  
> -	int (*de_imageblit)(struct lynx_accel *,const char *,u32,u32,u32,
> -						u32,u32,u32,u32,u32,u32,u32,u32,u32);
> +	int (*de_imageblit)(struct lynx_accel *, const char *, u32, u32, u32,
> +			    u32, u32, u32, u32, u32, u32, u32, u32, u32);
>  


This isn't fixing an error.

>  };
>  
> @@ -120,8 +120,9 @@ struct lynxfb_crtc{
>  	int(*proc_setColReg)(struct lynxfb_crtc*,ushort,ushort,ushort,ushort);
>  	void (*clear)(struct lynxfb_crtc*);
>          /* pan display */
> -        int(*proc_panDisplay)(struct lynxfb_crtc*, struct fb_var_screeninfo*,
> -                struct fb_info*);
> +	int (*proc_panDisplay)(struct lynxfb_crtc*,
> +			       const struct fb_var_screeninfo*,
> +			       const struct fb_info*);

That's a nice cleanup, but it's not even the correct cleanup.

Please just fix the specific warning, don't mix what you are doing in
one patch.  You aren't documenting this in the changelog information, so
I have to reject it, sorry.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH March 10, 2015, 8:17 p.m. UTC | #2
On Tue, Mar 10, 2015 at 09:11:00PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Mar 10, 2015 at 10:46:55PM +0530, Sudip Mukherjee wrote:
> > we were getting build warnings about assignment of incompatible
> > pointer types. some of the function definitions were having wrong
> > return type or arguments.
> > 
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > ---
> >  drivers/staging/sm750fb/sm750.h       | 11 ++++++-----
> >  drivers/staging/sm750fb/sm750_accel.c |  4 ++--
> >  drivers/staging/sm750fb/sm750_accel.h |  4 ++--
> >  3 files changed, 10 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
> > index d39968c..edb955a 100644
> > --- a/drivers/staging/sm750fb/sm750.h
> > +++ b/drivers/staging/sm750fb/sm750.h
> > @@ -23,7 +23,7 @@ struct lynx_accel{
> >  	volatile unsigned char __iomem * dpPortBase;
> >  
> >  	/* function fointers */
> > -	int (*de_init)(struct lynx_accel *);
> > +	void (*de_init)(struct lynx_accel *);
> 
> That's ok, but:
> 
> >  
> >  	int (*de_wait)(void);/* see if hardware ready to work */
> >  
> > @@ -34,8 +34,8 @@ struct lynx_accel{
> >  						u32,u32,u32,u32,
> >  						u32,u32,u32,u32);
> >  
> > -	int (*de_imageblit)(struct lynx_accel *,const char *,u32,u32,u32,
> > -						u32,u32,u32,u32,u32,u32,u32,u32,u32);
> > +	int (*de_imageblit)(struct lynx_accel *, const char *, u32, u32, u32,
> > +			    u32, u32, u32, u32, u32, u32, u32, u32, u32);
> >  
> 
> 
> This isn't fixing an error.
> 
> >  };
> >  
> > @@ -120,8 +120,9 @@ struct lynxfb_crtc{
> >  	int(*proc_setColReg)(struct lynxfb_crtc*,ushort,ushort,ushort,ushort);
> >  	void (*clear)(struct lynxfb_crtc*);
> >          /* pan display */
> > -        int(*proc_panDisplay)(struct lynxfb_crtc*, struct fb_var_screeninfo*,
> > -                struct fb_info*);
> > +	int (*proc_panDisplay)(struct lynxfb_crtc*,
> > +			       const struct fb_var_screeninfo*,
> > +			       const struct fb_info*);
> 
> That's a nice cleanup, but it's not even the correct cleanup.

Oops, sorry, it is the correct cleanup, I was looking at the '*'
placement, not the const part.

but the patch needs to be redone, I'll just go fix up the build warnings
for now...

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sudip Mukherjee March 11, 2015, 7:28 a.m. UTC | #3
On Tue, Mar 10, 2015 at 09:17:54PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Mar 10, 2015 at 09:11:00PM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Mar 10, 2015 at 10:46:55PM +0530, Sudip Mukherjee wrote:
> > > we were getting build warnings about assignment of incompatible
<snip>
> > 
> > That's a nice cleanup, but it's not even the correct cleanup.
> 
> Oops, sorry, it is the correct cleanup, I was looking at the '*'
> placement, not the const part.
> 
> but the patch needs to be redone, I'll just go fix up the build warnings
> for now...

thanks Greg for your patience and time to redo this patch. I saw you broke it into two - three different patches.
But just one doubt, was your compiler showing any warning for the patch - "Staging: sm750fb: provide error path for
 hw_sm750le_setBLANK()" , commit id - e74ac550298ec4635cc32e99f966568a808fd370 . my compiler didnot show any warning for that.
mine is gcc 4.7.3, is it time to update?

thanks again, and sorry that you had to do fix this. I should have made the patches in a more proper way.

regards
sudip

> 
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH March 11, 2015, 7:43 a.m. UTC | #4
On Wed, Mar 11, 2015 at 12:58:42PM +0530, Sudip Mukherjee wrote:
> On Tue, Mar 10, 2015 at 09:17:54PM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Mar 10, 2015 at 09:11:00PM +0100, Greg Kroah-Hartman wrote:
> > > On Tue, Mar 10, 2015 at 10:46:55PM +0530, Sudip Mukherjee wrote:
> > > > we were getting build warnings about assignment of incompatible
> <snip>
> > > 
> > > That's a nice cleanup, but it's not even the correct cleanup.
> > 
> > Oops, sorry, it is the correct cleanup, I was looking at the '*'
> > placement, not the const part.
> > 
> > but the patch needs to be redone, I'll just go fix up the build warnings
> > for now...
> 
> thanks Greg for your patience and time to redo this patch. I saw you
> broke it into two - three different patches.
> But just one doubt, was your compiler showing any warning for the
> patch - "Staging: sm750fb: provide error path for
>  hw_sm750le_setBLANK()" , commit id -
>  e74ac550298ec4635cc32e99f966568a808fd370 . my compiler didnot show
>  any warning for that.
> mine is gcc 4.7.3, is it time to update?

Yes it is, that's a really old version of gcc.  Also please wrap your
email lines at 72 columns :)

> thanks again, and sorry that you had to do fix this. I should have
> made the patches in a more proper way.

Not a problem, it's part of the learning process.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
index d39968c..edb955a 100644
--- a/drivers/staging/sm750fb/sm750.h
+++ b/drivers/staging/sm750fb/sm750.h
@@ -23,7 +23,7 @@  struct lynx_accel{
 	volatile unsigned char __iomem * dpPortBase;
 
 	/* function fointers */
-	int (*de_init)(struct lynx_accel *);
+	void (*de_init)(struct lynx_accel *);
 
 	int (*de_wait)(void);/* see if hardware ready to work */
 
@@ -34,8 +34,8 @@  struct lynx_accel{
 						u32,u32,u32,u32,
 						u32,u32,u32,u32);
 
-	int (*de_imageblit)(struct lynx_accel *,const char *,u32,u32,u32,
-						u32,u32,u32,u32,u32,u32,u32,u32,u32);
+	int (*de_imageblit)(struct lynx_accel *, const char *, u32, u32, u32,
+			    u32, u32, u32, u32, u32, u32, u32, u32, u32);
 
 };
 
@@ -120,8 +120,9 @@  struct lynxfb_crtc{
 	int(*proc_setColReg)(struct lynxfb_crtc*,ushort,ushort,ushort,ushort);
 	void (*clear)(struct lynxfb_crtc*);
         /* pan display */
-        int(*proc_panDisplay)(struct lynxfb_crtc*, struct fb_var_screeninfo*,
-                struct fb_info*);
+	int (*proc_panDisplay)(struct lynxfb_crtc*,
+			       const struct fb_var_screeninfo*,
+			       const struct fb_info*);
 	/* cursor information */
 	struct lynx_cursor cursor;
 };
diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
index ee211de..421adef 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -397,8 +397,8 @@  static unsigned int deGetTransparency(struct lynx_accel * accel)
 
 int hw_imageblit(
 struct lynx_accel * accel,
-unsigned char *pSrcbuf, /* pointer to start of source buffer in system memory */
-int srcDelta,          /* Pitch value (in bytes) of the source buffer, +ive means top down and -ive mean button up */
+const char *pSrcbuf, /* pointer to start of source buffer in system memory */
+unsigned int srcDelta,          /* Pitch value (in bytes) of the source buffer, +ive means top down and -ive mean button up */
 unsigned int startBit, /* Mono data can start at any bit in a byte, this value should be 0 to 7 */
 unsigned int dBase,    /* Address of destination: offset in frame buffer */
 unsigned int dPitch,   /* Pitch value of destination surface in BYTE */
diff --git a/drivers/staging/sm750fb/sm750_accel.h b/drivers/staging/sm750fb/sm750_accel.h
index 575f4a7..9c16618 100644
--- a/drivers/staging/sm750fb/sm750_accel.h
+++ b/drivers/staging/sm750fb/sm750_accel.h
@@ -260,8 +260,8 @@  unsigned int rop2);
 
 int hw_imageblit(
 struct lynx_accel * accel,
-unsigned char *pSrcbuf, /* pointer to start of source buffer in system memory */
-int srcDelta,          /* Pitch value (in bytes) of the source buffer, +ive means top down and -ive mean button up */
+const char *pSrcbuf, /* pointer to start of source buffer in system memory */
+unsigned int srcDelta, /* Pitch value (in bytes) of the source buffer, +ive means top down and -ive mean button up */
 unsigned int startBit, /* Mono data can start at any bit in a byte, this value should be 0 to 7 */
 unsigned int dBase,    /* Address of destination: offset in frame buffer */
 unsigned int dPitch,   /* Pitch value of destination surface in BYTE */