* [PATCH] media: ov8856: Add runtime PM callbacks
@ 2022-09-20 2:00 Hidenori Kobayashi
2022-09-20 8:37 ` Sakari Ailus
0 siblings, 1 reply; 4+ messages in thread
From: Hidenori Kobayashi @ 2022-09-20 2:00 UTC (permalink / raw)
To: Dongchun Zhu, Mauro Carvalho Chehab
Cc: 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. Calling existing
power controlling functions from callbacks properly turn them off/on.
Signed-off-by: Hidenori Kobayashi <hidenorik@chromium.org>
---
drivers/media/i2c/ov8856.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
index a9728afc81d4..3f57bc30b78b 100644
--- a/drivers/media/i2c/ov8856.c
+++ b/drivers/media/i2c/ov8856.c
@@ -2200,6 +2200,26 @@ static int __maybe_unused ov8856_resume(struct device *dev)
return 0;
}
+static int __maybe_unused ov8856_runtime_suspend(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct ov8856 *ov8856 = to_ov8856(sd);
+
+ __ov8856_power_off(ov8856);
+
+ return 0;
+}
+
+static int __maybe_unused ov8856_runtime_resume(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct ov8856 *ov8856 = to_ov8856(sd);
+
+ return __ov8856_power_on(ov8856);
+}
+
static int ov8856_set_format(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_format *fmt)
@@ -2540,6 +2560,7 @@ static int ov8856_probe(struct i2c_client *client)
static const struct dev_pm_ops ov8856_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(ov8856_suspend, ov8856_resume)
+ SET_RUNTIME_PM_OPS(ov8856_runtime_suspend, ov8856_runtime_resume, NULL)
};
#ifdef CONFIG_ACPI
--
2.37.2.789.g6183377224-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] media: ov8856: Add runtime PM callbacks
2022-09-20 2:00 [PATCH] media: ov8856: Add runtime PM callbacks Hidenori Kobayashi
@ 2022-09-20 8:37 ` Sakari Ailus
2022-09-21 1:32 ` Hidenori Kobayashi
0 siblings, 1 reply; 4+ messages in thread
From: Sakari Ailus @ 2022-09-20 8:37 UTC (permalink / raw)
To: Hidenori Kobayashi
Cc: Dongchun Zhu, Mauro Carvalho Chehab, linux-media, linux-kernel
Hi Hidenori,
On Tue, Sep 20, 2022 at 11:00:01AM +0900, Hidenori Kobayashi wrote:
> There were no runtime PM callbacks registered, leaving regulators being
> enabled while the device is suspended on DT systems. Calling existing
> power controlling functions from callbacks properly turn them off/on.
>
> Signed-off-by: Hidenori Kobayashi <hidenorik@chromium.org>
> ---
> drivers/media/i2c/ov8856.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
> index a9728afc81d4..3f57bc30b78b 100644
> --- a/drivers/media/i2c/ov8856.c
> +++ b/drivers/media/i2c/ov8856.c
> @@ -2200,6 +2200,26 @@ static int __maybe_unused ov8856_resume(struct device *dev)
> return 0;
> }
>
> +static int __maybe_unused ov8856_runtime_suspend(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct ov8856 *ov8856 = to_ov8856(sd);
> +
> + __ov8856_power_off(ov8856);
Could you refactor this a bit, changing the __ov8856_power_off() argument
to struct dev *?
> +
> + return 0;
> +}
> +
> +static int __maybe_unused ov8856_runtime_resume(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct ov8856 *ov8856 = to_ov8856(sd);
> +
> + return __ov8856_power_on(ov8856);
> +}
> +
> static int ov8856_set_format(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_format *fmt)
> @@ -2540,6 +2560,7 @@ static int ov8856_probe(struct i2c_client *client)
>
> static const struct dev_pm_ops ov8856_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(ov8856_suspend, ov8856_resume)
> + SET_RUNTIME_PM_OPS(ov8856_runtime_suspend, ov8856_runtime_resume, NULL)
> };
>
> #ifdef CONFIG_ACPI
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] media: ov8856: Add runtime PM callbacks
2022-09-20 8:37 ` Sakari Ailus
@ 2022-09-21 1:32 ` Hidenori Kobayashi
2022-09-21 7:44 ` Sakari Ailus
0 siblings, 1 reply; 4+ messages in thread
From: Hidenori Kobayashi @ 2022-09-21 1:32 UTC (permalink / raw)
To: Sakari Ailus
Cc: Hidenori Kobayashi, Dongchun Zhu, Mauro Carvalho Chehab,
linux-media, linux-kernel
Hi Sakari,
Thanks for your review!
On Tue, Sep 20, 2022 at 11:37:06AM +0300, Sakari Ailus wrote:
> Hi Hidenori,
>
> On Tue, Sep 20, 2022 at 11:00:01AM +0900, Hidenori Kobayashi wrote:
> > There were no runtime PM callbacks registered, leaving regulators being
> > enabled while the device is suspended on DT systems. Calling existing
> > power controlling functions from callbacks properly turn them off/on.
> >
> > Signed-off-by: Hidenori Kobayashi <hidenorik@chromium.org>
> > ---
> > drivers/media/i2c/ov8856.c | 21 +++++++++++++++++++++
> > 1 file changed, 21 insertions(+)
> >
> > diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
> > index a9728afc81d4..3f57bc30b78b 100644
> > --- a/drivers/media/i2c/ov8856.c
> > +++ b/drivers/media/i2c/ov8856.c
> > @@ -2200,6 +2200,26 @@ static int __maybe_unused ov8856_resume(struct device *dev)
> > return 0;
> > }
> >
> > +static int __maybe_unused ov8856_runtime_suspend(struct device *dev)
> > +{
> > + struct i2c_client *client = to_i2c_client(dev);
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct ov8856 *ov8856 = to_ov8856(sd);
> > +
> > + __ov8856_power_off(ov8856);
>
> Could you refactor this a bit, changing the __ov8856_power_off() argument
> to struct dev *?
Sure. I'll make V2 with this refactoring. I'm guessing that we want the
same for __ov8856_power_on(). Please let me know if otherwise.
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused ov8856_runtime_resume(struct device *dev)
> > +{
> > + struct i2c_client *client = to_i2c_client(dev);
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct ov8856 *ov8856 = to_ov8856(sd);
> > +
> > + return __ov8856_power_on(ov8856);
> > +}
> > +
> > static int ov8856_set_format(struct v4l2_subdev *sd,
> > struct v4l2_subdev_state *sd_state,
> > struct v4l2_subdev_format *fmt)
> > @@ -2540,6 +2560,7 @@ static int ov8856_probe(struct i2c_client *client)
> >
> > static const struct dev_pm_ops ov8856_pm_ops = {
> > SET_SYSTEM_SLEEP_PM_OPS(ov8856_suspend, ov8856_resume)
> > + SET_RUNTIME_PM_OPS(ov8856_runtime_suspend, ov8856_runtime_resume, NULL)
> > };
> >
> > #ifdef CONFIG_ACPI
Best regards,
Hidenori
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] media: ov8856: Add runtime PM callbacks
2022-09-21 1:32 ` Hidenori Kobayashi
@ 2022-09-21 7:44 ` Sakari Ailus
0 siblings, 0 replies; 4+ messages in thread
From: Sakari Ailus @ 2022-09-21 7:44 UTC (permalink / raw)
To: Hidenori Kobayashi
Cc: Dongchun Zhu, Mauro Carvalho Chehab, linux-media, linux-kernel
Hi Hidenori,
On Wed, Sep 21, 2022 at 10:32:12AM +0900, Hidenori Kobayashi wrote:
> Hi Sakari,
>
> Thanks for your review!
You're welcome!
> I'm guessing that we want the
> same for __ov8856_power_on().
Yes, please!
--
Sakari Ailus
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-09-21 7:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-20 2:00 [PATCH] media: ov8856: Add runtime PM callbacks Hidenori Kobayashi
2022-09-20 8:37 ` Sakari Ailus
2022-09-21 1:32 ` Hidenori Kobayashi
2022-09-21 7:44 ` Sakari Ailus
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.