* [PATCH v3] media: ov8856: Add runtime PM callbacks
@ 2022-09-21 9:24 Hidenori Kobayashi
2022-10-06 9:58 ` Hidenori Kobayashi
2022-10-06 10:12 ` Tomasz Figa
0 siblings, 2 replies; 5+ messages in thread
From: Hidenori Kobayashi @ 2022-09-21 9:24 UTC (permalink / raw)
To: Dongchun Zhu, Mauro Carvalho Chehab
Cc: Sakari Ailus, Hidenori Kobayashi, linux-media, linux-kernel
There were no runtime PM callbacks registered, leaving regulators being
enabled while the device is suspended on DT systems. Adjust and register
existing power controlling functions to turn them off/on.
Signed-off-by: Hidenori Kobayashi <hidenorik@chromium.org>
---
V2 -> V3: Remove redundant wrappers (were added in V1)
V1 -> V2: Change argument of power controlling functions
---
drivers/media/i2c/ov8856.c | 33 +++++++++++++++++++--------------
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
index a9728afc81d4..760611ee5485 100644
--- a/drivers/media/i2c/ov8856.c
+++ b/drivers/media/i2c/ov8856.c
@@ -2110,17 +2110,18 @@ static int ov8856_set_stream(struct v4l2_subdev *sd, int enable)
return ret;
}
-static int __ov8856_power_on(struct ov8856 *ov8856)
+static int ov8856_power_on(struct device *dev)
{
- struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
+ struct v4l2_subdev *sd = dev_get_drvdata(dev);
+ struct ov8856 *ov8856 = to_ov8856(sd);
int ret;
- if (is_acpi_node(dev_fwnode(&client->dev)))
+ if (is_acpi_node(dev_fwnode(dev)))
return 0;
ret = clk_prepare_enable(ov8856->xvclk);
if (ret < 0) {
- dev_err(&client->dev, "failed to enable xvclk\n");
+ dev_err(dev, "failed to enable xvclk\n");
return ret;
}
@@ -2132,7 +2133,7 @@ static int __ov8856_power_on(struct ov8856 *ov8856)
ret = regulator_bulk_enable(ARRAY_SIZE(ov8856_supply_names),
ov8856->supplies);
if (ret < 0) {
- dev_err(&client->dev, "failed to enable regulators\n");
+ dev_err(dev, "failed to enable regulators\n");
goto disable_clk;
}
@@ -2148,17 +2149,20 @@ static int __ov8856_power_on(struct ov8856 *ov8856)
return ret;
}
-static void __ov8856_power_off(struct ov8856 *ov8856)
+static int ov8856_power_off(struct device *dev)
{
- struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
+ struct v4l2_subdev *sd = dev_get_drvdata(dev);
+ struct ov8856 *ov8856 = to_ov8856(sd);
- if (is_acpi_node(dev_fwnode(&client->dev)))
- return;
+ if (is_acpi_node(dev_fwnode(dev)))
+ return 0;
gpiod_set_value_cansleep(ov8856->reset_gpio, 1);
regulator_bulk_disable(ARRAY_SIZE(ov8856_supply_names),
ov8856->supplies);
clk_disable_unprepare(ov8856->xvclk);
+
+ return 0;
}
static int __maybe_unused ov8856_suspend(struct device *dev)
@@ -2170,7 +2174,7 @@ static int __maybe_unused ov8856_suspend(struct device *dev)
if (ov8856->streaming)
ov8856_stop_streaming(ov8856);
- __ov8856_power_off(ov8856);
+ ov8856_power_off(dev);
mutex_unlock(&ov8856->mutex);
return 0;
@@ -2184,7 +2188,7 @@ static int __maybe_unused ov8856_resume(struct device *dev)
mutex_lock(&ov8856->mutex);
- __ov8856_power_on(ov8856);
+ ov8856_power_on(dev);
if (ov8856->streaming) {
ret = ov8856_start_streaming(ov8856);
if (ret) {
@@ -2451,7 +2455,7 @@ static int ov8856_remove(struct i2c_client *client)
pm_runtime_disable(&client->dev);
mutex_destroy(&ov8856->mutex);
- __ov8856_power_off(ov8856);
+ ov8856_power_off(&client->dev);
return 0;
}
@@ -2477,7 +2481,7 @@ static int ov8856_probe(struct i2c_client *client)
full_power = acpi_dev_state_d0(&client->dev);
if (full_power) {
- ret = __ov8856_power_on(ov8856);
+ ret = ov8856_power_on(&client->dev);
if (ret) {
dev_err(&client->dev, "failed to power on\n");
return ret;
@@ -2533,13 +2537,14 @@ static int ov8856_probe(struct i2c_client *client)
mutex_destroy(&ov8856->mutex);
probe_power_off:
- __ov8856_power_off(ov8856);
+ ov8856_power_off(&client->dev);
return ret;
}
static const struct dev_pm_ops ov8856_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(ov8856_suspend, ov8856_resume)
+ SET_RUNTIME_PM_OPS(ov8856_power_off, ov8856_power_on, NULL)
};
#ifdef CONFIG_ACPI
--
2.37.3.968.ga6b4b080e4-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v3] media: ov8856: Add runtime PM callbacks
2022-09-21 9:24 [PATCH v3] media: ov8856: Add runtime PM callbacks Hidenori Kobayashi
@ 2022-10-06 9:58 ` Hidenori Kobayashi
2022-10-06 10:17 ` Sakari Ailus
2022-10-25 11:33 ` Sakari Ailus
2022-10-06 10:12 ` Tomasz Figa
1 sibling, 2 replies; 5+ messages in thread
From: Hidenori Kobayashi @ 2022-10-06 9:58 UTC (permalink / raw)
To: Dongchun Zhu, Mauro Carvalho Chehab, Sakari Ailus
Cc: linux-media, linux-kernel
Hi Sakari,
Could you kindly take a look, please?
On Wed, Sep 21, 2022 at 06:24:17PM +0900, Hidenori Kobayashi wrote:
> There were no runtime PM callbacks registered, leaving regulators being
> enabled while the device is suspended on DT systems. Adjust and register
> existing power controlling functions to turn them off/on.
>
> Signed-off-by: Hidenori Kobayashi <hidenorik@chromium.org>
> ---
> V2 -> V3: Remove redundant wrappers (were added in V1)
> V1 -> V2: Change argument of power controlling functions
> ---
> drivers/media/i2c/ov8856.c | 33 +++++++++++++++++++--------------
> 1 file changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
> index a9728afc81d4..760611ee5485 100644
> --- a/drivers/media/i2c/ov8856.c
> +++ b/drivers/media/i2c/ov8856.c
> @@ -2110,17 +2110,18 @@ static int ov8856_set_stream(struct v4l2_subdev *sd, int enable)
> return ret;
> }
>
> -static int __ov8856_power_on(struct ov8856 *ov8856)
> +static int ov8856_power_on(struct device *dev)
> {
> - struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct ov8856 *ov8856 = to_ov8856(sd);
> int ret;
>
> - if (is_acpi_node(dev_fwnode(&client->dev)))
> + if (is_acpi_node(dev_fwnode(dev)))
> return 0;
>
> ret = clk_prepare_enable(ov8856->xvclk);
> if (ret < 0) {
> - dev_err(&client->dev, "failed to enable xvclk\n");
> + dev_err(dev, "failed to enable xvclk\n");
> return ret;
> }
>
> @@ -2132,7 +2133,7 @@ static int __ov8856_power_on(struct ov8856 *ov8856)
> ret = regulator_bulk_enable(ARRAY_SIZE(ov8856_supply_names),
> ov8856->supplies);
> if (ret < 0) {
> - dev_err(&client->dev, "failed to enable regulators\n");
> + dev_err(dev, "failed to enable regulators\n");
> goto disable_clk;
> }
>
> @@ -2148,17 +2149,20 @@ static int __ov8856_power_on(struct ov8856 *ov8856)
> return ret;
> }
>
> -static void __ov8856_power_off(struct ov8856 *ov8856)
> +static int ov8856_power_off(struct device *dev)
> {
> - struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct ov8856 *ov8856 = to_ov8856(sd);
>
> - if (is_acpi_node(dev_fwnode(&client->dev)))
> - return;
> + if (is_acpi_node(dev_fwnode(dev)))
> + return 0;
>
> gpiod_set_value_cansleep(ov8856->reset_gpio, 1);
> regulator_bulk_disable(ARRAY_SIZE(ov8856_supply_names),
> ov8856->supplies);
> clk_disable_unprepare(ov8856->xvclk);
> +
> + return 0;
> }
>
> static int __maybe_unused ov8856_suspend(struct device *dev)
> @@ -2170,7 +2174,7 @@ static int __maybe_unused ov8856_suspend(struct device *dev)
> if (ov8856->streaming)
> ov8856_stop_streaming(ov8856);
>
> - __ov8856_power_off(ov8856);
> + ov8856_power_off(dev);
> mutex_unlock(&ov8856->mutex);
>
> return 0;
> @@ -2184,7 +2188,7 @@ static int __maybe_unused ov8856_resume(struct device *dev)
>
> mutex_lock(&ov8856->mutex);
>
> - __ov8856_power_on(ov8856);
> + ov8856_power_on(dev);
> if (ov8856->streaming) {
> ret = ov8856_start_streaming(ov8856);
> if (ret) {
> @@ -2451,7 +2455,7 @@ static int ov8856_remove(struct i2c_client *client)
> pm_runtime_disable(&client->dev);
> mutex_destroy(&ov8856->mutex);
>
> - __ov8856_power_off(ov8856);
> + ov8856_power_off(&client->dev);
>
> return 0;
> }
> @@ -2477,7 +2481,7 @@ static int ov8856_probe(struct i2c_client *client)
>
> full_power = acpi_dev_state_d0(&client->dev);
> if (full_power) {
> - ret = __ov8856_power_on(ov8856);
> + ret = ov8856_power_on(&client->dev);
> if (ret) {
> dev_err(&client->dev, "failed to power on\n");
> return ret;
> @@ -2533,13 +2537,14 @@ static int ov8856_probe(struct i2c_client *client)
> mutex_destroy(&ov8856->mutex);
>
> probe_power_off:
> - __ov8856_power_off(ov8856);
> + ov8856_power_off(&client->dev);
>
> return ret;
> }
>
> static const struct dev_pm_ops ov8856_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(ov8856_suspend, ov8856_resume)
> + SET_RUNTIME_PM_OPS(ov8856_power_off, ov8856_power_on, NULL)
> };
>
> #ifdef CONFIG_ACPI
> --
> 2.37.3.968.ga6b4b080e4-goog
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v3] media: ov8856: Add runtime PM callbacks
2022-10-06 9:58 ` Hidenori Kobayashi
@ 2022-10-06 10:17 ` Sakari Ailus
2022-10-25 11:33 ` Sakari Ailus
1 sibling, 0 replies; 5+ messages in thread
From: Sakari Ailus @ 2022-10-06 10:17 UTC (permalink / raw)
To: Hidenori Kobayashi
Cc: Dongchun Zhu, Mauro Carvalho Chehab, linux-media, linux-kernel
On Thu, Oct 06, 2022 at 06:58:46PM +0900, Hidenori Kobayashi wrote:
> Hi Sakari,
>
> Could you kindly take a look, please?
It's actually in my tree already but I haven't pushed it to git.linuxtv.org
yet. It'll need a rebase on 6.1-rc1.
--
Sakari Ailus
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] media: ov8856: Add runtime PM callbacks
2022-10-06 9:58 ` Hidenori Kobayashi
2022-10-06 10:17 ` Sakari Ailus
@ 2022-10-25 11:33 ` Sakari Ailus
1 sibling, 0 replies; 5+ messages in thread
From: Sakari Ailus @ 2022-10-25 11:33 UTC (permalink / raw)
To: Hidenori Kobayashi
Cc: Dongchun Zhu, Mauro Carvalho Chehab, linux-media, linux-kernel
Hi Hidenori,
On Thu, Oct 06, 2022 at 06:58:46PM +0900, Hidenori Kobayashi wrote:
> > @@ -2148,17 +2149,20 @@ static int __ov8856_power_on(struct ov8856 *ov8856)
> > return ret;
> > }
> >
> > -static void __ov8856_power_off(struct ov8856 *ov8856)
> > +static int ov8856_power_off(struct device *dev)
> > {
> > - struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
> > + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > + struct ov8856 *ov8856 = to_ov8856(sd);
> >
> > - if (is_acpi_node(dev_fwnode(&client->dev)))
> > - return;
> > + if (is_acpi_node(dev_fwnode(dev)))
> > + return 0;
> >
> > gpiod_set_value_cansleep(ov8856->reset_gpio, 1);
> > regulator_bulk_disable(ARRAY_SIZE(ov8856_supply_names),
> > ov8856->supplies);
> > clk_disable_unprepare(ov8856->xvclk);
> > +
> > + return 0;
> > }
> >
> > static int __maybe_unused ov8856_suspend(struct device *dev)
Applied with the following diff as the remove function's return type is now
void.
diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
index 9e1361eed91f0..cf8384e09413b 100644
--- a/drivers/media/i2c/ov8856.c
+++ b/drivers/media/i2c/ov8856.c
@@ -2456,8 +2456,6 @@ static void ov8856_remove(struct i2c_client *client)
mutex_destroy(&ov8856->mutex);
ov8856_power_off(&client->dev);
-
- return 0;
}
static int ov8856_probe(struct i2c_client *client)
--
Sakari Ailus
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] media: ov8856: Add runtime PM callbacks
2022-09-21 9:24 [PATCH v3] media: ov8856: Add runtime PM callbacks Hidenori Kobayashi
2022-10-06 9:58 ` Hidenori Kobayashi
@ 2022-10-06 10:12 ` Tomasz Figa
1 sibling, 0 replies; 5+ messages in thread
From: Tomasz Figa @ 2022-10-06 10:12 UTC (permalink / raw)
To: Hidenori Kobayashi
Cc: Dongchun Zhu, Mauro Carvalho Chehab, Sakari Ailus, linux-media,
linux-kernel
Hi Hidenori,
On Wed, Sep 21, 2022 at 6:24 PM Hidenori Kobayashi
<hidenorik@chromium.org> wrote:
>
> There were no runtime PM callbacks registered, leaving regulators being
> enabled while the device is suspended on DT systems. Adjust and register
> existing power controlling functions to turn them off/on.
>
> Signed-off-by: Hidenori Kobayashi <hidenorik@chromium.org>
> ---
> V2 -> V3: Remove redundant wrappers (were added in V1)
> V1 -> V2: Change argument of power controlling functions
> ---
> drivers/media/i2c/ov8856.c | 33 +++++++++++++++++++--------------
> 1 file changed, 19 insertions(+), 14 deletions(-)
>
Thanks for the patch!
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
Best regards,
Tomasz
---
> diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
> index a9728afc81d4..760611ee5485 100644
> --- a/drivers/media/i2c/ov8856.c
> +++ b/drivers/media/i2c/ov8856.c
> @@ -2110,17 +2110,18 @@ static int ov8856_set_stream(struct v4l2_subdev *sd, int enable)
> return ret;
> }
>
> -static int __ov8856_power_on(struct ov8856 *ov8856)
> +static int ov8856_power_on(struct device *dev)
> {
> - struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct ov8856 *ov8856 = to_ov8856(sd);
> int ret;
>
> - if (is_acpi_node(dev_fwnode(&client->dev)))
> + if (is_acpi_node(dev_fwnode(dev)))
> return 0;
>
> ret = clk_prepare_enable(ov8856->xvclk);
> if (ret < 0) {
> - dev_err(&client->dev, "failed to enable xvclk\n");
> + dev_err(dev, "failed to enable xvclk\n");
> return ret;
> }
>
> @@ -2132,7 +2133,7 @@ static int __ov8856_power_on(struct ov8856 *ov8856)
> ret = regulator_bulk_enable(ARRAY_SIZE(ov8856_supply_names),
> ov8856->supplies);
> if (ret < 0) {
> - dev_err(&client->dev, "failed to enable regulators\n");
> + dev_err(dev, "failed to enable regulators\n");
> goto disable_clk;
> }
>
> @@ -2148,17 +2149,20 @@ static int __ov8856_power_on(struct ov8856 *ov8856)
> return ret;
> }
>
> -static void __ov8856_power_off(struct ov8856 *ov8856)
> +static int ov8856_power_off(struct device *dev)
> {
> - struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct ov8856 *ov8856 = to_ov8856(sd);
>
> - if (is_acpi_node(dev_fwnode(&client->dev)))
> - return;
> + if (is_acpi_node(dev_fwnode(dev)))
> + return 0;
>
> gpiod_set_value_cansleep(ov8856->reset_gpio, 1);
> regulator_bulk_disable(ARRAY_SIZE(ov8856_supply_names),
> ov8856->supplies);
> clk_disable_unprepare(ov8856->xvclk);
> +
> + return 0;
> }
>
> static int __maybe_unused ov8856_suspend(struct device *dev)
> @@ -2170,7 +2174,7 @@ static int __maybe_unused ov8856_suspend(struct device *dev)
> if (ov8856->streaming)
> ov8856_stop_streaming(ov8856);
>
> - __ov8856_power_off(ov8856);
> + ov8856_power_off(dev);
> mutex_unlock(&ov8856->mutex);
>
> return 0;
> @@ -2184,7 +2188,7 @@ static int __maybe_unused ov8856_resume(struct device *dev)
>
> mutex_lock(&ov8856->mutex);
>
> - __ov8856_power_on(ov8856);
> + ov8856_power_on(dev);
> if (ov8856->streaming) {
> ret = ov8856_start_streaming(ov8856);
> if (ret) {
> @@ -2451,7 +2455,7 @@ static int ov8856_remove(struct i2c_client *client)
> pm_runtime_disable(&client->dev);
> mutex_destroy(&ov8856->mutex);
>
> - __ov8856_power_off(ov8856);
> + ov8856_power_off(&client->dev);
>
> return 0;
> }
> @@ -2477,7 +2481,7 @@ static int ov8856_probe(struct i2c_client *client)
>
> full_power = acpi_dev_state_d0(&client->dev);
> if (full_power) {
> - ret = __ov8856_power_on(ov8856);
> + ret = ov8856_power_on(&client->dev);
> if (ret) {
> dev_err(&client->dev, "failed to power on\n");
> return ret;
> @@ -2533,13 +2537,14 @@ static int ov8856_probe(struct i2c_client *client)
> mutex_destroy(&ov8856->mutex);
>
> probe_power_off:
> - __ov8856_power_off(ov8856);
> + ov8856_power_off(&client->dev);
>
> return ret;
> }
>
> static const struct dev_pm_ops ov8856_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(ov8856_suspend, ov8856_resume)
> + SET_RUNTIME_PM_OPS(ov8856_power_off, ov8856_power_on, NULL)
> };
>
> #ifdef CONFIG_ACPI
> --
> 2.37.3.968.ga6b4b080e4-goog
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-10-25 11:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-21 9:24 [PATCH v3] media: ov8856: Add runtime PM callbacks Hidenori Kobayashi
2022-10-06 9:58 ` Hidenori Kobayashi
2022-10-06 10:17 ` Sakari Ailus
2022-10-25 11:33 ` Sakari Ailus
2022-10-06 10:12 ` Tomasz Figa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.