Sascha,
-----Original Message----- From: Sascha Hauer [mailto:s.hauer@pengutronix.de] Sent: Tuesday, June 03, 2008 3:08 AM To: Menon, Nishanth Cc: u-boot-users@lists.sourceforge.net; Laurent Desnogues; dirk.behme@googlemail.com; philip.balister@gmail.com; Gopinath, Thara; Kamat, Nishant; Syed Mohammed, Khasim Subject: Re: [Patch 02/17] U-Boot-V2:Common:Clock Handle case of clockrollover for get_time_ns
cycle_delta = (cycle_now - cs->cycle_last) & cs->mask;Look closer, the rollover case is handled implicitly by the unsigned arithmetics.
Agreed for logic when mask is the max value attainable. IMHO, The concept of cs->mask is messy. We do need a min, max and a mask. When: cycle_now = cs->read(); cs->cycle_last = cycle_now; (cycle_now - cs->cycle_last) & cs->mask is wrong. Assumptions made: A) The bits masked out by cs->mask will remain constant. This may not be true. B) Roll over assume the min is 0 and max is cs->mask. This need not be the case. It would be good to be explicit.
You are right, we do not have a possibility to detect a double rollover without interrupts. Normally this is not an issue as this code is used in timeout polling loops where the current time is polled often enough. Anyway, maybe some comment should mention that this function is not useful to measure long periods of time where 'long' is highly architecture specific.
Yes, there are indenting issue + no doxygen documentation.. It helps as clock_source is critical implementation required in the system.
Regards, Nishanth Menon