[U-Boot] [PATCH v1 (WIP) 09/16] [Timer]Replace get_timer() usage in drivers/block/
Signed-off-by: Graeme Russ graeme.r...@gmail.com --- drivers/block/mg_disk.c |9 - 1 files changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/block/mg_disk.c b/drivers/block/mg_disk.c index 2198017..c8cc195 100644 --- a/drivers/block/mg_disk.c +++ b/drivers/block/mg_disk.c @@ -88,17 +88,16 @@ static void mg_dump_status (const char *msg, unsigned int stat, unsigned err) static unsigned int mg_wait (u32 expect, u32 msec) { u8 status; - u32 from, cur, err; + u32 ts, err; err = MG_ERR_NONE; #ifdef CONFIG_NIOS2 reset_timer(); #endif - from = get_timer(0); + ts = time_now_ms(); status = readb(mg_base() + MG_REG_STATUS); do { - cur = get_timer(from); if (status MG_REG_STATUS_BIT_BUSY) { if (expect == MG_REG_STATUS_BIT_BUSY) break; @@ -119,9 +118,9 @@ static unsigned int mg_wait (u32 expect, u32 msec) break; } status = readb(mg_base() + MG_REG_STATUS); - } while (cur msec); + } while (time_since_ms(ts) msec); - if (cur = msec) + if (time_since_ms(ts) = msec) err = MG_ERR_TIMEOUT; return err; -- 1.7.5.2.317.g391b14 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v1 (WIP) 09/16] [Timer]Replace get_timer() usage in drivers/block/
Hi Graeme, On Tue, Jun 28, 2011 at 4:41 AM, Graeme Russ graeme.r...@gmail.com wrote: Signed-off-by: Graeme Russ graeme.r...@gmail.com --- drivers/block/mg_disk.c | 9 - 1 files changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/block/mg_disk.c b/drivers/block/mg_disk.c index 2198017..c8cc195 100644 --- a/drivers/block/mg_disk.c +++ b/drivers/block/mg_disk.c @@ -88,17 +88,16 @@ static void mg_dump_status (const char *msg, unsigned int stat, unsigned err) static unsigned int mg_wait (u32 expect, u32 msec) { u8 status; - u32 from, cur, err; + u32 ts, err; err = MG_ERR_NONE; #ifdef CONFIG_NIOS2 reset_timer(); #endif - from = get_timer(0); + ts = time_now_ms(); status = readb(mg_base() + MG_REG_STATUS); do { - cur = get_timer(from); ... - } while (cur msec); + } while (time_since_ms(ts) msec); Well I know i have asked this before, but I feel I should ask again because I didn't like the answer much. Imagine we change this code to: ts = time_now_ms() + msec do { ... } while (time_since_ms(ts) 0); That should be legal, right? But I don't think this can work since the 'since' functions return an unsigned. [aside: this provides for another idiom that I think we talked about: ts = time_future_ms(msec) do { ... } while (!time_passed(ts)) which I am not at all suggesting should be in the API :-) end aside] Regards. Simon - if (cur = msec) + if (time_since_ms(ts) = msec) err = MG_ERR_TIMEOUT; return err; -- 1.7.5.2.317.g391b14 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v1 (WIP) 09/16] [Timer]Replace get_timer() usage in drivers/block/
Dear All, Well I know i have asked this before, but I feel I should ask again because I didn't like the answer much. Imagine we change this code to: ts = time_now_ms() + msec do { ... } while (time_since_ms(ts) 0); That should be legal, right? But I don't think this can work since the 'since' functions return an unsigned. [aside: this provides for another idiom that I think we talked about: ts = time_future_ms(msec) do { ... } while (!time_passed(ts)) which I am not at all suggesting should be in the API :-) end aside] I still vouch for this concept, which is simple, clean, and easy to understand. Reinhard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v1 (WIP) 09/16] [Timer]Replace get_timer() usage in drivers/block/
Hi Reinhard, On Wed, Jun 29, 2011 at 3:06 PM, Reinhard Meyer u-b...@emk-elektronik.de wrote: Dear All, Well I know i have asked this before, but I feel I should ask again because I didn't like the answer much. Imagine we change this code to: ts = time_now_ms() + msec do { ... } while (time_since_ms(ts) 0); That should be legal, right? But I don't think this can work since the 'since' functions return an unsigned. [aside: this provides for another idiom that I think we talked about: ts = time_future_ms(msec) do { ... } while (!time_passed(ts)) which I am not at all suggesting should be in the API :-) end aside] I still vouch for this concept, which is simple, clean, and easy to understand. It really is a matter of personal taste ;) I find u32 start = time_now_ms(); do { ...blah... } while(time_since_ms(start) timeout); much easier to understand (Do whatever while time elapsed since I started is less than the timeout) u32 end = time_future_ms(timeout); do { ...blah... } while(time_now_ms() end); to me is a bit more clunky. Yes, it is probably computationally more efficient, but it does not naturally support: u32 start = time_now_ms(); u32 duration; ...blah... duration = time_since_ms(start); /* or duration = time_max_since_ms(start); */ Which we want for profiling. Also there are a few instances where there are multiple cascaded timeouts u32 start = time_now_ms(); do { ...blah... } while(time_since_ms(start) timeout_1); do { ...blah... } while(time_since_ms(start) timeout_2); Which means setting up all your timeouts in advance Regards, Graeme ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v1 (WIP) 09/16] [Timer]Replace get_timer() usage in drivers/block/
Hi Graeme, On Tue, Jun 28, 2011 at 10:19 PM, Graeme Russ graeme.r...@gmail.com wrote: Hi Reinhard, On Wed, Jun 29, 2011 at 3:06 PM, Reinhard Meyer u-b...@emk-elektronik.de wrote: Dear All, Well I know i have asked this before, but I feel I should ask again because I didn't like the answer much. Imagine we change this code to: ts = time_now_ms() + msec do { ... } while (time_since_ms(ts) 0); That should be legal, right? But I don't think this can work since the 'since' functions return an unsigned. [aside: this provides for another idiom that I think we talked about: ts = time_future_ms(msec) do { ... } while (!time_passed(ts)) which I am not at all suggesting should be in the API :-) end aside] I still vouch for this concept, which is simple, clean, and easy to understand. It really is a matter of personal taste ;) I find u32 start = time_now_ms(); do { ...blah... } while(time_since_ms(start) timeout); much easier to understand (Do whatever while time elapsed since I started is less than the timeout) u32 end = time_future_ms(timeout); do { ...blah... } while(time_now_ms() end); ... Actually: } while (time_passed_ms(end)) but anyway I agree it is a matter of taste and I'm quite happy with the approach here at the moment. But what about my question about signed ints for deltas? Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v1 (WIP) 09/16] [Timer]Replace get_timer() usage in drivers/block/
Hi Simon, u32 end = time_future_ms(timeout); do { ...blah... } while(time_now_ms() end); ... Actually: } while (time_passed_ms(end)) Sorry, but I think you've lost me here... but anyway I agree it is a matter of taste and I'm quite happy with the approach here at the moment. But what about my question about signed ints for deltas? We use signed int's to allow seamless roll-overs of the timer counter. One thing the API does not require is that a given low-level timer counts from zero - It can start with any value and therefore may roll-over at any time. By using unsigned provided there is at most one rollover between timing events (which for a 32-bit millisecond counter is a very long time) the logic remain trivial (time = end - start) - We don't have to try and detect the rollover. Also, we assume the u-Boot will never be installed in a time machine, and will therefore never need to calculate negative time. Please let us know if you plan to boot a TARDIS using U-Boot ;) Regards, Graeme ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot