Hi Andy, > On Mar 24, 2020, at 21:41, Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > On Tue, Mar 24, 2020 at 09:07:12PM +0800, Kai-Heng Feng 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. >> >> When the target time equals to jiffies, it's not included by >> time_is_before_jiffies(). So let's use a boolean to make sure the >> operation is done or timeout. > > Thank you for an update, my comments below. > >> unsigned long target = jiffies + msecs_to_jiffies(1000); >> u32 val; >> + bool done = false; >> >> 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) > >> + if (!(val & I2C_MST_CNTL_CYCLE_TRIGGER) >> + || (val & I2C_MST_CNTL_STATUS) != >> + I2C_MST_CNTL_STATUS_BUS_BUSY) { > > Bad formatting. But see below. > >> + done = true; >> break; >> + } >> usleep_range(500, 600); >> } while (time_is_after_jiffies(target)); >> >> - if (time_is_before_jiffies(target)) { >> + if (!done) { >> dev_err(i2cd->dev, "i2c timeout error %x\n", val); >> return -ETIMEDOUT; >> } > > > Overall it can use simple tries since you already have sleep inside, but > moreover, you may simple switch to readl_poll_timeout() this entire > loop. Yes that can make the retry loop much simpler. I'll send a v3 based on your suggestion. Kai-Heng > > -- > With Best Regards, > Andy Shevchenko > >