Hi,
Thank you for the patch.
On 6/13/23 07:05, bingbu.cao@xxxxxxxxx wrote:
> From: Bingbu Cao <bingbu.cao@xxxxxxxxx>
>
> On ACPI based platforms, the ov13b10 camera sensor need to be powered
> up by acquire the PM resource from INT3472. INT3472 will register one
> regulator 'avdd', one reset gpio and clock source for ov13b10.
> Add 2 power interfaces that are registered as runtime PM callbacks.
>
> Signed-off-by: Bingbu Cao <bingbu.cao@xxxxxxxxx>
> Signed-off-by: Hao Yao <hao.yao@xxxxxxxxx>
> ---
> drivers/media/i2c/ov13b10.c | 100 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 98 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/ov13b10.c b/drivers/media/i2c/ov13b10.c
> index 96d3bd6ab3bd..be2c7d8c83ac 100644
> --- a/drivers/media/i2c/ov13b10.c
> +++ b/drivers/media/i2c/ov13b10.c
> @@ -2,6 +2,9 @@
> // Copyright (c) 2021 Intel Corporation.
>
> #include <linux/acpi.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/i2c.h>
> #include <linux/module.h>
> #include <linux/pm_runtime.h>
> @@ -573,6 +576,11 @@ struct ov13b10 {
> struct media_pad pad;
>
> struct v4l2_ctrl_handler ctrl_handler;
> +
> + struct clk *img_clk;
> + struct regulator *avdd;
> + struct gpio_desc *reset;
> +
> /* V4L2 Controls */
> struct v4l2_ctrl *link_freq;
> struct v4l2_ctrl *pixel_rate;
> @@ -1051,6 +1059,52 @@ static int ov13b10_identify_module(struct ov13b10 *ov13b)
> return 0;
> }
>
> +static int ov13b10_power_off(struct device *dev)
> +{
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct ov13b10 *ov13b10 = to_ov13b10(sd);
> +
> + if (ov13b10->reset)
> + gpiod_set_value_cansleep(ov13b10->reset, 1);
Just like the clk APIs the gpiod APIs will happily take a NULL
pointer, so the "if (ov13b10->reset)" can be dropped
here and also in ov13b10_power_on().
> + if (ov13b10->avdd)
> + regulator_disable(ov13b10->avdd);
> +
> + clk_disable_unprepare(ov13b10->img_clk);
> +
> + return 0;
> +}
> +
> +static int ov13b10_power_on(struct device *dev)
> +{
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct ov13b10 *ov13b10 = to_ov13b10(sd);
> + int ret;
> +
> + ret = clk_prepare_enable(ov13b10->img_clk);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable imaging clock: %d", ret);
> + return ret;
> + }
> +
> + if (ov13b10->avdd) {
> + ret = regulator_enable(ov13b10->avdd);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable avdd: %d", ret);
> + clk_disable_unprepare(ov13b10->img_clk);
> + return ret;
> + }
> + }
> +
> + if (ov13b10->reset) {
> + gpiod_set_value_cansleep(ov13b10->reset, 0);
> + /* 5ms to wait ready after XSHUTDN assert */
> + usleep_range(5000, 5500);
> + }
> +
> + return 0;
> +}
> +
> static int ov13b10_start_streaming(struct ov13b10 *ov13b)
> {
> struct i2c_client *client = v4l2_get_subdevdata(&ov13b->sd);
> @@ -1317,6 +1371,34 @@ static void ov13b10_free_controls(struct ov13b10 *ov13b)
> mutex_destroy(&ov13b->mutex);
> }
>
> +static int ov13b10_get_pm_resources(struct device *dev)
> +{
> + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> + struct ov13b10 *ov13b = to_ov13b10(sd);
> + int ret;
> +
> + ov13b->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(ov13b->reset))
> + return dev_err_probe(dev, PTR_ERR(ov13b->reset),
> + "failed to get reset gpio\n");
> +
> + ov13b->img_clk = devm_clk_get_optional(dev, NULL);
> + if (IS_ERR(ov13b->img_clk))
> + return dev_err_probe(dev, PTR_ERR(ov13b->img_clk),
> + "failed to get imaging clock\n");
> +
> + ov13b->avdd = devm_regulator_get_optional(dev, "avdd");
> + if (IS_ERR(ov13b->avdd)) {
> + ret = PTR_ERR(ov13b->avdd);
> + ov13b->avdd = NULL;
> + if (ret != -ENODEV)
> + return dev_err_probe(dev, ret,
> + "failed to get avdd regulator\n");
> + }
> +
> + return 0;
> +}
> +
> static int ov13b10_check_hwcfg(struct device *dev)
> {
> struct v4l2_fwnode_endpoint bus_cfg = {
> @@ -1407,13 +1489,23 @@ static int ov13b10_probe(struct i2c_client *client)
> /* Initialize subdev */
> v4l2_i2c_subdev_init(&ov13b->sd, client, &ov13b10_subdev_ops);
>
> + ret = ov13b10_get_pm_resources(&client->dev);
> + if (ret)
> + return ret;
> +
> full_power = acpi_dev_state_d0(&client->dev);
> if (full_power) {
> + ov13b10_power_on(&client->dev);
> + if (ret) {
> + dev_err(&client->dev, "failed to power on\n");
> + return ret;
> + }
> +
> /* Check module identity */
> ret = ov13b10_identify_module(ov13b);
> if (ret) {
> dev_err(&client->dev, "failed to find sensor: %d\n", ret);
> - return ret;
> + goto error_power_off;
> }
> }
>
> @@ -1422,7 +1514,7 @@ static int ov13b10_probe(struct i2c_client *client)
>
> ret = ov13b10_init_controls(ov13b);
> if (ret)
> - return ret;
> + goto error_power_off;
>
> /* Initialize subdev */
> ov13b->sd.internal_ops = &ov13b10_internal_ops;
> @@ -1462,6 +1554,9 @@ static int ov13b10_probe(struct i2c_client *client)
> ov13b10_free_controls(ov13b);
> dev_err(&client->dev, "%s failed:%d\n", __func__, ret);
>
> +error_power_off:
> + ov13b10_power_off(&client->dev);
> +
> return ret;
> }
>
> @@ -1479,6 +1574,7 @@ static void ov13b10_remove(struct i2c_client *client)
>
> static const struct dev_pm_ops ov13b10_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(ov13b10_suspend, ov13b10_resume)
> + SET_RUNTIME_PM_OPS(ov13b10_power_off, ov13b10_power_on, NULL)
You also need to add ov13b10_power_off / ov13b10_power_on calls
to ov13b10_suspend and ov13b10_resume so that the sensor gets
properly powered-off if it was not already runtime-suspend during
system suspend.
Regards,
Hans
> };
>
> #ifdef CONFIG_ACPI
