diff mbox

[v3,7/8] video: xilinxfb: Fix sparse warnings

Message ID ea3d6fcbb9f7a00a9cdbd3d0a7a4d3401265ebf0.1370004925.git.michal.simek@xilinx.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michal Simek May 31, 2013, 12:55 p.m. UTC 
Use proper casting for fb_virt variable.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Changes in v3:
- New patch in this patchset

Changes in v2: None

 drivers/video/xilinxfb.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

--
1.8.2.3

Comments

Timur Tabi May 31, 2013, 1:26 p.m. UTC | #1 
On 05/31/2013 07:55 AM, Michal Simek wrote:

> diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
> index f3d4a69..e27a4f6 100644
> --- a/drivers/video/xilinxfb.c
> +++ b/drivers/video/xilinxfb.c
> @@ -131,7 +131,7 @@ struct xilinxfb_drvdata {
>  	dcr_host_t      dcr_host;
>  	unsigned int    dcr_len;
>  #endif
> -	void		*fb_virt;	/* virt. address of the frame buffer */
> +	void __iomem	*fb_virt;	/* virt. address of the frame buffer */
>  	dma_addr_t	fb_phys;	/* phys. address of the frame buffer */
>  	int		fb_alloced;	/* Flag, was the fb memory alloced? */
> 
> @@ -273,8 +273,10 @@ static int xilinxfb_assign(struct platform_device *pdev,
>  		drvdata->fb_virt = ioremap(pdata->fb_phys, fbsize);
>  	} else {
>  		drvdata->fb_alloced = 1;
> -		drvdata->fb_virt = dma_alloc_coherent(dev, PAGE_ALIGN(fbsize),
> -					&drvdata->fb_phys, GFP_KERNEL);
> +		drvdata->fb_virt = (__force void __iomem *)
> +				   dma_alloc_coherent(dev, PAGE_ALIGN(fbsize),
> +						      &drvdata->fb_phys,
> +						      GFP_KERNEL);

I think this is wrong.  At least, it would be on PowerPC.
dma_alloc_coherent() allocates regular RAM, not I/O memory.  So it
should not be __iomem.
Michal Simek May 31, 2013, 1:37 p.m. UTC | #2 
On 05/31/2013 03:26 PM, Timur Tabi wrote:
> On 05/31/2013 07:55 AM, Michal Simek wrote:
> 
>> diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
>> index f3d4a69..e27a4f6 100644
>> --- a/drivers/video/xilinxfb.c
>> +++ b/drivers/video/xilinxfb.c
>> @@ -131,7 +131,7 @@ struct xilinxfb_drvdata {
>>  	dcr_host_t      dcr_host;
>>  	unsigned int    dcr_len;
>>  #endif
>> -	void		*fb_virt;	/* virt. address of the frame buffer */
>> +	void __iomem	*fb_virt;	/* virt. address of the frame buffer */
>>  	dma_addr_t	fb_phys;	/* phys. address of the frame buffer */
>>  	int		fb_alloced;	/* Flag, was the fb memory alloced? */
>>
>> @@ -273,8 +273,10 @@ static int xilinxfb_assign(struct platform_device *pdev,
>>  		drvdata->fb_virt = ioremap(pdata->fb_phys, fbsize);
>>  	} else {
>>  		drvdata->fb_alloced = 1;
>> -		drvdata->fb_virt = dma_alloc_coherent(dev, PAGE_ALIGN(fbsize),
>> -					&drvdata->fb_phys, GFP_KERNEL);
>> +		drvdata->fb_virt = (__force void __iomem *)
>> +				   dma_alloc_coherent(dev, PAGE_ALIGN(fbsize),
>> +						      &drvdata->fb_phys,
>> +						      GFP_KERNEL);
> 
> I think this is wrong.  At least, it would be on PowerPC.
> dma_alloc_coherent() allocates regular RAM, not I/O memory.  So it
> should not be __iomem.

The same is for Microblaze. Driver shares fb_virt for IO memory
and for allocated memory. The purpose of this driver wasn't
to change the driver logic just resolved sparse warnings.
The other way is also wrong.
I have compiled this driver with ppc toolchain and it should
remove sparse warnings for PPC.

Thanks,
Michal
Timur Tabi May 31, 2013, 1:41 p.m. UTC | #3 
On 05/31/2013 08:37 AM, Michal Simek wrote:
> The same is for Microblaze. Driver shares fb_virt for IO memory
> and for allocated memory. The purpose of this driver wasn't
> to change the driver logic just resolved sparse warnings.
> The other way is also wrong.
> I have compiled this driver with ppc toolchain and it should
> remove sparse warnings for PPC.

But it's not I/O memory.  It's regular memory.  __iomem is for
memory-mapped I/O, which is limited to a specific range of memory locations.

If sometimes you use regular memory for the framebuffer, and other times
you use real I/O memory for the framebuffer, then you should have two
different pointers.
diff mbox

Patch

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index f3d4a69..e27a4f6 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -131,7 +131,7 @@  struct xilinxfb_drvdata {
 	dcr_host_t      dcr_host;
 	unsigned int    dcr_len;
 #endif
-	void		*fb_virt;	/* virt. address of the frame buffer */
+	void __iomem	*fb_virt;	/* virt. address of the frame buffer */
 	dma_addr_t	fb_phys;	/* phys. address of the frame buffer */
 	int		fb_alloced;	/* Flag, was the fb memory alloced? */

@@ -273,8 +273,10 @@  static int xilinxfb_assign(struct platform_device *pdev,
 		drvdata->fb_virt = ioremap(pdata->fb_phys, fbsize);
 	} else {
 		drvdata->fb_alloced = 1;
-		drvdata->fb_virt = dma_alloc_coherent(dev, PAGE_ALIGN(fbsize),
-					&drvdata->fb_phys, GFP_KERNEL);
+		drvdata->fb_virt = (__force void __iomem *)
+				   dma_alloc_coherent(dev, PAGE_ALIGN(fbsize),
+						      &drvdata->fb_phys,
+						      GFP_KERNEL);
 	}

 	if (!drvdata->fb_virt) {
@@ -287,7 +289,7 @@  static int xilinxfb_assign(struct platform_device *pdev,
 	}

 	/* Clear (turn to black) the framebuffer */
-	memset_io((void __iomem *)drvdata->fb_virt, 0, fbsize);
+	memset_io(drvdata->fb_virt, 0, fbsize);

 	/* Tell the hardware where the frame buffer is */
 	xilinx_fb_out32(drvdata, REG_FB_ADDR, drvdata->fb_phys);
@@ -307,7 +309,7 @@  static int xilinxfb_assign(struct platform_device *pdev,

 	/* Fill struct fb_info */
 	drvdata->info.device = dev;
-	drvdata->info.screen_base = (void __iomem *)drvdata->fb_virt;
+	drvdata->info.screen_base = drvdata->fb_virt;
 	drvdata->info.fbops = &xilinxfb_ops;
 	drvdata->info.fix = xilinx_fb_fix;
 	drvdata->info.fix.smem_start = drvdata->fb_phys;
@@ -341,8 +343,8 @@  static int xilinxfb_assign(struct platform_device *pdev,

 	if (drvdata->flags & BUS_ACCESS_FLAG) {
 		/* Put a banner in the log (for DEBUG) */
-		dev_dbg(dev, "regs: phys=%x, virt=%p\n", drvdata->regs_phys,
-					drvdata->regs);
+		dev_dbg(dev, "regs: phys=%x, virt=%p\n",
+			(u32)drvdata->regs_phys, drvdata->regs);
 	}
 	/* Put a banner in the log (for DEBUG) */
 	dev_dbg(dev, "fb: phys=%llx, virt=%p, size=%x\n",
@@ -355,8 +357,9 @@  err_regfb:

 err_cmap:
 	if (drvdata->fb_alloced)
-		dma_free_coherent(dev, PAGE_ALIGN(fbsize), drvdata->fb_virt,
-			drvdata->fb_phys);
+		dma_free_coherent(dev, PAGE_ALIGN(fbsize),
+				  (__force void *)drvdata->fb_virt,
+				  drvdata->fb_phys);
 	else
 		iounmap(drvdata->fb_virt);

@@ -388,7 +391,8 @@  static int xilinxfb_release(struct device *dev)

 	if (drvdata->fb_alloced)
 		dma_free_coherent(dev, PAGE_ALIGN(drvdata->info.fix.smem_len),
-				  drvdata->fb_virt, drvdata->fb_phys);
+				  (__force void *)drvdata->fb_virt,
+				  drvdata->fb_phys);
 	else
 		iounmap(drvdata->fb_virt);