* [PATCH 1/2] mtd: delete unneeded call to platform_get_drvdata
@ 2014-05-17 6:32 Julia Lawall
2014-05-17 7:07 ` Fabio Estevam
0 siblings, 1 reply; 5+ messages in thread
From: Julia Lawall @ 2014-05-17 6:32 UTC (permalink / raw)
To: David Woodhouse; +Cc: linux-mtd, Brian Norris, kernel-janitors, linux-kernel
From: Julia Lawall <Julia.Lawall@lip6.fr>
Platform_get_drvdata is an accessor function, and has no purpose if its
result is not used.
The semantic patch that fixes this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
identifier x;
type T;
@@
- T x = platform_get_drvdata(...);
... when != x
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/mtd/nand/bf5xx_nand.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
index b7a2494..b5fbd48 100644
--- a/drivers/mtd/nand/bf5xx_nand.c
+++ b/drivers/mtd/nand/bf5xx_nand.c
@@ -840,15 +840,11 @@ out_err_kzalloc:
static int bf5xx_nand_suspend(struct platform_device *dev, pm_message_t pm)
{
- struct bf5xx_nand_info *info = platform_get_drvdata(dev);
-
return 0;
}
static int bf5xx_nand_resume(struct platform_device *dev)
{
- struct bf5xx_nand_info *info = platform_get_drvdata(dev);
-
return 0;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] mtd: delete unneeded call to platform_get_drvdata
2014-05-17 6:32 [PATCH 1/2] mtd: delete unneeded call to platform_get_drvdata Julia Lawall
@ 2014-05-17 7:07 ` Fabio Estevam
2014-05-17 7:12 ` Julia Lawall
0 siblings, 1 reply; 5+ messages in thread
From: Fabio Estevam @ 2014-05-17 7:07 UTC (permalink / raw)
To: Julia Lawall
Cc: linux-mtd@lists.infradead.org, kernel-janitors, Brian Norris,
David Woodhouse, linux-kernel
On Sat, May 17, 2014 at 3:32 AM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> Platform_get_drvdata is an accessor function, and has no purpose if its
> result is not used.
>
> The semantic patch that fixes this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> identifier x;
> type T;
> @@
> - T x = platform_get_drvdata(...);
> ... when != x
> // </smpl>
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> ---
> drivers/mtd/nand/bf5xx_nand.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
> index b7a2494..b5fbd48 100644
> --- a/drivers/mtd/nand/bf5xx_nand.c
> +++ b/drivers/mtd/nand/bf5xx_nand.c
> @@ -840,15 +840,11 @@ out_err_kzalloc:
>
> static int bf5xx_nand_suspend(struct platform_device *dev, pm_message_t pm)
> {
> - struct bf5xx_nand_info *info = platform_get_drvdata(dev);
> -
> return 0;
> }
>
> static int bf5xx_nand_resume(struct platform_device *dev)
> {
> - struct bf5xx_nand_info *info = platform_get_drvdata(dev);
> -
> return 0;
In this case bf5xx_nand_suspend/resume could be removed?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] mtd: delete unneeded call to platform_get_drvdata
2014-05-17 7:07 ` Fabio Estevam
@ 2014-05-17 7:12 ` Julia Lawall
2014-05-20 8:35 ` Brian Norris
0 siblings, 1 reply; 5+ messages in thread
From: Julia Lawall @ 2014-05-17 7:12 UTC (permalink / raw)
To: Fabio Estevam
Cc: linux-mtd@lists.infradead.org, kernel-janitors, Brian Norris,
David Woodhouse, linux-kernel
On Sat, 17 May 2014, Fabio Estevam wrote:
> On Sat, May 17, 2014 at 3:32 AM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > Platform_get_drvdata is an accessor function, and has no purpose if its
> > result is not used.
> >
> > The semantic patch that fixes this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @@
> > identifier x;
> > type T;
> > @@
> > - T x = platform_get_drvdata(...);
> > ... when != x
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > ---
> > drivers/mtd/nand/bf5xx_nand.c | 4 ----
> > 1 file changed, 4 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
> > index b7a2494..b5fbd48 100644
> > --- a/drivers/mtd/nand/bf5xx_nand.c
> > +++ b/drivers/mtd/nand/bf5xx_nand.c
> > @@ -840,15 +840,11 @@ out_err_kzalloc:
> >
> > static int bf5xx_nand_suspend(struct platform_device *dev, pm_message_t pm)
> > {
> > - struct bf5xx_nand_info *info = platform_get_drvdata(dev);
> > -
> > return 0;
> > }
> >
> > static int bf5xx_nand_resume(struct platform_device *dev)
> > {
> > - struct bf5xx_nand_info *info = platform_get_drvdata(dev);
> > -
> > return 0;
>
> In this case bf5xx_nand_suspend/resume could be removed?
I don't know. It looks like it is intentional to have a definition that
returns an indication of success? The complete set of definitions is:
#ifdef CONFIG_PM
static int bf5xx_nand_suspend(struct platform_device *dev, pm_message_t
pm)
{
struct bf5xx_nand_info *info = platform_get_drvdata(dev);
return 0;
}
static int bf5xx_nand_resume(struct platform_device *dev)
{
struct bf5xx_nand_info *info = platform_get_drvdata(dev);
return 0;
}
#else
#define bf5xx_nand_suspend NULL
#define bf5xx_nand_resume NULL
#endif
julia
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] mtd: delete unneeded call to platform_get_drvdata
2014-05-17 7:12 ` Julia Lawall
@ 2014-05-20 8:35 ` Brian Norris
2014-05-20 9:15 ` Dan Carpenter
0 siblings, 1 reply; 5+ messages in thread
From: Brian Norris @ 2014-05-20 8:35 UTC (permalink / raw)
To: Julia Lawall
Cc: Mike Frysinger, Fabio Estevam, kernel-janitors, linux-kernel,
linux-mtd@lists.infradead.org, David Woodhouse
+ Mike
On Sat, May 17, 2014 at 03:12:02PM +0800, Julia Lawall wrote:
> On Sat, 17 May 2014, Fabio Estevam wrote:
> > On Sat, May 17, 2014 at 3:32 AM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> > > From: Julia Lawall <Julia.Lawall@lip6.fr>
> > >
> > > Platform_get_drvdata is an accessor function, and has no purpose if its
> > > result is not used.
> > >
> > > The semantic patch that fixes this problem is as follows:
> > > (http://coccinelle.lip6.fr/)
> > >
> > > // <smpl>
> > > @@
> > > identifier x;
> > > type T;
> > > @@
> > > - T x = platform_get_drvdata(...);
> > > ... when != x
> > > // </smpl>
> > >
> > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> > >
> > > ---
> > > drivers/mtd/nand/bf5xx_nand.c | 4 ----
> > > 1 file changed, 4 deletions(-)
> > >
> > > diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
> > > index b7a2494..b5fbd48 100644
> > > --- a/drivers/mtd/nand/bf5xx_nand.c
> > > +++ b/drivers/mtd/nand/bf5xx_nand.c
> > > @@ -840,15 +840,11 @@ out_err_kzalloc:
> > >
> > > static int bf5xx_nand_suspend(struct platform_device *dev, pm_message_t pm)
> > > {
> > > - struct bf5xx_nand_info *info = platform_get_drvdata(dev);
> > > -
> > > return 0;
> > > }
> > >
> > > static int bf5xx_nand_resume(struct platform_device *dev)
> > > {
> > > - struct bf5xx_nand_info *info = platform_get_drvdata(dev);
> > > -
> > > return 0;
> >
> > In this case bf5xx_nand_suspend/resume could be removed?
>
> I don't know. It looks like it is intentional to have a definition that
> returns an indication of success? The complete set of definitions is:
I'm not so sure. I believe suspend/resume works just fine without the
hooks, if those hooks would otherwise be doing nothing. It is notable
that this driver is not using the modern dev_pm_ops form of the
suspend/resume callbacks, so maybe it just hasn't aged gracefully--and
possibly was never supported properly in the first place.
I'll either take this patch as-is, or look to killing off the
suspend/resume callbacks entirely here, depending on Mike's feedback.
> #ifdef CONFIG_PM
>
> static int bf5xx_nand_suspend(struct platform_device *dev, pm_message_t
> pm)
> {
> struct bf5xx_nand_info *info = platform_get_drvdata(dev);
>
> return 0;
> }
>
> static int bf5xx_nand_resume(struct platform_device *dev)
> {
> struct bf5xx_nand_info *info = platform_get_drvdata(dev);
>
> return 0;
> }
>
> #else
> #define bf5xx_nand_suspend NULL
> #define bf5xx_nand_resume NULL
> #endif
Thanks,
Brian
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] mtd: delete unneeded call to platform_get_drvdata
2014-05-20 8:35 ` Brian Norris
@ 2014-05-20 9:15 ` Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2014-05-20 9:15 UTC (permalink / raw)
To: Brian Norris
Cc: Mike Frysinger, David Woodhouse, kernel-janitors, linux-kernel,
Julia Lawall, linux-mtd@lists.infradead.org, Fabio Estevam
On Tue, May 20, 2014 at 01:35:31AM -0700, Brian Norris wrote:
> > > > static int bf5xx_nand_resume(struct platform_device *dev)
> > > > {
> > > > - struct bf5xx_nand_info *info = platform_get_drvdata(dev);
> > > > -
> > > > return 0;
> > >
> > > In this case bf5xx_nand_suspend/resume could be removed?
> >
> > I don't know. It looks like it is intentional to have a definition that
> > returns an indication of success? The complete set of definitions is:
>
> I'm not so sure. I believe suspend/resume works just fine without the
> hooks, if those hooks would otherwise be doing nothing. It is notable
> that this driver is not using the modern dev_pm_ops form of the
> suspend/resume callbacks, so maybe it just hasn't aged gracefully--and
> possibly was never supported properly in the first place.
>
These are called from platform_legacy_suspend/platform_legacy_resume.
It looks like the functions are not needed.
(I am not an expert on pm, I just querried my Smatch Cross Function
Database which is awesome).
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-05-20 9:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-17 6:32 [PATCH 1/2] mtd: delete unneeded call to platform_get_drvdata Julia Lawall
2014-05-17 7:07 ` Fabio Estevam
2014-05-17 7:12 ` Julia Lawall
2014-05-20 8:35 ` Brian Norris
2014-05-20 9:15 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).