Re: [PATCH v2 2/2] staging: ion: chunk_heap: use %pad for printing dma_addr_t's

2015-05-04 Thread Mitchel Humpherys
On Mon, May 04 2015 at 01:05:50 PM, Colin Cross  wrote:
> On Mon, May 4, 2015 at 1:22 AM, Dan Carpenter  
> wrote:
>> On Thu, Apr 09, 2015 at 06:10:04PM -0700, Mitchel Humpherys wrote:
>>> We're currently using %lu and %ld to print some variables of type
>>> dma_addr_t, which results in the following warning when dma_addr_t is
>>> 64-bits wide:
>>>
>>> drivers/staging/android/ion/ion_chunk_heap.c: In function 
>>> 'ion_chunk_heap_create':
>>> drivers/staging/android/ion/ion_chunk_heap.c:176:2: warning: format 
>>> '%lu' expects argument of type 'long unsigned int', but argument 3 has type 
>>> 'dma_addr_t' [-Wformat=]
>>>   pr_info("%s: base %lu size %zu align %ld\n", __func__, 
>>> chunk_heap->base,
>>>   ^
>>> drivers/staging/android/ion/ion_chunk_heap.c:176:2: warning: format 
>>> '%ld' expects argument of type 'long int', but argument 5 has type 
>>> 'dma_addr_t' [-Wformat=]
>>>
>>> Fix this by using %pad as instructed in printk-formats.txt.
>>>
>>> Signed-off-by: Mitchel Humpherys 
>>
>> This one was just merged and I was about to email you that it introduces
>> some new Smatch warnings, but actually looking at it, it's just wrong.
>>
>> We want to print "chunk_heap->base" and not "&chunk_heap->base".
>
> This would be correct if base was a dma_addr_t...
>
>> And anyway "&chunk_heap->base" is a regular pointer, not a dma_addr_t.
>
> But it is actually an ion_phys_addr_t, which is currently typedef'd to
> unsigned long.  Are you using a local patch that replaces
> ion_phys_addr_t with dma_addr_t?
>
>> So please send a new patch that removes the &.
>
> Removing the & is not correct, lib/vsprintf.c will dereference the arg
> for %pad or %pap.  I think this patch should just be dropped, the old
> %lu was correct for what is in Linus' tree.

Ah, you're absolutely correct.  We have a local patch that makes
ion_phys_addr_t a dma_addr_t (needed for LPAE), which is why I needed to
convert the printk format...

Greg, can you please drop this patch from your tree?  We'll only need
this if/when mainline Ion gets LPAE support...


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v4] staging: sm750fb: use arch_phys_wc_add() and ioremap_wc()

2015-05-04 Thread Luis R. Rodriguez
From: "Luis R. Rodriguez" 

The same area used for ioremap() is used for the MTRR area.
Convert the driver from using the x86 specific MTRR code to
the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
will avoid MTRR if write-combining is available, in order to
take advantage of that also ensure the ioremap'd area is requested
as write-combining.

There are a few motivations for this:

a) Take advantage of PAT when available

b) Help bury MTRR code away, MTRR is architecture specific and on
   x86 its replaced by PAT

c) Help with the goal of eventually using _PAGE_CACHE_UC over
   _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
   de33c442e titled "x86 PAT: fix performance drop for glx,
   use UC minus for ioremap(), ioremap_nocache() and
   pci_mmap_page_range()")

The conversion done is expressed by the following Coccinelle
SmPL patch, it additionally required manual intervention to
address all the #ifdery and removal of redundant things which
arch_phys_wc_add() already addresses such as verbose message
about when MTRR fails and doing nothing when we didn't get
an MTRR.

@ mtrr_found @
expression index, base, size;
@@

-index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
+index = arch_phys_wc_add(base, size);

@ mtrr_rm depends on mtrr_found @
expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
@@

-mtrr_del(index, base, size);
+arch_phys_wc_del(index);

@ mtrr_rm_zero_arg depends on mtrr_found @
expression mtrr_found.index;
@@

-mtrr_del(index, 0, 0);
+arch_phys_wc_del(index);

@ mtrr_rm_fb_info depends on mtrr_found @
struct fb_info *info;
expression mtrr_found.index;
@@

-mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
+arch_phys_wc_del(index);

@ ioremap_replace_nocache depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@

-info->screen_base = ioremap_nocache(base, size);
+info->screen_base = ioremap_wc(base, size);

@ ioremap_replace_default depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@

-info->screen_base = ioremap(base, size);
+info->screen_base = ioremap_wc(base, size);

Generated-by: Coccinelle SmPL
Cc: Sudip Mukherjee 
Cc: Teddy Wang 
Cc: Greg Kroah-Hartman 
Cc: Suresh Siddha 
Cc: Ingo Molnar 
Cc: Thomas Gleixner 
Cc: Juergen Gross 
Cc: Daniel Vetter 
Cc: Andy Lutomirski 
Cc: Dave Airlie 
Cc: Antonino Daplas 
Cc: Jean-Christophe Plagniol-Villard 
Cc: Tomi Valkeinen 
Cc: de...@driverdev.osuosl.org
Cc: linux-fb...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Luis R. Rodriguez 
---
 drivers/staging/sm750fb/sm750.c| 36 
 drivers/staging/sm750fb/sm750.h|  3 ---
 drivers/staging/sm750fb/sm750_hw.c |  3 +--
 3 files changed, 5 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 3c7ea95..cf57e3e 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -16,9 +16,6 @@
 #include
 #include
 #include 
-#ifdef CONFIG_MTRR
-#include 
-#endif
 #include 
 #include "sm750.h"
 #include "sm750_hw.h"
@@ -47,9 +44,7 @@ typedef int (*PROC_SPEC_INITHW)(struct lynx_share*, struct 
pci_dev*);
 /* common var for all device */
 static int g_hwcursor = 1;
 static int g_noaccel;
-#ifdef CONFIG_MTRR
 static int g_nomtrr;
-#endif
 static const char *g_fbmode[] = {NULL, NULL};
 static const char *g_def_fbmode = "800x600-16@60";
 static char *g_settings = NULL;
@@ -1126,11 +1121,8 @@ static int lynxfb_pci_probe(struct pci_dev *pdev,
 
pr_info("share->revid = %02x\n", share->revid);
share->pdev = pdev;
-#ifdef CONFIG_MTRR
share->mtrr_off = g_nomtrr;
share->mtrr.vram = 0;
-   share->mtrr.vram_added = 0;
-#endif
share->accel_off = g_noaccel;
share->dual = g_dualview;
spin_lock_init(&share->slock);
@@ -1158,22 +1150,9 @@ static int lynxfb_pci_probe(struct pci_dev *pdev,
goto err_map;
}
 
-#ifdef CONFIG_MTRR
-   if (!share->mtrr_off) {
-   pr_info("enable mtrr\n");
-   share->mtrr.vram = mtrr_add(share->vidmem_start,
-   share->vidmem_size,
-   MTRR_TYPE_WRCOMB, 1);
-
-   if (share->mtrr.vram < 0) {
-   /* don't block driver with the failure of MTRR */
-   pr_err("Unable to setup MTRR.\n");
-   } else {
-   share->mtrr.vram_added = 1;
-   pr_info("MTRR added succesfully\n");
-   }
-   }
-#endif
+   if (!share->mtrr_off)
+   share->mtrr.vram = arch_phys_wc_add(share->vidmem_start,
+   share->vidmem_size);
 
memset_io(share->pvMem, 0, share->vidmem_size);
 
@@ -1274,12 +1253,7 @@ static void __exit lynxfb_pci_remove(struct pci_dev 
*pdev)
/* release frame buffer */
framebuffer_relea

Re: [PATCH v3] staging: sm750fb: use arch_phys_wc_add() and ioremap_wc()

2015-05-04 Thread Luis R. Rodriguez
On Sun, May 03, 2015 at 09:24:59PM +0200, Greg KH wrote:
> On Tue, Apr 21, 2015 at 01:12:03PM -0700, Luis R. Rodriguez wrote:
> > From: "Luis R. Rodriguez" 
> > 
> > The same area used for ioremap() is used for the MTRR area.
> > Convert the driver from using the x86 specific MTRR code to
> > the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
> > will avoid MTRR if write-combining is available, in order to
> > take advantage of that also ensure the ioremap'd area is requested
> > as write-combining.
> > 
> > There are a few motivations for this:
> > 
> > a) Take advantage of PAT when available
> > 
> > b) Help bury MTRR code away, MTRR is architecture specific and on
> >x86 its replaced by PAT
> > 
> > c) Help with the goal of eventually using _PAGE_CACHE_UC over
> >_PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
> >de33c442e titled "x86 PAT: fix performance drop for glx,
> >use UC minus for ioremap(), ioremap_nocache() and
> >pci_mmap_page_range()")
> > 
> > The conversion done is expressed by the following Coccinelle
> > SmPL patch, it additionally required manual intervention to
> > address all the #ifdery and removal of redundant things which
> > arch_phys_wc_add() already addresses such as verbose message
> > about when MTRR fails and doing nothing when we didn't get
> > an MTRR.
> > 
> > @ mtrr_found @
> > expression index, base, size;
> > @@
> > 
> > -index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
> > +index = arch_phys_wc_add(base, size);
> > 
> > @ mtrr_rm depends on mtrr_found @
> > expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
> > @@
> > 
> > -mtrr_del(index, base, size);
> > +arch_phys_wc_del(index);
> > 
> > @ mtrr_rm_zero_arg depends on mtrr_found @
> > expression mtrr_found.index;
> > @@
> > 
> > -mtrr_del(index, 0, 0);
> > +arch_phys_wc_del(index);
> > 
> > @ mtrr_rm_fb_info depends on mtrr_found @
> > struct fb_info *info;
> > expression mtrr_found.index;
> > @@
> > 
> > -mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
> > +arch_phys_wc_del(index);
> > 
> > @ ioremap_replace_nocache depends on mtrr_found @
> > struct fb_info *info;
> > expression base, size;
> > @@
> > 
> > -info->screen_base = ioremap_nocache(base, size);
> > +info->screen_base = ioremap_wc(base, size);
> > 
> > @ ioremap_replace_default depends on mtrr_found @
> > struct fb_info *info;
> > expression base, size;
> > @@
> > 
> > -info->screen_base = ioremap(base, size);
> > +info->screen_base = ioremap_wc(base, size);
> > 
> > Generated-by: Coccinelle SmPL
> > Cc: Sudip Mukherjee 
> > Cc: Teddy Wang 
> > Cc: Greg Kroah-Hartman 
> > Cc: Suresh Siddha 
> > Cc: Ingo Molnar 
> > Cc: Thomas Gleixner 
> > Cc: Juergen Gross 
> > Cc: Daniel Vetter 
> > Cc: Andy Lutomirski 
> > Cc: Dave Airlie 
> > Cc: Antonino Daplas 
> > Cc: Jean-Christophe Plagniol-Villard 
> > Cc: Tomi Valkeinen 
> > Cc: de...@driverdev.osuosl.org
> > Cc: linux-fb...@vger.kernel.org
> > Cc: linux-ker...@vger.kernel.org
> > Signed-off-by: Luis R. Rodriguez 
> > ---
> >  drivers/staging/sm750fb/sm750.c| 36 
> > 
> >  drivers/staging/sm750fb/sm750.h|  3 ---
> >  drivers/staging/sm750fb/sm750_hw.c |  3 +--
> >  3 files changed, 5 insertions(+), 37 deletions(-)
> 
> This doesn't apply to my staging-next branch :(

OK I'll resend a v4 now.

  Luis
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH 4/7] staging: fsl-mc: Upgraded MC bus driver to match MC fw 7.0.0

2015-05-04 Thread Jose Rivera


> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Thursday, April 30, 2015 7:13 AM
> To: Rivera Jose-B46482
> Cc: gre...@linuxfoundation.org; a...@arndb.de;
> de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org; Yoder Stuart-
> B08248; Sharma Bhupesh-B45370; ag...@suse.de; Hamciuc Bogdan-BHAMCIU1;
> Erez Nir-RM30794; katz Itai-RM05202; Wood Scott-B07421; Marginean
> Alexandru-R89243; Schmitt Richard-B43082
> Subject: Re: [PATCH 4/7] staging: fsl-mc: Upgraded MC bus driver to match
> MC fw 7.0.0
> 
> On Tue, Apr 28, 2015 at 12:39:07PM -0500, J. German Rivera wrote:
> > - Migrated MC bus driver to use DPRC flib 0.6.
> 
> What does this mean?  What is a flib?
> 
The DPRC flib is the API to manipulate DPRC objects. 

> After reading the patch, apparently it means that we can remove all the
> ifdefs from patch 1.  :)
> 
No, we cannot as the required GIC-ITS support is not upstream yet.
I added some of the #ifdefs back that were removed by mistake.

> > - Changed IRQ setup infrastructure to be able to program MSIs
> >   for MC objects in an object-independent way.
> 
> Are these two things related?
> 
No.

> Why does this patch add so many new EXPORTS()s?
> 
Removed new EXPORTs added as they are not needed yet.

> regards,
> dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

2015-05-04 Thread Jose Rivera
Dan,

Thanks for your comments. My replies inline.

Thanks,

German

> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Thursday, April 30, 2015 6:50 AM
> To: Rivera Jose-B46482
> Cc: gre...@linuxfoundation.org; a...@arndb.de;
> de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org; Yoder Stuart-
> B08248; Sharma Bhupesh-B45370; ag...@suse.de; Hamciuc Bogdan-BHAMCIU1;
> Erez Nir-RM30794; katz Itai-RM05202; Wood Scott-B07421; Marginean
> Alexandru-R89243; Schmitt Richard-B43082
> Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
> 
> On Tue, Apr 28, 2015 at 12:39:04PM -0500, J. German Rivera wrote:
> > Change-Id: I2a986c465989c3811de19cfe9ed0b77168250cb1
> > Reviewed-on: http://git.am.freescale.net:8181/33626
> > Tested-by: Review Code-CDREVIEW 
> 
> These things are totally useless to the rest of us.  Don't add them.
> 
Agreed. I'll remove them in the next respin.

> 
> > diff --git a/drivers/staging/fsl-mc/TODO b/drivers/staging/fsl-mc/TODO
> > index d78288b..1b8868d 100644
> > --- a/drivers/staging/fsl-mc/TODO
> > +++ b/drivers/staging/fsl-mc/TODO
> > @@ -8,6 +8,9 @@
> >  * Add at least one device driver for a DPAA2 object (child device of
> the
> >fsl-mc bus).
> >
> > +* Enable code blocks guarded by #ifdef GIC_ITS_MC_SUPPORT, when
> > +GIC-ITS
> > +  support for the MC MSIs gets merged.
> > +
> 
> When will that be?  I'd really prefer to not add these ifdeffed bits at
> all.
> 
The owner of the GIC-ITS support patches would need to answer the "when"
question. These #ifdefs are needed to be able to make the code compile
without the GIC-ITS support in place. Of course, the code will not be moved
out of staging with these #ifdefs. There is already an item for this
in the drivers/staging/fsl-mc/TODO file.

> > +   if (status & (DPRC_IRQ_EVENT_OBJ_ADDED |
> > + DPRC_IRQ_EVENT_OBJ_REMOVED |
> > + DPRC_IRQ_EVENT_CONTAINER_DESTROYED |
> > + DPRC_IRQ_EVENT_OBJ_DESTROYED |
> > + DPRC_IRQ_EVENT_OBJ_CREATED)) {
> > +   unsigned int irq_count;
> > +
> > +   error = dprc_scan_objects(mc_dev, &irq_count);
> > +   if (error < 0) {
> > +   dev_err(dev, "dprc_scan_objects() failed: %d\n",
> error);
> > +   goto out;
> > +   }
> > +
> > +   WARN_ON((int16_t)irq_count < 0);
> 
> This code is doing "WARN_ON(test_bit(15, (unsigned long *)&irq_count));".
> That seems like nonsense.  Anyway, just delete the WARN_ON().
> 
I disagree. This WARN_ON is checking that irq_count is in the expected range
(it fits in int16_t as a positive number). The dprc_scan_objects() function
expects irq_count to be of type "unsigned int" (which is 32-bit unsigned)

> > +
> > +   if ((int16_t)irq_count >
> > +   mc_bus->resource_pools[FSL_MC_POOL_IRQ].max_count) {
> 
> Why are we casting this?  Also can you align it like:
> 
This casting is done for safety, to prevent the comparison to be done
in "unsigned int" due to integer promotion rules.

>   if (irq_count >
>   mc_bus->resource_pools[FSL_MC_POOL_IRQ].max_count) {
> 
> [tab][tab][space][space][space][space]mc_bus->resource_pools[
> 
> That way you can tell the if condition from the indented block.  Plus
> that is standard kernel indenting style these days.
> 
Agreed. I'll change this in the next respin.

> 
> [ snip ]
> 
> > +   irqs = devm_kzalloc(&mc_dev->dev, irq_count * sizeof(irqs[0]),
> > +   GFP_KERNEL);
> > +   if (!irqs) {
> > +   error = -ENOMEM;
> > +   dev_err(&mc_dev->dev, "No memory to allocate irqs[]\n");
> > +   goto error;
> 
> I kind of hate One Err Style error handling, because the error labels are
> so general...  You can never guess the point of it until you scroll down
>
Agreed. I will replace the generic error cleanup exit point with
error-specific cleanup exit points, and return directly when no cleanup
is needed.

> to read what "goto error;" does.  The error handling here calls
> devm_kfree() which is not needed...  devm_ functions automatically clean
> up after themselves.  This seems a pattern throughout.  Do a search for
> devm_free() and see which ones are really needed or not.
> 
I know that memory allocated with devm_kzalloc() is freed at the end of the
lifetime of the device it is attached to. However, in error paths, why wait 
until the device is destroyed? Why not free the memory earlier so that it
can be used for other purposes? 

> Also the error message isn't needed here.  kzalloc() has its own better
> error messages built-in.  Adding these error messages which will never be
> printed is just a waste of RAM.
> 
Agreed. Error message removed.

> In other words this should look like:
> 
>   irqs = devm_kcalloc(&mc_dev->dev, sizeof(*irqs), irq_count,
>   GFP_KERNEL);
>   if (!irqs)
>   return -ENOMEM;
> 
> regards,
> dan carpen

RE: [PATCH] hv_netvsc: remove unused variable in netvsc_send()

2015-05-04 Thread Haiyang Zhang


> -Original Message-
> From: Jerry Snitselaar [mailto:jsnit...@redhat.com]
> Sent: Monday, May 4, 2015 10:57 AM
> To: linux-ker...@vger.kernel.org
> Cc: net...@vger.kernel.org; de...@linuxdriverproject.org; Haiyang Zhang;
> KY Srinivasan
> Subject: [PATCH] hv_netvsc: remove unused variable in netvsc_send()
> 
> With commit b56fc3c53654 ("hv_netvsc: Fix a bug in netvsc_start_xmit()"),
> skb variable is no longer used in netvsc_send. Remove variable and dead
> code that depended on it.
> 
> Cc: Haiyang Zhang 
> Cc: K. Y. Srinivasan 
> Signed-off-by: Jerry Snitselaar 

Thanks.

Signed-off-by: Haiyang Zhang 


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCHv2] staging: rtl8192e: fix wrong assignment

2015-05-04 Thread Mateusz Kulikowski
Hi,

On 04.05.2015 11:29, Gujulan Elango, Hari Prasath (H.) wrote:
> This patch addresses a spatch warning on assigning a negative
> value to a unsigned integer.Similar patch has been submitted by
> Larry Finger earlier to silence the same spatch warning in another
> file.
> 
> Signed-off-by: Hari Prasath Gujulan Elango 
> ---
>   v2: Address Dan Carpenter review comments for version 1 of this
>   patch.
> ---
>  drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c 
> b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
> index 352d381..a7a1ade 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
> @@ -2310,7 +2310,7 @@ static void rtl8192_rx_normal(struct net_device *dev)
>  
>   struct rtllib_rx_stats stats = {
>   .signal = 0,
> - .noise = -98,
> + .noise = (u8) -98,
>   .rate = 0,
>   .freq = RTLLIB_24GHZ_BAND,
>   };
> 

Small suggestion from my side - a bit late, but I was unable to check code 
earlier - so please don't
do v3 unless others say so.

As far as I know (radio) noise is rarely above 0 dBm - if it is, you're doing 
something wrong.
This means we can just change rtllib_rx_stats::noise to s8.

Then, we need to add cast in other place - when we access struct iw_quality 
(wireless.h).

Maybe we should even talk with wireless guys and change noise there as well.

Rationale:
127 dbM = ~5 GW (Giga Watt) - I doubt we can have transmitter with such power 
in nearest future;)
-128 dbM = <1fW (femto Watt, 1e-15) - thermal noise for wifi is > -100dBm


Regards,
Mateusz
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/2] staging: ion: chunk_heap: use %pad for printing dma_addr_t's

2015-05-04 Thread Colin Cross
On Mon, May 4, 2015 at 1:22 AM, Dan Carpenter  wrote:
> On Thu, Apr 09, 2015 at 06:10:04PM -0700, Mitchel Humpherys wrote:
>> We're currently using %lu and %ld to print some variables of type
>> dma_addr_t, which results in the following warning when dma_addr_t is
>> 64-bits wide:
>>
>> drivers/staging/android/ion/ion_chunk_heap.c: In function 
>> 'ion_chunk_heap_create':
>> drivers/staging/android/ion/ion_chunk_heap.c:176:2: warning: format 
>> '%lu' expects argument of type 'long unsigned int', but argument 3 has type 
>> 'dma_addr_t' [-Wformat=]
>>   pr_info("%s: base %lu size %zu align %ld\n", __func__, 
>> chunk_heap->base,
>>   ^
>> drivers/staging/android/ion/ion_chunk_heap.c:176:2: warning: format 
>> '%ld' expects argument of type 'long int', but argument 5 has type 
>> 'dma_addr_t' [-Wformat=]
>>
>> Fix this by using %pad as instructed in printk-formats.txt.
>>
>> Signed-off-by: Mitchel Humpherys 
>
> This one was just merged and I was about to email you that it introduces
> some new Smatch warnings, but actually looking at it, it's just wrong.
>
> We want to print "chunk_heap->base" and not "&chunk_heap->base".

This would be correct if base was a dma_addr_t...

> And anyway "&chunk_heap->base" is a regular pointer, not a dma_addr_t.

But it is actually an ion_phys_addr_t, which is currently typedef'd to
unsigned long.  Are you using a local patch that replaces
ion_phys_addr_t with dma_addr_t?

> So please send a new patch that removes the &.

Removing the & is not correct, lib/vsprintf.c will dereference the arg
for %pad or %pap.  I think this patch should just be dropped, the old
%lu was correct for what is in Linus' tree.

> regards,
> dan carpenter
>
>> ---
>>  drivers/staging/android/ion/ion_chunk_heap.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion_chunk_heap.c 
>> b/drivers/staging/android/ion/ion_chunk_heap.c
>> index 54746157d799..6b3e18aa1c64 100644
>> --- a/drivers/staging/android/ion/ion_chunk_heap.c
>> +++ b/drivers/staging/android/ion/ion_chunk_heap.c
>> @@ -173,8 +173,8 @@ struct ion_heap *ion_chunk_heap_create(struct 
>> ion_platform_heap *heap_data)
>>   chunk_heap->heap.ops = &chunk_heap_ops;
>>   chunk_heap->heap.type = ION_HEAP_TYPE_CHUNK;
>>   chunk_heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE;
>> - pr_debug("%s: base %lu size %zu align %ld\n", __func__, 
>> chunk_heap->base,
>> - heap_data->size, heap_data->align);
>> + pr_debug("%s: base %pad size %zu align %pad\n", __func__,
>> + &chunk_heap->base, heap_data->size, &heap_data->align);
>>
>>   return &chunk_heap->heap;
>>
>> --
>> Qualcomm Innovation Center, Inc.
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>>
>> ___
>> devel mailing list
>> de...@linuxdriverproject.org
>> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
>
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to kernel-team+unsubscr...@android.com.

+
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: unisys: Disable driver for UML

2015-05-04 Thread Richard Weinberger
UML has no io memory nor cpuid.
Let's disable this driver for UML.

Fixes:
drivers/staging/unisys/common-spar/include/iovmcall_gnuc.h: In function 
‘__unisys_vmcall_gnuc’:
drivers/staging/unisys/common-spar/include/iovmcall_gnuc.h:24:2: error: 
implicit declaration of function ‘cpuid’ [-Werror=implicit-function-declaration]
  cpuid(0x0001, &cpuid_eax, &cpuid_ebx, &cpuid_ecx, &cpuid_edx);
  ^
In file included from drivers/staging/unisys/uislib/uislib.c:33:0:
drivers/staging/unisys/include/uisutils.h: In function ‘dbg_ioremap_cache’:
drivers/staging/unisys/include/uisutils.h:78:2: error: implicit declaration of 
function ‘ioremap_cache’ [-Werror=implicit-function-declaration]
  new = ioremap_cache(addr, size);
  ^
drivers/staging/unisys/include/uisutils.h:78:6: warning: assignment makes 
pointer from integer without a cast [enabled by default]
  new = ioremap_cache(addr, size);
  ^
drivers/staging/unisys/include/uisutils.h: In function ‘dbg_ioremap’:
drivers/staging/unisys/include/uisutils.h:89:2: error: implicit declaration of 
function ‘ioremap’ [-Werror=implicit-function-declaration]
  new = ioremap(addr, size);
  ^
drivers/staging/unisys/include/uisutils.h:89:6: warning: assignment makes 
pointer from integer without a cast [enabled by default]
  new = ioremap(addr, size);
  ^
drivers/staging/unisys/include/uisutils.h: In function ‘dbg_iounmap’:
drivers/staging/unisys/include/uisutils.h:98:2: error: implicit declaration of 
function ‘iounmap’ [-Werror=implicit-function-declaration]
  iounmap(addr);

Signed-off-by: Richard Weinberger 
---
 drivers/staging/unisys/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/unisys/Kconfig b/drivers/staging/unisys/Kconfig
index 19fcb34..a6d6c2a 100644
--- a/drivers/staging/unisys/Kconfig
+++ b/drivers/staging/unisys/Kconfig
@@ -3,7 +3,7 @@
 #
 menuconfig UNISYSSPAR
bool "Unisys SPAR driver support"
-   depends on X86_64
+   depends on X86_64 && !UML
---help---
Support for the Unisys SPAR drivers
 
-- 
1.8.4.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] hv_netvsc: remove unused variable in netvsc_send()

2015-05-04 Thread Jerry Snitselaar
With commit b56fc3c53654 ("hv_netvsc: Fix a bug in netvsc_start_xmit()"),
skb variable is no longer used in netvsc_send. Remove variable and dead
code that depended on it.

Cc: Haiyang Zhang 
Cc: K. Y. Srinivasan 
Signed-off-by: Jerry Snitselaar 
---
 drivers/net/hyperv/netvsc.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 2d9ef53..ea091bc 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -826,7 +826,6 @@ int netvsc_send(struct hv_device *device,
u16 q_idx = packet->q_idx;
u32 pktlen = packet->total_data_buflen, msd_len = 0;
unsigned int section_index = NETVSC_INVALID_INDEX;
-   struct sk_buff *skb = NULL;
unsigned long flag;
struct multi_send_data *msdp;
struct hv_netvsc_packet *msd_send = NULL, *cur_send = NULL;
@@ -924,12 +923,8 @@ int netvsc_send(struct hv_device *device,
if (cur_send)
ret = netvsc_send_pkt(cur_send, net_device);
 
-   if (ret != 0) {
-   if (section_index != NETVSC_INVALID_INDEX)
-   netvsc_free_send_slot(net_device, section_index);
-   } else if (skb) {
-   dev_kfree_skb_any(skb);
-   }
+   if (ret != 0 && section_index != NETVSC_INVALID_INDEX)
+   netvsc_free_send_slot(net_device, section_index);
 
return ret;
 }
-- 
2.4.0.rc3.3.g6eb1401

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging/lustre: Always try kmalloc first for OBD_ALLOC_LARGE

2015-05-04 Thread green
From: Oleg Drokin 

Create libcfs_kvzalloc and libcfs_kvzalloc_cpt that
are designed to replace OBD_ALLOC_LARGE and OBD_CPT_ALLOC_LARGE.

Not a drop-in replacement as they also take gfp flags armument
for more flexibility.

Signed-off-by: Oleg Drokin 
---

Greg, I have addressed your comments wrt the copyright header.
Since I do not have the patches that replace OBD_ALLOC_LARGE with
this new function and Julia does, do you think you can accept this as is and
she'll follow with the replacement then or what is best to do here you think?

 .../staging/lustre/include/linux/libcfs/libcfs.h   |  4 ++
 .../staging/lustre/lustre/include/obd_support.h| 24 ++---
 drivers/staging/lustre/lustre/libcfs/Makefile  |  1 +
 .../staging/lustre/lustre/libcfs/linux/linux-mem.c | 59 ++
 4 files changed, 67 insertions(+), 21 deletions(-)
 create mode 100644 drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c

diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs.h 
b/drivers/staging/lustre/include/linux/libcfs/libcfs.h
index 4410d7f..947df7e 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs.h
@@ -184,4 +184,8 @@ static inline void *__container_of(void *ptr, unsigned long 
shift)
 
 #define _LIBCFS_H
 
+void *libcfs_kvzalloc(size_t size, gfp_t flags);
+void *libcfs_kvzalloc_cpt(struct cfs_cpt_table *cptab, int cpt, size_t size,
+ gfp_t flags);
+
 #endif /* _LIBCFS_H */
diff --git a/drivers/staging/lustre/lustre/include/obd_support.h 
b/drivers/staging/lustre/lustre/include/obd_support.h
index 2991d2e..379266d 100644
--- a/drivers/staging/lustre/lustre/include/obd_support.h
+++ b/drivers/staging/lustre/lustre/include/obd_support.h
@@ -676,37 +676,19 @@ do {  
  \
 __OBD_VMALLOC_VEROBSE(ptr, cptab, cpt, size)
 
 
-/* Allocations above this size are considered too big and could not be done
- * atomically.
- *
- * Be very careful when changing this value, especially when decreasing it,
- * since vmalloc in Linux doesn't perform well on multi-cores system, calling
- * vmalloc in critical path would hurt performance badly. See LU-66.
- */
-#define OBD_ALLOC_BIG (4 * PAGE_CACHE_SIZE)
-
 #define OBD_ALLOC_LARGE(ptr, size) \
 do { \
-   if (size > OBD_ALLOC_BIG)\
-   OBD_VMALLOC(ptr, size);\
-   else  \
-   OBD_ALLOC(ptr, size);\
+   ptr = libcfs_kvzalloc(size, GFP_NOFS);\
 } while (0)
 
 #define OBD_CPT_ALLOC_LARGE(ptr, cptab, cpt, size)   \
 do { \
-   if (size > OBD_ALLOC_BIG) \
-   OBD_CPT_VMALLOC(ptr, cptab, cpt, size);   \
-   else  \
-   OBD_CPT_ALLOC(ptr, cptab, cpt, size); \
+   ptr = libcfs_kvzalloc_cpt(cptab, cpt, size, GFP_NOFS);\
 } while (0)
 
 #define OBD_FREE_LARGE(ptr, size)   \
 do { \
-   if (size > OBD_ALLOC_BIG)\
-   OBD_VFREE(ptr, size);\
-   else  \
-   OBD_FREE(ptr, size);  \
+   kvfree(ptr);  \
 } while (0)
 
 
diff --git a/drivers/staging/lustre/lustre/libcfs/Makefile 
b/drivers/staging/lustre/lustre/libcfs/Makefile
index 2996a48..fabdd3e 100644
--- a/drivers/staging/lustre/lustre/libcfs/Makefile
+++ b/drivers/staging/lustre/lustre/libcfs/Makefile
@@ -7,6 +7,7 @@ libcfs-linux-objs += linux-curproc.o
 libcfs-linux-objs += linux-module.o
 libcfs-linux-objs += linux-crypto.o
 libcfs-linux-objs += linux-crypto-adler.o
+libcfs-linux-objs += linux-mem.o
 
 libcfs-linux-objs := $(addprefix linux/,$(libcfs-linux-objs))
 
diff --git a/drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c 
b/drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c
new file mode 100644
index 000..025e2f0
--- /dev/null
+++ b/drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c
@@ -0,0 +1,59 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 only,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distribu

Re: [PATCH] staging: rtl8192e: fix wrong assignment

2015-05-04 Thread Larry Finger

On 05/04/2015 02:21 AM, Gujulan Elango, Hari Prasath (H.) wrote:

On Thu, Apr 30, 2015 at 02:09:13PM -0500, Larry Finger wrote:


Please fix your mailer. The most recent material to which you were
replying was not indented any levels, but the "new" material was 3
levels deep. It is extremely difficult to follow the thread.

Larry



Please exuse me for the replies being screwed up. I just figured out how to 
reply to e-mails from Mutt.Hope you would be getting cleaner replies from now 
on.


Much better. Thank you.

Larry


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [HPDD-discuss] [PATCH 2/11] Staging: lustre: fld: Use kzalloc and kfree

2015-05-04 Thread Dan Carpenter
Email chopped up and responded to out of order.

> If this is acceptable to you it can be started right away.

No one is going to approve your patches until you send them.  Start and
then we'll see.  Send them in as they are ready so you don't get too far
along and we ask you to redo everything.

> Since I'm just starting to get involved in this work I'm not aware of the
> task you are looking for. What needs to be done from your perspective?

There are so many things.  Let's take drivers/staging/lustre/lnet/lnet/module.c

The copyright header has dead URLs and other information which should
probably be changed.

46  static int
47  lnet_configure(void *arg)
48  {
49  /* 'arg' only there so I can be passed to cfs_create_thread() */

If there was no comment then I would have assumed correctly that it was
an unused argument pointer.  Instead I assumed it was something more
complicated.  I took embarrassingly long to figure out that the comment
was just out of date.  Just delete it.

50  intrc = 0;

Every variable in this file is indented a different random number of
spaces.  Just use one space unless there is a good reason.

51  
52  LNET_MUTEX_LOCK(&lnet_config_mutex);

Use mutex_lock() directly instead of obfuscating.

53  
54  if (!the_lnet.ln_niinit_self) {
55  rc = LNetNIInit(LUSTRE_SRV_LNET_PID);

CamelCase.

56  if (rc >= 0) {
57  the_lnet.ln_niinit_self = 1;
58  rc = 0;
59  }
60  }
61  
62  LNET_MUTEX_UNLOCK(&lnet_config_mutex);
63  return rc;
64  }

Tons and tons of minor things everywhere.

> One of the things I have discussed with other developers is the idea
> of breaking the cleanup into two stages. First is bringing  libcfs/lnet
> up to date and synced to the upstream standards. This is due to lustre
> being a application of LNet. LNet is used by various vendors for other
> purposes.

In theory code sharing is nice, but it doesn't ever really work to share
stuff between the linux-kernel and the rest of the world.

Normally it's things like, "We want to support Windows, BSD and Linux
with the same code." so it ends up adding all kinds of abstraction.  It
sucks.  It's normally less work to just implement clean drivers on all
the OSes.  In lnet, it uses ifdefs in the .c file to separate the
userspace and kernel space code which is considered bad style.  Put the
kernelspace and user space code in different files and use the Makefile
to pull in the correct file.

There are a few files in ACPI which are shared and we can't touch them
in the kernel but those are very rare.  And I sometimes find myself
wanting to modify the ACPI files to add error handling or whatever.

In the linux-kernel we really don't care about things outside the kernel
tree so we often do tree wide renames.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [HPDD-discuss] [PATCH 2/11] Staging: lustre: fld: Use kzalloc and kfree

2015-05-04 Thread Simmons, James A.
>> > When is "soon"?  How about, if I don't see some real work happening from
>> > you all in the next 2 months (i.e. before 4.1-final), I drop lustre from
>> > the tree in 4.2-rc1.  Given that you all have had over 2 years to get
>> > your act together, and nothing has happened, I think I've been waiting
>> > long enough, don't you?

As you see from a earlier email from Oleg work is being done to change things.

>> I agree we've been much slower in doing a bunch of requested cleanups than 
>> initially
>> hoped for variety of reasons, not all of which are under our direct control.
>> 
>> Still, please don't drop Lustre client from the staging tree. People seem to 
>> be
>> actively using that port too (on smaller scale) and we'll improve the 
>> cleanups
>> situation.
>
>"much slower"?  Seriously?  It would take one junior developer a month
>tops to clean up all of the obvious issues with the in-kernel code, so
>that you could then tackle the real issues.  A "good" developer could do
>it all in a single week.  As that's obviously not going to ever happen,
>I have no choice but to delete the code from the kernel tree as no one
>is working to get it out of staging at all.
>
>Also, having it in the tree is wasting core kernel developer's time and
>energy trying to work around things in your codebase.

Since I'm just starting to get involved in this work I'm not aware of the
task you are looking for. What needs to be done from your perspective?
One of the things I have discussed with other developers is the idea
of breaking the cleanup into two stages. First is bringing  libcfs/lnet
up to date and synced to the upstream standards. This is due to lustre
being a application of LNet. LNet is used by various vendors for other
purposes. If this is acceptable to you it can be started right away.  Please
send of list of task that needs to be done for libcfs/lnet work.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH V5] staging: goldfish: Fix pointer cast for 32 bits

2015-05-04 Thread Dan Carpenter
Looks good.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH V5] staging: goldfish: Fix pointer cast for 32 bits

2015-05-04 Thread Peter Senna Tschudin
As the first argument of gf_write64() was of type unsigned long, and as
some calls to gf_write64() were casting the first argument from void *
to u64 the compiler and/or sparse were printing warnings for casts of
wrong sizes when compiling for i386.

This patch changes the type of the first argument of gf_write64() to
void *, and update calls to the function. This change fixed the
warnings and allowed to remove casts from 6 calls to gf_write64().

In addition gf_write64() was renamed to gf_write_ptr() as the name was
misleading because it only writes 32 bits on 32 bit systems.

gf_write_dma_addr() was added to handle dma_addr_t values which is
used at drivers/staging/goldfish/goldfish_audio.c.

Signed-off-by: Dan Carpenter 
Signed-off-by: Peter Senna Tschudin 
---
Tested by compilation only for x86 and for x86_64.

Changes from V4:
 - Added the function gf_write_dma_addr()
 - Changed the subject line

Changes from V3:
 - Changed type of first argument of gf_write64
 - Renamed from gf_write64 to gf_write_ptr
 - Updated all calls to gf_write_ptr

Changes from V2:
 - Fixed spelling of complains
 - Updated commit message

Changes from V1:
 - Updated commit message

 drivers/platform/goldfish/goldfish_pipe.c | 18 +-
 drivers/staging/goldfish/goldfish_audio.c |  2 +-
 drivers/staging/goldfish/goldfish_nand.c  |  2 +-
 drivers/tty/goldfish.c|  4 ++--
 include/linux/goldfish.h  | 18 ++
 5 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/drivers/platform/goldfish/goldfish_pipe.c 
b/drivers/platform/goldfish/goldfish_pipe.c
index d9a09d9..aad16bc 100644
--- a/drivers/platform/goldfish/goldfish_pipe.c
+++ b/drivers/platform/goldfish/goldfish_pipe.c
@@ -158,8 +158,8 @@ static u32 goldfish_cmd_status(struct goldfish_pipe *pipe, 
u32 cmd)
struct goldfish_pipe_dev *dev = pipe->dev;
 
spin_lock_irqsave(&dev->lock, flags);
-   gf_write64((u64)(unsigned long)pipe, dev->base + PIPE_REG_CHANNEL,
-   dev->base + PIPE_REG_CHANNEL_HIGH);
+   gf_write_ptr(pipe, dev->base + PIPE_REG_CHANNEL,
+dev->base + PIPE_REG_CHANNEL_HIGH);
writel(cmd, dev->base + PIPE_REG_COMMAND);
status = readl(dev->base + PIPE_REG_STATUS);
spin_unlock_irqrestore(&dev->lock, flags);
@@ -172,8 +172,8 @@ static void goldfish_cmd(struct goldfish_pipe *pipe, u32 
cmd)
struct goldfish_pipe_dev *dev = pipe->dev;
 
spin_lock_irqsave(&dev->lock, flags);
-   gf_write64((u64)(unsigned long)pipe, dev->base + PIPE_REG_CHANNEL,
-   dev->base + PIPE_REG_CHANNEL_HIGH);
+   gf_write_ptr(pipe, dev->base + PIPE_REG_CHANNEL,
+dev->base + PIPE_REG_CHANNEL_HIGH);
writel(cmd, dev->base + PIPE_REG_COMMAND);
spin_unlock_irqrestore(&dev->lock, flags);
 }
@@ -327,12 +327,12 @@ static ssize_t goldfish_pipe_read_write(struct file 
*filp, char __user *buffer,
spin_lock_irqsave(&dev->lock, irq_flags);
if (access_with_param(dev, CMD_WRITE_BUFFER + cmd_offset,
address, avail, pipe, &status)) {
-   gf_write64((u64)(unsigned long)pipe,
-  dev->base + PIPE_REG_CHANNEL,
-  dev->base + PIPE_REG_CHANNEL_HIGH);
+   gf_write_ptr(pipe, dev->base + PIPE_REG_CHANNEL,
+dev->base + PIPE_REG_CHANNEL_HIGH);
writel(avail, dev->base + PIPE_REG_SIZE);
-   gf_write64(address, dev->base + PIPE_REG_ADDRESS,
-   dev->base + PIPE_REG_ADDRESS_HIGH);
+   gf_write_ptr((void *)address,
+dev->base + PIPE_REG_ADDRESS,
+dev->base + PIPE_REG_ADDRESS_HIGH);
writel(CMD_WRITE_BUFFER + cmd_offset,
dev->base + PIPE_REG_COMMAND);
status = readl(dev->base + PIPE_REG_STATUS);
diff --git a/drivers/staging/goldfish/goldfish_audio.c 
b/drivers/staging/goldfish/goldfish_audio.c
index 702ae04..b0927e4 100644
--- a/drivers/staging/goldfish/goldfish_audio.c
+++ b/drivers/staging/goldfish/goldfish_audio.c
@@ -63,7 +63,7 @@ struct goldfish_audio {
 #define AUDIO_READ(data, addr) (readl(data->reg_base + addr))
 #define AUDIO_WRITE(data, addr, x) (writel(x, data->reg_base + addr))
 #define AUDIO_WRITE64(data, addr, addr2, x)\
-   (gf_write64((u64)(x), data->reg_base + addr, data->reg_base+addr2))
+   (gf_write_dma_addr((x), data->reg_base + addr, data->reg_base+addr2))
 
 /*
  *  temporary variable used between goldfish_audio_probe() and
diff --git a/drivers/staging/goldfish/goldfish_nand.c 
b/drivers/staging/goldfish/goldfish_nand.c
index 213877a..66ae48f 100644
--- a/drivers/staging/goldfish/goldfish_nan

Re: [PATCHv2] staging: rtl8192e: fix wrong assignment

2015-05-04 Thread Dan Carpenter
On Mon, May 04, 2015 at 09:29:05AM +, Gujulan Elango, Hari Prasath (H.) 
wrote:
> This patch addresses a spatch warning on assigning a negative
> value to a unsigned integer.Similar patch has been submitted by
> Larry Finger earlier to silence the same spatch warning in another
> file.

Or, more accurately, a Larry sent a totally different patch.

But this is still fine.

Acked-by: Dan Carpenter 

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCHv2] staging: rtl8192e: fix wrong assignment

2015-05-04 Thread Gujulan Elango, Hari Prasath (H.)
This patch addresses a spatch warning on assigning a negative
value to a unsigned integer.Similar patch has been submitted by
Larry Finger earlier to silence the same spatch warning in another
file.

Signed-off-by: Hari Prasath Gujulan Elango 
---
v2: Address Dan Carpenter review comments for version 1 of this
patch.
---
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c 
b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
index 352d381..a7a1ade 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
@@ -2310,7 +2310,7 @@ static void rtl8192_rx_normal(struct net_device *dev)
 
struct rtllib_rx_stats stats = {
.signal = 0,
-   .noise = -98,
+   .noise = (u8) -98,
.rate = 0,
.freq = RTLLIB_24GHZ_BAND,
};
-- 
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: rtl8192e: fix wrong assignment

2015-05-04 Thread Gujulan Elango, Hari Prasath (H.)
On Fri, May 01, 2015 at 10:46:53AM +0300, Dan Carpenter wrote:
> The subject says "fix" but this does not fix a run time bug, it just
> silences a warning.  It's still the correct thing according to Larry
> so that's good.
> 
> On Thu, Apr 30, 2015 at 12:06:28PM +, Gujulan Elango, Hari Prasath (H.) 
> wrote:
> > This patch addresses a spatch warning on assigning a negative
> > value to a unsigned integer.
> 
> It's not an unsigned integer, it's an u8.
> 
> > Similar patch has been submitted by
> > Larry Finger earlier to silence the same spatch warning in another
> > file.
> 
> A similar warning but a totally different patch.
> 
> > diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c 
> > b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
> > index 352d381..41d2f3f 100644
> > --- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
> > +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
> > @@ -2310,7 +2310,7 @@ static void rtl8192_rx_normal(struct net_device *dev)
> >  
> > struct rtllib_rx_stats stats = {
> > .signal = 0,
> > -   .noise = -98,
> > +   .noise = 158, /*-98 -dBm*/
> 
> Let's just do this:
> 
>   .noise = (u8) -98,
> 
> That way it silences the warning and we don't need the comment.
> 
> regards,
> dan carpenter
> 

I am sending version 2 of this patch with the above said changes for
review.

thanks
hari prasath
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: rtl8192e: fix wrong assignment

2015-05-04 Thread Dan Carpenter
On Mon, May 04, 2015 at 07:21:52AM +, Gujulan Elango, Hari Prasath (H.) 
wrote:
> 
> Please exuse me for the replies being screwed up. I just figured out how to 
> reply to e-mails from Mutt.Hope you would be getting cleaner replies from now 
> on. 

No problem.  Please put line breaks at 72 characters as well.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/2] staging: ion: chunk_heap: use %pad for printing dma_addr_t's

2015-05-04 Thread Dan Carpenter
On Thu, Apr 09, 2015 at 06:10:04PM -0700, Mitchel Humpherys wrote:
> We're currently using %lu and %ld to print some variables of type
> dma_addr_t, which results in the following warning when dma_addr_t is
> 64-bits wide:
> 
> drivers/staging/android/ion/ion_chunk_heap.c: In function 
> 'ion_chunk_heap_create':
> drivers/staging/android/ion/ion_chunk_heap.c:176:2: warning: format '%lu' 
> expects argument of type 'long unsigned int', but argument 3 has type 
> 'dma_addr_t' [-Wformat=]
>   pr_info("%s: base %lu size %zu align %ld\n", __func__, chunk_heap->base,
>   ^
> drivers/staging/android/ion/ion_chunk_heap.c:176:2: warning: format '%ld' 
> expects argument of type 'long int', but argument 5 has type 'dma_addr_t' 
> [-Wformat=]
> 
> Fix this by using %pad as instructed in printk-formats.txt.
> 
> Signed-off-by: Mitchel Humpherys 

This one was just merged and I was about to email you that it introduces
some new Smatch warnings, but actually looking at it, it's just wrong.

We want to print "chunk_heap->base" and not "&chunk_heap->base".

And anyway "&chunk_heap->base" is a regular pointer, not a dma_addr_t.

So please send a new patch that removes the &.

regards,
dan carpenter

> ---
>  drivers/staging/android/ion/ion_chunk_heap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/android/ion/ion_chunk_heap.c 
> b/drivers/staging/android/ion/ion_chunk_heap.c
> index 54746157d799..6b3e18aa1c64 100644
> --- a/drivers/staging/android/ion/ion_chunk_heap.c
> +++ b/drivers/staging/android/ion/ion_chunk_heap.c
> @@ -173,8 +173,8 @@ struct ion_heap *ion_chunk_heap_create(struct 
> ion_platform_heap *heap_data)
>   chunk_heap->heap.ops = &chunk_heap_ops;
>   chunk_heap->heap.type = ION_HEAP_TYPE_CHUNK;
>   chunk_heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE;
> - pr_debug("%s: base %lu size %zu align %ld\n", __func__, 
> chunk_heap->base,
> - heap_data->size, heap_data->align);
> + pr_debug("%s: base %pad size %zu align %pad\n", __func__,
> + &chunk_heap->base, heap_data->size, &heap_data->align);
>  
>   return &chunk_heap->heap;
>  
> -- 
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
> ___
> devel mailing list
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: rtl8192e: fix wrong assignment

2015-05-04 Thread Gujulan Elango, Hari Prasath (H.)
On Thu, Apr 30, 2015 at 02:09:13PM -0500, Larry Finger wrote:
> On 04/30/2015 12:50 PM, Gujulan Elango, Hari Prasath (H.) wrote:
> >
> >From: Larry Finger  on behalf of Larry Finger 
> >
> >Sent: Thursday, April 30, 2015 11:03 PM
> >To: Gujulan Elango, Hari Prasath (H.); Dan Carpenter
> >Cc: de...@driverdev.osuosl.org; julia.law...@lip6.fr; 
> >gre...@linuxfoundation.org; wlan...@realtek.com; 
> >mateusz.kulikow...@gmail.com; Babu, Viswanathan (V.)
> >Subject: Re: [PATCH] staging: rtl8192e: fix wrong assignment
> >
> >On 04/30/2015 11:59 AM, Gujulan Elango, Hari Prasath (H.) wrote:
> >>From: Dan Carpenter 
> >>Sent: Thursday, April 30, 2015 6:33 PM
> >>To: Gujulan Elango, Hari Prasath (H.)
> >>Cc: de...@driverdev.osuosl.org; julia.law...@lip6.fr; 
> >>gre...@linuxfoundation.org; wlan...@realtek.com; 
> >>mateusz.kulikow...@gmail.com; Babu, Viswanathan (V.); 
> >>larry.fin...@lwfinger.net
> >>Subject: Re: [PATCH] staging: rtl8192e: fix wrong assignment
> >>
> >>On Thu, Apr 30, 2015 at 12:06:28PM +, Gujulan Elango, Hari Prasath (H.) 
> >>wrote:
> >>>This patch addresses a spatch warning on assigning a negative
> >>>value to a unsigned integer.Similar patch has been submitted by
> >>>Larry Finger earlier to silence the same spatch warning in another
> >>>file.
> >>
> >>What's the git hash and title for Larry's patch?
> >>
> >>>The logic is the negative number to an unsigned quantity is
> >>>fixed by adding 256 to -98 to get the equivalent negative number as
> >>>per Larry Finger.
> >>
> It was a spatch warning.I am not sure if that change by Larry went in,but 
> here's the link where I found a submission by Larry.
> >>http://permalink.gmane.org/gmane.linux.kernel.wireless.general/113125
> >
> >If you actually read that patch, you will see that the "fix" was to remove 
> >the
> >noise member initialization. The git hash, title, and commit message for the
> >previous change was as follows:
> >
> >commit 354d0f3c40fb40193213e40f3177ff528798ca8d
> >Author: Larry Finger 
> >Date:   Wed Sep 25 12:57:47 2013 -0500
> >
> >  rtlwifi: Fix smatch warnings in usb.c
> >
> >  Smatch displays the following:
> >CHECK   drivers/net/wireless/rtlwifi/usb.c
> >  drivers/net/wireless/rtlwifi/usb.c:458 _rtl_usb_rx_process_agg() warn:
> >assigning (-98) to unsigned variable 'stats.noise'
> >  drivers/net/wireless/rtlwifi/usb.c:503 _rtl_usb_rx_process_noagg() 
> > warn:
> >assigning (-98) to unsigned variable 'stats.noise'
> >  drivers/net/wireless/rtlwifi/usb.c:596 _rtl_rx_get_padding() info: 
> > ignoring
> >unreachable code.
> >  drivers/net/wireless/rtlwifi/usb.c:596 _rtl_rx_get_padding() info: 
> > ignoring
> >unreachable code.
> >
> >  The variable 'stats.noise' is not used, thus the initializers are 
> > removed.
> >  The unreachable code info is fixed by including the appropriate 
> > section inside
> >  #ifdef .. #endif constructions.
> >
> >  Signed-off-by: Larry Finger 
> >  Signed-off-by: John W. Linville 
> >
> >If you are going to be submitting kernel patches, then I suggest a tutorial 
> >on
> >the usage of git so that you can find previous commits.
> >
> >Unlike the rtlwifi drivers, this one appears to set the noise value, thus a
> >simple removal of the initialization is not appropriate. Your fix of setting 
> >the
> >value to 256-98 seems to be correct.
> >
> I did see the git log related to this file. I was not sure if I could 
> remove the variable right away assuming the variable is used somewhere. 
> Hence I sticked with just silencing the smatch warning.
> >
> >
> >
> 
> Obviously, you cannot remove the variable now.
> 
> Please fix your mailer. The most recent material to which you were
> replying was not indented any levels, but the "new" material was 3
> levels deep. It is extremely difficult to follow the thread.
> 
> Larry
>

Please exuse me for the replies being screwed up. I just figured out how to 
reply to e-mails from Mutt.Hope you would be getting cleaner replies from now 
on. 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel