Hi Kai-Heng > -----Original Message----- > From: linux-i2c-owner@xxxxxxxxxxxxxxx <linux-i2c-owner@xxxxxxxxxxxxxxx> > On Behalf Of Kai-Heng Feng > Sent: Monday, March 23, 2020 10:21 AM > To: Ajay Gupta <ajayg@xxxxxxxxxx> > Cc: Wolfram Sang <wsa@xxxxxxxxxxxxx>; Andy Shevchenko > <andriy.shevchenko@xxxxxxxxxxxxxxx>; open list:I2C CONTROLLER DRIVER > FOR NVIDIA GPU <linux-i2c@xxxxxxxxxxxxxxx>; open list <linux- > kernel@xxxxxxxxxxxxxxx> > Subject: Re: [PATCH] i2c: nvidia-gpu: Handle timeout correctly in > gpu_i2c_check_status() > > External email: Use caution opening links or attachments > > > Hi Ajay, > > > On Mar 24, 2020, at 00:47, Ajay Gupta <ajayg@xxxxxxxxxx> wrote: > > > > Kai-Heng > > > >> -----Original Message----- > >> From: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> > >> Sent: Sunday, March 22, 2020 10:38 PM > >> To: Ajay Gupta <ajayg@xxxxxxxxxx> > >> Cc: Wolfram Sang <wsa@xxxxxxxxxxxxx>; Andy Shevchenko > >> <andriy.shevchenko@xxxxxxxxxxxxxxx>; open list:I2C CONTROLLER DRIVER > >> FOR NVIDIA GPU <linux-i2c@xxxxxxxxxxxxxxx>; open list <linux- > >> kernel@xxxxxxxxxxxxxxx> > >> Subject: Re: [PATCH] i2c: nvidia-gpu: Handle timeout correctly in > >> gpu_i2c_check_status() > >> > >> External email: Use caution opening links or attachments > >> > >> > >>> On Mar 12, 2020, at 00:58, Kai-Heng Feng > >>> <kai.heng.feng@xxxxxxxxxxxxx> > >> wrote: > >>> > >>> Nvidia card may come with a "phantom" UCSI device, and its driver > >>> gets stuck in probe routine, prevents any system PM operations like > suspend. > >>> > >>> Let's handle the unaccounted case that the target time equals to > >>> jiffies in gpu_i2c_check_status(), so the UCSI driver can let the > >>> probe fail as it should. > > If status is not seen in 999.5 ms then I don't see any reason why it > > will come exactly at 1000ms. In fact, we expect status to be seen > > within 160ms as per I2C_MST_I2C0_TIMING_TIMEOUT_CLK_CNT (16 > cycle) and > > I2C_MST_I2C0_TIMING_SCL_PERIOD_100KHZ (10ms/cycle) programmed in > > I2C_MST_I2C0_TIMING Register. We already have enough extra time to > > look For response. > > This is to handle when there's no response. > > When the while loop terminates because of "time_is_after_jiffies(target)" > (i.e. target <= jiffies), we also need to to handle "target == jiffies" case in the > following if clause to properly timeout. Ok got it. The change looks fine to me. Acked-by: Ajay Gupta <ajayg@xxxxxxxxxx> Thanks > nvpublic > > I don't think I2C timings here can affect jiffies. > > Kai-Heng > > > > > Thanks > >> nvpublic > >>> > >>> Fixes: c71bcdcb42a7 ("i2c: add i2c bus driver for NVIDIA GPU") > >>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> > >> > >> A gentle ping... > >> > >>> --- > >>> drivers/i2c/busses/i2c-nvidia-gpu.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c > >>> b/drivers/i2c/busses/i2c-nvidia-gpu.c > >>> index 62e18b4db0ed..1988e93c7925 100644 > >>> --- a/drivers/i2c/busses/i2c-nvidia-gpu.c > >>> +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c > >>> @@ -88,7 +88,7 @@ static int gpu_i2c_check_status(struct gpu_i2c_dev > >> *i2cd) > >>> usleep_range(500, 600); > >>> } while (time_is_after_jiffies(target)); > >>> > >>> - if (time_is_before_jiffies(target)) { > >>> + if (time_is_before_eq_jiffies(target)) { > >>> dev_err(i2cd->dev, "i2c timeout error %x\n", val); > >>> return -ETIMEDOUT; > >>> } > >>> -- > >>> 2.17.1 > >>> > >