[PATCH v2 1/3] keyboard: imx_keypad: Use clock_prepare_enable/clk_disable_unprepare()

[PATCH v2 1/3] keyboard: imx_keypad: Use clock_prepare_enable/clk_disable_unprepare()

[Fabio Estevam]

Adapt clock handling to the new i.mx clock framework and fix the following warning:

input: imx-keypad as /devices/platform/imx-keypad/input/input0                  
------------[ cut here ]------------                                            
WARNING: at drivers/clk/clk.c:511 __clk_enable+0x98/0xa8()                      
Modules linked in:                                                              
[<c001a680>] (unwind_backtrace+0x0/0xf4) from [<c002452c>] (warn_slowpath_commo)
[<c002452c>] (warn_slowpath_common+0x48/0x60) from [<c0024560>] (warn_slowpath_)
[<c0024560>] (warn_slowpath_null+0x1c/0x24) from [<c02c4ec4>] (__clk_enable+0x9)
[<c02c4ec4>] (__clk_enable+0x98/0xa8) from [<c02c4ef8>] (clk_enable+0x24/0x5c)  
[<c02c4ef8>] (clk_enable+0x24/0x5c) from [<c027ac6c>] (imx_keypad_open+0x28/0xc)
[<c027ac6c>] (imx_keypad_open+0x28/0xc8) from [<c0274b14>] (input_open_device+0)
[<c0274b14>] (input_open_device+0x78/0xa8) from [<c01ec884>] (kbd_connect+0x60/)
[<c01ec884>] (kbd_connect+0x60/0x80) from [<c0273b94>] (input_attach_handler+0x)
[<c0273b94>] (input_attach_handler+0x220/0x258) from [<c02755d4>] (input_regist)
[<c02755d4>] (input_register_device+0x31c/0x390) from [<c038da1c>] (imx_keypad_)
[<c038da1c>] (imx_keypad_probe+0x2e4/0x3b8) from [<c020326c>] (platform_drv_pro)
[<c020326c>] (platform_drv_probe+0x18/0x1c) from [<c0201f64>] (driver_probe_dev)
[<c0201f64>] (driver_probe_device+0x84/0x210) from [<c020217c>] (__driver_attac)
[<c020217c>] (__driver_attach+0x8c/0x90) from [<c02008f8>] (bus_for_each_dev+0x)
[<c02008f8>] (bus_for_each_dev+0x68/0x90) from [<c0201064>] (bus_add_driver+0xa)
[<c0201064>] (bus_add_driver+0xa4/0x23c) from [<c020275c>] (driver_register+0x7)
[<c020275c>] (driver_register+0x78/0x12c) from [<c00087c0>] (do_one_initcall+0x)
[<c00087c0>] (do_one_initcall+0x34/0x188) from [<c04b9310>] (kernel_init+0xe4/0)
[<c04b9310>] (kernel_init+0xe4/0x1a8) from [<c0015bd8>] (kernel_thread_exit+0x0)
---[ end trace 1d550e891d03d7ce ]--- 

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v1:
- Split the changes into 3 patches
- Handle the error code returne by clk_prepare_enable
- Do not convert to devm_clk_get

 drivers/input/keyboard/imx_keypad.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index 6ee7421..9d57945 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -378,7 +378,7 @@ static void imx_keypad_close(struct input_dev *dev)
 	imx_keypad_inhibit(keypad);
 
 	/* Disable clock unit */
-	clk_disable(keypad->clk);
+	clk_disable_unprepare(keypad->clk);
 }
 
 static int imx_keypad_open(struct input_dev *dev)
@@ -391,7 +391,7 @@ static int imx_keypad_open(struct input_dev *dev)
 	keypad->enabled = true;
 
 	/* Enable the kpp clock */
-	clk_enable(keypad->clk);
+	clk_prepare_enable(keypad->clk);
 	imx_keypad_config(keypad);
 
 	/* Sanity control, not all the rows must be actived now. */
@@ -581,7 +581,7 @@ static int imx_kbd_suspend(struct device *dev)
 	mutex_lock(&input_dev->mutex);
 
 	if (input_dev->users)
-		clk_disable(kbd->clk);
+		clk_disable_unprepare(kbd->clk);
 
 	mutex_unlock(&input_dev->mutex);
 
@@ -603,7 +603,7 @@ static int imx_kbd_resume(struct device *dev)
 	mutex_lock(&input_dev->mutex);
 
 	if (input_dev->users)
-		clk_enable(kbd->clk);
+		clk_prepare_enable(kbd->clk);
 
 	mutex_unlock(&input_dev->mutex);
 
-- 
1.7.1

[PATCH v2 2/3] keyboard: imx_keypad: Adapt the new kpp clock name

[Fabio Estevam]

With the new i.mx clock framework we should pass NULL as the keypad clock name.

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

diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index 9d57945..4830615 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -467,7 +467,7 @@ static int __devinit imx_keypad_probe(struct platform_device *pdev)
 		goto failed_free_priv;
 	}
 
-	keypad->clk = clk_get(&pdev->dev, "kpp");
+	keypad->clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(keypad->clk)) {
 		dev_err(&pdev->dev, "failed to get keypad clock\n");
 		error = PTR_ERR(keypad->clk);
-- 
1.7.1

[PATCH v2 3/3] keyboard: imx_keypad: Check error returned by clk_prepare_enable()

[Fabio Estevam]

Check error returned by clk_prepare_enable().

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/input/keyboard/imx_keypad.c |   17 +++++++++++++----
 1 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index 4830615..bdb8ee7 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -385,13 +385,17 @@ static int imx_keypad_open(struct input_dev *dev)
 {
 	struct imx_keypad *keypad = input_get_drvdata(dev);
 
+	int ret;
 	dev_dbg(&dev->dev, ">%s\n", __func__);
 
 	/* We became active from now */
 	keypad->enabled = true;
 
 	/* Enable the kpp clock */
-	clk_prepare_enable(keypad->clk);
+	ret = clk_prepare_enable(keypad->clk);
+	if (ret)
+		return ret;
+
 	imx_keypad_config(keypad);
 
 	/* Sanity control, not all the rows must be actived now. */
@@ -596,18 +600,23 @@ static int imx_kbd_resume(struct device *dev)
 	struct platform_device *pdev = to_platform_device(dev);
 	struct imx_keypad *kbd = platform_get_drvdata(pdev);
 	struct input_dev *input_dev = kbd->input_dev;
+	int ret = 0;
 
 	if (device_may_wakeup(&pdev->dev))
 		disable_irq_wake(kbd->irq);
 
 	mutex_lock(&input_dev->mutex);
 
-	if (input_dev->users)
-		clk_prepare_enable(kbd->clk);
+	if (input_dev->users) {
+		ret = clk_prepare_enable(kbd->clk);
+		if (ret)
+			goto err_clk;
+	}
 
+err_clk:
 	mutex_unlock(&input_dev->mutex);
 
-	return 0;
+	return ret;
 }
 #endif
 
-- 
1.7.1

Re: [PATCH v2 3/3] keyboard: imx_keypad: Check error returned by clk_prepare_enable()

[Dmitry Torokhov]

On Fri, Jul 06, 2012 at 02:41:54PM -0300, Fabio Estevam wrote:
> Check error returned by clk_prepare_enable().
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  drivers/input/keyboard/imx_keypad.c |   17 +++++++++++++----
>  1 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
> index 4830615..bdb8ee7 100644
> --- a/drivers/input/keyboard/imx_keypad.c
> +++ b/drivers/input/keyboard/imx_keypad.c
> @@ -385,13 +385,17 @@ static int imx_keypad_open(struct input_dev *dev)
>  {
>  	struct imx_keypad *keypad = input_get_drvdata(dev);
>  
> +	int ret;
>  	dev_dbg(&dev->dev, ">%s\n", __func__);
>  
>  	/* We became active from now */
>  	keypad->enabled = true;
>  
>  	/* Enable the kpp clock */
> -	clk_prepare_enable(keypad->clk);
> +	ret = clk_prepare_enable(keypad->clk);
> +	if (ret)
> +		return ret;
> +

That would leave "keypad->enabled = true" if clk_prepare_enable() fails.
You need something like below.

Thanks.

-- 
Dmitry

Input: imx_keypad - check error returned by clk_prepare_enable()

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

Check error returned by clk_prepare_enable().

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

 drivers/input/keyboard/imx_keypad.c |   19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)


diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index 4830615..ff4c0a8 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -384,14 +384,18 @@ static void imx_keypad_close(struct input_dev *dev)
 static int imx_keypad_open(struct input_dev *dev)
 {
 	struct imx_keypad *keypad = input_get_drvdata(dev);
+	int error;
 
 	dev_dbg(&dev->dev, ">%s\n", __func__);
 
+	/* Enable the kpp clock */
+	error = clk_prepare_enable(keypad->clk);
+	if (error)
+		return error;
+
 	/* We became active from now */
 	keypad->enabled = true;
 
-	/* Enable the kpp clock */
-	clk_prepare_enable(keypad->clk);
 	imx_keypad_config(keypad);
 
 	/* Sanity control, not all the rows must be actived now. */
@@ -596,18 +600,23 @@ static int imx_kbd_resume(struct device *dev)
 	struct platform_device *pdev = to_platform_device(dev);
 	struct imx_keypad *kbd = platform_get_drvdata(pdev);
 	struct input_dev *input_dev = kbd->input_dev;
+	int ret = 0;
 
 	if (device_may_wakeup(&pdev->dev))
 		disable_irq_wake(kbd->irq);
 
 	mutex_lock(&input_dev->mutex);
 
-	if (input_dev->users)
-		clk_prepare_enable(kbd->clk);
+	if (input_dev->users) {
+		ret = clk_prepare_enable(kbd->clk);
+		if (ret)
+			goto err_clk;
+	}
 
+err_clk:
 	mutex_unlock(&input_dev->mutex);
 
-	return 0;
+	return ret;
 }
 #endif
 

Re: [PATCH v2 3/3] keyboard: imx_keypad: Check error returned by clk_prepare_enable()

[Fabio Estevam]

Dmitry Torokhov wrote:
 
> That would leave "keypad->enabled = true" if clk_prepare_enable() fails.
> You need something like below.

You are right. Thanks for fixing it.