from:"Julia Lawall"

Re: coccinelle and bitmask arithmetic (was: Re: [patch] TTY: synclink, small cleanup in dtr_rts())

2013-01-29 Thread Julia Lawall



On Tue, 29 Jan 2013, Joe Perches wrote:

> On Tue, 2013-01-29 at 10:55 -0500, valdis.kletni...@vt.edu wrote:
> > On Sun, 27 Jan 2013 23:19:47 +0300, Dan Carpenter said:
> >
> > > Yeah.  I think it would be, but adding bitflags together instead of
> > > doing bitwise ORs is very common as well.
> >
> > The fact it's common doesn't mean it's good programming practice,
> > or even correct.  Consider:
> >
> > #define F_FOO 0x01
> > #define F_BAR 0x02
> > #define F_BAZ 0x04
> >
> > unsigned int flags = F_FOO;
> > ...
> >   flags |= F_BAR;
> >
> > Now some time later, another code path does this:
> >
> >   flags += F_FOO;
> >
> > If it was another |, it would be a no harm no foul class of bug.
> > But how long is it going to take you to figure out who set F_BAZ?
> >
> > I wonder if there's a way to write a coccinelle patch to find places
> > where we do arithmetic operations on bitmasks
>
> Not so far as I know, but maybe someone on the
> cocci lists does. (cc'd)
>
> I could imagine a test for variables that have
> uses of both arithmetic and bit operations but
> not a discriminator for when one type is
> appropriate and the other is not.

If the definition of a bitmask is an identifier in all capital letters,
that would be easy.  Another possibility is such an identifier that is
defined to a value expressed beginning with 0x.  Another possibility is
such an identifier that is sometimes used with & and | and sometimes used
with an arithmetic operation.  I will give them a try.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: coccinelle and bitmask arithmetic (was: Re: [patch] TTY: synclink, small cleanup in dtr_rts())

2013-01-29 Thread Julia Lawall

How about the following (from today's linux-next).  They appear to be
trying to do the same calculation, once with + and once with |.

arch/arm/common/it8152.c

int dma_set_coherent_mask(struct device *dev, u64 mask)
{
if (mask >= PHYS_OFFSET + SZ_64M - 1)
return 0;

return -EIO;
}

static int it8152_pci_platform_notify(struct device *dev)
{
if (dev->bus == &pci_bus_type) {
if (dev->dma_mask)
*dev->dma_mask = (SZ_64M - 1) | PHYS_OFFSET;
dev->coherent_dma_mask = (SZ_64M - 1) | PHYS_OFFSET;
dmabounce_register_dev(dev, 2048, 4096, it8152_needs_bounce);
}
return 0;
}

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: coccinelle and bitmask arithmetic (was: Re: [patch] TTY: synclink, small cleanup in dtr_rts())

2013-01-29 Thread Julia Lawall

The following rule looks promising:

@r@
constant c;
identifier i;
expression e;
@@

(
e | c@i
|
e & c@i
|
e |= c@i
|
e &= c@i
)

@@
constant r.c,c1;
identifier i1;
expression e;
@@

*c1@i1 + c

That is, the sum of two constants where at least one of them has been used
with & or |.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: coccinelle and bitmask arithmetic

2013-01-30 Thread Julia Lawall



On Wed, 30 Jan 2013, Russell King - ARM Linux wrote:

> On Wed, Jan 30, 2013 at 09:21:28AM +0100, walter harms wrote:
> > Great hit Joe :)
> >
> > Sometimes i am really surprised what code can be found
> > in the kernal and it is still working.
> > Having no clue of the code i suspect somebody tries to
> > check is mask outside the range it should read
> > PHYS_OFFSET |( SZ_64M - 1)
> > maybe someone should tell them that
> > 1+1=10 while 1|1=1
> > It does not seem to matter here (or ... ?)
>
> This PCI host is only used on one platform (ARMCORE).
>
> For this, PHYS_OFFSET will be a value with only the top few bits of a
> 32-bit word set (such as 0xc000) - it's certainly not going to have
> any bits set below bit 26 on the platform this driver gets used on.
> "SZ_64M - 1" is the size of the window that RAM appears.
>
> So, _either_ logical OR or addition works.
>
> If we _did_ end up with a PHYS_OFFSET with bits less than bit 26 set
> here, we'd have bigger problems - because the base of RAM in PCI space
> will not correspond with PHYS_OFFSET and all the DMA mapping stuff breaks.

The "problem" is that the computation is done inconsistently within the
same file.  Sometimes with + and sometimes with |.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] scripts/coccinelle: find constant additions that could be bit ors

2013-02-02 Thread Julia Lawall

From: Julia Lawall 

Semantic patch (http://coccinelle.lip6.fr/) to check for constants that are
added but are used elsewhere as bitmasks.

Signed-off-by: Julia Lawall 

---
 scripts/coccinelle/misc/orplus.cocci |   55 +++
 1 file changed, 55 insertions(+)

diff --git a/scripts/coccinelle/misc/orplus.cocci 
b/scripts/coccinelle/misc/orplus.cocci
new file mode 100644
index 000..4a28cef
--- /dev/null
+++ b/scripts/coccinelle/misc/orplus.cocci
@@ -0,0 +1,55 @@
+/// Check for constants that are added but are used elsewhere as bitmasks
+/// The results should be checked manually to ensure that the nonzero
+/// bits in the two constants are actually disjoint.
+///
+// Confidence: Moderate
+// Copyright: (C) 2013 Julia Lawall, INRIA/LIP6.  GPLv2.
+// Copyright: (C) 2013 Gilles Muller, INRIA/LIP6.  GPLv2.
+// URL: http://coccinelle.lip6.fr/
+// Comments:
+// Options: -no_includes -include_headers
+
+virtual org
+virtual report
+virtual context
+
+@r@
+constant c;
+identifier i;
+expression e;
+@@
+
+(
+e | c@i
+|
+e & c@i
+|
+e |= c@i
+|
+e &= c@i
+)
+
+@s@
+constant r.c,c1;
+identifier i1;
+position p;
+@@
+
+(
+ c1 + c - 1
+|
+*c1@i1 +@p c
+)
+
+@script:python depends on org@
+p << s.p;
+@@
+
+cocci.print_main("sum of probable bitmasks, consider |",p)
+
+@script:python depends on report@
+p << s.p;
+@@
+
+msg = "WARNING: sum of probable bitmasks, consider |"
+coccilib.report.print_report(p[0],msg)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] scripts/coccinelle: find constant additions that could be bit ors

2013-02-23 Thread Julia Lawall

On Sat, 23 Feb 2013, Dan Carpenter wrote:

> On Fri, Feb 22, 2013 at 11:44:31AM +0100, Michal Marek wrote:
> > On Sat, Feb 02, 2013 at 05:19:55PM +0100, Julia Lawall wrote:
> > > From: Julia Lawall 
> > > 
> > > Semantic patch (http://coccinelle.lip6.fr/) to check for constants that 
> > > are
> > > added but are used elsewhere as bitmasks.
> > > 
> > > Signed-off-by: Julia Lawall 
> > 
> > Applied to kbuild.git#misc.
> > 
> 
> I'm not sure we want to do this.  The one patch which was sent with
> this script wasn't applied.

I think that the view on it was that if it isn't broken, don't fix it.  
But if the rules that are being added to the kernel are primarily being 
used for new code, then it could be reasonable for developers to consider 
whether they would really rather use | rather than the confusing +.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/2] drivers/gpu/drm/tilcdc/tilcdc_drv.c: adjust duplicate test

2013-02-24 Thread Julia Lawall

From: Julia Lawall 

Delete successive tests to the same location.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// 
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// 

Signed-off-by: Julia Lawall 

---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c 
b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index c5b592d..3ad0a63 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -192,7 +192,7 @@ static int tilcdc_load(struct drm_device *dev, unsigned 
long flags)
}
 
priv->disp_clk = clk_get(dev->dev, "dpll_disp_ck");
-   if (IS_ERR(priv->clk)) {
+   if (IS_ERR(priv->disp_clk)) {
dev_err(dev->dev, "failed to get display clock\n");
ret = -ENODEV;
goto fail;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/2] drivers/regulator/s5m8767.c: adjust duplicate test

2013-02-24 Thread Julia Lawall

From: Julia Lawall 

Delete successive tests to the same location.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// 
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// 

Signed-off-by: Julia Lawall 

---
 drivers/regulator/s5m8767.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/s5m8767.c b/drivers/regulator/s5m8767.c
index 8a83194..8e56198 100644
--- a/drivers/regulator/s5m8767.c
+++ b/drivers/regulator/s5m8767.c
@@ -549,7 +549,7 @@ static int s5m8767_pmic_dt_parse_pdata(struct 
platform_device *pdev,
 
rmode = devm_kzalloc(&pdev->dev, sizeof(*rmode) *
pdata->num_regulators, GFP_KERNEL);
-   if (!rdata) {
+   if (!rmode) {
dev_err(iodev->dev,
"could not allocate memory for regulator mode\n");
return -ENOMEM;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 0/2] adjust duplicate test

2013-02-24 Thread Julia Lawall

These patches fix cases where a value is tested that was previously
tested.  Often the problem is that the tested value has not been updated
properly.  Sometimes the test is simply duplicated.  These problems were
found using the following semantic match (http://coccinelle.lip6.fr/):

// 
@r exists@
expression e1,e2;
identifier f,g;
position p1,p2;
@@

if@p1 ( \(e1->f == NULL\|IS_ERR(e1->f)\) ) { ... when forall
   return ...; }
... when != \(e1->f = e2\|e1->f += e2\|e1->f -= e2\|e1->f |= e2\|e1->f &= 
e2\|e1->f++\|e1->f--\|g(...,e1,...)\)
if@p2 ( \(e1->f == NULL\|IS_ERR(e1->f)\) ) { ... when forall
   return ...; }

@rsame exists@
expression e;
position r.p1,r.p2;
statement S1,S2;
@@

if@p1 ( e ) S1
... when any
if@p2 ( e ) S2

@rok depends on rsame exists@
expression e1,e2;
identifier f;
position r.p1,r.p2;
statement S1,S2;
@@

if@p1 ( \(e1->f == NULL\|IS_ERR(e1->f)\) ) S1
... when any
 \(e1->f = e2\|e1->f += e2\|e1->f -= e2\|e1->f |= e2\|e1->f &= 
e2\|e1->f++\|e1->f--\|g(...,e1,...)\)
... when any
if@p2 ( ... ) S2

@rother depends on rsame exists@
position r.p1,r.p2;
statement S;
@@

... when != if@p1 (...) S
if@p2 ( ... ) { ... return ...; }

@script:python depends on rsame && !rok && !rother@
p1 << r.p1;
p2 << r.p2;
@@

if (p1[0].line != p2[0].line):
  cocci.print_main("",p1)
  cocci.print_secs("",p2)

@s exists@
local idexpression y;
expression e;
position p1,p2;
@@


if@p1 ( \(y == NULL\|IS_ERR(y)\|y != 0\) ) { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= 
e\|y++\|y--\|&y\|XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
if@p2 ( \(y == NULL\|IS_ERR(y)\|y != 0\) ) { ... when forall
   return ...; }

@ssame exists@
expression e;
position s.p1,s.p2;
statement S1,S2;
@@

if@p1 ( e ) S1
... when any
if@p2 ( e ) S2

@sok depends on ssame exists@
local idexpression y;
expression e;
position s.p1,s.p2;
statement S1,S2;
@@

if@p1 ( \(y == NULL\|IS_ERR(y)\|y != 0\) ) S1
... when any
 \(y = e\|y += e\|y -= e\|y |= e\|y &= 
e\|y++\|y--\|&y\|XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
... when any
if@p2 ( ... ) S2

@sother depends on ssame exists@
position s.p1,s.p2;
statement S;
@@

... when != if@p1 (...) S
if@p2 ( ... ) { ... return ...; }

@script:python depends on ssame && !sok && !sother@
p1 << s.p1;
p2 << s.p2;
@@

if (p1[0].line != p2[0].line):
  cocci.print_main("",p1)
  cocci.print_secs("",p2)

// 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: question about arch/arm/mach-s3c24xx/irq.c

2013-02-24 Thread Julia Lawall

[Adding the person who introduced the code]

On Sun, 24 Feb 2013, Russell King - ARM Linux wrote:

> On Sun, Feb 24, 2013 at 12:45:11PM +0100, Julia Lawall wrote:
> > The function s3c24xx_irq_map in arch/arm/mach-s3c24xx/irq.c contains the
> > code:
> >
> > parent_irq_data = &parent_intc->irqs[irq_data->parent_irq];
> > if (!irq_data) {
> > pr_err("irq-s3c24xx: no irq data found for hwirq 
> > %lu\n",
> >hw);
> > goto err;
> > }
> >
> > At this point irq_data has already been tested, so the null test on
> > irq_data does not look correct.  But I wonder if parent_irq_data could
> > ever be null here?
>
> That would be really obscure - because that would require parent_intc to
> be a "negative" pointer (to counter-act the indexing by
> irq_data->parent_irq).  So it looks to me like the above is redundant.

Even at its original definition irq_data seems unlikely to be NULL:

struct s3c_irq_intc *intc = h->host_data;
struct s3c_irq_data *irq_data = &intc->irqs[hw];
...
if (!irq_data) {
pr_err("irq-s3c24xx: no irq data found for hwirq %lu\n", hw);
return -EINVAL;
}

That is, it could be an invalid value, but whether it actually hits 0
would seem to depend on the value hw?

Heiko, is NULL really a possibility?

thanks,
julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: question about arch/arm/mach-s3c24xx/irq.c

2013-02-24 Thread Julia Lawall

On Sun, 24 Feb 2013, Heiko Stübner wrote:

> Am Sonntag, 24. Februar 2013, 14:39:45 schrieb Julia Lawall:
> > [Adding the person who introduced the code]
> >
> > On Sun, 24 Feb 2013, Russell King - ARM Linux wrote:
> > > On Sun, Feb 24, 2013 at 12:45:11PM +0100, Julia Lawall wrote:
> > > > The function s3c24xx_irq_map in arch/arm/mach-s3c24xx/irq.c contains
> > > > the
> > > >
> > > > code:
> > > > parent_irq_data =
> > > > &parent_intc->irqs[irq_data->parent_irq];
> > > >
> > > > if (!irq_data) {
> > > >
> > > > pr_err("irq-s3c24xx: no irq data found for
> > > > hwirq %lu\n",
> > > >
> > > >hw);
> > > >
> > > > goto err;
> > > >
> > > > }
> > > >
> > > > At this point irq_data has already been tested, so the null test on
> > > > irq_data does not look correct.  But I wonder if parent_irq_data could
> > > > ever be null here?
> > >
> > > That would be really obscure - because that would require parent_intc to
> > > be a "negative" pointer (to counter-act the indexing by
> > > irq_data->parent_irq).  So it looks to me like the above is redundant.
> >
> > Even at its original definition irq_data seems unlikely to be NULL:
> >
> > struct s3c_irq_intc *intc = h->host_data;
> > struct s3c_irq_data *irq_data = &intc->irqs[hw];
> > ...
> > if (!irq_data) {
> > pr_err("irq-s3c24xx: no irq data found for hwirq %lu\n",
> > hw); return -EINVAL;
> > }
> >
> > That is, it could be an invalid value, but whether it actually hits 0
> > would seem to depend on the value hw?
> >
> > Heiko, is NULL really a possibility?
>
> The test you quoted is of course wrong ... it would need to test
> parent_irq_data. But you're also right that the test is not necessary at all.
>
> All the s3c_irq_data arrays used always contain 32 entries to reach all bits
> of the register (which is used differently on each SoC). So if we have found
> the parent_intc at all, it should contain a 32 entries array of irq_data
> structs, so no need to test for the existence of the individual array element.
>
>
> And now that I look at it, I also see another glitch. The code tests for
> parent_irq != 0, which of course won't work if the parent_irq is the 0-hwirq
> of the parent controller.
> The only SoC using such a mapping is the s3c2412 [0], which explains why I
> haven't been bitten by this myself.

Do you want to make all the fixes?

julia

[PATCH 0/7] fix error return code

2012-08-29 Thread Julia Lawall

These patches fix cases where the return code appears to be unintentially
nonnegative.

The complete semantic match that finds the problem is as follows:
(http://coccinelle.lip6.fr/)

// 
@ok exists@
identifier f,ret,i;
expression e;
constant c;
@@

f(...) {
<+...
(
return -c@i;
|
ret = -c@i;
... when != ret = e
return ret;
|
if (ret < 0) { ... return ret; }
)
...+> }

@r exists@
identifier ret,l,ok.f;
expression e1,e2,e3;
statement S;
position p1,p2,p3;
@@

f(...) {
... when any
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != &ret
(
 if (<+... ret = e3 ...+>) S
|
 if (<+... &ret ...+>) S
|
if@p2(...)
 {
  ... when != ret = e2
  when forall
 return@p3 ret;
}
)
... when any
}

@bad exists@
position r.p1,r.p2;
statement S1,S2;
identifier r.ret;
expression e1;
@@

(
if@p1 (\(ret < 0\|ret != 0\)) S1
|
ret@p1 = 0
)
... when any
ret = e1
... when any
if@p2(...) S2

@bad2@
position r.p1,r.p2;
identifier r.ret;
expression e1;
statement S2;
@@

ret@p1 = 0
... when != if (...) { ... ret = e1 ... return ret; }
when any
if@p2(...) S2


@script:python depends on !bad && !bad2@
p1 << r.p1;
p2 << r.p2;
p3 << r.p3;
@@

cocci.print_main("",p1)
cocci.print_secs("",p2)
cocci.print_secs("",p3)
// 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/7] net: ipv6: fix error return code

2012-08-29 Thread Julia Lawall

From: Julia Lawall 

Initialize return variable before exiting on an error path.

The initial initialization of the return variable is also dropped, because
that value is never used.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// 
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != &ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// 

Signed-off-by: Julia Lawall 

---
 net/ipv6/esp6.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 6dc7fd3..282f372 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -167,8 +167,6 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff 
*skb)
struct esp_data *esp = x->data;
 
/* skb is pure payload to encrypt */
-   err = -ENOMEM;
-
aead = esp->aead;
alen = crypto_aead_authsize(aead);
 
@@ -203,8 +201,10 @@ static int esp6_output(struct xfrm_state *x, struct 
sk_buff *skb)
}
 
tmp = esp_alloc_tmp(aead, nfrags + sglists, seqhilen);
-   if (!tmp)
+   if (!tmp) {
+   err = -ENOMEM;
goto error;
+   }
 
seqhi = esp_tmp_seqhi(tmp);
iv = esp_tmp_iv(aead, tmp, seqhilen);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 7/7] net/netfilter/nf_conntrack_netlink.c: fix error return code

2012-08-29 Thread Julia Lawall

From: Julia Lawall 

Initialize return variable before exiting on an error path.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// 
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != &ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// 

Signed-off-by: Julia Lawall 

---
 net/netfilter/nf_conntrack_netlink.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_netlink.c 
b/net/netfilter/nf_conntrack_netlink.c
index da4fc37..9807f32 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -2790,7 +2790,8 @@ static int __init ctnetlink_init(void)
goto err_unreg_subsys;
}
 
-   if (register_pernet_subsys(&ctnetlink_net_ops)) {
+   ret = register_pernet_subsys(&ctnetlink_net_ops);
+   if (ret < 0) {
pr_err("ctnetlink_init: cannot register pernet operations\n");
goto err_unreg_exp_subsys;
}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/7] ipvs: fix error return code

2012-08-29 Thread Julia Lawall

From: Julia Lawall 

Initialize return variable before exiting on an error path.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// 
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != &ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// 

Signed-off-by: Julia Lawall 

---
 net/netfilter/ipvs/ip_vs_ctl.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 3c60137..767cc12 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1171,8 +1171,10 @@ ip_vs_add_service(struct net *net, struct 
ip_vs_service_user_kern *u,
goto out_err;
}
svc->stats.cpustats = alloc_percpu(struct ip_vs_cpu_stats);
-   if (!svc->stats.cpustats)
+   if (!svc->stats.cpustats) {
+   ret = -ENOMEM;
goto out_err;
+   }
 
/* I'm the first user of the service */
atomic_set(&svc->usecnt, 0);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/7] logfs: fix error return code

2012-08-29 Thread Julia Lawall

From: Julia Lawall 

Initialize return variable before exiting on an error path.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// 
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != &ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// 

Signed-off-by: Julia Lawall 

---
 fs/logfs/segment.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/logfs/segment.c b/fs/logfs/segment.c
index 038da09..0cf91e4 100644
--- a/fs/logfs/segment.c
+++ b/fs/logfs/segment.c
@@ -601,6 +601,7 @@ static int __logfs_segment_read(struct inode *inode, void 
*buf,
"%llx: expected %x, got %x\n", ofs,
be32_to_cpu(oh.data_crc),
be32_to_cpu(crc));
+   err = -EIO;
goto out_err;
}
break;
@@ -619,6 +620,7 @@ static int __logfs_segment_read(struct inode *inode, void 
*buf,
be32_to_cpu(oh.data_crc),
be32_to_cpu(crc));
mutex_unlock(&logfs_super(sb)->s_journal_mutex);
+   err = -EIO;
goto out_err;
}
err = logfs_uncompress(compressor_buf, buf, len, block_len);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 6/7] net/netfilter/nfnetlink_log.c: fix error return code

2012-08-29 Thread Julia Lawall

From: Julia Lawall 

Initialize return variable before exiting on an error path.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// 
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != &ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// 

Signed-off-by: Julia Lawall 

---
 net/netfilter/nfnetlink_log.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 4142aac..9ef5438 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -1002,8 +1002,10 @@ static int __init nfnetlink_log_init(void)
 
 #ifdef CONFIG_PROC_FS
if (!proc_create("nfnetlink_log", 0440,
-proc_net_netfilter, &nful_file_ops))
+proc_net_netfilter, &nful_file_ops)) {
+   status = -ENOMEM;
goto cleanup_logger;
+   }
 #endif
return status;
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 5/7] net/xfrm/xfrm_state.c: fix error return code

2012-08-29 Thread Julia Lawall

From: Julia Lawall 

Initialize return variable before exiting on an error path.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// 
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != &ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// 

Signed-off-by: Julia Lawall 

---
 net/xfrm/xfrm_state.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 7856c33..b68e0cc 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1994,8 +1994,10 @@ int __xfrm_init_state(struct xfrm_state *x, bool 
init_replay)
goto error;
 
x->outer_mode = xfrm_get_mode(x->props.mode, family);
-   if (x->outer_mode == NULL)
+   if (x->outer_mode == NULL) {
+   err = -EPROTONOSUPPORT;
goto error;
+   }
 
if (init_replay) {
err = xfrm_init_replay(x);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/7] net/bluetooth/rfcomm/core.c: fix error return code

2012-08-29 Thread Julia Lawall

From: Julia Lawall 

Initialize return variable before exiting on an error path.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// 
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != &ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// 

Signed-off-by: Julia Lawall 

---
 net/bluetooth/rfcomm/core.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index c75107e..30141c0 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -2010,8 +2010,10 @@ static int rfcomm_add_listener(bdaddr_t *ba)
 
/* Add listening session */
s = rfcomm_session_add(sock, BT_LISTEN);
-   if (!s)
+   if (!s) {
+   err = -ENOMEM;
goto failed;
+   }
 
rfcomm_session_hold(s);
return 0;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 0/4] drop frees of devm_ alloc'd data

2012-09-01 Thread Julia Lawall

Drop frees of devm_ alloc'd data.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/4] drivers/spi/spi-sh-hspi.c: drop frees of devm_ alloc'd data

2012-09-01 Thread Julia Lawall

From: Julia Lawall 

devm free functions should not have to be explicitly used.

A semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// 
@@
@@

(
* devm_kfree(...);
|
* devm_free_irq(...);
|
* devm_iounmap(...);
|
* devm_release_region(...);
|
* devm_release_mem_region(...);
)
// 

Signed-off-by: Julia Lawall 

---
 drivers/spi/spi-sh-hspi.c |5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/spi/spi-sh-hspi.c b/drivers/spi/spi-sh-hspi.c
index 934138c..796c077 100644
--- a/drivers/spi/spi-sh-hspi.c
+++ b/drivers/spi/spi-sh-hspi.c
@@ -283,7 +283,7 @@ static int __devinit hspi_probe(struct platform_device 
*pdev)
ret = spi_register_master(master);
if (ret < 0) {
dev_err(&pdev->dev, "spi_register_master error.\n");
-   goto error2;
+   goto error1;
}
 
pm_runtime_enable(&pdev->dev);
@@ -292,8 +292,6 @@ static int __devinit hspi_probe(struct platform_device 
*pdev)
 
return 0;
 
- error2:
-   devm_iounmap(hspi->dev, hspi->addr);
  error1:
clk_put(clk);
  error0:
@@ -310,7 +308,6 @@ static int __devexit hspi_remove(struct platform_device 
*pdev)
 
clk_put(hspi->clk);
spi_unregister_master(hspi->master);
-   devm_iounmap(hspi->dev, hspi->addr);
 
return 0;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups

2012-09-01 Thread Julia Lawall

From: Julia Lawall 

devm free functions should not have to be explicitly used.

The only thing left that is useful in the function mpc5121_nfc_free is the
call to clk_disable, which is moved to the call sites.

This function also incorrectly called iounmap on devm_ioremap allocated
data.

Use devm_request_and_ioremap in place of devm_request_mem_region and
devm_ioremap.

Use devm_clk_get.

A semantic match that finds the first problem is as follows:
(http://coccinelle.lip6.fr/)

// 
@@
@@

(
* devm_kfree(...);
|
* devm_free_irq(...);
|
* devm_iounmap(...);
|
* devm_release_region(...);
|
* devm_release_mem_region(...);
)
// 

Signed-off-by: Julia Lawall 

---
 drivers/mtd/nand/mpc5121_nfc.c |   35 +--
 1 file changed, 5 insertions(+), 30 deletions(-)

diff --git a/drivers/mtd/nand/mpc5121_nfc.c b/drivers/mtd/nand/mpc5121_nfc.c
index c259c24..45183ba 100644
--- a/drivers/mtd/nand/mpc5121_nfc.c
+++ b/drivers/mtd/nand/mpc5121_nfc.c
@@ -632,21 +632,6 @@ out:
return ret;
 }
 
-/* Free driver resources */
-static void mpc5121_nfc_free(struct device *dev, struct mtd_info *mtd)
-{
-   struct nand_chip *chip = mtd->priv;
-   struct mpc5121_nfc_prv *prv = chip->priv;
-
-   if (prv->clk) {
-   clk_disable(prv->clk);
-   clk_put(prv->clk);
-   }
-
-   if (prv->csreg)
-   iounmap(prv->csreg);
-}
-
 static int __devinit mpc5121_nfc_probe(struct platform_device *op)
 {
struct device_node *rootnode, *dn = op->dev.of_node;
@@ -713,12 +698,7 @@ static int __devinit mpc5121_nfc_probe(struct 
platform_device *op)
regs_paddr = res.start;
regs_size = resource_size(&res);
 
-   if (!devm_request_mem_region(dev, regs_paddr, regs_size, DRV_NAME)) {
-   dev_err(dev, "Error requesting memory region!\n");
-   return -EBUSY;
-   }
-
-   prv->regs = devm_ioremap(dev, regs_paddr, regs_size);
+   prv->regs = devm_request_and_ioremap(dev, &res);
if (!prv->regs) {
dev_err(dev, "Error mapping memory region!\n");
return -ENOMEM;
@@ -752,11 +732,10 @@ static int __devinit mpc5121_nfc_probe(struct 
platform_device *op)
of_node_put(rootnode);
 
/* Enable NFC clock */
-   prv->clk = clk_get(dev, "nfc_clk");
+   prv->clk = devm_clk_get(dev, "nfc_clk");
if (IS_ERR(prv->clk)) {
dev_err(dev, "Unable to acquire NFC clock!\n");
-   retval = PTR_ERR(prv->clk);
-   goto error;
+   return PTR_ERR(prv->clk);
}
 
clk_enable(prv->clk);
@@ -803,7 +782,6 @@ static int __devinit mpc5121_nfc_probe(struct 
platform_device *op)
/* Detect NAND chips */
if (nand_scan(mtd, be32_to_cpup(chips_no))) {
dev_err(dev, "NAND Flash not found !\n");
-   devm_free_irq(dev, prv->irq, mtd);
retval = -ENXIO;
goto error;
}
@@ -828,7 +806,6 @@ static int __devinit mpc5121_nfc_probe(struct 
platform_device *op)
 
default:
dev_err(dev, "Unsupported NAND flash!\n");
-   devm_free_irq(dev, prv->irq, mtd);
retval = -ENXIO;
goto error;
}
@@ -839,13 +816,12 @@ static int __devinit mpc5121_nfc_probe(struct 
platform_device *op)
retval = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
if (retval) {
dev_err(dev, "Error adding MTD device!\n");
-   devm_free_irq(dev, prv->irq, mtd);
goto error;
}
 
return 0;
 error:
-   mpc5121_nfc_free(dev, mtd);
+   clk_disable(prv->clk);
return retval;
 }
 
@@ -857,8 +833,7 @@ static int __devexit mpc5121_nfc_remove(struct 
platform_device *op)
struct mpc5121_nfc_prv *prv = chip->priv;
 
nand_release(mtd);
-   devm_free_irq(dev, prv->irq, mtd);
-   mpc5121_nfc_free(dev, mtd);
+   clk_disable(prv->clk);
 
return 0;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/4] drivers/mtd/maps/autcpu12-nvram.c: drop frees of devm_ alloc'd data

2012-09-01 Thread Julia Lawall

From: Julia Lawall 

devm free functions should not have to be explicitly used.

A semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// 
@@
@@

(
* devm_kfree(...);
|
* devm_free_irq(...);
|
* devm_iounmap(...);
|
* devm_release_region(...);
|
* devm_release_mem_region(...);
)
// 

Signed-off-by: Julia Lawall 

---
 drivers/mtd/maps/autcpu12-nvram.c |   18 --
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/maps/autcpu12-nvram.c 
b/drivers/mtd/maps/autcpu12-nvram.c
index ef420d9..76fb594 100644
--- a/drivers/mtd/maps/autcpu12-nvram.c
+++ b/drivers/mtd/maps/autcpu12-nvram.c
@@ -38,7 +38,6 @@ static int __devinit autcpu12_nvram_probe(struct 
platform_device *pdev)
map_word tmp, save0, save1;
struct resource *res;
struct autcpu12_nvram_priv *priv;
-   int err;
 
priv = devm_kzalloc(&pdev->dev,
sizeof(struct autcpu12_nvram_priv), GFP_KERNEL);
@@ -50,8 +49,7 @@ static int __devinit autcpu12_nvram_probe(struct 
platform_device *pdev)
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
dev_err(&pdev->dev, "failed to get memory resource\n");
-   err = -ENOENT;
-   goto out;
+   return -ENOENT;
}
 
priv->map.bankwidth = 4;
@@ -61,8 +59,7 @@ static int __devinit autcpu12_nvram_probe(struct 
platform_device *pdev)
strcpy((char *)priv->map.name, res->name);
if (!priv->map.virt) {
dev_err(&pdev->dev, "failed to remap mem resource\n");
-   err = -EBUSY;
-   goto out;
+   return -EBUSY;
}
 
simple_map_init(&priv->map);
@@ -90,8 +87,7 @@ static int __devinit autcpu12_nvram_probe(struct 
platform_device *pdev)
priv->mtd = do_map_probe("map_ram", &priv->map);
if (!priv->mtd) {
dev_err(&pdev->dev, "probing failed\n");
-   err = -ENXIO;
-   goto out;
+   return -ENXIO;
}
 
priv->mtd->owner= THIS_MODULE;
@@ -106,12 +102,7 @@ static int __devinit autcpu12_nvram_probe(struct 
platform_device *pdev)
 
map_destroy(priv->mtd);
dev_err(&pdev->dev, "NV-RAM device addition failed\n");
-   err = -ENOMEM;
-
-out:
-   devm_kfree(&pdev->dev, priv);
-
-   return err;
+   return -ENOMEM;
 }
 
 static int __devexit autcpu12_nvram_remove(struct platform_device *pdev)
@@ -120,7 +111,6 @@ static int __devexit autcpu12_nvram_remove(struct 
platform_device *pdev)
 
mtd_device_unregister(priv->mtd);
map_destroy(priv->mtd);
-   devm_kfree(&pdev->dev, priv);
 
return 0;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/4] drivers/tty/serial/sirfsoc_uart.c: drop frees of devm_ alloc'd data

2012-09-01 Thread Julia Lawall

From: Julia Lawall 

devm free functions should not have to be explicitly used.

A semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// 
@@
@@

(
* devm_kfree(...);
|
* devm_free_irq(...);
|
* devm_iounmap(...);
|
* devm_release_region(...);
|
* devm_release_mem_region(...);
)
// 

Signed-off-by: Julia Lawall 

---
diff --git a/drivers/tty/serial/sirfsoc_uart.c 
b/drivers/tty/serial/sirfsoc_uart.c
index 5b3eda2..a9e2bd1 100644
--- a/drivers/tty/serial/sirfsoc_uart.c
+++ b/drivers/tty/serial/sirfsoc_uart.c
@@ -668,7 +668,7 @@ int sirfsoc_uart_probe(struct platform_device *pdev)
if (res == NULL) {
dev_err(&pdev->dev, "Insufficient resources.\n");
ret = -EFAULT;
-   goto irq_err;
+   goto err;
}
port->irq = res->start;
 
@@ -676,7 +676,7 @@ int sirfsoc_uart_probe(struct platform_device *pdev)
sirfport->p = pinctrl_get_select_default(&pdev->dev);
ret = IS_ERR(sirfport->p);
if (ret)
-   goto pin_err;
+   goto err;
}
 
port->ops = &sirfsoc_uart_ops;
@@ -695,9 +695,6 @@ port_err:
platform_set_drvdata(pdev, NULL);
if (sirfport->hw_flow_ctrl)
pinctrl_put(sirfport->p);
-pin_err:
-irq_err:
-   devm_iounmap(&pdev->dev, port->membase);
 err:
return ret;
 }
@@ -709,7 +706,6 @@ static int sirfsoc_uart_remove(struct platform_device *pdev)
platform_set_drvdata(pdev, NULL);
if (sirfport->hw_flow_ctrl)
pinctrl_put(sirfport->p);
-   devm_iounmap(&pdev->dev, port->membase);
uart_remove_one_port(&sirfsoc_uart_drv, port);
return 0;
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups

2012-09-04 Thread Julia Lawall

On Tue, 4 Sep 2012, Lars-Peter Clausen wrote:

> On 09/04/2012 10:42 AM, Artem Bityutskiy wrote:
> > Aiaiai! :-) [1] [2]
> >
> > I've build-tested this using aiaiai and it reports that this change breaks 
> > the build:
> >
> > dedekind@blue:~/git/maintaining$ ./verify ../l2-mtd/ mpc5121_nfc < 
> > ~/tmp/julia2.mbox
> > Tested the patch(es) on top of the following commits:
> > ba64756 Quick fixes - applied by aiaiai
> > 651c6fa JFFS2: don't fail on bitflips in OOB
> > e22ac84 mtd: autcpu12-nvram: drop frees of devm_ alloc'd data
> > ea9d312 mtd: cmdlinepart: minor cleanups
> >
> > 
> > Failed to build the following commit for configuration 
> > "powerpc-mpc512x_defconfig" (architecture powerpc)":
> >
> > 0fe13ab drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups
> >
> > ...
> > drivers/mtd/nand/mpc5121_nfc.c: In function 'mpc5121_nfc_probe':
> > drivers/mtd/nand/mpc5121_nfc.c:622:28: warning: variable 'regs_size' set 
> > but not used [-Wunused-but-set-variable]
> > drivers/mtd/nand/mpc5121_nfc.c:622:16: warning: variable 'regs_paddr' set 
> > but not used [-Wunused-but-set-variable]
> > drivers/built-in.o: In function `mpc5121_nfc_probe':
> > mpc5121_nfc.c:(.devinit.text+0x2a14): undefined reference to `devm_clk_get'
> > make[1]: *** [vmlinux] Error 1
> >
> > 
> >
> > I do not really know why, but it seems that clock framework is not 
> > supported for powerpc. CCing the PPC mailing list. Preserved the context 
> > below for the PPC people.
> >
>
> I've been bitten by the same issue recently, also cause by one of these
> cocci devm patches. devm_clk_get is only available if the generic
> clk_get/clk_put implementation is used. Not all architectures do this and
> some implement their own clk_get/clk_put, etc functions. Since devm_clk_get
> is merely a wrapper around clk_get/clk_put there is no reason why it should
> depend CLKDEV_LOOKUP. I've prepared a patch which makes them generically
> available if the clk_get/clk_put are implemented (i.e. if HAVE_CLK is set),
> but it is on a different machine right now, will try to submit it later today.

Sorry about this.  I wasn't aware that devm_clk_get wasn't supported by
all architectures, and I have no way of compiling code for these
architectures...  But I wonder why it is not, since devm-ness doesn't seem
to have anything to do with architecture-specific details?  It would be
really nice to have it for all architectures, because the clock functions
are just as (or at least almost as) common as kzalloc, ioremap, etc.

thanks,
julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] arch/arm/mach-s3c24xx/mach-h1940.c: delete double assignment

2012-09-12 Thread Julia Lawall

From: Julia Lawall 

Delete successive assignments to the same location.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// 
@@
expression i;
@@

*i = ...;
 i = ...;
// 

Signed-off-by: Julia Lawall 

---
Not compiled, and this may change the behavior of the code.  Without this
change, check_gpio2 could possibly be used uninitialized later.

 arch/arm/mach-s3c24xx/mach-h1940.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-s3c24xx/mach-h1940.c 
b/arch/arm/mach-s3c24xx/mach-h1940.c
index bb8d008..48c 100644
--- a/arch/arm/mach-s3c24xx/mach-h1940.c
+++ b/arch/arm/mach-s3c24xx/mach-h1940.c
@@ -380,7 +380,7 @@ int h1940_led_blink_set(unsigned gpio, int state,
default:
blink_gpio = S3C2410_GPA(3);
check_gpio1 = S3C2410_GPA(1);
-   check_gpio1 = S3C2410_GPA(7);
+   check_gpio2 = S3C2410_GPA(7);
break;
}
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code

2012-10-04 Thread Julia Lawall


On Thu, 4 Oct 2012, Joe Perches wrote:


On Thu, 2012-10-04 at 14:54 -0400, David Miller wrote:

From: Peter Senna Tschudin 

On Thu, Oct 4, 2012 at 8:23 PM, David Miller  wrote:

We want to know the implications of the bug being fixed.
Does it potentially cause an OOPS?  Bad reference counting and thus
potential leaks or early frees?

You have to analyze the implications and ramifications of the bug
being fixed.  We need that information.


You are asking for deeper level analysis that may not
be reasonably possible from the robotic patch fixed
by a robotic piece of a bit of code correction via a
static analysis.


As Peter pointed out, it is not even a robotic fix, if robotic means 
literally that it was done by a robot.  A tool was used to find a 
potential problem, and then Peter studied the code to see what fix was 
appropriate.


In this case, finding out the impact, requires looking up the call chain 
in all directions to find out what the callers do with the returned value. 
And understanding the likely impact along the way.  Often the call chain 
involves function pointers.  This is all possible to do.  And perhaps it 
would be even more helpful to the consumers of the patch than just having 
the problem fixed.  But the fact remains that at other places in the 
function, someone thought it was useful to return an error code, so in the 
place where the error code return is missing, it seems like a useful fault 
to fix, whether it has an impact now or might have one later.  The main 
human analysis required to generate the patch is to be convinced that the 
given return path really represents an error condition and that the 
function normally returns the specified kind of value in that case.  If 
there is some way in which these points are unclear, then the commit

message should certainly explain the reasoning that was used.

julia




It's just "bad error code, this is the script that fixed it, kthx,
bye" which is pretty much useless for anaylsis.


Which may be all but impossible but for the handful of
folks that know all the gory/intimate details of the
original bit of code.


What does it potentially cause the caller to do?  Will it potentially
treat an error as a success and as a result register an object
illegally?

Real analysis please.  The text you provided above is basically still
robotic and could be used to describe any error code return fix.


Right, it's useful to attempt but may be infeasible in
practice.



--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code

2012-10-05 Thread Julia Lawall


On Fri, 5 Oct 2012, Joe Perches wrote:


On Fri, 2012-10-05 at 07:22 +0200, Julia Lawall wrote:

A tool was used to find a potential problem, and then Peter
studied the code to see what fix was appropriate.


Hi Julia.

Was it true that a static analysis tool found the original
potential issue?  If so, what tool was it?


In the very beginning, I think that I found the problem in a patch when 
looking at patches that contain oopses.


From that I wrote a Coccinelle rule.  As Peter showed, the rule just 
produces a list of line numbers.  The fix cannot easily be automated, 
because there are many cases where 0 is a valid error value.  Some 
functions, for example, have their error value as a nonpositive integer.



But wasn't the scripted fix applied to the rest of the tree
robotically?


No.  Peter studied each case and considered what should be done, and then 
did that.  I guess a potentially bad fix could have been applied 
automatically and then cleaned up manually, but considering the number of 
cases where the fix would be wrong, that seem like a bad idea.  Also one 
might want to adapt a bit to local conventions about where the 
initialization should be added.


julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/6] arch/x86/xen: Use DIV_ROUND_UP

2008-02-14 Thread Julia Lawall

From: Julia Lawall <[EMAIL PROTECTED]>

The kernel.h macro DIV_ROUND_UP performs the computation (((n) + (d) - 1) /
(d)) but is perhaps more readable.

An extract of the semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// 
@haskernel@
@@

#include 

@depends on haskernel@
expression n,d;
@@

(
- (n + d - 1) / d
+ DIV_ROUND_UP(n,d)
|
- (n + (d - 1)) / d
+ DIV_ROUND_UP(n,d)
)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP((n),d)
+ DIV_ROUND_UP(n,d)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP(n,(d))
+ DIV_ROUND_UP(n,d)
// 

Signed-off-by: Julia Lawall <[EMAIL PROTECTED]>

---

diff -u -p a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
--- a/arch/x86/xen/enlighten.c 2008-02-02 15:28:17.0 +0100
+++ b/arch/x86/xen/enlighten.c 2008-02-13 20:48:15.0 +0100
@@ -293,7 +293,7 @@ static void xen_load_gdt(const struct de
unsigned long *frames;
unsigned long va = dtr->address;
unsigned int size = dtr->size + 1;
-   unsigned pages = (size + PAGE_SIZE - 1) / PAGE_SIZE;
+   unsigned pages = DIV_ROUND_UP(size, PAGE_SIZE);
int f;
struct multicall_space mcs;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/6] fs/direct-io.c: Use DIV_ROUND_UP

2008-02-14 Thread Julia Lawall

From: Julia Lawall <[EMAIL PROTECTED]>

The kernel.h macro DIV_ROUND_UP performs the computation (((n) + (d) - 1) /
(d)) but is perhaps more readable.

An extract of the semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// 
@haskernel@
@@

#include 

@depends on haskernel@
expression n,d;
@@

(
- (n + d - 1) / d
+ DIV_ROUND_UP(n,d)
|
- (n + (d - 1)) / d
+ DIV_ROUND_UP(n,d)
)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP((n),d)
+ DIV_ROUND_UP(n,d)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP(n,(d))
+ DIV_ROUND_UP(n,d)
// 

Signed-off-by: Julia Lawall <[EMAIL PROTECTED]>

---

diff -u -p a/fs/direct-io.c b/fs/direct-io.c
--- a/fs/direct-io.c 2008-02-08 08:58:17.0 +0100
+++ b/fs/direct-io.c 2008-02-13 20:58:53.0 +0100
@@ -976,7 +976,7 @@ direct_io_worker(int rw, struct kiocb *i
for (seg = 0; seg < nr_segs; seg++) {
user_addr = (unsigned long)iov[seg].iov_base;
dio->pages_in_io +=
-   ((user_addr+iov[seg].iov_len +PAGE_SIZE-1)/PAGE_SIZE
+   (DIV_ROUND_UP(user_addr + iov[seg].iov_len, PAGE_SIZE)
- user_addr/PAGE_SIZE);
}

@@ -998,7 +998,7 @@ direct_io_worker(int rw, struct kiocb *i
dio->total_pages++;
bytes -= PAGE_SIZE - (user_addr & (PAGE_SIZE - 1));
}
-   dio->total_pages += (bytes + PAGE_SIZE - 1) / PAGE_SIZE;
+   dio->total_pages += DIV_ROUND_UP(bytes, PAGE_SIZE);
dio->curr_user_address = user_addr;

ret = do_direct_IO(dio);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/6] fs/ocfs2: Use DIV_ROUND_UP

2008-02-14 Thread Julia Lawall

From: Julia Lawall <[EMAIL PROTECTED]>

The kernel.h macro DIV_ROUND_UP performs the computation (((n) + (d) - 1) /
(d)) but is perhaps more readable.

An extract of the semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// 
@haskernel@
@@

#include 

@depends on haskernel@
expression n,d;
@@

(
- (n + d - 1) / d
+ DIV_ROUND_UP(n,d)
|
- (n + (d - 1)) / d
+ DIV_ROUND_UP(n,d)
)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP((n),d)
+ DIV_ROUND_UP(n,d)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP(n,(d))
+ DIV_ROUND_UP(n,d)
// 

Signed-off-by: Julia Lawall <[EMAIL PROTECTED]>

---

diff -u -p a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
--- a/fs/ocfs2/cluster/heartbeat.c 2007-12-31 08:43:31.0 +0100
+++ b/fs/ocfs2/cluster/heartbeat.c 2008-02-13 20:59:54.0 +0100
@@ -1149,7 +1149,7 @@ static int o2hb_map_slot_data(struct o2h
slot->ds_raw_block = NULL;
}

-   reg->hr_num_pages = (reg->hr_blocks + spp - 1) / spp;
+   reg->hr_num_pages = DIV_ROUND_UP(reg->hr_blocks, spp);
mlog(ML_HEARTBEAT, "Going to require %u pages to cover %u blocks "
   "at %u blocks per page\n",
 reg->hr_num_pages, reg->hr_blocks, spp);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/6] fs/udf: Use DIV_ROUND_UP

2008-02-14 Thread Julia Lawall

From: Julia Lawall <[EMAIL PROTECTED]>

The kernel.h macro DIV_ROUND_UP performs the computation (((n) + (d) - 1) /
(d)) but is perhaps more readable.

An extract of the semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// 
@haskernel@
@@

#include 

@depends on haskernel@
expression n,d;
@@

(
- (n + d - 1) / d
+ DIV_ROUND_UP(n,d)
|
- (n + (d - 1)) / d
+ DIV_ROUND_UP(n,d)
)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP((n),d)
+ DIV_ROUND_UP(n,d)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP(n,(d))
+ DIV_ROUND_UP(n,d)
// 

Signed-off-by: Julia Lawall <[EMAIL PROTECTED]>

---

diff -u -p a/fs/udf/super.c b/fs/udf/super.c
--- a/fs/udf/super.c2008-02-10 17:13:55.0 +0100
+++ b/fs/udf/super.c2008-02-14 07:32:31.0 +0100
@@ -1025,10 +1025,9 @@ static void udf_load_fileset(struct supe
 int udf_compute_nr_groups(struct super_block *sb, u32 partition)
 {
struct udf_part_map *map = &UDF_SB(sb)->s_partmaps[partition];
-   return (map->s_partition_len +
-   (sizeof(struct spaceBitmapDesc) << 3) +
-   (sb->s_blocksize * 8) - 1) /
-   (sb->s_blocksize * 8);
+   return DIV_ROUND_UP(map->s_partition_len +
+   (sizeof(struct spaceBitmapDesc) << 3),
+   sb->s_blocksize * 8);
 }

 static struct udf_bitmap *udf_sb_alloc_bitmap(struct super_block *sb, u32 
index)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 5/6] drivers/atm: Use DIV_ROUND_UP

2008-02-14 Thread Julia Lawall

In each case below, I have followed the original semantics, but in
drivers/atm/eni.c and drivers/atm/horizon.c, I have some doubts as to
whether the original semantics is correct.  In drivers/atm/eni.c, is the
division intended to be by div or by div-1?  In drivers/atm/horizon.c, it
seems strange that "case round_down" is implemented by DIV_ROUND_UP, twice.
The round_down and default (ie round_up) cases seem to be inversed.

---

From: Julia Lawall <[EMAIL PROTECTED]>

The kernel.h macro DIV_ROUND_UP performs the computation (((n) + (d) - 1) /
(d)) but is perhaps more readable.

An extract of the semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// 
@haskernel@
@@

#include 

@depends on haskernel@
expression n,d;
@@

(
- (n + d - 1) / d
+ DIV_ROUND_UP(n,d)
|
- (n + (d - 1)) / d
+ DIV_ROUND_UP(n,d)
)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP((n),d)
+ DIV_ROUND_UP(n,d)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP(n,(d))
+ DIV_ROUND_UP(n,d)
// 

Signed-off-by: Julia Lawall <[EMAIL PROTECTED]>

---

diff -u -p a/drivers/atm/eni.c b/drivers/atm/eni.c
--- a/drivers/atm/eni.c 2007-07-20 15:28:28.0 +0200
+++ b/drivers/atm/eni.c 2008-02-13 20:50:10.0 +0100
@@ -1270,7 +1270,7 @@ static int comp_tx(struct eni_dev *eni_d
if (*pre < 3) (*pre)++; /* else fail later */
div = pre_div[*pre]*-*pcr;
DPRINTK("max div %d\n",div);
-   *res = (TS_CLOCK+div-1)/div-1;
+   *res = DIV_ROUND_UP(TS_CLOCK, div)-1;
}
if (*res < 0) *res = 0;
if (*res > MID_SEG_MAX_RATE) *res = MID_SEG_MAX_RATE;
diff -u -p a/drivers/atm/horizon.c b/drivers/atm/horizon.c
--- a/drivers/atm/horizon.c 2007-11-08 18:33:26.0 +0100
+++ b/drivers/atm/horizon.c 2008-02-13 20:50:13.0 +0100
@@ -635,7 +635,7 @@ static int make_rate (const hrz_dev * de
// take care of rounding
switch (r) {
case round_down:
-   pre = (br+(c<qos.rxtp.max_sdu = 65464;
/* fix this - we may want to receive 64kB SDUs
   later */
-   cells = (vcc->qos.rxtp.max_sdu+ATM_AAL5_TRAILER+
-   ATM_CELL_PAYLOAD-1)/ATM_CELL_PAYLOAD;
+   cells = DIV_ROUND_UP(vcc->qos.rxtp.max_sdu+ATM_AAL5_TRAILER,
+ATM_CELL_PAYLOAD);
zatm_vcc->pool = pool_index(cells*ATM_CELL_PAYLOAD);
}
else {
@@ -820,7 +820,7 @@ static int alloc_shaper(struct atm_dev *
}
else {
i = 255;
-   m = (ATM_OC3_PCR*255+max-1)/max;
+   m = DIV_ROUND_UP(ATM_OC3_PCR*255, max);
}
}
if (i > m) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 6/6] drivers/hid: Use DIV_ROUND_UP

2008-02-14 Thread Julia Lawall

From: Julia Lawall <[EMAIL PROTECTED]>

The kernel.h macro DIV_ROUND_UP performs the computation (((n) + (d) - 1) /
(d)) but is perhaps more readable.

An extract of the semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// 
@haskernel@
@@

#include 

@depends on haskernel@
expression n,d;
@@

(
- (n + d - 1) / d
+ DIV_ROUND_UP(n,d)
|
- (n + (d - 1)) / d
+ DIV_ROUND_UP(n,d)
)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP((n),d)
+ DIV_ROUND_UP(n,d)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP(n,(d))
+ DIV_ROUND_UP(n,d)
// 

Signed-off-by: Julia Lawall <[EMAIL PROTECTED]>

---

diff -u -p a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
--- a/drivers/hid/usbhid/hid-core.c 2007-11-08 18:33:32.0 +0100
+++ b/drivers/hid/usbhid/hid-core.c 2008-02-13 20:51:00.0 +0100
@@ -278,7 +278,7 @@ static int hid_submit_ctrl(struct hid_de
usbhid->urbctrl->pipe = usb_rcvctrlpipe(hid_to_usb_dev(hid), 0);
maxpacket = usb_maxpacket(hid_to_usb_dev(hid), 
usbhid->urbctrl->pipe, 0);
if (maxpacket > 0) {
-   padlen = (len + maxpacket - 1) / maxpacket;
+   padlen = DIV_ROUND_UP(len, maxpacket);
padlen *= maxpacket;
if (padlen > usbhid->bufsize)
padlen = usbhid->bufsize;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/6] fs/direct-io.c: Use DIV_ROUND_UP

2008-02-14 Thread Julia Lawall

On Thu, 14 Feb 2008, Pekka Enberg wrote:

> Hi Nishanth,
> 
> On Thu, Feb 14, 2008 at 7:38 PM, Nishanth Aravamudan <[EMAIL PROTECTED]> 
> wrote:
> >  Is it just me, or does
> >
> > ((user_addr + iov[seg].iov_len + PAGE_SIZE - 1)/PAGE_SIZE - 
> > user_addr/PAGE_SIZE)
> >
> >  not simplify to
> >
> > = ((iov[seg].iov_len + PAGE_SIZE - 1)/PAGE_SIZE + 
> > user_addr/PAGE_SIZE - user_addr/PAGE_SIZE)
> >
> > = ((iov[seg].iov_len + PAGE_SIZE - 1)/PAGE_SIZE)
> >
> > = DIV_ROUND_UP(iov[seg].iov_len, PAGE_SIZE)
> >
> >  CMIIW.
> 
> I double-checked this and I believe you're correct. It's simpler to
> see when you do:
> 
>   x = user_addr
>   y = iov[seg].iov_len
>   z = PAGE_SIZE
> 
> So
> 
>   (x + y + z - 1)/z - x/z
> 
>   = [x + (y + z - 1)]/z - x/z
> 
>   = [xz + z(y + z - 1)]/z^2 - x/z
> 
>   = x/z + (y + z - 1)/z - x/z
> 
> And the rest follows from your simplifications.

It doesn't work:

 ((3+4+5-1)/5) - (3/5) = 2
 ((4+5-1)/5) = 1

julia

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/4] : More use DIV_ROUND_UP

2008-02-16 Thread Julia Lawall

From: Julia Lawall <[EMAIL PROTECTED]>

The kernel.h macro DIV_ROUND_UP performs the computation (((n) + (d) - 1) /
(d)) but is perhaps more readable.

An extract of the semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// 
@haskernel@
@@

#include 

@depends on haskernel@
expression n,d;
@@

(
- (n + d - 1) / d
+ DIV_ROUND_UP(n,d)
|
- (n + (d - 1)) / d
+ DIV_ROUND_UP(n,d)
)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP((n),d)
+ DIV_ROUND_UP(n,d)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP(n,(d))
+ DIV_ROUND_UP(n,d)
// 

Signed-off-by: Julia Lawall <[EMAIL PROTECTED]>

---

diff -u -p a/drivers/ide/arm/palm_bk3710.c b/drivers/ide/arm/palm_bk3710.c
--- a/drivers/ide/arm/palm_bk3710.c 2008-02-13 20:42:50.0 +0100
+++ b/drivers/ide/arm/palm_bk3710.c 2008-02-15 22:25:33.0 +0100
@@ -96,11 +96,11 @@ static void palm_bk3710_setudmamode(void
u16 val16;

/* DMA Data Setup */
-   t0 = (palm_bk3710_udmatimings[mode].cycletime + ide_palm_clk - 1)
-   / ide_palm_clk - 1;
-   tenv = (20 + ide_palm_clk - 1) / ide_palm_clk - 1;
-   trp = (palm_bk3710_udmatimings[mode].rptime + ide_palm_clk - 1)
-   / ide_palm_clk - 1;
+   t0 = DIV_ROUND_UP(palm_bk3710_udmatimings[mode].cycletime,
+ ide_palm_clk) - 1;
+   tenv = DIV_ROUND_UP(20, ide_palm_clk) - 1;
+   trp = DIV_ROUND_UP(palm_bk3710_udmatimings[mode].rptime,
+  ide_palm_clk) - 1;

/* udmatim Register */
val16 = readw(base + BK3710_UDMATIM) & (dev ? 0xFF0F : 0xFFF0);
@@ -141,8 +141,8 @@ static void palm_bk3710_setdmamode(void
cycletime = max_t(int, t->cycle, min_cycle);

/* DMA Data Setup */
-   t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
-   td = (t->active + ide_palm_clk - 1) / ide_palm_clk;
+   t0 = DIV_ROUND_UP(cycletime, ide_palm_clk);
+   td = DIV_ROUND_UP(t->active, ide_palm_clk);
tkw = t0 - td - 1;
td -= 1;

@@ -168,9 +168,9 @@ static void palm_bk3710_setpiomode(void
struct ide_timing *t;

/* PIO Data Setup */
-   t0 = (cycletime + ide_palm_clk - 1) / ide_palm_clk;
-   t2 = (ide_timing_find_mode(XFER_PIO_0 + mode)->active +
- ide_palm_clk - 1) / ide_palm_clk;
+   t0 = DIV_ROUND_UP(cycletime, ide_palm_clk);
+   t2 = DIV_ROUND_UP(ide_timing_find_mode(XFER_PIO_0 + mode)->active,
+ ide_palm_clk);

t2i = t0 - t2 - 1;
t2 -= 1;
@@ -192,8 +192,8 @@ static void palm_bk3710_setpiomode(void

/* TASKFILE Setup */
t = ide_timing_find_mode(XFER_PIO_0 + mode);
-   t0 = (t->cyc8b + ide_palm_clk - 1) / ide_palm_clk;
-   t2 = (t->act8b + ide_palm_clk - 1) / ide_palm_clk;
+   t0 = DIV_ROUND_UP(t->cyc8b, ide_palm_clk);
+   t2 = DIV_ROUND_UP(t->act8b, ide_palm_clk);

t2i = t0 - t2 - 1;
t2 -= 1;
diff -u -p a/drivers/ide/pci/cmd640.c b/drivers/ide/pci/cmd640.c
--- a/drivers/ide/pci/cmd640.c 2008-02-08 08:58:12.0 +0100
+++ b/drivers/ide/pci/cmd640.c 2008-02-15 22:25:38.0 +0100
@@ -584,15 +584,15 @@ static void cmd640_set_mode (unsigned in
active_time = ide_pio_timings[pio_mode].active_time;
recovery_time = cycle_time - (setup_time + active_time);
clock_time = 1000 / bus_speed;
-   cycle_count = (cycle_time + clock_time - 1) / clock_time;
+   cycle_count = DIV_ROUND_UP(cycle_time, clock_time);

-   setup_count = (setup_time + clock_time - 1) / clock_time;
+   setup_count = DIV_ROUND_UP(setup_time, clock_time);

-   active_count = (active_time + clock_time - 1) / clock_time;
+   active_count = DIV_ROUND_UP(active_time, clock_time);
if (active_count < 2)
active_count = 2; /* minimum allowed by cmd640 */

-   recovery_count = (recovery_time + clock_time - 1) / clock_time;
+   recovery_count = DIV_ROUND_UP(recovery_time, clock_time);
recovery_count2 = cycle_count - (setup_count + active_count);
if (recovery_count2 > recovery_count)
recovery_count = recovery_count2;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/4] : More use DIV_ROUND_UP or roundup

2008-02-16 Thread Julia Lawall

From: Julia Lawall <[EMAIL PROTECTED]>

The kernel.h macro DIV_ROUND_UP performs the computation (((n) + (d) - 1) /
(d)) but is perhaps more readable.

An extract of the semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// 
@haskernel@
@@

#include 

@depends on haskernel@
expression n,d;
@@

(
- (n + d - 1) / d
+ DIV_ROUND_UP(n,d)
|
- (n + (d - 1)) / d
+ DIV_ROUND_UP(n,d)
)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP((n),d)
+ DIV_ROUND_UP(n,d)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP(n,(d))
+ DIV_ROUND_UP(n,d)
// 

Signed-off-by: Julia Lawall <[EMAIL PROTECTED]>

---

diff -u -p a/drivers/video/atafb.c b/drivers/video/atafb.c
--- a/drivers/video/atafb.c 2007-11-08 18:33:49.0 +0100
+++ b/drivers/video/atafb.c 2008-02-15 22:31:30.0 +0100
@@ -1270,7 +1270,7 @@ again:

gstart = (prescale / 2 + plen * left_margin) / prescale;
/* gend1 is for hde (gend-gstart multiple of align), shifter's xres */
-   gend1 = gstart + ((xres + align - 1) / align) * align * plen / prescale;
+   gend1 = gstart + roundup(xres, align) * plen / prescale;
/* gend2 is for hbb, visible xres (rest to gend1 is cut off by hblank) 
*/
gend2 = gstart + xres * plen / prescale;
par->HHT = plen * (left_margin + xres + right_margin) /
diff -u -p a/drivers/video/cirrusfb.c b/drivers/video/cirrusfb.c
--- a/drivers/video/cirrusfb.c 2007-11-08 18:33:49.0 +0100
+++ b/drivers/video/cirrusfb.c 2008-02-15 22:31:35.0 +0100
@@ -3117,7 +3117,7 @@ static void bestclock(long freq, long *b
}
}
}
-   d = ((143181 * n) + f - 1) / f;
+   d = DIV_ROUND_UP(143181 * n, f);
if ((d >= 7) && (d <= 63)) {
if (d > 31)
d = (d / 2) * 2;
diff -u -p a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
--- a/drivers/video/console/fbcon.c 2008-02-08 08:58:16.0 +0100
+++ b/drivers/video/console/fbcon.c 2008-02-15 22:31:36.0 +0100
@@ -620,8 +620,7 @@ static void fbcon_prepare_logo(struct vc
if (fb_get_color_depth(&info->var, &info->fix) == 1)
erase &= ~0x400;
logo_height = fb_prepare_logo(info, ops->rotate);
-   logo_lines = (logo_height + vc->vc_font.height - 1) /
-   vc->vc_font.height;
+   logo_lines = DIV_ROUND_UP(logo_height, vc->vc_font.height);
q = (unsigned short *) (vc->vc_origin +
vc->vc_size_row * rows);
step = logo_lines * cols;
diff -u -p a/drivers/video/gxt4500.c b/drivers/video/gxt4500.c
--- a/drivers/video/gxt4500.c 2007-11-08 18:33:50.0 +0100
+++ b/drivers/video/gxt4500.c 2008-02-15 22:31:41.0 +0100
@@ -238,7 +238,7 @@ static int calc_pll(int period_ps, struc
for (pdiv1 = 1; pdiv1 <= 8; ++pdiv1) {
for (pdiv2 = 1; pdiv2 <= pdiv1; ++pdiv2) {
postdiv = pdiv1 * pdiv2;
-   pll_period = (period_ps + postdiv - 1) / postdiv;
+   pll_period = DIV_ROUND_UP(period_ps, postdiv);
/* keep pll in range 350..600 MHz */
if (pll_period < 1666 || pll_period > 2857)
continue;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/4] : More use DIV_ROUND_UP

2008-02-16 Thread Julia Lawall

From: Julia Lawall <[EMAIL PROTECTED]>

The kernel.h macro DIV_ROUND_UP performs the computation (((n) + (d) - 1) /
(d)) but is perhaps more readable.

An extract of the semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// 
@haskernel@
@@

#include 

@depends on haskernel@
expression n,d;
@@

(
- (n + d - 1) / d
+ DIV_ROUND_UP(n,d)
|
- (n + (d - 1)) / d
+ DIV_ROUND_UP(n,d)
)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP((n),d)
+ DIV_ROUND_UP(n,d)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP(n,(d))
+ DIV_ROUND_UP(n,d)
// 

Signed-off-by: Julia Lawall <[EMAIL PROTECTED]>

---

diff -u -p a/drivers/usb/host/r8a66597-hcd.c b/drivers/usb/host/r8a66597-hcd.c
--- a/drivers/usb/host/r8a66597-hcd.c 2007-12-31 08:43:30.0 +0100
+++ b/drivers/usb/host/r8a66597-hcd.c 2008-02-15 22:31:12.0 +0100
@@ -959,9 +959,9 @@ static void prepare_packet_read(struct r
r8a66597_write(r8a66597, TRCLR,
td->pipe->pipetre);
r8a66597_write(r8a66597,
-   (urb->transfer_buffer_length
-   + td->maxpacket - 1)
-   / td->maxpacket,
+   DIV_ROUND_UP
+ (urb->transfer_buffer_length,
+  td->maxpacket),
td->pipe->pipetrn);
r8a66597_bset(r8a66597, TRENB,
td->pipe->pipetre);
diff -u -p a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
--- a/drivers/usb/misc/usbtest.c 2008-02-02 15:28:32.0 +0100
+++ b/drivers/usb/misc/usbtest.c 2008-02-15 22:31:16.0 +0100
@@ -1403,7 +1403,7 @@ static struct urb *iso_alloc_urb (
return NULL;
maxp = 0x7ff & le16_to_cpu(desc->wMaxPacketSize);
maxp *= 1 + (0x3 & (le16_to_cpu(desc->wMaxPacketSize) >> 11));
-   packets = (bytes + maxp - 1) / maxp;
+   packets = DIV_ROUND_UP(bytes, maxp);

urb = usb_alloc_urb (packets, GFP_KERNEL);
if (!urb)
diff -u -p a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
--- a/drivers/usb/serial/ftdi_sio.c 2008-02-02 15:28:32.0 +0100
+++ b/drivers/usb/serial/ftdi_sio.c 2008-02-15 22:31:19.0 +0100
@@ -1380,7 +1380,7 @@ static void ftdi_write_bulk_callback (st
data_offset = priv->write_offset;
if (data_offset > 0) {
/* Subtract the control bytes */
-   countback -= (data_offset * ((countback + (PKTSZ - 1)) / 
PKTSZ));
+   countback -= (data_offset * DIV_ROUND_UP(countback, PKTSZ));
}
spin_lock_irqsave(&priv->tx_lock, flags);
--priv->tx_outstanding_urbs;
@@ -1480,7 +1480,7 @@ static void ftdi_read_bulk_callback (str

/* count data bytes, but not status bytes */
countread = urb->actual_length;
-   countread -= 2 * ((countread + (PKTSZ - 1)) / PKTSZ);
+   countread -= 2 * DIV_ROUND_UP(countread, PKTSZ);
spin_lock_irqsave(&priv->rx_lock, flags);
priv->rx_bytes += countread;
spin_unlock_irqrestore(&priv->rx_lock, flags);
diff -u -p a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
--- a/drivers/usb/serial/io_ti.c 2008-02-02 15:28:32.0 +0100
+++ b/drivers/usb/serial/io_ti.c 2008-02-15 22:31:21.0 +0100
@@ -655,7 +655,7 @@ static void TIChasePort(struct edgeport_
/* (TIIsTxActive doesn't seem to wait for the last byte) */
if ((baud_rate=port->baud_rate) == 0)
baud_rate = 50;
-   msleep(max(1,(1+baud_rate-1)/baud_rate));
+   msleep(max(1, DIV_ROUND_UP(1, baud_rate)));
 }

 static int TIChooseConfiguration (struct usb_device *dev)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/4] : More use DIV_ROUND_UP

2008-02-16 Thread Julia Lawall

From: Julia Lawall <[EMAIL PROTECTED]>

The kernel.h macro DIV_ROUND_UP performs the computation (((n) + (d) - 1) /
(d)) but is perhaps more readable.

An extract of the semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// 
@haskernel@
@@

#include 

@depends on haskernel@
expression n,d;
@@

(
- (n + d - 1) / d
+ DIV_ROUND_UP(n,d)
|
- (n + (d - 1)) / d
+ DIV_ROUND_UP(n,d)
)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP((n),d)
+ DIV_ROUND_UP(n,d)

@depends on haskernel@
expression n,d;
@@

- DIV_ROUND_UP(n,(d))
+ DIV_ROUND_UP(n,d)
// 

Signed-off-by: Julia Lawall <[EMAIL PROTECTED]>

---

diff -u -p a/drivers/s390/net/claw.c b/drivers/s390/net/claw.c
--- a/drivers/s390/net/claw.c 2008-02-02 15:28:31.0 +0100
+++ b/drivers/s390/net/claw.c 2008-02-15 22:29:44.0 +0100
@@ -2114,8 +2114,7 @@ init_ccw_bk(struct net_device *dev)
 */
 ccw_blocks_perpage= PAGE_SIZE /  CCWBK_SIZE;
 ccw_pages_required=
-   (ccw_blocks_required+ccw_blocks_perpage -1) /
-ccw_blocks_perpage;
+   DIV_ROUND_UP(ccw_blocks_required, ccw_blocks_perpage);

 #ifdef DEBUGMSG
 printk(KERN_INFO "%s: %s() > ccw_blocks_perpage=%d\n",
@@ -2133,12 +2132,12 @@ init_ccw_bk(struct net_device *dev)
  */
 if (privptr->p_env->read_size < PAGE_SIZE) {
 claw_reads_perpage= PAGE_SIZE / privptr->p_env->read_size;
-claw_read_pages= (privptr->p_env->read_buffers +
-   claw_reads_perpage -1) / claw_reads_perpage;
+claw_read_pages=
+   DIV_ROUND_UP(privptr->p_env->read_buffers, claw_reads_perpage);
  }
  else {   /* > or equal  */
 privptr->p_buff_pages_perread=
-   (privptr->p_env->read_size + PAGE_SIZE - 1) / PAGE_SIZE;
+   DIV_ROUND_UP(privptr->p_env->read_size, PAGE_SIZE);
 claw_read_pages=
privptr->p_env->read_buffers * privptr->p_buff_pages_perread;
  }
@@ -2146,13 +2145,13 @@ init_ccw_bk(struct net_device *dev)
 claw_writes_perpage=
PAGE_SIZE / privptr->p_env->write_size;
 claw_write_pages=
-   (privptr->p_env->write_buffers + claw_writes_perpage -1) /
-   claw_writes_perpage;
+   DIV_ROUND_UP(privptr->p_env->write_buffers,
+claw_writes_perpage);

 }
 else {  /* >  or equal  */
 privptr->p_buff_pages_perwrite=
-(privptr->p_env->read_size + PAGE_SIZE - 1) / PAGE_SIZE;
+   DIV_ROUND_UP(privptr->p_env->read_size, PAGE_SIZE);
 claw_write_pages=
privptr->p_env->write_buffers * privptr->p_buff_pages_perwrite;
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Expressing linux-next collateral evolutions in formal grammar

2012-11-02 Thread Julia Lawall


The way I would envision this to be easier to manage is to break them
down by subsystem and the reviewer can then go and read the grammar
for their own subsystem of preference. The long term benefit of this
is that even if folks don't use SmPL for collateral evolutions we have
a possibility here of being able to backport a collateral evolution by
using the inverse of the SmPL grammar, if we can get to the point of
coming up with formal and proper grammar on each collateral evolution.

Thoughts?


I had liked the idea of lwn, because that makes it public but doesn't 
bother anyone.  But another option would be to just send an email to the 
relevant maintainer?  Perhaps it would be better to send the email just 
to the specific maintainer for the file, and not all the way up the 
maintainer hierarchy, because these would be just suggestions of things to 
look at, and not actual patch proposals.


thanks,
julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 0/16] use WARN

2012-11-03 Thread Julia Lawall

These patches use WARN, which combines printk and WARN_ON(1), or WARN_ONCE,
which combines printk and WARN_ON_ONCE(1).  This does not appear to affect
the behavior, but makes the code a little more concise.

The semantic patch that makes this transformation is as follows
(http://coccinelle.lip6.fr/).  In particular, it only transforms the case
where the WARN_ON or WARN_ON_ONCE is preceded by a single printk.

// 
@bad1@
position p;
@@

printk(...);
printk@p(...);
WARN_ON(1);

@ok1@
expression list es;
position p != bad1.p;
@@

-printk@p(
+WARN(1,
  es);
-WARN_ON(1);

@@
expression list ok1.es;
@@

if (...)
- {
  WARN(1,es);
- }

@bad2@
position p;
@@

printk(...);
printk@p(...);
WARN_ON_ONCE(1);

@ok2@
expression list es;
position p != bad2.p;
@@

-printk@p(
+WARN_ONCE(1,
  es);
-WARN_ON_ONCE(1);

@@
expression list ok2.es;
@@

if (...)
- {
  WARN_ONCE(1,es);
- }
// 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/16] drivers/md/raid5.c: use WARN

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// 

Signed-off-by: Julia Lawall 

---
 drivers/md/raid5.c |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 320df0c..8c3b9bb 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -373,13 +373,11 @@ static void init_stripe(struct stripe_head *sh, sector_t 
sector, int previous)
struct r5dev *dev = &sh->dev[i];
 
if (dev->toread || dev->read || dev->towrite || dev->written ||
-   test_bit(R5_LOCKED, &dev->flags)) {
-   printk(KERN_ERR "sector=%llx i=%d %p %p %p %p %d\n",
+   test_bit(R5_LOCKED, &dev->flags))
+   WARN(1, KERN_ERR "sector=%llx i=%d %p %p %p %p %d\n",
   (unsigned long long)sh->sector, i, dev->toread,
   dev->read, dev->towrite, dev->written,
   test_bit(R5_LOCKED, &dev->flags));
-   WARN_ON(1);
-   }
dev->flags = 0;
raid5_build_block(sh, i, previous);
}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 5/16] drivers/scsi: use WARN

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// 

Signed-off-by: Julia Lawall 

---
 drivers/scsi/initio.c   |3 +--
 drivers/scsi/scsi_lib.c |3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/initio.c b/drivers/scsi/initio.c
index dd741bc..1572860 100644
--- a/drivers/scsi/initio.c
+++ b/drivers/scsi/initio.c
@@ -2774,8 +2774,7 @@ static void i91uSCBPost(u8 * host_mem, u8 * cblk_mem)
host = (struct initio_host *) host_mem;
cblk = (struct scsi_ctrl_blk *) cblk_mem;
if ((cmnd = cblk->srb) == NULL) {
-   printk(KERN_ERR "i91uSCBPost: SRB pointer is empty\n");
-   WARN_ON(1);
+   WARN(1, KERN_ERR "i91uSCBPost: SRB pointer is empty\n");
initio_release_scb(host, cblk); /* Release SCB for current 
channel */
return;
}
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index da36a3a..e5fdcae 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2573,10 +2573,9 @@ void *scsi_kmap_atomic_sg(struct scatterlist *sgl, int 
sg_count,
}
 
if (unlikely(i == sg_count)) {
-   printk(KERN_ERR "%s: Bytes in sg: %zu, requested offset %zu, "
+   WARN(1, KERN_ERR "%s: Bytes in sg: %zu, requested offset %zu, "
"elements %d\n",
   __func__, sg_len, *offset, sg_count);
-   WARN_ON(1);
return NULL;
}
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 10/16] drivers/net/ethernet/ibm/emac/mal.c: use WARN

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// 

Signed-off-by: Julia Lawall 

---
 drivers/net/ethernet/ibm/emac/mal.c |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/mal.c 
b/drivers/net/ethernet/ibm/emac/mal.c
index 479e43e..84c6b6c 100644
--- a/drivers/net/ethernet/ibm/emac/mal.c
+++ b/drivers/net/ethernet/ibm/emac/mal.c
@@ -738,13 +738,11 @@ static int __devexit mal_remove(struct platform_device 
*ofdev)
/* Synchronize with scheduled polling */
napi_disable(&mal->napi);
 
-   if (!list_empty(&mal->list)) {
+   if (!list_empty(&mal->list))
/* This is *very* bad */
-   printk(KERN_EMERG
+   WARN(1, KERN_EMERG
   "mal%d: commac list is not empty on remove!\n",
   mal->index);
-   WARN_ON(1);
-   }
 
dev_set_drvdata(&ofdev->dev, NULL);
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 11/16] drivers/misc/kgdbts.c: use WARN

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// 

Signed-off-by: Julia Lawall 

---
 drivers/misc/kgdbts.c |5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c
index 3aa9a96..8b367db 100644
--- a/drivers/misc/kgdbts.c
+++ b/drivers/misc/kgdbts.c
@@ -114,9 +114,8 @@
touch_nmi_watchdog();   \
} while (0)
 #define eprintk(a...) do { \
-   printk(KERN_ERR a); \
-   WARN_ON(1); \
-   } while (0)
+   WARN(1, KERN_ERR a); \
+   } while (0)
 #define MAX_CONFIG_LEN 40
 
 static struct kgdb_io kgdbts_io_ops;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 12/16] fs/logfs/gc.c: use WARN

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// 

Signed-off-by: Julia Lawall 

---
 fs/logfs/gc.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/logfs/gc.c b/fs/logfs/gc.c
index d4efb06..62dee03 100644
--- a/fs/logfs/gc.c
+++ b/fs/logfs/gc.c
@@ -55,9 +55,8 @@ static u8 root_distance(struct super_block *sb, gc_level_t 
__gc_level)
/* inode file data or indirect blocks */
return super->s_ifile_levels - (gc_level - 6);
default:
-   printk(KERN_ERR"LOGFS: segment of unknown level %x found\n",
+   WARN(1, KERN_ERR "LOGFS: segment of unknown level %x found\n",
gc_level);
-   WARN_ON(1);
return super->s_ifile_levels + super->s_iblock_levels;
}
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 16/16] drivers/infiniband/hw/nes: use WARN

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// 

Signed-off-by: Julia Lawall 

---
 drivers/infiniband/hw/nes/nes_cm.c  |6 ++
 drivers/infiniband/hw/nes/nes_mgt.c |6 ++
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/hw/nes/nes_cm.c 
b/drivers/infiniband/hw/nes/nes_cm.c
index cfaacaf..8a15a1d 100644
--- a/drivers/infiniband/hw/nes/nes_cm.c
+++ b/drivers/infiniband/hw/nes/nes_cm.c
@@ -629,11 +629,9 @@ static void build_rdma0_msg(struct nes_cm_node *cm_node, 
struct nes_qp **nesqp_a
 
case SEND_RDMA_READ_ZERO:
default:
-   if (cm_node->send_rdma0_op != SEND_RDMA_READ_ZERO) {
-   printk(KERN_ERR "%s[%u]: Unsupported RDMA0 len 
operation=%u\n",
+   if (cm_node->send_rdma0_op != SEND_RDMA_READ_ZERO)
+   WARN(1, KERN_ERR "%s[%u]: Unsupported RDMA0 len 
operation=%u\n",
 __func__, __LINE__, cm_node->send_rdma0_op);
-   WARN_ON(1);
-   }
nes_debug(NES_DBG_CM, "Sending first rdma operation.\n");
wqe->wqe_words[NES_IWARP_SQ_WQE_MISC_IDX] =
cpu_to_le32(NES_IWARP_SQ_OP_RDMAR);
diff --git a/drivers/infiniband/hw/nes/nes_mgt.c 
b/drivers/infiniband/hw/nes/nes_mgt.c
index 3ba7be3..53bb88c 100644
--- a/drivers/infiniband/hw/nes/nes_mgt.c
+++ b/drivers/infiniband/hw/nes/nes_mgt.c
@@ -649,11 +649,9 @@ static void nes_chg_qh_handler(struct nes_device *nesdev, 
struct nes_cqp_request
nesqp = qh_chg->nesqp;
 
/* Should we handle the bad completion */
-   if (cqp_request->major_code) {
-   printk(KERN_ERR PFX "Invalid cqp_request major_code=0x%x\n",
+   if (cqp_request->major_code)
+   WARN(1, KERN_ERR PFX "Invalid cqp_request major_code=0x%x\n",
   cqp_request->major_code);
-   WARN_ON(1);
-   }
 
switch (nesqp->pau_state) {
case PAU_DEL_QH:

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 15/16] drivers/parisc/pdc_stable.c: use WARN

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// 

Signed-off-by: Julia Lawall 

---
 drivers/parisc/pdc_stable.c |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/parisc/pdc_stable.c b/drivers/parisc/pdc_stable.c
index 246a92f..0f54ab6 100644
--- a/drivers/parisc/pdc_stable.c
+++ b/drivers/parisc/pdc_stable.c
@@ -212,12 +212,10 @@ pdcspath_store(struct pdcspath_entry *entry)
entry, devpath, entry->addr);
 
/* addr, devpath and count must be word aligned */
-   if (pdc_stable_write(entry->addr, devpath, sizeof(*devpath)) != PDC_OK) 
{
-   printk(KERN_ERR "%s: an error occurred when writing to PDC.\n"
+   if (pdc_stable_write(entry->addr, devpath, sizeof(*devpath)) != PDC_OK)
+   WARN(1, KERN_ERR "%s: an error occurred when writing to PDC.\n"
"It is likely that the Stable Storage data has 
been corrupted.\n"
"Please check it carefully upon next 
reboot.\n", __func__);
-   WARN_ON(1);
-   }

/* kobject is already registered */
entry->ready = 2;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/16] drivers/usb/wusbcore: use WARN

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// 

Signed-off-by: Julia Lawall 

---
 drivers/usb/wusbcore/wa-xfer.c |3 +--
 drivers/usb/wusbcore/wusbhc.c  |3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/wusbcore/wa-xfer.c b/drivers/usb/wusbcore/wa-xfer.c
index 57c01ab..1b80601 100644
--- a/drivers/usb/wusbcore/wa-xfer.c
+++ b/drivers/usb/wusbcore/wa-xfer.c
@@ -1124,9 +1124,8 @@ int wa_urb_dequeue(struct wahc *wa, struct urb *urb)
switch (seg->status) {
case WA_SEG_NOTREADY:
case WA_SEG_READY:
-   printk(KERN_ERR "xfer %p#%u: dequeue bad state %u\n",
+   WARN(1, KERN_ERR "xfer %p#%u: dequeue bad state %u\n",
   xfer, cnt, seg->status);
-   WARN_ON(1);
break;
case WA_SEG_DELAYED:
seg->status = WA_SEG_ABORTED;
diff --git a/drivers/usb/wusbcore/wusbhc.c b/drivers/usb/wusbcore/wusbhc.c
index 0faca16..bb5e649 100644
--- a/drivers/usb/wusbcore/wusbhc.c
+++ b/drivers/usb/wusbcore/wusbhc.c
@@ -435,9 +435,8 @@ static void __exit wusbcore_exit(void)
char buf[256];
bitmap_scnprintf(buf, sizeof(buf), wusb_cluster_id_table,
 CLUSTER_IDS);
-   printk(KERN_ERR "BUG: WUSB Cluster IDs not released "
+   WARN(1, KERN_ERR "BUG: WUSB Cluster IDs not released "
   "on exit: %s\n", buf);
-   WARN_ON(1);
}
usb_unregister_notify(&wusb_usb_notifier);
destroy_workqueue(wusbd);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 14/16] drivers/ssb/main.c: use WARN

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// 

Signed-off-by: Julia Lawall 

---
 drivers/ssb/main.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/ssb/main.c b/drivers/ssb/main.c
index df0f145..519688d 100644
--- a/drivers/ssb/main.c
+++ b/drivers/ssb/main.c
@@ -1118,8 +1118,7 @@ static u32 ssb_tmslow_reject_bitmask(struct ssb_device 
*dev)
case SSB_IDLOW_SSBREV_27: /* same here */
return SSB_TMSLOW_REJECT;   /* this is a guess */
default:
-   printk(KERN_INFO "ssb: Backplane Revision 0x%.8X\n", rev);
-   WARN_ON(1);
+   WARN(1, KERN_INFO "ssb: Backplane Revision 0x%.8X\n", rev);
}
return (SSB_TMSLOW_REJECT | SSB_TMSLOW_REJECT_23);
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 13/16] fs/btrfs: use WARN

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// 

Signed-off-by: Julia Lawall 

---
Many of these end up with the form
if (somewhat_complex_condition) WARN(1, ...)
They could also be converted to WARN(somewhat_complex_condition, ...)
if that would be preferred.

 fs/btrfs/ctree.c   |   19 +++
 fs/btrfs/disk-io.c |6 ++
 fs/btrfs/extent-tree.c |7 +++
 fs/btrfs/extent_io.c   |9 +++--
 fs/btrfs/inode.c   |3 +--
 fs/btrfs/transaction.c |   12 
 fs/btrfs/volumes.c |3 +--
 7 files changed, 21 insertions(+), 38 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index cdfb4c4..adfa929 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -1361,19 +1361,16 @@ noinline int btrfs_cow_block(struct btrfs_trans_handle 
*trans,
u64 search_start;
int ret;
 
-   if (trans->transaction != root->fs_info->running_transaction) {
-   printk(KERN_CRIT "trans %llu running %llu\n",
+   if (trans->transaction != root->fs_info->running_transaction)
+   WARN(1, KERN_CRIT "trans %llu running %llu\n",
   (unsigned long long)trans->transid,
   (unsigned long long)
   root->fs_info->running_transaction->transid);
-   WARN_ON(1);
-   }
-   if (trans->transid != root->fs_info->generation) {
-   printk(KERN_CRIT "trans %llu running %llu\n",
+
+   if (trans->transid != root->fs_info->generation)
+   WARN(1, KERN_CRIT "trans %llu running %llu\n",
   (unsigned long long)trans->transid,
   (unsigned long long)root->fs_info->generation);
-   WARN_ON(1);
-   }
 
if (!should_cow_block(trans, root, buf)) {
*cow_ret = buf;
@@ -3642,11 +3639,9 @@ static noinline int __push_leaf_left(struct 
btrfs_trans_handle *trans,
btrfs_set_header_nritems(left, old_left_nritems + push_items);
 
/* fixup right node */
-   if (push_items > right_nritems) {
-   printk(KERN_CRIT "push items %d nr %u\n", push_items,
+   if (push_items > right_nritems)
+   WARN(1, KERN_CRIT "push items %d nr %u\n", push_items,
   right_nritems);
-   WARN_ON(1);
-   }
 
if (push_items < right_nritems) {
push_space = btrfs_item_offset_nr(right, push_items - 1) -
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 22a0439..1769e7d 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3383,14 +3383,12 @@ void btrfs_mark_buffer_dirty(struct extent_buffer *buf)
int was_dirty;
 
btrfs_assert_tree_locked(buf);
-   if (transid != root->fs_info->generation) {
-   printk(KERN_CRIT "btrfs transid mismatch buffer %llu, "
+   if (transid != root->fs_info->generation)
+   WARN(1, KERN_CRIT "btrfs transid mismatch buffer %llu, "
   "found %llu running %llu\n",
(unsigned long long)buf->start,
(unsigned long long)transid,
(unsigned long long)root->fs_info->generation);
-   WARN_ON(1);
-   }
was_dirty = set_extent_buffer_dirty(buf);
if (!was_dirty) {
spin_lock(&root->fs_info->delalloc_lock);
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 3d3e2c1..37353eb 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -6292,10 +6292,9 @@ use_block_rsv(struct btrfs_trans_handle *trans,
static DEFINE_RATELIMIT_STATE(_rs,
DEFAULT_RATELIMIT_INTERVAL,
/*DEFAULT_RATELIMIT_BURST*/ 2);
-   if (__ratelimit(&_rs)) {
-   printk(KERN_DEBUG "btrfs: block rsv returned %d\n", 
ret);
-   WARN_ON(1);
-   }
+   if (__ratelimit(&_rs))
+   WARN(1, KERN_DEBUG "btrfs: block rsv returned %d\n",
+ret);
ret = reserve_metadata_bytes(root, block_rsv, blocksize, 0);
if (!ret) {
return block_rsv;
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 472873a..3c062c8 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -341,12 +341,10 @@ static int insert_state(struct extent_io_tree *tree,
 {
struct rb_node *node;
 
-   if (end < start) {
-   pr

[PATCH 9/16] fs/ext4/indirect.c: use WARN

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// 

Signed-off-by: Julia Lawall 

---
 fs/ext4/indirect.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index 792e388..0cdd20f 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -354,9 +354,8 @@ static int ext4_alloc_blocks(handle_t *handle, struct inode 
*inode,
 * for the first direct block
 */
new_blocks[index] = current_block;
-   printk(KERN_INFO "%s returned more blocks than "
+   WARN(1, KERN_INFO "%s returned more blocks than "
"requested\n", __func__);
-   WARN_ON(1);
break;
}
}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 8/16] drivers/infiniband/hw/cxgb3/iwch_cm.c: use WARN

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// 

Signed-off-by: Julia Lawall 

---
 drivers/infiniband/hw/cxgb3/iwch_cm.c |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb3/iwch_cm.c 
b/drivers/infiniband/hw/cxgb3/iwch_cm.c
index aaf88ef..8baaf0d 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_cm.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_cm.c
@@ -128,9 +128,8 @@ static void stop_ep_timer(struct iwch_ep *ep)
 {
PDBG("%s ep %p\n", __func__, ep);
if (!timer_pending(&ep->timer)) {
-   printk(KERN_ERR "%s timer stopped when its not running!  ep %p 
state %u\n",
+   WARN(1, KERN_ERR "%s timer stopped when its not running!  ep %p 
state %u\n",
__func__, ep, ep->com.state);
-   WARN_ON(1);
return;
}
del_timer_sync(&ep->timer);
@@ -1756,9 +1755,8 @@ static void ep_timeout(unsigned long arg)
__state_set(&ep->com, ABORTING);
break;
default:
-   printk(KERN_ERR "%s unexpected state ep %p state %u\n",
+   WARN(1, KERN_ERR "%s unexpected state ep %p state %u\n",
__func__, ep, ep->com.state);
-   WARN_ON(1);
abort = 0;
}
spin_unlock_irqrestore(&ep->com.lock, flags);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 7/16] drivers/scsi/gdth.c: use WARN

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

If (count) is also merged into WARN, for further conciseness.

A simplified version of the semantic patch that makes part of this
transformation is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// 

Signed-off-by: Julia Lawall 

---
 drivers/scsi/gdth.c |7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index 5d72274..0dbcb27 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -2318,11 +2318,10 @@ static void gdth_copy_internal_data(gdth_ha_str *ha, 
Scsi_Cmnd *scp,
 break;
 buffer += cpnow;
 }
-} else if (count) {
-printk("GDT-HA %d: SCSI command with no buffers but data transfer 
expected!\n",
-   ha->hanum);
-WARN_ON(1);
 }
+   else
+   WARN(count, "GDT-HA %d: SCSI command with no buffers but data 
transfer expected!\n",
+ha->hanum);
 }
 
 static int gdth_internal_cache_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 6/16] drivers/infiniband/hw/cxgb4/cm.c: use WARN

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// 

Signed-off-by: Julia Lawall 

---
 drivers/infiniband/hw/cxgb4/cm.c |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 6cfd4d8..ed048b9 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -151,9 +151,8 @@ static void stop_ep_timer(struct c4iw_ep *ep)
 {
PDBG("%s ep %p\n", __func__, ep);
if (!timer_pending(&ep->timer)) {
-   printk(KERN_ERR "%s timer stopped when its not running! "
+   WARN(1, KERN_ERR "%s timer stopped when its not running! "
   "ep %p state %u\n", __func__, ep, ep->com.state);
-   WARN_ON(1);
return;
}
del_timer_sync(&ep->timer);
@@ -2551,9 +2550,8 @@ static void process_timeout(struct c4iw_ep *ep)
__state_set(&ep->com, ABORTING);
break;
default:
-   printk(KERN_ERR "%s unexpected state ep %p tid %u state %u\n",
+   WARN(1, KERN_ERR "%s unexpected state ep %p tid %u state %u\n",
__func__, ep, ep->hwtid, ep->com.state);
-   WARN_ON(1);
abort = 0;
}
mutex_unlock(&ep->com.mutex);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/16] fs/hfsplus/bnode.c: use WARN

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// 

Signed-off-by: Julia Lawall 

---
 fs/hfsplus/bnode.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
index 5c125ce..7a92c2c 100644
--- a/fs/hfsplus/bnode.c
+++ b/fs/hfsplus/bnode.c
@@ -588,8 +588,7 @@ struct hfs_bnode *hfs_bnode_create(struct hfs_btree *tree, 
u32 num)
node = hfs_bnode_findhash(tree, num);
spin_unlock(&tree->hash_lock);
if (node) {
-   printk(KERN_CRIT "new node %u already hashed?\n", num);
-   WARN_ON(1);
+   WARN(1, KERN_CRIT "new node %u already hashed?\n", num);
return node;
}
node = __hfs_bnode_create(tree, num);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/16] drivers/gpu/drm/drm_cache.c: use WARN_ONCE

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Use WARN_ONCE rather than printk followed by WARN_ON_ONCE(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression list es;
@@

-printk(
+WARN_ONCE(1,
  es);
-WARN_ON_ONCE(1);
// 

Signed-off-by: Julia Lawall 

---
 drivers/gpu/drm/drm_cache.c |9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index a575cb2..8df9a7b 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -94,8 +94,7 @@ drm_clflush_pages(struct page *pages[], unsigned long 
num_pages)
kunmap_atomic(page_virtual);
}
 #else
-   printk(KERN_ERR "Architecture has no drm_cache.c support\n");
-   WARN_ON_ONCE(1);
+   WARN_ONCE(1, KERN_ERR "Architecture has no drm_cache.c support\n");
 #endif
 }
 EXPORT_SYMBOL(drm_clflush_pages);
@@ -119,8 +118,7 @@ drm_clflush_sg(struct sg_table *st)
if (on_each_cpu(drm_clflush_ipi_handler, NULL, 1) != 0)
printk(KERN_ERR "Timed out waiting for cache flush.\n");
 #else
-   printk(KERN_ERR "Architecture has no drm_cache.c support\n");
-   WARN_ON_ONCE(1);
+   WARN_ONCE(1, KERN_ERR "Architecture has no drm_cache.c support\n");
 #endif
 }
 EXPORT_SYMBOL(drm_clflush_sg);
@@ -142,8 +140,7 @@ drm_clflush_virt_range(char *addr, unsigned long length)
if (on_each_cpu(drm_clflush_ipi_handler, NULL, 1) != 0)
printk(KERN_ERR "Timed out waiting for cache flush.\n");
 #else
-   printk(KERN_ERR "Architecture has no drm_cache.c support\n");
-   WARN_ON_ONCE(1);
+   WARN_ONCE(1, KERN_ERR "Architecture has no drm_cache.c support\n");
 #endif
 }
 EXPORT_SYMBOL(drm_clflush_virt_range);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 10/16] drivers/net/ethernet/ibm/emac/mal.c: use WARN

2012-11-03 Thread Julia Lawall


On Sat, 3 Nov 2012, walter harms wrote:




Am 03.11.2012 11:58, schrieb Julia Lawall:

From: Julia Lawall 

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// 

Signed-off-by: Julia Lawall 

---
 drivers/net/ethernet/ibm/emac/mal.c |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/mal.c 
b/drivers/net/ethernet/ibm/emac/mal.c
index 479e43e..84c6b6c 100644
--- a/drivers/net/ethernet/ibm/emac/mal.c
+++ b/drivers/net/ethernet/ibm/emac/mal.c
@@ -738,13 +738,11 @@ static int __devexit mal_remove(struct platform_device 
*ofdev)
/* Synchronize with scheduled polling */
napi_disable(&mal->napi);

-   if (!list_empty(&mal->list)) {
+   if (!list_empty(&mal->list))
/* This is *very* bad */
-   printk(KERN_EMERG
+   WARN(1, KERN_EMERG
   "mal%d: commac list is not empty on remove!\n",
   mal->index);
-   WARN_ON(1);
-   }

dev_set_drvdata(&ofdev->dev, NULL);




Hi Julia,
you are removing the {} behin the if. I prefer to be a bit conservative
about {}. There is suggest to keep them because WARN may be expanded in
future (with a second line) and that will cause subtle changes that do
no break the code. (Yes i know it is possible to write macros that
contain savely more than one line.)


WARN is already multi-line, surrounded by ({ }).  It seems to be set up to 
be used as an expression.  Is it necessary to assume that it might someday 
be changed from safe to unsafe?


julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/misc/kgdbts.c: remove eprintk

2012-11-03 Thread Julia Lawall


From: Julia Lawall 

eprintk is really just WARN(1, KERN_ERR ...).  Use WARN to be more
consistent with the rest of the code.

Signed-off-by: Julia Lawall 

---
 drivers/misc/kgdbts.c |   38 +-
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c
index 3aa9a96..433993b 100644
--- a/drivers/misc/kgdbts.c
+++ b/drivers/misc/kgdbts.c
@@ -113,10 +113,6 @@
printk(KERN_INFO a); \
touch_nmi_watchdog();   \
} while (0)
-#define eprintk(a...) do { \
-   printk(KERN_ERR a); \
-   WARN_ON(1); \
-   } while (0)
 #define MAX_CONFIG_LEN 40

 static struct kgdb_io kgdbts_io_ops;
@@ -323,7 +319,7 @@ static int check_and_rewind_pc(char *put_str, char *arg)
v2printk("Emul: rewind hit single step bp\n");
restart_from_top_after_write = 1;
} else if (strcmp(arg, "silent") && ip + offset != addr) {
-   eprintk("kgdbts: BP mismatch %lx expected %lx\n",
+   WARN(1, KERN_ERR "kgdbts: BP mismatch %lx expected %lx\n",
   ip + offset, addr);
return 1;
}
@@ -374,7 +370,7 @@ static int check_single_step(char *put_str, char *arg)
 continue_test:
matched_id = 0;
if (instruction_pointer(&kgdbts_regs) == addr) {
-   eprintk("kgdbts: SingleStep failed at %lx\n",
+   WARN(1, KERN_ERR "kgdbts: SingleStep failed at %lx\n",
   instruction_pointer(&kgdbts_regs));
return 1;
}
@@ -469,7 +465,7 @@ static void emul_sstep_get(char *arg)
break_helper("z0", NULL, sstep_addr);
break;
default:
-   eprintk("kgdbts: ERROR failed sstep get emulation\n");
+   WARN(1, KERN_ERR "kgdbts: ERROR failed sstep get emulation\n");
}
sstep_state++;
 }
@@ -496,13 +492,13 @@ static int emul_sstep_put(char *put_str, char *arg)
break;
case 2:
if (strncmp(put_str, "$OK", 3)) {
-   eprintk("kgdbts: failed sstep break set\n");
+   WARN(1, KERN_ERR "kgdbts: failed sstep break set\n");
return 1;
}
break;
case 3:
if (strncmp(put_str, "$T0", 3)) {
-   eprintk("kgdbts: failed continue sstep\n");
+   WARN(1, KERN_ERR "kgdbts: failed continue sstep\n");
return 1;
} else {
char *ptr = &put_str[11];
@@ -511,14 +507,14 @@ static int emul_sstep_put(char *put_str, char *arg)
break;
case 4:
if (strncmp(put_str, "$OK", 3)) {
-   eprintk("kgdbts: failed sstep break unset\n");
+   WARN(1, KERN_ERR "kgdbts: failed sstep break unset\n");
return 1;
}
/* Single step is complete so continue on! */
sstep_state = 0;
return 0;
default:
-   eprintk("kgdbts: ERROR failed sstep put emulation\n");
+   WARN(1, KERN_ERR "kgdbts: ERROR failed sstep put emulation\n");
}

/* Continue on the same test line until emulation is complete */
@@ -763,7 +759,7 @@ static int run_simple_test(int is_get_char, int chr)
}

if (get_buf[get_buf_cnt] == '\0') {
-   eprintk("kgdbts: ERROR GET: EOB on '%s' at %i\n",
+   WARN(1, KERN_ERR "kgdbts: ERROR GET: EOB on '%s' at 
%i\n",
   ts.name, ts.idx);
get_buf_cnt = 0;
fill_get_buf("D");
@@ -778,13 +774,13 @@ static int run_simple_test(int is_get_char, int chr)
 */
if (ts.tst[ts.idx].get[0] == '\0' && ts.tst[ts.idx].put[0] == '\0' &&
!ts.tst[ts.idx].get_handler) {
-   eprintk("kgdbts: ERROR: beyond end of test on"
+   WARN(1, KERN_ERR "kgdbts: ERROR: beyond end of test on"
   " '%s' line %i\n", ts.name, ts.idx);
return 0;
}

if (put_buf_cnt >= BUFMAX) {
-   eprintk("kgdbts: ERROR: put buffer overflow on"
+   WARN(1, KERN_ERR "kgdbts: ERROR: put buffer overflow on"
   " '%s' line %i\n", ts.name, ts.idx);
put_buf_cnt = 0;
r

Re: [PATCH 10/16] drivers/net/ethernet/ibm/emac/mal.c: use WARN

2012-11-03 Thread Julia Lawall


While looking i have noticed that a lot of drivers define there private 
"assert" macro.
It is very similar to warn.

(e.g.)
#define RTL819x_DEBUG
#ifdef RTL819x_DEBUG
#define assert(expr) \
   if (!(expr)) {  \
printk( "Assertion failed! %s,%s,%s,line=%d\n", \
   #expr,__FILE__,__FUNCTION__,__LINE__);  \
   }


WARN is more complicated.  At least with the right debugging options 
turned on, it dumps the stack, via warn_slowpath_common.


julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] scripts/coccinelle/misc/warn.cocci: use WARN

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Use WARN(1,...) rather than printk followed by WARN(1).

Signed-off-by: Julia Lawall 

---
 scripts/coccinelle/misc/warn.cocci |  109 +
 1 file changed, 109 insertions(+)

diff --git a/scripts/coccinelle/misc/warn.cocci 
b/scripts/coccinelle/misc/warn.cocci
new file mode 100644
index 000..fda8c35
--- /dev/null
+++ b/scripts/coccinelle/misc/warn.cocci
@@ -0,0 +1,109 @@
+/// Use WARN(1,...) rather than printk followed by WARN_ON(1)
+///
+// Confidence: High
+// Copyright: (C) 2012 Julia Lawall, INRIA/LIP6.  GPLv2.
+// Copyright: (C) 2012 Gilles Muller, INRIA/LiP6.  GPLv2.
+// URL: http://coccinelle.lip6.fr/
+// Comments:
+// Options: -no_includes -include_headers
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+@bad1@
+position p;
+@@
+
+printk(...);
+printk@p(...);
+WARN_ON(1);
+
+@r1 depends on context || report || org@
+position p != bad1.p;
+@@
+
+ printk@p(...);
+*WARN_ON(1);
+
+@script:python depends on org@
+p << r1.p;
+@@
+
+cocci.print_main("printk + WARN_ON can be just WARN",p)
+
+@script:python depends on report@
+p << r1.p;
+@@
+
+msg = "SUGGESTION: printk + WARN_ON can be just WARN"
+coccilib.report.print_report(p[0],msg)
+
+@ok1 depends on patch@
+expression list es;
+position p != bad1.p;
+@@
+
+-printk@p(
++WARN(1,
+  es);
+-WARN_ON(1);
+
+@depends on patch@
+expression list ok1.es;
+@@
+
+if (...)
+- {
+  WARN(1,es);
+- }
+
+// 
+
+@bad2@
+position p;
+@@
+
+printk(...);
+printk@p(...);
+WARN_ON_ONCE(1);
+
+@r2 depends on context || report || org@
+position p != bad1.p;
+@@
+
+ printk@p(...);
+*WARN_ON_ONCE(1);
+
+@script:python depends on org@
+p << r2.p;
+@@
+
+cocci.print_main("printk + WARN_ON_ONCE can be just WARN_ONCE",p)
+
+@script:python depends on report@
+p << r2.p;
+@@
+
+msg = "SUGGESTION: printk + WARN_ON_ONCE can be just WARN_ONCE"
+coccilib.report.print_report(p[0],msg)
+
+@ok2 depends on patch@
+expression list es;
+position p != bad2.p;
+@@
+
+-printk@p(
++WARN_ONCE(1,
+  es);
+-WARN_ON_ONCE(1);
+
+@depends on patch@
+expression list ok2.es;
+@@
+
+if (...)
+- {
+  WARN_ONCE(1,es);
+- }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 0/8] drop if around WARN_ON

2012-11-03 Thread Julia Lawall

These patches convert a conditional with a simple test expression and a
then branch that only calls WARN_ON(1) to just a call to WARN_ON, which
will test the condition.

// 
@@
expression e;
@@

(
if(<+...e(...)...+>) WARN_ON(1);
|
- if (e) WARN_ON(1);
+ WARN_ON(e);
)// 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/8] arch/x86/kernel/cpu/mtrr/cleanup.c: drop if around WARN_ON

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Just use WARN_ON rather than an if containing only WARN_ON(1).

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression e;
@@
- if (e) WARN_ON(1);
+ WARN_ON(e);
// 

Signed-off-by: Julia Lawall 

---
 arch/x86/kernel/cpu/mtrr/cleanup.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/mtrr/cleanup.c 
b/arch/x86/kernel/cpu/mtrr/cleanup.c
index 35ffda5..dd833bf 100644
--- a/arch/x86/kernel/cpu/mtrr/cleanup.c
+++ b/arch/x86/kernel/cpu/mtrr/cleanup.c
@@ -967,8 +967,7 @@ int __init mtrr_trim_uncached_memory(unsigned long end_pfn)
if (total_trim_size) {
pr_warning("WARNING: BIOS bug: CPU MTRRs don't cover all of 
memory, losing %lluMB of RAM.\n", total_trim_size >> 20);
 
-   if (!changed_by_mtrr_cleanup)
-   WARN_ON(1);
+   WARN_ON(!changed_by_mtrr_cleanup);
 
pr_info("update e820 for mtrr\n");
update_e820();

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/8] fs/btrfs: drop if around WARN_ON

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Just use WARN_ON rather than an if containing only WARN_ON(1).

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression e;
@@
- if (e) WARN_ON(1);
+ WARN_ON(e);
// 

Signed-off-by: Julia Lawall 

---
 fs/btrfs/backref.c |3 +--
 fs/btrfs/ctree.c   |9 +++--
 fs/btrfs/inode.c   |3 +--
 3 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index 208d8aa..a321952 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -890,8 +890,7 @@ again:
while (!list_empty(&prefs)) {
ref = list_first_entry(&prefs, struct __prelim_ref, list);
list_del(&ref->list);
-   if (ref->count < 0)
-   WARN_ON(1);
+   WARN_ON(ref->count < 0);
if (ref->count && ref->root_id && ref->parent == 0) {
/* no parent == root of tree */
ret = ulist_add(roots, ref->root_id, 0, GFP_NOFS);
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index cdfb4c4..8ecb995 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -1469,10 +1469,8 @@ int btrfs_realloc_node(struct btrfs_trans_handle *trans,
if (cache_only && parent_level != 1)
return 0;
 
-   if (trans->transaction != root->fs_info->running_transaction)
-   WARN_ON(1);
-   if (trans->transid != root->fs_info->generation)
-   WARN_ON(1);
+   WARN_ON(trans->transaction != root->fs_info->running_transaction);
+   WARN_ON(trans->transid != root->fs_info->generation);
 
parent_nritems = btrfs_header_nritems(parent);
blocksize = btrfs_level_size(root, parent_level - 1);
@@ -3403,8 +3401,7 @@ static noinline int __push_leaf_right(struct 
btrfs_trans_handle *trans,
if (push_items == 0)
goto out_unlock;
 
-   if (!empty && push_items == left_nritems)
-   WARN_ON(1);
+   WARN_ON(!empty && push_items == left_nritems);
 
/* push left to right */
right_nritems = btrfs_header_nritems(right);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 95542a1..0d169ab 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1657,8 +1657,7 @@ static noinline int add_pending_csums(struct 
btrfs_trans_handle *trans,
 int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end,
  struct extent_state **cached_state)
 {
-   if ((end & (PAGE_CACHE_SIZE - 1)) == 0)
-   WARN_ON(1);
+   WARN_ON((end & (PAGE_CACHE_SIZE - 1)) == 0);
return set_extent_delalloc(&BTRFS_I(inode)->io_tree, start, end,
   cached_state, GFP_NOFS);
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 5/8] drivers/scsi/qla2xxx/qla_nx.c: drop if around WARN_ON

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Just use WARN_ON rather than an if containing only WARN_ON(1).

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression e;
@@
- if (e) WARN_ON(1);
+ WARN_ON(e);
// 

Signed-off-by: Julia Lawall 

---
 drivers/scsi/qla2xxx/qla_nx.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_nx.c b/drivers/scsi/qla2xxx/qla_nx.c
index f5e297c..4c62a5d 100644
--- a/drivers/scsi/qla2xxx/qla_nx.c
+++ b/drivers/scsi/qla2xxx/qla_nx.c
@@ -388,8 +388,7 @@ qla82xx_pci_set_crbwindow(struct qla_hw_data *ha, u64 off)
 
if ((off >= QLA82XX_CRB_PCIX_HOST) && (off < QLA82XX_CRB_PCIX_HOST2)) {
/* We are in first CRB window */
-   if (ha->curr_window != 0)
-   WARN_ON(1);
+   WARN_ON(ha->curr_window != 0);
return off;
}
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 6/8] arch/arm/mach-omap2/dpll3xxx.c: drop if around WARN_ON

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Just use WARN_ON rather than an if containing only WARN_ON(1).

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression e;
@@
- if (e) WARN_ON(1);
+ WARN_ON(e);
// 

Signed-off-by: Julia Lawall 

---
 arch/arm/mach-omap2/dpll3xxx.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/dpll3xxx.c b/arch/arm/mach-omap2/dpll3xxx.c
index eacf51f..ed855b0 100644
--- a/arch/arm/mach-omap2/dpll3xxx.c
+++ b/arch/arm/mach-omap2/dpll3xxx.c
@@ -478,8 +478,7 @@ int omap3_noncore_dpll_set_rate(struct clk *clk, unsigned 
long rate)
if (!soc_is_am33xx() && !cpu_is_omap44xx() && 
!cpu_is_omap3630()) {
freqsel = _omap3_dpll_compute_freqsel(clk,
dd->last_rounded_n);
-   if (!freqsel)
-   WARN_ON(1);
+   WARN_ON(!freqsel);
}
 
pr_debug("clock: %s: set rate: locking rate to %lu.\n",

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 7/8] drivers/scsi/scsi_transport_fc.c: drop if around WARN_ON

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Just use WARN_ON rather than an if containing only WARN_ON(1).

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression e;
@@
- if (e) WARN_ON(1);
+ WARN_ON(e);
// 

Signed-off-by: Julia Lawall 

---
 drivers/scsi/scsi_transport_fc.c |5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index e894ca7..f54c945 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -1699,9 +1699,8 @@ fc_stat_show(const struct device *dev, char *buf, 
unsigned long offset)
struct fc_host_statistics *stats;
ssize_t ret = -ENOENT;
 
-   if (offset > sizeof(struct fc_host_statistics) ||
-   offset % sizeof(u64) != 0)
-   WARN_ON(1);
+   WARN_ON(offset > sizeof(struct fc_host_statistics) ||
+   offset % sizeof(u64) != 0);
 
if (i->f->get_fc_host_stats) {
stats = (i->f->get_fc_host_stats)(shost);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 8/8] drivers/net/wireless/ath/ath6kl/hif.c: drop if around WARN_ON

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Just use WARN_ON rather than an if containing only WARN_ON(1).

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression e;
@@
- if (e) WARN_ON(1);
+ WARN_ON(e);
// 

Signed-off-by: Julia Lawall 

---
 drivers/net/wireless/ath/ath6kl/hif.c |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/hif.c 
b/drivers/net/wireless/ath/ath6kl/hif.c
index 68ed6c2..9e47c4a 100644
--- a/drivers/net/wireless/ath/ath6kl/hif.c
+++ b/drivers/net/wireless/ath/ath6kl/hif.c
@@ -338,8 +338,7 @@ static int ath6kl_hif_proc_err_intr(struct ath6kl_device 
*dev)
status = hif_read_write_sync(dev->ar, ERROR_INT_STATUS_ADDRESS,
 reg_buf, 4, HIF_WR_SYNC_BYTE_FIX);
 
-   if (status)
-   WARN_ON(1);
+   WARN_ON(status);
 
return status;
 }
@@ -383,8 +382,7 @@ static int ath6kl_hif_proc_cpu_intr(struct ath6kl_device 
*dev)
status = hif_read_write_sync(dev->ar, CPU_INT_STATUS_ADDRESS,
 reg_buf, 4, HIF_WR_SYNC_BYTE_FIX);
 
-   if (status)
-   WARN_ON(1);
+   WARN_ON(status);
 
return status;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/8] drivers/scsi/bfa/bfa_svc.c: drop if around WARN_ON

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Just use WARN_ON rather than an if containing only WARN_ON(1).

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression e;
@@
- if (e) WARN_ON(1);
+ WARN_ON(e);
// 

Signed-off-by: Julia Lawall 

---
 drivers/scsi/bfa/bfa_svc.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_svc.c b/drivers/scsi/bfa/bfa_svc.c
index 299c1c8..765b8d0 100644
--- a/drivers/scsi/bfa/bfa_svc.c
+++ b/drivers/scsi/bfa/bfa_svc.c
@@ -626,8 +626,7 @@ bfa_fcxp_init_reqrsp(struct bfa_fcxp_s *fcxp,
/*
 * alloc required sgpgs
 */
-   if (n_sgles > BFI_SGE_INLINE)
-   WARN_ON(1);
+   WARN_ON(n_sgles > BFI_SGE_INLINE);
}
 
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/8] fs/ext3/inode.c: drop if around WARN_ON

2012-11-03 Thread Julia Lawall

From: Julia Lawall 

Just use WARN_ON rather than an if containing only WARN_ON(1).

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression e;
@@
- if (e) WARN_ON(1);
+ WARN_ON(e);
// 

Signed-off-by: Julia Lawall 

---
 fs/ext3/inode.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
index 7e87e37..b176d42 100644
--- a/fs/ext3/inode.c
+++ b/fs/ext3/inode.c
@@ -1071,8 +1071,7 @@ struct buffer_head *ext3_getblk(handle_t *handle, struct 
inode *inode,
 * mapped. 0 in case of a HOLE.
 */
if (err > 0) {
-   if (err > 1)
-   WARN_ON(1);
+   WARN_ON(err > 1);
err = 0;
}
*errp = err;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/8] drop if around WARN_ON

2012-11-04 Thread Julia Lawall


On Sun, 4 Nov 2012, Sasha Levin wrote:


Hi Julia,

On Sat, Nov 3, 2012 at 4:30 PM, Julia Lawall  wrote:

These patches convert a conditional with a simple test expression and a
then branch that only calls WARN_ON(1) to just a call to WARN_ON, which
will test the condition.

// 
@@
expression e;
@@

(
if(<+...e(...)...+>) WARN_ON(1);
|
- if (e) WARN_ON(1);
+ WARN_ON(e);
)// 


So this deals with WARN_ON(), are you considering doing the same for
the rest of it's friends?


I tried WARN_ON_ONCE, but the pattern never occurred.  Are there others 
that are worth trying?


thanks,
julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/8] drop if around WARN_ON

2012-11-04 Thread Julia Lawall


On Sun, 4 Nov 2012, Sasha Levin wrote:


On Sun, Nov 4, 2012 at 10:57 AM, Julia Lawall  wrote:

On Sun, 4 Nov 2012, Sasha Levin wrote:


Hi Julia,

On Sat, Nov 3, 2012 at 4:30 PM, Julia Lawall  wrote:


These patches convert a conditional with a simple test expression and a
then branch that only calls WARN_ON(1) to just a call to WARN_ON, which
will test the condition.

// 
@@
expression e;
@@

(
if(<+...e(...)...+>) WARN_ON(1);
|
- if (e) WARN_ON(1);
+ WARN_ON(e);
)// 



So this deals with WARN_ON(), are you considering doing the same for
the rest of it's friends?



I tried WARN_ON_ONCE, but the pattern never occurred.  Are there others that
are worth trying?


Definitely!

Here's the semantic patch I've got:

@@
expression e;
@@

(
- if (e) WARN_ON(1);
+ WARN_ON(e);
|
- if (e) WARN_ON_ONCE(1);
+ WARN_ON_ONCE(e);
|
- if (e) WARN_ON_SMP(1);
+ WARN_ON_SMP(e);
|
- if (e) BUG();
+ BUG_ON(e);
)

This gave me a really huge patch output.

I can send it out if you think the patch above looks good.


I didn't change any cases where the if test contains a function call.  The 
current definitions of WARN_ON seem to always evaluate the condition 
expression, but I was worried that that might not always be the case.  And 
calling a function (the ones I remember were some kinds of print 
functions) seems like something one might not want buried in the argument 
of a debugging macro.


WARN_ON_SMP is just WARN_ON if CONFIG_SMP is true, but it is just 0 
otherwise.  So in that case it seems important to check that one is not 
throwing away something important.


I remember working on the BUG_ON case several years ago, and other people 
worked on it too, but I guess some are still there...  The current 
definitions of BUG_ON seem to keep the condition, but there are quite a 
few specialized definitions, so someone at some point might make a version 
that does not have that property.


julia

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] drivers/misc/kgdbts.c: remove eprintk

2012-11-04 Thread Julia Lawall




On Sun, 4 Nov 2012, Arnd Bergmann wrote:


On Saturday 03 November 2012, Julia Lawall wrote:

@@ -113,10 +113,6 @@
printk(KERN_INFO a); \
touch_nmi_watchdog();   \
} while (0)
-#define eprintk(a...) do { \
-   printk(KERN_ERR a); \
-   WARN_ON(1); \
-   } while (0)
  #define MAX_CONFIG_LEN40

  static struct kgdb_io kgdbts_io_ops;
@@ -323,7 +319,7 @@ static int check_and_rewind_pc(char *put_str, char *arg)
v2printk("Emul: rewind hit single step bp\n");
restart_from_top_after_write = 1;
} else if (strcmp(arg, "silent") && ip + offset != addr) {
-   eprintk("kgdbts: BP mismatch %lx expected %lx\n",
+   WARN(1, KERN_ERR "kgdbts: BP mismatch %lx expected %lx\n",
   ip + offset, addr);
return 1;
}


Hmm, I did not think that WARN() took a KERN_ERR argument, which should
really be implied here. Looking at the code, it really seems to be required
at the moment, but only 5 out of 117 callers use it this way.

Any idea what is going on here?


I'm not sure to understand the 5 and 117.  Using grep, I get 30 with 
KERN_ERR, 61 with some KERN thing, and 1207 without KERN.  If things are 
set up such that warn_slowpath_fmt is called, then that function adds
KERN_WARNING.  There is an alternate definition of __WARN_printf that just 
does a printk.


So if eprintk wants KERN_ERR, then it seems that rewriting it with WARN is 
not a good idea.  I will check whether this problems arises with the other 
printks and WARNs that I suggested to merge.


thanks,
julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

merging printk and WARN

2012-11-04 Thread Julia Lawall

It looks like these patches were not a good idea, because in each case the 
printk provides an error level, and WARN then provides another one.


Sorry for the noise.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] drivers/misc/kgdbts.c: remove eprintk

2012-11-04 Thread Julia Lawall


On Sun, 4 Nov 2012, Arnd Bergmann wrote:


On Sunday 04 November 2012, Julia Lawall wrote:


Hmm, I did not think that WARN() took a KERN_ERR argument, which should
really be implied here. Looking at the code, it really seems to be required
at the moment, but only 5 out of 117 callers use it this way.

Any idea what is going on here?


I'm not sure to understand the 5 and 117.  Using grep, I get 30 with
KERN_ERR, 61 with some KERN thing, and 1207 without KERN.


Right, I was using 'grep -w', which misses a lot of the instances, although
I see still much fewer in the last category.


If things are
set up such that warn_slowpath_fmt is called, then that function adds
KERN_WARNING.  There is an alternate definition of __WARN_printf that just
does a printk.


I don't see yet where that KERN_WARNING gets added. Looking at
warn_slowpath_common, there are two or three lines that get printed at
KERN_WARNING level, followed by the format that got passed into WARN(),
which may or may not include a printk level, but I don't see one getting
added.


OK, I agree.  There are lots of KERN_WARNINGs, but not on the string that 
was passed in.  Still, maybe it is not so good to pass a KERN_XXX for some 
other XXX to WARN.


julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] drivers/misc/kgdbts.c: remove eprintk

2012-11-05 Thread Julia Lawall

On Mon, 5 Nov 2012, Arnd Bergmann wrote:

> On Sunday 04 November 2012, Julia Lawall wrote:
> > >
> > > I don't see yet where that KERN_WARNING gets added. Looking at
> > > warn_slowpath_common, there are two or three lines that get printed at
> > > KERN_WARNING level, followed by the format that got passed into WARN(),
> > > which may or may not include a printk level, but I don't see one getting
> > > added.
> >
> > OK, I agree.  There are lots of KERN_WARNINGs, but not on the string that
> > was passed in.  Still, maybe it is not so good to pass a KERN_XXX for some
> > other XXX to WARN.
>
> Given that most users don't pass anything here, and that those that do pass
> something are somewhat inconsistent, could we make the messages always get
> printed at KERN_WARNING level from WARN(), and kill off the instances
> that already pass a level?

OK, I could try proposing that, and if someone doesn't think it is the
right thing to do, they can ignore the patch.

Concretely, KERN_WARNING should be added in the printk called from WARN,
and all KERN information should be removed from the calls?

thanks,
julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/9] drivers/usb/host/ehci-atmel.c: use devm_ functions

2012-07-29 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall 

---
Not compiled.

 drivers/usb/host/ehci-atmel.c |   30 ++
 1 file changed, 6 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c
index a47e2cf..411bb74 100644
--- a/drivers/usb/host/ehci-atmel.c
+++ b/drivers/usb/host/ehci-atmel.c
@@ -150,31 +150,24 @@ static int __devinit ehci_atmel_drv_probe(struct 
platform_device *pdev)
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
 
-   if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len,
-   driver->description)) {
-   dev_dbg(&pdev->dev, "controller already in use\n");
-   retval = -EBUSY;
-   goto fail_request_resource;
-   }
-
-   hcd->regs = ioremap_nocache(hcd->rsrc_start, hcd->rsrc_len);
+   hcd->regs = devm_request_and_ioremap(&pdev->dev, res);
if (hcd->regs == NULL) {
dev_dbg(&pdev->dev, "error mapping memory\n");
retval = -EFAULT;
-   goto fail_ioremap;
+   goto fail_request_resource;
}
 
-   iclk = clk_get(&pdev->dev, "ehci_clk");
+   iclk = devm_clk_get(&pdev->dev, "ehci_clk");
if (IS_ERR(iclk)) {
dev_err(&pdev->dev, "Error getting interface clock\n");
retval = -ENOENT;
-   goto fail_get_iclk;
+   goto fail_request_resource;
}
-   fclk = clk_get(&pdev->dev, "uhpck");
+   fclk = devm_clk_get(&pdev->dev, "uhpck");
if (IS_ERR(fclk)) {
dev_err(&pdev->dev, "Error getting function clock\n");
retval = -ENOENT;
-   goto fail_get_fclk;
+   goto fail_request_resource;
}
 
atmel_start_ehci(pdev);
@@ -187,13 +180,6 @@ static int __devinit ehci_atmel_drv_probe(struct 
platform_device *pdev)
 
 fail_add_hcd:
atmel_stop_ehci(pdev);
-   clk_put(fclk);
-fail_get_fclk:
-   clk_put(iclk);
-fail_get_iclk:
-   iounmap(hcd->regs);
-fail_ioremap:
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 fail_request_resource:
usb_put_hcd(hcd);
 fail_create_hcd:
@@ -209,13 +195,9 @@ static int __devexit ehci_atmel_drv_remove(struct 
platform_device *pdev)
 
ehci_shutdown(hcd);
usb_remove_hcd(hcd);
-   iounmap(hcd->regs);
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
usb_put_hcd(hcd);
 
atmel_stop_ehci(pdev);
-   clk_put(fclk);
-   clk_put(iclk);
fclk = iclk = NULL;
 
return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] use devm_ functions

2012-07-29 Thread Julia Lawall

The semantic patch (http://coccinelle.lip6.fr/) used in generating this
patch is as follows.  Some manual cleanup may be required.  This improves
on the previous version in that more devm functions are treated.

virtual after_start
virtual returned
virtual returnedDup
virtual arg
virtual all_args
virtual get

// -
// find functions

@plat depends on !after_start@
identifier i,pfn,rfn;
position p;
@@

struct platform_driver i@p = {
  .probe = pfn,
  .remove = (<+...rfn...+>),
};

// -
// set up iteration

@initialize:ocaml@

type ret = UseReturned | UseReturned2 of string | UseReturnedDup
 | UseArg | UseAllArgs | UseGet

let add pfn rfn alloc free devm_alloc file rule =
   let it = new iteration() in
   it#set_files [file];
   it#add_virtual_rule After_start;
   (match rule with
  UseReturned -> it#add_virtual_rule Returned
| UseReturnedDup -> it#add_virtual_rule ReturnedDup
| UseReturned2(free) -> it#add_virtual_rule Returned;
  it#add_virtual_identifier Second_free free
| UseArg -> it#add_virtual_rule Arg
| UseAllArgs -> it#add_virtual_rule All_args
| UseGet -> it#add_virtual_rule Get);
   if not (pfn="") then it#add_virtual_identifier Pfn pfn;
   if not (rfn="") then it#add_virtual_identifier Rfn rfn;
   if not (alloc="") then it#add_virtual_identifier Alloc alloc;
   if not (free="") then it#add_virtual_identifier Free free;
   if not (devm_alloc="") then it#add_virtual_identifier Devm_alloc devm_alloc;
   it#register()

@script:ocaml@
pfn << plat.pfn;
rfn << plat.rfn;
p << plat.p;
@@

let file = (List.hd p).file in
add pfn rfn "kmalloc" "kfree" "devm_kzalloc" file UseReturned;
add pfn rfn "kzalloc" "kfree" "devm_kzalloc" file UseReturned;
add pfn rfn "ioremap" "iounmap" "devm_ioremap" file UseReturned;
add pfn rfn "ioremap_nocache" "iounmap" "devm_ioremap_nocache" file
   UseReturned;

add pfn rfn "clk_get" "clk_put" "devm_clk_get" file UseReturnedDup;
add pfn rfn "clk_get" "clk_put" "devm_clk_get_bad" file UseReturned;

add pfn rfn "usb_get_phy" "usb_put_phy" "devm_usb_get_phy" file UseReturned;

add pfn rfn "pinctrl_get" "pinctrl_put" "devm_pinctrl_get" file UseReturned;
add pfn rfn "pinctrl_get_select_default" "pinctrl_put"
"devm_pinctrl_get_select_default" file UseReturned;

add pfn rfn "regulator_get" "regulator_put" "devm_regulator_get"
file UseReturned;
add pfn rfn "regulator_bulk_get" "regulator_bulk_free"
"devm_regulator_bulk_get" file UseAllArgs;

add pfn rfn "gpio_request" "gpio_free" "devm_gpio_request" file UseArg;
add pfn rfn "gpio_request_one" "gpio_free" "devm_gpio_request_one" file UseArg;

add pfn rfn "request_irq" "free_irq" "devm_request_irq" file UseArg;
add pfn rfn "request_threaded_irq" "free_irq" "devm_request_threaded_irq" file
  UseArg;
add pfn rfn "dma_alloc_coherent" "dma_free_coherent" "dmam_alloc_coherent"
  file UseArg;
add pfn rfn "dma_alloc_noncoherent" "dma_free_noncoherent"
  "dmam_alloc_noncoherent" file UseArg;

(* several possibilities... *)
add pfn rfn "request_region" "release_region" "devm_request_region" file
  UseGet;
add pfn rfn "request_mem_region" "release_mem_region"
  "devm_request_mem_region" file UseGet;
add pfn rfn "request_region" "release_region" "devm_request_region" file
  UseArg;
add pfn rfn "request_mem_region" "release_mem_region"
  "devm_request_mem_region" file UseArg;
(* fix a bug at the same time *)
add pfn rfn "request_region" "release_resource" "devm_request_region" file
  (UseReturned2("kfree"));
add pfn rfn "request_mem_region" "release_resource"
  "devm_request_mem_region" file (UseReturned2("kfree"));
add pfn rfn "ioport_map" "ioport_unmap" "devm_ioport_map" file UseReturned

// -
// process the initial definition of the probe function

@preprobe@
identifier virtual.pfn;
position p;
@@

pfn@p(...) { ... }

@probe@
identifier pfn;
position preprobe.p;
@@

pfn@p(...) { ... }

@labelled_return@
identifier probe.pfn,l;
expression e;
@@

pfn(...) { <+... l: return e; ...+> }

// -
// transform functions where free uses the result

@prb depends on returned exists@
identifier probe.pfn,pdev,virtual.alloc,virtual.free,virtual.second_free;
expression x,y,e;
expression list args;
position p1,p2,p3;
type T1,T2,T3;
@@

pfn(struct platform_device *pdev) { ... when any
x = (T1)alloc@p1(args)
<... when strict
 when any
 when forall
(
y = x;
... when != y = e
when != &y
free@p2((T2)y,...);
... when != x
when != y
second_free@p3((T3)y,...);
|
y = x;
... when != y = e
when != &y
free@p2((T2)y,...);
|
free@p2((T2)x,...);
... when != x
second_free@p3((T3)x,...);
|
free@p2((T2)x,...);
)
...>
}

@reme exists@
identifier virtual.rfn,virtual.free;
expression prb.x,prb.y;
type T;
@@

rfn(...) { ... free((T)

[PATCH 4/9] drivers/usb/host/ehci-grlib.c: use devm_ functions

2012-07-29 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall 

---
Not compiled.

 drivers/usb/host/ehci-grlib.c |   18 +++---
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/host/ehci-grlib.c b/drivers/usb/host/ehci-grlib.c
index 22ca45c..3180cb3 100644
--- a/drivers/usb/host/ehci-grlib.c
+++ b/drivers/usb/host/ehci-grlib.c
@@ -127,12 +127,6 @@ static int __devinit ehci_hcd_grlib_probe(struct 
platform_device *op)
hcd->rsrc_start = res.start;
hcd->rsrc_len = resource_size(&res);
 
-   if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, hcd_name)) {
-   printk(KERN_ERR "%s: request_mem_region failed\n", __FILE__);
-   rv = -EBUSY;
-   goto err_rmr;
-   }
-
irq = irq_of_parse_and_map(dn, 0);
if (irq == NO_IRQ) {
printk(KERN_ERR "%s: irq_of_parse_and_map failed\n", __FILE__);
@@ -140,9 +134,9 @@ static int __devinit ehci_hcd_grlib_probe(struct 
platform_device *op)
goto err_irq;
}
 
-   hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
+   hcd->regs = devm_request_and_ioremap(&op->dev, &res);
if (!hcd->regs) {
-   printk(KERN_ERR "%s: ioremap failed\n", __FILE__);
+   pr_err("%s: devm_request_and_ioremap failed\n", __FILE__);
rv = -ENOMEM;
goto err_ioremap;
}
@@ -161,17 +155,13 @@ static int __devinit ehci_hcd_grlib_probe(struct 
platform_device *op)
 
rv = usb_add_hcd(hcd, irq, 0);
if (rv)
-   goto err_ehci;
+   goto err_ioremap;
 
return 0;
 
-err_ehci:
-   iounmap(hcd->regs);
 err_ioremap:
irq_dispose_mapping(irq);
 err_irq:
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
-err_rmr:
usb_put_hcd(hcd);
 
return rv;
@@ -188,9 +178,7 @@ static int ehci_hcd_grlib_remove(struct platform_device *op)
 
usb_remove_hcd(hcd);
 
-   iounmap(hcd->regs);
irq_dispose_mapping(hcd->irq);
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 
usb_put_hcd(hcd);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 5/9] drivers/usb/host/ehci-ixp4xx.c: use devm_ functions

2012-07-29 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall 

---
Not compiled.

 drivers/usb/host/ehci-ixp4xx.c |   19 +++
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/ehci-ixp4xx.c b/drivers/usb/host/ehci-ixp4xx.c
index 488d401..f224c0a 100644
--- a/drivers/usb/host/ehci-ixp4xx.c
+++ b/drivers/usb/host/ehci-ixp4xx.c
@@ -98,30 +98,19 @@ static int ixp4xx_ehci_probe(struct platform_device *pdev)
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
 
-   if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len,
-   driver->description)) {
-   dev_dbg(&pdev->dev, "controller already in use\n");
-   retval = -EBUSY;
-   goto fail_request_resource;
-   }
-
-   hcd->regs = ioremap_nocache(hcd->rsrc_start, hcd->rsrc_len);
+   hcd->regs = devm_request_and_ioremap(&pdev->dev, res);
if (hcd->regs == NULL) {
dev_dbg(&pdev->dev, "error mapping memory\n");
retval = -EFAULT;
-   goto fail_ioremap;
+   goto fail_request_resource;
}
 
retval = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (retval)
-   goto fail_add_hcd;
+   goto fail_request_resource;
 
return retval;
 
-fail_add_hcd:
-   iounmap(hcd->regs);
-fail_ioremap:
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 fail_request_resource:
usb_put_hcd(hcd);
 fail_create_hcd:
@@ -134,8 +123,6 @@ static int ixp4xx_ehci_remove(struct platform_device *pdev)
struct usb_hcd *hcd = platform_get_drvdata(pdev);
 
usb_remove_hcd(hcd);
-   iounmap(hcd->regs);
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
usb_put_hcd(hcd);
 
return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/9] drivers/usb/host/ehci-au1xxx.c: use devm_ functions

2012-07-29 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall 

---
Not compiled.

 drivers/usb/host/ehci-au1xxx.c |   20 
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/ehci-au1xxx.c b/drivers/usb/host/ehci-au1xxx.c
index cba10d6..65c945e 100644
--- a/drivers/usb/host/ehci-au1xxx.c
+++ b/drivers/usb/host/ehci-au1xxx.c
@@ -98,23 +98,17 @@ static int ehci_hcd_au1xxx_drv_probe(struct platform_device 
*pdev)
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
 
-   if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, hcd_name)) {
-   pr_debug("request_mem_region failed");
-   ret = -EBUSY;
-   goto err1;
-   }
-
-   hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
+   hcd->regs = devm_request_and_ioremap(&pdev->dev, res);
if (!hcd->regs) {
-   pr_debug("ioremap failed");
+   pr_debug("devm_request_and_ioremap failed");
ret = -ENOMEM;
-   goto err2;
+   goto err1;
}
 
if (alchemy_usb_control(ALCHEMY_USB_EHCI0, 1)) {
printk(KERN_INFO "%s: controller init failed!\n", pdev->name);
ret = -ENODEV;
-   goto err3;
+   goto err1;
}
 
ret = usb_add_hcd(hcd, pdev->resource[1].start,
@@ -125,10 +119,6 @@ static int ehci_hcd_au1xxx_drv_probe(struct 
platform_device *pdev)
}
 
alchemy_usb_control(ALCHEMY_USB_EHCI0, 0);
-err3:
-   iounmap(hcd->regs);
-err2:
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 err1:
usb_put_hcd(hcd);
return ret;
@@ -140,8 +130,6 @@ static int ehci_hcd_au1xxx_drv_remove(struct 
platform_device *pdev)
 
usb_remove_hcd(hcd);
alchemy_usb_control(ALCHEMY_USB_EHCI0, 0);
-   iounmap(hcd->regs);
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
usb_put_hcd(hcd);
platform_set_drvdata(pdev, NULL);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 6/9] drivers/usb/host/ehci-ls1x.c: use devm_ functions

2012-07-29 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall 

---
Not compiled.

 drivers/usb/host/ehci-ls1x.c |   18 +++---
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/host/ehci-ls1x.c b/drivers/usb/host/ehci-ls1x.c
index a283e59..aa0f328 100644
--- a/drivers/usb/host/ehci-ls1x.c
+++ b/drivers/usb/host/ehci-ls1x.c
@@ -106,29 +106,19 @@ static int ehci_hcd_ls1x_probe(struct platform_device 
*pdev)
hcd->rsrc_start = res->start;
hcd->rsrc_len   = resource_size(res);
 
-   if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, hcd_name)) {
-   dev_dbg(&pdev->dev, "controller already in use\n");
-   ret = -EBUSY;
-   goto err_put_hcd;
-   }
-
-   hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
+   hcd->regs = devm_request_and_ioremap(&pdev->dev, res);
if (hcd->regs == NULL) {
dev_dbg(&pdev->dev, "error mapping memory\n");
ret = -EFAULT;
-   goto err_release_region;
+   goto err_put_hcd;
}
 
ret = usb_add_hcd(hcd, irq, IRQF_DISABLED | IRQF_SHARED);
if (ret)
-   goto err_iounmap;
+   goto err_put_hcd;
 
return ret;
 
-err_iounmap:
-   iounmap(hcd->regs);
-err_release_region:
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 err_put_hcd:
usb_put_hcd(hcd);
return ret;
@@ -139,8 +129,6 @@ static int ehci_hcd_ls1x_remove(struct platform_device 
*pdev)
struct usb_hcd *hcd = platform_get_drvdata(pdev);
 
usb_remove_hcd(hcd);
-   iounmap(hcd->regs);
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
usb_put_hcd(hcd);
 
return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 7/9] drivers/usb/host/ehci-msm.c: use devm_ functions

2012-07-29 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

The original code was also missing a call to iounmap(hcd->regs); in the
remove function, so this patch also implicitly fixes a bug.

Signed-off-by: Julia Lawall 

---
Not compiled.

 drivers/usb/host/ehci-msm.c |   13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c
index 17dd9e9..4af4dc5 100644
--- a/drivers/usb/host/ehci-msm.c
+++ b/drivers/usb/host/ehci-msm.c
@@ -133,7 +133,7 @@ static int ehci_msm_probe(struct platform_device *pdev)
 
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
-   hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
+   hcd->regs = devm_ioremap(&pdev->dev, hcd->rsrc_start, hcd->rsrc_len);
if (!hcd->regs) {
dev_err(&pdev->dev, "ioremap failed\n");
ret = -ENOMEM;
@@ -145,17 +145,17 @@ static int ehci_msm_probe(struct platform_device *pdev)
 * powering up VBUS, mapping of registers address space and power
 * management.
 */
-   phy = usb_get_phy(USB_PHY_TYPE_USB2);
+   phy = devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
if (IS_ERR_OR_NULL(phy)) {
dev_err(&pdev->dev, "unable to find transceiver\n");
ret = -ENODEV;
-   goto unmap;
+   goto put_hcd;
}
 
ret = otg_set_host(phy->otg, &hcd->self);
if (ret < 0) {
dev_err(&pdev->dev, "unable to register with transceiver\n");
-   goto put_transceiver;
+   goto put_hcd;
}
 
device_init_wakeup(&pdev->dev, 1);
@@ -168,10 +168,6 @@ static int ehci_msm_probe(struct platform_device *pdev)
 
return 0;
 
-put_transceiver:
-   usb_put_phy(phy);
-unmap:
-   iounmap(hcd->regs);
 put_hcd:
usb_put_hcd(hcd);
 
@@ -187,7 +183,6 @@ static int __devexit ehci_msm_remove(struct platform_device 
*pdev)
pm_runtime_set_suspended(&pdev->dev);
 
otg_set_host(phy->otg, NULL);
-   usb_put_phy(phy);
 
usb_put_hcd(hcd);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 8/9] drivers/usb/host/ehci-mv.c: use devm_ functions

2012-07-29 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

A new label name is created in one case to better reflect the contents of
the error-handling code.

Signed-off-by: Julia Lawall 

---
Not compiled.

 drivers/usb/host/ehci-mv.c |   51 ++---
 1 file changed, 16 insertions(+), 35 deletions(-)

diff --git a/drivers/usb/host/ehci-mv.c b/drivers/usb/host/ehci-mv.c
index f6df1cc..f7bfc0b 100644
--- a/drivers/usb/host/ehci-mv.c
+++ b/drivers/usb/host/ehci-mv.c
@@ -161,7 +161,7 @@ static int mv_ehci_probe(struct platform_device *pdev)
return -ENOMEM;
 
size = sizeof(*ehci_mv) + sizeof(struct clk *) * pdata->clknum;
-   ehci_mv = kzalloc(size, GFP_KERNEL);
+   ehci_mv = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
if (ehci_mv == NULL) {
dev_err(&pdev->dev, "cannot allocate ehci_hcd_mv\n");
retval = -ENOMEM;
@@ -175,12 +175,12 @@ static int mv_ehci_probe(struct platform_device *pdev)
ehci_mv->clknum = pdata->clknum;
for (clk_i = 0; clk_i < ehci_mv->clknum; clk_i++) {
ehci_mv->clk[clk_i] =
-   clk_get(&pdev->dev, pdata->clkname[clk_i]);
+   devm_clk_get(&pdev->dev, pdata->clkname[clk_i]);
if (IS_ERR(ehci_mv->clk[clk_i])) {
dev_err(&pdev->dev, "error get clck \"%s\"\n",
pdata->clkname[clk_i]);
retval = PTR_ERR(ehci_mv->clk[clk_i]);
-   goto err_put_clk;
+   goto err_clear_drvdata;
}
}
 
@@ -188,34 +188,36 @@ static int mv_ehci_probe(struct platform_device *pdev)
if (r == NULL) {
dev_err(&pdev->dev, "no phy I/O memory resource defined\n");
retval = -ENODEV;
-   goto err_put_clk;
+   goto err_clear_drvdata;
}
 
-   ehci_mv->phy_regs = ioremap(r->start, resource_size(r));
+   ehci_mv->phy_regs = devm_ioremap(&pdev->dev, r->start,
+resource_size(r));
if (ehci_mv->phy_regs == 0) {
dev_err(&pdev->dev, "failed to map phy I/O memory\n");
retval = -EFAULT;
-   goto err_put_clk;
+   goto err_clear_drvdata;
}
 
r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "capregs");
if (!r) {
dev_err(&pdev->dev, "no I/O memory resource defined\n");
retval = -ENODEV;
-   goto err_iounmap_phyreg;
+   goto err_clear_drvdata;
}
 
-   ehci_mv->cap_regs = ioremap(r->start, resource_size(r));
+   ehci_mv->cap_regs = devm_ioremap(&pdev->dev, r->start,
+resource_size(r));
if (ehci_mv->cap_regs == NULL) {
dev_err(&pdev->dev, "failed to map I/O memory\n");
retval = -EFAULT;
-   goto err_iounmap_phyreg;
+   goto err_clear_drvdata;
}
 
retval = mv_ehci_enable(ehci_mv);
if (retval) {
dev_err(&pdev->dev, "init phy error %d\n", retval);
-   goto err_iounmap_capreg;
+   goto err_clear_drvdata;
}
 
offset = readl(ehci_mv->cap_regs) & CAPLENGTH_MASK;
@@ -239,7 +241,7 @@ static int mv_ehci_probe(struct platform_device *pdev)
ehci_mv->mode = pdata->mode;
if (ehci_mv->mode == MV_USB_MODE_OTG) {
 #ifdef CONFIG_USB_OTG_UTILS
-   ehci_mv->otg = usb_get_phy(USB_PHY_TYPE_USB2);
+   ehci_mv->otg = devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
if (IS_ERR_OR_NULL(ehci_mv->otg)) {
dev_err(&pdev->dev,
"unable to find transceiver\n");
@@ -252,7 +254,7 @@ static int mv_ehci_probe(struct platform_device *pdev)
dev_err(&pdev->dev,
"unable to register with transceiver\n");
retval = -ENODEV;
-   goto err_put_transceiver;
+   goto err_disable_clk;
}
/* otg will enable clock before use as host */
mv_ehci_disable(ehci_mv);
@@ -286,22 +288,10 @@ static int mv_ehci_probe(struct platform_device *pdev)
 err_set_vbus:
if (pdata->set_vbus)
pdata->set_vbus(0);
-#ifdef CONFIG_USB_OTG_UTILS
-err_

[PATCH 9/9] drivers/usb/host/ehci-mxc.c: use devm_ functions

2012-07-29 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall 

---
Not compiled.

 drivers/usb/host/ehci-mxc.c |   45 +---
 1 file changed, 10 insertions(+), 35 deletions(-)

diff --git a/drivers/usb/host/ehci-mxc.c b/drivers/usb/host/ehci-mxc.c
index 3420137..959e1a4 100644
--- a/drivers/usb/host/ehci-mxc.c
+++ b/drivers/usb/host/ehci-mxc.c
@@ -121,7 +121,7 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev)
if (!hcd)
return -ENOMEM;
 
-   priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+   priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
if (!priv) {
ret = -ENOMEM;
goto err_alloc;
@@ -131,34 +131,28 @@ static int ehci_mxc_drv_probe(struct platform_device 
*pdev)
if (!res) {
dev_err(dev, "Found HC with no register addr. Check setup!\n");
ret = -ENODEV;
-   goto err_get_resource;
+   goto err_alloc;
}
 
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
 
-   if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, hcd_name)) {
-   dev_dbg(dev, "controller already in use\n");
-   ret = -EBUSY;
-   goto err_request_mem;
-   }
-
-   hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
+   hcd->regs = devm_request_and_ioremap(&pdev->dev, res);
if (!hcd->regs) {
dev_err(dev, "error mapping memory\n");
ret = -EFAULT;
-   goto err_ioremap;
+   goto err_alloc;
}
 
/* enable clocks */
-   priv->usbclk = clk_get(dev, "ipg");
+   priv->usbclk = devm_clk_get(&pdev->dev, "ipg");
if (IS_ERR(priv->usbclk)) {
ret = PTR_ERR(priv->usbclk);
-   goto err_clk;
+   goto err_alloc;
}
clk_prepare_enable(priv->usbclk);
 
-   priv->ahbclk = clk_get(dev, "ahb");
+   priv->ahbclk = devm_clk_get(&pdev->dev, "ahb");
if (IS_ERR(priv->ahbclk)) {
ret = PTR_ERR(priv->ahbclk);
goto err_clk_ahb;
@@ -166,7 +160,7 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev)
clk_prepare_enable(priv->ahbclk);
 
/* "dr" device has its own clock on i.MX51 */
-   priv->phyclk = clk_get(dev, "phy");
+   priv->phyclk = devm_clk_get(&pdev->dev, "phy");
if (IS_ERR(priv->phyclk))
priv->phyclk = NULL;
if (priv->phyclk)
@@ -245,23 +239,12 @@ err_add:
if (pdata && pdata->exit)
pdata->exit(pdev);
 err_init:
-   if (priv->phyclk) {
+   if (priv->phyclk)
clk_disable_unprepare(priv->phyclk);
-   clk_put(priv->phyclk);
-   }
 
clk_disable_unprepare(priv->ahbclk);
-   clk_put(priv->ahbclk);
 err_clk_ahb:
clk_disable_unprepare(priv->usbclk);
-   clk_put(priv->usbclk);
-err_clk:
-   iounmap(hcd->regs);
-err_ioremap:
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
-err_request_mem:
-err_get_resource:
-   kfree(priv);
 err_alloc:
usb_put_hcd(hcd);
return ret;
@@ -280,22 +263,14 @@ static int __exit ehci_mxc_drv_remove(struct 
platform_device *pdev)
usb_phy_shutdown(pdata->otg);
 
usb_remove_hcd(hcd);
-   iounmap(hcd->regs);
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
usb_put_hcd(hcd);
platform_set_drvdata(pdev, NULL);
 
clk_disable_unprepare(priv->usbclk);
-   clk_put(priv->usbclk);
clk_disable_unprepare(priv->ahbclk);
-   clk_put(priv->ahbclk);
 
-   if (priv->phyclk) {
+   if (priv->phyclk)
clk_disable_unprepare(priv->phyclk);
-   clk_put(priv->phyclk);
-   }
-
-   kfree(priv);
 
return 0;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/9] drivers/usb/host/ehci-cns3xxx.c: use devm_ functions

2012-07-29 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall 

---
Not compiled.

 drivers/usb/host/ehci-cns3xxx.c |   16 ++--
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/host/ehci-cns3xxx.c b/drivers/usb/host/ehci-cns3xxx.c
index caaa3e5..d91708d 100644
--- a/drivers/usb/host/ehci-cns3xxx.c
+++ b/drivers/usb/host/ehci-cns3xxx.c
@@ -105,27 +105,17 @@ static int cns3xxx_ehci_probe(struct platform_device 
*pdev)
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
 
-   if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len,
-   driver->description)) {
-   dev_dbg(dev, "controller already in use\n");
-   retval = -EBUSY;
-   goto err1;
-   }
-
-   hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
+   hcd->regs = devm_request_and_ioremap(&pdev->dev, res);
if (hcd->regs == NULL) {
dev_dbg(dev, "error mapping memory\n");
retval = -EFAULT;
-   goto err2;
+   goto err1;
}
 
