| Message ID | 20240923035115.3008900-2-ruanjinjie@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] media: i2c: dw9768: Fix pm_runtime_set_suspended() with runtime pm enabled | expand |
Hi Jinjie, Thanks for the patch. On Mon, Sep 23, 2024 at 11:51:13AM +0800, Jinjie Ruan wrote: > It is not valid to call pm_runtime_set_suspended() and > pm_runtime_set_active() for devices with runtime PM enabled because it > returns -EAGAIN if it is enabled already and working. So, adjust the > order to fix it. > > Fixes: 5f9a089b6de3 ("dw9768: Enable low-power probe on ACPI") > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > --- > drivers/media/i2c/dw9768.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/i2c/dw9768.c b/drivers/media/i2c/dw9768.c > index 18ef2b35c9aa..61db1e8d2727 100644 > --- a/drivers/media/i2c/dw9768.c > +++ b/drivers/media/i2c/dw9768.c > @@ -471,7 +471,6 @@ static int dw9768_probe(struct i2c_client *client) > * to be powered on in an ACPI system. Similarly for power off in > * remove. > */ > - pm_runtime_enable(dev); > full_power = (is_acpi_node(dev_fwnode(dev)) && > acpi_dev_state_d0(dev)) || > (is_of_node(dev_fwnode(dev)) && !pm_runtime_enabled(dev)); pm_runtime_enabled() should be replaced by IS_ENABLED(CONFIG_PM) here. > @@ -484,6 +483,7 @@ static int dw9768_probe(struct i2c_client *client) > pm_runtime_set_active(dev); > } > > + pm_runtime_enable(dev); > ret = v4l2_async_register_subdev(&dw9768->sd); > if (ret < 0) { > dev_err(dev, "failed to register V4L2 subdev: %d", ret); > @@ -495,12 +495,12 @@ static int dw9768_probe(struct i2c_client *client) > return 0; > > err_power_off: > + pm_runtime_disable(dev); > if (full_power) { > dw9768_runtime_suspend(dev); > pm_runtime_set_suspended(dev); > } > err_clean_entity: > - pm_runtime_disable(dev); > media_entity_cleanup(&dw9768->sd.entity); > err_free_handler: > v4l2_ctrl_handler_free(&dw9768->ctrls); > @@ -517,12 +517,12 @@ static void dw9768_remove(struct i2c_client *client) > v4l2_async_unregister_subdev(&dw9768->sd); > v4l2_ctrl_handler_free(&dw9768->ctrls); > media_entity_cleanup(&dw9768->sd.entity); > + pm_runtime_disable(dev); > if ((is_acpi_node(dev_fwnode(dev)) && acpi_dev_state_d0(dev)) || > (is_of_node(dev_fwnode(dev)) && !pm_runtime_enabled(dev))) { And similarly here. The patch seems fine with that change. > dw9768_runtime_suspend(dev); > pm_runtime_set_suspended(dev); > } > - pm_runtime_disable(dev); > } > > static const struct of_device_id dw9768_of_table[] = {
diff --git a/drivers/media/i2c/dw9768.c b/drivers/media/i2c/dw9768.c index 18ef2b35c9aa..61db1e8d2727 100644 --- a/drivers/media/i2c/dw9768.c +++ b/drivers/media/i2c/dw9768.c @@ -471,7 +471,6 @@ static int dw9768_probe(struct i2c_client *client) * to be powered on in an ACPI system. Similarly for power off in * remove. */ - pm_runtime_enable(dev); full_power = (is_acpi_node(dev_fwnode(dev)) && acpi_dev_state_d0(dev)) || (is_of_node(dev_fwnode(dev)) && !pm_runtime_enabled(dev)); @@ -484,6 +483,7 @@ static int dw9768_probe(struct i2c_client *client) pm_runtime_set_active(dev); } + pm_runtime_enable(dev); ret = v4l2_async_register_subdev(&dw9768->sd); if (ret < 0) { dev_err(dev, "failed to register V4L2 subdev: %d", ret); @@ -495,12 +495,12 @@ static int dw9768_probe(struct i2c_client *client) return 0; err_power_off: + pm_runtime_disable(dev); if (full_power) { dw9768_runtime_suspend(dev); pm_runtime_set_suspended(dev); } err_clean_entity: - pm_runtime_disable(dev); media_entity_cleanup(&dw9768->sd.entity); err_free_handler: v4l2_ctrl_handler_free(&dw9768->ctrls); @@ -517,12 +517,12 @@ static void dw9768_remove(struct i2c_client *client) v4l2_async_unregister_subdev(&dw9768->sd); v4l2_ctrl_handler_free(&dw9768->ctrls); media_entity_cleanup(&dw9768->sd.entity); + pm_runtime_disable(dev); if ((is_acpi_node(dev_fwnode(dev)) && acpi_dev_state_d0(dev)) || (is_of_node(dev_fwnode(dev)) && !pm_runtime_enabled(dev))) { dw9768_runtime_suspend(dev); pm_runtime_set_suspended(dev); } - pm_runtime_disable(dev); } static const struct of_device_id dw9768_of_table[] = {
It is not valid to call pm_runtime_set_suspended() and pm_runtime_set_active() for devices with runtime PM enabled because it returns -EAGAIN if it is enabled already and working. So, adjust the order to fix it. Fixes: 5f9a089b6de3 ("dw9768: Enable low-power probe on ACPI") Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- drivers/media/i2c/dw9768.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)