Sascha, > -----Original Message----- > From: Sascha Hauer [mailto:[EMAIL PROTECTED] > Sent: Tuesday, June 03, 2008 3:08 AM > To: Menon, Nishanth > Cc: u-boot-users@lists.sourceforge.net; Laurent Desnogues; [EMAIL PROTECTED]; > [EMAIL PROTECTED]; 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 ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ U-Boot-Users mailing list U-Boot-Users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/u-boot-users
