All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: sm750fb: Fix sparse warning
@ 2015-03-09 20:57 ` Lorenzo Stoakes
  0 siblings, 0 replies; 17+ messages in thread
From: Lorenzo Stoakes @ 2015-03-09 20:57 UTC (permalink / raw)
  To: sudipm.mukherjee, teddy.wang, gregkh
  Cc: linux-fbdev, devel, linux-kernel, Lorenzo Stoakes

This patch fixes the following sparse warning:-

drivers/staging/sm750fb/ddk750_help.c: warning: incorrect type in assignment (different address spaces)

Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
 drivers/staging/sm750fb/ddk750_chip.h | 4 +++-
 drivers/staging/sm750fb/ddk750_help.c | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_chip.h b/drivers/staging/sm750fb/ddk750_chip.h
index 1c78875..a4e5bcc 100644
--- a/drivers/staging/sm750fb/ddk750_chip.h
+++ b/drivers/staging/sm750fb/ddk750_chip.h
@@ -3,6 +3,8 @@
 #define DEFAULT_INPUT_CLOCK 14318181 /* Default reference clock */
 #define SM750LE_REVISION_ID (char)0xfe
 
+#include <asm/io.h>
+
 /* This is all the chips recognized by this library */
 typedef enum _logical_chip_type_t
 {
@@ -70,7 +72,7 @@ logical_chip_type_t getChipType(void);
 unsigned int calcPllValue(unsigned int request,pll_value_t *pll);
 unsigned int calcPllValue2(unsigned int,pll_value_t *);
 unsigned int formatPllReg(pll_value_t *pPLL);
-void ddk750_set_mmio(volatile unsigned char *,unsigned short,char);
+void ddk750_set_mmio(volatile unsigned char __iomem *,unsigned short,char);
 unsigned int ddk750_getVMSize(void);
 int ddk750_initHw(initchip_param_t *);
 unsigned int getPllValue(clock_type_t clockType, pll_value_t *pPLL);
diff --git a/drivers/staging/sm750fb/ddk750_help.c b/drivers/staging/sm750fb/ddk750_help.c
index cc00d2b..6ad4dec 100644
--- a/drivers/staging/sm750fb/ddk750_help.c
+++ b/drivers/staging/sm750fb/ddk750_help.c
@@ -7,7 +7,7 @@ char revId750 = 0;
 unsigned short devId750 = 0;
 
 /* after driver mapped io registers, use this function first */
-void ddk750_set_mmio(volatile unsigned char * addr,unsigned short devId,char revId)
+void ddk750_set_mmio(volatile unsigned char __iomem * addr,unsigned short devId,char revId)
 {
 	mmio750 = addr;
 	devId750 = devId;
-- 
2.3.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH] staging: sm750fb: Fix sparse warning
@ 2015-03-09 20:57 ` Lorenzo Stoakes
  0 siblings, 0 replies; 17+ messages in thread
From: Lorenzo Stoakes @ 2015-03-09 20:57 UTC (permalink / raw)
  To: sudipm.mukherjee, teddy.wang, gregkh
  Cc: linux-fbdev, devel, linux-kernel, Lorenzo Stoakes

This patch fixes the following sparse warning:-

drivers/staging/sm750fb/ddk750_help.c: warning: incorrect type in assignment (different address spaces)

Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
---
 drivers/staging/sm750fb/ddk750_chip.h | 4 +++-
 drivers/staging/sm750fb/ddk750_help.c | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_chip.h b/drivers/staging/sm750fb/ddk750_chip.h
index 1c78875..a4e5bcc 100644
--- a/drivers/staging/sm750fb/ddk750_chip.h
+++ b/drivers/staging/sm750fb/ddk750_chip.h
@@ -3,6 +3,8 @@
 #define DEFAULT_INPUT_CLOCK 14318181 /* Default reference clock */
 #define SM750LE_REVISION_ID (char)0xfe
 
+#include <asm/io.h>
+
 /* This is all the chips recognized by this library */
 typedef enum _logical_chip_type_t
 {
@@ -70,7 +72,7 @@ logical_chip_type_t getChipType(void);
 unsigned int calcPllValue(unsigned int request,pll_value_t *pll);
 unsigned int calcPllValue2(unsigned int,pll_value_t *);
 unsigned int formatPllReg(pll_value_t *pPLL);
-void ddk750_set_mmio(volatile unsigned char *,unsigned short,char);
+void ddk750_set_mmio(volatile unsigned char __iomem *,unsigned short,char);
 unsigned int ddk750_getVMSize(void);
 int ddk750_initHw(initchip_param_t *);
 unsigned int getPllValue(clock_type_t clockType, pll_value_t *pPLL);
diff --git a/drivers/staging/sm750fb/ddk750_help.c b/drivers/staging/sm750fb/ddk750_help.c
index cc00d2b..6ad4dec 100644
--- a/drivers/staging/sm750fb/ddk750_help.c
+++ b/drivers/staging/sm750fb/ddk750_help.c
@@ -7,7 +7,7 @@ char revId750 = 0;
 unsigned short devId750 = 0;
 
 /* after driver mapped io registers, use this function first */
-void ddk750_set_mmio(volatile unsigned char * addr,unsigned short devId,char revId)
+void ddk750_set_mmio(volatile unsigned char __iomem * addr,unsigned short devId,char revId)
 {
 	mmio750 = addr;
 	devId750 = devId;
-- 
2.3.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] staging: sm750fb: Fix sparse warning
  2015-03-09 20:57 ` Lorenzo Stoakes
@ 2015-03-10  7:00   ` Dan Carpenter
  -1 siblings, 0 replies; 17+ messages in thread
From: Dan Carpenter @ 2015-03-10  7:00 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: sudipm.mukherjee, teddy.wang, gregkh, devel, linux-fbdev, linux-kernel

On Mon, Mar 09, 2015 at 08:57:08PM +0000, Lorenzo Stoakes wrote:
> This patch fixes the following sparse warning:-
> 
> drivers/staging/sm750fb/ddk750_help.c: warning: incorrect type in assignment (different address spaces)
> 
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
> ---
>  drivers/staging/sm750fb/ddk750_chip.h | 4 +++-
>  drivers/staging/sm750fb/ddk750_help.c | 2 +-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_chip.h b/drivers/staging/sm750fb/ddk750_chip.h
> index 1c78875..a4e5bcc 100644
> --- a/drivers/staging/sm750fb/ddk750_chip.h
> +++ b/drivers/staging/sm750fb/ddk750_chip.h
> @@ -3,6 +3,8 @@
>  #define DEFAULT_INPUT_CLOCK 14318181 /* Default reference clock */
>  #define SM750LE_REVISION_ID (char)0xfe
>  
> +#include <asm/io.h>
> +
>  /* This is all the chips recognized by this library */
>  typedef enum _logical_chip_type_t
>  {
> @@ -70,7 +72,7 @@ logical_chip_type_t getChipType(void);
>  unsigned int calcPllValue(unsigned int request,pll_value_t *pll);
>  unsigned int calcPllValue2(unsigned int,pll_value_t *);
>  unsigned int formatPllReg(pll_value_t *pPLL);
> -void ddk750_set_mmio(volatile unsigned char *,unsigned short,char);
> +void ddk750_set_mmio(volatile unsigned char __iomem *,unsigned short,char);

No need for the volatile.  Really mmio750 should be a "void __iomem *"
it is declared as "unsigned char __iomem *" but it's not a char pointer
that's only to make the pointer math work.  It would work just as well
as a void and we could remove some ugly casting.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] staging: sm750fb: Fix sparse warning
@ 2015-03-10  7:00   ` Dan Carpenter
  0 siblings, 0 replies; 17+ messages in thread
From: Dan Carpenter @ 2015-03-10  7:00 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: sudipm.mukherjee, teddy.wang, gregkh, devel, linux-fbdev, linux-kernel

On Mon, Mar 09, 2015 at 08:57:08PM +0000, Lorenzo Stoakes wrote:
> This patch fixes the following sparse warning:-
> 
> drivers/staging/sm750fb/ddk750_help.c: warning: incorrect type in assignment (different address spaces)
> 
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
> ---
>  drivers/staging/sm750fb/ddk750_chip.h | 4 +++-
>  drivers/staging/sm750fb/ddk750_help.c | 2 +-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_chip.h b/drivers/staging/sm750fb/ddk750_chip.h
> index 1c78875..a4e5bcc 100644
> --- a/drivers/staging/sm750fb/ddk750_chip.h
> +++ b/drivers/staging/sm750fb/ddk750_chip.h
> @@ -3,6 +3,8 @@
>  #define DEFAULT_INPUT_CLOCK 14318181 /* Default reference clock */
>  #define SM750LE_REVISION_ID (char)0xfe
>  
> +#include <asm/io.h>
> +
>  /* This is all the chips recognized by this library */
>  typedef enum _logical_chip_type_t
>  {
> @@ -70,7 +72,7 @@ logical_chip_type_t getChipType(void);
>  unsigned int calcPllValue(unsigned int request,pll_value_t *pll);
>  unsigned int calcPllValue2(unsigned int,pll_value_t *);
>  unsigned int formatPllReg(pll_value_t *pPLL);
> -void ddk750_set_mmio(volatile unsigned char *,unsigned short,char);
> +void ddk750_set_mmio(volatile unsigned char __iomem *,unsigned short,char);

No need for the volatile.  Really mmio750 should be a "void __iomem *"
it is declared as "unsigned char __iomem *" but it's not a char pointer
that's only to make the pointer math work.  It would work just as well
as a void and we could remove some ugly casting.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] staging: sm750fb: Fix sparse warning
  2015-03-09 20:57 ` Lorenzo Stoakes
@ 2015-03-10  7:54   ` Sudip Mukherjee
  -1 siblings, 0 replies; 17+ messages in thread
From: Sudip Mukherjee @ 2015-03-10  7:42 UTC (permalink / raw)
  To: Lorenzo Stoakes; +Cc: teddy.wang, gregkh, linux-fbdev, devel, linux-kernel

On Mon, Mar 09, 2015 at 08:57:08PM +0000, Lorenzo Stoakes wrote:
>  
> +#include <asm/io.h>
> +
apart from what Dan Carpenter has said,
this is introducing one checkpatch warning, better to use #include <linux/io.h>
checkpatch is also giving warning about your patch subject

only one concern - if Greg first applies the patches i sent yesterday then this patch will not apply, and if this one is applied first then my series will not apply ...

regards
sudip

>  /* This is all the chips recognized by this library */
>  	mmio750 = addr;
>  	devId750 = devId;
> -- 
> 2.3.2
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] staging: sm750fb: Fix sparse warning
@ 2015-03-10  7:54   ` Sudip Mukherjee
  0 siblings, 0 replies; 17+ messages in thread
From: Sudip Mukherjee @ 2015-03-10  7:54 UTC (permalink / raw)
  To: Lorenzo Stoakes; +Cc: teddy.wang, gregkh, linux-fbdev, devel, linux-kernel

On Mon, Mar 09, 2015 at 08:57:08PM +0000, Lorenzo Stoakes wrote:
>  
> +#include <asm/io.h>
> +
apart from what Dan Carpenter has said,
this is introducing one checkpatch warning, better to use #include <linux/io.h>
checkpatch is also giving warning about your patch subject

only one concern - if Greg first applies the patches i sent yesterday then this patch will not apply, and if this one is applied first then my series will not apply ...

regards
sudip

>  /* This is all the chips recognized by this library */
>  	mmio750 = addr;
>  	devId750 = devId;
> -- 
> 2.3.2
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] staging: sm750fb: Fix sparse warning
  2015-03-10  7:54   ` Sudip Mukherjee
  (?)
@ 2015-03-10  8:44   ` Lorenzo Stoakes
  -1 siblings, 0 replies; 17+ messages in thread
From: Lorenzo Stoakes @ 2015-03-10  8:44 UTC (permalink / raw)
  To: Sudip Mukherjee; +Cc: teddy.wang, Greg KH, linux-fbdev, devel, linux-kernel

On 10 March 2015 at 07:42, Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote:
> only one concern - if Greg first applies the patches i sent yesterday then this patch will not apply, and if this one is applied first then my series will not apply ...

Indeed, I am more than happy to adapt the patch as required if your
patch series is applied first, I will keep an eye out for this,
additionally if you ping me if this occurs I will fix it right away.

Best,

-- 
Lorenzo Stoakes
https:/ljs.io

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] staging: sm750fb: Fix sparse warning
  2015-03-10  7:00   ` Dan Carpenter
  (?)
@ 2015-03-10  8:47   ` Lorenzo Stoakes
  2015-03-10  8:59       ` Dan Carpenter
  -1 siblings, 1 reply; 17+ messages in thread
From: Lorenzo Stoakes @ 2015-03-10  8:47 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Sudip Mukherjee, teddy.wang, Greg KH, devel, linux-fbdev, linux-kernel

On 10 March 2015 at 07:00, Dan Carpenter <dan.carpenter@oracle.com> wrote:

> No need for the volatile.

Will remove.

> Really mmio750 should be a "void __iomem *"
> it is declared as "unsigned char __iomem *" but it's not a char pointer
> that's only to make the pointer math work.  It would work just as well
> as a void and we could remove some ugly casting.

I'm thinking the change to "void __iomem *" from "unsigned char
__iomem *" ought to be a separate patch in order to keep this cleanly
focused on solving the sparse warning? Am more than happy to do this
and make the appropriate changes to the macros in ddk750_help.h.

-- 
Lorenzo Stoakes
https:/ljs.io

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] staging: sm750fb: Fix sparse warning
  2015-03-10  8:47   ` Lorenzo Stoakes
@ 2015-03-10  8:59       ` Dan Carpenter
  0 siblings, 0 replies; 17+ messages in thread
From: Dan Carpenter @ 2015-03-10  8:59 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: Sudip Mukherjee, teddy.wang, Greg KH, devel, linux-fbdev, linux-kernel

On Tue, Mar 10, 2015 at 08:47:47AM +0000, Lorenzo Stoakes wrote:
> On 10 March 2015 at 07:00, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 
> > No need for the volatile.
> 
> Will remove.
> 
> > Really mmio750 should be a "void __iomem *"
> > it is declared as "unsigned char __iomem *" but it's not a char pointer
> > that's only to make the pointer math work.  It would work just as well
> > as a void and we could remove some ugly casting.
> 
> I'm thinking the change to "void __iomem *" from "unsigned char
> __iomem *" ought to be a separate patch in order to keep this cleanly
> focused on solving the sparse warning? Am more than happy to do this
> and make the appropriate changes to the macros in ddk750_help.h.
> 

The patch is:

[patch] cleanup the type of mmio750

silencing a Sparse warning is just a side benifit of using correct
data types.  The "one thing per patch" rule also means that you should
fix a whole problem instead of half a thing per patch.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] staging: sm750fb: Fix sparse warning
@ 2015-03-10  8:59       ` Dan Carpenter
  0 siblings, 0 replies; 17+ messages in thread
From: Dan Carpenter @ 2015-03-10  8:59 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: Sudip Mukherjee, teddy.wang, Greg KH, devel, linux-fbdev, linux-kernel

On Tue, Mar 10, 2015 at 08:47:47AM +0000, Lorenzo Stoakes wrote:
> On 10 March 2015 at 07:00, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 
> > No need for the volatile.
> 
> Will remove.
> 
> > Really mmio750 should be a "void __iomem *"
> > it is declared as "unsigned char __iomem *" but it's not a char pointer
> > that's only to make the pointer math work.  It would work just as well
> > as a void and we could remove some ugly casting.
> 
> I'm thinking the change to "void __iomem *" from "unsigned char
> __iomem *" ought to be a separate patch in order to keep this cleanly
> focused on solving the sparse warning? Am more than happy to do this
> and make the appropriate changes to the macros in ddk750_help.h.
> 

The patch is:

[patch] cleanup the type of mmio750

silencing a Sparse warning is just a side benifit of using correct
data types.  The "one thing per patch" rule also means that you should
fix a whole problem instead of half a thing per patch.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] staging: sm750fb: Fix sparse warning
  2015-03-10  8:59       ` Dan Carpenter
  (?)
@ 2015-03-10  9:14       ` Lorenzo Stoakes
  -1 siblings, 0 replies; 17+ messages in thread
From: Lorenzo Stoakes @ 2015-03-10  9:14 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Sudip Mukherjee, teddy.wang, Greg KH, devel, linux-fbdev, linux-kernel

On 10 March 2015 at 08:59, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> The patch is:
>
> [patch] cleanup the type of mmio750
>
> silencing a Sparse warning is just a side benifit of using correct
> data types.  The "one thing per patch" rule also means that you should
> fix a whole problem instead of half a thing per patch.
>
> regards,
> dan carpenter
>

Sure, sorry saw the patch as purely a sparse fixup, will adapt to be
an overall mmio750 cleanup and resubmit!

Best,

-- 
Lorenzo Stoakes
https:/ljs.io

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] staging: sm750fb: fix sparse warning for lock
  2015-03-09 20:57 ` Lorenzo Stoakes
@ 2015-08-05 13:26 ` Peng Fan
  -1 siblings, 0 replies; 17+ messages in thread
From: Peng Fan @ 2015-08-05 13:26 UTC (permalink / raw)
  To: sudipm.mukherjee, teddy.wang, gregkh
  Cc: linux-fbdev, devel, linux-kernel, van.freenix

Use __acquire() and __release() in the right place to silence the sparse
lock checking warning.

drivers/staging/sm750fb/sm750.c:177:13: warning: context imbalance in 'lynxfb_ops_fillrect' - different lock contexts for basic block
drivers/staging/sm750fb/sm750.c:243:9: warning: context imbalance in 'lynxfb_ops_copyarea' - different lock contexts for basic block
drivers/staging/sm750fb/sm750.c:247:13: warning: context imbalance in 'lynxfb_ops_imageblit' - different lock contexts for basic block

Signed-off-by: Peng Fan <van.freenix@gmail.com>
Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Cc: Teddy Wang <teddy.wang@siliconmotion.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/staging/sm750fb/sm750.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 8e201f1..5ba1c06 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -203,6 +203,8 @@ static void lynxfb_ops_fillrect(struct fb_info *info,
 	 */
 	if (share->dual)
 		spin_lock(&share->slock);
+	else
+		__acquire(&share->slock);
 
 	share->accel.de_fillrect(&share->accel,
 				 base, pitch, Bpp,
@@ -211,6 +213,8 @@ static void lynxfb_ops_fillrect(struct fb_info *info,
 				 color, rop);
 	if (share->dual)
 		spin_unlock(&share->slock);
+	else
+		__release(&share->slock);
 }
 
 static void lynxfb_ops_copyarea(struct fb_info *info,
@@ -235,6 +239,8 @@ static void lynxfb_ops_copyarea(struct fb_info *info,
 	 */
 	if (share->dual)
 		spin_lock(&share->slock);
+	else
+		__acquire(&share->slock);
 
 	share->accel.de_copyarea(&share->accel,
 				 base, pitch, region->sx, region->sy,
@@ -242,6 +248,8 @@ static void lynxfb_ops_copyarea(struct fb_info *info,
 				 region->width, region->height, HW_ROP2_COPY);
 	if (share->dual)
 		spin_unlock(&share->slock);
+	else
+		__release(&share->slock);
 }
 
 static void lynxfb_ops_imageblit(struct fb_info *info,
@@ -282,6 +290,8 @@ _do_work:
 	 */
 	if (share->dual)
 		spin_lock(&share->slock);
+	else
+		__acquire(&share->slock);
 
 	share->accel.de_imageblit(&share->accel,
 				  image->data, image->width>>3, 0,
@@ -291,6 +301,8 @@ _do_work:
 				  fgcol, bgcol, HW_ROP2_COPY);
 	if (share->dual)
 		spin_unlock(&share->slock);
+	else
+		__release(&share->slock);
 }
 
 static int lynxfb_ops_pan_display(struct fb_var_screeninfo *var,
-- 
1.8.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH] staging: sm750fb: fix sparse warning for lock
@ 2015-08-05 13:26 ` Peng Fan
  0 siblings, 0 replies; 17+ messages in thread
From: Peng Fan @ 2015-08-05 13:26 UTC (permalink / raw)
  To: sudipm.mukherjee, teddy.wang, gregkh
  Cc: linux-fbdev, devel, linux-kernel, van.freenix

Use __acquire() and __release() in the right place to silence the sparse
lock checking warning.

drivers/staging/sm750fb/sm750.c:177:13: warning: context imbalance in 'lynxfb_ops_fillrect' - different lock contexts for basic block
drivers/staging/sm750fb/sm750.c:243:9: warning: context imbalance in 'lynxfb_ops_copyarea' - different lock contexts for basic block
drivers/staging/sm750fb/sm750.c:247:13: warning: context imbalance in 'lynxfb_ops_imageblit' - different lock contexts for basic block

Signed-off-by: Peng Fan <van.freenix@gmail.com>
Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Cc: Teddy Wang <teddy.wang@siliconmotion.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/staging/sm750fb/sm750.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 8e201f1..5ba1c06 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -203,6 +203,8 @@ static void lynxfb_ops_fillrect(struct fb_info *info,
 	 */
 	if (share->dual)
 		spin_lock(&share->slock);
+	else
+		__acquire(&share->slock);
 
 	share->accel.de_fillrect(&share->accel,
 				 base, pitch, Bpp,
@@ -211,6 +213,8 @@ static void lynxfb_ops_fillrect(struct fb_info *info,
 				 color, rop);
 	if (share->dual)
 		spin_unlock(&share->slock);
+	else
+		__release(&share->slock);
 }
 
 static void lynxfb_ops_copyarea(struct fb_info *info,
@@ -235,6 +239,8 @@ static void lynxfb_ops_copyarea(struct fb_info *info,
 	 */
 	if (share->dual)
 		spin_lock(&share->slock);
+	else
+		__acquire(&share->slock);
 
 	share->accel.de_copyarea(&share->accel,
 				 base, pitch, region->sx, region->sy,
@@ -242,6 +248,8 @@ static void lynxfb_ops_copyarea(struct fb_info *info,
 				 region->width, region->height, HW_ROP2_COPY);
 	if (share->dual)
 		spin_unlock(&share->slock);
+	else
+		__release(&share->slock);
 }
 
 static void lynxfb_ops_imageblit(struct fb_info *info,
@@ -282,6 +290,8 @@ _do_work:
 	 */
 	if (share->dual)
 		spin_lock(&share->slock);
+	else
+		__acquire(&share->slock);
 
 	share->accel.de_imageblit(&share->accel,
 				  image->data, image->width>>3, 0,
@@ -291,6 +301,8 @@ _do_work:
 				  fgcol, bgcol, HW_ROP2_COPY);
 	if (share->dual)
 		spin_unlock(&share->slock);
+	else
+		__release(&share->slock);
 }
 
 static int lynxfb_ops_pan_display(struct fb_var_screeninfo *var,
-- 
1.8.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] staging: sm750fb: fix sparse warning for lock
  2015-08-05 13:26 ` Peng Fan
@ 2015-08-05 19:01   ` Greg KH
  -1 siblings, 0 replies; 17+ messages in thread
From: Greg KH @ 2015-08-05 19:01 UTC (permalink / raw)
  To: Peng Fan; +Cc: sudipm.mukherjee, teddy.wang, devel, linux-fbdev, linux-kernel

On Wed, Aug 05, 2015 at 09:26:44PM +0800, Peng Fan wrote:
> Use __acquire() and __release() in the right place to silence the sparse
> lock checking warning.
> 
> drivers/staging/sm750fb/sm750.c:177:13: warning: context imbalance in 'lynxfb_ops_fillrect' - different lock contexts for basic block
> drivers/staging/sm750fb/sm750.c:243:9: warning: context imbalance in 'lynxfb_ops_copyarea' - different lock contexts for basic block
> drivers/staging/sm750fb/sm750.c:247:13: warning: context imbalance in 'lynxfb_ops_imageblit' - different lock contexts for basic block
> 
> Signed-off-by: Peng Fan <van.freenix@gmail.com>
> Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> Cc: Teddy Wang <teddy.wang@siliconmotion.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/staging/sm750fb/sm750.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index 8e201f1..5ba1c06 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -203,6 +203,8 @@ static void lynxfb_ops_fillrect(struct fb_info *info,
>  	 */
>  	if (share->dual)
>  		spin_lock(&share->slock);
> +	else
> +		__acquire(&share->slock);

That's horrid, please don't do stuff like this just to make a static
checker "quiet".

greg k-h

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] staging: sm750fb: fix sparse warning for lock
@ 2015-08-05 19:01   ` Greg KH
  0 siblings, 0 replies; 17+ messages in thread
From: Greg KH @ 2015-08-05 19:01 UTC (permalink / raw)
  To: Peng Fan; +Cc: sudipm.mukherjee, teddy.wang, devel, linux-fbdev, linux-kernel

On Wed, Aug 05, 2015 at 09:26:44PM +0800, Peng Fan wrote:
> Use __acquire() and __release() in the right place to silence the sparse
> lock checking warning.
> 
> drivers/staging/sm750fb/sm750.c:177:13: warning: context imbalance in 'lynxfb_ops_fillrect' - different lock contexts for basic block
> drivers/staging/sm750fb/sm750.c:243:9: warning: context imbalance in 'lynxfb_ops_copyarea' - different lock contexts for basic block
> drivers/staging/sm750fb/sm750.c:247:13: warning: context imbalance in 'lynxfb_ops_imageblit' - different lock contexts for basic block
> 
> Signed-off-by: Peng Fan <van.freenix@gmail.com>
> Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> Cc: Teddy Wang <teddy.wang@siliconmotion.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/staging/sm750fb/sm750.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index 8e201f1..5ba1c06 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -203,6 +203,8 @@ static void lynxfb_ops_fillrect(struct fb_info *info,
>  	 */
>  	if (share->dual)
>  		spin_lock(&share->slock);
> +	else
> +		__acquire(&share->slock);

That's horrid, please don't do stuff like this just to make a static
checker "quiet".

greg k-h

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] staging: sm750fb: fix sparse warning for lock
  2015-08-05 13:26 ` Peng Fan
@ 2015-08-05 22:34   ` Dan Carpenter
  -1 siblings, 0 replies; 17+ messages in thread
From: Dan Carpenter @ 2015-08-05 22:34 UTC (permalink / raw)
  To: Peng Fan
  Cc: sudipm.mukherjee, teddy.wang, gregkh, devel, linux-fbdev, linux-kernel

On Wed, Aug 05, 2015 at 09:26:44PM +0800, Peng Fan wrote:
> Use __acquire() and __release() in the right place to silence the sparse
> lock checking warning.
> 
> drivers/staging/sm750fb/sm750.c:177:13: warning: context imbalance in 'lynxfb_ops_fillrect' - different lock contexts for basic block
> drivers/staging/sm750fb/sm750.c:243:9: warning: context imbalance in 'lynxfb_ops_copyarea' - different lock contexts for basic block
> drivers/staging/sm750fb/sm750.c:247:13: warning: context imbalance in 'lynxfb_ops_imageblit' - different lock contexts for basic block
> 

Sparse is really bad at locking stuff.  Smatch is also really bad for
locking and I have been promising to re-write that check for years, but
I take comfort always in the fact that at least it's not as bad as
Sparse.

You should pretty much ignore Sparse locking warnings.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] staging: sm750fb: fix sparse warning for lock
@ 2015-08-05 22:34   ` Dan Carpenter
  0 siblings, 0 replies; 17+ messages in thread
From: Dan Carpenter @ 2015-08-05 22:34 UTC (permalink / raw)
  To: Peng Fan
  Cc: sudipm.mukherjee, teddy.wang, gregkh, devel, linux-fbdev, linux-kernel

On Wed, Aug 05, 2015 at 09:26:44PM +0800, Peng Fan wrote:
> Use __acquire() and __release() in the right place to silence the sparse
> lock checking warning.
> 
> drivers/staging/sm750fb/sm750.c:177:13: warning: context imbalance in 'lynxfb_ops_fillrect' - different lock contexts for basic block
> drivers/staging/sm750fb/sm750.c:243:9: warning: context imbalance in 'lynxfb_ops_copyarea' - different lock contexts for basic block
> drivers/staging/sm750fb/sm750.c:247:13: warning: context imbalance in 'lynxfb_ops_imageblit' - different lock contexts for basic block
> 

Sparse is really bad at locking stuff.  Smatch is also really bad for
locking and I have been promising to re-write that check for years, but
I take comfort always in the fact that at least it's not as bad as
Sparse.

You should pretty much ignore Sparse locking warnings.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2015-08-05 22:35 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-09 20:57 [PATCH] staging: sm750fb: Fix sparse warning Lorenzo Stoakes
2015-03-09 20:57 ` Lorenzo Stoakes
2015-03-10  7:00 ` Dan Carpenter
2015-03-10  7:00   ` Dan Carpenter
2015-03-10  8:47   ` Lorenzo Stoakes
2015-03-10  8:59     ` Dan Carpenter
2015-03-10  8:59       ` Dan Carpenter
2015-03-10  9:14       ` Lorenzo Stoakes
2015-03-10  7:42 ` Sudip Mukherjee
2015-03-10  7:54   ` Sudip Mukherjee
2015-03-10  8:44   ` Lorenzo Stoakes
2015-08-05 13:26 [PATCH] staging: sm750fb: fix sparse warning for lock Peng Fan
2015-08-05 13:26 ` Peng Fan
2015-08-05 19:01 ` Greg KH
2015-08-05 19:01   ` Greg KH
2015-08-05 22:34 ` Dan Carpenter
2015-08-05 22:34   ` Dan Carpenter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.