Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1437753 > unrolled thread

[PATCH] regmap: add iopoll-like polling macro

Back to article view | Back to linux.kernel

Contents

  [PATCH] regmap: add iopoll-like polling macro Philipp Zabel <p.zabel@pengutronix.de> - 2016-07-06 16:20 +0200
    Re: [PATCH] regmap: add iopoll-like polling macro Mark Brown <broonie@kernel.org> - 2016-07-07 11:50 +0200
      Re: [PATCH] regmap: add iopoll-like polling macro Philipp Zabel <p.zabel@pengutronix.de> - 2016-07-07 12:10 +0200
        Re: [PATCH] regmap: add iopoll-like polling macro Mark Brown <broonie@kernel.org> - 2016-07-08 16:50 +0200
          Re: [PATCH] regmap: add iopoll-like polling macro Philipp Zabel <p.zabel@pengutronix.de> - 2016-07-08 18:30 +0200

#1437753 — [PATCH] regmap: add iopoll-like polling macro

This patch adds a macro regmap_read_poll_timeout that works similar
to the readx_poll_timeout defined in linux/iopoll.h, except that this
can also return the error value returned by a failed regmap_read.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 include/linux/regmap.h | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 3dc08ce..26914df 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -95,6 +95,45 @@ struct reg_sequence {
 #define	regmap_fields_force_update_bits(field, id, mask, val) \
 	regmap_fields_update_bits_base(field, id, mask, val, NULL, false, true)
 
+/**
+ * regmap_read_poll_timeout - Poll until a condition is met or a timeout occurs
+ * @map: Regmap to read from
+ * @addr: Address to poll
+ * @val: Unsigned integer variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @sleep_us: Maximum time to sleep between reads in us (0
+ *            tight-loops).  Should be less than ~20ms since usleep_range
+ *            is used (see Documentation/timers/timers-howto.txt).
+ * @timeout_us: Timeout in us, 0 means never timeout
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout or the regmap_read
+ * error return value in case of a error read. In the two former cases,
+ * the last read value at @addr is stored in @val. Must not be called
+ * from atomic context if sleep_us or timeout_us are used.
+ *
+ * This is modelled after the readx_poll_timeout macros in linux/iopoll.h.
+ */
+#define regmap_read_poll_timeout(map, addr, val, cond, sleep_us, timeout_us) \
+({ \
+	ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+	int ret; \
+	might_sleep_if(sleep_us); \
+	for (;;) { \
+		ret = regmap_read((map), (addr), &(val)); \
+		if (ret) \
+			break; \
+		if (cond) \
+			break; \
+		if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+			ret = regmap_read((map), (addr), &(val)); \
+			break; \
+		} \
+		if (sleep_us) \
+			usleep_range((sleep_us >> 2) + 1, sleep_us); \
+	} \
+	ret ?: ((cond) ? 0 : -ETIMEDOUT); \
+})
+
 #ifdef CONFIG_REGMAP
 
 enum regmap_endian {
-- 
2.8.1

[toc] | [next] | [standalone]

#1438472

[Multipart message — attachments visible in raw view] — view raw

On Wed, Jul 06, 2016 at 04:19:41PM +0200, Philipp Zabel wrote:

> This patch adds a macro regmap_read_poll_timeout that works similar
> to the readx_poll_timeout defined in linux/iopoll.h, except that this
> can also return the error value returned by a failed regmap_read.

Please make this a proper function.

> +		if (sleep_us) \
> +			usleep_range((sleep_us >> 2) + 1, sleep_us); \

Just write / 4 rather than a shift, the compiler should do the right
thing and it's easier for readers.

[toc] | [prev] | [next] | [standalone]

#1438487

Hi Mark,

thank you for the comments.

Am Donnerstag, den 07.07.2016, 11:42 +0200 schrieb Mark Brown:
> On Wed, Jul 06, 2016 at 04:19:41PM +0200, Philipp Zabel wrote:
> 
> > This patch adds a macro regmap_read_poll_timeout that works similar
> > to the readx_poll_timeout defined in linux/iopoll.h, except that this
> > can also return the error value returned by a failed regmap_read.
> 
> Please make this a proper function.

I can't, the condition has to be evaluated inside the loop. This is
basically a poor man's function template.

> > +		if (sleep_us) \
> > +			usleep_range((sleep_us >> 2) + 1, sleep_us); \
> 
> Just write / 4 rather than a shift, the compiler should do the right
> thing and it's easier for readers.

Ok. Since this is copied from the readx_poll_timeout macro in
include/linux/iopoll.h, I should probably suggest the same change there.

regards
Philipp

[toc] | [prev] | [next] | [standalone]

#1439541

[Multipart message — attachments visible in raw view] — view raw

On Thu, Jul 07, 2016 at 12:01:43PM +0200, Philipp Zabel wrote:
> Am Donnerstag, den 07.07.2016, 11:42 +0200 schrieb Mark Brown:
> > On Wed, Jul 06, 2016 at 04:19:41PM +0200, Philipp Zabel wrote:

> > > This patch adds a macro regmap_read_poll_timeout that works similar
> > > to the readx_poll_timeout defined in linux/iopoll.h, except that this
> > > can also return the error value returned by a failed regmap_read.

> > Please make this a proper function.

> I can't, the condition has to be evaluated inside the loop. This is
> basically a poor man's function template.

Given that the condition is always going to be essentially the same one
checking that (read & mask) == value we could just parameterize it
couldn't we?

[toc] | [prev] | [next] | [standalone]

#1439642

Am Freitag, den 08.07.2016, 16:39 +0200 schrieb Mark Brown:
> On Thu, Jul 07, 2016 at 12:01:43PM +0200, Philipp Zabel wrote:
> > Am Donnerstag, den 07.07.2016, 11:42 +0200 schrieb Mark Brown:
> > > On Wed, Jul 06, 2016 at 04:19:41PM +0200, Philipp Zabel wrote:
> 
> > > > This patch adds a macro regmap_read_poll_timeout that works similar
> > > > to the readx_poll_timeout defined in linux/iopoll.h, except that this
> > > > can also return the error value returned by a failed regmap_read.
> 
> > > Please make this a proper function.
> 
> > I can't, the condition has to be evaluated inside the loop. This is
> > basically a poor man's function template.
> 
> Given that the condition is always going to be essentially the same one
> checking that (read & mask) == value we could just parameterize it
> couldn't we?

The iopoll macros also allow comparisons like < > !=, more complicated
expressions, or even function calls.

Granted, the only existing example I am aware of is
drivers/dma/qcom/hidma_ll.c, which calls a static function in
readl_poll_wait_timeout, that evaluates to ((read == 1) || (read == 2)).

There's a similar condition in rt5659_headset_detect, although that may
be dismissed as it's using snd_soc_read and variable delays:
        while (i < 5) {
                msleep(sleep_time[i]);
                val = snd_soc_read(codec, RT5659_EJD_CTRL_2) & 0x0003;
                i++;
                if (val == 0x1 || val == 0x2 || val == 0x3)
                        break;
        }

A potential user for an inequality condition is
drivers/media/tuners/it913x.c, which has a loop where the condition is
(read != 0):
                #define TIMEOUT 50
                timeout = jiffies + msecs_to_jiffies(TIMEOUT);
                while (!time_after(jiffies, timeout)) {
                        ret = regmap_read(dev->regmap, 0x80ec82, &utmp);
                        if (ret)
                                goto err;

                        if (utmp)
                                break;
                }

Even if by far the most common cases can be covered as you suggest,
I would find it useful to keep this the same as the other
readx_poll_wait_timeout variants.

regards
Philipp

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web