[U-Boot] [PATCH v4 0/5] Add wait_for_bit()
Changes in v4: - Rename timeout -> timeout_ms - foolproof code is good :) - Re-compiled it for affected platforms (just to be sure)
I did not do remaining improvements - they look like good ideas and will do them in future as separate series. Possibly migrating more of u-boot code to it on the way.
Changes in V3: - wait_for_bit code is moved to header and converted to static inline function. Previous version added ~7byte of rodata to all boards (even not using wait_for_bit).
Changes in V2: - wait_bit.o is always compiled in - Removed CONFIG_LIB_WAIT_BIT from configs/Kconfigs - Constified arguments to wait_bit - Removed check for CONFIG_... in drivers - Added tested-by to ohci-lp32xx @Sylvain Lemieux: I didn't changed driver logic with v2, so allowed myself to add your tested-by directly.
Tested on: - USB driver on Dragonboard (not yet in mainline) - Infinite sleep (if timeout/interruption works) Build tested on single board for each driver/commit: - hikey - zynq_microzed - platinum
(Old) notes from V1:
This series add generic function to poll register waiting for one or more bits to change.
Very similar function was used in several drivers: - dwc2 - ohci-lp32xx - ehci-mx6 - zynq_gem
First patch adds function, following patches update drivers and board config files / defconfigs.
This series was compile-tested with buildman for ~50 boards (most or even all boards affected by change)
Code was also run-tested on ehci-msm driver (not yet in mainline)
There is single difference in behavior: ohci-lp32xx driver will not print "Timeout..." message with debug disabled. I think it's not a big issue as this driver seems unused, but if it's an issue - please drop that patch.
Mateusz Kulikowski (5): lib: Add wait_for_bit usb: dwc2: Use shared wait_for_bit usb: ohci-lpc32xx: Use shared wait_for_bit usb: ehci-mx6: Use shared wait_for_bit net: zynq_gem: Use shared wait_for_bit
drivers/net/zynq_gem.c | 35 ++------------------ drivers/usb/host/dwc2.c | 41 ++++++++---------------- drivers/usb/host/ehci-mx6.c | 32 +++---------------- drivers/usb/host/ohci-lpc32xx.c | 34 ++++---------------- include/wait_bit.h | 71 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 97 insertions(+), 116 deletions(-) create mode 100644 include/wait_bit.h
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
diff --git a/include/wait_bit.h b/include/wait_bit.h new file mode 100644 index 0000000..061a2db --- /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_ms 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_ms, + const bool breakable) +{ + 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_ms) + break; + + if (breakable && ctrlc()) { + puts("Abort\n"); + return -EINTR; + } + + udelay(1); + } + + debug("%s: Timeout (reg=%p mask=%08x wait_set=%i)\n", prefix, reg, mask, + set); + + return -ETIMEDOUT; +} + + +#endif
On Sat, Jan 23, 2016 at 11:54:29AM +0100, Mateusz Kulikowski 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
Reviewed-by: Tom Rini trini@konsulko.com
On Sat, Jan 23, 2016 at 11:54:29AM +0100, Mateusz Kulikowski 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 Reviewed-by: Tom Rini trini@konsulko.com
Applied to u-boot/master, thanks!
Use existing library function to poll bit(s).
Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com ---
drivers/usb/host/dwc2.c | 41 +++++++++++++---------------------------- 1 file changed, 13 insertions(+), 28 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index 5ef6deb..9e95ecb 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -13,6 +13,7 @@ #include <memalign.h> #include <phys2bus.h> #include <usbroothubdes.h> +#include <wait_bit.h> #include <asm/io.h>
#include "dwc2.h" @@ -52,27 +53,6 @@ static struct dwc2_priv local; /* * DWC2 IP interface */ -static int wait_for_bit(void *reg, const uint32_t mask, bool set) -{ - unsigned int timeout = 1000000; - uint32_t val; - - while (--timeout) { - val = readl(reg); - if (!set) - val = ~val; - - if ((val & mask) == mask) - return 0; - - udelay(1); - } - - debug("%s: Timeout (reg=%p mask=%08x wait_set=%i)\n", - __func__, reg, mask, set); - - return -ETIMEDOUT; -}
/* * Initializes the FSLSPClkSel field of the HCFG register @@ -117,7 +97,8 @@ static void dwc_otg_flush_tx_fifo(struct dwc2_core_regs *regs, const int num)
writel(DWC2_GRSTCTL_TXFFLSH | (num << DWC2_GRSTCTL_TXFNUM_OFFSET), ®s->grstctl); - ret = wait_for_bit(®s->grstctl, DWC2_GRSTCTL_TXFFLSH, 0); + ret = wait_for_bit(__func__, ®s->grstctl, DWC2_GRSTCTL_TXFFLSH, + false, 1000, false); if (ret) printf("%s: Timeout!\n", __func__);
@@ -135,7 +116,8 @@ static void dwc_otg_flush_rx_fifo(struct dwc2_core_regs *regs) int ret;
writel(DWC2_GRSTCTL_RXFFLSH, ®s->grstctl); - ret = wait_for_bit(®s->grstctl, DWC2_GRSTCTL_RXFFLSH, 0); + ret = wait_for_bit(__func__, ®s->grstctl, DWC2_GRSTCTL_RXFFLSH, + false, 1000, false); if (ret) printf("%s: Timeout!\n", __func__);
@@ -152,13 +134,15 @@ static void dwc_otg_core_reset(struct dwc2_core_regs *regs) int ret;
/* Wait for AHB master IDLE state. */ - ret = wait_for_bit(®s->grstctl, DWC2_GRSTCTL_AHBIDLE, 1); + ret = wait_for_bit(__func__, ®s->grstctl, DWC2_GRSTCTL_AHBIDLE, + true, 1000, false); if (ret) printf("%s: Timeout!\n", __func__);
/* Core Soft Reset */ writel(DWC2_GRSTCTL_CSFTRST, ®s->grstctl); - ret = wait_for_bit(®s->grstctl, DWC2_GRSTCTL_CSFTRST, 0); + ret = wait_for_bit(__func__, ®s->grstctl, DWC2_GRSTCTL_CSFTRST, + false, 1000, false); if (ret) printf("%s: Timeout!\n", __func__);
@@ -243,8 +227,8 @@ static void dwc_otg_core_host_init(struct dwc2_core_regs *regs) clrsetbits_le32(®s->hc_regs[i].hcchar, DWC2_HCCHAR_EPDIR, DWC2_HCCHAR_CHEN | DWC2_HCCHAR_CHDIS); - ret = wait_for_bit(®s->hc_regs[i].hcchar, - DWC2_HCCHAR_CHEN, 0); + ret = wait_for_bit(__func__, ®s->hc_regs[i].hcchar, + DWC2_HCCHAR_CHEN, false, 1000, false); if (ret) printf("%s: Timeout!\n", __func__); } @@ -737,7 +721,8 @@ int wait_for_chhltd(struct dwc2_core_regs *regs, uint32_t *sub, int *toggle, int ret; uint32_t hcint, hctsiz;
- ret = wait_for_bit(&hc_regs->hcint, DWC2_HCINT_CHHLTD, true); + ret = wait_for_bit(__func__, &hc_regs->hcint, DWC2_HCINT_CHHLTD, true, + 1000, false); if (ret) return ret;
On Sat, Jan 23, 2016 at 11:54:30AM +0100, Mateusz Kulikowski wrote:
Use existing library function to poll bit(s).
Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com
Applied to u-boot/master, thanks!
Use existing library function to poll bit(s).
Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com Tested-by: Sylvain Lemieux slemieux@tycoint.com ---
drivers/usb/host/ohci-lpc32xx.c | 34 +++++++--------------------------- 1 file changed, 7 insertions(+), 27 deletions(-)
diff --git a/drivers/usb/host/ohci-lpc32xx.c b/drivers/usb/host/ohci-lpc32xx.c index 48d338e..9245126 100644 --- a/drivers/usb/host/ohci-lpc32xx.c +++ b/drivers/usb/host/ohci-lpc32xx.c @@ -10,6 +10,7 @@
#include <common.h> #include <errno.h> +#include <wait_bit.h> #include <asm/io.h> #include <asm/arch/cpu.h> #include <asm/arch/clk.h> @@ -80,30 +81,6 @@ struct otg_regs { static struct otg_regs *otg = (struct otg_regs *)USB_BASE; static struct clk_pm_regs *clk_pwr = (struct clk_pm_regs *)CLK_PM_BASE;
-static int wait_for_bit(void *reg, const u32 mask, bool set) -{ - 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) > CONFIG_SYS_HZ) - break; - - udelay(1); - } - - error("Timeout (reg=%p mask=%08x wait_set=%i)\n", reg, mask, set); - - return -ETIMEDOUT; -} - static int isp1301_set_value(int reg, u8 value) { return i2c_write(ISP1301_I2C_ADDR, reg, 1, &value, 1); @@ -158,7 +135,8 @@ static int usbpll_setup(void) setbits_le32(&clk_pwr->usb_ctrl, CLK_USBCTRL_POSTDIV_2POW(0x01)); setbits_le32(&clk_pwr->usb_ctrl, CLK_USBCTRL_PLL_PWRUP);
- ret = wait_for_bit(&clk_pwr->usb_ctrl, CLK_USBCTRL_PLL_STS, 1); + ret = wait_for_bit(__func__, &clk_pwr->usb_ctrl, CLK_USBCTRL_PLL_STS, + true, CONFIG_SYS_HZ, false); if (ret) return ret;
@@ -183,7 +161,8 @@ int usb_cpu_init(void)
/* enable I2C clock */ writel(OTG_CLK_I2C_EN, &otg->otg_clk_ctrl); - ret = wait_for_bit(&otg->otg_clk_sts, OTG_CLK_I2C_EN, 1); + ret = wait_for_bit(__func__, &otg->otg_clk_sts, OTG_CLK_I2C_EN, true, + CONFIG_SYS_HZ, false); if (ret) return ret;
@@ -203,7 +182,8 @@ int usb_cpu_init(void) OTG_CLK_I2C_EN | OTG_CLK_HOST_EN; writel(mask, &otg->otg_clk_ctrl);
- ret = wait_for_bit(&otg->otg_clk_sts, mask, 1); + ret = wait_for_bit(__func__, &otg->otg_clk_sts, mask, true, + CONFIG_SYS_HZ, false); if (ret) return ret;
On Sat, Jan 23, 2016 at 11:54:31AM +0100, Mateusz Kulikowski wrote:
Use existing library function to poll bit(s).
Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com Tested-by: Sylvain Lemieux slemieux@tycoint.com
Applied to u-boot/master, thanks!
Use existing library function to poll bit(s).
Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com ---
drivers/usb/host/ehci-mx6.c | 32 ++++---------------------------- 1 file changed, 4 insertions(+), 28 deletions(-)
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index 2666351..e1c67f7 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -8,6 +8,7 @@ #include <common.h> #include <usb.h> #include <errno.h> +#include <wait_bit.h> #include <linux/compiler.h> #include <usb/ehci-fsl.h> #include <asm/io.h> @@ -117,32 +118,6 @@ static void usb_power_config(int index) pll_480_ctrl_set); }
-static int wait_for_bit(u32 *reg, const u32 mask, bool set) -{ - u32 val; - const unsigned int timeout = 10000; - 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; - - udelay(1); - } - - debug("%s: Timeout (reg=%p mask=%08x wait_set=%i)\n", - __func__, reg, mask, set); - - return -ETIMEDOUT; -} - /* Return 0 : host node, <>0 : device mode */ static int usb_phy_enable(int index, struct usb_ehci *ehci) { @@ -160,12 +135,13 @@ static int usb_phy_enable(int index, struct usb_ehci *ehci)
/* Stop then Reset */ clrbits_le32(usb_cmd, UCMD_RUN_STOP); - ret = wait_for_bit(usb_cmd, UCMD_RUN_STOP, 0); + ret = wait_for_bit(__func__, usb_cmd, UCMD_RUN_STOP, false, 10000, + false); if (ret) return ret;
setbits_le32(usb_cmd, UCMD_RESET); - ret = wait_for_bit(usb_cmd, UCMD_RESET, 0); + ret = wait_for_bit(__func__, usb_cmd, UCMD_RESET, false, 10000, false); if (ret) return ret;
On Sat, Jan 23, 2016 at 11:54:32AM +0100, Mateusz Kulikowski wrote:
Use existing library function to poll bit(s).
Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com
Applied to u-boot/master, thanks!
Use existing library function to poll bit(s). Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com ---
drivers/net/zynq_gem.c | 35 ++--------------------------------- 1 file changed, 2 insertions(+), 33 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 7059c84..97e30f3 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -19,6 +19,7 @@ #include <asm/io.h> #include <phy.h> #include <miiphy.h> +#include <wait_bit.h> #include <watchdog.h> #include <asm/system.h> #include <asm/arch/hardware.h> @@ -448,38 +449,6 @@ static int zynq_gem_init(struct udevice *dev) return 0; }
-static int wait_for_bit(const char *func, u32 *reg, const u32 mask, - bool set, unsigned int timeout) -{ - 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 (ctrlc()) { - puts("Abort\n"); - return -EINTR; - } - - udelay(1); - } - - debug("%s: Timeout (reg=%p mask=%08x wait_set=%i)\n", - func, reg, mask, set); - - return -ETIMEDOUT; -} - static int zynq_gem_send(struct udevice *dev, void *ptr, int len) { u32 addr, size; @@ -521,7 +490,7 @@ static int zynq_gem_send(struct udevice *dev, void *ptr, int len) printf("TX buffers exhausted in mid frame\n");
return wait_for_bit(__func__, ®s->txsr, ZYNQ_GEM_TSR_DONE, - true, 20000); + true, 20000, true); }
/* Do not check frame_recd flag in rx_status register 0x20 - just poll BD */
On Sat, Jan 23, 2016 at 11:54:33AM +0100, Mateusz Kulikowski wrote:
Use existing library function to poll bit(s). Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com
Applied to u-boot/master, thanks!
participants (2)
-
Mateusz Kulikowski -
Tom Rini