retval = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (retval == 0)
return retval;
 
-   iounmap(hcd->regs);
-err2:
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 err1:
usb_put_hcd(hcd);
 
@@ -137,8 +127,6 @@ static int cns3xxx_ehci_remove(struct platform_device *pdev)
struct usb_hcd *hcd = platform_get_drvdata(pdev);
 
usb_remove_hcd(hcd);
-   iounmap(hcd->regs);
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 
/*
 * EHCI and OHCI share the same clock and power,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/7] drivers/usb/host/ehci-ppc-of.c: use devm_ functions

2012-07-30 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall 

---
Not compiled

 drivers/usb/host/ehci-ppc-of.c |   28 +++-
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/host/ehci-ppc-of.c b/drivers/usb/host/ehci-ppc-of.c
index bbbe89d..fa937d0 100644
--- a/drivers/usb/host/ehci-ppc-of.c
+++ b/drivers/usb/host/ehci-ppc-of.c
@@ -114,12 +114,6 @@ static int __devinit ehci_hcd_ppc_of_probe(struct 
platform_device *op)
hcd->rsrc_start = res.start;
hcd->rsrc_len = resource_size(&res);
 
-   if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, hcd_name)) {
-   printk(KERN_ERR "%s: request_mem_region failed\n", __FILE__);
-   rv = -EBUSY;
-   goto err_rmr;
-   }
-
irq = irq_of_parse_and_map(dn, 0);
if (irq == NO_IRQ) {
printk(KERN_ERR "%s: irq_of_parse_and_map failed\n", __FILE__);
@@ -127,9 +121,9 @@ static int __devinit ehci_hcd_ppc_of_probe(struct 
platform_device *op)
goto err_irq;
}
 
-   hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
+   hcd->regs = devm_request_and_ioremap(&op->dev, &res);
if (!hcd->regs) {
-   printk(KERN_ERR "%s: ioremap failed\n", __FILE__);
+   pr_err("%s: devm_request_and_ioremap failed\n", __FILE__);
rv = -ENOMEM;
goto err_ioremap;
}
@@ -139,8 +133,10 @@ static int __devinit ehci_hcd_ppc_of_probe(struct 
platform_device *op)
if (np != NULL) {
/* claim we really affected by usb23 erratum */
if (!of_address_to_resource(np, 0, &res))
-   ehci->ohci_hcctrl_reg = ioremap(res.start +
-   OHCI_HCCTRL_OFFSET, OHCI_HCCTRL_LEN);
+   ehci->ohci_hcctrl_reg =
+   devm_ioremap(&op->dev,
+res.start + OHCI_HCCTRL_OFFSET,
+OHCI_HCCTRL_LEN);
else
pr_debug("%s: no ohci offset in fdt\n", __FILE__);
if (!ehci->ohci_hcctrl_reg) {
@@ -169,19 +165,13 @@ static int __devinit ehci_hcd_ppc_of_probe(struct 
platform_device *op)
 
rv = usb_add_hcd(hcd, irq, 0);
if (rv)
-   goto err_ehci;
+   goto err_ioremap;
 
return 0;
 
-err_ehci:
-   if (ehci->has_amcc_usb23)
-   iounmap(ehci->ohci_hcctrl_reg);
-   iounmap(hcd->regs);
 err_ioremap:
irq_dispose_mapping(irq);
 err_irq:
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
-err_rmr:
usb_put_hcd(hcd);
 
return rv;
@@ -202,9 +192,7 @@ static int ehci_hcd_ppc_of_remove(struct platform_device 
*op)
 
usb_remove_hcd(hcd);
 
-   iounmap(hcd->regs);
irq_dispose_mapping(hcd->irq);
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 
/* use request_mem_region to test if the ohci driver is loaded.  if so
 * ensure the ohci core is operational.
@@ -222,8 +210,6 @@ static int ehci_hcd_ppc_of_remove(struct platform_device 
*op)
pr_debug("%s: no ohci offset in fdt\n", 
__FILE__);
of_node_put(np);
}
-
-   iounmap(ehci->ohci_hcctrl_reg);
}
usb_put_hcd(hcd);
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 7/7] drivers/usb/host/ehci-xilinx-of.c: use devm_ functions

2012-07-30 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall 

---
Not compiled

 drivers/usb/host/ehci-xilinx-of.c |   20 +++-
 1 file changed, 3 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/host/ehci-xilinx-of.c 
b/drivers/usb/host/ehci-xilinx-of.c
index 39f24fa..6a3f921 100644
--- a/drivers/usb/host/ehci-xilinx-of.c
+++ b/drivers/usb/host/ehci-xilinx-of.c
@@ -152,12 +152,6 @@ static int __devinit ehci_hcd_xilinx_of_probe(struct 
platform_device *op)
hcd->rsrc_start = res.start;
hcd->rsrc_len = resource_size(&res);
 
-   if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, hcd_name)) {
-   printk(KERN_ERR "%s: request_mem_region failed\n", __FILE__);
-   rv = -EBUSY;
-   goto err_rmr;
-   }
-
irq = irq_of_parse_and_map(dn, 0);
if (!irq) {
printk(KERN_ERR "%s: irq_of_parse_and_map failed\n", __FILE__);
@@ -165,11 +159,11 @@ static int __devinit ehci_hcd_xilinx_of_probe(struct 
platform_device *op)
goto err_irq;
}
 
-   hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
+   hcd->regs = devm_request_and_ioremap(&op->dev, &res);
if (!hcd->regs) {
-   printk(KERN_ERR "%s: ioremap failed\n", __FILE__);
+   pr_err("%s: devm_request_and_ioremap failed\n", __FILE__);
rv = -ENOMEM;
-   goto err_ioremap;
+   goto err_irq;
}
 
ehci = hcd_to_ehci(hcd);
@@ -200,12 +194,7 @@ static int __devinit ehci_hcd_xilinx_of_probe(struct 
platform_device *op)
if (rv == 0)
return 0;
 
-   iounmap(hcd->regs);
-
-err_ioremap:
 err_irq:
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
-err_rmr:
usb_put_hcd(hcd);
 
return rv;
@@ -227,9 +216,6 @@ static int ehci_hcd_xilinx_of_remove(struct platform_device 
*op)
 
usb_remove_hcd(hcd);
 
-   iounmap(hcd->regs);
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
-
usb_put_hcd(hcd);
 
return 0;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 5/7] drivers/usb/host/ehci-tegra.c: use devm_ functions

2012-07-30 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall 

---
Not compiled

 drivers/usb/host/ehci-tegra.c |   40 +---
 1 file changed, 13 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 950e95e..ba5039f 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -634,7 +634,8 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 
setup_vbus_gpio(pdev, pdata);
 
-   tegra = kzalloc(sizeof(struct tegra_ehci_hcd), GFP_KERNEL);
+   tegra = devm_kzalloc(&pdev->dev, sizeof(struct tegra_ehci_hcd),
+GFP_KERNEL);
if (!tegra)
return -ENOMEM;
 
@@ -642,13 +643,12 @@ static int tegra_ehci_probe(struct platform_device *pdev)
dev_name(&pdev->dev));
if (!hcd) {
dev_err(&pdev->dev, "Unable to create HCD\n");
-   err = -ENOMEM;
-   goto fail_hcd;
+   return -ENOMEM;
}
 
platform_set_drvdata(pdev, tegra);
 
-   tegra->clk = clk_get(&pdev->dev, NULL);
+   tegra->clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(tegra->clk)) {
dev_err(&pdev->dev, "Can't get ehci clock\n");
err = PTR_ERR(tegra->clk);
@@ -657,9 +657,9 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 
err = clk_prepare_enable(tegra->clk);
if (err)
-   goto fail_clken;
+   goto fail_clk;
 
-   tegra->emc_clk = clk_get(&pdev->dev, "emc");
+   tegra->emc_clk = devm_clk_get(&pdev->dev, "emc");
if (IS_ERR(tegra->emc_clk)) {
dev_err(&pdev->dev, "Can't get emc clock\n");
err = PTR_ERR(tegra->emc_clk);
@@ -677,7 +677,7 @@ static int tegra_ehci_probe(struct platform_device *pdev)
}
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
-   hcd->regs = ioremap(res->start, resource_size(res));
+   hcd->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
if (!hcd->regs) {
dev_err(&pdev->dev, "Failed to remap I/O memory\n");
err = -ENOMEM;
@@ -702,7 +702,7 @@ static int tegra_ehci_probe(struct platform_device *pdev)
default:
err = -ENODEV;
dev_err(&pdev->dev, "unknown usb instance\n");
-   goto fail_phy;
+   goto fail_io;
}
}
 
@@ -712,7 +712,7 @@ static int tegra_ehci_probe(struct platform_device *pdev)
if (IS_ERR(tegra->phy)) {
dev_err(&pdev->dev, "Failed to open USB phy\n");
err = -ENXIO;
-   goto fail_phy;
+   goto fail_io;
}
 
err = tegra_usb_phy_power_on(tegra->phy);
@@ -733,7 +733,8 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 
 #ifdef CONFIG_USB_OTG_UTILS
if (pdata->operating_mode == TEGRA_USB_OTG) {
-   tegra->transceiver = usb_get_phy(USB_PHY_TYPE_USB2);
+   tegra->transceiver =
+   devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
if (!IS_ERR_OR_NULL(tegra->transceiver))
otg_set_host(tegra->transceiver->otg, &hcd->self);
}
@@ -757,25 +758,16 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 
 fail:
 #ifdef CONFIG_USB_OTG_UTILS
-   if (!IS_ERR_OR_NULL(tegra->transceiver)) {
+   if (!IS_ERR_OR_NULL(tegra->transceiver))
otg_set_host(tegra->transceiver->otg, NULL);
-   usb_put_phy(tegra->transceiver);
-   }
 #endif
tegra_usb_phy_close(tegra->phy);
-fail_phy:
-   iounmap(hcd->regs);
 fail_io:
clk_disable_unprepare(tegra->emc_clk);
-   clk_put(tegra->emc_clk);
 fail_emc_clk:
clk_disable_unprepare(tegra->clk);
-fail_clken:
-   clk_put(tegra->clk);
 fail_clk:
usb_put_hcd(hcd);
-fail_hcd:
-   kfree(tegra);
return err;
 }
 
@@ -792,25 +784,19 @@ static int tegra_ehci_remove(struct platform_device *pdev)
pm_runtime_put_noidle(&pdev->dev);
 
 #ifdef CONFIG_USB_OTG_UTILS
-   if (!IS_ERR_OR_NULL(tegra->transceiver)) {
+   if (!IS_ERR_OR_NULL(tegra->transceiver))
otg_set_host(tegra->transceiver->otg, NULL);
-   usb_put_ph

[PATCH 2/7] drivers/usb/host/ehci-s5p.c: use devm_ functions

2012-07-30 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall 

---
Not compiled

 drivers/usb/host/ehci-s5p.c |7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
index 9d8f1dd..d055503 100644
--- a/drivers/usb/host/ehci-s5p.c
+++ b/drivers/usb/host/ehci-s5p.c
@@ -128,7 +128,7 @@ static int __devinit s5p_ehci_probe(struct platform_device 
*pdev)
}
 
s5p_ehci->hcd = hcd;
-   s5p_ehci->clk = clk_get(&pdev->dev, "usbhost");
+   s5p_ehci->clk = devm_clk_get(&pdev->dev, "usbhost");
 
if (IS_ERR(s5p_ehci->clk)) {
dev_err(&pdev->dev, "Failed to get usbhost clock\n");
@@ -138,7 +138,7 @@ static int __devinit s5p_ehci_probe(struct platform_device 
*pdev)
 
err = clk_enable(s5p_ehci->clk);
if (err)
-   goto fail_clken;
+   goto fail_clk;
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
@@ -184,8 +184,6 @@ static int __devinit s5p_ehci_probe(struct platform_device 
*pdev)
 
 fail_io:
clk_disable(s5p_ehci->clk);
-fail_clken:
-   clk_put(s5p_ehci->clk);
 fail_clk:
usb_put_hcd(hcd);
return err;
@@ -203,7 +201,6 @@ static int __devexit s5p_ehci_remove(struct platform_device 
*pdev)
pdata->phy_exit(pdev, S5P_USB_PHY_HOST);
 
clk_disable(s5p_ehci->clk);
-   clk_put(s5p_ehci->clk);
 
usb_put_hcd(hcd);
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/7] drivers/usb/host/ehci-sead3.c: use devm_ functions

2012-07-30 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall 

---
Not compiled

 drivers/usb/host/ehci-sead3.c |   15 ++-
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/host/ehci-sead3.c b/drivers/usb/host/ehci-sead3.c
index 58c96bd..802dad8 100644
--- a/drivers/usb/host/ehci-sead3.c
+++ b/drivers/usb/host/ehci-sead3.c
@@ -112,17 +112,11 @@ static int ehci_hcd_sead3_drv_probe(struct 
platform_device *pdev)
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
 
-   if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, hcd_name)) {
-   pr_debug("request_mem_region failed");
-   ret = -EBUSY;
-   goto err1;
-   }
-
-   hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
+   hcd->regs = devm_request_and_ioremap(&pdev->dev, res);
if (!hcd->regs) {
pr_debug("ioremap failed");
ret = -ENOMEM;
-   goto err2;
+   goto err1;
}
 
/* Root hub has integrated TT. */
@@ -135,9 +129,6 @@ static int ehci_hcd_sead3_drv_probe(struct platform_device 
*pdev)
return ret;
}
 
