[PATCH] i2c: pnx: fix warnings caused by enabling unprepared clock

[PATCH] i2c: pnx: fix warnings caused by enabling unprepared clock

[Vladimir Zapolskiy]

If common clock framework is configured, the driver generates a warning,
which is fixed by this change:

    WARNING: CPU: 0 PID: 1 at drivers/clk/clk.c:727 clk_core_enable+0x2c/0xa4()
    Modules linked in:
    CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0-rc2+ #171
    Hardware name: LPC32XX SoC (Flattened Device Tree)
    Backtrace:
    [<>] (dump_backtrace) from [<>] (show_stack+0x18/0x1c)
    [<>] (show_stack) from [<>] (dump_stack+0x20/0x28)
    [<>] (dump_stack) from [<>] (warn_slowpath_common+0x90/0xb8)
    [<>] (warn_slowpath_common) from [<>] (warn_slowpath_null+0x24/0x2c)
    [<>] (warn_slowpath_null) from [<>] (clk_core_enable+0x2c/0xa4)
    [<>] (clk_core_enable) from [<>] (clk_enable+0x24/0x38)
    [<>] (clk_enable) from [<>] (i2c_pnx_probe+0x108/0x2a8)
    [<>] (i2c_pnx_probe) from [<>] (platform_drv_probe+0x50/0xa0)
    [<>] (platform_drv_probe) from [<>] (driver_probe_device+0x18c/0x408)
    [<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94)
    [<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98)
    [<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28)
    [<>] (driver_attach) from [<>] (bus_add_driver+0x11c/0x248)
    [<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8)
    [<>] (driver_register) from [<>] (__platform_driver_register+0x50/0x64)
    [<>] (__platform_driver_register) from [<>] (i2c_adap_pnx_init+0x18/0x20)
    [<>] (i2c_adap_pnx_init) from [<>] (do_one_initcall+0x11c/0x1dc)
    [<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4)
    [<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec)
    [<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24)

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
 drivers/i2c/busses/i2c-pnx.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pnx.c b/drivers/i2c/busses/i2c-pnx.c
index e814a36..6f8b446 100644
--- a/drivers/i2c/busses/i2c-pnx.c
+++ b/drivers/i2c/busses/i2c-pnx.c
@@ -600,7 +600,7 @@ static int i2c_pnx_controller_suspend(struct device *dev)
 {
 	struct i2c_pnx_algo_data *alg_data = dev_get_drvdata(dev);
 
-	clk_disable(alg_data->clk);
+	clk_disable_unprepare(alg_data->clk);
 
 	return 0;
 }
@@ -609,7 +609,7 @@ static int i2c_pnx_controller_resume(struct device *dev)
 {
 	struct i2c_pnx_algo_data *alg_data = dev_get_drvdata(dev);
 
-	return clk_enable(alg_data->clk);
+	return clk_prepare_enable(alg_data->clk);
 }
 
 static SIMPLE_DEV_PM_OPS(i2c_pnx_pm,
@@ -672,7 +672,7 @@ static int i2c_pnx_probe(struct platform_device *pdev)
 	if (IS_ERR(alg_data->ioaddr))
 		return PTR_ERR(alg_data->ioaddr);
 
-	ret = clk_enable(alg_data->clk);
+	ret = clk_prepare_enable(alg_data->clk);
 	if (ret)
 		return ret;
 
@@ -726,7 +726,7 @@ static int i2c_pnx_probe(struct platform_device *pdev)
 	return 0;
 
 out_clock:
-	clk_disable(alg_data->clk);
+	clk_disable_unprepare(alg_data->clk);
 	return ret;
 }
 
@@ -735,7 +735,7 @@ static int i2c_pnx_remove(struct platform_device *pdev)
 	struct i2c_pnx_algo_data *alg_data = platform_get_drvdata(pdev);
 
 	i2c_del_adapter(&alg_data->adapter);
-	clk_disable(alg_data->clk);
+	clk_disable_unprepare(alg_data->clk);
 
 	return 0;
 }
-- 
2.1.4

Re: [PATCH] i2c: pnx: fix warnings caused by enabling unprepared clock

[Shubhrajyoti Datta]

On Sun, Oct 18, 2015 at 12:22 AM, Vladimir Zapolskiy <vz@mleia.com> wrote:
> If common clock framework is configured, the driver generates a warning,
> which is fixed by this change:
Maybe just describe the issue and the fix.
Is it just a warning or  the clk enable doesn't work ?

I feel the crash log in the commit msg is not very informative.

>
>     WARNING: CPU: 0 PID: 1 at drivers/clk/clk.c:727 clk_core_enable+0x2c/0xa4()
>     Modules linked in:
>     CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0-rc2+ #171
>     Hardware name: LPC32XX SoC (Flattened Device Tree)
>     Backtrace:
>     [<>] (dump_backtrace) from [<>] (show_stack+0x18/0x1c)
>     [<>] (show_stack) from [<>] (dump_stack+0x20/0x28)
>     [<>] (dump_stack) from [<>] (warn_slowpath_common+0x90/0xb8)
>     [<>] (warn_slowpath_common) from [<>] (warn_slowpath_null+0x24/0x2c)
>     [<>] (warn_slowpath_null) from [<>] (clk_core_enable+0x2c/0xa4)
>     [<>] (clk_core_enable) from [<>] (clk_enable+0x24/0x38)
>     [<>] (clk_enable) from [<>] (i2c_pnx_probe+0x108/0x2a8)
>     [<>] (i2c_pnx_probe) from [<>] (platform_drv_probe+0x50/0xa0)
>     [<>] (platform_drv_probe) from [<>] (driver_probe_device+0x18c/0x408)
>     [<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94)
>     [<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98)
>     [<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28)
>     [<>] (driver_attach) from [<>] (bus_add_driver+0x11c/0x248)
>     [<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8)
>     [<>] (driver_register) from [<>] (__platform_driver_register+0x50/0x64)
>     [<>] (__platform_driver_register) from [<>] (i2c_adap_pnx_init+0x18/0x20)
>     [<>] (i2c_adap_pnx_init) from [<>] (do_one_initcall+0x11c/0x1dc)
>     [<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4)
>     [<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec)
>     [<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24)
>
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
> ---
>  drivers/i2c/busses/i2c-pnx.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-pnx.c b/drivers/i2c/busses/i2c-pnx.c
> index e814a36..6f8b446 100644
> --- a/drivers/i2c/busses/i2c-pnx.c
> +++ b/drivers/i2c/busses/i2c-pnx.c
> @@ -600,7 +600,7 @@ static int i2c_pnx_controller_suspend(struct device *dev)
>  {
>         struct i2c_pnx_algo_data *alg_data = dev_get_drvdata(dev);
>
> -       clk_disable(alg_data->clk);
> +       clk_disable_unprepare(alg_data->clk);
>
>         return 0;
>  }
> @@ -609,7 +609,7 @@ static int i2c_pnx_controller_resume(struct device *dev)
>  {
>         struct i2c_pnx_algo_data *alg_data = dev_get_drvdata(dev);
>
> -       return clk_enable(alg_data->clk);
> +       return clk_prepare_enable(alg_data->clk);
>  }
>
>  static SIMPLE_DEV_PM_OPS(i2c_pnx_pm,
> @@ -672,7 +672,7 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>         if (IS_ERR(alg_data->ioaddr))
>                 return PTR_ERR(alg_data->ioaddr);
>
> -       ret = clk_enable(alg_data->clk);
> +       ret = clk_prepare_enable(alg_data->clk);
>         if (ret)
>                 return ret;
>
> @@ -726,7 +726,7 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>         return 0;
>
>  out_clock:
> -       clk_disable(alg_data->clk);
> +       clk_disable_unprepare(alg_data->clk);
>         return ret;
>  }
>
> @@ -735,7 +735,7 @@ static int i2c_pnx_remove(struct platform_device *pdev)
>         struct i2c_pnx_algo_data *alg_data = platform_get_drvdata(pdev);
>
>         i2c_del_adapter(&alg_data->adapter);
> -       clk_disable(alg_data->clk);
> +       clk_disable_unprepare(alg_data->clk);
>
>         return 0;
>  }
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: pnx: fix warnings caused by enabling unprepared clock

[Vladimir Zapolskiy]

On 19.10.2015 19:21, Shubhrajyoti Datta wrote:
> On Sun, Oct 18, 2015 at 12:22 AM, Vladimir Zapolskiy <vz@mleia.com> wrote:
>> If common clock framework is configured, the driver generates a warning,
>> which is fixed by this change:
> Maybe just describe the issue and the fix.
> Is it just a warning or  the clk enable doesn't work ?
> 
> I feel the crash log in the commit msg is not very informative.

It is not a crash, it is a WARN_ON(1) backtrace.

The backtrace is informative enough IMHO, because if you check the code
around given drivers/clk/clk.c:727 you may find the rootcause, the
WARN_ON() and that the clock is not enabled as a result.

CLK_COMMON selects HAVE_CLK_PREPARE and all drivers used on platforms
with common clock framework must have clk_prepare/clk_unprepare, this
information is omitted as well-known.

But if you insist, I will improve the commit message, I'm interested in
applying this trivial fix as soon as possible, then concentrate on doing
something more fascinating.

