Linux IIO development
 help / color / mirror / Atom feed
* [PATCH v2] iio: light: cm32181: Fix PM support on system with 2 I2C resources
@ 2023-01-18 17:04 Kai-Heng Feng
  2023-01-18 17:07 ` Hans de Goede
  0 siblings, 1 reply; 3+ messages in thread
From: Kai-Heng Feng @ 2023-01-18 17:04 UTC (permalink / raw)
  To: ktsai, jic23, lars
  Cc: hdegoede, Kai-Heng Feng, Wahaj, linux-iio, linux-kernel

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@protonmail.com>
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
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]);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio: light: cm32181: Fix PM support on system with 2 I2C resources
  2023-01-18 17:04 [PATCH v2] iio: light: cm32181: Fix PM support on system with 2 I2C resources Kai-Heng Feng
@ 2023-01-18 17:07 ` Hans de Goede
  2023-01-21 17:51   ` Jonathan Cameron
  0 siblings, 1 reply; 3+ messages in thread
From: Hans de Goede @ 2023-01-18 17:07 UTC (permalink / raw)
  To: Kai-Heng Feng, ktsai, jic23, lars; +Cc: Wahaj, linux-iio, linux-kernel

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@protonmail.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.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]);


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio: light: cm32181: Fix PM support on system with 2 I2C resources
  2023-01-18 17:07 ` Hans de Goede
@ 2023-01-21 17:51   ` Jonathan Cameron
  0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2023-01-21 17:51 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Kai-Heng Feng, ktsai, lars, Wahaj, linux-iio, linux-kernel

On Wed, 18 Jan 2023 18:07:45 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> 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@protonmail.com>
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>  
> 
> Thanks, patch looks good to me:
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 
> Regards,
> 
> Hans
Applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks

Jonathan

> 
> 
> 
> > ---
> > 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]);  
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-01-21 17:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-18 17:04 [PATCH v2] iio: light: cm32181: Fix PM support on system with 2 I2C resources Kai-Heng Feng
2023-01-18 17:07 ` Hans de Goede
2023-01-21 17:51   ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox