Messages in this thread | | | Date | Wed, 16 Jul 2014 07:28:16 +0100 | From | Srinivas Kandagatla <> | Subject | Re: [PATCH v4 1/2] phy: qcom: Add driver for QCOM APQ8064 SATA PHY |
| |
On 15/07/14 17:56, Bartlomiej Zolnierkiewicz wrote: >> + >> >+/* Helper function to do poll and timeout */ >> >+static int read_poll_timeout(void __iomem *addr, u32 mask) >> >+{ >> >+ unsigned long timeout = jiffies + msecs_to_jiffies(TIMEOUT_MS); >> >+ >> >+ do { >> >+ if (readl_relaxed(addr) & mask) >> >+ return 0; >> >+ >> >+ usleep_range(DELAY_INTERVAL_US, DELAY_INTERVAL_US + 50); >> >+ } while (!time_after(jiffies, timeout)); >> >+ >> >+ return -ETIMEDOUT; >> >+} > Thanks for reworking this code, unfortunately it still has a one > (unlikely but still theoretically possible) problem. If there is > i.e. a big IRQ load between first usleep_range() call and first
Very unlikely but as you said it possible in theory :-)
> time_after() check the function will timeout without checking > the register. To fix it you needs to add an additonal register > checking before returning -ETIMEDOUT value or replace time_after() > condition with a fixed number of retries (100000 to cover 1sec > timeout). I will send out a fix on top of my previous patches to fix this.
thanks, srini
| |