[PATCH 1/6] staging: sm750fb: Use memset_io instead of memset

[PATCH 1/6] staging: sm750fb: Use memset_io instead of memset

[Lorenzo Stoakes]

This patch uses memset_io instead of memset when using memset on __iomem
qualified pointers. This fixes the following sparse warnings:-

drivers/staging/sm750fb/sm750.c:489:17: warning: incorrect type in argument 1 (different address spaces)
drivers/staging/sm750fb/sm750.c:490:17: warning: incorrect type in argument 1 (different address spaces)
drivers/staging/sm750fb/sm750.c:501:17: warning: incorrect type in argument 1 (different address spaces)
drivers/staging/sm750fb/sm750.c:502:17: warning: incorrect type in argument 1 (different address spaces)
drivers/staging/sm750fb/sm750.c:833:5: warning: incorrect type in argument 1 (different address spaces)
drivers/staging/sm750fb/sm750.c:1154:9: warning: incorrect type in argument 1 (different address spaces)

Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
 drivers/staging/sm750fb/sm750.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index aa0888c..3e36b6a 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -486,8 +486,8 @@ static int lynxfb_resume(struct pci_dev* pdev)
 		par = info->par;
 		crtc = &par->crtc;
 		cursor = &crtc->cursor;
-		memset(cursor->vstart, 0x0, cursor->size);
-		memset(crtc->vScreen,0x0,crtc->vidmem_size);
+		memset_io(cursor->vstart, 0x0, cursor->size);
+		memset_io(crtc->vScreen,0x0,crtc->vidmem_size);
 		lynxfb_ops_set_par(info);
 		fb_set_suspend(info, 0);
 	}
@@ -498,8 +498,8 @@ static int lynxfb_resume(struct pci_dev* pdev)
 		par = info->par;
 		crtc = &par->crtc;
 		cursor = &crtc->cursor;
-		memset(cursor->vstart, 0x0, cursor->size);
-		memset(crtc->vScreen,0x0,crtc->vidmem_size);
+		memset_io(cursor->vstart, 0x0, cursor->size);
+		memset_io(crtc->vScreen,0x0,crtc->vidmem_size);
 		lynxfb_ops_set_par(info);
 		fb_set_suspend(info, 0);
 	}
@@ -830,7 +830,7 @@ static int lynxfb_set_fbinfo(struct fb_info* info,int index)
 
 
     crtc->cursor.share = share;
