[PATCH 13/15] drivers/usb/musb/omap2430.c: adjust duplicate test

2013-01-21 Thread Julia Lawall
From: Julia Lawall 

Delete successive tests to the same location.  Data is the just previously
allocated and tested value.  Test the result of the allocation made here
instead.

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\)
when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// 

Signed-off-by: Julia Lawall 

---
The previous code could be further simplified by removing the
initializations of ret to -ENOMEM, as is absent in this case.  But that is
probably for another patch.

 drivers/usb/musb/omap2430.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index da00af4..533c4fd 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -545,7 +545,7 @@ static int omap2430_probe(struct platform_device *pdev)
}
 
config = devm_kzalloc(&pdev->dev, sizeof(*config), GFP_KERNEL);
-   if (!data) {
+   if (!config) {
dev_err(&pdev->dev,
"failed to allocate musb hdrc config\n");
goto err2;

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


[PATCH 2/15] drivers/usb/chipidea/core.c: adjust duplicate test

2013-01-21 Thread Julia Lawall
From: Julia Lawall 

Delete successive tests to the same location.  In this case res has already
been tested for being NULL, and calling devm_request_and_ioremap will not
make it NULL.  On the other hand, devm_request_and_ioremap can return NULL
on failure.

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\)
when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// 

Signed-off-by: Julia Lawall 

---
This code could be improved in other ways.  The previous NULL test on res
is not needed, and the dev_err in the error-handling code for the call to
devm_request_and_ioremap is redundant with respect to the logging already
done by devm_request_and_ioremap.  But these should probably be done
separately.

 drivers/usb/chipidea/core.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index aebf695..57cae1f 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -411,7 +411,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
}
 
base = devm_request_and_ioremap(dev, res);
-   if (!res) {
+   if (!base) {
dev_err(dev, "can't request and ioremap resource\n");
return -ENOMEM;
}

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


[PATCH 00/26] constify local structures

2016-09-11 Thread Julia Lawall
Constify local structures.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// 
// The first rule ignores some cases that posed problems
@r disable optional_qualifier@
identifier s != {peri_clk_data,threshold_attr,tracer_flags,tracer};
identifier i != {s5k5baf_cis_rect,smtcfb_fix};
position p;
@@
static struct s i@p = { ... };

@lstruct@
identifier r.s;
@@
struct s { ... };

@used depends on lstruct@
identifier r.i;
@@
i

@bad1@
expression e;
identifier r.i;
assignment operator a;
@@
 (<+...i...+>) a e

@bad2@
identifier r.i;
@@
 &(<+...i...+>)

@bad3@
identifier r.i;
declarer d;
@@
 d(...,<+...i...+>,...);

@bad4@
identifier r.i;
type T;
T[] e;
identifier f;
position p;
@@

f@p(...,
(
  (<+...i...+>)
&
  e
)
,...)

@bad4a@
identifier r.i;
type T;
T *e;
identifier f;
position p;
@@

f@p(...,
(
  (<+...i...+>)
&
  e
)
,...)

@ok5@
expression *e;
identifier r.i;
position p;
@@
e =@p i

@bad5@
expression *e;
identifier r.i;
position p != ok5.p;
@@
e =@p (<+...i...+>)

@rr depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5@
identifier s,r.i;
position r.p;
@@

static
+const
 struct s i@p = { ... };

@depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5
 disable optional_qualifier@
identifier rr.s,r.i;
@@

static
+const
 struct s i;
// 

---

 drivers/acpi/acpi_apd.c  |8 +++---
 drivers/char/tpm/tpm-interface.c |   10 
 drivers/char/tpm/tpm-sysfs.c |2 -
 drivers/cpufreq/intel_pstate.c   |8 +++---
 drivers/infiniband/hw/i40iw/i40iw_uk.c   |6 ++---
 drivers/media/i2c/tvp514x.c  |2 -
 drivers/media/pci/ddbridge/ddbridge-core.c   |   18 +++
 drivers/media/pci/ngene/ngene-cards.c|   14 ++--
 drivers/media/pci/smipcie/smipcie-main.c |8 +++---
 drivers/misc/sgi-xp/xpc_uv.c |2 -
 drivers/net/arcnet/com20020-pci.c|   10 
 drivers/net/can/c_can/c_can_pci.c|4 +--
 drivers/net/can/sja1000/plx_pci.c|   20 -
 drivers/net/ethernet/mellanox/mlx4/main.c|4 +--
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c |2 -
 drivers/net/ethernet/renesas/sh_eth.c|   14 ++--
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c |2 -
 drivers/net/wireless/ath/dfs_pattern_detector.c  |2 -
 drivers/net/wireless/intel/iwlegacy/3945.c   |4 +--
 drivers/net/wireless/realtek/rtlwifi/rtl8188ee/sw.c  |2 -
 drivers/net/wireless/realtek/rtlwifi/rtl8192ce/sw.c  |2 -
 drivers/net/wireless/realtek/rtlwifi/rtl8192de/sw.c  |2 -
 drivers/net/wireless/realtek/rtlwifi/rtl8192ee/sw.c  |2 -
 drivers/net/wireless/realtek/rtlwifi/rtl8192se/sw.c  |2 -
 drivers/net/wireless/realtek/rtlwifi/rtl8723ae/sw.c  |2 -
 drivers/net/wireless/realtek/rtlwifi/rtl8723be/sw.c  |2 -
 drivers/net/wireless/realtek/rtlwifi/rtl8821ae/sw.c  |2 -
 drivers/platform/chrome/chromeos_laptop.c|   22 +--
 drivers/platform/x86/intel_scu_ipc.c |6 ++---
 drivers/platform/x86/intel_telemetry_debugfs.c   |2 -
 drivers/scsi/esas2r/esas2r_flash.c   |2 -
 drivers/scsi/hptiop.c|6 ++---
 drivers/spi/spi-dw-pci.c |4 +--
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c |2 -
 drivers/usb/misc/ezusb.c |2 -
 drivers/video/fbdev/matrox/matroxfb_g450.c   |2 -
 lib/crc64_ecma.c |2 -
 sound/pci/ctxfi/ctatc.c  |2 -
 sound/pci/hda/patch_ca0132.c |   10 
 sound/pci/riptide/riptide.c  |2 -
 40 files changed, 110 insertions(+), 110 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 16/26] ezusb: constify local structures

2016-09-11 Thread Julia Lawall
For structure types defined in the same file or local header files, find
top-level static structure declarations that have the following
properties:
1. Never reassigned.
2. Address never taken
3. Not passed to a top-level macro call
4. No pointer or array-typed field passed to a function or stored in a
variable.
Declare structures having all of these properties as const.

Done using Coccinelle.
Based on a suggestion by Joe Perches .

Signed-off-by: Julia Lawall 

---
The semantic patch seems too long for a commit log, but is in the cover
letter.

 drivers/usb/misc/ezusb.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/misc/ezusb.c b/drivers/usb/misc/ezusb.c
index 947811b..837208f 100644
--- a/drivers/usb/misc/ezusb.c
+++ b/drivers/usb/misc/ezusb.c
@@ -22,7 +22,7 @@ struct ezusb_fx_type {
unsigned short max_internal_adress;
 };
 
-static struct ezusb_fx_type ezusb_fx1 = {
+static const struct ezusb_fx_type ezusb_fx1 = {
.cpucs_reg = 0x7F92,
.max_internal_adress = 0x1B3F,
 };

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


Re: [PATCH 00/26] constify local structures

2016-09-11 Thread Julia Lawall

On Sun, 11 Sep 2016, Joe Perches wrote:

> On Sun, 2016-09-11 at 15:05 +0200, Julia Lawall wrote:
> > Constify local structures.
>
> Thanks Julia.
>
> A few suggestions & questions:
>
> Perhaps the script should go into scripts/coccinelle/
> so that future cases could be caught by the robot
> and commit message referenced by the patch instances.

OK.

> Can you please compile the files modified using the
> appropriate defconfig/allyesconfig and show the

I currently send patches for this issue only for files that compile using
the x86 allyesconfig.

> movement from data to const by using
>   $ size .new/old
> and include that in the changelogs (maybe next time)?

OK, thanks for the suggestion.

> Is it possible for a rule to trace the instances where
> an address of a struct or struct member is taken by
> locally defined and declared function call where the
> callee does not modify any dereferenced object?
>
> ie:
>
> struct foo {
>   int bar;
>   char *baz;
> };
>
> struct foo qux[] = {
>   { 1, "description 1" },
>   { 2, "dewcription 2" },
>   [ n, "etc" ]...,
> };
>
> void message(struct foo *msg)
> {
>   printk("%d %s\n", msg->bar, msg->baz);
> }
>
> where some code uses
>
>   message(qux[index]);
>
> So could a coccinelle script change:
>
> struct foo qux[] = { to const struct foo quz[] = {
>
> and
>
> void message(struct foo *msg) to void message(const struct foo *msg)

Yes, this could be possible too.

Thanks for the feedback.

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


Re: [PATCH 00/26] constify local structures

2016-09-12 Thread Julia Lawall


On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:

> On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> > Constify local structures.
> >
> > The semantic patch that makes this change is as follows:
> > (http://coccinelle.lip6.fr/)
>
> Just my two cents but:
>
> 1. You *can* use a static analysis too to find bugs or other issues.
> 2. However, you should manually do the commits and proper commit
>messages to subsystems based on your findings. And I generally think
>that if one contributes code one should also at least smoke test changes
>somehow.
>
> I don't know if I'm alone with my opinion. I just think that one should
> also do the analysis part and not blindly create and submit patches.

All of the patches are compile tested.  And the individual patches are
submitted to the relevant maintainers.  The individual commit messages
give a more detailed explanation of the strategy used to decide that the
structure was constifiable.  It seemed redundant to put that in the cover
letter, which will not be committed anyway.

julia

>
> Anyway, I'll apply the TPM change at some point. As I said they were
> for better. Thanks.
>
> /Jarkko
>
> > // 
> > // The first rule ignores some cases that posed problems
> > @r disable optional_qualifier@
> > identifier s != {peri_clk_data,threshold_attr,tracer_flags,tracer};
> > identifier i != {s5k5baf_cis_rect,smtcfb_fix};
> > position p;
> > @@
> > static struct s i@p = { ... };
> >
> > @lstruct@
> > identifier r.s;
> > @@
> > struct s { ... };
> >
> > @used depends on lstruct@
> > identifier r.i;
> > @@
> > i
> >
> > @bad1@
> > expression e;
> > identifier r.i;
> > assignment operator a;
> > @@
> >  (<+...i...+>) a e
> >
> > @bad2@
> > identifier r.i;
> > @@
> >  &(<+...i...+>)
> >
> > @bad3@
> > identifier r.i;
> > declarer d;
> > @@
> >  d(...,<+...i...+>,...);
> >
> > @bad4@
> > identifier r.i;
> > type T;
> > T[] e;
> > identifier f;
> > position p;
> > @@
> >
> > f@p(...,
> > (
> >   (<+...i...+>)
> > &
> >   e
> > )
> > ,...)
> >
> > @bad4a@
> > identifier r.i;
> > type T;
> > T *e;
> > identifier f;
> > position p;
> > @@
> >
> > f@p(...,
> > (
> >   (<+...i...+>)
> > &
> >   e
> > )
> > ,...)
> >
> > @ok5@
> > expression *e;
> > identifier r.i;
> > position p;
> > @@
> > e =@p i
> >
> > @bad5@
> > expression *e;
> > identifier r.i;
> > position p != ok5.p;
> > @@
> > e =@p (<+...i...+>)
> >
> > @rr depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5@
> > identifier s,r.i;
> > position r.p;
> > @@
> >
> > static
> > +const
> >  struct s i@p = { ... };
> >
> > @depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5
> >  disable optional_qualifier@
> > identifier rr.s,r.i;
> > @@
> >
> > static
> > +const
> >  struct s i;
> > // 
> >
> > ---
> >
> >  drivers/acpi/acpi_apd.c  |8 +++---
> >  drivers/char/tpm/tpm-interface.c |   10 
> >  drivers/char/tpm/tpm-sysfs.c |2 -
> >  drivers/cpufreq/intel_pstate.c   |8 +++---
> >  drivers/infiniband/hw/i40iw/i40iw_uk.c   |6 ++---
> >  drivers/media/i2c/tvp514x.c  |2 -
> >  drivers/media/pci/ddbridge/ddbridge-core.c   |   18 +++
> >  drivers/media/pci/ngene/ngene-cards.c|   14 ++--
> >  drivers/media/pci/smipcie/smipcie-main.c |8 +++---
> >  drivers/misc/sgi-xp/xpc_uv.c |2 -
> >  drivers/net/arcnet/com20020-pci.c|   10 
> >  drivers/net/can/c_can/c_can_pci.c|4 +--
> >  drivers/net/can/sja1000/plx_pci.c|   20 
> > -
> >  drivers/net/ethernet/mellanox/mlx4/main.c|4 +--
> >  drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c |2 -
> >  drivers/net/ethernet/renesas/sh_eth.c|   14 ++--
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c |   

Re: [PATCH 00/26] constify local structures

2016-09-12 Thread Julia Lawall


On Mon, 12 Sep 2016, Jarkko Sakkinen wrote:

> On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> >
> >
> > On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> >
> > > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> > > > Constify local structures.
> > > >
> > > > The semantic patch that makes this change is as follows:
> > > > (http://coccinelle.lip6.fr/)
> > >
> > > Just my two cents but:
> > >
> > > 1. You *can* use a static analysis too to find bugs or other issues.
> > > 2. However, you should manually do the commits and proper commit
> > >messages to subsystems based on your findings. And I generally think
> > >that if one contributes code one should also at least smoke test 
> > > changes
> > >somehow.
> > >
> > > I don't know if I'm alone with my opinion. I just think that one should
> > > also do the analysis part and not blindly create and submit patches.
> >
> > All of the patches are compile tested.  And the individual patches are
>
> Compile-testing is not testing. If you are not able to test a commit,
> you should explain why.
>
> > submitted to the relevant maintainers.  The individual commit messages
> > give a more detailed explanation of the strategy used to decide that the
> > structure was constifiable.  It seemed redundant to put that in the cover
> > letter, which will not be committed anyway.
>
> I don't mean to be harsh but I do not care about your thought process
> *that much* when I review a commit (sometimes it might make sense to
> explain that but it depends on the context).
>
> I mostly only care why a particular change makes sense for this
> particular subsystem. The report given by a static analysis tool can
> be a starting point for making a commit but it's not sufficient.
> Based on the report you should look subsystems as individuals.

OK, thanks for the feedback.

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


Re: [PATCH 00/26] constify local structures

2016-09-12 Thread Julia Lawall


On Mon, 12 Sep 2016, Felipe Balbi wrote:

>
> Hi,
>
> Jarkko Sakkinen  writes:
> > On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> >>
> >>
> >> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> >>
> >> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> >> > > Constify local structures.
> >> > >
> >> > > The semantic patch that makes this change is as follows:
> >> > > (http://coccinelle.lip6.fr/)
> >> >
> >> > Just my two cents but:
> >> >
> >> > 1. You *can* use a static analysis too to find bugs or other issues.
> >> > 2. However, you should manually do the commits and proper commit
> >> >messages to subsystems based on your findings. And I generally think
> >> >that if one contributes code one should also at least smoke test 
> >> > changes
> >> >somehow.
> >> >
> >> > I don't know if I'm alone with my opinion. I just think that one should
> >> > also do the analysis part and not blindly create and submit patches.
> >>
> >> All of the patches are compile tested.  And the individual patches are
> >
> > Compile-testing is not testing. If you are not able to test a commit,
> > you should explain why.
>
> Dude, Julia has been doing semantic patching for years already and
> nobody has raised any concerns so far. There's already an expectation
> that Coccinelle *works* and Julia's sematic patches are sound.
>
> Besides, adding 'const' is something that causes virtually no functional
> changes to the point that build-testing is really all you need. Any
> problems caused by adding 'const' to a definition will be seen by build
> errors or warnings.
>
> Really, just stop with the pointless discussion and go read a bit about
> Coccinelle and what semantic patches are giving you. The work done by
> Julia and her peers are INRIA have measurable benefits.
>
> You're really making a thunderstorm in a glass of water.

Thanks for the defense, but since a lot of these patches torned out to be
wrong, due to an incorrect parse by Coccinelle, combined with an
unpleasantly lax compiler, Jarkko does have a point that I should have
looked at the patches more carefully.  In any case, I have written to the
maintainers relevant to the patches that turned out to be incorrect.

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


Re: [PATCH 00/26] constify local structures

2016-09-12 Thread Julia Lawall


On Mon, 12 Sep 2016, Jarkko Sakkinen wrote:

> On Mon, Sep 12, 2016 at 04:43:58PM +0300, Felipe Balbi wrote:
> >
> > Hi,
> >
> > Jarkko Sakkinen  writes:
> > > On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> > >>
> > >>
> > >> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> > >>
> > >> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> > >> > > Constify local structures.
> > >> > >
> > >> > > The semantic patch that makes this change is as follows:
> > >> > > (http://coccinelle.lip6.fr/)
> > >> >
> > >> > Just my two cents but:
> > >> >
> > >> > 1. You *can* use a static analysis too to find bugs or other issues.
> > >> > 2. However, you should manually do the commits and proper commit
> > >> >messages to subsystems based on your findings. And I generally think
> > >> >that if one contributes code one should also at least smoke test 
> > >> > changes
> > >> >somehow.
> > >> >
> > >> > I don't know if I'm alone with my opinion. I just think that one should
> > >> > also do the analysis part and not blindly create and submit patches.
> > >>
> > >> All of the patches are compile tested.  And the individual patches are
> > >
> > > Compile-testing is not testing. If you are not able to test a commit,
> > > you should explain why.
> >
> > Dude, Julia has been doing semantic patching for years already and
> > nobody has raised any concerns so far. There's already an expectation
> > that Coccinelle *works* and Julia's sematic patches are sound.
> >
> > Besides, adding 'const' is something that causes virtually no functional
> > changes to the point that build-testing is really all you need. Any
> > problems caused by adding 'const' to a definition will be seen by build
> > errors or warnings.
> >
> > Really, just stop with the pointless discussion and go read a bit about
> > Coccinelle and what semantic patches are giving you. The work done by
> > Julia and her peers are INRIA have measurable benefits.
> >
> > You're really making a thunderstorm in a glass of water.
>
> Hmm... I've been using coccinelle in cyclic basis for some time now.
> My comment was oversized but I didn't mean it to be impolite or attack
> of any kind for that matter.

No problem :)  Thanks for the feedback.

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


Re: [Outreachy kernel] [PATCH 5/6] staging: lustre: Remove unnecessary 'return'

2016-09-15 Thread Julia Lawall
On Thu, 15 Sep 2016, Rehas Sachdeva wrote:

> Fixes checkpatch.pl warning:
> WARNING: void function return statements are not generally useful.

It would be better to say what you did, rather than saying fix.  Here you
could say:

Remove unnecessary void return at the end of a function.  Issue detected
by checkpatch.

julia


>
> Signed-off-by: Rehas Sachdeva 
> ---
>  drivers/staging/lustre/lustre/llite/rw.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/llite/rw.c 
> b/drivers/staging/lustre/lustre/llite/rw.c
> index bb85d16..23747fa 100644
> --- a/drivers/staging/lustre/lustre/llite/rw.c
> +++ b/drivers/staging/lustre/lustre/llite/rw.c
> @@ -667,7 +667,6 @@ static void ras_update_stride_detector(struct 
> ll_readahead_state *ras,
>   ras->ras_stride_length = stride_gap + ras->ras_consecutive_pages;
>
>   RAS_CDEBUG(ras);
> - return;
>  }
>
>  /* Stride Read-ahead window will be increased inc_len according to
> @@ -883,7 +882,6 @@ out_unlock:
>   RAS_CDEBUG(ras);
>   ras->ras_request_index++;
>   spin_unlock(&ras->ras_lock);
> - return;
>  }
>
>  int ll_writepage(struct page *vmpage, struct writeback_control *wbc)
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/bfedf0128abd4c08512b015481be1d5fd1d0d01e.1473923837.git.aquannie%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


question about drivers/usb/musb/musb_core.c

2016-10-28 Thread Julia Lawall
The file drivers/usb/musb/musb_core.c contains the code:

static DEVICE_ATTR(srp, 0644, NULL, musb_srp_store);

Is it correct to have NULL in the third argument for an attribute that can
be read?  Should the permission be 0444 instead?

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


Re: question about drivers/usb/musb/musb_core.c

2016-10-28 Thread Julia Lawall


On Fri, 28 Oct 2016, Julia Lawall wrote:

> The file drivers/usb/musb/musb_core.c contains the code:
>
> static DEVICE_ATTR(srp, 0644, NULL, musb_srp_store);
>
> Is it correct to have NULL in the third argument for an attribute that can
> be read?  Should the permission be 0444 instead?

Sorry, I got that backwards.  Should the permission be 0200?

julia

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


Re: question about drivers/usb/musb/musb_core.c

2016-10-28 Thread Julia Lawall
[Corrected email for Felipe]

On Fri, 28 Oct 2016, Greg Kroah-Hartman wrote:

> On Fri, Oct 28, 2016 at 10:33:19PM +0200, Julia Lawall wrote:
> >
> >
> > On Fri, 28 Oct 2016, Julia Lawall wrote:
> >
> > > The file drivers/usb/musb/musb_core.c contains the code:
> > >
> > > static DEVICE_ATTR(srp, 0644, NULL, musb_srp_store);
> > >
> > > Is it correct to have NULL in the third argument for an attribute that can
> > > be read?  Should the permission be 0444 instead?
> >
> > Sorry, I got that backwards.  Should the permission be 0200?
>
> Even better yet, it should be using DEVICE_ATTR_WO() to be explicit and
> make it impossible to get wrong.
>
> Unless this file is set up this way for userspace to later change the
> permission so that others can write to it?  I don't know the history
> here...

Felipe,

This line has been there since your commit 550a7375fe720 introduced the
file in 2008.  Do you recall any reason why there should be a special case
here?

julia

>
> thanks,
>
> greg k-h
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: question about drivers/usb/musb/musb_core.c

2016-10-28 Thread Julia Lawall


On Fri, 28 Oct 2016, Greg Kroah-Hartman wrote:

> On Fri, Oct 28, 2016 at 10:33:19PM +0200, Julia Lawall wrote:
> >
> >
> > On Fri, 28 Oct 2016, Julia Lawall wrote:
> >
> > > The file drivers/usb/musb/musb_core.c contains the code:
> > >
> > > static DEVICE_ATTR(srp, 0644, NULL, musb_srp_store);
> > >
> > > Is it correct to have NULL in the third argument for an attribute that can
> > > be read?  Should the permission be 0444 instead?
> >
> > Sorry, I got that backwards.  Should the permission be 0200?
>
> Even better yet, it should be using DEVICE_ATTR_WO() to be explicit and
> make it impossible to get wrong.
>
> Unless this file is set up this way for userspace to later change the
> permission so that others can write to it?  I don't know the history
> here...

Felipe,

This line has been there since your commit 550a7375fe720 introduced the
file in 2008.  Do you recall any reason why there should be a special case
here?

julia

>
> thanks,
>
> greg k-h
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: question about drivers/usb/musb/musb_core.c

2016-10-28 Thread Julia Lawall


On Fri, 28 Oct 2016, Bin Liu wrote:

> On Fri, Oct 28, 2016 at 04:53:03PM -0400, Greg Kroah-Hartman wrote:
> > On Fri, Oct 28, 2016 at 10:33:19PM +0200, Julia Lawall wrote:
> > >
> > >
> > > On Fri, 28 Oct 2016, Julia Lawall wrote:
> > >
> > > > The file drivers/usb/musb/musb_core.c contains the code:
> > > >
> > > > static DEVICE_ATTR(srp, 0644, NULL, musb_srp_store);
> > > >
> > > > Is it correct to have NULL in the third argument for an attribute that 
> > > > can
> > > > be read?  Should the permission be 0444 instead?
> > >
> > > Sorry, I got that backwards.  Should the permission be 0200?
> >
> > Even better yet, it should be using DEVICE_ATTR_WO() to be explicit and
> > make it impossible to get wrong.
> >
> > Unless this file is set up this way for userspace to later change the
> > permission so that others can write to it?  I don't know the history
> > here...
>
> The musb driver history has so many unknowns... I am wondering if there
> is still anyone uses srp on musb...
>
> But I think we can change the driver to whatever we need nowadays, just
> like what we did on parameter use_dma about a year ago (51676c8d).

Thanks for the feedback.

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


[PATCH 00/15] use permission-specific DEVICE_ATTR variants

2016-10-29 Thread Julia Lawall

Use DEVICE_ATTR_RO etc. for read only attributes etc.  This simplifies the
source code, improves readbility, and reduces the chance of
inconsistencies.

The complete semantic patch is as follows:
(http://coccinelle.lip6.fr/)

// 
@ro@
declarer name DEVICE_ATTR;
identifier x,x_show;
@@

DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);

@wo@
declarer name DEVICE_ATTR;
identifier x,x_store;
@@

DEVICE_ATTR(x, \(0200\|S_IWUSR\), NULL, x_store);

@rw@
declarer name DEVICE_ATTR;
identifier x,x_show,x_store;
@@

DEVICE_ATTR(x, \(0644\|S_IRUGO|S_IWUSR\), x_show, x_store);

@script:ocaml@
x << ro.x;
x_show << ro.x_show;
@@

if not (x^"_show" = x_show) then Coccilib.include_match false

@script:ocaml@
x << wo.x;
x_store << wo.x_store;
@@

if not (x^"_store" = x_store) then Coccilib.include_match false

@script:ocaml@
x << rw.x;
x_show << rw.x_show;
x_store << rw.x_store;
@@

if not (x^"_show" = x_show && x^"_store" = x_store)
then Coccilib.include_match false

@@
declarer name DEVICE_ATTR_RO;
identifier ro.x,ro.x_show;
@@

- DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);
+ DEVICE_ATTR_RO(x);

@@
declarer name DEVICE_ATTR_WO;
identifier wo.x,wo.x_store;
@@

- DEVICE_ATTR(x, \(0200\|S_IWUSR\), NULL, x_store);
+ DEVICE_ATTR_WO(x);

@@
declarer name DEVICE_ATTR_RW;
identifier rw.x,rw.x_show,rw.x_store;
@@

- DEVICE_ATTR(x, \(0644\|S_IRUGO|S_IWUSR\), x_show, x_store);
+ DEVICE_ATTR_RW(x);
// 

---

 arch/mips/txx9/generic/7segled.c  |4 ++--
 arch/powerpc/kernel/iommu.c   |3 +--
 arch/tile/kernel/sysfs.c  |   14 +++---
 drivers/atm/solos-pci.c   |2 +-
 drivers/pci/pcie/aspm.c   |4 ++--
 drivers/power/supply/wm8350_power.c   |2 +-
 drivers/ptp/ptp_sysfs.c   |2 +-
 drivers/thermal/int340x_thermal/int3400_thermal.c |2 +-
 drivers/thermal/thermal_hwmon.c   |2 +-
 drivers/tty/nozomi.c  |4 ++--
 drivers/usb/wusbcore/dev-sysfs.c  |6 +++---
 drivers/usb/wusbcore/wusbhc.c |   13 +
 drivers/video/fbdev/wm8505fb.c|2 +-
 sound/soc/omap/mcbsp.c|4 ++--
 sound/soc/soc-dapm.c  |2 +-
 15 files changed, 31 insertions(+), 35 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 02/15] wusbcore: wusbhc: use permission-specific DEVICE_ATTR variants

2016-10-29 Thread Julia Lawall
Use DEVICE_ATTR_RW for read-write attributes.  This simplifies the
source code, improves readbility, and reduces the chance of
inconsistencies.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// 
@rw@
declarer name DEVICE_ATTR;
identifier x,x_show,x_store;
@@

DEVICE_ATTR(x, \(0644\|S_IRUGO|S_IWUSR\), x_show, x_store);

@script:ocaml@
x << rw.x;
x_show << rw.x_show;
x_store << rw.x_store;
@@

if not (x^"_show" = x_show && x^"_store" = x_store)
then Coccilib.include_match false

@@
declarer name DEVICE_ATTR_RW;
identifier rw.x,rw.x_show,rw.x_store;
@@

- DEVICE_ATTR(x, \(0644\|S_IRUGO|S_IWUSR\), x_show, x_store);
+ DEVICE_ATTR_RW(x);
// 

Signed-off-by: Julia Lawall 

---
 drivers/usb/wusbcore/wusbhc.c |   13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/wusbcore/wusbhc.c b/drivers/usb/wusbcore/wusbhc.c
index 94f401a..a273a91 100644
--- a/drivers/usb/wusbcore/wusbhc.c
+++ b/drivers/usb/wusbcore/wusbhc.c
@@ -84,8 +84,7 @@ static ssize_t wusb_trust_timeout_store(struct device *dev,
 out:
return result < 0 ? result : size;
 }
-static DEVICE_ATTR(wusb_trust_timeout, 0644, wusb_trust_timeout_show,
-wusb_trust_timeout_store);
+static DEVICE_ATTR_RW(wusb_trust_timeout);
 
 /*
  * Show the current WUSB CHID.
@@ -145,7 +144,7 @@ static ssize_t wusb_chid_store(struct device *dev,
result = wusbhc_chid_set(wusbhc, &chid);
return result < 0 ? result : size;
 }
-static DEVICE_ATTR(wusb_chid, 0644, wusb_chid_show, wusb_chid_store);
+static DEVICE_ATTR_RW(wusb_chid);
 
 
 static ssize_t wusb_phy_rate_show(struct device *dev,
@@ -174,8 +173,7 @@ static ssize_t wusb_phy_rate_store(struct device *dev,
wusbhc->phy_rate = phy_rate;
return size;
 }
-static DEVICE_ATTR(wusb_phy_rate, 0644, wusb_phy_rate_show,
-   wusb_phy_rate_store);
+static DEVICE_ATTR_RW(wusb_phy_rate);
 
 static ssize_t wusb_dnts_show(struct device *dev,
  struct device_attribute *attr,
@@ -205,7 +203,7 @@ static ssize_t wusb_dnts_store(struct device *dev,
 
return size;
 }
-static DEVICE_ATTR(wusb_dnts, 0644, wusb_dnts_show, wusb_dnts_store);
+static DEVICE_ATTR_RW(wusb_dnts);
 
 static ssize_t wusb_retry_count_show(struct device *dev,
  struct device_attribute *attr,
@@ -234,8 +232,7 @@ static ssize_t wusb_retry_count_store(struct device *dev,
 
return size;
 }
-static DEVICE_ATTR(wusb_retry_count, 0644, wusb_retry_count_show,
-   wusb_retry_count_store);
+static DEVICE_ATTR_RW(wusb_retry_count);
 
 /* Group all the WUSBHC attributes */
 static struct attribute *wusbhc_attrs[] = {

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


[PATCH 01/15] wusbcore: dev-sysfs: use permission-specific DEVICE_ATTR variants

2016-10-29 Thread Julia Lawall
Use DEVICE_ATTR_WO for write ony attributes and DEVICE_ATTR_RO for read
only attributes.  This simplifies the source code, improves readbility,
and reduces the chance of inconsistencies.

The semantic patch for the RO case is as follows:
(http://coccinelle.lip6.fr/)

// 
@ro@
declarer name DEVICE_ATTR;
identifier x,x_show;
@@

DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);

@script:ocaml@
x << ro.x;
x_show << ro.x_show;
@@

if not (x^"_show" = x_show) then Coccilib.include_match false

@@
declarer name DEVICE_ATTR_RO;
identifier ro.x,ro.x_show;
@@

- DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);
+ DEVICE_ATTR_RO(x);
// 

Signed-off-by: Julia Lawall 

---
 drivers/usb/wusbcore/dev-sysfs.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/wusbcore/dev-sysfs.c b/drivers/usb/wusbcore/dev-sysfs.c
index 415b140..d4de56b 100644
--- a/drivers/usb/wusbcore/dev-sysfs.c
+++ b/drivers/usb/wusbcore/dev-sysfs.c
@@ -53,7 +53,7 @@ static ssize_t wusb_disconnect_store(struct device *dev,
wusbhc_put(wusbhc);
return size;
 }
-static DEVICE_ATTR(wusb_disconnect, 0200, NULL, wusb_disconnect_store);
+static DEVICE_ATTR_WO(wusb_disconnect);
 
 static ssize_t wusb_cdid_show(struct device *dev,
  struct device_attribute *attr, char *buf)
@@ -69,7 +69,7 @@ static ssize_t wusb_cdid_show(struct device *dev,
wusb_dev_put(wusb_dev);
return result + 1;
 }
-static DEVICE_ATTR(wusb_cdid, 0444, wusb_cdid_show, NULL);
+static DEVICE_ATTR_RO(wusb_cdid);
 
 static ssize_t wusb_ck_store(struct device *dev,
 struct device_attribute *attr,
@@ -105,7 +105,7 @@ static ssize_t wusb_ck_store(struct device *dev,
wusbhc_put(wusbhc);
return result < 0 ? result : size;
 }
-static DEVICE_ATTR(wusb_ck, 0200, NULL, wusb_ck_store);
+static DEVICE_ATTR_WO(wusb_ck);
 
 static struct attribute *wusb_dev_attrs[] = {
&dev_attr_wusb_disconnect.attr,

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


Re: [PATCH 2/9] usb: host: max3421-hcd: use list_for_each_entry*

2015-12-18 Thread Julia Lawall
Geliang,

Please check whether line 762 can be reached in the case where the
list_for_each_entry reaches the end of the list.  If that can happen,
max3421_ep should not be dereferenced.

julia

On Sat, 19 Dec 2015, kbuild test robot wrote:

> CC: kbuild-...@01.org
> In-Reply-To: 
> <45e8397e370ed99ceb8aa1719c2b56a7ce741eb3.1450455485.git.geliangt...@163.com>
> TO: Geliang Tang 
> CC: Greg Kroah-Hartman , Sergei Shtylyov 
> 
> CC: Geliang Tang , linux-usb@vger.kernel.org, 
> linux-ker...@vger.kernel.org
>
> Hi Geliang,
>
> [auto build test WARNING on usb/usb-testing]
> [also build test WARNING on v4.4-rc5 next-20151218]
>
> url:
> https://github.com/0day-ci/linux/commits/Geliang-Tang/usb-host-fotg210-use-list_for_each_entry_safe/20151219-003955
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git 
> usb-testing
> :: branch date: 66 minutes ago
> :: commit date: 66 minutes ago
>
> >> drivers/usb/host/max3421-hcd.c:762:5-15: ERROR: invalid reference to the 
> >> index variable of the iterator on line 675
>
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 610e92adf2a45ef78039582494d8023f385d12ec
> vim +762 drivers/usb/host/max3421-hcd.c
>
> 2d53139f David Mosberger 2014-04-28  669
> 2d53139f David Mosberger 2014-04-28  670  
> spin_lock_irqsave(&max3421_hcd->lock, flags);
> 2d53139f David Mosberger 2014-04-28  671
> 2d53139f David Mosberger 2014-04-28  672  for (;
> 2d53139f David Mosberger 2014-04-28  673   max3421_hcd->sched_pass < 
> SCHED_PASS_DONE;
> 2d53139f David Mosberger 2014-04-28  674   ++max3421_hcd->sched_pass)
> 610e92ad Geliang Tang2015-12-19 @675  
> list_for_each_entry(max3421_ep, &max3421_hcd->ep_list,
> 610e92ad Geliang Tang2015-12-19  676  
> ep_list) {
> 2d53139f David Mosberger 2014-04-28  677  urb = NULL;
> 2d53139f David Mosberger 2014-04-28  678  ep = 
> max3421_ep->ep;
> 2d53139f David Mosberger 2014-04-28  679
> 2d53139f David Mosberger 2014-04-28  680  switch 
> (usb_endpoint_type(&ep->desc)) {
> 2d53139f David Mosberger 2014-04-28  681  case 
> USB_ENDPOINT_XFER_ISOC:
> 2d53139f David Mosberger 2014-04-28  682  case 
> USB_ENDPOINT_XFER_INT:
> 2d53139f David Mosberger 2014-04-28  683  if 
> (max3421_hcd->sched_pass !=
> 2d53139f David Mosberger 2014-04-28  684  
> SCHED_PASS_PERIODIC)
> 2d53139f David Mosberger 2014-04-28  685  
> continue;
> 2d53139f David Mosberger 2014-04-28  686  break;
> 2d53139f David Mosberger 2014-04-28  687
> 2d53139f David Mosberger 2014-04-28  688  case 
> USB_ENDPOINT_XFER_CONTROL:
> 2d53139f David Mosberger 2014-04-28  689  case 
> USB_ENDPOINT_XFER_BULK:
> 2d53139f David Mosberger 2014-04-28  690  if 
> (max3421_hcd->sched_pass !=
> 2d53139f David Mosberger 2014-04-28  691  
> SCHED_PASS_NON_PERIODIC)
> 2d53139f David Mosberger 2014-04-28  692  
> continue;
> 2d53139f David Mosberger 2014-04-28  693  break;
> 2d53139f David Mosberger 2014-04-28  694  }
> 2d53139f David Mosberger 2014-04-28  695
> 2d53139f David Mosberger 2014-04-28  696  if 
> (list_empty(&ep->urb_list))
> 2d53139f David Mosberger 2014-04-28  697  
> continue;   /* nothing to do */
> 2d53139f David Mosberger 2014-04-28  698  urb = 
> list_first_entry(&ep->urb_list, struct urb,
> 2d53139f David Mosberger 2014-04-28  699  
>urb_list);
> 2d53139f David Mosberger 2014-04-28  700  if 
> (urb->unlinked) {
> 2d53139f David Mosberger 2014-04-28  701  
> dev_dbg(&spi->dev, "%s: URB %p unlinked=%d",
> 2d53139f David Mosberger 2014-04-28  702  
> __func__, urb, urb->unlinked);
> 2d53139f David Mosberger 2014-04-28  703  
> max3421_hcd->curr_urb = urb;
> 2d53139f David Mosberger 2014-04-28  704  
> max3421_hcd->urb_done = 1;
> 2d53139f David Mosberger 2014-04-28  705  
> spin_unlock_irqrestore(&max3421_hcd->lock,
> 2d53139f David Mosberger 2014-04-28  706  
>flags);
> 2d53139f David Mosberger 2014-04-28  707  return 
> 1;
> 2d53139f David Mosberger 2014-04-28  708  }
> 2d53139f David Mosberger 2014-04-28  709
> 2d53139f David Mosberger 2014-04-28  710  switch 
> (usb_endpoint_type(&ep->desc)) {
> 2d53139f David Mosberger 2014-04-28  

Re: [PATCH 5/9] usb: xhci: use list_for_each_entry

2015-12-18 Thread Julia Lawall
Geliang,

Please check whether it is acceptable that last_unlinked_td point to the
dummy entry at th beginning of the list, in the case where the
list_for_each_entry loop runs out normally.

It seems that you have sent a bunch of these patches.  Please recheck them
all to see if they really follow the semantics of list_for_each_entry
properly.  If particular, you should not normally use the index variable
after leaving the loop, unless it is guaranteed that the exit from the
loop was via a break.

julia

On Sat, 19 Dec 2015, kbuild test robot wrote:

> CC: kbuild-...@01.org
> In-Reply-To: 
> <0fbea26fdbcb76e24188fc0800d425f927f45b6f.1450455485.git.geliangt...@163.com>
> TO: Geliang Tang 
> CC: Mathias Nyman , Greg Kroah-Hartman 
> 
> CC: Geliang Tang , linux-usb@vger.kernel.org, 
> linux-ker...@vger.kernel.org
>
> Hi Geliang,
>
> [auto build test WARNING on usb/usb-testing]
> [also build test WARNING on v4.4-rc5 next-20151218]
>
> url:
> https://github.com/0day-ci/linux/commits/Geliang-Tang/usb-host-fotg210-use-list_for_each_entry_safe/20151219-003955
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git 
> usb-testing
> :: branch date: 2 hours ago
> :: commit date: 2 hours ago
>
> >> drivers/usb/host/xhci-ring.c:714:20-26: ERROR: invalid reference to the 
> >> index variable of the iterator on line 672
>
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 0af06cc5bab47032f7fc8e2e6a3df0fb29513b52
> vim +714 drivers/usb/host/xhci-ring.c
>
> ae6367471 Sarah Sharp  2009-04-29  666
> ae6367471 Sarah Sharp  2009-04-29  667/* Fix up the ep ring first, so 
> HW stops executing cancelled TDs.
> ae6367471 Sarah Sharp  2009-04-29  668 * We have the xHCI lock, so 
> nothing can modify this list until we drop
> ae6367471 Sarah Sharp  2009-04-29  669 * it.  We're also in the event 
> handler, so we can't get re-interrupted
> ae6367471 Sarah Sharp  2009-04-29  670 * if another Stop Endpoint 
> command completes
> ae6367471 Sarah Sharp  2009-04-29  671 */
> 0af06cc5b Geliang Tang 2015-12-19 @672list_for_each_entry(cur_td, 
> &ep->cancelled_td_list, cancelled_td_list) {
> aa50b2906 Xenia Ragiadakou 2013-08-14  673xhci_dbg_trace(xhci, 
> trace_xhci_dbg_cancel_urb,
> aa50b2906 Xenia Ragiadakou 2013-08-14  674
> "Removing canceled TD starting at 0x%llx (dma).",
> 79688acfb Sarah Sharp  2011-12-19  675
> (unsigned long long)xhci_trb_virt_to_dma(
> 79688acfb Sarah Sharp  2011-12-19  676
> cur_td->start_seg, cur_td->first_trb));
> e9df17eb1 Sarah Sharp  2010-04-02  677ep_ring = 
> xhci_urb_to_transfer_ring(xhci, cur_td->urb);
> e9df17eb1 Sarah Sharp  2010-04-02  678if (!ep_ring) {
> e9df17eb1 Sarah Sharp  2010-04-02  679/* This 
> shouldn't happen unless a driver is mucking
> e9df17eb1 Sarah Sharp  2010-04-02  680 * with the 
> stream ID after submission.  This will
> e9df17eb1 Sarah Sharp  2010-04-02  681 * leave the TD 
> on the hardware ring, and the hardware
> e9df17eb1 Sarah Sharp  2010-04-02  682 * will try to 
> execute it, and may access a buffer
> e9df17eb1 Sarah Sharp  2010-04-02  683 * that has 
> already been freed.  In the best case, the
> e9df17eb1 Sarah Sharp  2010-04-02  684 * hardware 
> will execute it, and the event handler will
> e9df17eb1 Sarah Sharp  2010-04-02  685 * ignore the 
> completion event for that TD, since it was
> e9df17eb1 Sarah Sharp  2010-04-02  686 * removed from 
> the td_list for that endpoint.  In
> e9df17eb1 Sarah Sharp  2010-04-02  687 * short, don't 
> muck with the stream ID after
> e9df17eb1 Sarah Sharp  2010-04-02  688 * submission.
> e9df17eb1 Sarah Sharp  2010-04-02  689 */
> e9df17eb1 Sarah Sharp  2010-04-02  690xhci_warn(xhci, 
> "WARN Cancelled URB %p "
> e9df17eb1 Sarah Sharp  2010-04-02  691
> "has invalid stream ID %u.\n",
> e9df17eb1 Sarah Sharp  2010-04-02  692
> cur_td->urb,
> e9df17eb1 Sarah Sharp  2010-04-02  693
> cur_td->urb->stream_id);
> e9df17eb1 Sarah Sharp  2010-04-02  694goto 
> remove_finished_td;
> e9df17eb1 Sarah Sharp  2010-04-02  695}
> ae6367471 Sarah Sharp  2009-04-29  696/*
> ae6367471 Sarah Sharp  2009-04-29  697 * If we stopped on the 
> TD we need to cancel, then we have to
> ae6367471 Sarah Sharp  2009-04-29  698 * move the xHC 
> endpoint ring dequeue pointer past this TD.
> ae6367

[PATCH] usb: gadget: rndis: fix itnull.cocci warnings

2015-12-18 Thread Julia Lawall
The index variable of list_for_each_entry_safe is never NULL.

Generated by: scripts/coccinelle/iterators/itnull.cocci

CC: Geliang Tang 
Signed-off-by: Fengguang Wu 
Signed-off-by: Julia Lawall 
---

 rndis.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/usb/gadget/function/rndis.c
+++ b/drivers/usb/gadget/function/rndis.c
@@ -1009,7 +1009,7 @@ void rndis_free_response(struct rndis_pa
rndis_resp_t *r, *n;
 
list_for_each_entry_safe(r, n, ¶ms->resp_queue, list) {
-   if (r && r->buf == buf) {
+   if (r->buf == buf) {
list_del(&r->list);
kfree(r);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 7/9] usb: dwc2: host: use list_for_each_entry_safe

2015-12-18 Thread Julia Lawall
The call on line 1120 looks suspicious, because qtd could be non-null but 
also not a valid element, if the loop has exited normally.

julia

On Sat, 19 Dec 2015, kbuild test robot wrote:

> CC: kbuild-...@01.org
> In-Reply-To: 
> 
> TO: Geliang Tang 
> CC: John Youn , Greg Kroah-Hartman 
> 
> CC: Geliang Tang , linux-usb@vger.kernel.org, 
> linux-ker...@vger.kernel.org
> 
> Hi Geliang,
> 
> [auto build test WARNING on usb/usb-testing]
> [also build test WARNING on v4.4-rc5 next-20151218]
> 
> url:
> https://github.com/0day-ci/linux/commits/Geliang-Tang/usb-host-fotg210-use-list_for_each_entry_safe/20151219-003955
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git 
> usb-testing
> :: branch date: 2 hours ago
> :: commit date: 2 hours ago
> 
> >> drivers/usb/dwc2/hcd_ddma.c:1119:11-14: ERROR: invalid reference to the 
> >> index variable of the iterator on line 1096
> 
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 61366173f3eb9bee0d5cd22e6a7c5407b14087cb
> vim +1119 drivers/usb/dwc2/hcd_ddma.c
> 
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1090  
> if (chan->halt_status == DWC2_HC_XFER_URB_DEQUEUE) {
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1091  
> list_for_each_entry(qtd, &qh->qtd_list, qtd_list_entry)
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1092  
> qtd->in_process = 0;
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1093  
> return;
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1094  
> }
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1095  
> 61366173 drivers/usb/dwc2/hcd_ddma.c Geliang Tang   2015-12-19 @1096  
> list_for_each_entry_safe(qtd, tmp, &qh->qtd_list, qtd_list_entry) {
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1097  
> int i;
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1098  
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1099  
> xfer_done = 0;
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1100  
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1101  
> for (i = 0; i < qtd->n_desc; i++) {
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1102  
> if (dwc2_process_non_isoc_desc(hsotg, chan, chnum, qtd,
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1103  
>desc_num, halt_status,
> fbd1cd20 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-11-22  1104  
>&xfer_done)) {
> fbd1cd20 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-11-22  1105  
> qtd = NULL;
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1106  
> break;
> fbd1cd20 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-11-22  1107  
> }
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1108  
> desc_num++;
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1109  
> }
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1110  
> }
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11    
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1112  
> if (qh->ep_type != USB_ENDPOINT_XFER_CONTROL) {
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1113  
> /*
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1114  
>  * Resetting the data toggle for bulk and interrupt endpoints
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1115  
>  * in case of stall. See handle_hc_stall_intr().
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1116  
>  */
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1117  
> if (halt_status == DWC2_HC_XFER_STALL)
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1118  
> qh->data_toggle = DWC2_HC_PID_DATA0;
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11 @1119  
> else if (qtd)
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1120  
> dwc2_hcd_save_data_toggle(hsotg, chan, chnum, qtd);
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1121  
> }
> dc4c76e7 drivers/staging/dwc2/hcd_ddma.c Paul Zimmerman 2013-03-11  1122  
> 
> :: The code at line 1119 was first introduced by commit
> :: dc4c76e7b22cdcc1ba71ff87edee55f464e01658 staging: HCD descriptor 

Re: [PATCH] usb: gadget: rndis: fix itnull.cocci warnings

2015-12-22 Thread Julia Lawall


On Tue, 22 Dec 2015, Felipe Balbi wrote:

> Julia Lawall  writes:
>
> > The index variable of list_for_each_entry_safe is never NULL.
> >
> > Generated by: scripts/coccinelle/iterators/itnull.cocci
> >
> > CC: Geliang Tang 
> > Signed-off-by: Fengguang Wu 
> > Signed-off-by: Julia Lawall 
>
> doesn't apply. Does this depend on anything ?

It may be derived from a patch posted to a mailing list.  I don't have the
original reference any more.

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


[PATCH 4/7] usb: gadget: legacy: fix error return code

2015-12-26 Thread Julia Lawall
Return a negative error code on failure.

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

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

Signed-off-by: Julia Lawall 

---
 drivers/usb/gadget/legacy/acm_ms.c   |4 +++-
 drivers/usb/gadget/legacy/audio.c|4 +++-
 drivers/usb/gadget/legacy/cdc2.c |4 +++-
 drivers/usb/gadget/legacy/ether.c|4 +++-
 drivers/usb/gadget/legacy/hid.c  |4 +++-
 drivers/usb/gadget/legacy/mass_storage.c |4 +++-
 drivers/usb/gadget/legacy/ncm.c  |4 +++-
 7 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/legacy/acm_ms.c 
b/drivers/usb/gadget/legacy/acm_ms.c
index c16089e..2679264 100644
--- a/drivers/usb/gadget/legacy/acm_ms.c
+++ b/drivers/usb/gadget/legacy/acm_ms.c
@@ -211,8 +211,10 @@ static int acm_ms_bind(struct usb_composite_dev *cdev)
struct usb_descriptor_header *usb_desc;
 
usb_desc = usb_otg_descriptor_alloc(gadget);
-   if (!usb_desc)
+   if (!usb_desc) {
+   status = -ENOMEM;
goto fail_string_ids;
+   }
usb_otg_descriptor_init(gadget, usb_desc);
otg_desc[0] = usb_desc;
otg_desc[1] = NULL;
diff --git a/drivers/usb/gadget/legacy/audio.c 
b/drivers/usb/gadget/legacy/audio.c
index 5d7b3c6..f61936b 100644
--- a/drivers/usb/gadget/legacy/audio.c
+++ b/drivers/usb/gadget/legacy/audio.c
@@ -249,8 +249,10 @@ static int audio_bind(struct usb_composite_dev *cdev)
struct usb_descriptor_header *usb_desc;
 
usb_desc = usb_otg_descriptor_alloc(cdev->gadget);
-   if (!usb_desc)
+   if (!usb_desc) {
+   status = -ENOMEM;
goto fail;
+   }
usb_otg_descriptor_init(cdev->gadget, usb_desc);
otg_desc[0] = usb_desc;
otg_desc[1] = NULL;
diff --git a/drivers/usb/gadget/legacy/cdc2.c b/drivers/usb/gadget/legacy/cdc2.c
index 51c0868..94261a1 100644
--- a/drivers/usb/gadget/legacy/cdc2.c
+++ b/drivers/usb/gadget/legacy/cdc2.c
@@ -183,8 +183,10 @@ static int cdc_bind(struct usb_composite_dev *cdev)
struct usb_descriptor_header *usb_desc;
 
usb_desc = usb_otg_descriptor_alloc(gadget);
-   if (!usb_desc)
+   if (!usb_desc) {
+   status = -ENOMEM;
goto fail1;
+   }
usb_otg_descriptor_init(gadget, usb_desc);
otg_desc[0] = usb_desc;
otg_desc[1] = NULL;
diff --git a/drivers/usb/gadget/legacy/ether.c 
b/drivers/usb/gadget/legacy/ether.c
index 25a2c2e..3396e71 100644
--- a/drivers/usb/gadget/legacy/ether.c
+++ b/drivers/usb/gadget/legacy/ether.c
@@ -407,8 +407,10 @@ static int eth_bind(struct usb_composite_dev *cdev)
struct usb_descriptor_header *usb_desc;
 
usb_desc = usb_otg_descriptor_alloc(gadget);
-   if (!usb_desc)
+   if (!usb_desc) {
+   status = -ENOMEM;
goto fail1;
+   }
usb_otg_descriptor_init(gadget, usb_desc);
otg_desc[0] = usb_desc;
otg_desc[1] = NULL;
diff --git a/drivers/usb/gadget/legacy/hid.c b/drivers/usb/gadget/legacy/hid.c
index a71a884..cccbb94 100644
--- a/drivers/usb/gadget/legacy/hid.c
+++ b/drivers/usb/gadget/legacy/hid.c
@@ -175,8 +175,10 @@ static int hid_bind(struct usb_composite_dev *cdev)
struct usb_descriptor_header *usb_desc;
 
usb_desc = usb_otg_descriptor_alloc(gadget);
-   if (!usb_desc)
+   if (!usb_desc) {
+   status = -ENOMEM;
goto put;
+   }
usb_otg_descriptor_init(gadget, usb_desc);
otg_desc[0] = usb_desc;
otg_desc[1] = NULL;
diff --git a/drivers/usb/gadget/legacy/mass_storage.c 
b/drivers/usb/gadget/legacy/mass_storage.c
index e61af53..93f2b8f 100644
--- a/drivers/usb/gadget/legacy/mass_storage.c
+++ b/drivers/usb/gadget/legacy/mass_storage.c
@@ -200,8 +200,10 @@ static int msg_bind(struct usb_composite_dev *cdev)
struct usb_descriptor_header *usb_desc;
 
usb_desc = usb_otg_descriptor_alloc(cdev->gadget);
-   if (!usb_desc)
+   if (!usb_desc) {
+   status = -ENOMEM;
goto fail_string_ids;
+   }
usb_otg_descriptor_init(cdev->gadget, usb_desc);
otg_desc[0] = usb_desc;
otg_desc[1] =

[PATCH 0/7] fix error return code

2015-12-26 Thread Julia Lawall
The complate semantic patch that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

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

// identify a function that returns a negative return value at least once.
f(...) {
... when any
(
return -c@i;
|
ret = -c@i;
... when != ret = e
return ret;
|
if (ret < 0) { ... return ret; }
)
... when any
}

@r exists@
identifier ret,ok.f,fn;
expression e1,e2,e3,e4,e5,e6,x;
statement S,S1;
position p1,p2,p3;
@@

// identify a case where the return variable is set to a non-negative value
// and then returned in error-handling code
f(...) {
... when any
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != \(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\)
when != &ret
when any
(
 if (<+... ret = e5 ...+>) S1
|
 if (<+... &ret ...+>) S1
|
if@p2(<+...x = fn(...)...+>)
 {
  ... when != ret = e6
  when forall
 return@p3 ret;
}
|
break;
|
x = fn(...)
... when != \(ret = e4\|ret++\|ret--\|ret+=e4\|ret-=e4\)
when != &ret
(
 if (<+... ret = e3 ...+>) S
|
 if (<+... &ret ...+>) S
|
if@p2(<+...\(x != 0\|x < 0\|x == NULL\|IS_ERR(x)\)...+>)
 {
  ... when != ret = e2
  when forall
 return@p3 ret;
}
)
)
... when any
}

@printer depends on r@
position p;
identifier ok.f,pr;
constant char [] c;
@@

f(...) { <...pr@p(...,c,...)...> }

@bad0 exists@
identifier r.ret,ok.f,g != {ERR_PTR,IS_ERR};
position p != printer.p;
@@

f(...) { ... when any
g@p(...,ret,...)
... when any
 }

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

// ignore the above if there is some path where the variable is set to
// something else
(
if@p1 (\(ret < 0\|ret != 0\)) S1
|
ret@p1 = 0
)
... when any
 \(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\|&ret\)
... when any
if@p2(...) S2

@bad1 depends on !bad0 && !bad exists@
position r.p2;
statement S2;
identifier r.ret;
expression e1;
constant c;
@@

ret = -c
... when != \(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\)
when != &ret
when any
if@p2(...) S2

@bad2 depends on !bad0 && !bad && !bad1 exists@
position r.p1,r.p2;
identifier r.ret;
expression e1;
statement S2;
constant c;
@@

// likewise ignore it if there has been an intervening return
ret@p1 = 0
... when != if (...) { ... ret = e1 ... return ret; }
when != if (...) { ... return -c; }
when any
if@p2(...) S2

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

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

---

 drivers/block/rsxx/core.c |1 
 drivers/block/umem.c  |5 +++-
 drivers/cdrom/gdrom.c |8 +-
 drivers/net/ethernet/ti/cpsw.c|8 +-
 drivers/soc/ti/knav_qmss_queue.c  |1 
 drivers/soc/ti/wkup_m3_ipc.c  |1 
 drivers/usb/gadget/legacy/acm_ms.c|4 ++-
 drivers/usb/gadget/legacy/audio.c |4 ++-
 drivers/usb/gadget/legacy/cdc2.c  |4 ++-
 drivers/usb/gadget/legacy/ether.c |4 ++-
 drivers/usb/gadget/legacy/hid.c   |4 ++-
 drivers/usb/gadget/legacy/mass_storage.c  |4 ++-
 drivers/usb/gadget/legacy/ncm.c   |4 ++-
 drivers/video/fbdev/omap2/omapfb/displays/encoder-tpd12s015.c |   12 +++---
 14 files changed, 49 insertions(+), 15 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/2] remove assignment from IS_ERR argument

2015-12-26 Thread Julia Lawall
The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// 
@@
expression e1,e2;
statement S1,S2;
@@

+e1 = e2;
if (IS_ERR(
e1
-   = e2
   )) S1 else S2
// 

---

 drivers/usb/mon/mon_text.c |6 --
 ipc/mqueue.c   |3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] USB: usbmon: remove assignment from IS_ERR argument

2015-12-26 Thread Julia Lawall
The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// 
@@
expression e1,e2;
statement S1,S2;
@@

+e1 = e2;
if (IS_ERR(
e1
-   = e2
   )) S1 else S2
// 

Signed-off-by: Julia Lawall 

---
 drivers/usb/mon/mon_text.c |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/mon/mon_text.c b/drivers/usb/mon/mon_text.c
index 98e4f63..e59334b 100644
--- a/drivers/usb/mon/mon_text.c
+++ b/drivers/usb/mon/mon_text.c
@@ -387,7 +387,8 @@ static ssize_t mon_text_read_t(struct file *file, char 
__user *buf,
struct mon_event_text *ep;
struct mon_text_ptr ptr;
 
-   if (IS_ERR(ep = mon_text_read_wait(rp, file)))
+   ep = mon_text_read_wait(rp, file);
+   if (IS_ERR(ep))
return PTR_ERR(ep);
mutex_lock(&rp->printf_lock);
ptr.cnt = 0;
@@ -414,7 +415,8 @@ static ssize_t mon_text_read_u(struct file *file, char 
__user *buf,
struct mon_event_text *ep;
struct mon_text_ptr ptr;
 
-   if (IS_ERR(ep = mon_text_read_wait(rp, file)))
+   ep = mon_text_read_wait(rp, file);
+   if (IS_ERR(ep))
return PTR_ERR(ep);
mutex_lock(&rp->printf_lock);
ptr.cnt = 0;

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


[PATCH] usb: renesas_usbhs: constify usbhs_pkt_handle structures

2015-12-27 Thread Julia Lawall
The usbhs_pkt_handle structures are never modified, so declare them as
const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/usb/renesas_usbhs/fifo.c |   20 ++--
 drivers/usb/renesas_usbhs/fifo.h |   20 ++--
 drivers/usb/renesas_usbhs/pipe.h |2 +-
 3 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
index c0f5c65..b4de70e 100644
--- a/drivers/usb/renesas_usbhs/fifo.c
+++ b/drivers/usb/renesas_usbhs/fifo.c
@@ -46,7 +46,7 @@ static int usbhsf_null_handle(struct usbhs_pkt *pkt, int 
*is_done)
return -EINVAL;
 }
 
-static struct usbhs_pkt_handle usbhsf_null_handler = {
+static const struct usbhs_pkt_handle usbhsf_null_handler = {
.prepare = usbhsf_null_handle,
.try_run = usbhsf_null_handle,
 };
@@ -422,12 +422,12 @@ static int usbhs_dcp_dir_switch_done(struct usbhs_pkt 
*pkt, int *is_done)
return 0;
 }
 
-struct usbhs_pkt_handle usbhs_dcp_status_stage_in_handler = {
+const struct usbhs_pkt_handle usbhs_dcp_status_stage_in_handler = {
.prepare = usbhs_dcp_dir_switch_to_write,
.try_run = usbhs_dcp_dir_switch_done,
 };
 
-struct usbhs_pkt_handle usbhs_dcp_status_stage_out_handler = {
+const struct usbhs_pkt_handle usbhs_dcp_status_stage_out_handler = {
.prepare = usbhs_dcp_dir_switch_to_read,
.try_run = usbhs_dcp_dir_switch_done,
 };
@@ -449,7 +449,7 @@ static int usbhsf_dcp_data_stage_try_push(struct usbhs_pkt 
*pkt, int *is_done)
return pkt->handler->prepare(pkt, is_done);
 }
 
-struct usbhs_pkt_handle usbhs_dcp_data_stage_out_handler = {
+const struct usbhs_pkt_handle usbhs_dcp_data_stage_out_handler = {
.prepare = usbhsf_dcp_data_stage_try_push,
 };
 
@@ -488,7 +488,7 @@ static int usbhsf_dcp_data_stage_prepare_pop(struct 
usbhs_pkt *pkt,
return pkt->handler->prepare(pkt, is_done);
 }
 
-struct usbhs_pkt_handle usbhs_dcp_data_stage_in_handler = {
+const struct usbhs_pkt_handle usbhs_dcp_data_stage_in_handler = {
.prepare = usbhsf_dcp_data_stage_prepare_pop,
 };
 
@@ -600,7 +600,7 @@ static int usbhsf_pio_prepare_push(struct usbhs_pkt *pkt, 
int *is_done)
return usbhsf_pio_try_push(pkt, is_done);
 }
 
-struct usbhs_pkt_handle usbhs_fifo_pio_push_handler = {
+const struct usbhs_pkt_handle usbhs_fifo_pio_push_handler = {
.prepare = usbhsf_pio_prepare_push,
.try_run = usbhsf_pio_try_push,
 };
@@ -730,7 +730,7 @@ usbhs_fifo_read_busy:
return ret;
 }
 
-struct usbhs_pkt_handle usbhs_fifo_pio_pop_handler = {
+const struct usbhs_pkt_handle usbhs_fifo_pio_pop_handler = {
.prepare = usbhsf_prepare_pop,
.try_run = usbhsf_pio_try_pop,
 };
@@ -747,7 +747,7 @@ static int usbhsf_ctrl_stage_end(struct usbhs_pkt *pkt, int 
*is_done)
return 0;
 }
 
-struct usbhs_pkt_handle usbhs_ctrl_stage_end_handler = {
+const struct usbhs_pkt_handle usbhs_ctrl_stage_end_handler = {
.prepare = usbhsf_ctrl_stage_end,
.try_run = usbhsf_ctrl_stage_end,
 };
@@ -934,7 +934,7 @@ static int usbhsf_dma_push_done(struct usbhs_pkt *pkt, int 
*is_done)
return 0;
 }
 
-struct usbhs_pkt_handle usbhs_fifo_dma_push_handler = {
+const struct usbhs_pkt_handle usbhs_fifo_dma_push_handler = {
.prepare= usbhsf_dma_prepare_push,
.dma_done   = usbhsf_dma_push_done,
 };
@@ -1182,7 +1182,7 @@ static int usbhsf_dma_pop_done(struct usbhs_pkt *pkt, int 
*is_done)
return usbhsf_dma_pop_done_with_rx_irq(pkt, is_done);
 }
 
-struct usbhs_pkt_handle usbhs_fifo_dma_pop_handler = {
+const struct usbhs_pkt_handle usbhs_fifo_dma_pop_handler = {
.prepare= usbhsf_dma_prepare_pop,
.try_run= usbhsf_dma_try_pop,
.dma_done   = usbhsf_dma_pop_done
diff --git a/drivers/usb/renesas_usbhs/fifo.h b/drivers/usb/renesas_usbhs/fifo.h
index c7d9b86..8b98507 100644
--- a/drivers/usb/renesas_usbhs/fifo.h
+++ b/drivers/usb/renesas_usbhs/fifo.h
@@ -54,7 +54,7 @@ struct usbhs_pkt_handle;
 struct usbhs_pkt {
struct list_head node;
struct usbhs_pipe *pipe;
-   struct usbhs_pkt_handle *handler;
+   const struct usbhs_pkt_handle *handler;
void (*done)(struct usbhs_priv *priv,
 struct usbhs_pkt *pkt);
struct work_struct work;
@@ -86,18 +86,18 @@ void usbhs_fifo_clear_dcp(struct usbhs_pipe *pipe);
 /*
  * packet info
  */
-extern struct usbhs_pkt_handle usbhs_fifo_pio_push_handler;
-extern struct usbhs_pkt_handle usbhs_fifo_pio_pop_handler;
-extern struct usbhs_pkt_handle usbhs_ctrl_stage_end_handler;
+extern const struct usbhs_pkt_handle usbhs_fifo_pio_push_handler;
+extern const struct usbhs_pkt_handle usbhs_fifo_pio_pop_handler;
+extern const struct usbhs_pkt_handle usbhs_ctrl_stage_end_handler;
 
-extern struct usbhs_pkt_handle usbhs_fifo_dma_push_handler;
-extern struct usbhs_pkt_handle usbhs_fifo_dma_pop_handler;
+

[PATCH] usbip: fix odd_ptr_err.cocci warnings

2015-12-30 Thread Julia Lawall
PTR_ERR should access the value just tested by IS_ERR

Generated by: scripts/coccinelle/tests/odd_ptr_err.cocci

CC: Nobuo Iwata 
Signed-off-by: Fengguang Wu 
Signed-off-by: Julia Lawall 
---

I haven't checked the complete context, but the code looks suspicious.

 usbip_ux.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/usb/usbip/usbip_ux.c
+++ b/drivers/usb/usbip/usbip_ux.c
@@ -571,7 +571,7 @@ static int __init usbip_ux_init(void)
usbip_ux_dev = device_create(usbip_ux_class, NULL, usbip_ux_devno, 
NULL, USBIP_UX_DEV_NAME);
if (IS_ERR(usbip_ux_dev)) {
pr_err("Fail to create sysfs entry for %s\n", 
USBIP_UX_DEV_NAME);
-   ret = PTR_ERR(usbip_ux_class);
+   ret = PTR_ERR(usbip_ux_dev);
goto err_cdev_del;
}
return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch v2] usb: gadget: f_midi: missing unlock on error path

2016-01-05 Thread Julia Lawall


On Tue, 5 Jan 2016, kbuild test robot wrote:

> Hi Dan,
>
> [auto build test WARNING on balbi-usb/next]
> [also build test WARNING on v4.4-rc8 next-20160105]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improving the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Dan-Carpenter/usb-gadget-f_midi-missing-unlock-on-error-path/20160105-183115
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
>
>
> coccinelle warnings: (new ones prefixed by >>)
>
> >> drivers/usb/gadget/function/f_midi.c:1233:14-21: ERROR: midi is NULL but 
> >> dereferenced.

It's a false positive for coccinelle, but I wonder if avoiding duplicating
the mutex_lock is really worth it?  There is a slightly subtle interaction
between the possibility of midi being NULL and the value of i that make it
all work.

julia


>
> vim +1233 drivers/usb/gadget/function/f_midi.c
>
> e1e3d7ec Felipe F. Tonello 2015-12-01  1217
> 6f1de344 Andrzej Pietrasiewicz 2014-10-16  1218   ++opts->refcnt;
> 6f1de344 Andrzej Pietrasiewicz 2014-10-16  1219   
> mutex_unlock(&opts->lock);
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1220
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1221   midi->func.name 
> = "gmidi function";
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1222   midi->func.bind 
> = f_midi_bind;
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1223   midi->func.unbind   
> = f_midi_unbind;
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1224   midi->func.set_alt  
> = f_midi_set_alt;
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1225   midi->func.disable  
> = f_midi_disable;
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1226   midi->func.free_func
> = f_midi_free;
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1227
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1228   return &midi->func;
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1229
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1230  setup_fail:
> 39920a18 Dan Carpenter 2016-01-05  1231   
> mutex_unlock(&opts->lock);
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1232   for (--i; i >= 0; i--)
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16 @1233   
> kfree(midi->in_port[i]);
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1234   kfree(midi);
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1235   return ERR_PTR(status);
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1236  }
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1237
> b85e9de9 Andrzej Pietrasiewicz 2014-10-16  1238  
> DECLARE_USB_FUNCTION_INIT(midi, f_midi_alloc_inst, f_midi_alloc);
>
> :: The code at line 1233 was first introduced by commit
> :: b85e9de9e818de0dcbc50b7b4242192eb6194855 usb: gadget: f_midi: convert 
> to new function interface with backward compatibility
>
> :: TO: Andrzej Pietrasiewicz 
> :: CC: Felipe Balbi 
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
> --
> 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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch v2] usb: gadget: f_midi: missing unlock on error path

2016-01-05 Thread Julia Lawall


On Tue, 5 Jan 2016, Felipe Ferreri Tonello wrote:

> Hi Dan,
> 
> On 05/01/16 12:44, Dan Carpenter wrote:
> > On Tue, Jan 05, 2016 at 01:28:11PM +0100, Julia Lawall wrote:
> >>
> >>
> >> On Tue, 5 Jan 2016, kbuild test robot wrote:
> >>
> >>> Hi Dan,
> >>>
> >>> [auto build test WARNING on balbi-usb/next]
> >>> [also build test WARNING on v4.4-rc8 next-20160105]
> >>> [if your patch is applied to the wrong git tree, please drop us a note to 
> >>> help improving the system]
> >>>
> >>> url:
> >>> https://github.com/0day-ci/linux/commits/Dan-Carpenter/usb-gadget-f_midi-missing-unlock-on-error-path/20160105-183115
> >>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
> >>>
> >>>
> >>> coccinelle warnings: (new ones prefixed by >>)
> >>>
> >>>>> drivers/usb/gadget/function/f_midi.c:1233:14-21: ERROR: midi is NULL 
> >>>>> but dereferenced.
> >>
> >> It's a false positive for coccinelle, but I wonder if avoiding duplicating
> >> the mutex_lock is really worth it?
> > 
> > It's not the most beautiful code in the world.  I considered a bunch of
> > different ways to write it...  This is what Felipe Tonello wanted,
> > though.
> 
> This case is not a matter of been pretty but a matter of been less error
> prone.
> 
> What would you suggest?

I thought to be a little less subtle about midi, it would be easier to 
keep the first few mutex_unlocks up where they were.  But up to you.

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


[PATCH] usb: gadget: rndis: fix itnull.cocci warnings

2016-01-25 Thread Julia Lawall
The index variable of list_for_each_entry_safe is an offset from a list
pointer, and thus should not be NULL.

Generated by: scripts/coccinelle/iterators/itnull.cocci

CC: Geliang Tang 
Signed-off-by: Fengguang Wu 
Signed-off-by: Julia Lawall 
---

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
usb-testing
head:   0e781c2258ffb2a42bf44f62dea8662f38cbfd34
commit: f6281af9d62e128aa6efad29cf7265062af114f2 [27/38] usb: gadget:
rndis: use list_for_each_entry_safe

 rndis.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/usb/gadget/function/rndis.c
+++ b/drivers/usb/gadget/function/rndis.c
@@ -1009,7 +1009,7 @@ void rndis_free_response(struct rndis_pa
rndis_resp_t *r, *n;

list_for_each_entry_safe(r, n, ¶ms->resp_queue, list) {
-   if (r && r->buf == buf) {
+   if (r->buf == buf) {
list_del(&r->list);
kfree(r);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 10/12] usb: dbc: add handshake between debug target and host

2016-01-26 Thread Julia Lawall
Please check.  As far as I can see, the call to early_xdbc_read ends up at
xdbc_bulk_transfer, which return negative error codes on failure.

julia

On Tue, 26 Jan 2016, kbuild test robot wrote:

> CC: kbuild-...@01.org
> In-Reply-To: <1453781665-4714-11-git-send-email-baolu...@linux.intel.com>
> TO: Lu Baolu 
> CC: Greg Kroah-Hartman 
> CC: linux-usb@vger.kernel.org, linux-ker...@vger.kernel.org, Lu Baolu 
> 
>
> Hi Lu,
>
> [auto build test WARNING on usb/usb-testing]
> [also build test WARNING on v4.5-rc1 next-20160125]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improving the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Lu-Baolu/usb-early-add-support-for-early-printk-through-USB3-debug-port/20160126-122049
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git 
> usb-testing
> :: branch date: 5 hours ago
> :: commit date: 5 hours ago
>
> >> drivers/usb/early/xhci-dbc.c:917:6-10: WARNING: Unsigned expression 
> >> compared with zero: size > 0
>
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout cdae956781925bbc269faa7f40c931a2825be11a
> vim +917 drivers/usb/early/xhci-dbc.c
>
> 5319db53 Lu Baolu 2016-01-26  901
> 5319db53 Lu Baolu 2016-01-26  902 /* hand over the owner of host from 
> BIOS */
> 5319db53 Lu Baolu 2016-01-26  903 xdbc_bios_handoff();
> 5319db53 Lu Baolu 2016-01-26  904
> 5319db53 Lu Baolu 2016-01-26  905 ret = xdbc_setup();
> 5319db53 Lu Baolu 2016-01-26  906 if (ret < 0) {
> 5319db53 Lu Baolu 2016-01-26  907 pr_notice("failed to setup xHCI 
> DbC connection\n");
> 5319db53 Lu Baolu 2016-01-26  908 xdbcp->xhci_base = NULL;
> 5319db53 Lu Baolu 2016-01-26  909 xdbcp->xdbc_reg = NULL;
> 12cd5775 Lu Baolu 2016-01-26  910 xdbc_dump_debug_buffer();
> 5319db53 Lu Baolu 2016-01-26  911 return ret;
> 5319db53 Lu Baolu 2016-01-26  912 }
> 5319db53 Lu Baolu 2016-01-26  913
> cdae9567 Lu Baolu 2016-01-26  914 while (retry > 0) {
> cdae9567 Lu Baolu 2016-01-26  915 early_xdbc_write(NULL, ping, 
> strlen(ping));
> cdae9567 Lu Baolu 2016-01-26  916 size = early_xdbc_read(NULL, 
> pong, 64);
> cdae9567 Lu Baolu 2016-01-26 @917 if (size > 0) {
> cdae9567 Lu Baolu 2016-01-26  918 xdbc_trace("%s: pong 
> message: %s\n", __func__, pong);
> cdae9567 Lu Baolu 2016-01-26  919 if (pong[0] == 'Y' || 
> pong[0] == 'y')
> cdae9567 Lu Baolu 2016-01-26  920 break;
> cdae9567 Lu Baolu 2016-01-26  921 } else {
> cdae9567 Lu Baolu 2016-01-26  922 xdbc_trace("%s: pong 
> message error %d\n",
> cdae9567 Lu Baolu 2016-01-26  923 __func__, size);
> cdae9567 Lu Baolu 2016-01-26  924 }
> cdae9567 Lu Baolu 2016-01-26  925
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0982/1285] Replace numeric parameter like 0444 with macro

2016-08-02 Thread Julia Lawall


On Tue, 2 Aug 2016, Baole Ni wrote:

> I find that the developers often just specified the numeric value
> when calling a macro which is defined with a parameter for access permission.
> As we know, these numeric value for access permission have had the 
> corresponding macro,
> and that using macro can improve the robustness and readability of the code,
> thus, I suggest replacing the numeric parameter with the macro.
>
> Signed-off-by: Chuansheng Liu 
> Signed-off-by: Baole Ni 
> ---
>  drivers/usb/host/fotg210-hcd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
> index 66efa9a..7ad395e 100644
> --- a/drivers/usb/host/fotg210-hcd.c
> +++ b/drivers/usb/host/fotg210-hcd.c
> @@ -4789,7 +4789,7 @@ out_unlock:
>   return ret;
>  }
>
> -static DEVICE_ATTR(uframe_periodic_max, 0644, show_uframe_periodic_max,
> +static DEVICE_ATTR(uframe_periodic_max, S_IRUSR | S_IWUSR | S_IRGRP | 
> S_IROTH, show_uframe_periodic_max,

Over 80 chars.

julia

>  store_uframe_periodic_max);
>
>  static inline int create_sysfs_files(struct fotg210_hcd *fotg210)
> --
> 2.9.2
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 00/11] use of_property_read_bool

2016-08-05 Thread Julia Lawall
Use of_property_read_bool to check for the existence of a property.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// 
@@
expression e1,e2;
statement S2,S1;
@@
-   if (of_get_property(e1,e2,NULL))
+   if (of_property_read_bool(e1,e2))
S1 else S2
// 

---

 arch/powerpc/sysdev/mpic.c  |2 +-
 drivers/i2c/busses/i2c-mpc.c|2 +-
 drivers/mmc/host/sdhci-of-esdhc.c   |2 +-
 drivers/net/ethernet/freescale/xgmac_mdio.c |3 +--
 drivers/phy/phy-qcom-ufs.c  |2 +-
 drivers/pinctrl/nomadik/pinctrl-nomadik.c   |2 +-
 drivers/soc/fsl/qe/qe_tdm.c |2 +-
 drivers/soc/ti/knav_qmss_queue.c|2 +-
 drivers/tty/serial/atmel_serial.c   |8 
 drivers/usb/host/fsl-mph-dr-of.c|6 +++---
 sound/soc/codecs/ab8500-codec.c |   10 +-
 sound/soc/sh/rcar/ssi.c |2 +-
 sound/soc/soc-core.c|2 +-
 13 files changed, 22 insertions(+), 23 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 02/11] usb: host: fsl-mph-dr-of: use of_property_read_bool

2016-08-05 Thread Julia Lawall
Use of_property_read_bool to check for the existence of a property.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// 
@@
expression e1,e2;
statement S2,S1;
@@
-   if (of_get_property(e1,e2,NULL))
+   if (of_property_read_bool(e1,e2))
S1 else S2
// 

Signed-off-by: Julia Lawall 

---
 drivers/usb/host/fsl-mph-dr-of.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/fsl-mph-dr-of.c b/drivers/usb/host/fsl-mph-dr-of.c
index 1044b0f..8bbca74 100644
--- a/drivers/usb/host/fsl-mph-dr-of.c
+++ b/drivers/usb/host/fsl-mph-dr-of.c
@@ -222,11 +222,11 @@ static int fsl_usb2_mph_dr_of_probe(struct 
platform_device *ofdev)
pdata->controller_ver = usb_get_ver_info(np);
 
/* Activate Erratum by reading property in device tree */
-   if (of_get_property(np, "fsl,usb-erratum-a007792", NULL))
+   if (of_property_read_bool(np, "fsl,usb-erratum-a007792"))
pdata->has_fsl_erratum_a007792 = 1;
else
pdata->has_fsl_erratum_a007792 = 0;
-   if (of_get_property(np, "fsl,usb-erratum-a005275", NULL))
+   if (of_property_read_bool(np, "fsl,usb-erratum-a005275"))
pdata->has_fsl_erratum_a005275 = 1;
else
pdata->has_fsl_erratum_a005275 = 0;
@@ -235,7 +235,7 @@ static int fsl_usb2_mph_dr_of_probe(struct platform_device 
*ofdev)
 * Determine whether phy_clk_valid needs to be checked
 * by reading property in device tree
 */
-   if (of_get_property(np, "phy-clk-valid", NULL))
+   if (of_property_read_bool(np, "phy-clk-valid"))
pdata->check_phy_clk_valid = 1;
else
pdata->check_phy_clk_valid = 0;

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


[PATCH 02/11 v2] usb: host: fsl-mph-dr-of: use of_property_read_bool

2016-08-05 Thread Julia Lawall
Use of_property_read_bool to check for the existence of a property.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// 
@@
expression e1,e2,x;
@@
-   if (of_get_property(e1,e2,NULL))
-   x = true;
-   else
-   x = false;
+   x = of_property_read_bool(e1,e2);
// 

Signed-off-by: Julia Lawall 

---
v2: special case for propagating true and false

 drivers/usb/host/fsl-mph-dr-of.c |   18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/fsl-mph-dr-of.c b/drivers/usb/host/fsl-mph-dr-of.c
index 1044b0f..f07ccb2 100644
--- a/drivers/usb/host/fsl-mph-dr-of.c
+++ b/drivers/usb/host/fsl-mph-dr-of.c
@@ -222,23 +222,17 @@ static int fsl_usb2_mph_dr_of_probe(struct 
platform_device *ofdev)
pdata->controller_ver = usb_get_ver_info(np);

/* Activate Erratum by reading property in device tree */
-   if (of_get_property(np, "fsl,usb-erratum-a007792", NULL))
-   pdata->has_fsl_erratum_a007792 = 1;
-   else
-   pdata->has_fsl_erratum_a007792 = 0;
-   if (of_get_property(np, "fsl,usb-erratum-a005275", NULL))
-   pdata->has_fsl_erratum_a005275 = 1;
-   else
-   pdata->has_fsl_erratum_a005275 = 0;
+   pdata->has_fsl_erratum_a007792 =
+   of_property_read_bool(np, "fsl,usb-erratum-a007792");
+   pdata->has_fsl_erratum_a005275 =
+   of_property_read_bool(np, "fsl,usb-erratum-a005275");

/*
 * Determine whether phy_clk_valid needs to be checked
 * by reading property in device tree
 */
-   if (of_get_property(np, "phy-clk-valid", NULL))
-   pdata->check_phy_clk_valid = 1;
-   else
-   pdata->check_phy_clk_valid = 0;
+   pdata->check_phy_clk_valid =
+   of_property_read_bool(np, "phy-clk-valid");

if (pdata->have_sysif_regs) {
if (pdata->controller_ver == FSL_USB_VER_NONE) {

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


Re: [PATCH 10/11] soc: ti: knav_qmss_queue: use of_property_read_bool

2016-08-05 Thread Julia Lawall


On Fri, 5 Aug 2016, Robin Murphy wrote:

> Hi Julia,
>
> On 05/08/16 09:56, Julia Lawall wrote:
> > Use of_property_read_bool to check for the existence of a property.
>
> This caught my eye since Rob told me off for doing the same recently[1].
>
> > The semantic patch that makes this change is as follows:
> > (http://coccinelle.lip6.fr/)
> >
> > // 
> > @@
> > expression e1,e2;
> > statement S2,S1;
> > @@
> > -   if (of_get_property(e1,e2,NULL))
> > +   if (of_property_read_bool(e1,e2))
> > S1 else S2
> > // 
> >
> > Signed-off-by: Julia Lawall 
> >
> > ---
> >  drivers/soc/ti/knav_qmss_queue.c |2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/ti/knav_qmss_queue.c 
> > b/drivers/soc/ti/knav_qmss_queue.c
> > index b73e353..56b5d7c 100644
> > --- a/drivers/soc/ti/knav_qmss_queue.c
> > +++ b/drivers/soc/ti/knav_qmss_queue.c
> > @@ -1240,7 +1240,7 @@ static int knav_setup_queue_range(struct knav_device 
> > *kdev,
> > if (of_get_property(node, "qalloc-by-id", NULL))
>
> According to the binding, "qalloc-by-id" _is_ a boolean property, so
> this one really does deserve to be of_property_read_bool()...
>
> > range->flags |= RANGE_RESERVED;
> >
> > -   if (of_get_property(node, "accumulator", NULL)) {
> > +   if (of_property_read_bool(node, "accumulator")) {
>
> ...whereas "accumulator" must have a value, so this isn't technically
> appropriate. In general, most of these "if the property exists, read the
> property and do stuff" checks are probably a sign of code that could be
> simplified by refactoring the "do stuff" step to just specifically
> handle the "read the property" step returning -EINVAL when it's not present.

Thanks for the very helpful feedback.  I will rethink the patch set in
light of this information.

julia

> Robin.
>
> [1]:https://www.mail-archive.com/iommu@lists.linux-foundation.org/msg13375.html
>
> > ret = knav_init_acc_range(kdev, node, range);
> > if (ret < 0) {
> > devm_kfree(dev, range);
> >
> >
> > ___
> > linux-arm-kernel mailing list
> > linux-arm-ker...@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
>
> --
> 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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/5] r8152: constify ethtool_ops structures

2016-08-31 Thread Julia Lawall
Check for ethtool_ops structures that are only stored in the ethtool_ops
field of a net_device structure or passed as the second argument to
netdev_set_default_ethtool_ops.  These contexts are declared const, so
ethtool_ops structures that have these properties can be declared as const
also.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct ethtool_ops i@p = { ... };

@ok1@
identifier r.i;
struct net_device e;
position p;
@@
e.ethtool_ops = &i@p;

@ok2@
identifier r.i;
expression e;
position p;
@@
netdev_set_default_ethtool_ops(e, &i@p)

@bad@
position p != {r.p,ok1.p,ok2.p};
identifier r.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct ethtool_ops i = { ... };
// 

Suggested-by: Stephen Hemminger 

Signed-off-by: Julia Lawall 

---
 drivers/net/usb/r8152.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f41a8ad..f72f807 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -4032,7 +4032,7 @@ static int rtl8152_set_coalesce(struct net_device *netdev,
return ret;
 }
 
-static struct ethtool_ops ops = {
+static const struct ethtool_ops ops = {
.get_drvinfo = rtl8152_get_drvinfo,
.get_settings = rtl8152_get_settings,
.set_settings = rtl8152_set_settings,

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


[PATCH 0/5] constify ethtool_ops structures

2016-08-31 Thread Julia Lawall
Constify ethtool_ops structures.

---

 drivers/net/ethernet/mediatek/mtk_eth_soc.c   |2 +-
 drivers/net/ethernet/synopsys/dwc_eth_qos.c   |2 +-
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c |2 +-
 drivers/net/usb/r8152.c   |2 +-
 drivers/staging/netlogic/xlr_net.c|2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] USB: ftdi_sio: constify ftdi_sio_quirk structures

2016-04-09 Thread Julia Lawall
The ftdi_sio_quirk structures are never modified, so declare them as const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/usb/serial/ftdi_sio.c |   16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 3a814e8..0082080 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -93,27 +93,27 @@ static int   ftdi_8u2232c_probe(struct usb_serial *serial);
 static void  ftdi_USB_UIRT_setup(struct ftdi_private *priv);
 static void  ftdi_HE_TIRA1_setup(struct ftdi_private *priv);
 
-static struct ftdi_sio_quirk ftdi_jtag_quirk = {
+static const struct ftdi_sio_quirk ftdi_jtag_quirk = {
.probe  = ftdi_jtag_probe,
 };
 
-static struct ftdi_sio_quirk ftdi_NDI_device_quirk = {
+static const struct ftdi_sio_quirk ftdi_NDI_device_quirk = {
.probe  = ftdi_NDI_device_setup,
 };
 
-static struct ftdi_sio_quirk ftdi_USB_UIRT_quirk = {
+static const struct ftdi_sio_quirk ftdi_USB_UIRT_quirk = {
.port_probe = ftdi_USB_UIRT_setup,
 };
 
-static struct ftdi_sio_quirk ftdi_HE_TIRA1_quirk = {
+static const struct ftdi_sio_quirk ftdi_HE_TIRA1_quirk = {
.port_probe = ftdi_HE_TIRA1_setup,
 };
 
-static struct ftdi_sio_quirk ftdi_stmclite_quirk = {
+static const struct ftdi_sio_quirk ftdi_stmclite_quirk = {
.probe  = ftdi_stmclite_probe,
 };
 
-static struct ftdi_sio_quirk ftdi_8u2232c_quirk = {
+static const struct ftdi_sio_quirk ftdi_8u2232c_quirk = {
.probe  = ftdi_8u2232c_probe,
 };
 
@@ -1775,7 +1775,7 @@ static void remove_sysfs_attrs(struct usb_serial_port 
*port)
 static int ftdi_sio_probe(struct usb_serial *serial,
const struct usb_device_id *id)
 {
-   struct ftdi_sio_quirk *quirk =
+   const struct ftdi_sio_quirk *quirk =
(struct ftdi_sio_quirk *)id->driver_info;
 
if (quirk && quirk->probe) {
@@ -1792,7 +1792,7 @@ static int ftdi_sio_probe(struct usb_serial *serial,
 static int ftdi_sio_port_probe(struct usb_serial_port *port)
 {
struct ftdi_private *priv;
-   struct ftdi_sio_quirk *quirk = usb_get_serial_data(port->serial);
+   const struct ftdi_sio_quirk *quirk = usb_get_serial_data(port->serial);
 
 
priv = kzalloc(sizeof(struct ftdi_private), GFP_KERNEL);

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


[PATCH 2/2] drivers/mmc/host/vub300.c: add missing usb_free_urb

2012-07-24 Thread Julia Lawall
From: Julia Lawall 

Add missing usb_free_urb on failure path after usb_alloc_urb.

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

// 
@km exists@
local idexpression e;
expression e1,e2,e3;
type T,T1;
identifier f;
@@

* e = usb_alloc_urb(...)
... when any
when != e = e1
when != e1 = (T)e
when != e1(...,(T)e,...)
when != &e->f
if(...) { ... when != e2(...,(T1)e,...)
 when != e3 = e
 when forall
(
 return <+...e...+>;
|
* return ...;
) }
// 

Signed-off-by: Julia Lawall 

---
 drivers/mmc/host/vub300.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
index 3135a1a..6c39bf1 100644
--- a/drivers/mmc/host/vub300.c
+++ b/drivers/mmc/host/vub300.c
@@ -2358,9 +2358,9 @@ error5:
 * which is contained at the end of struct mmc
 */
 error4:
-   usb_free_urb(command_out_urb);
-error1:
usb_free_urb(command_res_urb);
+error1:
+   usb_free_urb(command_out_urb);
 error0:
return retval;
 }

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


[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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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 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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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 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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/5] drivers/usb/wusbcore/wa-hc.c: fix error return code

2012-08-13 Thread Julia Lawall
From: Julia Lawall 

Convert a 0 error return code to a negative one, as returned elsewhere in the
function.

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

// 
@@
identifier ret;
expression e,e1,e2,e3,e4,x;
@@

(
if (\(ret != 0\|ret < 0\) || ...) { ... return ...; }
|
ret = 0
)
... when != ret = e1
*x = 
\(kmalloc\|kzalloc\|kcalloc\|devm_kzalloc\|ioremap\|ioremap_nocache\|devm_ioremap\|devm_ioremap_nocache\)(...);
... when != x = e2
when != ret = e3
*if (x == NULL || ...)
{
  ... when != ret = e4
*  return ret;
}
// 

Signed-off-by: Julia Lawall 

---
 drivers/usb/wusbcore/wa-hc.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/wusbcore/wa-hc.c b/drivers/usb/wusbcore/wa-hc.c
index 9e4a924..a09b65e 100644
--- a/drivers/usb/wusbcore/wa-hc.c
+++ b/drivers/usb/wusbcore/wa-hc.c
@@ -46,8 +46,10 @@ int wa_create(struct wahc *wa, struct usb_interface *iface)
wa->dto_epd = &iface->cur_altsetting->endpoint[2].desc;
wa->xfer_result_size = usb_endpoint_maxp(wa->dti_epd);
wa->xfer_result = kmalloc(wa->xfer_result_size, GFP_KERNEL);
-   if (wa->xfer_result == NULL)
+   if (wa->xfer_result == NULL) {
+   result = -ENOMEM;
goto error_xfer_result_alloc;
+   }
result = wa_nep_create(wa, iface);
if (result < 0) {
dev_err(dev, "WA-CDS: can't initialize notif endpoint: %d\n",

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


[PATCH 5/5] drivers/usb/host/ehci-platform.c: fix error return code

2012-08-13 Thread Julia Lawall
From: Julia Lawall 

Convert a possibly 0 error return code to a negative one, as returned
elsewhere in the function.

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

// 
@@
identifier ret;
expression e,e1,e2,e3,e4,x;
@@

(
if (\(ret != 0\|ret < 0\) || ...) { ... return ...; }
|
ret = 0
)
... when != ret = e1
*x = 
\(kmalloc\|kzalloc\|kcalloc\|devm_kzalloc\|ioremap\|ioremap_nocache\|devm_ioremap\|devm_ioremap_nocache\)(...);
... when != x = e2
when != ret = e3
*if (x == NULL || ...)
{
  ... when != ret = e4
*  return ret;
}
// 

Signed-off-by: Julia Lawall 

---
 drivers/usb/host/ehci-platform.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index 91acdde..764e010 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -128,8 +128,10 @@ static int __devinit ehci_platform_probe(struct 
platform_device *dev)
}
 
hcd->regs = ioremap_nocache(hcd->rsrc_start, hcd->rsrc_len);
-   if (!hcd->regs)
+   if (!hcd->regs) {
+   err = -ENOMEM;
goto err_release_region;
+   }
err = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (err)
goto err_iounmap;

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


[PATCH 1/5] drivers/usb/gadget/s3c-hsotg.c: fix error return code

2012-08-13 Thread Julia Lawall
From: Julia Lawall 

Convert a 0 error return code to a negative one, as returned elsewhere in the
function.

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

// 
@@
identifier ret;
expression e,e1,e2,e3,e4,x;
@@

(
if (\(ret != 0\|ret < 0\) || ...) { ... return ...; }
|
ret = 0
)
... when != ret = e1
*x = 
\(kmalloc\|kzalloc\|kcalloc\|devm_kzalloc\|ioremap\|ioremap_nocache\|devm_ioremap\|devm_ioremap_nocache\)(...);
... when != x = e2
when != ret = e3
*if (x == NULL || ...)
{
  ... when != ret = e4
*  return ret;
}
// 

Signed-off-by: Julia Lawall 

---
 drivers/usb/gadget/s3c-hsotg.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index b13e0bb..0bb617e 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -3599,6 +3599,7 @@ static int __devinit s3c_hsotg_probe(struct 
platform_device *pdev)
 
if (hsotg->num_of_eps == 0) {
dev_err(dev, "wrong number of EPs (zero)\n");
+   ret = -EINVAL;
goto err_supplies;
}
 
@@ -3606,6 +3607,7 @@ static int __devinit s3c_hsotg_probe(struct 
platform_device *pdev)
  GFP_KERNEL);
if (!eps) {
dev_err(dev, "cannot get memory\n");
+   ret = -ENOMEM;
goto err_supplies;
}
 
@@ -3622,6 +3624,7 @@ static int __devinit s3c_hsotg_probe(struct 
platform_device *pdev)
 GFP_KERNEL);
if (!hsotg->ctrl_req) {
dev_err(dev, "failed to allocate ctrl req\n");
+   ret = -ENOMEM;
goto err_ep_mem;
}
 

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


[PATCH 4/5] drivers/usb/host/ohci-platform.c: fix error return code

2012-08-13 Thread Julia Lawall
From: Julia Lawall 

Convert a possibly 0 error return code to a negative one, as returned
elsewhere in the function.

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

// 
@@
identifier ret;
expression e,e1,e2,e3,e4,x;
@@

(
if (\(ret != 0\|ret < 0\) || ...) { ... return ...; }
|
ret = 0
)
... when != ret = e1
*x = 
\(kmalloc\|kzalloc\|kcalloc\|devm_kzalloc\|ioremap\|ioremap_nocache\|devm_ioremap\|devm_ioremap_nocache\)(...);
... when != x = e2
when != ret = e3
*if (x == NULL || ...)
{
  ... when != ret = e4
*  return ret;
}
// 

Signed-off-by: Julia Lawall 

---
 drivers/usb/host/ohci-platform.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
index 10d85b9..e24ec9f 100644
--- a/drivers/usb/host/ohci-platform.c
+++ b/drivers/usb/host/ohci-platform.c
@@ -130,8 +130,10 @@ static int __devinit ohci_platform_probe(struct 
platform_device *dev)
}
 
hcd->regs = ioremap_nocache(hcd->rsrc_start, hcd->rsrc_len);
-   if (!hcd->regs)
+   if (!hcd->regs) {
+   err = -ENOMEM;
goto err_release_region;
+   }
err = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (err)
goto err_iounmap;

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


[PATCH 3/6] drivers/usb/gadget/lpc32xx_udc.c: adjust inconsistent IS_ERR and PTR_ERR

2012-08-25 Thread Julia Lawall
From: Julia Lawall 

Change the call to PTR_ERR to access the value just tested by IS_ERR.

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

// 
@@
expression e,e1;
@@

(
if (IS_ERR(e)) { ... PTR_ERR(e) ... }
|
if (IS_ERR(e=e1)) { ... PTR_ERR(e) ... }
|
*if (IS_ERR(e))
 { ...
*  PTR_ERR(e1)
   ... }
)
// 

Signed-off-by: Julia Lawall 

---
 drivers/usb/gadget/lpc32xx_udc.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/lpc32xx_udc.c b/drivers/usb/gadget/lpc32xx_udc.c
index f1ec99e..3a6cdd0 100644
--- a/drivers/usb/gadget/lpc32xx_udc.c
+++ b/drivers/usb/gadget/lpc32xx_udc.c
@@ -3208,7 +3208,7 @@ static int __init lpc32xx_udc_probe(struct 
platform_device *pdev)
udc->usb_otg_clk = clk_get(&pdev->dev, "ck_usb_otg");
if (IS_ERR(udc->usb_otg_clk)) {
dev_err(udc->dev, "failed to acquire USB otg clock\n");
-   retval = PTR_ERR(udc->usb_slv_clk);
+   retval = PTR_ERR(udc->usb_otg_clk);
goto usb_otg_clk_get_fail;
}
 

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


[PATCH 4/6] drivers/usb/host/ohci-nxp.c: adjust inconsistent IS_ERR and PTR_ERR

2012-08-25 Thread Julia Lawall
From: Julia Lawall 

Change the call to PTR_ERR to access the value just tested by IS_ERR.

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

// 
@@
expression e,e1;
@@

(
if (IS_ERR(e)) { ... PTR_ERR(e) ... }
|
if (IS_ERR(e=e1)) { ... PTR_ERR(e) ... }
|
*if (IS_ERR(e))
 { ...
*  PTR_ERR(e1)
   ... }
)
// 

Signed-off-by: Julia Lawall 

---
 drivers/usb/host/ohci-nxp.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ohci-nxp.c b/drivers/usb/host/ohci-nxp.c
index a446386..c60066a 100644
--- a/drivers/usb/host/ohci-nxp.c
+++ b/drivers/usb/host/ohci-nxp.c
@@ -355,7 +355,7 @@ static int __devinit usb_hcd_nxp_probe(struct 
platform_device *pdev)
usb_otg_clk = clk_get(&pdev->dev, "ck_usb_otg");
if (IS_ERR(usb_otg_clk)) {
dev_err(&pdev->dev, "failed to acquire USB DEV Clock\n");
-   ret = PTR_ERR(usb_dev_clk);
+   ret = PTR_ERR(usb_otg_clk);
goto out6;
}
 

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


Re: [PATCH] backports: backport drvdata = NULL core driver fixes

2013-07-19 Thread Julia Lawall
On Fri, 19 Jul 2013, Luis R. Rodriguez wrote:

> On Fri, Jul 19, 2013 at 2:07 PM, Luis R. Rodriguez
>  wrote:
> >> This is not a very good idea.  Although setting drvdata to NULL allowed
> >> a lot of code to be removed, it also exposed a bunch of hidden bugs --
> >> drivers were using the drvdata value even after their remove function
> >> returned.
> >
> > Eek, have we not SmPL'ify'd a proof yet to ensure code like this no
> > longer exists? Julia? :)
> 
> Come to think of it, perhaps we should require *proof* with SmPL like
> this in future to avoid regressions ?

Is it a concurrency problem?  SmPL is not so good at that in the general 
case.  One would have to know a specific case where other functions of the 
driver can be invoked after remove.

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


Re: [PATCH] USBIP:stub_dev.c fixing string splitted into multiple line issue

2013-07-31 Thread Julia Lawall
On Wed, 31 Jul 2013, Kumar Gaurav wrote:

> Fixed  String splitted into multiple line issue using macro

I'm not an expert on this kind of style issue, but I prefer strings that 
look like strings.

julia

> Signed-off-by: Kumar Gaurav 
> ---
>  drivers/staging/usbip/stub_dev.c |   17 ++---
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/usbip/stub_dev.c 
> b/drivers/staging/usbip/stub_dev.c
> index 83d629a..3b881e1 100644
> --- a/drivers/staging/usbip/stub_dev.c
> +++ b/drivers/staging/usbip/stub_dev.c
> @@ -30,6 +30,8 @@
>   * In most cases, wildcard matching will be okay because driver binding can 
> be
>   * changed dynamically by a userland program.
>   */
> +#define USBIP_REG_INTF "register new interface (bus %u dev %u ifn %u)\n"
> +#define USBIP_REG_DEV "register new device (bus %u dev %u ifn %u)\n"
>  static struct usb_device_id stub_table[] = {
>  #if 0
>   /* just an example */
> @@ -357,8 +359,9 @@ static int stub_probe(struct usb_interface *interface,
>   busid_priv = get_busid_priv(udev_busid);
>   if (!busid_priv || (busid_priv->status == STUB_BUSID_REMOV) ||
>   (busid_priv->status == STUB_BUSID_OTHER)) {
> - dev_info(&interface->dev, "%s is not in match_busid table... "
> -  "skip!\n", udev_busid);
> + dev_info(&interface->dev,
> +  "%s is not in match_busid table... skip!\n",
> + udev_busid);
>  
>   /*
>* Return value should be ENODEV or ENOXIO to continue trying
> @@ -386,8 +389,7 @@ static int stub_probe(struct usb_interface *interface,
>   return -ENODEV;
>  
>   busid_priv->interf_count++;
> - dev_info(&interface->dev, "usbip-host: register new interface "
> -  "(bus %u dev %u ifn %u)\n",
> + dev_info(&interface->dev, USBIP_REG_INTF,
>udev->bus->busnum, udev->devnum,
>interface->cur_altsetting->desc.bInterfaceNumber);
>  
> @@ -412,8 +414,8 @@ static int stub_probe(struct usb_interface *interface,
>   if (!sdev)
>   return -ENOMEM;
>  
> - dev_info(&interface->dev, "usbip-host: register new device "
> -  "(bus %u dev %u ifn %u)\n", udev->bus->busnum, udev->devnum,
> + dev_info(&interface->dev, USBIP_REG_DEV,
> +  udev->bus->busnum, udev->devnum,
>interface->cur_altsetting->desc.bInterfaceNumber);
>  
>   busid_priv->interf_count = 0;
> @@ -426,7 +428,8 @@ static int stub_probe(struct usb_interface *interface,
>  
>   err = stub_add_files(&interface->dev);
>   if (err) {
> - dev_err(&interface->dev, "stub_add_files for %s\n", udev_busid);
> + dev_err(&interface->dev, "stub_add_files for %s\n",
> + udev_busid);
>   usb_set_intfdata(interface, NULL);
>   usb_put_intf(interface);
>   usb_put_dev(udev);
> -- 
> 1.7.9.5
> 
> --
> 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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] USBIP:stub_dev.c fixing string splitted into multiple line issue

2013-08-01 Thread Julia Lawall
> Thanks you all for suggestions. I'll try finding some other way.

If the strings don't fit, you can just go over 80 characters.

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


[PATCH 29/29] usb: dwc3: omap: simplify use of devm_ioremap_resource

2013-08-14 Thread Julia Lawall
From: Julia Lawall 

Remove unneeded error handling on the result of a call to
platform_get_resource when the value is passed to devm_ioremap_resource.

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

// 
@@
expression pdev,res,n,e,e1;
expression ret != 0;
identifier l;
@@

- res = platform_get_resource(pdev, IORESOURCE_MEM, n);
  ... when != res
- if (res == NULL) { ... \(goto l;\|return ret;\) }
  ... when != res
+ res = platform_get_resource(pdev, IORESOURCE_MEM, n);
  e = devm_ioremap_resource(e1, res);
// 

Signed-off-by: Julia Lawall 

---
 drivers/usb/dwc3/dwc3-omap.c |5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 7f7ea62..cbcd972 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -432,11 +432,6 @@ static int dwc3_omap_probe(struct platform_device *pdev)
}
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!res) {
-   dev_err(dev, "missing memory base resource\n");
-   return -EINVAL;
-   }
-
base = devm_ioremap_resource(dev, res);
if (IS_ERR(base))
return PTR_ERR(base);

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


[PATCH 0/29] simplify use of devm_ioremap_resource

2013-08-14 Thread Julia Lawall
devm_ioremap_resource often uses the result of a call to
platform_get_resource as its last argument.  devm_ioremap_resource does
appropriate error handling on this argument, so error handling can be
removed from the call site.  To make the connection between the call to
platform_get_resource and the call to devm_ioremap_resource more clear, the
former is also moved down to be adjacent to the latter.

In many cases, this patch changes the specific error value that is
returned on failure of platform_get_resource.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// 
@@
expression pdev,res,n,e,e1;
expression ret != 0;
identifier l;
@@

(
  res = platform_get_resource(pdev, IORESOURCE_MEM, n);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
  e = devm_ioremap_resource(e1, res);
|
- res = platform_get_resource(pdev, IORESOURCE_MEM, n);
  ... when != res
- if (res == NULL) { ... \(goto l;\|return ret;\) }
  ... when != res
+ res = platform_get_resource(pdev, IORESOURCE_MEM, n);
  e = devm_ioremap_resource(e1, res);
)
// 

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


Re: [PATCH 1/6] drivers:usb:host:xhci.h Removing xhci_hcd from argument

2013-08-16 Thread Julia Lawall
On Sat, 17 Aug 2013, Kumar Gaurav wrote:

> On Saturday 17 August 2013 12:30 AM, Greg KH wrote:
> > On Sat, Aug 17, 2013 at 12:18:42AM +0530, Kumar Gaurav wrote:
> > > Removed struct xhci_hcd from xhci_readl fucntion as it's no more in use.
> > > ---
> > >   drivers/usb/host/xhci.h |3 +--
> > >   1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.hthe
> > > index c338741..7cf0e41 100644
> > > --- a/drivers/usb/host/xhci.h
> > > +++ b/drivers/usb/host/xhci.h
> > > @@ -1598,8 +1598,7 @@ static inline struct usb_hcd *xhci_to_hcd(struct
> > > xhci_hcd *xhci)
> > > /* TODO: copied from ehci.h - can be refactored? */
> > >   /* xHCI spec says all registers are little endian */
> > > -static inline unsigned int xhci_readl(const struct xhci_hcd *xhci,
> > > - __le32 __iomem *regs)
> > > +static inline unsigned int xhci_readl(__le32 __iomem *regs)
> > And you broke the build :(
> > 
> > Sorry, that's not ok.
> > 
> > greg k-h
> I'm writing patch to change function definition along with changes in program
> files where this function is called.
> After changing all files i built the module and it compiled as well.
> 
> There were 6 files which were using xhci_read. I fixed all the calls and
> compiled.
> Am i missing anything? Please suggest then i'll send patches again

I haven't followed along in detail, but I think Greg wants one patch that 
fixes both the definition and all of the calls, and not one patch for each 
file.

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


Re: [PATCH 5/6] for fixing xhci_readl call in xhci_hub.c after removing xhci_hcd from function definition

2013-08-17 Thread Julia Lawall
On Sat, 17 Aug 2013, Dan Carpenter wrote:

> Wait what?  Why did we break the build in the first place?

Kumar didn't understand the relation between "one issue one patch" and 
"thou shalt not break the build".  I believe he either has sent or will 
send a new patch set that makes both changes at once.

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


[PATCH 0/6] simplify platform_get_resource_byname/devm_ioremap_resource

2013-08-19 Thread Julia Lawall
devm_ioremap_resource often uses the result of a call to
platform_get_resource_byname as its last argument.  devm_ioremap_resource
does appropriate error handling on this argument, so error handling can be
removed from the call to platform_get_resource_byname.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// 
@@
expression pdev,res,n,e,e1;
expression ret != 0;
identifier l,f;
expression list es;
@@

(
  res = f(...);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
  e = devm_ioremap_resource(e1, res);
|
- res = f(es);
  ... when != res
- if (res == NULL) { ... \(goto l;\|return ret;\) }
  ... when != res
+ res = f(es);
  e = devm_ioremap_resource(e1, res);
)
// 

In practice, f is always platform_get_resource_byname (or
platform_get_resource, which was handled by a previous patch series).  And
the call to platform_get_resource_byname always immediately precedes the
call to devm_ioremap_resource.

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


[PATCH 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

2013-08-19 Thread Julia Lawall
From: Julia Lawall 

Remove unneeded error handling on the result of a call to
platform_get_resource_byname when the value is passed to devm_ioremap_resource.

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

// 
@@
expression pdev,res,e,e1;
expression ret != 0;
identifier l;
@@

  res = platform_get_resource_byname(...);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
  e = devm_ioremap_resource(e1, res);
// 

Signed-off-by: Julia Lawall 

---
 drivers/usb/musb/musb_dsps.c |3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 4ffbaac..f2f9710 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -357,9 +357,6 @@ static int dsps_musb_init(struct musb *musb)
u32 rev, val;
 
r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
-   if (!r)
-   return -EINVAL;
-
reg_base = devm_ioremap_resource(dev, r);
if (!musb->ctrl_base)
return -EINVAL;

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


Re: [PATCH 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

2013-08-19 Thread Julia Lawall


On Mon, 19 Aug 2013, Svenning Sørensen wrote:

> On 19-08-2013 10:51, Julia Lawall wrote:
> > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> > index 4ffbaac..f2f9710 100644
> > --- a/drivers/usb/musb/musb_dsps.c
> > +++ b/drivers/usb/musb/musb_dsps.c
> > @@ -357,9 +357,6 @@ static int dsps_musb_init(struct musb *musb)
> > u32 rev, val;
> > r = platform_get_resource_byname(parent, IORESOURCE_MEM, 
> > "control");
> > -   if (!r)
> > -   return -EINVAL;
> > -
> > reg_base = devm_ioremap_resource(dev, r);
> > if (!musb->ctrl_base)
> > return -EINVAL;
> Not really related to Julia's patch, apart from making it more obvious that
> there's a bug here.
> I believe it is reg_base that needs to be checked, right?

Indeed, it is all backwards.  I could extend the patch, if you want.

julia

[PATCH 4/6 v2] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

2013-08-19 Thread Julia Lawall
From: Julia Lawall 

Remove unneeded error handling on the result of a call to
platform_get_resource_byname when the value is passed to
devm_ioremap_resource.

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

// 
@@
expression pdev,res,e,e1;
expression ret != 0;
identifier l;
@@

  res = platform_get_resource_byname(...);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
  e = devm_ioremap_resource(e1, res);
// 

This patch also adjusts the error-handling code on the call to
devm_ioremap_resource to check the right value, noticed by Svenning Sørensen.

Signed-off-by: Julia Lawall 

---
v2: Fixed the bug on the test of the result of devm_ioremap_resource

 drivers/usb/musb/musb_dsps.c |7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 4ffbaac..e60be6f 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -357,12 +357,9 @@ static int dsps_musb_init(struct musb *musb)
u32 rev, val;

r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
-   if (!r)
-   return -EINVAL;
-
reg_base = devm_ioremap_resource(dev, r);
-   if (!musb->ctrl_base)
-   return -EINVAL;
+if (IS_ERR(reg_base))
+   return PTR_ERR(reg_base);
musb->ctrl_base = reg_base;

/* NOP driver needs change if supporting dual instance */

Re: [PATCH 4/6] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

2013-08-19 Thread Julia Lawall
On Mon, 19 Aug 2013, Svenning Sørensen wrote:

>
> On 19-08-2013 13:35, Julia Lawall wrote:
> > reg_base = devm_ioremap_resource(dev, r);
> > if (!musb->ctrl_base)
> > return -EINVAL;
> > > Not really related to Julia's patch, apart from making it more obvious
> > > that
> > > there's a bug here.
> > > I believe it is reg_base that needs to be checked, right?
> > Indeed, it is all backwards.  I could extend the patch, if you want.
> >
> I'll let Felipe decide on that, but I can't imagine any objections.
> IS_ERR() is the proper test, of course :)

I've already sent a patch, in case it is useful.

julia

Re: [PATCH 4/6 v2] usb: musb: dsps: simplify platform_get_resource_byname/devm_ioremap_resource

2013-08-19 Thread Julia Lawall


On Mon, 19 Aug 2013, Sergei Shtylyov wrote:

> Hello.
>
> On 08/19/2013 03:47 PM, Julia Lawall wrote:
>
> > From: Julia Lawall 
>
> > Remove unneeded error handling on the result of a call to
> > platform_get_resource_byname when the value is passed to
> > devm_ioremap_resource.
>
> > A simplified version of the semantic patch that makes this change is as
> > follows: (http://coccinelle.lip6.fr/)
>
> > // 
> > @@
> > expression pdev,res,e,e1;
> > expression ret != 0;
> > identifier l;
> > @@
> >
> >res = platform_get_resource_byname(...);
> > - if (res == NULL) { ... \(goto l;\|return ret;\) }
> >e = devm_ioremap_resource(e1, res);
> > // 
>
> > This patch also adjusts the error-handling code on the call to
> > devm_ioremap_resource to check the right value, noticed by Svenning
> > Sørensen.
>
>Saying "also" in the changelog is usuallly a good sign the patch should be
> split.

OK, the original patch already does the first part.  I will send a new
patch for the bug fix.

julia

>
> > Signed-off-by: Julia Lawall 
>
> > ---
> > v2: Fixed the bug on the test of the result of devm_ioremap_resource
>
> >   drivers/usb/musb/musb_dsps.c |7 ++-
> >   1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> > index 4ffbaac..e60be6f 100644
> > --- a/drivers/usb/musb/musb_dsps.c
> > +++ b/drivers/usb/musb/musb_dsps.c
> > @@ -357,12 +357,9 @@ static int dsps_musb_init(struct musb *musb)
> > u32 rev, val;
> >
> > r = platform_get_resource_byname(parent, IORESOURCE_MEM, "control");
> > -   if (!r)
> > -   return -EINVAL;
> > -
> > reg_base = devm_ioremap_resource(dev, r);
> > -   if (!musb->ctrl_base)
> > -   return -EINVAL;
> > +if (IS_ERR(reg_base))
> > +   return PTR_ERR(reg_base);
>
>This is indented with space, not tabs. And of course, this was a matter of
> a separate *fix* patch, while the original patch was a *cleanup*.
>
> WBR, Sergei
>
> --
> 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
>

[PATCH] usb: musb: dsps: fix devm_ioremap_resource error detection code

2013-08-19 Thread Julia Lawall
From: Julia Lawall 

devm_ioremap_resource returns an ERR_PTR value, not NULL, on failure.
Furthermore, the value returned by devm_ioremap_resource should be tested.

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

// 
@@
expression e,e1;
statement S;
@@

*e = devm_ioremap_resource(...);
if (!e1) S

// 

Signed-off-by: Julia Lawall 

---
 drivers/usb/musb/musb_dsps.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 4ffbaac..4ad52e7 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -361,8 +361,8 @@ static int dsps_musb_init(struct musb *musb)
return -EINVAL;
 
reg_base = devm_ioremap_resource(dev, r);
-   if (!musb->ctrl_base)
-   return -EINVAL;
+   if (IS_ERR(reg_base))
+   return PTR_ERR(reg_base);
musb->ctrl_base = reg_base;
 
/* NOP driver needs change if supporting dual instance */

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


[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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 10/25] usb: gadget: fix error return code

2013-12-29 Thread Julia Lawall
From: Julia Lawall 

Set the return variable to an error code as done elsewhere in the function.

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 

---
Not tested.

 drivers/usb/gadget/printer.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/gadget/printer.c b/drivers/usb/gadget/printer.c
index bf7a56b..92b55bd 100644
--- a/drivers/usb/gadget/printer.c
+++ b/drivers/usb/gadget/printer.c
@@ -1133,6 +1133,7 @@ static int __init printer_bind_config(struct 
usb_configuration *c)
  NULL, "g_printer");
if (IS_ERR(dev->pdev)) {
ERROR(dev, "Failed to create device: g_printer\n");
+   status = PTR_ERR(dev->pdev);
goto fail;
}
 

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


[PATCH 0/25] fix error return code

2013-12-29 Thread Julia Lawall
These patches fix cases where the return variable is not set to an error
code in an error case.

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


[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-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 6/7] host: ehci-w90x900: fix error return code

2014-11-20 Thread Julia Lawall
From: Julia Lawall 

Return a negative error code on failure.

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

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

Signed-off-by: Julia Lawall 

---
The patches in this series are independent of each other.

 drivers/usb/host/ehci-w90x900.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-w90x900.c b/drivers/usb/host/ehci-w90x900.c
index 93d9cd6..e42a29e 100644
--- a/drivers/usb/host/ehci-w90x900.c
+++ b/drivers/usb/host/ehci-w90x900.c
@@ -75,8 +75,10 @@ static int usb_w90x900_probe(const struct hc_driver *driver,
__raw_writel(val, ehci->regs+PHY1_CTR);
 
irq = platform_get_irq(pdev, 0);
-   if (irq < 0)
+   if (irq < 0) {
+   retval = irq;
goto err2;
+   }
 
retval = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (retval != 0)

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


[PATCH 0/7] fix error return code

2014-11-20 Thread Julia Lawall
The complate semantic patch that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

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

// identify a function that returns a negative return value at least once.
f(...) {
... when any
(
return -c@i;
|
ret = -c@i;
... when != ret = e
return ret;
|
if (ret < 0) { ... return ret; }
)
... when any
}

@r exists@
identifier ret,ok.f,fn;
expression e1,e2,e3,e4,e5,e6,x;
statement S,S1;
position p1,p2,p3;
@@

// identify a case where the return variable is set to a non-negative value
// and then returned in error-handling code
f(...) {
... when any
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != \(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\)
when != &ret
when any
(
 if (<+... ret = e5 ...+>) S1
|
 if (<+... &ret ...+>) S1
|
if@p2(<+...x = fn(...)...+>)
 {
  ... when != ret = e6
  when forall
 return@p3 ret;
}
|
break;
|
x = fn(...)
... when != \(ret = e4\|ret++\|ret--\|ret+=e4\|ret-=e4\)
when != &ret
(
 if (<+... ret = e3 ...+>) S
|
 if (<+... &ret ...+>) S
|
if@p2(<+...\(x != 0\|x < 0\|x == NULL\|IS_ERR(x)\)...+>)
 {
  ... when != ret = e2
  when forall
 return@p3 ret;
}
)
)
... when any
}

@printer depends on r@
position p;
identifier ok.f,pr;
constant char [] c;
@@

f(...) { <...pr@p(...,c,...)...> }

@bad0 exists@
identifier r.ret,ok.f,g != {ERR_PTR,IS_ERR};
position p != printer.p;
@@

f(...) { ... when any
g@p(...,ret,...)
... when any
 }

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

// ignore the above if there is some path where the variable is set to
// something else
(
if@p1 (\(ret < 0\|ret != 0\)) S1
|
ret@p1 = 0
)
... when any
 \(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\|&ret\)
... when any
if@p2(...) S2

@bad1 depends on !bad0 && !bad exists@
position r.p2;
statement S2;
identifier r.ret;
expression e1;
constant c;
@@

ret = -c
... when != \(ret = e1\|ret++\|ret--\|ret+=e1\|ret-=e1\)
when != &ret
when any
if@p2(...) S2

@bad2 depends on !bad0 && !bad && !bad1 exists@
position r.p1,r.p2;
identifier r.ret;
expression e1;
statement S2;
constant c;
@@

// likewise ignore it if there has been an intervening return
ret@p1 = 0
... when != if (...) { ... ret = e1 ... return ret; }
when != if (...) { ... return -c; }
when any
if@p2(...) S2


@script:python depends on !bad0 && !bad && !bad1 && !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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/7] dwc3: dwc3-keystone: fix error return code

2014-11-20 Thread Julia Lawall
From: Julia Lawall 

Return a negative error code on failure.

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

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

Signed-off-by: Julia Lawall 

---
The patches in this series are independent of each other.

 drivers/usb/dwc3/dwc3-keystone.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c
index dd8d2df..fe3b933 100644
--- a/drivers/usb/dwc3/dwc3-keystone.c
+++ b/drivers/usb/dwc3/dwc3-keystone.c
@@ -123,6 +123,7 @@ static int kdwc3_probe(struct platform_device *pdev)
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
dev_err(&pdev->dev, "missing irq\n");
+   error = irq;
goto err_irq;
}
 

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


Re: [PATCH 1/1] USB: serial: Deletion of an unnecessary check before the function call "release_firmware"

2014-11-21 Thread Julia Lawall


On Fri, 21 Nov 2014, SF Markus Elfring wrote:

> From: Markus Elfring 
> Date: Fri, 21 Nov 2014 16:15:34 +0100
>
> The release_firmware() function tests whether its argument is NULL
> and then returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring 
> ---
>  drivers/usb/serial/mxuport.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c
> index ab1d690..3653ec1 100644
> --- a/drivers/usb/serial/mxuport.c
> +++ b/drivers/usb/serial/mxuport.c
> @@ -1101,8 +1101,7 @@ static int mxuport_probe(struct usb_serial *serial,
>*/
>   usb_set_serial_data(serial, (void *)id->driver_info);
>  out:
> - if (fw_p)
> - release_firmware(fw_p);
> + release_firmware(fw_p);

I think that the if should stay.  There were two cases on the allocation,
so there should be two cases on the release.

julia

>   return err;
>  }
>
> --
> 2.1.3
>
> --
> 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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: USB: serial: Deletion of an unnecessary check before the function call "release_firmware"

2014-11-21 Thread Julia Lawall


On Fri, 21 Nov 2014, SF Markus Elfring wrote:

> >> diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c
> >> index ab1d690..3653ec1 100644
> >> --- a/drivers/usb/serial/mxuport.c
> >> +++ b/drivers/usb/serial/mxuport.c
> >> @@ -1101,8 +1101,7 @@ static int mxuport_probe(struct usb_serial *serial,
> >> */
> >>usb_set_serial_data(serial, (void *)id->driver_info);
> >>  out:
> >> -  if (fw_p)
> >> -  release_firmware(fw_p);
> >> +  release_firmware(fw_p);
> >
> > I think that the if should stay.
>
> I have got an other opinion.
>
>
> > There were two cases on the allocation, so there should be two cases
> > on the release.
>
> I find that this implementation detail does not really matter for the
> necessity of a null pointer check directly before such a function call.

Conceptually, if it is clear 10 lines above that nothing was allocated,
and there is a fallback to existing data (according to the comment above)
it is strange to be releasing something.

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


[PATCH] usb: dwc3: return error code from the most recent call

2014-11-22 Thread Julia Lawall
From: Julia Lawall 

Copy-paste error from the previous block of error handling code.

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

// 
@@
expression e,e1;
@@

if (IS_ERR(e)) {
  ...
(
  ret = PTR_ERR(e);
|
*  ret = PTR_ERR(e1);
)
  ...
  return ret;
}
// 

Signed-off-by: Julia Lawall 

---
 drivers/usb/dwc3/dwc3-st.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
index c7602b5..4a1a543 100644
--- a/drivers/usb/dwc3/dwc3-st.c
+++ b/drivers/usb/dwc3/dwc3-st.c
@@ -243,7 +243,7 @@ static int st_dwc3_probe(struct platform_device *pdev)
dwc3_data->rstc_rst = devm_reset_control_get(dev, "softreset");
if (IS_ERR(dwc3_data->rstc_rst)) {
dev_err(&pdev->dev, "could not get reset controller\n");
-   ret = PTR_ERR(dwc3_data->rstc_pwrdn);
+   ret = PTR_ERR(dwc3_data->rstc_rst);
goto undo_powerdown;
}
 

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


[PATCH] usb: gadget: fix platform_no_drv_owner.cocci warnings

2014-11-25 Thread Julia Lawall
Remove .owner field if calls are used which set it automatically

Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci

Signed-off-by: Julia Lawall 

---

 bdc_core.c |1 -
 1 file changed, 1 deletion(-)

--- a/drivers/usb/gadget/udc/bdc/bdc_core.c
+++ b/drivers/usb/gadget/udc/bdc/bdc_core.c
@@ -521,7 +521,6 @@ static int bdc_remove(struct platform_de
 static struct platform_driver bdc_driver = {
.driver = {
.name   = BRCM_BDC_NAME,
-   .owner  = THIS_MODULE
},
.probe  = bdc_probe,
.remove = bdc_remove,
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 8/8] wusb: replace memset by memzero_explicit

2014-11-30 Thread Julia Lawall
From: Julia Lawall 

Memset on a local variable may be removed when it is called just before the
variable goes out of scope.  Using memzero_explicit defeats this
optimization.  A simplified version of the semantic patch that makes this
change is as follows: (http://coccinelle.lip6.fr/)

// 
@@
identifier x;
type T;
@@

{
... when any
T x[...];
... when any
when exists
- memset
+ memzero_explicit
  (x,
-0,
  ...)
... when != x
when strict
}
// 

This change was suggested by Daniel Borkmann 

Signed-off-by: Julia Lawall 

---
Daniel Borkmann suggested that these patches could go through Herbert Xu's
cryptodev tree.

 drivers/usb/wusbcore/dev-sysfs.c |2 +-
 drivers/usb/wusbcore/security.c  |8 
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/wusbcore/dev-sysfs.c b/drivers/usb/wusbcore/dev-sysfs.c
index 1018345..415b140 100644
--- a/drivers/usb/wusbcore/dev-sysfs.c
+++ b/drivers/usb/wusbcore/dev-sysfs.c
@@ -101,7 +101,7 @@ static ssize_t wusb_ck_store(struct device *dev,
if (wusbhc == NULL)
return -ENODEV;
result = wusb_dev_4way_handshake(wusbhc, usb_dev->wusb_dev, &ck);
-   memset(&ck, 0, sizeof(ck));
+   memzero_explicit(&ck, sizeof(ck));
wusbhc_put(wusbhc);
return result < 0 ? result : size;
 }
diff --git a/drivers/usb/wusbcore/security.c b/drivers/usb/wusbcore/security.c
index cc74d66..b66faaf 100644
--- a/drivers/usb/wusbcore/security.c
+++ b/drivers/usb/wusbcore/security.c
@@ -522,10 +522,10 @@ error_hs3:
 error_hs2:
 error_hs1:
memset(hs, 0, 3*sizeof(hs[0]));
-   memset(&keydvt_out, 0, sizeof(keydvt_out));
-   memset(&keydvt_in, 0, sizeof(keydvt_in));
-   memset(&ccm_n, 0, sizeof(ccm_n));
-   memset(mic, 0, sizeof(mic));
+   memzero_explicit(&keydvt_out, sizeof(keydvt_out));
+   memzero_explicit(&keydvt_in, sizeof(keydvt_in));
+   memzero_explicit(&ccm_n, sizeof(ccm_n));
+   memzero_explicit(mic, sizeof(mic));
if (result < 0)
wusb_dev_set_encryption(usb_dev, 0);
 error_dev_set_encryption:

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


[PATCH 0/8] replace memset by memzero_explicit

2014-11-30 Thread Julia Lawall
Memset on a local variable may be removed when it is called just before the
variable goes out of scope.  Using memzero_explicit defeats this
optimization.  The complete semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)

// 
@@
identifier x;
local idexpression e;
type T,T1;
@@

{
... when any
T x[...];
... when any
when exists
(
e = (T1)x
|
e = (T1)&x[0]
)
... when any
when exists
- memset
+ memzero_explicit
  (x,
-0,
  ...)
... when != x
when != e
when strict
}

@@
identifier i,x;
local idexpression e;
type T;
@@

{
... when any
struct i x;
... when any
when exists
e = (T)&x
... when any
when exists
- memset
+ memzero_explicit
  (&x,
-0,
  ...)
... when != x
when != e
when strict
}

// 

@@
identifier x;
type T,T1;
expression e;
@@

{
... when any
T x[...];
... when any
when exists
when != e = (T1)x
when != e = (T1)&x[0]
- memset
+ memzero_explicit
  (x,
-0,
  ...)
... when != x
when strict
}

@@
identifier i,x;
expression e;
type T;
@@

{
... when any
struct i x;
... when any
when exists
when != e = (T)&x
- memset
+ memzero_explicit
  (&x,
-0,
  ...)
... when != x
when strict
}
// 

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


[PATCH 8/8 v2] wusb: replace memset by memzero_explicit

2014-11-30 Thread Julia Lawall
From: Julia Lawall 

Memset on a local variable may be removed when it is called just before the
variable goes out of scope.  Using memzero_explicit defeats this
optimization.  A simplified version of the semantic patch that makes this
change is as follows: (http://coccinelle.lip6.fr/)

// 
@@
identifier x;
type T;
@@

{
... when any
T x[...];
... when any
when exists
- memset
+ memzero_explicit
  (x,
-0,
  ...)
... when != x
when strict
}
// 

This change was suggested by Daniel Borkmann 

Signed-off-by: Julia Lawall 

---
Daniel Borkmann suggested that these patches could go through Herbert Xu's
cryptodev tree.

v2: fixed email address

 drivers/usb/wusbcore/dev-sysfs.c |2 +-
 drivers/usb/wusbcore/security.c  |8 
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/wusbcore/dev-sysfs.c b/drivers/usb/wusbcore/dev-sysfs.c
index 1018345..415b140 100644
--- a/drivers/usb/wusbcore/dev-sysfs.c
+++ b/drivers/usb/wusbcore/dev-sysfs.c
@@ -101,7 +101,7 @@ static ssize_t wusb_ck_store(struct device *dev,
if (wusbhc == NULL)
return -ENODEV;
result = wusb_dev_4way_handshake(wusbhc, usb_dev->wusb_dev, &ck);
-   memset(&ck, 0, sizeof(ck));
+   memzero_explicit(&ck, sizeof(ck));
wusbhc_put(wusbhc);
return result < 0 ? result : size;
 }
diff --git a/drivers/usb/wusbcore/security.c b/drivers/usb/wusbcore/security.c
index cc74d66..b66faaf 100644
--- a/drivers/usb/wusbcore/security.c
+++ b/drivers/usb/wusbcore/security.c
@@ -522,10 +522,10 @@ error_hs3:
 error_hs2:
 error_hs1:
memset(hs, 0, 3*sizeof(hs[0]));
-   memset(&keydvt_out, 0, sizeof(keydvt_out));
-   memset(&keydvt_in, 0, sizeof(keydvt_in));
-   memset(&ccm_n, 0, sizeof(ccm_n));
-   memset(mic, 0, sizeof(mic));
+   memzero_explicit(&keydvt_out, sizeof(keydvt_out));
+   memzero_explicit(&keydvt_in, sizeof(keydvt_in));
+   memzero_explicit(&ccm_n, sizeof(ccm_n));
+   memzero_explicit(mic, sizeof(mic));
if (result < 0)
wusb_dev_set_encryption(usb_dev, 0);
 error_dev_set_encryption:

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


[PATCH 0/8] replace memset by memzero_explicit

2014-11-30 Thread Julia Lawall
Memset on a local variable may be removed when it is called just before the
variable goes out of scope.  Using memzero_explicit defeats this
optimization.  The complete semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)

// 
@@
identifier x;
local idexpression e;
type T,T1;
@@

{
... when any
T x[...];
... when any
when exists
(
e = (T1)x
|
e = (T1)&x[0]
)
... when any
when exists
- memset
+ memzero_explicit
  (x,
-0,
  ...)
... when != x
when != e
when strict
}

@@
identifier i,x;
local idexpression e;
type T;
@@

{
... when any
struct i x;
... when any
when exists
e = (T)&x
... when any
when exists
- memset
+ memzero_explicit
  (&x,
-0,
  ...)
... when != x
when != e
when strict
}

// 

@@
identifier x;
type T,T1;
expression e;
@@

{
... when any
T x[...];
... when any
when exists
when != e = (T1)x
when != e = (T1)&x[0]
- memset
+ memzero_explicit
  (x,
-0,
  ...)
... when != x
when strict
}

@@
identifier i,x;
expression e;
type T;
@@

{
... when any
struct i x;
... when any
when exists
when != e = (T)&x
- memset
+ memzero_explicit
  (&x,
-0,
  ...)
... when != x
when strict
}
// 

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


[PATCH 4/4] usbip: remove unneeded structure

2014-11-30 Thread Julia Lawall
From: Julia Lawall 

Delete a local structure that is only used to be initialized by memset.

A semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// 
@@
identifier x,i;
@@

{
... when any
-struct i x;
<+... when != x
- memset(&x,...);
...+>
}
// 

Signed-off-by: Julia Lawall 

---
 tools/usb/usbip/src/usbipd.c |2 --
 1 file changed, 2 deletions(-)

diff --git a/tools/usb/usbip/src/usbipd.c b/tools/usb/usbip/src/usbipd.c
index 2f87f2d..2a7cd2b 100644
--- a/tools/usb/usbip/src/usbipd.c
+++ b/tools/usb/usbip/src/usbipd.c
@@ -91,7 +91,6 @@ static void usbipd_help(void)
 static int recv_request_import(int sockfd)
 {
struct op_import_request req;
-   struct op_common reply;
struct usbip_exported_device *edev;
struct usbip_usb_device pdu_udev;
struct list_head *i;
@@ -100,7 +99,6 @@ static int recv_request_import(int sockfd)
int rc;
 
memset(&req, 0, sizeof(req));
-   memset(&reply, 0, sizeof(reply));
 
rc = usbip_net_recv(sockfd, &req, sizeof(req));
if (rc < 0) {

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


[PATCH 0/4] remove unneeded array

2014-11-30 Thread Julia Lawall
Remove an array or structure that only serves as the first argument to
memset.  The complete semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// 
@@
identifier x;
type T;
@@

{
... when any
-T x[...];
<+... when != x
- memset(x,...);
...+>
}

@@
identifier x,i;
@@

{
... when any
-struct i x;
<+... when != x
- memset(&x,...);
...+>
}
// 

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


[PATCH 8/20] USB: isp1760: fix misspelling of current function in string

2014-12-07 Thread Julia Lawall
Replace a misspelled function name by %s and then __func__.

The function name starts with isp, not ips.

This was done using Coccinelle, including the use of Levenshtein distance,
as proposed by Rasmus Villemoes.

Signed-off-by: Julia Lawall 

---
The semantic patch is difficult to summarize, but is available in the cover
letter of this patch series.

 drivers/usb/host/isp1760-if.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/isp1760-if.c b/drivers/usb/host/isp1760-if.c
index 09254a4..939d376 100644
--- a/drivers/usb/host/isp1760-if.c
+++ b/drivers/usb/host/isp1760-if.c
@@ -318,7 +318,7 @@ static void isp1761_pci_remove(struct pci_dev *dev)
 
 static void isp1761_pci_shutdown(struct pci_dev *dev)
 {
-   printk(KERN_ERR "ips1761_pci_shutdown\n");
+   printk(KERN_ERR "%s\n", __func__);
 }
 
 static const struct pci_device_id isp1760_plx [] = {

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


[PATCH 18/20] usb: gadget: fix misspelling of current function in string

2014-12-07 Thread Julia Lawall
Replace a misspelled function name by %s and then __func__.

This was done using Coccinelle, including the use of Levenshtein distance,
as proposed by Rasmus Villemoes.

Signed-off-by: Julia Lawall 

---
The semantic patch is difficult to summarize, but is available in the cover
letter of this patch series.

 drivers/usb/gadget/function/f_hid.c  |5 +++--
 drivers/usb/gadget/function/f_midi.c |2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index a904403..259b656 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -520,7 +520,7 @@ static void f_midi_transmit(struct f_midi *midi, struct 
usb_request *req)
req = midi_alloc_ep_req(ep, midi->buflen);
 
if (!req) {
-   ERROR(midi, "gmidi_transmit: alloc_ep_request failed\n");
+   ERROR(midi, "%s: alloc_ep_request failed\n", __func__);
return;
}
req->length = 0;
diff --git a/drivers/usb/gadget/function/f_hid.c 
b/drivers/usb/gadget/function/f_hid.c
index 6e04e30..a1bc3e3 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -399,8 +399,9 @@ static int hidg_setup(struct usb_function *f,
value   = __le16_to_cpu(ctrl->wValue);
length  = __le16_to_cpu(ctrl->wLength);
 
-   VDBG(cdev, "hid_setup crtl_request : bRequestType:0x%x bRequest:0x%x "
-   "Value:0x%x\n", ctrl->bRequestType, ctrl->bRequest, value);
+   VDBG(cdev,
+"%s crtl_request : bRequestType:0x%x bRequest:0x%x Value:0x%x\n",
+__func__, ctrl->bRequestType, ctrl->bRequest, value);
 
switch ((ctrl->bRequestType << 8) | ctrl->bRequest) {
case ((USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE) << 8

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


[PATCH 0/20] fix misspelling of current function in string

2014-12-07 Thread Julia Lawall
These patches replace what appears to be a reference to the name of the
current function but is misspelled in some way by either the name of the
function itself, or by %s and then __func__ in an argument list.

// 
// sudo apt-get install python-pip
// sudo pip install python-Levenshtein
// spatch requires the argument --in-place

virtual after_start

@initialize:ocaml@
@@

let extensible_functions = ref ([] : string list)
let restarted = ref false

let restart _ =
  restarted := true;
  let it = new iteration() in
  it#add_virtual_rule After_start;
  Printf.eprintf "restarting\n";
  it#register()

@initialize:python@
@@
import re
from Levenshtein import distance
mindist = 1 // 1 to find only misspellings
maxdist = 2
ignore_leading = True

// -

@r0@
constant char [] c;
identifier f;
@@

f(...,c,...)

@script:ocaml@
c << r0.c;
f << r0.f;
@@

(if not !restarted then restart());
match Str.split_delim (Str.regexp "%") c with
  _::_::_ ->
if not (List.mem f !extensible_functions)
then extensible_functions := f :: !extensible_functions
| _ -> ()

// -

@r depends on after_start@
constant char [] c;
position p;
identifier f;
@@

f(...,c@p,...)

@script:python flt@
c << r.c;
p << r.p;
matched;
@@

func = p[0].current_element
wpattern = "[a-zA-Z_][a-zA-Z0-9_]*"
if ignore_leading:
   func = func.strip("_")
   wpattern = "[a-zA-Z][a-zA-Z0-9_]*"
lf = len(func)
cocci.include_match(False)
// ignore extremely short function names
if lf > 3:
   words = [w for w in re.findall(wpattern, c) if abs(len(w) - lf) <= maxdist]
   for w in words:
   d = distance(w, func) 
   if mindist <= d and d <= maxdist:
  coccinelle.matched = w
  cocci.include_match(True)
  //print "%s:%d:%s():%d: %s" % (p[0].file, int(p[0].line), func, d, c)
  break

@script:ocaml r2@
c << r.c;
f << r.f;
matched << flt.matched;
fixed;
@@

let pieces = Str.split_delim (Str.regexp_string matched) c in
match pieces with
  [before;after] ->
let preceeding =
  List.length(Str.split (Str.regexp_string "%") before) > 1 in
if preceeding
then Coccilib.include_match false
else
  if List.mem f !extensible_functions
  then fixed := before ^ "%s" ^ after
  else Coccilib.include_match false
| _ -> Coccilib.include_match false

@changed1@
constant char [] r.c;
identifier r2.fixed;
position r.p;
identifier r.f;
@@

f(...,
-c@p
+fixed,__func__
 ,...)

// ---

@s depends on after_start@
constant char [] c;
position p;
identifier f;
@@

f(...,c@p,...)

@script:python flt2@
c << s.c;
p << s.p;
matched;
@@

func = p[0].current_element
wpattern = "[a-zA-Z_][a-zA-Z0-9_]*"
if ignore_leading:
   func = func.strip("_")
   wpattern = "[a-zA-Z][a-zA-Z0-9_]*"
lf = len(func)
cocci.include_match(False)
// ignore extremely short function names
if lf > 3:
   words = [w for w in re.findall(wpattern, c) if abs(len(w) - lf) <= maxdist]
   for w in words:
   d = distance(w, func) 
   if mindist <= d and d <= maxdist:
  coccinelle.matched = w
  cocci.include_match(True)
  //print "%s:%d:%s():%d: %s" % (p[0].file, int(p[0].line), func, d, c)
  break

@script:ocaml s2@
c << s.c;
f << s.f;
p << s.p;
matched << flt2.matched;
fixed;
@@

let ce = (List.hd p).current_element in
let pieces = Str.split_delim (Str.regexp_string matched) c in
match pieces with
  [before;after] ->
let preceeding =
  List.length(Str.split (Str.regexp_string "%") before) > 1 in
if preceeding
then Coccilib.include_match false
else
  if List.mem f !extensible_functions
  then Coccilib.include_match false
  else fixed := before ^ ce ^ after
| _ -> Coccilib.include_match false

@changed2@
constant char [] s.c;
identifier s2.fixed;
position s.p;
identifier s.f;
@@

f(...,
-c@p
+fixed
 ,...)
// 

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


  1   2   >