linux-kernel - Re: [PATCH v2] iio: light: cm32181: Fix PM support on system with 2 I2C resources

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: <3d51f766-bd80-870d-c939-99660526c654@redhat.com>
Date:   Wed, 18 Jan 2023 18:07:45 +0100
From:   Hans de Goede <hdegoede@...hat.com>
To:     Kai-Heng Feng <kai.heng.feng@...onical.com>,
        ktsai@...ellamicro.com, jic23@...nel.org, lars@...afoo.de
Cc:     Wahaj <wahajaved@...tonmail.com>, linux-iio@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] iio: light: cm32181: Fix PM support on system with 2
 I2C resources

Hi,

On 1/18/23 18:04, Kai-Heng Feng wrote:
> Commit c1e62062ff54 ("iio: light: cm32181: Handle CM3218 ACPI devices
> with 2 I2C resources") creates a second client for the actual I2C
> address, but the "struct device" passed to PM ops is the first I2C
> client that can't talk to the sensor.
> 
> That means the I2C transfers in both suspend and resume routines can
> fail and blocking the whole suspend process.
> 
> Instead of using the first client for I2C transfer, use the I2C client
> stored in the cm32181 private struct so the PM ops can get the correct
> I2C client to really talk to the sensor device.
> 
> Fixes: 68c1b3dd5c48 ("iio: light: cm32181: Add PM support")
> BugLink: https://bugs.launchpad.net/bugs/1988346
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2152281
> Tested-by: Wahaj <wahajaved@...tonmail.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@...onical.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@...hat.com>

Regards,

Hans



> ---
> v2:
>  - Removed setting drvdata to the dummy client.
>  - Added bug links.
>  - Wording.
> 
>  drivers/iio/light/cm32181.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
> index 001055d097509..b1674a5bfa368 100644
> --- a/drivers/iio/light/cm32181.c
> +++ b/drivers/iio/light/cm32181.c
> @@ -440,6 +440,8 @@ static int cm32181_probe(struct i2c_client *client)
>  	if (!indio_dev)
>  		return -ENOMEM;
>  
> +	i2c_set_clientdata(client, indio_dev);
> +
>  	/*
>  	 * Some ACPI systems list 2 I2C resources for the CM3218 sensor, the
>  	 * SMBus Alert Response Address (ARA, 0x0c) and the actual I2C address.
> @@ -460,8 +462,6 @@ static int cm32181_probe(struct i2c_client *client)
>  			return PTR_ERR(client);
>  	}
>  
> -	i2c_set_clientdata(client, indio_dev);
> -
>  	cm32181 = iio_priv(indio_dev);
>  	cm32181->client = client;
>  	cm32181->dev = dev;
> @@ -490,7 +490,8 @@ static int cm32181_probe(struct i2c_client *client)
>  
>  static int cm32181_suspend(struct device *dev)
>  {
> -	struct i2c_client *client = to_i2c_client(dev);
> +	struct cm32181_chip *cm32181 = iio_priv(dev_get_drvdata(dev));
> +	struct i2c_client *client = cm32181->client;
>  
>  	return i2c_smbus_write_word_data(client, CM32181_REG_ADDR_CMD,
>  					 CM32181_CMD_ALS_DISABLE);
> @@ -498,8 +499,8 @@ static int cm32181_suspend(struct device *dev)
>  
>  static int cm32181_resume(struct device *dev)
>  {
> -	struct i2c_client *client = to_i2c_client(dev);
>  	struct cm32181_chip *cm32181 = iio_priv(dev_get_drvdata(dev));
> +	struct i2c_client *client = cm32181->client;
>  
>  	return i2c_smbus_write_word_data(client, CM32181_REG_ADDR_CMD,
>  					 cm32181->conf_regs[CM32181_REG_ADDR_CMD]);