| Message ID | 1578392889-16587-1-git-send-email-spujar@nvidia.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | regmap: add iopoll-like atomic polling macro | expand |
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.
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.
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.
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
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(+)