On 15/05/17 11:32, Koivunen, Mikko wrote:
On 7.5.2017 14:42, Jonathan Cameron wrote:
On 05/05/17 12:19, Mikko Koivunen wrote:
Set sensor measurement off after probe fail in pm_runtime_set_active() or
iio_device_register(). Without this change sensor measurement stays on
even though probe fails on these calls.
This is maybe rare case, but causes constant power drain without any
benefits when it happens. Power drain is 20-500uA, typically 180uA.
Signed-off-by: Mikko Koivunen <mikko.koivunen@xxxxxxxxxxxxxxxxx>
Looks good to me, but I will be looking for Acks from Daniel on this
set. Daniel let me know if you want to pass on responsibility for the
driver (perhaps Mikko if willing?)
one tiny point inline.
---
Sensor measurement is turned on rpr0521_init() so in probe it should be
turned off in error cases after that. With CONFIG_PM it's probably
unnecessary for iio_device_register()-fail, but without CONFIG_PM it is
needed. Writing power off twice when CONFIG_PM enabled is ok.
drivers/iio/light/rpr0521.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/light/rpr0521.c b/drivers/iio/light/rpr0521.c
index 03504f6..5d077f4 100644
--- a/drivers/iio/light/rpr0521.c
+++ b/drivers/iio/light/rpr0521.c
@@ -516,13 +516,26 @@ static int rpr0521_probe(struct i2c_client *client,
ret = pm_runtime_set_active(&client->dev);
if (ret < 0)
- return ret;
+ goto err_poweroff;
pm_runtime_enable(&client->dev);
pm_runtime_set_autosuspend_delay(&client->dev, RPR0521_SLEEP_DELAY_MS);
pm_runtime_use_autosuspend(&client->dev);
- return iio_device_register(indio_dev);
+ ret = iio_device_register(indio_dev);
+ if (ret)
+ goto err_pm_disable;
+
+ return 0;
+
+err_pm_disable:
+ pm_runtime_disable(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
+ pm_runtime_put_noidle(&client->dev);
+err_poweroff:
+ (void) rpr0521_poweroff(data);
shouldn't need the void cast.
It is there because discarding the return value is done on purpose, not
by accident. Should I still remove it?
In an error path it's normal not to check so I don't think it needs
to be explicit...
+
+ return ret;
}
static int rpr0521_remove(struct i2c_client *client)
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
