[PATCH] regmap: add iopoll-like atomic polling macro

[PATCH] regmap: add iopoll-like atomic polling macro

[Sameer Pujar]

This patch adds a macro 'regmap_read_poll_timeout_atomic' that works
similar to 'readx_poll_timeout_atomic' defined in linux/iopoll.h; This
is atomic version of already available 'regmap_read_poll_timeout' macro.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
 include/linux/regmap.h | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index dfe493a..09d79ea 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -145,6 +145,47 @@ struct reg_sequence {
 })
 
 /**
+ * regmap_read_poll_timeout_atomic - 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)
+ * @delay_us: Time to udelay between reads in us (0 tight-loops).
+ *            Should be less than ~10us since udelay is used
+ *            (see Documentation/timers/timers-howto.rst).
+ * @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.
+ *
+ * This is modelled after the readx_poll_timeout_atomic macros in linux/iopoll.h.
+ */
+#define regmap_read_poll_timeout_atomic(map, addr, val, cond, delay_us, timeout_us) \
+({ \
+	u64 __timeout_us = (timeout_us); \
+	unsigned long __delay_us = (delay_us); \
+	ktime_t __timeout = ktime_add_us(ktime_get(), __timeout_us); \
+	int __ret; \
+	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 (__delay_us) \
+			udelay(__delay_us); \
+	} \
+	__ret ?: ((cond) ? 0 : -ETIMEDOUT); \
+})
+
+/**
  * regmap_field_read_poll_timeout - Poll until a condition is met or timeout
  *
  * @field: Regmap field to read from
-- 
2.7.4

Re: [PATCH] regmap: add iopoll-like atomic polling macro

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 629 bytes --]

On Tue, Jan 07, 2020 at 03:58:09PM +0530, Sameer Pujar wrote:
> This patch adds a macro 'regmap_read_poll_timeout_atomic' that works
> similar to 'readx_poll_timeout_atomic' defined in linux/iopoll.h; This
> is atomic version of already available 'regmap_read_poll_timeout' macro.

In general regmap really can't be used in atomic contexts - we do have
options to configure a regmap so it can be used there but they're not
the default.  It'd be better if the comment mentioned this and warned
against use with normal regmaps so people are less likely to try to use
this in an atomic context when the regmap doesn't support that.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Re: [PATCH] regmap: add iopoll-like atomic polling macro

[Sameer Pujar]

On 1/7/2020 5:39 PM, Mark Brown wrote:
> On Tue, Jan 07, 2020 at 03:58:09PM +0530, Sameer Pujar wrote:
>> This patch adds a macro 'regmap_read_poll_timeout_atomic' that works
>> similar to 'readx_poll_timeout_atomic' defined in linux/iopoll.h; This
>> is atomic version of already available 'regmap_read_poll_timeout' macro.
> In general regmap really can't be used in atomic contexts - we do have
> options to configure a regmap so it can be used there but they're not
> the default.  It'd be better if the comment mentioned this and warned
> against use with normal regmaps so people are less likely to try to use
> this in an atomic context when the regmap doesn't support that.

Oh I see.

While using regmap_read_poll_timeout() in snd_soc_dai_ops trigger 
callback, I was hitting below print.
"BUG: scheduling while atomic" and kernel panic there after.

While checking the documentation on snd_soc_dai_ops, it appears 
trigger() is in atomic context.
This means I cannot use regmap interface (with default configuration) in 
trigger().
The above issue went away with usage of 
regmap_read_poll_timeout_atomic() and now I think I just got lucky.

Also, with the limited testing, I did not see the issue in current 
linux-next, where as I hit above issue in
older kernel. Has anything changed with respect to above?

Though I need to test this, I guess there is a way to use non-atomic 
versions of PCM operations by setting
'nonatomic' flag in snd_pcm and continue to use 
regmap_read_poll_timeout(). In that case new macro may not
be necessary.

Re: Re: [PATCH] regmap: add iopoll-like atomic polling macro

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 566 bytes --]

On Wed, Jan 08, 2020 at 10:20:37AM +0530, Sameer Pujar wrote:

> While checking the documentation on snd_soc_dai_ops, it appears trigger() is
> in atomic context.
> This means I cannot use regmap interface (with default configuration) in
> trigger().
> The above issue went away with usage of regmap_read_poll_timeout_atomic()
> and now I think I just got lucky.

It's fine if your regmap is set up to support atomic use (using a flat
or no cache and MMIO) but not for regmaps in general, I'm asking you to
document the fact that it's only usable with some regmaps.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]