-   iounmap(hcd->regs);
-err2:
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 err1:
usb_put_hcd(hcd);
return ret;
@@ -148,8 +139,6 @@ static int ehci_hcd_sead3_drv_remove(struct platform_device 
*pdev)
struct usb_hcd *hcd = platform_get_drvdata(pdev);
 
usb_remove_hcd(hcd);
-   iounmap(hcd->regs);
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
usb_put_hcd(hcd);
platform_set_drvdata(pdev, NULL);
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/7] drivers/usb/host/ehci-sh.c: use devm_ functions

2012-07-30 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall 

---
Not compiled

 drivers/usb/host/ehci-sh.c |   35 +++
 1 file changed, 7 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/host/ehci-sh.c b/drivers/usb/host/ehci-sh.c
index b3f1e36..6081e1e 100644
--- a/drivers/usb/host/ehci-sh.c
+++ b/drivers/usb/host/ehci-sh.c
@@ -125,33 +125,27 @@ static int ehci_hcd_sh_probe(struct platform_device *pdev)
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
 
-   if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len,
-   driver->description)) {
-   dev_dbg(&pdev->dev, "controller already in use\n");
-   ret = -EBUSY;
-   goto fail_request_resource;
-   }
-
-   hcd->regs = ioremap_nocache(hcd->rsrc_start, hcd->rsrc_len);
+   hcd->regs = devm_request_and_ioremap(&pdev->dev, res);
if (hcd->regs == NULL) {
dev_dbg(&pdev->dev, "error mapping memory\n");
ret = -ENXIO;
-   goto fail_ioremap;
+   goto fail_request_resource;
}
 
-   priv = kmalloc(sizeof(struct ehci_sh_priv), GFP_KERNEL);
+   priv = devm_kzalloc(&pdev->dev, sizeof(struct ehci_sh_priv),
+   GFP_KERNEL);
if (!priv) {
dev_dbg(&pdev->dev, "error allocating priv data\n");
ret = -ENOMEM;
-   goto fail_alloc;
+   goto fail_request_resource;
}
 
/* These are optional, we don't care if they fail */
-   priv->fclk = clk_get(&pdev->dev, "usb_fck");
+   priv->fclk = devm_clk_get(&pdev->dev, "usb_fck");
if (IS_ERR(priv->fclk))
priv->fclk = NULL;
 
-   priv->iclk = clk_get(&pdev->dev, "usb_ick");
+   priv->iclk = devm_clk_get(&pdev->dev, "usb_ick");
if (IS_ERR(priv->iclk))
priv->iclk = NULL;
 
@@ -176,14 +170,6 @@ fail_add_hcd:
clk_disable(priv->iclk);
clk_disable(priv->fclk);
 
-   clk_put(priv->iclk);
-   clk_put(priv->fclk);
-
-   kfree(priv);
-fail_alloc:
-   iounmap(hcd->regs);
-fail_ioremap:
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 fail_request_resource:
usb_put_hcd(hcd);
 fail_create_hcd:
