Re: [PATCH v2 1/2] phy: qcom: Add driver for QCOM APQ8064 SATA PHY

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux