------- Original Message -------
On Monday, January 16th, 2023 at 5:27 PM, Wahaj <wahajaved@xxxxxxxxxxxxxx> wrote:
>
>
>
> ------- Original Message -------
> On Monday, January 16th, 2023 at 3:59 PM, Wahaj wahajaved@xxxxxxxxxxxxxx wrote:
>
>
>
> > ------- Original Message -------
> > On Thursday, January 12th, 2023 at 5:48 AM, Kai-Heng Feng kai.heng.feng@xxxxxxxxxxxxx wrote:
> >
> > > Hi Wahaj,
> > >
> > > On Sun, Jan 8, 2023 at 8:33 PM Jonathan Cameron jic23@xxxxxxxxxx wrote:
> > >
> > > > On Sat, 07 Jan 2023 00:37:40 +0000
> > > > Wahaj wahajaved@xxxxxxxxxxxxxx wrote:
> > > >
> > > > > ------- Original Message -------
> > > > > On Friday, January 6th, 2023 at 9:08 AM, Kai-Heng Feng kai.heng.feng@xxxxxxxxxxxxx wrote:
> > > > >
> > > > > > Hi Wahaj,
> > > > > >
> > > > > > On Fri, Jan 6, 2023 at 12:26 AM Wahaj wahajaved@xxxxxxxxxxxxxx wrote:
> > > > > >
> > > > > > > ------- Original Message -------
> > > > > > >
> > > > > > > On Tuesday, January 3rd, 2023 at 2:26 PM, Jonathan Cameron Jonathan.Cameron@xxxxxxxxxx wrote:
> > > > > > >
> > > > > > > > On Wed, 28 Dec 2022 14:05:24 +0000
> > > > > > > > Wahaj wahajaved@xxxxxxxxxxxxxx wrote:
> > > > > > >
> > > > > > > > > Hi Jonathan
> > > > > > > > >
> > > > > > > > > Hope you're doing well. I have been using a laptop that comes with a
> > > > > > > > > CM32181 Light Sensor and after upgrading to the Linux kernel 6.0+, my
> > > > > > > > > laptop cannot seem to suspend because of the PM subsystem error. I
> > > > > > > > > have narrowed the problem down to this module and I believe that the
> > > > > > > > > commit 68c1b3dd5c48b2323067f8c1f0649ae2f31ab20bis the culprit
> > > > > > > > >
> > > > > > > > > The following lines were provided from the journalctl logs:
> > > > > > > > >
> > > > > > > > > > cm32181 i2c-CPLM3218:00: PM: dpm_run_callback():
> > > > > > > > > > acpi_subsys_suspend+0x0/0x60 returns -121 cm32181 i2c-CPLM3218:00:
> > > > > > > > > > PM: failed to suspend async: error -121
> > > > > > > > >
> > > > > > > > > I would love the chance to be able to work on this given any guidance
> > > > > > > > > on where to start
> > > > > > > >
> > > > > > > > Hi Wahaj,
> > > > > > > >
> > > > > > > > Certainly seems likely that you have identified the right commit.
> > > > > > > > As a starting point, resend this email to linux-iio@xxxxxxxxxxxxxxx
> > > > > > > > and Kai-Heng Feng kai.heng.feng@xxxxxxxxxxxxx
> > > > > > > >
> > > > > > > > If you could try reverting the commit to be completely sure it is
> > > > > > > > the cause that would help avoid any doubt.
> > > > > > > > Superficially the only thing that I can see causing this problem is
> > > > > > > > a fail of the i2c bus write.
> > > > > > > >
> > > > > > > > Does the device work prior to suspend? Try cat /sys/bus/iio/iio:device0/*
> > > > > > > > and see if you get any errors (may be device1 etc)
> > > > > > > >
> > > > > > > > If the device wasn't working at all the register writes in probe() should
> > > > > > > > have failed so we shouldn't be trying to suspend it.
> > > > > > > > It's possible your machine has some unusual power dependencies or
> > > > > > > > similar that mean the device is getting powered down before we try to
> > > > > > > > suspend it.
> > > > > > > >
> > > > > > > > Anyhow, better to have this discussion on list as there are many other people
> > > > > > > > who may have more insight than me or be able to replicate and help debug.
> > > > > > > >
> > > > > > > > Jonathan
> > > > > > > >
> > > > > > > > > Best Regards,
> > > > > > > > > Wahaj Javed
> > > > > > >
> > > > > > > Hi Jonathan and Kai-Heng Feng,
> > > > > > >
> > > > > > > I am currently using the 5.15 linux kernel for a while now which works perfectly fine.
> > > > > > >
> > > > > > > From what I gather the suspend functionality does work when using an older Linux version without the PM i2c bus writes.
> > > > > >
> > > > > > Does your system use S3 or S2idle to perform suspend?
> > > > >
> > > > > My system uses S3 to perform suspend
> > > > >
> > > > > > > The device does work fine prior to and post attempted suspend with no errors showing in cat /sys/bus/iio/iio:device0/*
> > > > > >
> > > > > > Does in_illuminance_input value change after system suspend?
> > > > > > Yes, the in_illuminance_input value changes after system suspend with or without the suspend and resume functions
> > > > >
> > > > > > Kai-Heng
> > > > > >
> > > > > > > Let me know if there's anything I should start looking into
> > > > > > >
> > > > > > > Best Regards
> > > > > > > Wahaj Javed
> > > > >
> > > > > How did the kernel used to suspend/resume before using the suspend and resume functions in the light sensor?
> > > > >
> > > > > Was the DEFINE_SIMPLE_DEV_PM_OPS introduced as a part of the Linux 6.0+ PM rework?
> > > >
> > > > Until https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=68c1b3dd5c48b2323067f8c1f0649ae2f31ab20b
> > > > which will have hit mainline in 6.0, the driver had no suspend / resume support.
> > > >
> > > > That patch introduced an i2c write to put the chip into a low power mode.
> > > > My best guess for a cause would be that something else is incorrectly being disabled
> > > > before this call and hence breaking the i2c comms.
> > >
> > > Can you please try the patch here and see if the device supports
> > > DISABLE command at all:
> > >
> > > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1988346/comments/52
> >
> > I've tried it, this is the response
> >
> > tee: '/sys/bus/iio/devices/iio:device0/disable': Permission denied
> >
> > I don't think it supports the disable command
> >
> > Wahaj Javed
>
> My apologies, loaded the unmodified kernel before. After applying the patch, I get the following dmesg output
>
> - [ 97.659932] iio iio:device0: CM32181_CMD_ALS_DISABLE succeeded.
>
> Although disabling it doesn't affect the suspend timeline
>
> - [ 112.456347] sd 0:0:0:0: [sda] Synchronizing SCSI cache
> [ 112.463074] cm32181 i2c-CPLM3218:00: PM: dpm_run_callback(): acpi_subsys_suspend+0x0/0x60 returns -121
> [ 112.463093] cm32181 i2c-CPLM3218:00: PM: failed to suspend async: error -121
> [ 112.466463] sd 0:0:0:0: [sda] Stopping disk
> [ 115.543097] PM: Some devices failed to suspend, or early wake event detected
>
> Wahaj Javed
>
Hi Kai-Heng
I have found some weird stuff going on while applying some changes. PFA a patch for your reference
System goes to suspend, fans immediately slow down, unlike the vanilla v6.x kernel, the screen doesn't wake up immediately, the fans stop spinning completely after about 4 minutes and does not wake back up along with power button light on (not normal), have to force reboot after suspending
Here are the jounralctl logs after closing lid
- systemd[1]: Reached target Sleep.
- systemd[1]: Starting System Suspend...
- systemd-sleep[2150]: Entering sleep state 'suspend'...
The following entry only appears after the 4 minute mark when fans shut off
- kernel: PM: suspend entry (deep)
I think the issue may have something to do with how the iio device is initialized. Are there any differences between the various ways of fetching the i2c client from the device struct?
Also, the logs do not get printed in the suspend and resume entrypoints do not get printed
Best Regards
Wahaj Javed
> > > Kai-Heng
> > >
> > > > Jonathan
> > > >
> > > > > I would like to take on this bug and try to solve it if that's possible
> > > > >
> > > > > Best Regards
> > > > > Wahaj Javed
diff --git a/drivers/iio/light/cm32181.c b/drivers/iio/light/cm32181.c
index 001055d09750..f198e3869ab3 100644
--- a/drivers/iio/light/cm32181.c
+++ b/drivers/iio/light/cm32181.c
@@ -490,19 +490,29 @@ 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_to_iio_dev(dev));
+ struct i2c_client *client = cm32181->client;
+ int ret;
- return i2c_smbus_write_word_data(client, CM32181_REG_ADDR_CMD,
+ dev_info(dev, "%s: Entered suspend path \n", __func__);
+ ret = i2c_smbus_write_word_data(client, CM32181_REG_ADDR_CMD,
CM32181_CMD_ALS_DISABLE);
+
+ dev_info(dev, "%s: Write disable command to CM32181 with return code %i \n", __func__, ret);
+ return ret;
}
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 cm32181_chip *cm32181 = iio_priv(dev_to_iio_dev(dev));
+ struct i2c_client *client = cm32181->client;
+ int ret;
- return i2c_smbus_write_word_data(client, CM32181_REG_ADDR_CMD,
+ dev_info(dev, "%s: Entered resume path \n", __func__);
+ ret = i2c_smbus_write_word_data(client, CM32181_REG_ADDR_CMD,
cm32181->conf_regs[CM32181_REG_ADDR_CMD]);
+ dev_info(dev, "%s: Write enable command to CM32181 with return code %i \n", __func__, ret);
+ return ret;
}
static DEFINE_SIMPLE_DEV_PM_OPS(cm32181_pm_ops, cm32181_suspend, cm32181_resume);
