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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <53C03E27.3000003@linaro.org>
Date:	Fri, 11 Jul 2014 20:42:31 +0100
From:	Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To:	Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
CC:	Kishon Vijay Abraham I <kishon@...com>,
	Grant Likely <grant.likely@...aro.org>,
	Rob Herring <robh+dt@...nel.org>, linux-kernel@...r.kernel.org,
	linux-arm-msm@...r.kernel.org, linux-ide@...r.kernel.org
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/