>>
>>     WARNING: CPU: 0 PID: 1 at drivers/clk/clk.c:727 clk_core_enable+0x2c/0xa4()
>>     Modules linked in:
>>     CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0-rc2+ #171
>>     Hardware name: LPC32XX SoC (Flattened Device Tree)
>>     Backtrace:
>>     [<>] (dump_backtrace) from [<>] (show_stack+0x18/0x1c)
>>     [<>] (show_stack) from [<>] (dump_stack+0x20/0x28)
>>     [<>] (dump_stack) from [<>] (warn_slowpath_common+0x90/0xb8)
>>     [<>] (warn_slowpath_common) from [<>] (warn_slowpath_null+0x24/0x2c)
>>     [<>] (warn_slowpath_null) from [<>] (clk_core_enable+0x2c/0xa4)
>>     [<>] (clk_core_enable) from [<>] (clk_enable+0x24/0x38)
>>     [<>] (clk_enable) from [<>] (i2c_pnx_probe+0x108/0x2a8)
>>     [<>] (i2c_pnx_probe) from [<>] (platform_drv_probe+0x50/0xa0)
>>     [<>] (platform_drv_probe) from [<>] (driver_probe_device+0x18c/0x408)
>>     [<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94)
>>     [<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98)
>>     [<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28)
>>     [<>] (driver_attach) from [<>] (bus_add_driver+0x11c/0x248)
>>     [<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8)
>>     [<>] (driver_register) from [<>] (__platform_driver_register+0x50/0x64)
>>     [<>] (__platform_driver_register) from [<>] (i2c_adap_pnx_init+0x18/0x20)
>>     [<>] (i2c_adap_pnx_init) from [<>] (do_one_initcall+0x11c/0x1dc)
>>     [<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4)
>>     [<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec)
>>     [<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24)
>>
>> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
>> ---
>>  drivers/i2c/busses/i2c-pnx.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-pnx.c b/drivers/i2c/busses/i2c-pnx.c
>> index e814a36..6f8b446 100644
>> --- a/drivers/i2c/busses/i2c-pnx.c
>> +++ b/drivers/i2c/busses/i2c-pnx.c
>> @@ -600,7 +600,7 @@ static int i2c_pnx_controller_suspend(struct device *dev)
>>  {
>>         struct i2c_pnx_algo_data *alg_data = dev_get_drvdata(dev);
>>
>> -       clk_disable(alg_data->clk);
>> +       clk_disable_unprepare(alg_data->clk);
>>
>>         return 0;
>>  }
>> @@ -609,7 +609,7 @@ static int i2c_pnx_controller_resume(struct device *dev)
>>  {
>>         struct i2c_pnx_algo_data *alg_data = dev_get_drvdata(dev);
>>
>> -       return clk_enable(alg_data->clk);
>> +       return clk_prepare_enable(alg_data->clk);
>>  }
>>
>>  static SIMPLE_DEV_PM_OPS(i2c_pnx_pm,
>> @@ -672,7 +672,7 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>>         if (IS_ERR(alg_data->ioaddr))
>>                 return PTR_ERR(alg_data->ioaddr);
>>
>> -       ret = clk_enable(alg_data->clk);
>> +       ret = clk_prepare_enable(alg_data->clk);
>>         if (ret)
>>                 return ret;
>>
>> @@ -726,7 +726,7 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>>         return 0;
>>
>>  out_clock:
>> -       clk_disable(alg_data->clk);
>> +       clk_disable_unprepare(alg_data->clk);
>>         return ret;
>>  }
>>
>> @@ -735,7 +735,7 @@ static int i2c_pnx_remove(struct platform_device *pdev)
>>         struct i2c_pnx_algo_data *alg_data = platform_get_drvdata(pdev);
>>
>>         i2c_del_adapter(&alg_data->adapter);
>> -       clk_disable(alg_data->clk);
>> +       clk_disable_unprepare(alg_data->clk);
>>
>>         return 0;
>>  }
>> --
>>

--
With best wishes,
Vladimir

Re: [PATCH] i2c: pnx: fix warnings caused by enabling unprepared clock

[Shubhrajyoti Datta]

On Mon, Oct 19, 2015 at 11:49 PM, Vladimir Zapolskiy <vz@mleia.com> wrote:
> On 19.10.2015 19:21, Shubhrajyoti Datta wrote:
>> On Sun, Oct 18, 2015 at 12:22 AM, Vladimir Zapolskiy <vz@mleia.com> wrote:
>>> If common clock framework is configured, the driver generates a warning,
>>> which is fixed by this change:
>> Maybe just describe the issue and the fix.
>> Is it just a warning or  the clk enable doesn't work ?
>>
>> I feel the crash log in the commit msg is not very informative.
>
> It is not a crash, it is a WARN_ON(1) backtrace.

ok

>
> The backtrace is informative enough IMHO, because if you check the code
> around given drivers/clk/clk.c:727 you may find the rootcause, the
> WARN_ON() and that the clock is not enabled as a result.
>
> CLK_COMMON selects HAVE_CLK_PREPARE and all drivers used on platforms
> with common clock framework must have clk_prepare/clk_unprepare, this
> information is omitted as well-known.
>
> But if you insist, I will improve the commit message, I'm interested in
> applying this trivial fix as soon as possible, then concentrate on doing
> something more fascinating.

My request will be to describe the issue.
Like the probe fails or you access registers with clocks not enabled

Or it is a warning fix and the controller works etc.
>
>>>

Re: [PATCH] i2c: pnx: fix warnings caused by enabling unprepared clock

[Vladimir Zapolskiy]

On 20.10.2015 11:54, Shubhrajyoti Datta wrote:
> On Mon, Oct 19, 2015 at 11:49 PM, Vladimir Zapolskiy <vz@mleia.com> wrote:
>> On 19.10.2015 19:21, Shubhrajyoti Datta wrote:
>>> On Sun, Oct 18, 2015 at 12:22 AM, Vladimir Zapolskiy <vz@mleia.com> wrote:
>>>> If common clock framework is configured, the driver generates a warning,
>>>> which is fixed by this change:
>>> Maybe just describe the issue and the fix.
>>> Is it just a warning or  the clk enable doesn't work ?
>>>
>>> I feel the crash log in the commit msg is not very informative.
>>
>> It is not a crash, it is a WARN_ON(1) backtrace.
> 
> ok
> 
>>
>> The backtrace is informative enough IMHO, because if you check the code
>> around given drivers/clk/clk.c:727 you may find the rootcause, the
>> WARN_ON() and that the clock is not enabled as a result.
>>
>> CLK_COMMON selects HAVE_CLK_PREPARE and all drivers used on platforms
>> with common clock framework must have clk_prepare/clk_unprepare, this
>> information is omitted as well-known.
>>
>> But if you insist, I will improve the commit message, I'm interested in
>> applying this trivial fix as soon as possible, then concentrate on doing
>> something more fascinating.
> 
> My request will be to describe the issue.
> Like the probe fails or you access registers with clocks not enabled
> 
> Or it is a warning fix and the controller works etc.

If I change initial description to the one below, would you be satisfied
with it?

The driver can not be used on a platform with common clock framework
until clk_prepare/clk_unprepare calls are added, otherwise clk_enable
calls will fail and a warning is generated.

--
With best wishes,
Vladimir

Re: [PATCH] i2c: pnx: fix warnings caused by enabling unprepared clock

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1941 bytes --]

On Sat, Oct 17, 2015 at 09:52:27PM +0300, Vladimir Zapolskiy wrote:
> If common clock framework is configured, the driver generates a warning,
> which is fixed by this change:
> 
>     WARNING: CPU: 0 PID: 1 at drivers/clk/clk.c:727 clk_core_enable+0x2c/0xa4()
>     Modules linked in:
>     CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0-rc2+ #171
>     Hardware name: LPC32XX SoC (Flattened Device Tree)
>     Backtrace:
>     [<>] (dump_backtrace) from [<>] (show_stack+0x18/0x1c)
>     [<>] (show_stack) from [<>] (dump_stack+0x20/0x28)
>     [<>] (dump_stack) from [<>] (warn_slowpath_common+0x90/0xb8)
>     [<>] (warn_slowpath_common) from [<>] (warn_slowpath_null+0x24/0x2c)
>     [<>] (warn_slowpath_null) from [<>] (clk_core_enable+0x2c/0xa4)
>     [<>] (clk_core_enable) from [<>] (clk_enable+0x24/0x38)
>     [<>] (clk_enable) from [<>] (i2c_pnx_probe+0x108/0x2a8)
>     [<>] (i2c_pnx_probe) from [<>] (platform_drv_probe+0x50/0xa0)
>     [<>] (platform_drv_probe) from [<>] (driver_probe_device+0x18c/0x408)
>     [<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94)
>     [<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98)
>     [<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28)
>     [<>] (driver_attach) from [<>] (bus_add_driver+0x11c/0x248)
>     [<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8)
>     [<>] (driver_register) from [<>] (__platform_driver_register+0x50/0x64)
>     [<>] (__platform_driver_register) from [<>] (i2c_adap_pnx_init+0x18/0x20)
>     [<>] (i2c_adap_pnx_init) from [<>] (do_one_initcall+0x11c/0x1dc)
>     [<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4)
>     [<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec)
>     [<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24)
> 
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>

Applied to for-next with your updated commit message, thanks!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] i2c: pnx: fix warnings caused by enabling unprepared clock

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 109 bytes --]

> Applied to for-next with your updated commit message, thanks!

Actually, applied to for-current, thanks!



[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]