[PATCH 0/4] input: sirfsoc-onkey - a bundle of minor clean or fix

[PATCH 0/4] input: sirfsoc-onkey - a bundle of minor clean or fix

[Barry Song]

From: Barry Song <Baohua.Song@csr.com>

Hi Dmitry,
this patchset depends on Xianglong's
"[PATCH v2] input: sirfsoc-onkey - report onkey untouch event by
 detecting pin status"

Barry Song (2):
  input: sirfsoc-onkey - drop the IRQF_SHARED flag
  input: sirfsoc-onkey - update copyright years to 2014

Xianglong Du (2):
  input: sirfsoc-onkey - namespace pwrc_resume function
  input: sirfsoc-onkey - use dev_get_drvdata instead of
    platform_get_drvdata

 drivers/input/misc/sirfsoc-onkey.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

-- 
1.7.5.4

[PATCH 1/4] input: sirfsoc-onkey - drop the IRQF_SHARED flag

[Barry Song]

From: Barry Song <Baohua.Song@csr.com>

since the irq handler always return IRQ_HANDLED, it means this irq is not
a shared IRQ at all. or at least, the SW is not self-consistent now.

Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/input/misc/sirfsoc-onkey.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/input/misc/sirfsoc-onkey.c b/drivers/input/misc/sirfsoc-onkey.c
index 4d13903..4d54744 100644
--- a/drivers/input/misc/sirfsoc-onkey.c
+++ b/drivers/input/misc/sirfsoc-onkey.c
@@ -114,7 +114,7 @@ static int sirfsoc_pwrc_probe(struct platform_device *pdev)
 
 	pwrcdrv->irq = platform_get_irq(pdev, 0);
 	error = devm_request_irq(&pdev->dev, pwrcdrv->irq,
-				 sirfsoc_pwrc_isr, IRQF_SHARED,
+				 sirfsoc_pwrc_isr, 0,
 				 "sirfsoc_pwrc_int", pwrcdrv);
 	if (error) {
 		dev_err(&pdev->dev, "unable to claim irq %d, error: %d\n",
-- 
1.7.5.4

[PATCH 2/4] input: sirfsoc-onkey - namespace pwrc_resume function

[Barry Song]

From: Xianglong Du <Xianglong.Du@csr.com>

this function lost namespace, this patch fixes it.

Signed-off-by: Xianglong Du <Xianglong.Du@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/input/misc/sirfsoc-onkey.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/sirfsoc-onkey.c b/drivers/input/misc/sirfsoc-onkey.c
index 4d54744..097c10a 100644
--- a/drivers/input/misc/sirfsoc-onkey.c
+++ b/drivers/input/misc/sirfsoc-onkey.c
@@ -155,7 +155,7 @@ static int sirfsoc_pwrc_remove(struct platform_device *pdev)
 }
 
 #ifdef CONFIG_PM_SLEEP
-static int pwrc_resume(struct device *dev)
+static int sirfsoc_pwrc_resume(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct sirfsoc_pwrc_drvdata *pwrcdrv = platform_get_drvdata(pdev);
@@ -173,7 +173,7 @@ static int pwrc_resume(struct device *dev)
 }
 #endif
 
-static SIMPLE_DEV_PM_OPS(sirfsoc_pwrc_pm_ops, NULL, pwrc_resume);
+static SIMPLE_DEV_PM_OPS(sirfsoc_pwrc_pm_ops, NULL, sirfsoc_pwrc_resume);
 
 static struct platform_driver sirfsoc_pwrc_driver = {
 	.probe		= sirfsoc_pwrc_probe,
-- 
1.7.5.4

[PATCH 3/4] input: sirfsoc-onkey - use dev_get_drvdata instead of platform_get_drvdata

[Barry Song]

From: Xianglong Du <Xianglong.Du@csr.com>

In resume entry, use dev_get_drvdata() instead of to_platform_device(dev) +
platform_get_drvdata(pdev).

Signed-off-by: Xianglong Du <Xianglong.Du@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/input/misc/sirfsoc-onkey.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/sirfsoc-onkey.c b/drivers/input/misc/sirfsoc-onkey.c
index 097c10a..8e45bf11 100644
--- a/drivers/input/misc/sirfsoc-onkey.c
+++ b/drivers/input/misc/sirfsoc-onkey.c
@@ -157,8 +157,7 @@ static int sirfsoc_pwrc_remove(struct platform_device *pdev)
 #ifdef CONFIG_PM_SLEEP
 static int sirfsoc_pwrc_resume(struct device *dev)
 {
-	struct platform_device *pdev = to_platform_device(dev);
-	struct sirfsoc_pwrc_drvdata *pwrcdrv = platform_get_drvdata(pdev);
+	struct sirfsoc_pwrc_drvdata *pwrcdrv = dev_get_drvdata(dev);
 
 	/*
 	 * Do not mask pwrc interrupt as we want pwrc work as a wakeup source
-- 
1.7.5.4

[PATCH 4/4] input: sirfsoc-onkey - update copyright years to 2014

[Barry Song]

From: Barry Song <Baohua.Song@csr.com>

Happy the year of horse, 2014.

                             ,((((^`\
                            ((((  (6 \
                          ,((((( ,    \
      ,,,_              ,(((((  /"._  ,`,
     ((((\\ ,...       ,((((   /    `-.-'
     )))  ;'    `"'"'""((((   (
    (((  /            (((      \
     )) |                      |
    ((  |        .       '     |
    ))  \     _ '      `t   ,.')
    (   |   y;- -,-""'"-.\   \/
    )   / ./  ) /         `\  \
       |./   ( (           / /'
       ||     \\          //'|
       ||      \\       _//'||
       ||       ))     |_/  ||
       \_\     |_/          ||
       `'"                  \_\
                            `'"
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/input/misc/sirfsoc-onkey.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/input/misc/sirfsoc-onkey.c b/drivers/input/misc/sirfsoc-onkey.c
index 8e45bf11..1a11207 100644
--- a/drivers/input/misc/sirfsoc-onkey.c
+++ b/drivers/input/misc/sirfsoc-onkey.c
@@ -1,7 +1,8 @@
 /*
  * Power key driver for SiRF PrimaII
  *
- * Copyright (c) 2013 Cambridge Silicon Radio Limited, a CSR plc group company.
+ * Copyright (c) 2013 - 2014 Cambridge Silicon Radio Limited, a CSR plc group
+ * company.
  *
  * Licensed under GPLv2 or later.
  */
-- 
1.7.5.4

Re: [PATCH 3/4] input: sirfsoc-onkey - use dev_get_drvdata instead of platform_get_drvdata

[Dmitry Torokhov]

On Thu, Feb 13, 2014 at 02:40:23PM +0800, Barry Song wrote:
> From: Xianglong Du <Xianglong.Du@csr.com>
> 
> In resume entry, use dev_get_drvdata() instead of to_platform_device(dev) +
> platform_get_drvdata(pdev).
> 
> Signed-off-by: Xianglong Du <Xianglong.Du@csr.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---
>  drivers/input/misc/sirfsoc-onkey.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/misc/sirfsoc-onkey.c b/drivers/input/misc/sirfsoc-onkey.c
> index 097c10a..8e45bf11 100644
> --- a/drivers/input/misc/sirfsoc-onkey.c
> +++ b/drivers/input/misc/sirfsoc-onkey.c
> @@ -157,8 +157,7 @@ static int sirfsoc_pwrc_remove(struct platform_device *pdev)
>  #ifdef CONFIG_PM_SLEEP
>  static int sirfsoc_pwrc_resume(struct device *dev)
>  {
> -	struct platform_device *pdev = to_platform_device(dev);
> -	struct sirfsoc_pwrc_drvdata *pwrcdrv = platform_get_drvdata(pdev);
> +	struct sirfsoc_pwrc_drvdata *pwrcdrv = dev_get_drvdata(dev);

I believe we should use accessors matching the object type. Currently
dev_get_drvdata and platform_get_drvdata return the same object, but
they do not have to.

IOW I prefer the original code.

Thanks.

-- 
Dmitry

Re: [PATCH 3/4] input: sirfsoc-onkey - use dev_get_drvdata instead of platform_get_drvdata

[Barry Song]

2014-02-13 15:24 GMT+08:00 Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> On Thu, Feb 13, 2014 at 02:40:23PM +0800, Barry Song wrote:
>> From: Xianglong Du <Xianglong.Du@csr.com>
>>
>> In resume entry, use dev_get_drvdata() instead of to_platform_device(dev) +
>> platform_get_drvdata(pdev).
>>
>> Signed-off-by: Xianglong Du <Xianglong.Du@csr.com>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> ---
>>  drivers/input/misc/sirfsoc-onkey.c |    3 +--
>>  1 files changed, 1 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/input/misc/sirfsoc-onkey.c b/drivers/input/misc/sirfsoc-onkey.c
>> index 097c10a..8e45bf11 100644
>> --- a/drivers/input/misc/sirfsoc-onkey.c
>> +++ b/drivers/input/misc/sirfsoc-onkey.c
>> @@ -157,8 +157,7 @@ static int sirfsoc_pwrc_remove(struct platform_device *pdev)
>>  #ifdef CONFIG_PM_SLEEP
>>  static int sirfsoc_pwrc_resume(struct device *dev)
>>  {
>> -     struct platform_device *pdev = to_platform_device(dev);
>> -     struct sirfsoc_pwrc_drvdata *pwrcdrv = platform_get_drvdata(pdev);
>> +     struct sirfsoc_pwrc_drvdata *pwrcdrv = dev_get_drvdata(dev);
>
> I believe we should use accessors matching the object type. Currently
> dev_get_drvdata and platform_get_drvdata return the same object, but
> they do not have to.
>
> IOW I prefer the original code.

i did see many commits in kernel which did same jobs with this one. e.g:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a1216394e620d0dfbb03c712ae3210e7b77c9e11
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=bab8b563ef08455440badca7fe79b2c700bd1b75
...

in my understand, the changed context is a general pm_ops to general
"driver" and not a legacy suspend/resume in "platform_driver" bound
with pdev, so in the context, we are caring about "device" more than
"pdev".

how do you think if i do a more change in probe() with this by:

- platform_set_drvdata(pdev, pwrcdrv);
+ dev_set_drvdata(&pdev->dev, pwrcdrv);

would this make everything look fine?

>
> Thanks.
>
> --
> Dmitry

-barry

Re: [PATCH 3/4] input: sirfsoc-onkey - use dev_get_drvdata instead of platform_get_drvdata

[Dmitry Torokhov]

On Thu, Feb 13, 2014 at 03:47:33PM +0800, Barry Song wrote:
> 2014-02-13 15:24 GMT+08:00 Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > On Thu, Feb 13, 2014 at 02:40:23PM +0800, Barry Song wrote:
> >> From: Xianglong Du <Xianglong.Du@csr.com>
> >>
> >> In resume entry, use dev_get_drvdata() instead of to_platform_device(dev) +
> >> platform_get_drvdata(pdev).
> >>
> >> Signed-off-by: Xianglong Du <Xianglong.Du@csr.com>
> >> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> >> ---
> >>  drivers/input/misc/sirfsoc-onkey.c |    3 +--
> >>  1 files changed, 1 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/input/misc/sirfsoc-onkey.c b/drivers/input/misc/sirfsoc-onkey.c
> >> index 097c10a..8e45bf11 100644
> >> --- a/drivers/input/misc/sirfsoc-onkey.c
> >> +++ b/drivers/input/misc/sirfsoc-onkey.c
> >> @@ -157,8 +157,7 @@ static int sirfsoc_pwrc_remove(struct platform_device *pdev)
> >>  #ifdef CONFIG_PM_SLEEP
> >>  static int sirfsoc_pwrc_resume(struct device *dev)
> >>  {
> >> -     struct platform_device *pdev = to_platform_device(dev);
> >> -     struct sirfsoc_pwrc_drvdata *pwrcdrv = platform_get_drvdata(pdev);
> >> +     struct sirfsoc_pwrc_drvdata *pwrcdrv = dev_get_drvdata(dev);
> >
> > I believe we should use accessors matching the object type. Currently
> > dev_get_drvdata and platform_get_drvdata return the same object, but
> > they do not have to.
> >
> > IOW I prefer the original code.
> 
> i did see many commits in kernel which did same jobs with this one. e.g:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a1216394e620d0dfbb03c712ae3210e7b77c9e11
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=bab8b563ef08455440badca7fe79b2c700bd1b75
> ...
> 
> in my understand, the changed context is a general pm_ops to general
> "driver" and not a legacy suspend/resume in "platform_driver" bound
> with pdev, so in the context, we are caring about "device" more than
> "pdev".

It is more about who owns the data - generic device or platform device?

> 
> how do you think if i do a more change in probe() with this by:
> 
> - platform_set_drvdata(pdev, pwrcdrv);
> + dev_set_drvdata(&pdev->dev, pwrcdrv);
> 
> would this make everything look fine?

Yes, it would, since we would now know that it is generic driver layer
that has ownership of this data item.

Thanks.

-- 
Dmitry