-    memset(crtc->cursor.vstart, 0, crtc->cursor.size);
+    memset_io(crtc->cursor.vstart, 0, crtc->cursor.size);
     if(!g_hwcursor){
         lynxfb_ops.fb_cursor = NULL;
         crtc->cursor.disable(&crtc->cursor);
@@ -1151,7 +1151,7 @@ static int lynxfb_pci_probe(struct pci_dev * pdev,
 	}
 #endif
 
-	memset(share->pvMem,0,share->vidmem_size);
+	memset_io(share->pvMem,0,share->vidmem_size);
 
 	pr_info("sm%3x mmio address = %p\n",share->devid,share->pvReg);
 
-- 
2.3.2

[PATCH 2/6] staging: sm750fb: Fix non-ANSI function declarations

[Lorenzo Stoakes]

Fixes Function declarations which expect no parameters to have a parameter list consisting of void. This fixes the following sparse warnings:-

drivers/staging/sm750fb/sm750_hw.c:584:23: warning: non-ANSI function declaration of function 'hw_sm750le_deWait'
drivers/staging/sm750fb/sm750_hw.c:601:21: warning: non-ANSI function declaration of function 'hw_sm750_deWait'
9,13d7
drivers/staging/sm750fb/ddk750_chip.c:14:33: warning: non-ANSI function declaration of function 'getChipType'
drivers/staging/sm750fb/ddk750_chip.c:94:27: warning: non-ANSI function declaration of function 'getChipClock'
drivers/staging/sm750fb/ddk750_chip.c:235:31: warning: non-ANSI function declaration of function 'ddk750_getVMSize'
drivers/staging/sm750fb/ddk750_power.c:18:27: warning: non-ANSI function declaration of function 'getPowerMode'
drivers/staging/sm750fb/ddk750_display.c:276:24: warning: non-ANSI function declaration of function 'ddk750_initDVIDisp'
19,22d12
drivers/staging/sm750fb/ddk750_sii164.c:37:34: warning: non-ANSI function declaration of function 'sii164GetVendorID'
drivers/staging/sm750fb/ddk750_sii164.c:54:34: warning: non-ANSI function declaration of function 'sii164GetDeviceID'
drivers/staging/sm750fb/ddk750_dvi.c:65:31: warning: non-ANSI function declaration of function 'dviGetVendorID'
drivers/staging/sm750fb/ddk750_dvi.c:85:31: warning: non-ANSI function declaration of function 'dviGetDeviceID'

Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
 drivers/staging/sm750fb/ddk750_chip.c    | 6 +++---
 drivers/staging/sm750fb/ddk750_display.c | 2 +-
 drivers/staging/sm750fb/ddk750_dvi.c     | 4 ++--
 drivers/staging/sm750fb/ddk750_power.c   | 2 +-
 drivers/staging/sm750fb/ddk750_sii164.c  | 4 ++--
 drivers/staging/sm750fb/sm750_hw.c       | 4 ++--
 6 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_chip.c b/drivers/staging/sm750fb/ddk750_chip.c
index b71169e..3c77207 100644
--- a/drivers/staging/sm750fb/ddk750_chip.c
+++ b/drivers/staging/sm750fb/ddk750_chip.c
@@ -11,7 +11,7 @@ typedef struct _pllcalparam{
 pllcalparam;
 
 
-logical_chip_type_t getChipType()
+logical_chip_type_t getChipType(void)
 {
 	unsigned short physicalID;
 	char physicalRev;
@@ -91,7 +91,7 @@ unsigned int getPllValue(clock_type_t clockType, pll_value_t *pPLL)
 }
 
 
-unsigned int getChipClock()
+unsigned int getChipClock(void)
 {
     pll_value_t pll;
 #if 1
@@ -232,7 +232,7 @@ void setMasterClock(unsigned int frequency)
 }
 
 
-unsigned int ddk750_getVMSize()
+unsigned int ddk750_getVMSize(void)
 {
 	unsigned int reg;
 	unsigned int data;
diff --git a/drivers/staging/sm750fb/ddk750_display.c b/drivers/staging/sm750fb/ddk750_display.c
index a282a94..c84196a 100644
--- a/drivers/staging/sm750fb/ddk750_display.c
+++ b/drivers/staging/sm750fb/ddk750_display.c
@@ -273,7 +273,7 @@ void ddk750_setLogicalDispOut(disp_output_t output)
 }
 
 
-int ddk750_initDVIDisp()
+int ddk750_initDVIDisp(void)
 {
     /* Initialize DVI. If the dviInit fail and the VendorID or the DeviceID are
        not zeroed, then set the failure flag. If it is zeroe, it might mean
diff --git a/drivers/staging/sm750fb/ddk750_dvi.c b/drivers/staging/sm750fb/ddk750_dvi.c
index 1c083e7..f5932bb 100644
--- a/drivers/staging/sm750fb/ddk750_dvi.c
+++ b/drivers/staging/sm750fb/ddk750_dvi.c
@@ -62,7 +62,7 @@ int dviInit(
  *  Output:
  *      Vendor ID
  */
-unsigned short dviGetVendorID()
+unsigned short dviGetVendorID(void)
 {
     dvi_ctrl_device_t *pCurrentDviCtrl;
 
@@ -82,7 +82,7 @@ unsigned short dviGetVendorID()
  *  Output:
  *      Device ID
  */
-unsigned short dviGetDeviceID()
+unsigned short dviGetDeviceID(void)
 {
     dvi_ctrl_device_t *pCurrentDviCtrl;
 
diff --git a/drivers/staging/sm750fb/ddk750_power.c b/drivers/staging/sm750fb/ddk750_power.c
index 98dfcbd..cbb9767 100644
--- a/drivers/staging/sm750fb/ddk750_power.c
+++ b/drivers/staging/sm750fb/ddk750_power.c
@@ -15,7 +15,7 @@ void ddk750_setDPMS(DPMS_t state)
 	}
 }
 
-unsigned int getPowerMode()
+unsigned int getPowerMode(void)
 {
 	if(getChipType() = SM750LE)
 		return 0;
diff --git a/drivers/staging/sm750fb/ddk750_sii164.c b/drivers/staging/sm750fb/ddk750_sii164.c
index faf8250..bdd7742 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ b/drivers/staging/sm750fb/ddk750_sii164.c
@@ -34,7 +34,7 @@ static char *gDviCtrlChipName = "Silicon Image SiI 164";
  *  Output:
  *      Vendor ID
  */
-unsigned short sii164GetVendorID()
+unsigned short sii164GetVendorID(void)
 {
     unsigned short vendorID;
 
@@ -51,7 +51,7 @@ unsigned short sii164GetVendorID()
  *  Output:
  *      Device ID
  */
-unsigned short sii164GetDeviceID()
+unsigned short sii164GetDeviceID(void)
 {
     unsigned short deviceID;
 
diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index c44a50b..3050847 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -581,7 +581,7 @@ void hw_sm750_initAccel(struct lynx_share * share)
 	share->accel.de_init(&share->accel);
 }
 
-int hw_sm750le_deWait()
+int hw_sm750le_deWait(void)
 {
 	int i=0x10000000;
 	while(i--){
@@ -598,7 +598,7 @@ int hw_sm750le_deWait()
 }
 
 
-int hw_sm750_deWait()
+int hw_sm750_deWait(void)
 {
 	int i=0x10000000;
 	while(i--){
-- 
2.3.2

[PATCH 3/6] staging: sm750fb: Make internal functions static

[Lorenzo Stoakes]

This patch declares externally unavailable functions static. This fixes the
following sparse warnings:-

drivers/staging/sm750fb/ddk750_swi2c.c:223:6: warning: symbol 'swI2CStart' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_swi2c.c:234:6: warning: symbol 'swI2CStop' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_swi2c.c:252:6: warning: symbol 'swI2CWriteByte' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_swi2c.c:320:15: warning: symbol 'swI2CReadByte' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_swi2c.c:361:6: warning: symbol 'swI2CInit_SM750LE' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_hwi2c.c:63:6: warning: symbol 'hwI2CWaitTXDone' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_hwi2c.c:93:14: warning: symbol 'hwI2CWriteData' was not declared. Should it be static?
drivers/staging/sm750fb/ddk750_hwi2c.c:160:14: warning: symbol 'hwI2CReadData' was not declared. Should it be static?

Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
 drivers/staging/sm750fb/ddk750_hwi2c.c |  6 +++---
 drivers/staging/sm750fb/ddk750_swi2c.c | 10 +++++-----
 drivers/staging/sm750fb/sm750_accel.c  |  2 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_hwi2c.c b/drivers/staging/sm750fb/ddk750_hwi2c.c
index 84dfb6f..7826376 100644
--- a/drivers/staging/sm750fb/ddk750_hwi2c.c
+++ b/drivers/staging/sm750fb/ddk750_hwi2c.c
@@ -60,7 +60,7 @@ void hwI2CClose(void)
 }
 
 
-long hwI2CWaitTXDone(void)
+static long hwI2CWaitTXDone(void)
 {
     unsigned int timeout;
 
@@ -90,7 +90,7 @@ long hwI2CWaitTXDone(void)
  *  Return Value:
  *      Total number of bytes those are actually written.
  */
-unsigned int hwI2CWriteData(
+static unsigned int hwI2CWriteData(
     unsigned char deviceAddress,
     unsigned int length,
     unsigned char *pBuffer
@@ -157,7 +157,7 @@ unsigned int hwI2CWriteData(
  *  Return Value:
  *      Total number of actual bytes read from the slave device
  */
-unsigned int hwI2CReadData(
+static unsigned int hwI2CReadData(
     unsigned char deviceAddress,
     unsigned int length,
     unsigned char *pBuffer
diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
index 1249759..516f5bb 100644
--- a/drivers/staging/sm750fb/ddk750_swi2c.c
+++ b/drivers/staging/sm750fb/ddk750_swi2c.c
@@ -220,7 +220,7 @@ static void swI2CAck(void)
 /*
  *  This function sends the start command to the slave device
  */
-void swI2CStart(void)
+static void swI2CStart(void)
 {
     /* Start I2C */
     swI2CSDA(1);
@@ -231,7 +231,7 @@ void swI2CStart(void)
 /*
  *  This function sends the stop command to the slave device
  */
-void swI2CStop(void)
+static void swI2CStop(void)
 {
     /* Stop the I2C */
     swI2CSCL(1);
@@ -249,7 +249,7 @@ void swI2CStop(void)
  *       0   - Success
  *      -1   - Fail to write byte
  */
-long swI2CWriteByte(unsigned char data)
+static long swI2CWriteByte(unsigned char data)
 {
     unsigned char value = data;
     int i;
@@ -317,7 +317,7 @@ long swI2CWriteByte(unsigned char data)
  *  Return Value:
  *      One byte data read from the Slave device
  */
-unsigned char swI2CReadByte(unsigned char ack)
+static unsigned char swI2CReadByte(unsigned char ack)
 {
     int i;
     unsigned char data = 0;
@@ -358,7 +358,7 @@ unsigned char swI2CReadByte(unsigned char ack)
  *      -1   - Fail to initialize the i2c
  *       0   - Success
  */
-long swI2CInit_SM750LE(
+static long swI2CInit_SM750LE(
     unsigned char i2cClkGPIO,
     unsigned char i2cDataGPIO
 )
diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
index 6521c3b..4aa763b 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -92,7 +92,7 @@ void hw_set2dformat(struct lynx_accel * accel,int fmt)
 /* seems sm712 RectFill command is broken,so need use BitBlt to
  * replace it. */
 
-int hw712_fillrect(struct lynx_accel * accel,
+static int hw712_fillrect(struct lynx_accel * accel,
 				u32 base,u32 pitch,u32 Bpp,
 				u32 x,u32 y,u32 width,u32 height,
 				u32 color,u32 rop)
-- 
2.3.2

Re: [PATCH 3/6] staging: sm750fb: Make internal functions static

[Sudip Mukherjee]

On Wed, Mar 11, 2015 at 01:28:42AM +0000, Lorenzo Stoakes wrote:
> This patch declares externally unavailable functions static. This fixes the
> following sparse warnings:-
<snip>
> diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
> index 6521c3b..4aa763b 100644
> --- a/drivers/staging/sm750fb/sm750_accel.c
> +++ b/drivers/staging/sm750fb/sm750_accel.c
> @@ -92,7 +92,7 @@ void hw_set2dformat(struct lynx_accel * accel,int fmt)
>  /* seems sm712 RectFill command is broken,so need use BitBlt to
>   * replace it. */
>  
> -int hw712_fillrect(struct lynx_accel * accel,
> +static int hw712_fillrect(struct lynx_accel * accel,
>  				u32 base,u32 pitch,u32 Bpp,
>  				u32 x,u32 y,u32 width,u32 height,
>  				u32 color,u32 rop)
this is introducing a build warning, better remove this from your patch
and send a separate patch to remove the function as this function is
not used anywhere.

regards
sudip

> -- 
> 2.3.2
> 

Re: [PATCH 3/6] staging: sm750fb: Make internal functions static

[Lorenzo Stoakes]

On 11 March 2015 at 09:30, Sudip Mukherjee
> this is introducing a build warning, better remove this from your patch
> and send a separate patch to remove the function as this function is
> not used anywhere.

Hi Sudip,

I didn't realise I'd included the move to static in this patch. In a
later patch I expose this function in the header file. I'll update
this patch not to touch hw712_fillrect then remove it in a later patch
altogether.

Best,

-- 
Lorenzo Stoakes
https:/ljs.io

[PATCH 4/6] staging: sm750fb: Expose hw712_fillrect externally

[Lorenzo Stoakes]

This patch adds a reference to hw712_fillrect which is not used elsewhere in the driver,
but appears to be an alternative to the hw_fillrect method. This patch fixes the following sparse warning:-

drivers/staging/sm750fb/sm750_accel.c:95:5: warning: symbol 'hw712_fillrect' was not declared. Should it be static?

Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
 drivers/staging/sm750fb/sm750_accel.c | 2 +-
 drivers/staging/sm750fb/sm750_accel.h | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
index 4aa763b..6521c3b 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -92,7 +92,7 @@ void hw_set2dformat(struct lynx_accel * accel,int fmt)
 /* seems sm712 RectFill command is broken,so need use BitBlt to
  * replace it. */
 
-static int hw712_fillrect(struct lynx_accel * accel,
+int hw712_fillrect(struct lynx_accel * accel,
 				u32 base,u32 pitch,u32 Bpp,
 				u32 x,u32 y,u32 width,u32 height,
 				u32 color,u32 rop)
diff --git a/drivers/staging/sm750fb/sm750_accel.h b/drivers/staging/sm750fb/sm750_accel.h
index 3ee0bd8..51a9367 100644
--- a/drivers/staging/sm750fb/sm750_accel.h
+++ b/drivers/staging/sm750fb/sm750_accel.h
@@ -238,11 +238,16 @@ void hw_set2dformat(struct lynx_accel * accel,int fmt);
 
 void hw_de_init(struct lynx_accel * accel);
 
+int hw712_fillrect(struct lynx_accel * accel,
+				u32 base,u32 pitch,u32 Bpp,
+				u32 x,u32 y,u32 width,u32 height,
+				u32 color,u32 rop);
+
 int hw_fillrect(struct lynx_accel * accel,
 				u32 base,u32 pitch,u32 Bpp,
 				u32 x,u32 y,u32 width,u32 height,
 				u32 color,u32 rop);
 
 int hw_copyarea(
 struct lynx_accel * accel,
 unsigned int sBase,  /* Address of source: offset in frame buffer */
-- 
2.3.2

Re: [PATCH 4/6] staging: sm750fb: Expose hw712_fillrect externally

[Dan Carpenter]

On Wed, Mar 11, 2015 at 01:28:43AM +0000, Lorenzo Stoakes wrote:
> This patch adds a reference to hw712_fillrect which is not used elsewhere in the driver,
> but appears to be an alternative to the hw_fillrect method. This patch fixes the following sparse warning:-
> 
> drivers/staging/sm750fb/sm750_accel.c:95:5: warning: symbol 'hw712_fillrect' was not declared. Should it be static?
> 

Just delete it.

regards,
dan carpenter

Re: [PATCH 4/6] staging: sm750fb: Expose hw712_fillrect externally

[Sudip Mukherjee]

On Wed, Mar 11, 2015 at 01:28:43AM +0000, Lorenzo Stoakes wrote:
> 
> diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
> index 4aa763b..6521c3b 100644
> --- a/drivers/staging/sm750fb/sm750_accel.c
> +++ b/drivers/staging/sm750fb/sm750_accel.c
> @@ -92,7 +92,7 @@ void hw_set2dformat(struct lynx_accel * accel,int fmt)
>  /* seems sm712 RectFill command is broken,so need use BitBlt to
>   * replace it. */
>  
> -static int hw712_fillrect(struct lynx_accel * accel,
> +int hw712_fillrect(struct lynx_accel * accel,
>  				u32 base,u32 pitch,u32 Bpp,
>  				u32 x,u32 y,u32 width,u32 height,
>  				u32 color,u32 rop)
in your previous patch 3/6 you made it static now you are again
removing the static keyword. may i ask why you changed it in 3/6 if you
again change it back to original in this patch?
anyways, like Dan said, delete this function, its not used anywhere.
it will not be used also, i missed removing this function from the
vendor crude drver.

regards
sudip

> 

Re: [PATCH 4/6] staging: sm750fb: Expose hw712_fillrect externally

[Lorenzo Stoakes]

On 11 March 2015 at 09:37, Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote:
> in your previous patch 3/6 you made it static now you are again
> removing the static keyword. may i ask why you changed it in 3/6 if you
> again change it back to original in this patch?

There's no good reason, it's just a mistake :) I'll fix it shortly.

> anyways, like Dan said, delete this function, its not used anywhere.
> it will not be used also, i missed removing this function from the
> vendor crude drver.

Will do!

Best,
-- 
Lorenzo Stoakes
https:/ljs.io

[PATCH 5/6] staging: sm750fb: Fix __iomem pointer types

[Lorenzo Stoakes]

This patch annotates pointers as referring to I/O mapped memory where they ought
to be, removes now unnecessary ugly casts, eliminates an incorrect deref on I/O
mapped memory by using iowrite16 instead, and updates the pointer arithmetic
accordingly to take into account that the pointers are now byte-sized. This
fixes the following sparse warnings:-

drivers/staging/sm750fb/sm750_cursor.c:113:19: warning: cast removes address space of expression
drivers/staging/sm750fb/sm750_cursor.c:204:19: warning: cast removes address space of expression

Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
 drivers/staging/sm750fb/sm750_cursor.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750_cursor.c b/drivers/staging/sm750fb/sm750_cursor.c
index 6cceef1..c2ff3bd 100644
--- a/drivers/staging/sm750fb/sm750_cursor.c
+++ b/drivers/staging/sm750fb/sm750_cursor.c
@@ -98,7 +98,7 @@ void hw_cursor_setData(struct lynx_cursor * cursor,
 	int i,j,count,pitch,offset;
 	u8 color,mask,opr;
 	u16 data;
-	u16 * pbuffer,*pstart;
+	void __iomem * pbuffer,*pstart;
 
 	/*  in byte*/
 	pitch = cursor->w >> 3;
@@ -106,11 +106,11 @@ void hw_cursor_setData(struct lynx_cursor * cursor,
 	/* in byte	*/
 	count = pitch * cursor->h;
 
-	/* in ushort */
-	offset = cursor->maxW * 2 / 8 / 2;
+	/* in byte */
+	offset = cursor->maxW * 2 / 8;
 
 	data = 0;
-	pstart = (u16 *)cursor->vstart;
+	pstart = cursor->vstart;
 	pbuffer = pstart;
 
 /*
@@ -161,7 +161,7 @@ void hw_cursor_setData(struct lynx_cursor * cursor,
 			}
 		}
 #endif
-		*pbuffer = data;
+		iowrite16(data, pbuffer);
 
 		/* assume pitch is 1,2,4,8,...*/
 #if 0
@@ -174,7 +174,7 @@ void hw_cursor_setData(struct lynx_cursor * cursor,
 			pstart += offset;
 			pbuffer = pstart;
 		}else{
-			pbuffer++;
+			pbuffer += sizeof(u16);
 		}
 
 	}
@@ -189,7 +189,7 @@ void hw_cursor_setData2(struct lynx_cursor * cursor,
 	int i,j,count,pitch,offset;
 	u8 color, mask;
 	u16 data;
-	u16 * pbuffer,*pstart;
+	void __iomem * pbuffer,*pstart;
 
 	/*  in byte*/
 	pitch = cursor->w >> 3;
@@ -197,11 +197,11 @@ void hw_cursor_setData2(struct lynx_cursor * cursor,
 	/* in byte	*/
 	count = pitch * cursor->h;
 
-	/* in ushort */
-	offset = cursor->maxW * 2 / 8 / 2;
+	/* in byte */
+	offset = cursor->maxW * 2 / 8;
 
 	data = 0;
-	pstart = (u16 *)cursor->vstart;
+	pstart = cursor->vstart;
 	pbuffer = pstart;
 
 	for(i=0;i<count;i++)
@@ -234,7 +234,7 @@ void hw_cursor_setData2(struct lynx_cursor * cursor,
 				data |= ((color & (1<<j))?1:2)<<(j*2);
 		}
 #endif
-		*pbuffer = data;
+		iowrite16(data, pbuffer);
 
 		/* assume pitch is 1,2,4,8,...*/
 		if(!(i&(pitch-1)))
@@ -244,7 +244,7 @@ void hw_cursor_setData2(struct lynx_cursor * cursor,
 			pstart += offset;
 			pbuffer = pstart;
 		}else{
-			pbuffer++;
+			pbuffer += sizeof(u16);
 		}
 
 	}
-- 
2.3.2

[PATCH 6/6] staging: sm750fb: Spinlock and unlock in the same block

[Lorenzo Stoakes]

This patch combines spinlock locks and unlocks together in the same block rather
than occurring in separate blocks preventing a possible deadlock. This fixes the
following sparse warnings:-

drivers/staging/sm750fb/sm750.c:218:22: warning: context imbalance in 'lynxfb_ops_fillrect' - different lock contexts for basic block
drivers/staging/sm750fb/sm750.c:241:22: warning: context imbalance in 'lynxfb_ops_copyarea' - different lock contexts for basic block
drivers/staging/sm750fb/sm750.c:282:22: warning: context imbalance in 'lynxfb_ops_imageblit' - different lock contexts for basic block

Unfortunately this change involves code (and comment) duplication.

Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
 drivers/staging/sm750fb/sm750.c | 76 +++++++++++++++++++++++------------------
 1 file changed, 43 insertions(+), 33 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 3e36b6a..58c7518 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -56,23 +56,6 @@ static char * g_settings = NULL;
 static int g_dualview = 0;
 static char * g_option = NULL;
 
-/* if not use spin_lock,system will die if user load driver
- * and immediatly unload driver frequently (dual)*/
-static inline void myspin_lock(spinlock_t * sl){
-	struct lynx_share * share;
-	share = container_of(sl,struct lynx_share,slock);
-	if(share->dual){
-		spin_lock(sl);
-	}
-}
-
-static inline void myspin_unlock(spinlock_t * sl){
-	struct lynx_share * share;
-	share = container_of(sl,struct lynx_share,slock);
-	if(share->dual){
-		spin_unlock(sl);
-	}
-}
 static const struct fb_videomode lynx750_ext[] = {
 	/*  	1024x600-60 VESA 	[1.71:1]	*/
 	{NULL,  60, 1024, 600, 20423, 144,  40, 18, 1, 104, 3,
@@ -209,13 +192,22 @@ static void lynxfb_ops_fillrect(struct fb_info* info,const struct fb_fillrect* r
 	color = (Bpp = 1)?region->color:((u32*)info->pseudo_palette)[region->color];
 	rop = ( region->rop != ROP_COPY ) ? HW_ROP2_XOR:HW_ROP2_COPY;
 
-	myspin_lock(&share->slock);
-	share->accel.de_fillrect(&share->accel,
-							base,pitch,Bpp,
-							region->dx,region->dy,
-							region->width,region->height,
-							color,rop);
-	myspin_unlock(&share->slock);
+	/* if not use spin_lock,system will die if user load driver
+	 * and immediatly unload driver frequently (dual)*/
+	if (share->dual) {
+		spin_lock(&share->slock);
+		share->accel.de_fillrect(&share->accel,
+					base,pitch,Bpp,
+					region->dx,region->dy,
+					region->width,region->height,
+					color,rop);
+		spin_unlock(&share->slock);
+	} else
+		share->accel.de_fillrect(&share->accel,
+					base,pitch,Bpp,
+					region->dx,region->dy,
+					region->width,region->height,
+					color,rop);
 }
 
 static void lynxfb_ops_copyarea(struct fb_info * info,const struct fb_copyarea * region)
@@ -233,12 +225,20 @@ static void lynxfb_ops_copyarea(struct fb_info * info,const struct fb_copyarea *
 	pitch = info->fix.line_length;
 	Bpp = info->var.bits_per_pixel >> 3;
 
-	myspin_lock(&share->slock);
-	share->accel.de_copyarea(&share->accel,
-							base,pitch,region->sx,region->sy,
-							base,pitch,Bpp,region->dx,region->dy,
-							region->width,region->height,HW_ROP2_COPY);
-	myspin_unlock(&share->slock);
+	/* if not use spin_lock,system will die if user load driver
+	 * and immediatly unload driver frequently (dual)*/
+	if (share->dual) {
+		spin_lock(&share->slock);
+		share->accel.de_copyarea(&share->accel,
+					base,pitch,region->sx,region->sy,
+					base,pitch,Bpp,region->dx,region->dy,
+					region->width,region->height,HW_ROP2_COPY);
+		spin_unlock(&share->slock);
+	} else
+		share->accel.de_copyarea(&share->accel,
+					base,pitch,region->sx,region->sy,
+					base,pitch,Bpp,region->dx,region->dy,
+					region->width,region->height,HW_ROP2_COPY);
 }
 
 static void lynxfb_ops_imageblit(struct fb_info*info,const struct fb_image* image)
@@ -272,14 +272,24 @@ static void lynxfb_ops_imageblit(struct fb_info*info,const struct fb_image* imag
 	}
 	return;
 _do_work:
-	myspin_lock(&share->slock);
-	share->accel.de_imageblit(&share->accel,
+	/* if not use spin_lock,system will die if user load driver
+	 * and immediatly unload driver frequently (dual)*/
+	if (share->dual) {
+		spin_lock(&share->slock);
+		share->accel.de_imageblit(&share->accel,
+					image->data,image->width>>3,0,
+					base,pitch,Bpp,
+					image->dx,image->dy,
+					image->width,image->height,
+					fgcol,bgcol,HW_ROP2_COPY);
+		spin_unlock(&share->slock);
+	} else
+		share->accel.de_imageblit(&share->accel,
 					image->data,image->width>>3,0,
 					base,pitch,Bpp,
 					image->dx,image->dy,
 					image->width,image->height,
 					fgcol,bgcol,HW_ROP2_COPY);
-	myspin_unlock(&share->slock);
 }
 
 static int lynxfb_ops_pan_display(struct fb_var_screeninfo *var,
-- 
2.3.2

Re: [PATCH 6/6] staging: sm750fb: Spinlock and unlock in the same block

[Dan Carpenter]

On Wed, Mar 11, 2015 at 01:28:45AM +0000, Lorenzo Stoakes wrote:
> -static inline void myspin_lock(spinlock_t * sl){
> -	struct lynx_share * share;
> -	share = container_of(sl,struct lynx_share,slock);
> -	if(share->dual){
> -		spin_lock(sl);
> -	}
> -}

Yes, good.  We all hate locking wrappers but these are worse than
normal.

> +	/* if not use spin_lock,system will die if user load driver
> +	 * and immediatly unload driver frequently (dual)*/
> +	if (share->dual) {
> +		spin_lock(&share->slock);
> +		share->accel.de_fillrect(&share->accel,
> +					base,pitch,Bpp,
> +					region->dx,region->dy,
> +					region->width,region->height,
> +					color,rop);
> +		spin_unlock(&share->slock);
> +	} else
> +		share->accel.de_fillrect(&share->accel,
> +					base,pitch,Bpp,
> +					region->dx,region->dy,
> +					region->width,region->height,
> +					color,rop);
>  }

No.  You've made the code uglier to work around Sparse stupidness.  Also
the braces are not according to kernel style.

	if (share->dual)
		spin_lock(&share->slock);

	share->accel.de_fillrect(&share->accel,
				 base,pitch,Bpp,
				 region->dx,region->dy,
				 region->width,region->height,
				 color,rop);

	if (share->dual)
		spin_unlock(&share->slock);

Sparse will still complain but no one cares.

regards,

Re: [PATCH 1/6] staging: sm750fb: Use memset_io instead of memset

[Dan Carpenter]

On Wed, Mar 11, 2015 at 01:28:40AM +0000, Lorenzo Stoakes wrote:
> This patch uses memset_io instead of memset when using memset on __iomem
> qualified pointers. This fixes the following sparse warnings:-
> 
> drivers/staging/sm750fb/sm750.c:489:17: warning: incorrect type in argument 1 (different address spaces)
> drivers/staging/sm750fb/sm750.c:490:17: warning: incorrect type in argument 1 (different address spaces)
> drivers/staging/sm750fb/sm750.c:501:17: warning: incorrect type in argument 1 (different address spaces)
> drivers/staging/sm750fb/sm750.c:502:17: warning: incorrect type in argument 1 (different address spaces)
> drivers/staging/sm750fb/sm750.c:833:5: warning: incorrect type in argument 1 (different address spaces)
> drivers/staging/sm750fb/sm750.c:1154:9: warning: incorrect type in argument 1 (different address spaces)
> 
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>

When I see a patch like this, then I worry, "What if the Sparse
annotations are wrong?  The patch description doesn't say anything about
that."  After review then I think the annotations are correct so that's
fine.

Btw, do you have this hardware?  Are you able to test these changes?

regards,
dan carpenter

Re: [PATCH 1/6] staging: sm750fb: Use memset_io instead of memset

[Lorenzo Stoakes]

On 11 March 2015 at 08:54, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> When I see a patch like this, then I worry, "What if the Sparse
> annotations are wrong?  The patch description doesn't say anything about
> that."  After review then I think the annotations are correct so that's
> fine.

How do you mean? I was careful to check what sparse was referring to,
then investigate how memset should be used with pointers with a
__iomem qualifier. I'd like to be able to improve my patch
descriptions going forward as best I can :)

> Btw, do you have this hardware?  Are you able to test these changes?

Unfortunately not, I am trying to keep these changes as simple code
fixes that ought not to affect actual hardware behaviour as I can
(though of course you can never be entirely sure that's the case!)

I suspect that Sudip must have some real hardware, is this the case
Sudip? If it isn't too presumptuous of me to ask, perhaps you might be
able to check patches that are successfully merged into
staging-testing?

Best,

-- 
Lorenzo Stoakes
https:/ljs.io

Re: [PATCH 1/6] staging: sm750fb: Use memset_io instead of memset

[Dan Carpenter]

On Wed, Mar 11, 2015 at 09:11:52AM +0000, Lorenzo Stoakes wrote:
> On 11 March 2015 at 08:54, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > When I see a patch like this, then I worry, "What if the Sparse
> > annotations are wrong?  The patch description doesn't say anything about
> > that."  After review then I think the annotations are correct so that's
> > fine.
> 
> How do you mean? I was careful to check what sparse was referring to,
> then investigate how memset should be used with pointers with a
> __iomem qualifier. I'd like to be able to improve my patch
> descriptions going forward as best I can :)
> 

Yes.  The patch is correct.  I wasn't asking you to redo it.  From later
patches it's actually clear that you know that this change is a bugfix
and a behavior change.  But we get a lot of patches where people just
randomly change things to please Sparse and it maybe silences a warning
but it's not correct.  I can think of a few recentish examples where
people used standard struct types which hold __iomem or __user pointers
but they used them in non-standard ways so the pointers were actually
normal kernel pointers.

I guess the rule here is that the patch should explain the effect of the
bugfix for the user.  Often you won't know the effect, but it's a
helpful thing to think about.

> > Btw, do you have this hardware?  Are you able to test these changes?
> 
> Unfortunately not, I am trying to keep these changes as simple code
> fixes that ought not to affect actual hardware behaviour as I can
> (though of course you can never be entirely sure that's the case!)

That's fine.  I was just wondering.  It affects how paranoid I am when I
review the code.

regards,
dan carpenter

Re: [PATCH 1/6] staging: sm750fb: Use memset_io instead of memset

[Sudip Mukherjee]

On Wed, Mar 11, 2015 at 09:11:52AM +0000, Lorenzo Stoakes wrote:
> On 11 March 2015 at 08:54, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > Btw, do you have this hardware?  Are you able to test these changes?
> 
> Unfortunately not, I am trying to keep these changes as simple code
> fixes that ought not to affect actual hardware behaviour as I can
> (though of course you can never be entirely sure that's the case!)
> 
> I suspect that Sudip must have some real hardware, is this the case
> Sudip? If it isn't too presumptuous of me to ask, perhaps you might be
> able to check patches that are successfully merged into
> staging-testing?
yes, i have the hardware and will test on it. but your patch 5/6 and
6/6 is scaring me :)

regards
sudip
> 
> Best,
> 
> -- 
> Lorenzo Stoakes
> https:/ljs.io

Re: [PATCH 1/6] staging: sm750fb: Use memset_io instead of memset

[Sudip Mukherjee]

On Wed, Mar 11, 2015 at 03:18:06PM +0530, Sudip Mukherjee wrote:
> On Wed, Mar 11, 2015 at 09:11:52AM +0000, Lorenzo Stoakes wrote:
> > On 11 March 2015 at 08:54, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > > Btw, do you have this hardware?  Are you able to test these changes?
> > 
> > Unfortunately not, I am trying to keep these changes as simple code
> > fixes that ought not to affect actual hardware behaviour as I can
> > (though of course you can never be entirely sure that's the case!)
> > 
> > I suspect that Sudip must have some real hardware, is this the case
> > Sudip? If it isn't too presumptuous of me to ask, perhaps you might be
> > able to check patches that are successfully merged into
> > staging-testing?
> yes, i have the hardware and will test on it. but your patch 5/6 and
> 6/6 is scaring me :)

i think i will better check v2 of your series on hardware, and while
you are preparing that v2 keep in mind the changelog should not exceed
72 characters. in your this series for all patches it was more than
that.

regards
sudip

> 
> regards
> sudip
> > 
> > Best,
> > 
> > -- 
> > Lorenzo Stoakes
> > https:/ljs.io

Re: [PATCH 1/6] staging: sm750fb: Use memset_io instead of memset

[Lorenzo Stoakes]

On 11 March 2015 at 10:35, Sudip Mukherjee
> i think i will better check v2 of your series on hardware

This is incoming in just a moment (though I only v2 patches in the
series I've changed which I think is the right way to make
modifications with a patch series.)

> , and while
> you are preparing that v2 keep in mind the changelog should not exceed
> 72 characters. in your this series for all patches it was more than
> that.

I will update the messages in the changed patches accordingly, I'm not
sure this is worth a resend of all previous patches for however? I do
see quite a few patches in the log that exceed this.

Additionally, I suspect it would make the patches less readable to
wrap sparse warning lines so I think those ought to sit outside of
this limit.

I am more than happy to change these though if these ought to be kept
*strictly* to a 72 character limit throughout?

Best,

-- 
Lorenzo Stoakes
https:/ljs.io