Re: [PATCH 1/4] staging: sm750fb: wrong type for print

Re: [PATCH 1/4] staging: sm750fb: wrong type for print

[Giedrius Statkevičius]

On 2015.03.08 14:31, Sudip Mukherjee wrote:
> mention correct format specifier while printing
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> 
> this patch will give checkpatch warnings about use of printk.
> this patch was mainly to fix the build warnings. printk will be
> converted to pr_* and dev_* in a later patch.
> 
>  drivers/staging/sm750fb/sm750.c    | 24 ++++++++++++------------
>  drivers/staging/sm750fb/sm750.h    |  8 ++++----
>  drivers/staging/sm750fb/sm750_hw.c |  4 ++--
>  3 files changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index 520c69e..753869e 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -530,20 +530,20 @@ static int lynxfb_ops_mmap(struct fb_info * info, struct vm_area_struct * vma)
>  	if (vma->vm_pgoff > (~0UL >> PAGE_SHIFT))
>  		return -EINVAL;
>  	off = vma->vm_pgoff << PAGE_SHIFT;
> -	printk("lynxfb mmap pgoff: %x\n", vma->vm_pgoff);
> -	printk("lynxfb mmap off 1: %x\n", off);
> +	printk("lynxfb mmap pgoff: %lx\n", vma->vm_pgoff);
> +	printk("lynxfb mmap off 1: %lx\n", off);
>  	
>  	/* frame buffer memory */
>  	start = info->fix.smem_start;
>  	len = PAGE_ALIGN((start & ~PAGE_MASK) + info->fix.smem_len);
>  	
> -	printk("lynxfb mmap start 1: %x\n", start);
> +	printk("lynxfb mmap start 1: %lx\n", start);
>  	printk("lynxfb mmap len 1: %x\n", len);
>  	
>  	if (off >= len) {
>  		/* memory mapped io */
>  		off -= len;
> -		printk("lynxfb mmap off 2: %x\n", off);
> +		printk("lynxfb mmap off 2: %lx\n", off);
>  		if (info->var.accel_flags) {
>  			printk("lynxfb mmap accel flags true");
>  			return -EINVAL;
> @@ -551,28 +551,28 @@ static int lynxfb_ops_mmap(struct fb_info * info, struct vm_area_struct * vma)
>  		start = info->fix.mmio_start;
>  		len = PAGE_ALIGN((start & ~PAGE_MASK) + info->fix.mmio_len);
>  		
> -		printk("lynxfb mmap start 2: %x\n", start);
> +		printk("lynxfb mmap start 2: %lx\n", start);
>  		printk("lynxfb mmap len 2: %x\n", len);
>  	}
>  	start &= PAGE_MASK;
> -	printk("lynxfb mmap start 3: %x\n", start);
> -	printk("lynxfb mmap vm start: %x\n", vma->vm_start);
> -	printk("lynxfb mmap vm end: %x\n", vma->vm_end);
> +	printk("lynxfb mmap start 3: %lx\n", start);
> +	printk("lynxfb mmap vm start: %lx\n", vma->vm_start);
> +	printk("lynxfb mmap vm end: %lx\n", vma->vm_end);
>  	printk("lynxfb mmap len: %x\n", len);
> -	printk("lynxfb mmap off: %x\n", off);
> +	printk("lynxfb mmap off: %lx\n", off);
>  	if ((vma->vm_end - vma->vm_start + off) > len)
>  	{
>  		return -EINVAL;
>  	}
>  	off += start;
> -	printk("lynxfb mmap off 3: %x\n", off);
> +	printk("lynxfb mmap off 3: %lx\n", off);
>  	vma->vm_pgoff = off >> PAGE_SHIFT;
>  	/* This is an IO map - tell maydump to skip this VMA */
>  	vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
>  	vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
>  	fb_pgprotect(file, vma, off);
> -	printk("lynxfb mmap off 4: %x\n", off);
> -	printk("lynxfb mmap pgprot: %x\n", vma->vm_page_prot);
> +	printk("lynxfb mmap off 4: %lx\n", off);
> +	printk("lynxfb mmap pgprot: %lx\n", (unsigned long) pgprot_val(vma->vm_page_prot));
>  	if (io_remap_pfn_range(vma, vma->vm_start, off >> PAGE_SHIFT,
>  			     vma->vm_end - vma->vm_start, vma->vm_page_prot))
>  		return -EAGAIN;
> diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
> index 711676c..2ab7b74 100644
> --- a/drivers/staging/sm750fb/sm750.h
> +++ b/drivers/staging/sm750fb/sm750.h
> @@ -59,10 +59,10 @@ struct lynx_share{
>  		}mtrr;
>  #endif
>  	/* all smi graphic adaptor got below attributes */
> -	resource_size_t vidmem_start;
> -	resource_size_t vidreg_start;
> -	resource_size_t vidmem_size;
> -	resource_size_t vidreg_size;
> +	unsigned long vidmem_start;
> +	unsigned long vidreg_start;
> +	unsigned long vidmem_size;
> +	unsigned long vidreg_size;

Have you checked other places where these are used? resource_size_t can
be either u64 or u32 depending on if CONFIG_PHYS_ADDR_T_64BIT is
#defined. Are you sure you aren't losing information when results of
functions are being assigned to this? Maybe there should be a function
similar to printk that changes between %u and %llu depending on whether
that is defined?

-- 
Thanks,
Giedrius

[PATCH 1/4] staging: sm750fb: wrong type for print

[Sudip Mukherjee]

mention correct format specifier while printing

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

this patch will give checkpatch warnings about use of printk.
this patch was mainly to fix the build warnings. printk will be
converted to pr_* and dev_* in a later patch.

 drivers/staging/sm750fb/sm750.c    | 24 ++++++++++++------------
 drivers/staging/sm750fb/sm750.h    |  8 ++++----
 drivers/staging/sm750fb/sm750_hw.c |  4 ++--
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 520c69e..753869e 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -530,20 +530,20 @@ static int lynxfb_ops_mmap(struct fb_info * info, struct vm_area_struct * vma)
 	if (vma->vm_pgoff > (~0UL >> PAGE_SHIFT))
 		return -EINVAL;
 	off = vma->vm_pgoff << PAGE_SHIFT;
-	printk("lynxfb mmap pgoff: %x\n", vma->vm_pgoff);
-	printk("lynxfb mmap off 1: %x\n", off);
+	printk("lynxfb mmap pgoff: %lx\n", vma->vm_pgoff);
+	printk("lynxfb mmap off 1: %lx\n", off);
 	
 	/* frame buffer memory */
 	start = info->fix.smem_start;
 	len = PAGE_ALIGN((start & ~PAGE_MASK) + info->fix.smem_len);
 	
-	printk("lynxfb mmap start 1: %x\n", start);
+	printk("lynxfb mmap start 1: %lx\n", start);
 	printk("lynxfb mmap len 1: %x\n", len);
 	
 	if (off >= len) {
 		/* memory mapped io */
 		off -= len;
-		printk("lynxfb mmap off 2: %x\n", off);
+		printk("lynxfb mmap off 2: %lx\n", off);
 		if (info->var.accel_flags) {
 			printk("lynxfb mmap accel flags true");
 			return -EINVAL;
@@ -551,28 +551,28 @@ static int lynxfb_ops_mmap(struct fb_info * info, struct vm_area_struct * vma)
 		start = info->fix.mmio_start;
 		len = PAGE_ALIGN((start & ~PAGE_MASK) + info->fix.mmio_len);
 		
-		printk("lynxfb mmap start 2: %x\n", start);
+		printk("lynxfb mmap start 2: %lx\n", start);
 		printk("lynxfb mmap len 2: %x\n", len);
 	}
 	start &= PAGE_MASK;
-	printk("lynxfb mmap start 3: %x\n", start);
-	printk("lynxfb mmap vm start: %x\n", vma->vm_start);
-	printk("lynxfb mmap vm end: %x\n", vma->vm_end);
+	printk("lynxfb mmap start 3: %lx\n", start);
+	printk("lynxfb mmap vm start: %lx\n", vma->vm_start);
+	printk("lynxfb mmap vm end: %lx\n", vma->vm_end);
 	printk("lynxfb mmap len: %x\n", len);
-	printk("lynxfb mmap off: %x\n", off);
+	printk("lynxfb mmap off: %lx\n", off);
 	if ((vma->vm_end - vma->vm_start + off) > len)
 	{
 		return -EINVAL;
 	}
 	off += start;
-	printk("lynxfb mmap off 3: %x\n", off);
+	printk("lynxfb mmap off 3: %lx\n", off);
 	vma->vm_pgoff = off >> PAGE_SHIFT;
 	/* This is an IO map - tell maydump to skip this VMA */
 	vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
 	vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
 	fb_pgprotect(file, vma, off);
-	printk("lynxfb mmap off 4: %x\n", off);
-	printk("lynxfb mmap pgprot: %x\n", vma->vm_page_prot);
+	printk("lynxfb mmap off 4: %lx\n", off);
+	printk("lynxfb mmap pgprot: %lx\n", (unsigned long) pgprot_val(vma->vm_page_prot));
 	if (io_remap_pfn_range(vma, vma->vm_start, off >> PAGE_SHIFT,
 			     vma->vm_end - vma->vm_start, vma->vm_page_prot))
 		return -EAGAIN;
diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
index 711676c..2ab7b74 100644
--- a/drivers/staging/sm750fb/sm750.h
+++ b/drivers/staging/sm750fb/sm750.h
@@ -59,10 +59,10 @@ struct lynx_share{
 		}mtrr;
 #endif
 	/* all smi graphic adaptor got below attributes */
-	resource_size_t vidmem_start;
-	resource_size_t vidreg_start;
-	resource_size_t vidmem_size;
-	resource_size_t vidreg_size;
+	unsigned long vidmem_start;
+	unsigned long vidreg_start;
+	unsigned long vidmem_size;
+	unsigned long vidreg_size;
 	volatile unsigned char __iomem * pvReg;
 	unsigned char __iomem * pvMem;
 	/* locks*/
diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index cd971bd..5a62721 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -36,7 +36,7 @@ int hw_sm750_map(struct lynx_share* share,struct pci_dev* pdev)
 	share->vidreg_start  = pci_resource_start(pdev,1);
 	share->vidreg_size = MB(2);
 
-	pr_info("mmio phyAddr = %x\n",share->vidreg_start);
+	pr_info("mmio phyAddr = %lx\n", share->vidreg_start);
 
 	/* reserve the vidreg space of smi adaptor
 	 * if you do this, u need to add release region code
@@ -73,7 +73,7 @@ int hw_sm750_map(struct lynx_share* share,struct pci_dev* pdev)
 	 * @hw_sm750_getVMSize function can be safe.
 	 * */
 	share->vidmem_size = hw_sm750_getVMSize(share);
-	pr_info("video memory phyAddr = %x, size = %d bytes\n",
+	pr_info("video memory phyAddr = %lx, size = %ld bytes\n",
 	share->vidmem_start,share->vidmem_size);
 
 	/* reserve the vidmem space of smi adaptor */
-- 
1.8.1.2

[PATCH 2/4] staging: sm750fb: remove pragma optimize

[Sudip Mukherjee]

remove use of #pragma optimize which will usually be ignored by the
compiler.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/sm750fb/ddk750_swi2c.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_swi2c.c b/drivers/staging/sm750fb/ddk750_swi2c.c
index b53407b..cae6b9b 100644
--- a/drivers/staging/sm750fb/ddk750_swi2c.c
+++ b/drivers/staging/sm750fb/ddk750_swi2c.c
@@ -217,8 +217,6 @@ static unsigned char swI2CReadSDA(void)
         return 0;
 }
 
-#pragma optimize( "", off )
-
 /*
  *  This function sends ACK signal
  */
@@ -356,7 +354,6 @@ unsigned char swI2CReadByte(unsigned char ack)
 
     return data;
 }
-#pragma optimize( "", on )
 
 /*
  * This function initializes GPIO port for SW I2C communication.
-- 
1.8.1.2

[PATCH 3/4] staging: sm750fb: correctly define SM750LE_REVISION_ID

[Sudip Mukherjee]

check if it is already defined before defining SM750LE_REVISION_ID
again and at the same time mention correct data type.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/sm750fb/ddk750_chip.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/sm750fb/ddk750_chip.h b/drivers/staging/sm750fb/ddk750_chip.h
index 1c78875..e7d27e8 100644
--- a/drivers/staging/sm750fb/ddk750_chip.h
+++ b/drivers/staging/sm750fb/ddk750_chip.h
@@ -1,7 +1,9 @@
 #ifndef DDK750_CHIP_H__
 #define DDK750_CHIP_H__
 #define DEFAULT_INPUT_CLOCK 14318181 /* Default reference clock */
-#define SM750LE_REVISION_ID (char)0xfe
+#ifndef SM750LE_REVISION_ID
+#define SM750LE_REVISION_ID ((unsigned char)0xfe)
+#endif
 
 /* This is all the chips recognized by this library */
 typedef enum _logical_chip_type_t
-- 
1.8.1.2

[PATCH 4/4] staging: sm750fb: fix undeclared function

[Sudip Mukherjee]

kbuild test robot reported that for microblaze-allyesconfig
chan_to_field() and lynxfb_ops_set_par() were not defined. These two
functions were defined under CONFIG_PM, so for any archtecture if
CONFIG_PM is not defined we will have this error.
while moving the lynxfb_suspend() function some very obvious
checkpatch errors were taken care of.

Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/sm750fb/sm750.c | 109 +++++++++++++++++++---------------------
 1 file changed, 52 insertions(+), 57 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 753869e..8a78ea5 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -303,62 +303,6 @@ static int lynxfb_ops_pan_display(struct fb_var_screeninfo *var,
     return ret;
 }
 
-
-
-
-#ifdef CONFIG_PM
-static int lynxfb_suspend(struct pci_dev * pdev,pm_message_t mesg)
-{
-	struct fb_info * info;
-	struct lynx_share * share;
-	int ret;
-	
-
-	if(mesg.event = pdev->dev.power.power_state.event)
-		return 0;
-
-	ret = 0;
-	share = pci_get_drvdata(pdev);
-	switch (mesg.event) {
-	case PM_EVENT_FREEZE:
-	case PM_EVENT_PRETHAW:
-		pdev->dev.power.power_state = mesg;
-		return 0;
-	}
-
-	console_lock();
-	if (mesg.event & PM_EVENT_SLEEP) {
-		info = share->fbinfo[0];
-		if(info)
-			fb_set_suspend(info, 1);/* 1 means do suspend*/
-
-		info = share->fbinfo[1];
-		if(info)
-			fb_set_suspend(info, 1);/* 1 means do suspend*/
-
-		ret = pci_save_state(pdev);
-		if(ret){
-			pr_err("error:%d occured in pci_save_state\n",ret);
-			return ret;
-		}
-
-		/* set chip to sleep mode	*/
-		if(share->suspend)
-			(*share->suspend)(share);
-
-		pci_disable_device(pdev);
-		ret = pci_set_power_state(pdev,pci_choose_state(pdev,mesg));
-		if(ret){
-			pr_err("error:%d occured in pci_set_power_state\n",ret);
-			return ret;
-		}
-	}
-
-	pdev->dev.power.power_state = mesg;
-	console_unlock();
-	return ret;
-}
-
 static int lynxfb_ops_set_par(struct fb_info * info)
 {
 	struct lynxfb_par * par;
@@ -369,7 +313,6 @@ static int lynxfb_ops_set_par(struct fb_info * info)
 	struct fb_fix_screeninfo * fix;
 	int ret;
 	unsigned int line_length;
-	
 
 	if(!info)
 		return -EINVAL;
@@ -441,6 +384,7 @@ static int lynxfb_ops_set_par(struct fb_info * info)
 		ret = output->proc_setMode(output,var,fix);
 	return ret;
 }
+
 static inline unsigned int chan_to_field(unsigned int chan,struct fb_bitfield * bf)
 {
 	chan &= 0xffff;
@@ -448,6 +392,57 @@ static inline unsigned int chan_to_field(unsigned int chan,struct fb_bitfield *
 	return chan << bf->offset;
 }
 
+#ifdef CONFIG_PM
+static int lynxfb_suspend(struct pci_dev *pdev, pm_message_t mesg)
+{
+	struct fb_info *info;
+	struct lynx_share *share;
+	int ret;
+
+	if (mesg.event = pdev->dev.power.power_state.event)
+		return 0;
+
+	ret = 0;
+	share = pci_get_drvdata(pdev);
+	switch (mesg.event) {
+	case PM_EVENT_FREEZE:
+	case PM_EVENT_PRETHAW:
+		pdev->dev.power.power_state = mesg;
+		return 0;
+	}
+
+	console_lock();
+	if (mesg.event & PM_EVENT_SLEEP) {
+		info = share->fbinfo[0];
+		if (info)
+			fb_set_suspend(info, 1);/* 1 means do suspend*/
+
+		info = share->fbinfo[1];
+		if (info)
+			fb_set_suspend(info, 1);/* 1 means do suspend*/
+
+		ret = pci_save_state(pdev);
+		if (ret) {
+			pr_err("error:%d occurred in pci_save_state\n", ret);
+			return ret;
+		}
+
+		/* set chip to sleep mode	*/
+		if (share->suspend)
+			(*share->suspend)(share);
+
+		pci_disable_device(pdev);
+		ret = pci_set_power_state(pdev, pci_choose_state(pdev, mesg));
+		if (ret) {
+			pr_err("error:%d occurred in pci_set_power_state\n", ret);
+			return ret;
+		}
+	}
+
+	pdev->dev.power.power_state = mesg;
+	console_unlock();
+	return ret;
+}
 
 static int lynxfb_resume(struct pci_dev* pdev)
 {
-- 
1.8.1.2

Re: [PATCH 3/4] staging: sm750fb: correctly define SM750LE_REVISION_ID

[Giedrius Statkevičius]

On 2015.03.08 14:31, Sudip Mukherjee wrote:
> check if it is already defined before defining SM750LE_REVISION_ID
> again and at the same time mention correct data type.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
>  drivers/staging/sm750fb/ddk750_chip.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_chip.h b/drivers/staging/sm750fb/ddk750_chip.h
> index 1c78875..e7d27e8 100644
> --- a/drivers/staging/sm750fb/ddk750_chip.h
> +++ b/drivers/staging/sm750fb/ddk750_chip.h
> @@ -1,7 +1,9 @@
>  #ifndef DDK750_CHIP_H__
>  #define DDK750_CHIP_H__
>  #define DEFAULT_INPUT_CLOCK 14318181 /* Default reference clock */
> -#define SM750LE_REVISION_ID (char)0xfe
> +#ifndef SM750LE_REVISION_ID
> +#define SM750LE_REVISION_ID ((unsigned char)0xfe)
> +#endif

Do you need these parantheses? Also, you can now then fix up this line
too to avoid a redundant cast in the same patch:
sm750_hw.c: if(share->revid = (unsigned char)SM750LE_REVISION_ID){


-- 
Thanks,
Giedrius

Re: [PATCH 1/4] staging: sm750fb: wrong type for print

[Sudip Mukherjee]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 1554 bytes --]

On Sun, Mar 08, 2015 at 02:40:03PM +0200, Giedrius Statkevičius wrote:
> On 2015.03.08 14:31, Sudip Mukherjee wrote:
> > mention correct format specifier while printing
> > index 711676c..2ab7b74 100644
> > --- a/drivers/staging/sm750fb/sm750.h
> > +++ b/drivers/staging/sm750fb/sm750.h
> > @@ -59,10 +59,10 @@ struct lynx_share{
> >  		}mtrr;
> >  #endif
> >  	/* all smi graphic adaptor got below attributes */
> > -	resource_size_t vidmem_start;
> > -	resource_size_t vidreg_start;
> > -	resource_size_t vidmem_size;
> > -	resource_size_t vidreg_size;
> > +	unsigned long vidmem_start;
> > +	unsigned long vidreg_start;
> > +	unsigned long vidmem_size;
> > +	unsigned long vidreg_size;
> 
> Have you checked other places where these are used? resource_size_t can
> be either u64 or u32 depending on if CONFIG_PHYS_ADDR_T_64BIT is
> #defined. Are you sure you aren't losing information when results of
> functions are being assigned to this? Maybe there should be a function
> similar to printk that changes between %u and %llu depending on whether
> that is defined?

oops .. no .. :(
i checked in other framebuffer drivers and saw they are mostly unsigned long.
I will check further on this.

Greg: can you please drop this patch (1/4) from your queue and not apply this to your tree.
I will send it as a v2.

regards
sudip

> 
> -- 
> Thanks,
> Giedrius
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] staging: sm750fb: fix undeclared function

[Giedrius Statkevičius]

On 2015.03.08 14:31, Sudip Mukherjee wrote:
> kbuild test robot reported that for microblaze-allyesconfig
> chan_to_field() and lynxfb_ops_set_par() were not defined. These two
> functions were defined under CONFIG_PM, so for any archtecture if
> CONFIG_PM is not defined we will have this error.
> while moving the lynxfb_suspend() function some very obvious
> checkpatch errors were taken care of.

Such as? If you do you could also fix up the poor things with that
function too such as the comment style here (no space between ; and the
comment):

> +		if (info)
> +			fb_set_suspend(info, 1);/* 1 means do suspend*/
> +

or here (extra unneeded spaces):

> +
> +		/* set chip to sleep mode	*/
> +		if (share->suspend)
> +			(*share->suspend)(share);

-- 
Thanks,
Giedrius

Re: [PATCH 3/4] staging: sm750fb: correctly define SM750LE_REVISION_ID

[Sudip Mukherjee]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 874 bytes --]

On Sun, Mar 08, 2015 at 02:59:03PM +0200, Giedrius Statkevičius wrote:
> On 2015.03.08 14:31, Sudip Mukherjee wrote:
> > -#define SM750LE_REVISION_ID (char)0xfe
> > +#ifndef SM750LE_REVISION_ID
> > +#define SM750LE_REVISION_ID ((unsigned char)0xfe)
> > +#endif
> 
> Do you need these parantheses? Also, you can now then fix up this line
> too to avoid a redundant cast in the same patch:
> sm750_hw.c: if(share->revid = (unsigned char)SM750LE_REVISION_ID){
good idea, thanks.
that extra parentheses is for checkpatch warning of complex macro value.

i will better send a v2 of the whole series after checking that resource_size_t.

regards
sudip
> 
> 
> -- 
> Thanks,
> Giedrius
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] staging: sm750fb: wrong type for print

[Dan Carpenter]

On Sun, Mar 08, 2015 at 06:01:23PM +0530, Sudip Mukherjee wrote:
> diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
> index 711676c..2ab7b74 100644
> --- a/drivers/staging/sm750fb/sm750.h
> +++ b/drivers/staging/sm750fb/sm750.h
> @@ -59,10 +59,10 @@ struct lynx_share{
>  		}mtrr;
>  #endif
>  	/* all smi graphic adaptor got below attributes */
> -	resource_size_t vidmem_start;
> -	resource_size_t vidreg_start;
> -	resource_size_t vidmem_size;
> -	resource_size_t vidreg_size;
> +	unsigned long vidmem_start;
> +	unsigned long vidreg_start;
> +	unsigned long vidmem_size;
> +	unsigned long vidreg_size;
>  	volatile unsigned char __iomem * pvReg;
>  	unsigned char __iomem * pvMem;
>  	/* locks*/

This seems like a very risky sort of change.  It's not explained very
well in the changelog.  What's the deal?

regards,
dan carpenter

Re: [PATCH 1/4] staging: sm750fb: wrong type for print

[Sudip Mukherjee]

On Mon, Mar 09, 2015 at 12:22:09PM +0300, Dan Carpenter wrote:
> On Sun, Mar 08, 2015 at 06:01:23PM +0530, Sudip Mukherjee wrote:
> > diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
> > index 711676c..2ab7b74 100644
> > --- a/drivers/staging/sm750fb/sm750.h
> > +++ b/drivers/staging/sm750fb/sm750.h
> > @@ -59,10 +59,10 @@ struct lynx_share{
> >  		}mtrr;
> >  #endif
> >  	/* all smi graphic adaptor got below attributes */
> > -	resource_size_t vidmem_start;
> > -	resource_size_t vidreg_start;
> > -	resource_size_t vidmem_size;
> > -	resource_size_t vidreg_size;
> > +	unsigned long vidmem_start;
> > +	unsigned long vidreg_start;
> > +	unsigned long vidmem_size;
> > +	unsigned long vidreg_size;
> >  	volatile unsigned char __iomem * pvReg;
> >  	unsigned char __iomem * pvMem;
> >  	/* locks*/
> 
> This seems like a very risky sort of change.  It's not explained very
> well in the changelog.  What's the deal?
it mainly started with the build warnings of incorrect format specifier.

v2 gives a little more details in the comments section. copying that here for your convenience:

"V2: Giedrius commented resource_size_t can be either u64 or u32
depending on if CONFIG_PHYS_ADDR_T_64BIT. based on his comments i
should have kept the datatype as resource_size_t and  used %pa as the
format specifier. But since this is a framebuffer device and it
should follow what the framebuffer layer is suggesting in
struct fb_fix_screeninfo at smem_start and mmio_start."

so accoringly, like all other framebuffer devices, vidmem_start and vidreg_start should be unsigned long and vidmem_size and vidreg_size should be __u32.


regards
sudip
> 
> regards,
> dan carpenter
>