[U-Boot] [PATCH v3 0/5] Add wait_for_bit()
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..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, + 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; +} + + +#endif
@Tom: I forgot to add you on CC :(
I think this patch goes to you.
On Sun, Dec 27, 2015 at 06:28:08PM +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
Regards, Mateusz
On Sun, Dec 27, 2015 at 06:28:08PM +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
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
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
Hi,
On 20.01.2016 05:34, Simon Glass wrote: [...]
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.
[...]
Sorry I only just saw this, but thought I'd make a few comments.
Nooo, I was expecting at least this to be merged during this merge window :)
[...]
- @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
This may be a good idea to make it more foolproof -
@trini: Will v4 with small change like that delay merging this series into mainline?
const bool breakable)Wow this is a pretty big inline function.
I personally probably could just drop inline and leave "static" but still keep it in header (so it may not be inlined), but it would probably violate some unwritten holy rules :)
First version was compiled into object file, but then either it would require extra config option, or would pollute rodata of all boards (which is bad).
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.
I prefer to keep it as is (for now).
This function is supposed to be drop-in replacement for four almost the same functions in drivers (dwc2, ohci-lpc..., ehci-mx6 and zynq_gem).
My intent was to keep all changes as small as possible so I would not cause regressions, but will make some people happy.
As for argument count - there was already request to add new feature [1], which is nice (I appended it to my task queue), so I can rework it a bit later (and perhaps use it in even more places where it would be useful).
As long as this function is inlined - argument count doesn't matter that much imo - as long as one remembers argument order or has smart IDE that does it for him.
+{
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?
Same arguments as above apply.
Although I agree that in future it may be useful not to have puts here.
Is it ok with you (timeout -> timeout_ms if possible I'll do now, rest + [1] in future)?
[1] http://lists.denx.de/pipermail/u-boot/2015-December/239468.html
Regards, Mateusz
Hi Matueuz,
On 20 January 2016 at 14:03, Mateusz Kulikowski mateusz.kulikowski@gmail.com wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
Hi,
On 20.01.2016 05:34, Simon Glass wrote: [...]
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.
[...]
Sorry I only just saw this, but thought I'd make a few comments.
Nooo, I was expecting at least this to be merged during this merge window :)
[...]
- @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
This may be a good idea to make it more foolproof -
@trini: Will v4 with small change like that delay merging this series into mainline?
const bool breakable)Wow this is a pretty big inline function.
I personally probably could just drop inline and leave "static" but still keep it in header (so it may not be inlined), but it would probably violate some unwritten holy rules :)
First version was compiled into object file, but then either it would require extra config option, or would pollute rodata of all boards (which is bad).
If you drop the string the rodata add-on (presumably due to the gcc bug) would be tiny, so I don't think it would need a Kconfig.
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.
I prefer to keep it as is (for now).
This function is supposed to be drop-in replacement for four almost the same functions in drivers (dwc2, ohci-lpc..., ehci-mx6 and zynq_gem).
My intent was to keep all changes as small as possible so I would not cause regressions, but will make some people happy.
As for argument count - there was already request to add new feature [1], which is nice (I appended it to my task queue), so I can rework it a bit later (and perhaps use it in even more places where it would be useful).
As long as this function is inlined - argument count doesn't matter that much imo - as long as one remembers argument order or has smart IDE that does it for him.
+{
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?
Same arguments as above apply.
Although I agree that in future it may be useful not to have puts here.
Is it ok with you (timeout -> timeout_ms if possible I'll do now, rest + [1] in future)?
Please go ahead, you already have a review by Tom. My comment are just ideas.
[1] http://lists.denx.de/pipermail/u-boot/2015-December/239468.html
Regards, Mateusz
Regards, Simon
On Wed, Jan 20, 2016 at 10:03:40PM +0100, Mateusz Kulikowski wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
Hi,
On 20.01.2016 05:34, Simon Glass wrote: [...]
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.
[...]
Sorry I only just saw this, but thought I'd make a few comments.
Nooo, I was expecting at least this to be merged during this merge window :)
[...]
- @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
This may be a good idea to make it more foolproof -
@trini: Will v4 with small change like that delay merging this series into mainline?
Nope, just get it posted soon please :)
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;
On Sonntag, 27. Dezember 2015 18:28:09 CET Mateusz Kulikowski wrote:
Use existing library function to poll bit(s).
Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com
It might be useful to have not only a relative timeout, but also an absolute timeout. For dwc2, the timeout handling could be moved from the transactions wrappers to the chunk_msg function. As the USB timeouts are specified for the whole transaction, it would be very neat to calculate one deadline for the whole transaction and hand this absolute timeout to the wait_for_bit(..) function. At the end, this would also result in less code.
Kind regards,
Stefan
Hi Stefan,
On Sun, Dec 27, 2015 at 11:17:55PM +0100, Stefan Bruens wrote:
On Sonntag, 27. Dezember 2015 18:28:09 CET Mateusz Kulikowski wrote:
Use existing library function to poll bit(s).
Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com
It might be useful to have not only a relative timeout, but also an absolute timeout. For dwc2, the timeout handling could be moved from the transactions wrappers to the chunk_msg function. As the USB timeouts are specified for the whole transaction, it would be very neat to calculate one deadline for the whole transaction and hand this absolute timeout to the wait_for_bit(..) function. At the end, this would also result in less code.
This may be a good idea, but I prefer to add it to my todo list (or you can do it once this series gets submitted or as dependant patch).
I need wait_for_bit for my board/driver. Doing another set of upgrades will delay it. I will also have access to dwc2 device around mid. Jan so will be able to test "bigger" refactors.
Is it ok with you?
Regards, Mateusz
On Sun, Dec 27, 2015 at 06:28:09PM +0100, Mateusz Kulikowski wrote:
Use existing library function to poll bit(s).
Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com
Reviewed-by: Tom Rini trini@konsulko.com
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 Sun, Dec 27, 2015 at 06:28:10PM +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
Reviewed-by: Tom Rini trini@konsulko.com
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 Sun, Dec 27, 2015 at 06:28:11PM +0100, Mateusz Kulikowski wrote:
Use existing library function to poll bit(s).
Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com
Reviewed-by: Tom Rini trini@konsulko.com
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 Sun, Dec 27, 2015 at 06:28:12PM +0100, Mateusz Kulikowski wrote:
Use existing library function to poll bit(s). Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com
Reviewed-by: Tom Rini trini@konsulko.com
On Thu, Jan 14, 2016 at 12:08 PM, Tom Rini trini@konsulko.com wrote:
On Sun, Dec 27, 2015 at 06:28:12PM +0100, Mateusz Kulikowski wrote:
Use existing library function to poll bit(s). Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com
Reviewed-by: Tom Rini trini@konsulko.com
Reviewed-by: Moritz Fischer moritz.fischer@ettus.com
Cheers,
Moritz
participants (5)
-
Mateusz Kulikowski -
Moritz Fischer -
Simon Glass -
Stefan Bruens -
Tom Rini