Messages in this thread | | | Date | Fri, 11 Jul 2014 20:42:31 +0100 | From | Srinivas Kandagatla <> | Subject | Re: [PATCH v2 1/2] phy: qcom: Add driver for QCOM APQ8064 SATA PHY |
| |
Thanks Bartlomiej for the advice,
I will give it a try and see.
On 11/07/14 15:33, Bartlomiej Zolnierkiewicz wrote: >>>> struct qcom_apq8064_sata_phy { >>>> > >>+ void __iomem *mmio; >>>> > >>+ struct clk *cfg_clk; >>>> > >>+ struct device *dev; >>>> > >>+}; >>>> > >>+ >>>> > >>+/* 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); >>>> > >>+ u32 val; >>>> > >>+ >>>> > >>+ do { >>>> > >>+ cpu_relax(); >>>> > >>+ val = readl_relaxed(addr); >>>> > >>+ if (val & mask) >>>> > >>+ break; >>>> > >>+ } while (!time_after(jiffies, timeout)); >>> > > >>> > >It would be better to use usleep_[range]() (or even msleep() if needed) >>> > >instead of just using cpu_relax(). >> > >> >We really want to poll the register here, usleep/msleep would be useful >> >if we know already know how much delay is required, but in this case the >> >its not true. > I don't mean replacing the whole function, you can still do polling with > i.e. doing usleep_range(1000, 2000) with 1000 retries. The advantage of > doing it this way would be that processor could do some useful work or > sleep during wait time instead of just busy waiting. > > One example of many how to do it: > > drivers/i2c/busses/i2c-s3c2410.c: > > static bool is_ack(struct s3c24xx_i2c *i2c) > { > int tries; > > for (tries = 50; tries; --tries) { > if (readl(i2c->regs + S3C2410_IICCON) > & S3C2410_IICCON_IRQPEND) { > if (!(readl(i2c->regs + S3C2410_IICSTAT) > & S3C2410_IICSTAT_LASTBIT)) > return true; > } > usleep_range(1000, 2000); > } > dev_err(i2c->dev, "ack was not recieved\n"); > return false; > }
thanks, srini
| |