input: ambakmi: Fix system PM by converting to modern callbacks

Message ID 1429008108-29295-1-git-send-email-ulf.hansson@linaro.org
State Accepted
Commit cee3d8ccbecb8af6788edaaac46befca78b000dc
Headers show

Commit Message

Ulf Hansson April 14, 2015, 10:41 a.m.
The legacy system PM support has long time ago been dropped from the
AMBA bus. Align to that by converting to the modern system PM
callbacks.

Fixes: 26825cfd90f9 (ARM: 7914/1: amba: Drop legacy PM support ...)
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/input/serio/ambakmi.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Ulf Hansson April 15, 2015, 11:54 a.m. | #1
[...]

>
> Had I not noticed your message, you would have probably ended up removing
> a driver which is very much in use, is functional, and therefore is not
> as broken as Ulf claims.

I claimed the system PM needed to be fixed, nothing else. :-)

>
> The other lesson to come away from this is that just because someone claims
> something is broken does *not* make it broken.  It means _they_ are seeing
> some problem which maybe no one else is seeing.  Again, that's no basis to
> jump on the "lets remove the whole driver then" bandwagon.
>
> And I doubt that Ulf even has the hardware to be able to test this change,
> which makes it even worse.

Correct, this patch is only compile time tested.

Actually I found it when I just realized that commit 26825cfd90f9
("ARM: 7914/1: amba: Drop legacy PM support and use the pm_generic
functions"), missed to remove the legacy system PM callbacks from the
struct amba_driver, as it should have.

I intended to send a patch for the above when I found these legacy
callbacks still being used. Somehow I missed this once when removing
the legacy system PM support in the amba bus. Sorry about that.

Kind regards
Uffe
Ulf Hansson May 22, 2015, 2:37 p.m. | #2
On 14 April 2015 at 12:41, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> The legacy system PM support has long time ago been dropped from the
> AMBA bus. Align to that by converting to the modern system PM
> callbacks.
>
> Fixes: 26825cfd90f9 (ARM: 7914/1: amba: Drop legacy PM support ...)
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

What happened with this one? Can we queue it for 4.2?

Kind regards
Uffe

> ---
>  drivers/input/serio/ambakmi.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/serio/ambakmi.c b/drivers/input/serio/ambakmi.c
> index 8b748d9..058e1d8 100644
> --- a/drivers/input/serio/ambakmi.c
> +++ b/drivers/input/serio/ambakmi.c
> @@ -175,15 +175,19 @@ static int amba_kmi_remove(struct amba_device *dev)
>         return 0;
>  }
>
> -static int amba_kmi_resume(struct amba_device *dev)
> +#ifdef CONFIG_PM_SLEEP
> +static int amba_kmi_resume(struct device *dev)
>  {
> -       struct amba_kmi_port *kmi = amba_get_drvdata(dev);
> +       struct amba_kmi_port *kmi = dev_get_drvdata(dev);
>
>         /* kick the serio layer to rescan this port */
>         serio_reconnect(kmi->io);
>
>         return 0;
>  }
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(amba_kmi_dev_pm_ops, NULL, amba_kmi_resume);
>
>  static struct amba_id amba_kmi_idtable[] = {
>         {
> @@ -199,11 +203,11 @@ static struct amba_driver ambakmi_driver = {
>         .drv            = {
>                 .name   = "kmi-pl050",
>                 .owner  = THIS_MODULE,
> +               .pm     = &amba_kmi_dev_pm_ops,
>         },
>         .id_table       = amba_kmi_idtable,
>         .probe          = amba_kmi_probe,
>         .remove         = amba_kmi_remove,
> -       .resume         = amba_kmi_resume,
>  };
>
>  module_amba_driver(ambakmi_driver);
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson May 26, 2015, 1:50 p.m. | #3
On 22 May 2015 at 16:39, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, May 22, 2015 at 04:37:12PM +0200, Ulf Hansson wrote:
>> On 14 April 2015 at 12:41, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> > The legacy system PM support has long time ago been dropped from the
>> > AMBA bus. Align to that by converting to the modern system PM
>> > callbacks.
>> >
>> > Fixes: 26825cfd90f9 (ARM: 7914/1: amba: Drop legacy PM support ...)
>> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>
>> What happened with this one? Can we queue it for 4.2?
>
> Why does it have a Fixes: tag?  Is it fixing a real bug, or is it just
> fixing a warning that the kernel prints?

I would expect it to fix a real bug, but I am not able to test this on HW.

Since the commit 26825cfd90f9, the amba bus doesn't invoke the legacy
system PM callbacks any more. Instead it uses pm_generic_resume() as
its ->resume() callback which means amba_kmi_resume() won't be invoked
during system PM resume.

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson July 16, 2015, 8:48 a.m. | #4
On 22 May 2015 at 18:49, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Fri, May 22, 2015 at 04:37:12PM +0200, Ulf Hansson wrote:
>> On 14 April 2015 at 12:41, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> > The legacy system PM support has long time ago been dropped from the
>> > AMBA bus. Align to that by converting to the modern system PM
>> > callbacks.
>> >
>> > Fixes: 26825cfd90f9 (ARM: 7914/1: amba: Drop legacy PM support ...)
>> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>
>> What happened with this one? Can we queue it for 4.2?
>
> I was hoping that Russel could give it a spin since he has the
> relevant hardware.
>

Hmm, as there are still no progress could we give it a try and queue
it to get some test in linux-next? I think the patch is trivial and it
would be nice to get it queued as a fix.

Kind regards
Uffe

Patch

diff --git a/drivers/input/serio/ambakmi.c b/drivers/input/serio/ambakmi.c
index 8b748d9..058e1d8 100644
--- a/drivers/input/serio/ambakmi.c
+++ b/drivers/input/serio/ambakmi.c
@@ -175,15 +175,19 @@  static int amba_kmi_remove(struct amba_device *dev)
 	return 0;
 }
 
-static int amba_kmi_resume(struct amba_device *dev)
+#ifdef CONFIG_PM_SLEEP
+static int amba_kmi_resume(struct device *dev)
 {
-	struct amba_kmi_port *kmi = amba_get_drvdata(dev);
+	struct amba_kmi_port *kmi = dev_get_drvdata(dev);
 
 	/* kick the serio layer to rescan this port */
 	serio_reconnect(kmi->io);
 
 	return 0;
 }
+#endif
+
+static SIMPLE_DEV_PM_OPS(amba_kmi_dev_pm_ops, NULL, amba_kmi_resume);
 
 static struct amba_id amba_kmi_idtable[] = {
 	{
@@ -199,11 +203,11 @@  static struct amba_driver ambakmi_driver = {
 	.drv		= {
 		.name	= "kmi-pl050",
 		.owner	= THIS_MODULE,
+		.pm	= &amba_kmi_dev_pm_ops,
 	},
 	.id_table	= amba_kmi_idtable,
 	.probe		= amba_kmi_probe,
 	.remove		= amba_kmi_remove,
-	.resume		= amba_kmi_resume,
 };
 
 module_amba_driver(ambakmi_driver);