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
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 wrote:
> Hi Reinhard,
>
> On Wed, Jun 29, 2011 at 3:06 PM, Reinhard Meyer
> 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 Reinhard,
On Wed, Jun 29, 2011 at 3:06 PM, Reinhard Meyer
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/
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 Graeme,
On Tue, Jun 28, 2011 at 4:41 AM, Graeme Russ wrote:
>
> Signed-off-by: Graeme Russ
> ---
> 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
