pm_runtime_get_sync may not always succeed depending on SoC involved. So
handle the error appropriately.
Reported-by: Joachim Eastwood <[email protected]>
Signed-off-by: Nishanth Menon <[email protected]>
---
based on v3.15-rc1
Report-thread: http://marc.info/?t=139750330400002&r=1&w=2
omap2plus_defconfig + CONFIG_CRYPTO_DEV_OMAP_DES=y
pandaboard-es-before: http://slexy.org/raw/s21rGPFnKl
pandaboard-es-after: http://slexy.org/raw/s2A4UFQVna
drivers/crypto/omap-des.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/omap-des.c b/drivers/crypto/omap-des.c
index ec5f131..c2d362f 100644
--- a/drivers/crypto/omap-des.c
+++ b/drivers/crypto/omap-des.c
@@ -223,12 +223,18 @@ static void omap_des_write_n(struct omap_des_dev *dd, u32 offset,
static int omap_des_hw_init(struct omap_des_dev *dd)
{
+ int err;
+
/*
* clocks are enabled when request starts and disabled when finished.
* It may be long delays between requests.
* Device might go to off mode to save power.
*/
- pm_runtime_get_sync(dd->dev);
+ err = pm_runtime_get_sync(dd->dev);
+ if (err < 0) {
+ dev_err(dd->dev, "failed to get_sync(%d)\n", err);
+ return err;
+ }
if (!(dd->flags & FLAGS_INIT)) {
dd->flags |= FLAGS_INIT;
@@ -1083,7 +1089,11 @@ static int omap_des_probe(struct platform_device *pdev)
dd->phys_base = res->start;
pm_runtime_enable(dev);
- pm_runtime_get_sync(dev);
+ err = pm_runtime_get_sync(dev);
+ if (err < 0) {
+ dev_err(dd->dev, "failed to get_sync(%d)\n", err);
+ goto err_get;
+ }
omap_des_dma_stop(dd);
@@ -1148,6 +1158,7 @@ err_algs:
err_irq:
tasklet_kill(&dd->done_task);
tasklet_kill(&dd->queue_task);
+err_get:
pm_runtime_disable(dev);
err_res:
dd = NULL;
@@ -1191,8 +1202,7 @@ static int omap_des_suspend(struct device *dev)
static int omap_des_resume(struct device *dev)
{
- pm_runtime_get_sync(dev);
- return 0;
+ return pm_runtime_get_sync(dev);
}
#endif
--
1.7.9.5
Hi,
On Tue, Apr 15, 2014 at 10:33:02AM -0500, Nishanth Menon wrote:
> pm_runtime_get_sync may not always succeed depending on SoC involved. So
> handle the error appropriately.
>
> Reported-by: Joachim Eastwood <[email protected]>
> Signed-off-by: Nishanth Menon <[email protected]>
> ---
>
> based on v3.15-rc1
>
> Report-thread: http://marc.info/?t=139750330400002&r=1&w=2
>
> omap2plus_defconfig + CONFIG_CRYPTO_DEV_OMAP_DES=y
>
> pandaboard-es-before: http://slexy.org/raw/s21rGPFnKl
> pandaboard-es-after: http://slexy.org/raw/s2A4UFQVna
>
> drivers/crypto/omap-des.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/crypto/omap-des.c b/drivers/crypto/omap-des.c
> index ec5f131..c2d362f 100644
> --- a/drivers/crypto/omap-des.c
> +++ b/drivers/crypto/omap-des.c
> @@ -223,12 +223,18 @@ static void omap_des_write_n(struct omap_des_dev *dd, u32 offset,
>
> static int omap_des_hw_init(struct omap_des_dev *dd)
> {
> + int err;
> +
> /*
> * clocks are enabled when request starts and disabled when finished.
> * It may be long delays between requests.
> * Device might go to off mode to save power.
> */
> - pm_runtime_get_sync(dd->dev);
> + err = pm_runtime_get_sync(dd->dev);
> + if (err < 0) {
> + dev_err(dd->dev, "failed to get_sync(%d)\n", err);
> + return err;
even when get() fails, you must put().
> + }
>
> if (!(dd->flags & FLAGS_INIT)) {
> dd->flags |= FLAGS_INIT;
> @@ -1083,7 +1089,11 @@ static int omap_des_probe(struct platform_device *pdev)
> dd->phys_base = res->start;
>
> pm_runtime_enable(dev);
> - pm_runtime_get_sync(dev);
> + err = pm_runtime_get_sync(dev);
> + if (err < 0) {
> + dev_err(dd->dev, "failed to get_sync(%d)\n", err);
> + goto err_get;
this leaves usage_count incremented, needs a put() here too.
--
balbi
On 04/15/2014 10:50 AM, Felipe Balbi wrote:
> Hi,
>
> On Tue, Apr 15, 2014 at 10:33:02AM -0500, Nishanth Menon wrote:
>> pm_runtime_get_sync may not always succeed depending on SoC involved. So
>> handle the error appropriately.
>>
>> Reported-by: Joachim Eastwood <[email protected]>
>> Signed-off-by: Nishanth Menon <[email protected]>
>> ---
>>
>> based on v3.15-rc1
>>
>> Report-thread: http://marc.info/?t=139750330400002&r=1&w=2
>>
>> omap2plus_defconfig + CONFIG_CRYPTO_DEV_OMAP_DES=y
>>
>> pandaboard-es-before: http://slexy.org/raw/s21rGPFnKl
>> pandaboard-es-after: http://slexy.org/raw/s2A4UFQVna
>>
>> drivers/crypto/omap-des.c | 18 ++++++++++++++----
>> 1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/crypto/omap-des.c b/drivers/crypto/omap-des.c
>> index ec5f131..c2d362f 100644
>> --- a/drivers/crypto/omap-des.c
>> +++ b/drivers/crypto/omap-des.c
>> @@ -223,12 +223,18 @@ static void omap_des_write_n(struct omap_des_dev *dd, u32 offset,
>>
>> static int omap_des_hw_init(struct omap_des_dev *dd)
>> {
>> + int err;
>> +
>> /*
>> * clocks are enabled when request starts and disabled when finished.
>> * It may be long delays between requests.
>> * Device might go to off mode to save power.
>> */
>> - pm_runtime_get_sync(dd->dev);
>> + err = pm_runtime_get_sync(dd->dev);
>> + if (err < 0) {
>> + dev_err(dd->dev, "failed to get_sync(%d)\n", err);
>> + return err;
>
> even when get() fails, you must put().
>
>> + }
>>
>> if (!(dd->flags & FLAGS_INIT)) {
>> dd->flags |= FLAGS_INIT;
>> @@ -1083,7 +1089,11 @@ static int omap_des_probe(struct platform_device *pdev)
>> dd->phys_base = res->start;
>>
>> pm_runtime_enable(dev);
>> - pm_runtime_get_sync(dev);
>> + err = pm_runtime_get_sync(dev);
>> + if (err < 0) {
>> + dev_err(dd->dev, "failed to get_sync(%d)\n", err);
>> + goto err_get;
>
> this leaves usage_count incremented, needs a put() here too.
>
Yeah, I see the code in runtime.c, and pm_runtime_put_noidle is the
right invocation in the case of error.
Wish we had a coccinelle script for this.
--
Regards,
Nishanth Menon
pm_runtime_get_sync may not always succeed depending on SoC involved.
So handle the error appropriately ensuring usage_count is accurate in
case of failure.
Signed-off-by: Nishanth Menon <[email protected]>
---
V2:
- review fixes, print function names in error log as well.
V1: https://patchwork.kernel.org/patch/3993861/
drivers/crypto/omap-des.c | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/omap-des.c b/drivers/crypto/omap-des.c
index ec5f131..1d570c4 100644
--- a/drivers/crypto/omap-des.c
+++ b/drivers/crypto/omap-des.c
@@ -223,12 +223,19 @@ static void omap_des_write_n(struct omap_des_dev *dd, u32 offset,
static int omap_des_hw_init(struct omap_des_dev *dd)
{
+ int err;
+
/*
* clocks are enabled when request starts and disabled when finished.
* It may be long delays between requests.
* Device might go to off mode to save power.
*/
- pm_runtime_get_sync(dd->dev);
+ err = pm_runtime_get_sync(dd->dev);
+ if (err < 0) {
+ pm_runtime_put_noidle(dd->dev);
+ dev_err(dd->dev, "%s: failed to get_sync(%d)\n", __func__, err);
+ return err;
+ }
if (!(dd->flags & FLAGS_INIT)) {
dd->flags |= FLAGS_INIT;
@@ -1083,7 +1090,12 @@ static int omap_des_probe(struct platform_device *pdev)
dd->phys_base = res->start;
pm_runtime_enable(dev);
- pm_runtime_get_sync(dev);
+ err = pm_runtime_get_sync(dev);
+ if (err < 0) {
+ pm_runtime_put_noidle(dev);
+ dev_err(dd->dev, "%s: failed to get_sync(%d)\n", __func__, err);
+ goto err_get;
+ }
omap_des_dma_stop(dd);
@@ -1148,6 +1160,7 @@ err_algs:
err_irq:
tasklet_kill(&dd->done_task);
tasklet_kill(&dd->queue_task);
+err_get:
pm_runtime_disable(dev);
err_res:
dd = NULL;
@@ -1191,7 +1204,14 @@ static int omap_des_suspend(struct device *dev)
static int omap_des_resume(struct device *dev)
{
- pm_runtime_get_sync(dev);
+ int err;
+
+ err = pm_runtime_get_sync(dev);
+ if (err < 0) {
+ pm_runtime_put_noidle(dev);
+ dev_err(dev, "%s: failed to get_sync(%d)\n", __func__, err);
+ return err;
+ }
return 0;
}
#endif
--
1.7.9.5
On 15 April 2014 18:58, Nishanth Menon <[email protected]> wrote:
> pm_runtime_get_sync may not always succeed depending on SoC involved.
> So handle the error appropriately ensuring usage_count is accurate in
> case of failure.
>
> Signed-off-by: Nishanth Menon <[email protected]>
> ---
> V2:
> - review fixes, print function names in error log as well.
>
> V1: https://patchwork.kernel.org/patch/3993861/
> drivers/crypto/omap-des.c | 26 +++++++++++++++++++++++---
> 1 file changed, 23 insertions(+), 3 deletions(-)
>
Fixes the runtime warning from omap_l3_noc when omap-des is built-in
on my VAR-SOM-OM44 (OMAP4460) board.
Tested-by: Joachim Eastwood <[email protected]>
regards
Joachim Eastwood
On Tue, Apr 15, 2014 at 11:58:31AM -0500, Nishanth Menon wrote:
> pm_runtime_get_sync may not always succeed depending on SoC involved.
> So handle the error appropriately ensuring usage_count is accurate in
> case of failure.
>
> Signed-off-by: Nishanth Menon <[email protected]>
Reviewed-by: Felipe Balbi <[email protected]>
> ---
> V2:
> - review fixes, print function names in error log as well.
>
> V1: https://patchwork.kernel.org/patch/3993861/
> drivers/crypto/omap-des.c | 26 +++++++++++++++++++++++---
> 1 file changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/crypto/omap-des.c b/drivers/crypto/omap-des.c
> index ec5f131..1d570c4 100644
> --- a/drivers/crypto/omap-des.c
> +++ b/drivers/crypto/omap-des.c
> @@ -223,12 +223,19 @@ static void omap_des_write_n(struct omap_des_dev *dd, u32 offset,
>
> static int omap_des_hw_init(struct omap_des_dev *dd)
> {
> + int err;
> +
> /*
> * clocks are enabled when request starts and disabled when finished.
> * It may be long delays between requests.
> * Device might go to off mode to save power.
> */
> - pm_runtime_get_sync(dd->dev);
> + err = pm_runtime_get_sync(dd->dev);
> + if (err < 0) {
> + pm_runtime_put_noidle(dd->dev);
> + dev_err(dd->dev, "%s: failed to get_sync(%d)\n", __func__, err);
> + return err;
> + }
>
> if (!(dd->flags & FLAGS_INIT)) {
> dd->flags |= FLAGS_INIT;
> @@ -1083,7 +1090,12 @@ static int omap_des_probe(struct platform_device *pdev)
> dd->phys_base = res->start;
>
> pm_runtime_enable(dev);
> - pm_runtime_get_sync(dev);
> + err = pm_runtime_get_sync(dev);
> + if (err < 0) {
> + pm_runtime_put_noidle(dev);
> + dev_err(dd->dev, "%s: failed to get_sync(%d)\n", __func__, err);
> + goto err_get;
> + }
>
> omap_des_dma_stop(dd);
>
> @@ -1148,6 +1160,7 @@ err_algs:
> err_irq:
> tasklet_kill(&dd->done_task);
> tasklet_kill(&dd->queue_task);
> +err_get:
> pm_runtime_disable(dev);
> err_res:
> dd = NULL;
> @@ -1191,7 +1204,14 @@ static int omap_des_suspend(struct device *dev)
>
> static int omap_des_resume(struct device *dev)
> {
> - pm_runtime_get_sync(dev);
> + int err;
> +
> + err = pm_runtime_get_sync(dev);
> + if (err < 0) {
> + pm_runtime_put_noidle(dev);
> + dev_err(dev, "%s: failed to get_sync(%d)\n", __func__, err);
> + return err;
> + }
> return 0;
> }
> #endif
> --
> 1.7.9.5
>
--
balbi
On 04/15/2014 12:06 PM, Joachim Eastwood wrote:
> On 15 April 2014 18:58, Nishanth Menon <[email protected]> wrote:
>> pm_runtime_get_sync may not always succeed depending on SoC involved.
>> So handle the error appropriately ensuring usage_count is accurate in
>> case of failure.
>>
>> Signed-off-by: Nishanth Menon <[email protected]>
>> ---
>> V2:
>> - review fixes, print function names in error log as well.
>>
>> V1: https://patchwork.kernel.org/patch/3993861/
>> drivers/crypto/omap-des.c | 26 +++++++++++++++++++++++---
>> 1 file changed, 23 insertions(+), 3 deletions(-)
>>
>
> Fixes the runtime warning from omap_l3_noc when omap-des is built-in
> on my VAR-SOM-OM44 (OMAP4460) board.
>
> Tested-by: Joachim Eastwood <[email protected]>
Thanks Joachim. Looks like in my hurry, I missed proper
acknowledgement of your report. adding the following for patchworks to
pickup.
Reported-by: Joachim Eastwood <[email protected]>
--
Regards,
Nishanth Menon
On 04/15/2014 12:18 PM, Nishanth Menon wrote:
> On 04/15/2014 12:06 PM, Joachim Eastwood wrote:
>> On 15 April 2014 18:58, Nishanth Menon <[email protected]> wrote:
>>> pm_runtime_get_sync may not always succeed depending on SoC involved.
>>> So handle the error appropriately ensuring usage_count is accurate in
>>> case of failure.
>>>
>>> Signed-off-by: Nishanth Menon <[email protected]>
>>> ---
>>> V2:
>>> - review fixes, print function names in error log as well.
>>>
>>> V1: https://patchwork.kernel.org/patch/3993861/
>>> drivers/crypto/omap-des.c | 26 +++++++++++++++++++++++---
>>> 1 file changed, 23 insertions(+), 3 deletions(-)
>>>
>>
>> Fixes the runtime warning from omap_l3_noc when omap-des is built-in
>> on my VAR-SOM-OM44 (OMAP4460) board.
>>
>> Tested-by: Joachim Eastwood <[email protected]>
>
> Thanks Joachim. Looks like in my hurry, I missed proper
> acknowledgement of your report. adding the following for patchworks to
> pickup.
>
> Reported-by: Joachim Eastwood <[email protected]>
>
Acked-by: Joel Fernandes <[email protected]>
Regards,
-Joel
On Tue, Apr 15, 2014 at 12:51:43PM -0500, Joel Fernandes wrote:
> On 04/15/2014 12:18 PM, Nishanth Menon wrote:
> > On 04/15/2014 12:06 PM, Joachim Eastwood wrote:
> >> On 15 April 2014 18:58, Nishanth Menon <[email protected]> wrote:
> >>> pm_runtime_get_sync may not always succeed depending on SoC involved.
> >>> So handle the error appropriately ensuring usage_count is accurate in
> >>> case of failure.
> >>>
> >>> Signed-off-by: Nishanth Menon <[email protected]>
> >>> ---
> >>> V2:
> >>> - review fixes, print function names in error log as well.
> >>>
> >>> V1: https://patchwork.kernel.org/patch/3993861/
> >>> drivers/crypto/omap-des.c | 26 +++++++++++++++++++++++---
> >>> 1 file changed, 23 insertions(+), 3 deletions(-)
> >>>
> >>
> >> Fixes the runtime warning from omap_l3_noc when omap-des is built-in
> >> on my VAR-SOM-OM44 (OMAP4460) board.
> >>
> >> Tested-by: Joachim Eastwood <[email protected]>
> >
> > Thanks Joachim. Looks like in my hurry, I missed proper
> > acknowledgement of your report. adding the following for patchworks to
> > pickup.
> >
> > Reported-by: Joachim Eastwood <[email protected]>
> >
>
> Acked-by: Joel Fernandes <[email protected]>
Patch applied. Thanks!
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt