* [Xenomai] [PATCH 1/1] rtcan_flexcan: with open firmware, use devm_clk_put instead of clk_put
@ 2015-09-25 13:22 Thierry Bultel
2015-09-25 16:16 ` Michael Haberler
2015-10-02 17:34 ` Gilles Chanteperdrix
0 siblings, 2 replies; 6+ messages in thread
From: Thierry Bultel @ 2015-09-25 13:22 UTC (permalink / raw)
To: xenomai; +Cc: Thierry Bultel
Signed-off-by: Thierry Bultel <thierry.bultel@basystemes.fr>
---
ksrc/drivers/can/rtcan_flexcan.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/ksrc/drivers/can/rtcan_flexcan.c b/ksrc/drivers/can/rtcan_flexcan.c
index f5477db..91c5ebf 100644
--- a/ksrc/drivers/can/rtcan_flexcan.c
+++ b/ksrc/drivers/can/rtcan_flexcan.c
@@ -235,6 +235,7 @@ struct flexcan_priv {
struct regulator *reg_xceiver;
struct clk *clk_ipg;
struct clk *clk_per;
+ struct platform_device *pdev;
#endif
};
@@ -1054,8 +1055,8 @@ static void put_clocks(struct flexcan_priv *priv)
#if LINUX_VERSION_CODE < KERNEL_VERSION(3,11,0)
clk_put(priv->clk);
#else
- clk_put(priv->clk_per);
- clk_put(priv->clk_ipg);
+ devm_clk_put(&priv->pdev->dev,priv->clk_per);
+ devm_clk_put(&priv->pdev->dev,priv->clk_ipg);
#endif
}
@@ -1132,6 +1133,7 @@ static int flexcan_probe(struct platform_device *pdev)
}
clock_freq = clk_get_rate(priv->clk_per);
}
+ priv->pdev = pdev;
#endif
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Xenomai] [PATCH 1/1] rtcan_flexcan: with open firmware, use devm_clk_put instead of clk_put
2015-09-25 13:22 [Xenomai] [PATCH 1/1] rtcan_flexcan: with open firmware, use devm_clk_put instead of clk_put Thierry Bultel
@ 2015-09-25 16:16 ` Michael Haberler
2015-09-25 17:19 ` Thierry Bultel
2015-10-02 17:34 ` Gilles Chanteperdrix
1 sibling, 1 reply; 6+ messages in thread
From: Michael Haberler @ 2015-09-25 16:16 UTC (permalink / raw)
To: Thierry Bultel; +Cc: xenomai
Hi Thiery,
could you give a bit of context about this patch?
what problem does the patch resolve?
what is 'open firmware'
which kernel version did you apply this?
which platform?
- Michael
> Am 25.09.2015 um 15:22 schrieb Thierry Bultel <thierry.bultel@basystemes.fr>:
>
> Signed-off-by: Thierry Bultel <thierry.bultel@basystemes.fr>
> ---
> ksrc/drivers/can/rtcan_flexcan.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/ksrc/drivers/can/rtcan_flexcan.c b/ksrc/drivers/can/rtcan_flexcan.c
> index f5477db..91c5ebf 100644
> --- a/ksrc/drivers/can/rtcan_flexcan.c
> +++ b/ksrc/drivers/can/rtcan_flexcan.c
> @@ -235,6 +235,7 @@ struct flexcan_priv {
> struct regulator *reg_xceiver;
> struct clk *clk_ipg;
> struct clk *clk_per;
> + struct platform_device *pdev;
> #endif
> };
>
> @@ -1054,8 +1055,8 @@ static void put_clocks(struct flexcan_priv *priv)
> #if LINUX_VERSION_CODE < KERNEL_VERSION(3,11,0)
> clk_put(priv->clk);
> #else
> - clk_put(priv->clk_per);
> - clk_put(priv->clk_ipg);
> + devm_clk_put(&priv->pdev->dev,priv->clk_per);
> + devm_clk_put(&priv->pdev->dev,priv->clk_ipg);
> #endif
> }
>
> @@ -1132,6 +1133,7 @@ static int flexcan_probe(struct platform_device *pdev)
> }
> clock_freq = clk_get_rate(priv->clk_per);
> }
> + priv->pdev = pdev;
> #endif
>
> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> --
> 1.9.1
>
>
> _______________________________________________
> Xenomai mailing list
> Xenomai@xenomai.org
> http://xenomai.org/mailman/listinfo/xenomai
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Xenomai] [PATCH 1/1] rtcan_flexcan: with open firmware, use devm_clk_put instead of clk_put
2015-09-25 16:16 ` Michael Haberler
@ 2015-09-25 17:19 ` Thierry Bultel
0 siblings, 0 replies; 6+ messages in thread
From: Thierry Bultel @ 2015-09-25 17:19 UTC (permalink / raw)
To: Michael Haberler; +Cc: xenomai
Le 25/09/2015 18:16, Michael Haberler a écrit :
> Hi Thiery,
Hi Michael,
>
> could you give a bit of context about this patch?
>
> what problem does the patch resolve?
When the rtcan_flexcan driver is loaded as a module, the module
unloading does not perform all the mandatory cleanup.
The symptom is that when loading the module again thereafter,
the kernel complains about the clocks:
root@CF_thierry:~>insmod /boot/xeno_can_flexcan.ko
root@CF_thierry:~>dmesg -c
[ 86.611247] ------------[ cut here ]------------
[ 86.611276] WARNING: CPU: 1 PID: 2805 at include/linux/kref.h:47
__clk_get+0x80/0x90()
[ 86.611282] Modules linked in: xeno_can_flexcan(+) [last unloaded:
xeno_can_flexcan]
[ 86.611304] CPU: 1 PID: 2805 Comm: insmod Not tainted 3.18.12_4.1.0 #2
[ 86.611338] [<80016550>] (unwind_backtrace) from [<80012194>]
(show_stack+0x10/0x14)
[ 86.611362] [<80012194>] (show_stack) from [<806fba78>]
(dump_stack+0x84/0xc4)
[ 86.611377] [<806fba78>] (dump_stack) from [<80022a90>]
(warn_slowpath_common+0x74/0x90)
[ 86.611389] [<80022a90>] (warn_slowpath_common) from [<80022b48>]
(warn_slowpath_null+0x1c/0x24)
[ 86.611402] [<80022b48>] (warn_slowpath_null) from [<80532750>]
(__clk_get+0x80/0x90)
[ 86.611417] [<80532750>] (__clk_get) from [<8052f7cc>]
(of_clk_get_by_clkspec+0x2c/0x48)
[ 86.611429] [<8052f7cc>] (of_clk_get_by_clkspec) from [<8052f820>]
(of_clk_get.part.2+0x38/0x40)
[ 86.611441] [<8052f820>] (of_clk_get.part.2) from [<8052f8cc>]
(of_clk_get_by_name+0x90/0xcc)
[ 86.611452] [<8052f8cc>] (of_clk_get_by_name) from [<8052f92c>]
(clk_get+0x24/0x54)
[ 86.611464] [<8052f92c>] (clk_get) from [<8052f508>]
(devm_clk_get+0x34/0x70)
[ 86.611485] [<8052f508>] (devm_clk_get) from [<7f007a20>]
(flexcan_probe+0xcc/0x548 [xeno_can_flexcan])
[ 86.611511] [<7f007a20>] (flexcan_probe [xeno_can_flexcan]) from
[<803e993c>] (platform_drv_probe+0x44/0xa4)
[ 86.611526] [<803e993c>] (platform_drv_probe) from [<803e7f80>]
(driver_probe_device+0x108/0x23c)
[ 86.611538] [<803e7f80>] (driver_probe_device) from [<803e8184>]
(__driver_attach+0x8c/0x90)
[ 86.611549] [<803e8184>] (__driver_attach) from [<803e6534>]
(bus_for_each_dev+0x6c/0xa0)
[ 86.611560] [<803e6534>] (bus_for_each_dev) from [<803e77a8>]
(bus_add_driver+0x14c/0x1f4)
[ 86.611571] [<803e77a8>] (bus_add_driver) from [<803e89e0>]
(driver_register+0x78/0xf8)
[ 86.611582] [<803e89e0>] (driver_register) from [<80008cb0>]
(do_one_initcall+0x8c/0x1c4)
[ 86.611596] [<80008cb0>] (do_one_initcall) from [<80085c34>]
(load_module+0x17f4/0x2034)
[ 86.611607] [<80085c34>] (load_module) from [<800865cc>]
(SyS_finit_module+0x68/0x78)
[ 86.611619] [<800865cc>] (SyS_finit_module) from [<8000ebe0>]
(ret_fast_syscall+0x0/0x30)
[ 86.611627] ---[ end trace bf01e794993cef27 ]---
> what is 'open firmware'
right. I should probably reword my commit,
this not dependent of CONFIG_OF but just with the kernel
version. The bug happens with version >= KERNEL_VERSION(3,11,0)
FYI
> which kernel version did you apply this?
this is a 3.18.12
> which platform?
IMX6q SOC.
Thierry
>
> - Michael
>
>> Am 25.09.2015 um 15:22 schrieb Thierry Bultel <thierry.bultel@basystemes.fr>:
>>
>> Signed-off-by: Thierry Bultel <thierry.bultel@basystemes.fr>
>> ---
>> ksrc/drivers/can/rtcan_flexcan.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/ksrc/drivers/can/rtcan_flexcan.c b/ksrc/drivers/can/rtcan_flexcan.c
>> index f5477db..91c5ebf 100644
>> --- a/ksrc/drivers/can/rtcan_flexcan.c
>> +++ b/ksrc/drivers/can/rtcan_flexcan.c
>> @@ -235,6 +235,7 @@ struct flexcan_priv {
>> struct regulator *reg_xceiver;
>> struct clk *clk_ipg;
>> struct clk *clk_per;
>> + struct platform_device *pdev;
>> #endif
>> };
>>
>> @@ -1054,8 +1055,8 @@ static void put_clocks(struct flexcan_priv *priv)
>> #if LINUX_VERSION_CODE < KERNEL_VERSION(3,11,0)
>> clk_put(priv->clk);
>> #else
>> - clk_put(priv->clk_per);
>> - clk_put(priv->clk_ipg);
>> + devm_clk_put(&priv->pdev->dev,priv->clk_per);
>> + devm_clk_put(&priv->pdev->dev,priv->clk_ipg);
>> #endif
>> }
>>
>> @@ -1132,6 +1133,7 @@ static int flexcan_probe(struct platform_device *pdev)
>> }
>> clock_freq = clk_get_rate(priv->clk_per);
>> }
>> + priv->pdev = pdev;
>> #endif
>>
>> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> Xenomai mailing list
>> Xenomai@xenomai.org
>> http://xenomai.org/mailman/listinfo/xenomai
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Xenomai] [PATCH 1/1] rtcan_flexcan: with open firmware, use devm_clk_put instead of clk_put
2015-09-25 13:22 [Xenomai] [PATCH 1/1] rtcan_flexcan: with open firmware, use devm_clk_put instead of clk_put Thierry Bultel
2015-09-25 16:16 ` Michael Haberler
@ 2015-10-02 17:34 ` Gilles Chanteperdrix
2015-10-03 12:24 ` Thierry Bultel
1 sibling, 1 reply; 6+ messages in thread
From: Gilles Chanteperdrix @ 2015-10-02 17:34 UTC (permalink / raw)
To: Thierry Bultel; +Cc: xenomai
On Fri, Sep 25, 2015 at 03:22:50PM +0200, Thierry Bultel wrote:
> Signed-off-by: Thierry Bultel <thierry.bultel@basystemes.fr>
> ---
> ksrc/drivers/can/rtcan_flexcan.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/ksrc/drivers/can/rtcan_flexcan.c b/ksrc/drivers/can/rtcan_flexcan.c
> index f5477db..91c5ebf 100644
> --- a/ksrc/drivers/can/rtcan_flexcan.c
> +++ b/ksrc/drivers/can/rtcan_flexcan.c
> @@ -235,6 +235,7 @@ struct flexcan_priv {
> struct regulator *reg_xceiver;
> struct clk *clk_ipg;
> struct clk *clk_per;
> + struct platform_device *pdev;
> #endif
> };
>
> @@ -1054,8 +1055,8 @@ static void put_clocks(struct flexcan_priv *priv)
> #if LINUX_VERSION_CODE < KERNEL_VERSION(3,11,0)
> clk_put(priv->clk);
> #else
> - clk_put(priv->clk_per);
> - clk_put(priv->clk_ipg);
> + devm_clk_put(&priv->pdev->dev,priv->clk_per);
> + devm_clk_put(&priv->pdev->dev,priv->clk_ipg);
> #endif
> }
>
> @@ -1132,6 +1133,7 @@ static int flexcan_probe(struct platform_device *pdev)
> }
> clock_freq = clk_get_rate(priv->clk_per);
> }
> + priv->pdev = pdev;
> #endif
>
> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
I think you can completely drop the calls to clk_put: with
devm_clk_get, the clocks should be automatically put when the device
is destroyed. At least, this is what the doc says:
http://lxr.free-electrons.com/source/include/linux/clk.h#L219
Could you try the following patch, instead?
diff --git a/ksrc/drivers/can/rtcan_flexcan.c b/ksrc/drivers/can/rtcan_flexcan.c
index f5477db..ddc3708 100644
--- a/ksrc/drivers/can/rtcan_flexcan.c
+++ b/ksrc/drivers/can/rtcan_flexcan.c
@@ -1053,9 +1053,6 @@ static void put_clocks(struct flexcan_priv *priv)
{
#if LINUX_VERSION_CODE < KERNEL_VERSION(3,11,0)
clk_put(priv->clk);
-#else
- clk_put(priv->clk_per);
- clk_put(priv->clk_ipg);
#endif
}
--
Gilles.
https://click-hack.org
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Xenomai] [PATCH 1/1] rtcan_flexcan: with open firmware, use devm_clk_put instead of clk_put
2015-10-02 17:34 ` Gilles Chanteperdrix
@ 2015-10-03 12:24 ` Thierry Bultel
2015-10-04 11:47 ` Gilles Chanteperdrix
0 siblings, 1 reply; 6+ messages in thread
From: Thierry Bultel @ 2015-10-03 12:24 UTC (permalink / raw)
To: Gilles Chanteperdrix; +Cc: xenomai
Le 02/10/2015 19:34, Gilles Chanteperdrix a écrit :
> On Fri, Sep 25, 2015 at 03:22:50PM +0200, Thierry Bultel wrote:
>> Signed-off-by: Thierry Bultel <thierry.bultel@basystemes.fr>
>> ---
>> ksrc/drivers/can/rtcan_flexcan.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/ksrc/drivers/can/rtcan_flexcan.c b/ksrc/drivers/can/rtcan_flexcan.c
>> index f5477db..91c5ebf 100644
>> --- a/ksrc/drivers/can/rtcan_flexcan.c
>> +++ b/ksrc/drivers/can/rtcan_flexcan.c
>> @@ -235,6 +235,7 @@ struct flexcan_priv {
>> struct regulator *reg_xceiver;
>> struct clk *clk_ipg;
>> struct clk *clk_per;
>> + struct platform_device *pdev;
>> #endif
>> };
>>
>> @@ -1054,8 +1055,8 @@ static void put_clocks(struct flexcan_priv *priv)
>> #if LINUX_VERSION_CODE < KERNEL_VERSION(3,11,0)
>> clk_put(priv->clk);
>> #else
>> - clk_put(priv->clk_per);
>> - clk_put(priv->clk_ipg);
>> + devm_clk_put(&priv->pdev->dev,priv->clk_per);
>> + devm_clk_put(&priv->pdev->dev,priv->clk_ipg);
>> #endif
>> }
>>
>> @@ -1132,6 +1133,7 @@ static int flexcan_probe(struct platform_device *pdev)
>> }
>> clock_freq = clk_get_rate(priv->clk_per);
>> }
>> + priv->pdev = pdev;
>> #endif
>>
>> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
> I think you can completely drop the calls to clk_put: with
> devm_clk_get, the clocks should be automatically put when the device
> is destroyed. At least, this is what the doc says:
> http://lxr.free-electrons.com/source/include/linux/clk.h#L219
>
> Could you try the following patch, instead?
This is enough indeed, thanks.
Will you commit it ?
Thierry
>
> diff --git a/ksrc/drivers/can/rtcan_flexcan.c b/ksrc/drivers/can/rtcan_flexcan.c
> index f5477db..ddc3708 100644
> --- a/ksrc/drivers/can/rtcan_flexcan.c
> +++ b/ksrc/drivers/can/rtcan_flexcan.c
> @@ -1053,9 +1053,6 @@ static void put_clocks(struct flexcan_priv *priv)
> {
> #if LINUX_VERSION_CODE < KERNEL_VERSION(3,11,0)
> clk_put(priv->clk);
> -#else
> - clk_put(priv->clk_per);
> - clk_put(priv->clk_ipg);
> #endif
> }
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Xenomai] [PATCH 1/1] rtcan_flexcan: with open firmware, use devm_clk_put instead of clk_put
2015-10-03 12:24 ` Thierry Bultel
@ 2015-10-04 11:47 ` Gilles Chanteperdrix
0 siblings, 0 replies; 6+ messages in thread
From: Gilles Chanteperdrix @ 2015-10-04 11:47 UTC (permalink / raw)
To: Thierry Bultel; +Cc: xenomai
On Sat, Oct 03, 2015 at 02:24:53PM +0200, Thierry Bultel wrote:
>
>
> Le 02/10/2015 19:34, Gilles Chanteperdrix a écrit :
> > On Fri, Sep 25, 2015 at 03:22:50PM +0200, Thierry Bultel wrote:
> >> Signed-off-by: Thierry Bultel <thierry.bultel@basystemes.fr>
> >> ---
> >> ksrc/drivers/can/rtcan_flexcan.c | 6 ++++--
> >> 1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/ksrc/drivers/can/rtcan_flexcan.c b/ksrc/drivers/can/rtcan_flexcan.c
> >> index f5477db..91c5ebf 100644
> >> --- a/ksrc/drivers/can/rtcan_flexcan.c
> >> +++ b/ksrc/drivers/can/rtcan_flexcan.c
> >> @@ -235,6 +235,7 @@ struct flexcan_priv {
> >> struct regulator *reg_xceiver;
> >> struct clk *clk_ipg;
> >> struct clk *clk_per;
> >> + struct platform_device *pdev;
> >> #endif
> >> };
> >>
> >> @@ -1054,8 +1055,8 @@ static void put_clocks(struct flexcan_priv *priv)
> >> #if LINUX_VERSION_CODE < KERNEL_VERSION(3,11,0)
> >> clk_put(priv->clk);
> >> #else
> >> - clk_put(priv->clk_per);
> >> - clk_put(priv->clk_ipg);
> >> + devm_clk_put(&priv->pdev->dev,priv->clk_per);
> >> + devm_clk_put(&priv->pdev->dev,priv->clk_ipg);
> >> #endif
> >> }
> >>
> >> @@ -1132,6 +1133,7 @@ static int flexcan_probe(struct platform_device *pdev)
> >> }
> >> clock_freq = clk_get_rate(priv->clk_per);
> >> }
> >> + priv->pdev = pdev;
> >> #endif
> >>
> >> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >
> > I think you can completely drop the calls to clk_put: with
> > devm_clk_get, the clocks should be automatically put when the device
> > is destroyed. At least, this is what the doc says:
> > http://lxr.free-electrons.com/source/include/linux/clk.h#L219
> >
> > Could you try the following patch, instead?
>
> This is enough indeed, thanks.
> Will you commit it ?
It is merged. Thanks for testing.
--
Gilles.
https://click-hack.org
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-10-04 11:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-25 13:22 [Xenomai] [PATCH 1/1] rtcan_flexcan: with open firmware, use devm_clk_put instead of clk_put Thierry Bultel
2015-09-25 16:16 ` Michael Haberler
2015-09-25 17:19 ` Thierry Bultel
2015-10-02 17:34 ` Gilles Chanteperdrix
2015-10-03 12:24 ` Thierry Bultel
2015-10-04 11:47 ` Gilles Chanteperdrix
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.