* [U-Boot] [PATCH v2 0/5] Add wait_for_bit()
@ 2015-12-16 21:58 Mateusz Kulikowski
2015-12-16 21:58 ` [U-Boot] [PATCH v2 1/5] lib: Add wait_for_bit Mateusz Kulikowski
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Mateusz Kulikowski @ 2015-12-16 21:58 UTC (permalink / raw)
To: u-boot
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 | 35 ++++++++++++++++++++++++++++++++
lib/Makefile | 1 +
lib/wait_bit.c | 45 +++++++++++++++++++++++++++++++++++++++++
7 files changed, 107 insertions(+), 116 deletions(-)
create mode 100644 include/wait_bit.h
create mode 100644 lib/wait_bit.c
--
2.5.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/5] lib: Add wait_for_bit
2015-12-16 21:58 [U-Boot] [PATCH v2 0/5] Add wait_for_bit() Mateusz Kulikowski
@ 2015-12-16 21:58 ` Mateusz Kulikowski
2015-12-16 22:11 ` Marek Vasut
2015-12-16 21:58 ` [U-Boot] [PATCH v2 2/5] usb: dwc2: Use shared wait_for_bit Mateusz Kulikowski
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Mateusz Kulikowski @ 2015-12-16 21:58 UTC (permalink / raw)
To: u-boot
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 | 35 +++++++++++++++++++++++++++++++++++
lib/Makefile | 1 +
lib/wait_bit.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 81 insertions(+)
create mode 100644 include/wait_bit.h
create mode 100644 lib/wait_bit.c
diff --git a/include/wait_bit.h b/include/wait_bit.h
new file mode 100644
index 0000000..052a09b
--- /dev/null
+++ b/include/wait_bit.h
@@ -0,0 +1,35 @@
+/*
+ * 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
+
+/**
+ * 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
+ */
+int wait_for_bit(const char *prefix, const u32 *reg, const u32 mask,
+ const bool set, const unsigned int timeout,
+ const bool breakable);
+
+#endif
diff --git a/lib/Makefile b/lib/Makefile
index 1f1ff6f..6fe3ab5 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -81,6 +81,7 @@ obj-y += time.o
obj-$(CONFIG_TRACE) += trace.o
obj-$(CONFIG_LIB_UUID) += uuid.o
obj-$(CONFIG_LIB_RAND) += rand.o
+obj-y += wait_bit.o
ifdef CONFIG_SPL_BUILD
# SPL U-Boot may use full-printf, tiny-printf or none at all
diff --git a/lib/wait_bit.c b/lib/wait_bit.c
new file mode 100644
index 0000000..ac35f68
--- /dev/null
+++ b/lib/wait_bit.c
@@ -0,0 +1,45 @@
+/*
+ * Wait for bit interruptible by timeout or ctrlc
+ *
+ * (C) Copyright 2015 Mateusz Kulikowski <mateusz.kulikowski@gmail.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <common.h>
+#include <console.h>
+#include <asm/io.h>
+#include <asm/errno.h>
+
+int wait_for_bit(const char *prefix, const u32 *reg, const u32 mask,
+ const bool set, const unsigned int timeout,
+ 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)
+ 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;
+}
--
2.5.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 2/5] usb: dwc2: Use shared wait_for_bit
2015-12-16 21:58 [U-Boot] [PATCH v2 0/5] Add wait_for_bit() Mateusz Kulikowski
2015-12-16 21:58 ` [U-Boot] [PATCH v2 1/5] lib: Add wait_for_bit Mateusz Kulikowski
@ 2015-12-16 21:58 ` Mateusz Kulikowski
2015-12-16 21:58 ` [U-Boot] [PATCH v2 3/5] usb: ohci-lpc32xx: " Mateusz Kulikowski
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Mateusz Kulikowski @ 2015-12-16 21:58 UTC (permalink / raw)
To: u-boot
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 541c0f9..42d31e3 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;
--
2.5.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 3/5] usb: ohci-lpc32xx: Use shared wait_for_bit
2015-12-16 21:58 [U-Boot] [PATCH v2 0/5] Add wait_for_bit() Mateusz Kulikowski
2015-12-16 21:58 ` [U-Boot] [PATCH v2 1/5] lib: Add wait_for_bit Mateusz Kulikowski
2015-12-16 21:58 ` [U-Boot] [PATCH v2 2/5] usb: dwc2: Use shared wait_for_bit Mateusz Kulikowski
@ 2015-12-16 21:58 ` Mateusz Kulikowski
2015-12-16 21:58 ` [U-Boot] [PATCH v2 4/5] usb: ehci-mx6: " Mateusz Kulikowski
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Mateusz Kulikowski @ 2015-12-16 21:58 UTC (permalink / raw)
To: u-boot
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;
--
2.5.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 4/5] usb: ehci-mx6: Use shared wait_for_bit
2015-12-16 21:58 [U-Boot] [PATCH v2 0/5] Add wait_for_bit() Mateusz Kulikowski
` (2 preceding siblings ...)
2015-12-16 21:58 ` [U-Boot] [PATCH v2 3/5] usb: ohci-lpc32xx: " Mateusz Kulikowski
@ 2015-12-16 21:58 ` Mateusz Kulikowski
2015-12-16 21:58 ` [U-Boot] [PATCH v2 5/5] net: zynq_gem: " Mateusz Kulikowski
2015-12-17 13:57 ` [U-Boot] [PATCH v2 0/5] Add wait_for_bit() LEMIEUX, SYLVAIN
5 siblings, 0 replies; 11+ messages in thread
From: Mateusz Kulikowski @ 2015-12-16 21:58 UTC (permalink / raw)
To: u-boot
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;
--
2.5.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 5/5] net: zynq_gem: Use shared wait_for_bit
2015-12-16 21:58 [U-Boot] [PATCH v2 0/5] Add wait_for_bit() Mateusz Kulikowski
` (3 preceding siblings ...)
2015-12-16 21:58 ` [U-Boot] [PATCH v2 4/5] usb: ehci-mx6: " Mateusz Kulikowski
@ 2015-12-16 21:58 ` Mateusz Kulikowski
2015-12-17 13:57 ` [U-Boot] [PATCH v2 0/5] Add wait_for_bit() LEMIEUX, SYLVAIN
5 siblings, 0 replies; 11+ messages in thread
From: Mateusz Kulikowski @ 2015-12-16 21:58 UTC (permalink / raw)
To: u-boot
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 0a41281..430c2a4 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>
@@ -452,38 +453,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;
@@ -525,7 +494,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 */
--
2.5.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/5] lib: Add wait_for_bit
2015-12-16 21:58 ` [U-Boot] [PATCH v2 1/5] lib: Add wait_for_bit Mateusz Kulikowski
@ 2015-12-16 22:11 ` Marek Vasut
2015-12-16 23:32 ` Mateusz Kulikowski
0 siblings, 1 reply; 11+ messages in thread
From: Marek Vasut @ 2015-12-16 22:11 UTC (permalink / raw)
To: u-boot
On Wednesday, December 16, 2015 at 10:58:38 PM, 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>
> ---
>
> include/wait_bit.h | 35 +++++++++++++++++++++++++++++++++++
> lib/Makefile | 1 +
> lib/wait_bit.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 81 insertions(+)
> create mode 100644 include/wait_bit.h
> create mode 100644 lib/wait_bit.c
>
> diff --git a/include/wait_bit.h b/include/wait_bit.h
> new file mode 100644
> index 0000000..052a09b
> --- /dev/null
> +++ b/include/wait_bit.h
> @@ -0,0 +1,35 @@
> +/*
> + * 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
> +
> +/**
> + * 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
> + */
> +int wait_for_bit(const char *prefix, const u32 *reg, const u32 mask,
> + const bool set, const unsigned int timeout,
> + const bool breakable);
> +
> +#endif
> diff --git a/lib/Makefile b/lib/Makefile
> index 1f1ff6f..6fe3ab5 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -81,6 +81,7 @@ obj-y += time.o
> obj-$(CONFIG_TRACE) += trace.o
> obj-$(CONFIG_LIB_UUID) += uuid.o
> obj-$(CONFIG_LIB_RAND) += rand.o
> +obj-y += wait_bit.o
>
> ifdef CONFIG_SPL_BUILD
> # SPL U-Boot may use full-printf, tiny-printf or none at all
> diff --git a/lib/wait_bit.c b/lib/wait_bit.c
> new file mode 100644
> index 0000000..ac35f68
> --- /dev/null
> +++ b/lib/wait_bit.c
> @@ -0,0 +1,45 @@
> +/*
> + * Wait for bit interruptible by timeout or ctrlc
> + *
> + * (C) Copyright 2015 Mateusz Kulikowski <mateusz.kulikowski@gmail.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <console.h>
> +#include <asm/io.h>
> +#include <asm/errno.h>
> +
> +int wait_for_bit(const char *prefix, const u32 *reg, const u32 mask,
> + const bool set, const unsigned int timeout,
> + const bool breakable)
> +{
I wonder, what would happen if you stuffed this function into the header
file altogether ? I think this would allow the compiler to do interprocedure
optimalization on whichever file this would be included into. I wonder if
that would have any impact on the resulting code size.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/5] lib: Add wait_for_bit
2015-12-16 22:11 ` Marek Vasut
@ 2015-12-16 23:32 ` Mateusz Kulikowski
2015-12-16 23:52 ` Marek Vasut
0 siblings, 1 reply; 11+ messages in thread
From: Mateusz Kulikowski @ 2015-12-16 23:32 UTC (permalink / raw)
To: u-boot
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Hi Marek,
On 16.12.2015 23:11, Marek Vasut wrote:
> On Wednesday, December 16, 2015 at 10:58:38 PM, Mateusz Kulikowski wrote:
[...]
>> +#include <console.h>
>> +#include <asm/io.h>
>> +#include <asm/errno.h>
>> +
>> +int wait_for_bit(const char *prefix, const u32 *reg, const u32 mask,
>> + const bool set, const unsigned int timeout,
>> + const bool breakable)
>> +{
>
> I wonder, what would happen if you stuffed this function into the header
> file altogether ? I think this would allow the compiler to do interprocedure
> optimalization on whichever file this would be included into. I wonder if
> that would have any impact on the resulting code size.
>
Of course I can make it static inline.
I was suggested not to care about possible leftovers that are
not garbage-collected by linker so didn't changed that on V2.
It's (max) few bytes that may be consumed by section alignment.
Regards,
Mateusz
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQEcBAEBCAAGBQJWcfSBAAoJELvtohmVtQzB+BsIAJK6ZVnU80+rbjfzyBJCPUcJ
PtHLuz++kgkRZJevNr0RgUgiaWBs0eWfcN7FSpP3P1jUayGuE8MeYqJKbv7frlwO
lxxd+nvXJB5OiO0jZMShJjDDo/pzFeVz5iFGHi7e7Gglxbk5Q+WQ+Q2D2ZCra5SN
ktYReI4vViJWC2k+bGFYE4NL8p0OItdmT7gPpXPR8KOMg/Yq8MpQRZtCwCO3xNoU
Fj3kIwouCErF1AjHKQ1OXP7E/W6iw2jF/i1L28bnI4BX4ArACcf4QF6dNNrO63tX
jY322BHfyJG1sas2clMvq/QcA04jU2AQsx3poglJ6r9TTc72Kr3fxQl7IsNYS+s=
=LpYg
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/5] lib: Add wait_for_bit
2015-12-16 23:32 ` Mateusz Kulikowski
@ 2015-12-16 23:52 ` Marek Vasut
2015-12-20 16:19 ` Mateusz Kulikowski
0 siblings, 1 reply; 11+ messages in thread
From: Marek Vasut @ 2015-12-16 23:52 UTC (permalink / raw)
To: u-boot
On Thursday, December 17, 2015 at 12:32:26 AM, Mateusz Kulikowski wrote:
> Hi Marek,
Hi,
> On 16.12.2015 23:11, Marek Vasut wrote:
> > On Wednesday, December 16, 2015 at 10:58:38 PM, Mateusz Kulikowski wrote:
> [...]
>
> >> +#include <console.h>
> >> +#include <asm/io.h>
> >> +#include <asm/errno.h>
> >> +
> >> +int wait_for_bit(const char *prefix, const u32 *reg, const u32 mask,
> >> + const bool set, const unsigned int timeout,
> >> + const bool breakable)
> >> +{
> >
> > I wonder, what would happen if you stuffed this function into the header
> > file altogether ? I think this would allow the compiler to do
> > interprocedure optimalization on whichever file this would be included
> > into. I wonder if that would have any impact on the resulting code size.
>
> Of course I can make it static inline.
>
> I was suggested not to care about possible leftovers that are
> not garbage-collected by linker so didn't changed that on V2.
>
> It's (max) few bytes that may be consumed by section alignment.
I was just curious about how much difference this would make.
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 0/5] Add wait_for_bit()
2015-12-16 21:58 [U-Boot] [PATCH v2 0/5] Add wait_for_bit() Mateusz Kulikowski
` (4 preceding siblings ...)
2015-12-16 21:58 ` [U-Boot] [PATCH v2 5/5] net: zynq_gem: " Mateusz Kulikowski
@ 2015-12-17 13:57 ` LEMIEUX, SYLVAIN
5 siblings, 0 replies; 11+ messages in thread
From: LEMIEUX, SYLVAIN @ 2015-12-17 13:57 UTC (permalink / raw)
To: u-boot
> From: Mateusz Kulikowski [mailto:mateusz.kulikowski at gmail.com]
> Sent: 16-Dec-15 4:59 PM
> To: u-boot at lists.denx.de; Marek Vasut; LEMIEUX, SYLVAIN; Joe Hershberger
> Cc: Mateusz Kulikowski
> Subject: [PATCH v2 0/5] Add 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.
No problem, thanks
>
> 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
>
...
>
> --
> 2.5.0
________________________________
This e-mail contains privileged and confidential information intended for the use of the addressees named above. If you are not the intended recipient of this e-mail, you are hereby notified that you must not disseminate, copy or take any action in respect of any information contained in it. If you have received this e-mail in error, please notify the sender immediately by e-mail and immediately destroy this e-mail and its attachments.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2 1/5] lib: Add wait_for_bit
2015-12-16 23:52 ` Marek Vasut
@ 2015-12-20 16:19 ` Mateusz Kulikowski
0 siblings, 0 replies; 11+ messages in thread
From: Mateusz Kulikowski @ 2015-12-20 16:19 UTC (permalink / raw)
To: u-boot
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Hi Marek,
On 17.12.2015 00:52, Marek Vasut wrote:
[...]
>> Of course I can make it static inline.
>>
>> I was suggested not to care about possible leftovers that are
>> not garbage-collected by linker so didn't changed that on V2.
>>
>> It's (max) few bytes that may be consumed by section alignment.
>
> I was just curious about how much difference this would make.
I added one more commit (to do wait_for_bit static inline), and ran
buildman on 4 boards (2x using and 2x not using wait_bit).
tl;dr:
For (at least) some boards it increases .rodata by 7 bytes.
Even if they don't use wait_for_bit.
Inline version works better, as it doesn't touch them
My proposal - I'll send v3 with wait_for_bit as static inline...
Buildman output:
$ tools/buildman/buildman -b wait_for_bit_v2 -o buildman hikey sandbox usb_a9263 zynq_picozed -sdSB
boards.cfg is up to date. Nothing to do.
Summary of 7 commits for 4 boards (4 threads, 1 job per thread)
01: eeprom: fix eeprom write procedure
02: lib: Add wait_for_bit
sandbox: (for 1/1 boards) all +224.0 text +224.0
sandbox : all +224 text +224
u-boot: add: 1/0, grow: 0/0 bytes: 137/0 (137)
function old new delta
wait_for_bit - 137 +137
arm: (for 2/2 boards) spl/u-boot-spl:all +3.5 spl/u-boot-spl:rodata +3.5
zynq_picozed : spl/u-boot-spl:all +7 spl/u-boot-spl:rodata +7
03: usb: dwc2: Use shared wait_for_bit
aarch64: (for 1/1 boards) all +168.0 rodata +16.0 text +152.0
hikey : all +168 rodata +16 text +152
u-boot: add: 0/0, grow: 4/0 bytes: 156/0 (156)
function old new delta
wait_for_bit 104 164 +60
usb_lowlevel_init 820 864 +44
dwc_otg_core_reset 120 156 +36
wait_for_chhltd 140 156 +16
04: usb: ohci-lpc32xx: Use shared wait_for_bit
05: usb: ehci-mx6: Use shared wait_for_bit
06: net: zynq_gem: Use shared wait_for_bit
arm: (for 2/2 boards) all +53.0 bss +6.0 rodata +7.0 text +40.0
zynq_picozed : all +106 bss +12 rodata +14 text +80
u-boot: add: 1/0, grow: 0/-1 bytes: 140/-68 (72)
function old new delta
wait_for_bit - 140 +140
zynq_gem_send 276 208 -68
07: make wait_bit static inline
sandbox: (for 1/1 boards) all -224.0 text -224.0
sandbox : all -224 text -224
arm: (for 2/2 boards) spl/u-boot-spl:all -3.5 spl/u-boot-spl:rodata -3.5
zynq_picozed : spl/u-boot-spl:all -7 spl/u-boot-spl:rodata -7
Regards,
Mateusz
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQEcBAEBCAAGBQJWdtT1AAoJELvtohmVtQzBtPwH/2aY2GbCIevGGdN5x4109xDm
zrrkmu6IQORQeOlTckeP2x83plY2KLGCoMtrA8fN3GbhqAJ9gM9EjtOzo7HYcYyD
fENKksvqSWTGKPI1nAU3liRri6lhCyO0eq+ZwPNvQF8m5GT88r12QL599P1LXgOI
tM93wzTaDWxDVUdFsUgBgzS0arcjG6eE2SWTE9SQY1OIKK/6CY79Y0q4h6UFSXwj
DflZ4rrYTQSMMHs2tKvas/MrZozKHsim2g8glZj87dkVcyeTHPcEEvUa6UPbr4RJ
NTQlarzFR3zsBa4Xl9wbROlL10drb+zgo6eNt8CyZ6z7II1r+oPI2m/OcyHoyDU=
=Lfit
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-12-20 16:19 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-16 21:58 [U-Boot] [PATCH v2 0/5] Add wait_for_bit() Mateusz Kulikowski
2015-12-16 21:58 ` [U-Boot] [PATCH v2 1/5] lib: Add wait_for_bit Mateusz Kulikowski
2015-12-16 22:11 ` Marek Vasut
2015-12-16 23:32 ` Mateusz Kulikowski
2015-12-16 23:52 ` Marek Vasut
2015-12-20 16:19 ` Mateusz Kulikowski
2015-12-16 21:58 ` [U-Boot] [PATCH v2 2/5] usb: dwc2: Use shared wait_for_bit Mateusz Kulikowski
2015-12-16 21:58 ` [U-Boot] [PATCH v2 3/5] usb: ohci-lpc32xx: " Mateusz Kulikowski
2015-12-16 21:58 ` [U-Boot] [PATCH v2 4/5] usb: ehci-mx6: " Mateusz Kulikowski
2015-12-16 21:58 ` [U-Boot] [PATCH v2 5/5] net: zynq_gem: " Mateusz Kulikowski
2015-12-17 13:57 ` [U-Boot] [PATCH v2 0/5] Add wait_for_bit() LEMIEUX, SYLVAIN
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.