[SRU] [E/F/Unstable/OEM-OSP1-B] [PATCH 0/1] Fix Nvidia's ucsi_ccg to make system be able to sleep

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU] [E/F/Unstable/OEM-OSP1-B] [PATCH 0/1] Fix Nvidia's ucsi_ccg to make system be able to sleep

Kai-Heng Feng
BugLink: https://bugs.launchpad.net/bugs/1850238

[Impact]
Some systems have a "phantom" Nvidia UCSI, which prevent systems from
suspending.

[Fix]
ucsi_ccg is stuck in its probe routine because of the i2c bus never
timeouts. Let it timeouts and probe can fail since it's just a phantom
device.

[Test]
After applying this patch system can suspend/resume succesfully.

[Regression Potential]
Low. It's a trivial change to correctly handle timeout.

Kai-Heng Feng (1):
  i2c: nvidia-gpu: Handle timeout correctly in gpu_i2c_check_status()

 drivers/i2c/busses/i2c-nvidia-gpu.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/1] i2c: nvidia-gpu: Handle timeout correctly in gpu_i2c_check_status()

Kai-Heng Feng
BugLink: https://bugs.launchpad.net/bugs/1850238

Nvidia card may come with a "phantom" UCSI device, and its driver gets
stuck in probe routine, prevents any system PM operations like suspend.

There's an unaccounted case that the target time can equal to jiffies in
gpu_i2c_check_status(), let's solve that by using readl_poll_timeout()
instead of jiffies comparison functions.

Fixes: c71bcdcb42a7 ("i2c: add i2c bus driver for NVIDIA GPU")
Suggested-by: Andy Shevchenko <[hidden email]>
Signed-off-by: Kai-Heng Feng <[hidden email]>
Reviewed-by: Andy Shevchenko <[hidden email]>
Reviewed-by: Ajay Gupta <[hidden email]>
Tested-by: Ajay Gupta <[hidden email]>
Signed-off-by: Wolfram Sang <[hidden email]>
(cherry picked from commit d944b27df121e2ee854a6c2fad13d6c6300792d4 linux-next)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/i2c/busses/i2c-nvidia-gpu.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c b/drivers/i2c/busses/i2c-nvidia-gpu.c
index 5a1235fd86bb..32cd62188a3d 100644
--- a/drivers/i2c/busses/i2c-nvidia-gpu.c
+++ b/drivers/i2c/busses/i2c-nvidia-gpu.c
@@ -8,6 +8,7 @@
 #include <linux/delay.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
