* [PATCH v2] net, can, ti_hecc: fix a run time warn_on.
@ 2015-10-20 14:39 Anton Glukhov
2015-10-20 14:46 ` Marc Kleine-Budde
0 siblings, 1 reply; 4+ messages in thread
From: Anton Glukhov @ 2015-10-20 14:39 UTC (permalink / raw)
To: linux-can; +Cc: mkl, hs, netdev, Anton Glukhov
This patch fixes a warning in clk_enable by calling
clk_prepare_enable instead.
Signed-off-by: Heiko Schocher <hs@denx.de>
Signed-off-by: Anton Glukhov <anton.a.glukhov@gmail.com>
---
drivers/net/can/ti_hecc.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index cf345cb..77dd170 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -951,7 +951,7 @@ static int ti_hecc_probe(struct platform_device *pdev)
netif_napi_add(ndev, &priv->napi, ti_hecc_rx_poll,
HECC_DEF_NAPI_WEIGHT);
- clk_enable(priv->clk);
+ clk_prepare_enable(priv->clk);
err = register_candev(ndev);
if (err) {
dev_err(&pdev->dev, "register_candev() failed\n");
@@ -966,7 +966,8 @@ static int ti_hecc_probe(struct platform_device *pdev)
return 0;
probe_exit_clk:
- clk_put(priv->clk);
+ clk_disable_unprepare(priv->clk);
+ priv->clk = NULL;
probe_exit_candev:
free_candev(ndev);
probe_exit_iounmap:
@@ -984,8 +985,8 @@ static int ti_hecc_remove(struct platform_device *pdev)
struct ti_hecc_priv *priv = netdev_priv(ndev);
unregister_candev(ndev);
- clk_disable(priv->clk);
- clk_put(priv->clk);
+ clk_disable_unprepare(priv->clk);
+ priv->clk = NULL;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
iounmap(priv->base);
release_mem_region(res->start, resource_size(res));
@@ -1009,7 +1010,7 @@ static int ti_hecc_suspend(struct platform_device *pdev, pm_message_t state)
hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
priv->can.state = CAN_STATE_SLEEPING;
- clk_disable(priv->clk);
+ clk_disable_unprepare(priv->clk);
return 0;
}
@@ -1019,7 +1020,7 @@ static int ti_hecc_resume(struct platform_device *pdev)
struct net_device *dev = platform_get_drvdata(pdev);
struct ti_hecc_priv *priv = netdev_priv(dev);
- clk_enable(priv->clk);
+ clk_prepare_enable(priv->clk);
hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
priv->can.state = CAN_STATE_ERROR_ACTIVE;
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] net, can, ti_hecc: fix a run time warn_on.
2015-10-20 14:39 [PATCH v2] net, can, ti_hecc: fix a run time warn_on Anton Glukhov
@ 2015-10-20 14:46 ` Marc Kleine-Budde
2015-10-20 16:30 ` Anton.Glukhov
0 siblings, 1 reply; 4+ messages in thread
From: Marc Kleine-Budde @ 2015-10-20 14:46 UTC (permalink / raw)
To: Anton Glukhov, linux-can; +Cc: hs, netdev
[-- Attachment #1: Type: text/plain, Size: 2699 bytes --]
On 10/20/2015 04:39 PM, Anton Glukhov wrote:
> This patch fixes a warning in clk_enable by calling
> clk_prepare_enable instead.
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Signed-off-by: Anton Glukhov <anton.a.glukhov@gmail.com>
> ---
> drivers/net/can/ti_hecc.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
> index cf345cb..77dd170 100644
> --- a/drivers/net/can/ti_hecc.c
> +++ b/drivers/net/can/ti_hecc.c
> @@ -951,7 +951,7 @@ static int ti_hecc_probe(struct platform_device *pdev)
> netif_napi_add(ndev, &priv->napi, ti_hecc_rx_poll,
> HECC_DEF_NAPI_WEIGHT);
>
> - clk_enable(priv->clk);
> + clk_prepare_enable(priv->clk);
please add error checking
> err = register_candev(ndev);
> if (err) {
> dev_err(&pdev->dev, "register_candev() failed\n");
> @@ -966,7 +966,8 @@ static int ti_hecc_probe(struct platform_device *pdev)
> return 0;
>
> probe_exit_clk:
> - clk_put(priv->clk);
Why do you remove the clk_put()?
> + clk_disable_unprepare(priv->clk);
> + priv->clk = NULL;
The NULLing of priv->clk is not needed.
> probe_exit_candev:
> free_candev(ndev);
> probe_exit_iounmap:
> @@ -984,8 +985,8 @@ static int ti_hecc_remove(struct platform_device *pdev)
> struct ti_hecc_priv *priv = netdev_priv(ndev);
>
> unregister_candev(ndev);
> - clk_disable(priv->clk);
> - clk_put(priv->clk);
Why do you remoe the clk_put()?
> + clk_disable_unprepare(priv->clk);
> + priv->clk = NULL;
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> iounmap(priv->base);
> release_mem_region(res->start, resource_size(res));
> @@ -1009,7 +1010,7 @@ static int ti_hecc_suspend(struct platform_device *pdev, pm_message_t state)
> hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
> priv->can.state = CAN_STATE_SLEEPING;
>
> - clk_disable(priv->clk);
> + clk_disable_unprepare(priv->clk);
>
> return 0;
> }
> @@ -1019,7 +1020,7 @@ static int ti_hecc_resume(struct platform_device *pdev)
> struct net_device *dev = platform_get_drvdata(pdev);
> struct ti_hecc_priv *priv = netdev_priv(dev);
>
> - clk_enable(priv->clk);
> + clk_prepare_enable(priv->clk);
please add error checking.
>
> hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
> priv->can.state = CAN_STATE_ERROR_ACTIVE;
>
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] net, can, ti_hecc: fix a run time warn_on.
2015-10-20 14:46 ` Marc Kleine-Budde
@ 2015-10-20 16:30 ` Anton.Glukhov
2015-10-20 19:11 ` Marc Kleine-Budde
0 siblings, 1 reply; 4+ messages in thread
From: Anton.Glukhov @ 2015-10-20 16:30 UTC (permalink / raw)
To: Marc Kleine-Budde, linux-can; +Cc: hs, netdev
Thank you for review Marc!
On 20.10.2015 17:46, Marc Kleine-Budde wrote:
> On 10/20/2015 04:39 PM, Anton Glukhov wrote:
>> This patch fixes a warning in clk_enable by calling
>> clk_prepare_enable instead.
>>
>> Signed-off-by: Heiko Schocher <hs@denx.de>
>> Signed-off-by: Anton Glukhov <anton.a.glukhov@gmail.com>
>> ---
>> drivers/net/can/ti_hecc.c | 13 +++++++------
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
>> index cf345cb..77dd170 100644
>> --- a/drivers/net/can/ti_hecc.c
>> +++ b/drivers/net/can/ti_hecc.c
>> @@ -951,7 +951,7 @@ static int ti_hecc_probe(struct platform_device *pdev)
>> netif_napi_add(ndev, &priv->napi, ti_hecc_rx_poll,
>> HECC_DEF_NAPI_WEIGHT);
>>
>> - clk_enable(priv->clk);
>> + clk_prepare_enable(priv->clk);
>
> please add error checking
>
>> err = register_candev(ndev);
>> if (err) {
>> dev_err(&pdev->dev, "register_candev() failed\n");
>> @@ -966,7 +966,8 @@ static int ti_hecc_probe(struct platform_device *pdev)
>> return 0;
>>
>> probe_exit_clk:
>> - clk_put(priv->clk);
>
> Why do you remove the clk_put()?
>
My fault. I'll fix it in ver3.
>> + clk_disable_unprepare(priv->clk);
>> + priv->clk = NULL;
>
> The NULLing of priv->clk is not needed.
>
>> probe_exit_candev:
>> free_candev(ndev);
>> probe_exit_iounmap:
>> @@ -984,8 +985,8 @@ static int ti_hecc_remove(struct platform_device *pdev)
>> struct ti_hecc_priv *priv = netdev_priv(ndev);
>>
>> unregister_candev(ndev);
>> - clk_disable(priv->clk);
>> - clk_put(priv->clk);
>
> Why do you remoe the clk_put()?
>
>> + clk_disable_unprepare(priv->clk);
>> + priv->clk = NULL;
>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> iounmap(priv->base);
>> release_mem_region(res->start, resource_size(res));
>> @@ -1009,7 +1010,7 @@ static int ti_hecc_suspend(struct platform_device *pdev, pm_message_t state)
>> hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
>> priv->can.state = CAN_STATE_SLEEPING;
>>
>> - clk_disable(priv->clk);
>> + clk_disable_unprepare(priv->clk);
>>
>> return 0;
>> }
>> @@ -1019,7 +1020,7 @@ static int ti_hecc_resume(struct platform_device *pdev)
>> struct net_device *dev = platform_get_drvdata(pdev);
>> struct ti_hecc_priv *priv = netdev_priv(dev);
>>
>> - clk_enable(priv->clk);
>> + clk_prepare_enable(priv->clk);
>
> please add error checking.
>
Why it wasn't added with clk_enable? Is it bug? I'm confused here, because in so many drivers
there is no checking err after clk_enable or clk_prepare_enable. Maybe I missed something...
Could you clarify this part please?
>>
>> hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
>> priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>
>
> Marc
>
regards,
Anton
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] net, can, ti_hecc: fix a run time warn_on.
2015-10-20 16:30 ` Anton.Glukhov
@ 2015-10-20 19:11 ` Marc Kleine-Budde
0 siblings, 0 replies; 4+ messages in thread
From: Marc Kleine-Budde @ 2015-10-20 19:11 UTC (permalink / raw)
To: Anton.Glukhov, linux-can; +Cc: hs, netdev
[-- Attachment #1: Type: text/plain, Size: 1521 bytes --]
On 10/20/2015 06:30 PM, Anton.Glukhov wrote:
>>> + clk_disable_unprepare(priv->clk);
>>> + priv->clk = NULL;
>>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> iounmap(priv->base);
>>> release_mem_region(res->start, resource_size(res));
>>> @@ -1009,7 +1010,7 @@ static int ti_hecc_suspend(struct platform_device *pdev, pm_message_t state)
>>> hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
>>> priv->can.state = CAN_STATE_SLEEPING;
>>>
>>> - clk_disable(priv->clk);
>>> + clk_disable_unprepare(priv->clk);
>>>
>>> return 0;
>>> }
>>> @@ -1019,7 +1020,7 @@ static int ti_hecc_resume(struct platform_device *pdev)
>>> struct net_device *dev = platform_get_drvdata(pdev);
>>> struct ti_hecc_priv *priv = netdev_priv(dev);
>>>
>>> - clk_enable(priv->clk);
>>> + clk_prepare_enable(priv->clk);
>>
>> please add error checking.
>>
>
> Why it wasn't added with clk_enable? Is it bug? I'm confused here, because in so many drivers
> there is no checking err after clk_enable or clk_prepare_enable. Maybe I missed something...
> Could you clarify this part please?
I wouldn't call it a bug in the original driver, it probably slipped
through the review or I didn't know better.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-10-20 19:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-20 14:39 [PATCH v2] net, can, ti_hecc: fix a run time warn_on Anton Glukhov
2015-10-20 14:46 ` Marc Kleine-Budde
2015-10-20 16:30 ` Anton.Glukhov
2015-10-20 19:11 ` Marc Kleine-Budde
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).