[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <f2a3ed14-ecb8-1bab-a959-f63adb370ad6@linux.intel.com>
Date: Fri, 29 Jan 2021 10:58:48 +0800
From: Bingbu Cao <bingbu.cao@...ux.intel.com>
To: Sakari Ailus <sakari.ailus@...ux.intel.com>,
linux-i2c@...r.kernel.org
Cc: Wolfram Sang <wsa@...-dreams.de>,
"Rafael J. Wysocki" <rafael@...nel.org>,
linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
rajmohan.mani@...el.com, Tomasz Figa <tfiga@...omium.org>,
Bartosz Golaszewski <bgolaszewski@...libre.com>,
Bingbu Cao <bingbu.cao@...el.com>,
Chiranjeevi Rapolu <chiranjeevi.rapolu@...el.com>,
Hyungwoo Yang <hyungwoo.yang@...el.com>,
linux-media@...r.kernel.org
Subject: Re: [PATCH v9 6/7] media: i2c: imx319: Support probe while the device
is off
Reviewed-by: Bingbu Cao <bingbu.cao@...el.com>
On 1/29/21 7:27 AM, Sakari Ailus wrote:
> From: Rajmohan Mani <rajmohan.mani@...el.com>
>
> Tell ACPI device PM code that the driver supports the device being powered
> off when the driver's probe function is entered.
>
> Signed-off-by: Rajmohan Mani <rajmohan.mani@...el.com>
> Signed-off-by: Sakari Ailus <sakari.ailus@...ux.intel.com>
> ---
> drivers/media/i2c/imx319.c | 72 +++++++++++++++++++++++---------------
> 1 file changed, 44 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/media/i2c/imx319.c b/drivers/media/i2c/imx319.c
> index 8473c0bbb35d6..e0b22e9318fed 100644
> --- a/drivers/media/i2c/imx319.c
> +++ b/drivers/media/i2c/imx319.c
> @@ -140,6 +140,8 @@ struct imx319 {
>
> /* Streaming on/off */
> bool streaming;
> + /* True if the device has been identified */
> + bool identified;
> };
>
> static const struct imx319_reg imx319_global_regs[] = {
> @@ -2084,6 +2086,31 @@ imx319_set_pad_format(struct v4l2_subdev *sd,
> return 0;
> }
>
> +/* Verify chip ID */
> +static int imx319_identify_module(struct imx319 *imx319)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&imx319->sd);
> + int ret;
> + u32 val;
> +
> + if (imx319->identified)
> + return 0;
> +
> + ret = imx319_read_reg(imx319, IMX319_REG_CHIP_ID, 2, &val);
> + if (ret)
> + return ret;
> +
> + if (val != IMX319_CHIP_ID) {
> + dev_err(&client->dev, "chip id mismatch: %x!=%x",
> + IMX319_CHIP_ID, val);
> + return -EIO;
> + }
> +
> + imx319->identified = true;
> +
> + return 0;
> +}
> +
> /* Start streaming */
> static int imx319_start_streaming(struct imx319 *imx319)
> {
> @@ -2091,6 +2118,10 @@ static int imx319_start_streaming(struct imx319 *imx319)
> const struct imx319_reg_list *reg_list;
> int ret;
>
> + ret = imx319_identify_module(imx319);
> + if (ret)
> + return ret;
> +
> /* Global Setting */
> reg_list = &imx319_global_setting;
> ret = imx319_write_regs(imx319, reg_list->regs, reg_list->num_of_regs);
> @@ -2208,26 +2239,6 @@ static int __maybe_unused imx319_resume(struct device *dev)
> return ret;
> }
>
> -/* Verify chip ID */
> -static int imx319_identify_module(struct imx319 *imx319)
> -{
> - struct i2c_client *client = v4l2_get_subdevdata(&imx319->sd);
> - int ret;
> - u32 val;
> -
> - ret = imx319_read_reg(imx319, IMX319_REG_CHIP_ID, 2, &val);
> - if (ret)
> - return ret;
> -
> - if (val != IMX319_CHIP_ID) {
> - dev_err(&client->dev, "chip id mismatch: %x!=%x",
> - IMX319_CHIP_ID, val);
> - return -EIO;
> - }
> -
> - return 0;
> -}
> -
> static const struct v4l2_subdev_core_ops imx319_subdev_core_ops = {
> .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> @@ -2422,6 +2433,7 @@ static struct imx319_hwcfg *imx319_get_hwcfg(struct device *dev)
> static int imx319_probe(struct i2c_client *client)
> {
> struct imx319 *imx319;
> + bool low_power;
> int ret;
> u32 i;
>
> @@ -2434,11 +2446,14 @@ static int imx319_probe(struct i2c_client *client)
> /* Initialize subdev */
> v4l2_i2c_subdev_init(&imx319->sd, client, &imx319_subdev_ops);
>
> - /* Check module identity */
> - ret = imx319_identify_module(imx319);
> - if (ret) {
> - dev_err(&client->dev, "failed to find sensor: %d", ret);
> - goto error_probe;
> + low_power = acpi_dev_state_low_power(&client->dev);
> + if (!low_power) {
> + /* Check module identity */
> + ret = imx319_identify_module(imx319);
> + if (ret) {
> + dev_err(&client->dev, "failed to find sensor: %d", ret);
> + goto error_probe;
> + }
> }
>
> imx319->hwcfg = imx319_get_hwcfg(&client->dev);
> @@ -2491,10 +2506,10 @@ static int imx319_probe(struct i2c_client *client)
> goto error_media_entity;
>
> /*
> - * Device is already turned on by i2c-core with ACPI domain PM.
> - * Enable runtime PM and turn off the device.
> + * Don't set the device's state to active if it's in a low power state.
> */
> - pm_runtime_set_active(&client->dev);
> + if (!low_power)
> + pm_runtime_set_active(&client->dev);
> pm_runtime_enable(&client->dev);
> pm_runtime_idle(&client->dev);
>
> @@ -2547,6 +2562,7 @@ static struct i2c_driver imx319_i2c_driver = {
> },
> .probe_new = imx319_probe,
> .remove = imx319_remove,
> + .flags = I2C_DRV_FL_ALLOW_LOW_POWER_PROBE,
> };
> module_i2c_driver(imx319_i2c_driver);
>
>
--
Best regards,
Bingbu Cao
Powered by blists - more mailing lists