Re: [PATCH v1 4/7] power: bq24190_charger: Uniform pm_runtime_get() failure handling

Sebastian Reichel
2017-03-24T09:13:07Z

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

Hi,

On Tue, Mar 21, 2017 at 03:09:18PM -0700, Liam Breck wrote:
> From: Liam Breck <kernel@networkimprov.net>
>
> On pm_runtime_get() failure, always emit error message,
> and call pm_runtime_put_noidle() in irq handler.
>
> Fixes: 13d6fa8447fa power: bq24190_charger: Use PM runtime autosuspend
> Cc: Mark Greer <mgreer@animalcreek.com>
> Signed-off-by: Liam Breck <kernel@networkimprov.net>
> ---
> drivers/power/supply/bq24190_charger.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
> index c443988..ad09e48 100644
> --- a/drivers/power/supply/bq24190_charger.c
> +++ b/drivers/power/supply/bq24190_charger.c
> @@ -1267,12 +1267,13 @@ static void bq24190_check_status(struct bq24190_dev_info *bdi)
> static irqreturn_t bq24190_irq_handler_thread(int irq, void *data)
> {
> struct bq24190_dev_info *bdi = data;
> - int ret;
> + int error;
>
> bdi->irq_event = true;
> - ret = pm_runtime_get_sync(bdi->dev);
> - if (ret < 0) {
> - dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", ret);
> + error = pm_runtime_get_sync(bdi->dev);
> + if (error < 0) {
> + dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", error);
> + pm_runtime_put_noidle(bdi->dev);
> return IRQ_NONE;
> }
> bq24190_check_status(bdi);
> @@ -1399,8 +1400,10 @@ static int bq24190_probe(struct i2c_client *client,
> pm_runtime_use_autosuspend(dev);
> pm_runtime_set_autosuspend_delay(dev, 600);
> ret = pm_runtime_get_sync(dev);
> - if (ret < 0)
> + if (ret < 0) {
> + dev_err(dev, "pm_runtime_get failed: %i\n", ret);
> goto out1;
> + }

This should goto out2, which will make out1 not used anymore.
Would be nice, if you move this patch to the beginning of the
series in the next version, so that I can pick it already while
waiting for Acks on the DT binding.

-- Sebastian

>
> ret = bq24190_hw_init(bdi);
> if (ret < 0) {
> --
> 2.9.3
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]