@@ -75,20 +76,15 @@ static void gpu_enable_i2c_bus(struct gpu_i2c_dev *i2cd)
 
 static int gpu_i2c_check_status(struct gpu_i2c_dev *i2cd)
 {
- unsigned long target = jiffies + msecs_to_jiffies(1000);
  u32 val;
+ int ret;
 
- do {
- val = readl(i2cd->regs + I2C_MST_CNTL);
- if (!(val & I2C_MST_CNTL_CYCLE_TRIGGER))
- break;
- if ((val & I2C_MST_CNTL_STATUS) !=
- I2C_MST_CNTL_STATUS_BUS_BUSY)
- break;
- usleep_range(500, 600);
- } while (time_is_after_jiffies(target));
-
- if (time_is_before_jiffies(target)) {
+ ret = readl_poll_timeout(i2cd->regs + I2C_MST_CNTL, val,
+ !(val & I2C_MST_CNTL_CYCLE_TRIGGER) ||
+ (val & I2C_MST_CNTL_STATUS) != I2C_MST_CNTL_STATUS_BUS_BUSY,
+ 500, 1000 * USEC_PER_MSEC);
+
+ if (ret) {
  dev_err(i2cd->dev, "i2c timeout error %x\n", val);
  return -ETIMEDOUT;
  }
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED [OEM-OSP1-B] Re: [SRU] [E/F/Unstable/OEM-OSP1-B] [PATCH 0/1] Fix Nvidia's ucsi_ccg to make system be able to sleep

Timo Aaltonen-6
In reply to this post by Kai-Heng Feng
On 26.3.2020 15.05, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1850238
>
> [Impact]
> Some systems have a "phantom" Nvidia UCSI, which prevent systems from
> suspending.
>
> [Fix]
> ucsi_ccg is stuck in its probe routine because of the i2c bus never
> timeouts. Let it timeouts and probe can fail since it's just a phantom
> device.
>
> [Test]
> After applying this patch system can suspend/resume succesfully.
>
> [Regression Potential]
> Low. It's a trivial change to correctly handle timeout.
>
> Kai-Heng Feng (1):
>   i2c: nvidia-gpu: Handle timeout correctly in gpu_i2c_check_status()
>
>  drivers/i2c/busses/i2c-nvidia-gpu.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
>

applied to osp1 oem-next, thanks

--
t

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU] [E/F/Unstable/OEM-OSP1-B] [PATCH 0/1] Fix Nvidia's ucsi_ccg to make system be able to sleep

Sultan Alsawaf
In reply to this post by Kai-Heng Feng
Acked-by: Sultan Alsawaf <[hidden email]>

On Thu, Mar 26, 2020 at 09:05:01PM +0800, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1850238
>
> [Impact]
> Some systems have a "phantom" Nvidia UCSI, which prevent systems from
> suspending.
>
> [Fix]
> ucsi_ccg is stuck in its probe routine because of the i2c bus never
> timeouts. Let it timeouts and probe can fail since it's just a phantom
> device.
>
> [Test]
> After applying this patch system can suspend/resume succesfully.
>
> [Regression Potential]
> Low. It's a trivial change to correctly handle timeout.
>
> Kai-Heng Feng (1):
>   i2c: nvidia-gpu: Handle timeout correctly in gpu_i2c_check_status()
>
>  drivers/i2c/busses/i2c-nvidia-gpu.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
>
> --
> 2.17.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [PATCH 1/1] i2c: nvidia-gpu: Handle timeout correctly in gpu_i2c_check_status()

Kleber Souza
In reply to this post by Kai-Heng Feng
On 26.03.20 14:05, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1850238
>
> Nvidia card may come with a "phantom" UCSI device, and its driver gets
> stuck in probe routine, prevents any system PM operations like suspend.
>
> There's an unaccounted case that the target time can equal to jiffies in
> gpu_i2c_check_status(), let's solve that by using readl_poll_timeout()
> instead of jiffies comparison functions.
>
> Fixes: c71bcdcb42a7 ("i2c: add i2c bus driver for NVIDIA GPU")
> Suggested-by: Andy Shevchenko <[hidden email]>
> Signed-off-by: Kai-Heng Feng <[hidden email]>
> Reviewed-by: Andy Shevchenko <[hidden email]>
> Reviewed-by: Ajay Gupta <[hidden email]>
> Tested-by: Ajay Gupta <[hidden email]>
> Signed-off-by: Wolfram Sang <[hidden email]>
> (cherry picked from commit d944b27df121e2ee854a6c2fad13d6c6300792d4 linux-next)

This patch has already been merged to Linus' tree with the same sha1,
the 'linux-next' part can be removed.

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

> Signed-off-by: Kai-Heng Feng <[hidden email]>
> ---
>  drivers/i2c/busses/i2c-nvidia-gpu.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c b/drivers/i2c/busses/i2c-nvidia-gpu.c
> index 5a1235fd86bb..32cd62188a3d 100644
> --- a/drivers/i2c/busses/i2c-nvidia-gpu.c
> +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c
> @@ -8,6 +8,7 @@
>  #include <linux/delay.h>
>  #include <linux/i2c.h>
>  #include <linux/interrupt.h>
> +#include <linux/iopoll.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
>  #include <linux/platform_device.h>
> @@ -75,20 +76,15 @@ static void gpu_enable_i2c_bus(struct gpu_i2c_dev *i2cd)
>  
>  static int gpu_i2c_check_status(struct gpu_i2c_dev *i2cd)
>  {
> - unsigned long target = jiffies + msecs_to_jiffies(1000);
>   u32 val;
> + int ret;
>  
> - do {
> - val = readl(i2cd->regs + I2C_MST_CNTL);
> - if (!(val & I2C_MST_CNTL_CYCLE_TRIGGER))
> - break;
> - if ((val & I2C_MST_CNTL_STATUS) !=
> - I2C_MST_CNTL_STATUS_BUS_BUSY)
> - break;
> - usleep_range(500, 600);
> - } while (time_is_after_jiffies(target));
> -
> - if (time_is_before_jiffies(target)) {
> + ret = readl_poll_timeout(i2cd->regs + I2C_MST_CNTL, val,
> + !(val & I2C_MST_CNTL_CYCLE_TRIGGER) ||
> + (val & I2C_MST_CNTL_STATUS) != I2C_MST_CNTL_STATUS_BUS_BUSY,
> + 500, 1000 * USEC_PER_MSEC);
> +
> + if (ret) {
>   dev_err(i2cd->dev, "i2c timeout error %x\n", val);
>   return -ETIMEDOUT;
>   }
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

NAK[F/Unstable]: [SRU] [E/F/Unstable/OEM-OSP1-B] [PATCH 0/1] Fix Nvidia's ucsi_ccg to make system be able to sleep

Seth Forshee
In reply to this post by Kai-Heng Feng
On Thu, Mar 26, 2020 at 09:05:01PM +0800, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1850238
>
> [Impact]
> Some systems have a "phantom" Nvidia UCSI, which prevent systems from
> suspending.
>
> [Fix]
> ucsi_ccg is stuck in its probe routine because of the i2c bus never
> timeouts. Let it timeouts and probe can fail since it's just a phantom
> device.
>
> [Test]
> After applying this patch system can suspend/resume succesfully.
>
> [Regression Potential]
> Low. It's a trivial change to correctly handle timeout.

This is upstream now, and has made it to focal via stable updates.
Thanks!

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED[E]: [SRU] [E/F/Unstable/OEM-OSP1-B] [PATCH 0/1] Fix Nvidia's ucsi_ccg to make system be able to sleep

Kelsey Skunberg
In reply to this post by Kai-Heng Feng
On 2020-03-26 21:05:01 , Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1850238
>
> [Impact]
> Some systems have a "phantom" Nvidia UCSI, which prevent systems from
> suspending.
>
> [Fix]
> ucsi_ccg is stuck in its probe routine because of the i2c bus never
> timeouts. Let it timeouts and probe can fail since it's just a phantom
> device.
>
> [Test]
> After applying this patch system can suspend/resume succesfully.
>
> [Regression Potential]
> Low. It's a trivial change to correctly handle timeout.
>
> Kai-Heng Feng (1):
>   i2c: nvidia-gpu: Handle timeout correctly in gpu_i2c_check_status()
>
>  drivers/i2c/busses/i2c-nvidia-gpu.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
>
> --
> 2.17.1
>
>
Applied to eoan/master-next. 'linux-next' was removed from the cherry-picked
line since this patch is already merged into Linus' tree. Thank you!

-Kelsey
 
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team