Hi,
On 27 December 2015 at 10:28, Mateusz Kulikowski mateusz.kulikowski@gmail.com wrote:
Add function to poll register waiting for specific bit(s). Similar functions are implemented in few drivers - they are almost identical and can be generalized. Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com
include/wait_bit.h | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 include/wait_bit.h
Sorry I only just saw this, but thought I'd make a few comments.
diff --git a/include/wait_bit.h b/include/wait_bit.h new file mode 100644 index 0000000..4867ced --- /dev/null +++ b/include/wait_bit.h @@ -0,0 +1,71 @@ +/*
- Wait for bit with timeout and ctrlc
- (C) Copyright 2015 Mateusz Kulikowski mateusz.kulikowski@gmail.com
- SPDX-License-Identifier: GPL-2.0+
- */
+#ifndef __WAIT_BIT_H +#define __WAIT_BIT_H
+#include <common.h> +#include <console.h> +#include <asm/errno.h> +#include <asm/io.h>
+/**
- wait_for_bit() waits for bit set/cleared in register
- Function polls register waiting for specific bit(s) change
- (either 0->1 or 1->0). It can fail under two conditions:
- Timeout
- User interaction (CTRL-C)
- Function succeeds only if all bits of masked register are set/cleared
- (depending on set option).
- @param prefix Prefix added to timeout messagge (message visible only
with debug enabled)
- @param reg Register that will be read (using readl())
- @param mask Bit(s) of register that must be active
- @param set Selects wait condition (bit set or clear)
- @param timeout Timeout (in miliseconds)
- @param breakable Enables CTRL-C interruption
- @return 0 on success, -ETIMEDOUT or -EINTR on failure
- */
+static inline int wait_for_bit(const char *prefix, const u32 *reg,
const u32 mask, const bool set,const unsigned int timeout,
timeout_ms would be more obvious
const bool breakable)
Wow this is a pretty big inline function.
Do you need the 'prefix' parameter? It seems that the callers print messages anyway. How about adding a flags word for @set and @breakable? Those params could then be combined, and you end up with 4 parameters instead of 6.
+{
u32 val;unsigned long start = get_timer(0);while (1) {val = readl(reg);if (!set)val = ~val;if ((val & mask) == mask)return 0;if (get_timer(start) > timeout)break;if (breakable && ctrlc()) {puts("Abort\n");
This is bad if used from drivers. We try not to output things. It it necessary?
return -EINTR;}udelay(1);}debug("%s: Timeout (reg=%p mask=%08x wait_set=%i)\n", prefix, reg, mask,set);return -ETIMEDOUT;+}
+#endif
2.5.0
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot