[PATCH] staging: sm750fb: Fix sparse warning

[PATCH] staging: sm750fb: Fix sparse warning

[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

Re: [PATCH] staging: sm750fb: Fix sparse warning

[Dan Carpenter]

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

Re: [PATCH] staging: sm750fb: Fix sparse warning

[Lorenzo Stoakes]

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

Re: [PATCH] staging: sm750fb: Fix sparse warning

[Dan Carpenter]

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

Re: [PATCH] staging: sm750fb: Fix sparse warning

[Lorenzo Stoakes]

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

Re: [PATCH] staging: sm750fb: Fix sparse warning

[Sudip Mukherjee]

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
> 

Re: [PATCH] staging: sm750fb: Fix sparse warning

[Lorenzo Stoakes]

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