@@ -198,19 +184,12 @@ static int __exit ehci_hcd_sh_remove(struct 
platform_device *pdev)
struct usb_hcd *hcd = priv->hcd;
 
usb_remove_hcd(hcd);
-   iounmap(hcd->regs);
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
usb_put_hcd(hcd);
platform_set_drvdata(pdev, NULL);
 
clk_disable(priv->fclk);
clk_disable(priv->iclk);
 
-   clk_put(priv->fclk);
-   clk_put(priv->iclk);
-
-   kfree(priv);
-
return 0;
 }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 6/7] drivers/usb/host/ehci-vt8500.c: use devm_ functions

2012-07-30 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall 

---
Not compiled

 drivers/usb/host/ehci-vt8500.c |   15 ++-
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/host/ehci-vt8500.c b/drivers/usb/host/ehci-vt8500.c
index 4d147c4..a891617 100644
--- a/drivers/usb/host/ehci-vt8500.c
+++ b/drivers/usb/host/ehci-vt8500.c
@@ -106,17 +106,11 @@ static int vt8500_ehci_drv_probe(struct platform_device 
*pdev)
hcd->rsrc_start = res->start;
hcd->rsrc_len = resource_size(res);
 
-   if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len, hcd_name)) {
-   pr_debug("request_mem_region failed");
-   ret = -EBUSY;
-   goto err1;
-   }
-
-   hcd->regs = ioremap(hcd->rsrc_start, hcd->rsrc_len);
+   hcd->regs = devm_request_and_ioremap(&pdev->dev, res);
if (!hcd->regs) {
pr_debug("ioremap failed");
ret = -ENOMEM;
-   goto err2;
+   goto err1;
}
 
ehci = hcd_to_ehci(hcd);
@@ -129,9 +123,6 @@ static int vt8500_ehci_drv_probe(struct platform_device 
*pdev)
return ret;
}
 
-   iounmap(hcd->regs);
-err2:
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 err1:
usb_put_hcd(hcd);
return ret;
@@ -142,8 +133,6 @@ static int vt8500_ehci_drv_remove(struct platform_device 
*pdev)
struct usb_hcd *hcd = platform_get_drvdata(pdev);
 
usb_remove_hcd(hcd);
-   iounmap(hcd->regs);
-   release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
usb_put_hcd(hcd);
platform_set_drvdata(pdev, NULL);
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/3] drivers/media/radio/radio-timb.c: use devm_ functions

2012-07-31 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall 

---
 drivers/media/radio/radio-timb.c |8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/media/radio/radio-timb.c b/drivers/media/radio/radio-timb.c
index 7052adc..09fc560 100644
--- a/drivers/media/radio/radio-timb.c
+++ b/drivers/media/radio/radio-timb.c
@@ -157,7 +157,7 @@ static int __devinit timbradio_probe(struct platform_device 
*pdev)
goto err;
}
 
-   tr = kzalloc(sizeof(*tr), GFP_KERNEL);
+   tr = devm_kzalloc(&pdev->dev, sizeof(*tr), GFP_KERNEL);
if (!tr) {
err = -ENOMEM;
goto err;
@@ -177,7 +177,7 @@ static int __devinit timbradio_probe(struct platform_device 
*pdev)
strlcpy(tr->v4l2_dev.name, DRIVER_NAME, sizeof(tr->v4l2_dev.name));
err = v4l2_device_register(NULL, &tr->v4l2_dev);
if (err)
-   goto err_v4l2_dev;
+   goto err;
 
tr->video_dev.v4l2_dev = &tr->v4l2_dev;
 
@@ -195,8 +195,6 @@ static int __devinit timbradio_probe(struct platform_device 
*pdev)
 err_video_req:
video_device_release_empty(&tr->video_dev);
v4l2_device_unregister(&tr->v4l2_dev);
-err_v4l2_dev:
-   kfree(tr);
 err:
dev_err(&pdev->dev, "Failed to register: %d\n", err);
 
@@ -212,8 +210,6 @@ static int __devexit timbradio_remove(struct 
platform_device *pdev)
 
v4l2_device_unregister(&tr->v4l2_dev);
 
-   kfree(tr);
-
return 0;
 }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/3] drivers/media/radio/radio-wl1273.c: use devm_ functions

2012-07-31 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

In two cases, the original memory allocation function was kmalloc, which
has been changed to a zeroing allocation to benefit from the devm function.

Signed-off-by: Julia Lawall 

---
 drivers/media/radio/radio-wl1273.c |   23 +++
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/media/radio/radio-wl1273.c 
b/drivers/media/radio/radio-wl1273.c
index e8428f5..a22ad1c 100644
--- a/drivers/media/radio/radio-wl1273.c
+++ b/drivers/media/radio/radio-wl1273.c
@@ -1983,9 +1983,6 @@ static int wl1273_fm_radio_remove(struct platform_device 
*pdev)
v4l2_ctrl_handler_free(&radio->ctrl_handler);
video_unregister_device(&radio->videodev);
v4l2_device_unregister(&radio->v4l2dev);
-   kfree(radio->buffer);
-   kfree(radio->write_buf);
-   kfree(radio);
 
return 0;
 }
@@ -2005,7 +2002,7 @@ static int __devinit wl1273_fm_radio_probe(struct 
platform_device *pdev)
goto pdata_err;
}
 
-   radio = kzalloc(sizeof(*radio), GFP_KERNEL);
+   radio = devm_kzalloc(&pdev->dev, sizeof(*radio), GFP_KERNEL);
if (!radio) {
r = -ENOMEM;
goto pdata_err;
@@ -2013,11 +2010,11 @@ static int __devinit wl1273_fm_radio_probe(struct 
platform_device *pdev)
 
/* RDS buffer allocation */
radio->buf_size = rds_buf * RDS_BLOCK_SIZE;
-   radio->buffer = kmalloc(radio->buf_size, GFP_KERNEL);
+   radio->buffer = devm_kzalloc(&pdev->dev, radio->buf_size, GFP_KERNEL);
if (!radio->buffer) {
pr_err("Cannot allocate memory for RDS buffer.\n");
r = -ENOMEM;
-   goto err_kmalloc;
+   goto pdata_err;
}
 
radio->core = *core;
@@ -2043,7 +2040,7 @@ static int __devinit wl1273_fm_radio_probe(struct 
platform_device *pdev)
if (r) {
dev_err(radio->dev, WL1273_FM_DRIVER_NAME
": Cannot get platform data\n");
-   goto err_resources;
+   goto pdata_err;
}
 
dev_dbg(radio->dev, "irq: %d\n", radio->core->client->irq);
@@ -2061,13 +2058,13 @@ static int __devinit wl1273_fm_radio_probe(struct 
platform_device *pdev)
dev_err(radio->dev, WL1273_FM_DRIVER_NAME ": Core WL1273 IRQ"
" not configured");
r = -EINVAL;
-   goto err_resources;
+   goto pdata_err;
}
 
init_completion(&radio->busy);
init_waitqueue_head(&radio->read_queue);
 
-   radio->write_buf = kmalloc(256, GFP_KERNEL);
+   radio->write_buf = devm_kzalloc(&pdev->dev, 256, GFP_KERNEL);
if (!radio->write_buf) {
r = -ENOMEM;
goto write_buf_err;
@@ -2080,7 +2077,7 @@ static int __devinit wl1273_fm_radio_probe(struct 
platform_device *pdev)
r = v4l2_device_register(&pdev->dev, &radio->v4l2dev);
if (r) {
dev_err(&pdev->dev, "Cannot register v4l2_device.\n");
-   goto device_register_err;
+   goto write_buf_err;
}
 
/* V4L2 configuration */
@@ -2135,16 +2132,10 @@ static int __devinit wl1273_fm_radio_probe(struct 
platform_device *pdev)
 handler_init_err:
v4l2_ctrl_handler_free(&radio->ctrl_handler);
v4l2_device_unregister(&radio->v4l2dev);
-device_register_err:
-   kfree(radio->write_buf);
 write_buf_err:
free_irq(radio->core->client->irq, radio);
 err_request_irq:
radio->core->pdata->free_resources();
-err_resources:
-   kfree(radio->buffer);
-err_kmalloc:
-   kfree(radio);
 pdata_err:
return r;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/3] drivers/media/radio/radio-si4713.c: use devm_ functions

2012-07-31 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

Signed-off-by: Julia Lawall 

---
 drivers/media/radio/radio-si4713.c |7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/media/radio/radio-si4713.c 
b/drivers/media/radio/radio-si4713.c
index c54210c..5f366d1 100644
--- a/drivers/media/radio/radio-si4713.c
+++ b/drivers/media/radio/radio-si4713.c
@@ -268,7 +268,7 @@ static int radio_si4713_pdriver_probe(struct 
platform_device *pdev)
goto exit;
}
 
-   rsdev = kzalloc(sizeof *rsdev, GFP_KERNEL);
+   rsdev = devm_kzalloc(&pdev->dev, sizeof(*rsdev), GFP_KERNEL);
if (!rsdev) {
dev_err(&pdev->dev, "Failed to alloc video device.\n");
rval = -ENOMEM;
@@ -278,7 +278,7 @@ static int radio_si4713_pdriver_probe(struct 
platform_device *pdev)
rval = v4l2_device_register(&pdev->dev, &rsdev->v4l2_dev);
if (rval) {
dev_err(&pdev->dev, "Failed to register v4l2 device.\n");
-   goto free_rsdev;
+   goto exit;
}
 
adapter = i2c_get_adapter(pdata->i2c_bus);
@@ -322,8 +322,6 @@ put_adapter:
i2c_put_adapter(adapter);
 unregister_v4l2_dev:
v4l2_device_unregister(&rsdev->v4l2_dev);
-free_rsdev:
-   kfree(rsdev);
 exit:
return rval;
 }
@@ -342,7 +340,6 @@ static int __exit radio_si4713_pdriver_remove(struct 
platform_device *pdev)
video_unregister_device(rsdev->radio_dev);
i2c_put_adapter(client->adapter);
v4l2_device_unregister(&rsdev->v4l2_dev);
-   kfree(rsdev);
 
return 0;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] drivers/iio/adc/at91_adc.c: use devm_ functions

2012-07-31 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

The call to platform_get_resource(pdev, IORESOURCE_MEM, 0) is moved closer
to the new call to devm_request_and_ioremap where its result is first
used.  devm_request_and_ioremap takes case of the NULL test on the result
of platform_get_resource(pdev, IORESOURCE_MEM, 0), so that is dropped.

Signed-off-by: Julia Lawall 

---
Not compiled.

 drivers/iio/adc/at91_adc.c |   56 +++--
 1 file changed, 14 insertions(+), 42 deletions(-)

diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index f61780a..b0277bf 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -545,13 +545,6 @@ static int __devinit at91_adc_probe(struct platform_device 
*pdev)
goto error_free_device;
}
 
-   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!res) {
-   dev_err(&pdev->dev, "No resource defined\n");
-   ret = -ENXIO;
-   goto error_ret;
-   }
-
platform_set_drvdata(pdev, idev);
 
idev->dev.parent = &pdev->dev;
@@ -566,18 +559,13 @@ static int __devinit at91_adc_probe(struct 
platform_device *pdev)
goto error_free_device;
}
 
-   if (!request_mem_region(res->start, resource_size(res),
-   "AT91 adc registers")) {
-   dev_err(&pdev->dev, "Resources are unavailable.\n");
-   ret = -EBUSY;
-   goto error_free_device;
-   }
+   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
-   st->reg_base = ioremap(res->start, resource_size(res));
+   st->reg_base = devm_request_and_ioremap(&pdev->dev, res);
if (!st->reg_base) {
dev_err(&pdev->dev, "Failed to map registers.\n");
ret = -ENOMEM;
-   goto error_release_mem;
+   goto error_free_device;
}
 
/*
@@ -585,27 +573,27 @@ static int __devinit at91_adc_probe(struct 
platform_device *pdev)
 */
at91_adc_writel(st, AT91_ADC_CR, AT91_ADC_SWRST);
at91_adc_writel(st, AT91_ADC_IDR, 0x);
-   ret = request_irq(st->irq,
- at91_adc_eoc_trigger,
- 0,
- pdev->dev.driver->name,
- idev);
+   ret = devm_request_irq(&pdev->dev, st->irq,
+  at91_adc_eoc_trigger,
+  0,
+  pdev->dev.driver->name,
+  idev);
if (ret) {
dev_err(&pdev->dev, "Failed to allocate IRQ.\n");
-   goto error_unmap_reg;
+   goto error_free_device;
}
 
-   st->clk = clk_get(&pdev->dev, "adc_clk");
+   st->clk = devm_clk_get(&pdev->dev, "adc_clk");
if (IS_ERR(st->clk)) {
dev_err(&pdev->dev, "Failed to get the clock.\n");
ret = PTR_ERR(st->clk);
-   goto error_free_irq;
+   goto error_free_device;
}
 
ret = clk_prepare(st->clk);
if (ret) {
dev_err(&pdev->dev, "Could not prepare the clock.\n");
-   goto error_free_clk;
+   goto error_free_device;
}
 
ret = clk_enable(st->clk);
@@ -614,7 +602,7 @@ static int __devinit at91_adc_probe(struct platform_device 
*pdev)
goto error_unprepare_clk;
}
 
-   st->adc_clk = clk_get(&pdev->dev, "adc_op_clk");
+   st->adc_clk = devm_clk_get(&pdev->dev, "adc_op_clk");
if (IS_ERR(st->adc_clk)) {
dev_err(&pdev->dev, "Failed to get the ADC clock.\n");
ret = PTR_ERR(st->clk);
@@ -624,7 +612,7 @@ static int __devinit at91_adc_probe(struct platform_device 
*pdev)
ret = clk_prepare(st->adc_clk);
if (ret) {
dev_err(&pdev->dev, "Could not prepare the ADC clock.\n");
-   goto error_free_adc_clk;
+   goto error_disable_clk;
}
 
ret = clk_enable(st->adc_clk);
@@ -697,20 +685,10 @@ error_disable_adc_clk:
clk_disable(st->adc_clk);
 error_unprepare_adc_clk:
clk_unprepare(st->adc_clk);
-error_free_adc_clk:
-   clk_put(st->adc_clk);
 error_disable_clk:
clk_disable(st->clk);
 error_unprepare_clk:
clk_unprepare(st->clk);
-error_free_clk:
-   clk_put(st->clk);
-error_fre

Re: [PATCH] drivers/iio/adc/at91_adc.c: use devm_ functions

2012-07-31 Thread Julia Lawall

On Tue, 31 Jul 2012, Lars-Peter Clausen wrote:

> Hi,
>
> On 07/31/2012 12:09 PM, Julia Lawall wrote:
> > From: Julia Lawall 
> > @@ -720,20 +698,14 @@ error_ret:
> >  static int __devexit at91_adc_remove(struct platform_device *pdev)
> >  {
> > struct iio_dev *idev = platform_get_drvdata(pdev);
> > -   struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > struct at91_adc_state *st = iio_priv(idev);
> >
> > iio_device_unregister(idev);
> > [...]
> > -   free_irq(st->irq, idev);
> > [...]
> > iio_device_free(idev);
>
> I think we have to be careful here. The interrupted is now freed after the
> device has been freed, which means that it could trigger after the device
> has been freed. And since we use the device in the interrupt handler we'll
> get a use after free.

OK, thanks for the feedback.  I'll try again, and un-devm_ this function.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] drivers/iio/adc/at91_adc.c: use devm_ functions

2012-07-31 Thread Julia Lawall

From: Julia Lawall 

The various devm_ functions allocate memory that is released when a driver
detaches.  This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.

The call to platform_get_resource(pdev, IORESOURCE_MEM, 0) is moved coser
to the call to devm_request_and_ioremap, which is th first use of the
result of platform_get_resource.

This does not use devm_request_irq to ensure that free_irq is executed
before its idev argument is freed.

Signed-off-by: Julia Lawall 

---
 drivers/iio/adc/at91_adc.c |   41 -
 1 file changed, 8 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index f61780a..3506e3d 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -545,13 +545,6 @@ static int __devinit at91_adc_probe(struct platform_device 
*pdev)
goto error_free_device;
}

-   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!res) {
-   dev_err(&pdev->dev, "No resource defined\n");
-   ret = -ENXIO;
-   goto error_ret;
-   }
-
platform_set_drvdata(pdev, idev);

idev->dev.parent = &pdev->dev;
@@ -566,18 +559,13 @@ static int __devinit at91_adc_probe(struct 
platform_device *pdev)
goto error_free_device;
}

-   if (!request_mem_region(res->start, resource_size(res),
-   "AT91 adc registers")) {
-   dev_err(&pdev->dev, "Resources are unavailable.\n");
-   ret = -EBUSY;
-   goto error_free_device;
-   }
+   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

-   st->reg_base = ioremap(res->start, resource_size(res));
+   st->reg_base = devm_request_and_ioremap(&pdev->dev, res);
if (!st->reg_base) {
dev_err(&pdev->dev, "Failed to map registers.\n");
ret = -ENOMEM;
-   goto error_release_mem;
+   goto error_free_device;
}

/*
@@ -592,10 +580,10 @@ static int __devinit at91_adc_probe(struct 
platform_device *pdev)
  idev);
if (ret) {
dev_err(&pdev->dev, "Failed to allocate IRQ.\n");
-   goto error_unmap_reg;
+   goto error_free_device;
}

-   st->clk = clk_get(&pdev->dev, "adc_clk");
+   st->clk = devm_clk_get(&pdev->dev, "adc_clk");
if (IS_ERR(st->clk)) {
dev_err(&pdev->dev, "Failed to get the clock.\n");
ret = PTR_ERR(st->clk);
@@ -605,7 +593,7 @@ static int __devinit at91_adc_probe(struct platform_device 
*pdev)
ret = clk_prepare(st->clk);
if (ret) {
dev_err(&pdev->dev, "Could not prepare the clock.\n");
-   goto error_free_clk;
+   goto error_free_irq;
}

ret = clk_enable(st->clk);
@@ -614,7 +602,7 @@ static int __devinit at91_adc_probe(struct platform_device 
*pdev)
goto error_unprepare_clk;
}

-   st->adc_clk = clk_get(&pdev->dev, "adc_op_clk");
+   st->adc_clk = devm_clk_get(&pdev->dev, "adc_op_clk");
if (IS_ERR(st->adc_clk)) {
dev_err(&pdev->dev, "Failed to get the ADC clock.\n");
ret = PTR_ERR(st->clk);
@@ -624,7 +612,7 @@ static int __devinit at91_adc_probe(struct platform_device 
*pdev)
ret = clk_prepare(st->adc_clk);
if (ret) {
dev_err(&pdev->dev, "Could not prepare the ADC clock.\n");
-   goto error_free_adc_clk;
+   goto error_disable_clk;
}

ret = clk_enable(st->adc_clk);
@@ -697,20 +685,12 @@ error_disable_adc_clk:
clk_disable(st->adc_clk);
 error_unprepare_adc_clk:
clk_unprepare(st->adc_clk);
-error_free_adc_clk:
-   clk_put(st->adc_clk);
 error_disable_clk:
clk_disable(st->clk);
 error_unprepare_clk:
clk_unprepare(st->clk);
-error_free_clk:
-   clk_put(st->clk);
 error_free_irq:
free_irq(st->irq, idev);
-error_unmap_reg:
-   iounmap(st->reg_base);
-error_release_mem:
-   release_mem_region(res->start, resource_size(res));
 error_free_device:
iio_device_free(idev);
 error_ret:
@@ -720,20 +700,15 @@ error_ret:
 static int __devexit at91_adc_remove(struct platform_device *pdev)
 {
struct iio_dev *idev = platform_get_drvdata(pdev);
-   struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
struct at91_adc_state *st = iio_priv(idev);

iio_device_unregiste

Re: [PATCH] drivers/iio/adc/at91_adc.c: use devm_ functions

2012-07-31 Thread Julia Lawall



On Tue, 31 Jul 2012, Lars-Peter Clausen wrote:

> Hi,
>
> On 07/31/2012 12:09 PM, Julia Lawall wrote:
> > From: Julia Lawall 
> > @@ -720,20 +698,14 @@ error_ret:
> >  static int __devexit at91_adc_remove(struct platform_device *pdev)
> >  {
> > struct iio_dev *idev = platform_get_drvdata(pdev);
> > -   struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > struct at91_adc_state *st = iio_priv(idev);
> >
> > iio_device_unregister(idev);
> > [...]
> > -   free_irq(st->irq, idev);
> > [...]
> > iio_device_free(idev);
>
> I think we have to be careful here. The interrupted is now freed after the
> device has been freed, which means that it could trigger after the device
> has been freed. And since we use the device in the interrupt handler we'll
> get a use after free.

Perhaps the same would be true in the following code, from the file
drivers/edac/highbank_l2_edac.c:

res = devm_request_irq(&pdev->dev, drvdata->sb_irq,
   highbank_l2_err_handler,
   0, dev_name(&pdev->dev), dci);
if (res < 0)
goto err;

dci->mod_name = dev_name(&pdev->dev);
dci->dev_name = dev_name(&pdev->dev);

if (edac_device_add_device(dci))
goto err;

devres_close_group(&pdev->dev, NULL);
return 0;
err:
devres_release_group(&pdev->dev, NULL);
edac_device_free_ctl_info(dci);

Is devm_request_irq perhaps not a very good idea?

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

1 2 3 4 5 6 7 8 9 10 >

1 - 100 of 4211 matches

Mail list logo