linux-kernel - Re: [PATCH v5 3/6] ov5670: Support probe whilst the device is in a low power state

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7a1fa217-7fd1-1d36-0b1c-ad5d09ea11a0@linux.intel.com>
Date:   Wed, 12 Aug 2020 17:12:28 +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, Bingbu Cao <bingbu.cao@...el.com>,
        linux-media@...r.kernel.org,
        Chiranjeevi Rapolu <chiranjeevi.rapolu@...el.com>,
        Hyungwoo Yang <hyungwoo.yang@...el.com>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Arnd Bergmann <arnd@...db.de>, linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        rajmohan.mani@...el.com, Tomasz Figa <tfiga@...omium.org>,
        "Qiu, Tian Shu" <tian.shu.qiu@...el.com>
Subject: Re: [PATCH v5 3/6] ov5670: Support probe whilst the device is in a
 low power state



On 8/10/20 10:27 PM, Sakari Ailus wrote:
> Tell ACPI device PM code that the driver supports the device being in a
> low power state when the driver's probe function is entered.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@...ux.intel.com>
> ---
>  drivers/media/i2c/ov5670.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
> index f26252e35e08d..1f75b888d2a18 100644
> --- a/drivers/media/i2c/ov5670.c
> +++ b/drivers/media/i2c/ov5670.c
> @@ -2456,6 +2456,7 @@ static int ov5670_probe(struct i2c_client *client)
>  	struct ov5670 *ov5670;
>  	const char *err_msg;
>  	u32 input_clk = 0;
> +	bool low_power;
>  	int ret;
>  
>  	device_property_read_u32(&client->dev, "clock-frequency", &input_clk);
> @@ -2472,11 +2473,14 @@ static int ov5670_probe(struct i2c_client *client)
>  	/* Initialize subdev */
>  	v4l2_i2c_subdev_init(&ov5670->sd, client, &ov5670_subdev_ops);
>  
> -	/* Check module identity */
> -	ret = ov5670_identify_module(ov5670);
> -	if (ret) {
> -		err_msg = "ov5670_identify_module() error";
> -		goto error_print;
> +	low_power = acpi_dev_state_low_power(&client->dev);
> +	if (!low_power) {
> +		/* Check module identity */
> +		ret = ov5670_identify_module(ov5670);
> +		if (ret) {
> +			err_msg = "ov5670_identify_module() error";
> +			goto error_print;
> +	

Sakari, thanks for your patch.
one question - With this change, there will be no chance for driver to guarantee
that the camera sensor plugged in is the camera that the matched driver actually
can drive until try to streaming the camera, so is it necessary to return
appropriate error in .s_stream ops to notify user it is not the hardware that
current driver can drive? if no other better way.

-- 
Best regards,
Bingbu Cao