[PATCH v2 0/2] use read_poll_timeout macro to simplify code

[PATCH v2 0/2] use read_poll_timeout macro to simplify code

[Dejin Zheng]

Simplify implementation of regmap_read_poll_timeout() macro and
regmap_field_read_poll_timeout by read_poll_timeout() macro, they have
many similar codes.

v1 -> v2:
	- modify the commit comments by Markus's suggestion .

Dejin Zheng (2):
  regmap: Simplify implementation of the regmap_read_poll_timeout()
    macro
  regmap: Simplify implementation of the
    regmap_field_read_poll_timeout() macro

 include/linux/regmap.h | 48 ++++++++----------------------------------
 1 file changed, 9 insertions(+), 39 deletions(-)

-- 
2.25.0

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

[Dejin Zheng]

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

Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com>
---
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

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

[Markus Elfring]

…
> +++ 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

[PATCH v2 2/2] regmap: Simplify implementation of the regmap_field_read_poll_timeout() macro

[Dejin Zheng]

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

Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com>
---
v1 -> v2:
	- modify the commit comments by Markus's suggestion .

 include/linux/regmap.h | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 299c1f6a03b4..78ddf224f988 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -194,25 +194,10 @@ struct reg_sequence {
  */
 #define regmap_field_read_poll_timeout(field, 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 pollret; \
-	might_sleep_if(__sleep_us); \
-	for (;;) { \
-		pollret = regmap_field_read((field), &(val)); \
-		if (pollret) \
-			break; \
-		if (cond) \
-			break; \
-		if (__timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
-			pollret = regmap_field_read((field), &(val)); \
-			break; \
-		} \
-		if (__sleep_us) \
-			usleep_range((__sleep_us >> 2) + 1, __sleep_us); \
-	} \
-	pollret ?: ((cond) ? 0 : -ETIMEDOUT); \
+	int __ret, __tmp; \
+	__tmp = read_poll_timeout(regmap_field_read, __ret, __ret || (cond), \
+			sleep_us, timeout_us, false, (field), &(val)); \
+	__ret ?: __tmp; \
 })
 
 #ifdef CONFIG_REGMAP
-- 
2.25.0

Re: [PATCH v2 0/2] use read_poll_timeout macro to simplify code

[Mark Brown]

On Mon, 20 Apr 2020 21:46:45 +0800, Dejin Zheng wrote:
> Simplify implementation of regmap_read_poll_timeout() macro and
> regmap_field_read_poll_timeout by read_poll_timeout() macro, they have
> many similar codes.
> 
> v1 -> v2:
> 	- modify the commit comments by Markus's suggestion .
> 
> [...]

Applied to

	https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-5.8

Thanks!

[1/2] regmap: Simplify implementation of the regmap_read_poll_timeout() macro
      commit: e44ab4e14d6f4c448ae555132090c1a116b19e5c
[2/2] regmap: Simplify implementation of the regmap_field_read_poll_timeout() macro
      commit: 148c01d176237115d9c2805f6d29c0b6a72fbd10

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark