[PATCH] Input: imx_keypad - Place SIMPLE_DEV_PM_OPS inside CONFIG_PM_SLEEP if block

[PATCH] Input: imx_keypad - Place SIMPLE_DEV_PM_OPS inside CONFIG_PM_SLEEP if block

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

SIMPLE_DEV_PM_OPS should be inside the CONFIG_PM_SLEEP 'if' block because
imx_kbd_suspend and imx_kbd_resume definitions are inside this block.

While at it, also add a definition for IMX_KBD_PM_OPS, which can handle the case
when CONFIG_PM_SLEEP is not selected.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/input/keyboard/imx_keypad.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index 03c8cc5..2663c91 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -580,15 +580,17 @@ err_clk:
 
 	return ret;
 }
-#endif
-
 static SIMPLE_DEV_PM_OPS(imx_kbd_pm_ops, imx_kbd_suspend, imx_kbd_resume);
+#define IMX_KBD_PM_OPS (&imx_kbd_pm_ops)
+#else
+#define IMX_KBD_PM_OPS NULL
+#endif /* CONFIG_PM_SLEEP */
 
 static struct platform_driver imx_keypad_driver = {
 	.driver		= {
 		.name	= "imx-keypad",
 		.owner	= THIS_MODULE,
-		.pm	= &imx_kbd_pm_ops,
+		.pm	= IMX_KBD_PM_OPS,
 		.of_match_table = of_match_ptr(imx_keypad_of_match),
 	},
 	.probe		= imx_keypad_probe,
-- 
1.7.9.5

Re: [PATCH] Input: imx_keypad - Place SIMPLE_DEV_PM_OPS inside CONFIG_PM_SLEEP if block

[Dmitry Torokhov]

Fabio Estevam <festevam@gmail.com> wrote:

>From: Fabio Estevam <fabio.estevam@freescale.com>
>
>SIMPLE_DEV_PM_OPS should be inside the CONFIG_PM_SLEEP 'if' block
>because
>imx_kbd_suspend and imx_kbd_resume definitions are inside this block.

No, it does not. It compiles just fine if CONFIG_PM_SLEEP is not defined.

>
>While at it, also add a definition for IMX_KBD_PM_OPS, which can handle
>the case
>when CONFIG_PM_SLEEP is not selected.
>
>Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
>---
> drivers/input/keyboard/imx_keypad.c |    8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/input/keyboard/imx_keypad.c
>b/drivers/input/keyboard/imx_keypad.c
>index 03c8cc5..2663c91 100644
>--- a/drivers/input/keyboard/imx_keypad.c
>+++ b/drivers/input/keyboard/imx_keypad.c
>@@ -580,15 +580,17 @@ err_clk:
> 
> 	return ret;
> }
>-#endif
>-
>static SIMPLE_DEV_PM_OPS(imx_kbd_pm_ops, imx_kbd_suspend,
>imx_kbd_resume);
>+#define IMX_KBD_PM_OPS (&imx_kbd_pm_ops)
>+#else
>+#define IMX_KBD_PM_OPS NULL
>+#endif /* CONFIG_PM_SLEEP */
> 
> static struct platform_driver imx_keypad_driver = {
> 	.driver		= {
> 		.name	= "imx-keypad",
> 		.owner	= THIS_MODULE,
>-		.pm	= &imx_kbd_pm_ops,
>+		.pm	= IMX_KBD_PM_OPS,
> 		.of_match_table = of_match_ptr(imx_keypad_of_match),
> 	},
> 	.probe		= imx_keypad_probe,

Hi Fabio,
Thanks.

-- 
Dmitry

Re: [PATCH] Input: imx_keypad - Place SIMPLE_DEV_PM_OPS inside CONFIG_PM_SLEEP if block

[Dmitry Torokhov]

On Wednesday, April 10, 2013 10:28:22 PM Dmitry Torokhov wrote:
> Fabio Estevam <festevam@gmail.com> wrote:
> >From: Fabio Estevam <fabio.estevam@freescale.com>
> >
> >SIMPLE_DEV_PM_OPS should be inside the CONFIG_PM_SLEEP 'if' block
> >because
> >imx_kbd_suspend and imx_kbd_resume definitions are inside this block.
> 
> No, it does not. It compiles just fine if CONFIG_PM_SLEEP is not defined.
> 
> >While at it, also add a definition for IMX_KBD_PM_OPS, which can handle
> >the case
> >when CONFIG_PM_SLEEP is not selected.
> >
> >Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> >---
> >
> > drivers/input/keyboard/imx_keypad.c |    8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> >diff --git a/drivers/input/keyboard/imx_keypad.c
> >b/drivers/input/keyboard/imx_keypad.c
> >index 03c8cc5..2663c91 100644
> >--- a/drivers/input/keyboard/imx_keypad.c
> >+++ b/drivers/input/keyboard/imx_keypad.c
> >
> >@@ -580,15 +580,17 @@ err_clk:
> > 	return ret;
> > 
> > }
> >
> >-#endif
> >-
> >static SIMPLE_DEV_PM_OPS(imx_kbd_pm_ops, imx_kbd_suspend,
> >imx_kbd_resume);
> >+#define IMX_KBD_PM_OPS (&imx_kbd_pm_ops)
> >+#else
> >+#define IMX_KBD_PM_OPS NULL
> >+#endif /* CONFIG_PM_SLEEP */
> >
> > static struct platform_driver imx_keypad_driver = {
> > 
> > 	.driver		= {
> > 	
> > 		.name	= "imx-keypad",
> > 		.owner	= THIS_MODULE,
> >
> >-		.pm	= &imx_kbd_pm_ops,
> >+		.pm	= IMX_KBD_PM_OPS,
> >
> > 		.of_match_table = of_match_ptr(imx_keypad_of_match),
> > 	
> > 	},
> > 	.probe		= imx_keypad_probe,
> 
> Hi Fabio,
> Thanks.

Hmm, K-9's quoting style is really peculiar...

-- 
Dmitry

Re: [PATCH] Input: imx_keypad - Place SIMPLE_DEV_PM_OPS inside CONFIG_PM_SLEEP if block

[Fabio Estevam]

Hi Dmitry,

On Thu, Apr 11, 2013 at 2:28 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Fabio Estevam <festevam@gmail.com> wrote:
>
>>From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>>SIMPLE_DEV_PM_OPS should be inside the CONFIG_PM_SLEEP 'if' block
>>because
>>imx_kbd_suspend and imx_kbd_resume definitions are inside this block.
>
> No, it does not. It compiles just fine if CONFIG_PM_SLEEP is not defined.

I am not sure how to disable CONFIG_PM_SLEEP properly in the
defconfig, but I did this quick test to simulate it:

--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -536,7 +536,7 @@ static int imx_keypad_probe(struct platform_device *pdev)
        return 0;
 }

-#ifdef CONFIG_PM_SLEEP
+#if 0
 static int imx_kbd_suspend(struct device *dev)
 {
        struct platform_device *pdev = to_platform_device(dev);


and then I see:

  CC      drivers/input/keyboard/imx_keypad.o
drivers/input/keyboard/imx_keypad.c:585: error: 'imx_kbd_suspend'
undeclared here (not in a function)
drivers/input/keyboard/imx_keypad.c:585: error: 'imx_kbd_resume'
undeclared here (not in a function)
make[3]: *** [drivers/input/keyboard/imx_keypad.o] Error 1
make[2]: *** [drivers/input/keyboard] Error 2

Re: [PATCH] Input: imx_keypad - Place SIMPLE_DEV_PM_OPS inside CONFIG_PM_SLEEP if block

[Dmitry Torokhov]

Fabio Estevam <festevam@gmail.com> wrote:

>Hi Dmitry,
>
>On Thu, Apr 11, 2013 at 2:28 AM, Dmitry Torokhov
><dmitry.torokhov@gmail.com> wrote:
>> Fabio Estevam <festevam@gmail.com> wrote:
>>
>>>From: Fabio Estevam <fabio.estevam@freescale.com>
>>>
>>>SIMPLE_DEV_PM_OPS should be inside the CONFIG_PM_SLEEP 'if' block
>>>because
>>>imx_kbd_suspend and imx_kbd_resume definitions are inside this block.
>>
>> No, it does not. It compiles just fine if CONFIG_PM_SLEEP is not
>defined.
>
>I am not sure how to disable CONFIG_PM_SLEEP properly in the
>defconfig, but I did this quick test to simulate it:
>
>--- a/drivers/input/keyboard/imx_keypad.c
>+++ b/drivers/input/keyboard/imx_keypad.c
>@@ -536,7 +536,7 @@ static int imx_keypad_probe(struct platform_device
>*pdev)
>        return 0;
> }
>
>-#ifdef CONFIG_PM_SLEEP
>+#if 0

This is not the same thing as SIMPLE_DEV_PM_OPS change their definition depending on CONFIG_PM_SLEEP.


Thanks.

-- 
Dmitry