2020-04-20 14:58:36

by Dejin Zheng

Subject: [PATCH v2 1/2] regmap: Simplify implementation of the regmap_read_poll_timeout() macro

Simplify the implementation of the macro regmap_read_poll_timeout()
by using the macro read_poll_timeout().

Signed-off-by: Dejin Zheng <[email protected]>
---
v1 -> v2:
- modify the commit comments by Markus's suggestion .

include/linux/regmap.h | 25 +++++--------------------
1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 40b07168fd8e..299c1f6a03b4 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -17,6 +17,7 @@
#include <linux/err.h>
#include <linux/bug.h>
#include <linux/lockdep.h>
+#include <linux/iopoll.h>

struct module;
struct clk;
@@ -122,26 +123,10 @@ struct reg_sequence {
*/
#define regmap_read_poll_timeout(map, addr, val, cond, sleep_us, timeout_us) \
({ \
- u64 __timeout_us = (timeout_us); \
- unsigned long __sleep_us = (sleep_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); \
+ int __ret, __tmp; \
+ __tmp = read_poll_timeout(regmap_read, __ret, __ret || (cond), \
+ sleep_us, timeout_us, false, (map), (addr), &(val)); \
+ __ret ?: __tmp; \
})

/**
--
2.25.0

2020-04-20 15:14:29

by Markus Elfring

Subject: Re: [PATCH v2 1/2] regmap: Simplify implementation of the regmap_read_poll_timeout() macro

…
> +++ b/include/linux/regmap.h
…
> @@ -122,26 +123,10 @@ struct reg_sequence {
> */
> #define regmap_read_poll_timeout(map, addr, val, cond, sleep_us, timeout_us) \
> ({ \
…
> + int __ret, __tmp; \
> + __tmp = read_poll_timeout(regmap_read, __ret, __ret || (cond), \
> + sleep_us, timeout_us, false, (map), (addr), &(val)); \
> + __ret ?: __tmp; \
> })

* Would you like to delete double underscores from these variable names?

* I find another implementation detail suspicious.
Should the parameters “sleep_us” and “timeout_us” be enclosed by
additional parentheses (similar to four other macro arguments)?

* Can the tag “Fixes” be relevant also for such adjustments?

Regards,
Markus