On Sat May 17 13:41:03 2025 +0200, Hans de Goede wrote:
> The i2c-client's power-domain has already been powered up when probe()
> gets called. So e.g. ACPI resources for regulators and clks have
> already been enabled and only the GPIOs need to be set to the on state.
>
> Instead of calling pm_runtime_set_suspended() while the domain is
> already powered up and then have detect() do a pm_runtime_get()
> to set the GPIOs do the following:
>
> 1. Call gc0310_power_on() to only set the GPIOs
> 2. Set the device's runtime-PM state to active instead of suspended
> 3. Avoid the device getting suspended as soon as pm_runtime_enable()
> gets called by calling pm_runtime_get() before _enable(), this means
> moving the pm_runtime_get() / _put() from detect() to probe ()
>
> This fixes power_on() not getting called when runtime-PM is not
> enabled in the Kconfig and this keeps the sensor powered-up while
> registering it avoiding unnecessary power cycles.
>
> Also modify gc0310_remove() to power-off the device if it is in
> active state when gc0310_remove() runs.
>
> Signed-off-by: Hans de Goede <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Patch committed.
Thanks,
Mauro Carvalho Chehab
drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 58 +++++++++++-----------
1 file changed, 28 insertions(+), 30 deletions(-)
---
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
index c4d85d65737c..173ddf41ad47 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
@@ -416,11 +416,7 @@ static int gc0310_detect(struct gc0310_device *sensor)
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
return -ENODEV;
- ret = pm_runtime_get_sync(&client->dev);
- if (ret >= 0)
- ret = cci_read(sensor->regmap, GC0310_SC_CMMN_CHIP_ID_REG,
- &val, NULL);
- pm_runtime_put(&client->dev);
+ ret = cci_read(sensor->regmap, GC0310_SC_CMMN_CHIP_ID_REG, &val, NULL);
if (ret < 0) {
dev_err(&client->dev, "read sensor_id failed: %d\n", ret);
return -ENODEV;
@@ -650,13 +646,15 @@ static void gc0310_remove(struct i2c_client *client)
struct v4l2_subdev *sd = i2c_get_clientdata(client);
struct gc0310_device *sensor = to_gc0310_sensor(sd);
- dev_dbg(&client->dev, "gc0310_remove...\n");
-
v4l2_async_unregister_subdev(sd);
v4l2_subdev_cleanup(sd);
media_entity_cleanup(&sensor->sd.entity);
v4l2_ctrl_handler_free(&sensor->ctrls.handler);
pm_runtime_disable(&client->dev);
+ if (!pm_runtime_status_suspended(&client->dev)) {
+ gc0310_power_off(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
+ }
}
static int gc0310_check_hwcfg(struct device *dev)
@@ -744,16 +742,15 @@ static int gc0310_probe(struct i2c_client *client)
if (IS_ERR(sensor->regmap))
return PTR_ERR(sensor->regmap);
- pm_runtime_set_suspended(&client->dev);
+ gc0310_power_on(&client->dev);
+
+ pm_runtime_set_active(&client->dev);
+ pm_runtime_get_noresume(&client->dev);
pm_runtime_enable(&client->dev);
- pm_runtime_set_autosuspend_delay(&client->dev, 1000);
- pm_runtime_use_autosuspend(&client->dev);
ret = gc0310_detect(sensor);
- if (ret) {
- gc0310_remove(client);
- return ret;
- }
+ if (ret)
+ goto err_power_down;
sensor->sd.internal_ops = &gc0310_internal_ops;
sensor->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
@@ -761,31 +758,32 @@ static int gc0310_probe(struct i2c_client *client)
sensor->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
ret = gc0310_init_controls(sensor);
- if (ret) {
- gc0310_remove(client);
- return ret;
- }
+ if (ret)
+ goto err_power_down;
ret = media_entity_pads_init(&sensor->sd.entity, 1, &sensor->pad);
- if (ret) {
- gc0310_remove(client);
- return ret;
- }
+ if (ret)
+ goto err_power_down;
sensor->sd.state_lock = sensor->ctrls.handler.lock;
ret = v4l2_subdev_init_finalize(&sensor->sd);
- if (ret) {
- gc0310_remove(client);
- return ret;
- }
+ if (ret)
+ goto err_power_down;
ret = v4l2_async_register_subdev_sensor(&sensor->sd);
- if (ret) {
- gc0310_remove(client);
- return ret;
- }
+ if (ret)
+ goto err_power_down;
+
+ pm_runtime_set_autosuspend_delay(&client->dev, 1000);
+ pm_runtime_use_autosuspend(&client->dev);
+ pm_runtime_put_autosuspend(&client->dev);
return 0;
+
+err_power_down:
+ pm_runtime_put_noidle(&client->dev);
+ gc0310_remove(client);
+ return ret;
}
static DEFINE_RUNTIME_DEV_PM_OPS(gc0310_pm_ops,
