LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] staging: sm750fb: Cleanup the type of mmio750
@ 2015-03-10  9:57 Lorenzo Stoakes
  2015-03-10 11:40 ` Dan Carpenter
  0 siblings, 1 reply; 8+ messages in thread
From: Lorenzo Stoakes @ 2015-03-10  9:57 UTC (permalink / raw)
  To: sudipm.mukherjee, teddy.wang, gregkh
  Cc: linux-fbdev, devel, linux-kernel, Lorenzo Stoakes

This patch assigns the more appropriate void* type to the mmio750 variable
eliminating an unnecessary volatile qualifier in the process. Additionally it
updates parameter types as necessary where those parameters interact with
mmio750 and removes unnecessary casts.

As a consequence, this patch fixes the following sparse warning:-

drivers/staging/sm750fb/ddk750_help.c:12:17: 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 |  4 ++--
 drivers/staging/sm750fb/ddk750_help.h | 10 +++++-----
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_chip.h b/drivers/staging/sm750fb/ddk750_chip.h
index 1c78875..d067b06 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 <linux/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(void __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..c68ff3b 100644
--- a/drivers/staging/sm750fb/ddk750_help.c
+++ b/drivers/staging/sm750fb/ddk750_help.c
@@ -2,12 +2,12 @@
 //#include "ddk750_chip.h"
 #include "ddk750_help.h"
 
-volatile unsigned char __iomem * mmio750 = NULL;
+void __iomem * mmio750 = NULL;
 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(void __iomem * addr,unsigned short devId,char revId)
 {
 	mmio750 = addr;
 	devId750 = devId;
diff --git a/drivers/staging/sm750fb/ddk750_help.h b/drivers/staging/sm750fb/ddk750_help.h
index 4fc93b5..07c8264 100644
--- a/drivers/staging/sm750fb/ddk750_help.h
+++ b/drivers/staging/sm750fb/ddk750_help.h
@@ -12,14 +12,14 @@
 #if 0
 /* if 718 big endian turned on,be aware that don't use this driver for general use,only for ppc big-endian */
 #warning "big endian on target cpu and enable nature big endian support of 718 capability !"
-#define PEEK32(addr)  			__raw_readl((void __iomem *)(mmio750)+(addr))
-#define POKE32(addr,data) 		__raw_writel((data),(void __iomem*)(mmio750)+(addr))
+#define PEEK32(addr)  			__raw_readl(mmio750 + addr)
+#define POKE32(addr,data) 		__raw_writel(data, mmio750 + addr)
 #else /* software control endianess */
-#define PEEK32(addr) readl((addr)+mmio750)
-#define POKE32(addr,data) writel((data),(addr)+mmio750)
+#define PEEK32(addr) readl(addr + mmio750)
+#define POKE32(addr,data) writel(data, addr + mmio750)
 #endif
 
-extern volatile unsigned  char __iomem * mmio750;
+extern void __iomem * mmio750;
 extern char revId750;
 extern unsigned short devId750;
 #else
-- 
2.3.2


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

* Re: [PATCH] staging: sm750fb: Cleanup the type of mmio750
  2015-03-10  9:57 [PATCH] staging: sm750fb: Cleanup the type of mmio750 Lorenzo Stoakes
@ 2015-03-10 11:40 ` Dan Carpenter
  2015-03-10 12:36   ` Sudip Mukherjee
  0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2015-03-10 11:40 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: sudipm.mukherjee, teddy.wang, gregkh, devel, linux-fbdev, linux-kernel

On Tue, Mar 10, 2015 at 09:57:06AM +0000, Lorenzo Stoakes wrote:
> This patch assigns the more appropriate void* type to the mmio750 variable
> eliminating an unnecessary volatile qualifier in the process. Additionally it
> updates parameter types as necessary where those parameters interact with
> mmio750 and removes unnecessary casts.
> 
> As a consequence, this patch fixes the following sparse warning:-
> 
> drivers/staging/sm750fb/ddk750_help.c:12:17: warning: incorrect type in assignment (different address spaces)
> 
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>

Looks good.  Thanks for doing this.

regards,
dan carpenter


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

* Re: [PATCH] staging: sm750fb: Cleanup the type of mmio750
  2015-03-10 11:40 ` Dan Carpenter
@ 2015-03-10 12:36   ` Sudip Mukherjee
  2015-03-10 12:47     ` Lorenzo Stoakes
  0 siblings, 1 reply; 8+ messages in thread
From: Sudip Mukherjee @ 2015-03-10 12:36 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Lorenzo Stoakes, teddy.wang, gregkh, devel, linux-fbdev, linux-kernel

On Tue, Mar 10, 2015 at 02:40:30PM +0300, Dan Carpenter wrote:
> On Tue, Mar 10, 2015 at 09:57:06AM +0000, Lorenzo Stoakes wrote:
> > This patch assigns the more appropriate void* type to the mmio750 variable
> > eliminating an unnecessary volatile qualifier in the process. Additionally it
> > updates parameter types as necessary where those parameters interact with
> > mmio750 and removes unnecessary casts.
> > 
> > As a consequence, this patch fixes the following sparse warning:-
> > 
> > drivers/staging/sm750fb/ddk750_help.c:12:17: warning: incorrect type in assignment (different address spaces)
> > 
> > Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
> 
> Looks good.  Thanks for doing this.

but it is introducing two new build warnings:

drivers/staging/sm750fb/sm750_hw.c: In function ‘hw_sm750_map’:
drivers/staging/sm750fb/sm750_hw.c:67:2: warning: passing argument 1 of ‘ddk750_set_mmio’ discards ‘volatile’ qualifier from pointer target type [enabled by default]
In file included from drivers/staging/sm750fb/ddk750_mode.h:4:0,
                    from drivers/staging/sm750fb/ddk750.h:15,
                    from drivers/staging/sm750fb/sm750_hw.c:24:

and

drivers/staging/sm750fb/ddk750_chip.h:77:6: note: expected ‘void *’ but argument is of type ‘volatile unsigned char *’

care to make another patch to solve these two new warnings, and send this patch and the new one in a series and while sending mark the version number in the subject.

regards
sudip
> 
> regards,
> dan carpenter
> 

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

* Re: [PATCH] staging: sm750fb: Cleanup the type of mmio750
  2015-03-10 12:36   ` Sudip Mukherjee
@ 2015-03-10 12:47     ` Lorenzo Stoakes
  2015-03-10 13:06       ` Dan Carpenter
  2015-03-10 15:04       ` Greg KH
  0 siblings, 2 replies; 8+ messages in thread
From: Lorenzo Stoakes @ 2015-03-10 12:47 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Dan Carpenter, teddy.wang, Greg KH, devel, linux-fbdev, linux-kernel

On 10 March 2015 at 12:36, Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote:
> but it is introducing two new build warnings:
>
> drivers/staging/sm750fb/sm750_hw.c: In function ‘hw_sm750_map’:
> drivers/staging/sm750fb/sm750_hw.c:67:2: warning: passing argument 1 of ‘ddk750_set_mmio’ discards ‘volatile’ qualifier from pointer target type [enabled by default]
> In file included from drivers/staging/sm750fb/ddk750_mode.h:4:0,
>                     from drivers/staging/sm750fb/ddk750.h:15,
>                     from drivers/staging/sm750fb/sm750_hw.c:24:
>
> and
>
> drivers/staging/sm750fb/ddk750_chip.h:77:6: note: expected ‘void *’ but argument is of type ‘volatile unsigned char *’
>
> care to make another patch to solve these two new warnings, and send this patch and the new one in a series and while sending mark the version number in the subject.

I think the second warning is simply additional information attached
to the 1st to give context?

I noticed this issue but felt changing the type of this field would
sit outside the purview of this patch as then I'm not only changing
the type of mmio750 and code that *directly* interacts with this
variable, but also code that indirectly interacts with it, so I felt
that should perhaps be a separate patch.

I'd love to additionally provide some further patches to help out with
issues here too, incidentally! I will try to prepare some further
patches tonight in this vein.

-- 
Lorenzo Stoakes
https:/ljs.io

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

* Re: [PATCH] staging: sm750fb: Cleanup the type of mmio750
  2015-03-10 12:47     ` Lorenzo Stoakes
@ 2015-03-10 13:06       ` Dan Carpenter
  2015-03-10 13:22         ` Lorenzo Stoakes
  2015-03-10 15:04       ` Greg KH
  1 sibling, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2015-03-10 13:06 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: Sudip Mukherjee, teddy.wang, Greg KH, devel, linux-fbdev, linux-kernel

On Tue, Mar 10, 2015 at 12:47:44PM +0000, Lorenzo Stoakes wrote:
> On 10 March 2015 at 12:36, Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote:
> > but it is introducing two new build warnings:
> >
> > drivers/staging/sm750fb/sm750_hw.c: In function ‘hw_sm750_map’:
> > drivers/staging/sm750fb/sm750_hw.c:67:2: warning: passing argument 1 of ‘ddk750_set_mmio’ discards ‘volatile’ qualifier from pointer target type [enabled by default]
> > In file included from drivers/staging/sm750fb/ddk750_mode.h:4:0,
> >                     from drivers/staging/sm750fb/ddk750.h:15,
> >                     from drivers/staging/sm750fb/sm750_hw.c:24:
> >
> > and
> >
> > drivers/staging/sm750fb/ddk750_chip.h:77:6: note: expected ‘void *’ but argument is of type ‘volatile unsigned char *’
> >
> > care to make another patch to solve these two new warnings, and send this patch and the new one in a series and while sending mark the version number in the subject.
> 
> I think the second warning is simply additional information attached
> to the 1st to give context?
> 
> I noticed this issue but felt changing the type of this field would
> sit outside the purview of this patch as then I'm not only changing
> the type of mmio750 and code that *directly* interacts with this
> variable, but also code that indirectly interacts with it, so I felt
> that should perhaps be a separate patch.

You should have said that in the patch description or under the ---
cut off.  But anyway, it's not ok.  And we'll need to redo this patch.
Breaking up patches into logical changes is sort of tricky because
everything touches everything else so the patch gets larger and larger.

You could maybe break it up:

[patch 1/2] staging: sm750fb: Cleanup the type of mmio750
	This would add some temporary casting until the rest of the
	code was cleaned up.  It wouldn't touch change the function
	parameters of ddk750_set_mmio().
[patch 2/2] staging: sm750fb: Cleanup the types for ddk750_set_mmio()
	This would change the function paramters and the type for
	->pvReg and remove the temporary casts.

But maybe it's only one line larger than the patch you just send?  In
that case just fold it in and don't do the temporary casting.

The next patch after that could get rid of all the ramaining "volatile"
keywords.

regards,
dan carpenter


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

* Re: [PATCH] staging: sm750fb: Cleanup the type of mmio750
  2015-03-10 13:06       ` Dan Carpenter
@ 2015-03-10 13:22         ` Lorenzo Stoakes
  0 siblings, 0 replies; 8+ messages in thread
From: Lorenzo Stoakes @ 2015-03-10 13:22 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Sudip Mukherjee, teddy.wang, Greg KH, devel, linux-fbdev, linux-kernel

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

> You should have said that in the patch description or under the ---
> cut off.  But anyway, it's not ok.  And we'll need to redo this patch.
> Breaking up patches into logical changes is sort of tricky because
> everything touches everything else so the patch gets larger and larger.
>

Major apologies, I am still getting used to kernel development! I'll
be careful to not make such assumptions in future when it comes to
warnings/errors.

[snip]

> But maybe it's only one line larger than the patch you just send?  In
> that case just fold it in and don't do the temporary casting.
>
> The next patch after that could get rid of all the ramaining "volatile"
> keywords.

It seems that we can in fact fix this problem with a single additional
change, I will submit a v2 shortly.

Best,

-- 
Lorenzo Stoakes
https:/ljs.io

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

* Re: [PATCH] staging: sm750fb: Cleanup the type of mmio750
  2015-03-10 12:47     ` Lorenzo Stoakes
  2015-03-10 13:06       ` Dan Carpenter
@ 2015-03-10 15:04       ` Greg KH
  2015-03-10 15:08         ` Lorenzo Stoakes
  1 sibling, 1 reply; 8+ messages in thread
From: Greg KH @ 2015-03-10 15:04 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: Sudip Mukherjee, devel, linux-fbdev, teddy.wang, linux-kernel,
	Dan Carpenter

On Tue, Mar 10, 2015 at 12:47:44PM +0000, Lorenzo Stoakes wrote:
> On 10 March 2015 at 12:36, Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote:
> > but it is introducing two new build warnings:
> >
> > drivers/staging/sm750fb/sm750_hw.c: In function ‘hw_sm750_map’:
> > drivers/staging/sm750fb/sm750_hw.c:67:2: warning: passing argument 1 of ‘ddk750_set_mmio’ discards ‘volatile’ qualifier from pointer target type [enabled by default]
> > In file included from drivers/staging/sm750fb/ddk750_mode.h:4:0,
> >                     from drivers/staging/sm750fb/ddk750.h:15,
> >                     from drivers/staging/sm750fb/sm750_hw.c:24:
> >
> > and
> >
> > drivers/staging/sm750fb/ddk750_chip.h:77:6: note: expected ‘void *’ but argument is of type ‘volatile unsigned char *’
> >
> > care to make another patch to solve these two new warnings, and send this patch and the new one in a series and while sending mark the version number in the subject.
> 
> I think the second warning is simply additional information attached
> to the 1st to give context?
> 
> I noticed this issue but felt changing the type of this field would
> sit outside the purview of this patch as then I'm not only changing
> the type of mmio750 and code that *directly* interacts with this
> variable, but also code that indirectly interacts with it, so I felt
> that should perhaps be a separate patch.
> 
> I'd love to additionally provide some further patches to help out with
> issues here too, incidentally! I will try to prepare some further
> patches tonight in this vein.

I can't apply patches that add new build warnings, sorry.  Please fix
this up in the patch itself.

greg k-h

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

* Re: [PATCH] staging: sm750fb: Cleanup the type of mmio750
  2015-03-10 15:04       ` Greg KH
@ 2015-03-10 15:08         ` Lorenzo Stoakes
  0 siblings, 0 replies; 8+ messages in thread
From: Lorenzo Stoakes @ 2015-03-10 15:08 UTC (permalink / raw)
  To: Greg KH
  Cc: Sudip Mukherjee, devel, linux-fbdev, teddy.wang, linux-kernel,
	Dan Carpenter

On 10 March 2015 at 15:04, Greg KH <gregkh@linuxfoundation.org> wrote:
> I can't apply patches that add new build warnings, sorry.  Please fix
> this up in the patch itself.
>
> greg k-h

Hi Greg,

Apologies for this, I've resolved this issue in v2 of the patch, no
warning messages are added in the updated version of this patch.

Best,

-- 
Lorenzo Stoakes
https:/ljs.io

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

end of thread, other threads:[~2015-03-10 15:08 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-10  9:57 [PATCH] staging: sm750fb: Cleanup the type of mmio750 Lorenzo Stoakes
2015-03-10 11:40 ` Dan Carpenter
2015-03-10 12:36   ` Sudip Mukherjee
2015-03-10 12:47     ` Lorenzo Stoakes
2015-03-10 13:06       ` Dan Carpenter
2015-03-10 13:22         ` Lorenzo Stoakes
2015-03-10 15:04       ` Greg KH
2015-03-10 15:08         ` Lorenzo Stoakes

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).