On 05.06.20 16:18, Rasmus Villemoes wrote:
On 05/06/2020 15.37, Stefan Roese wrote:
On 05.06.20 14:13, Stefan Roese wrote:
On 05.06.20 14:11, Rasmus Villemoes wrote:
On 05/06/2020 13.48, Stefan Roese wrote:
On 05.06.20 13:16, Rasmus Villemoes wrote:
This is what I had in mind. I also considered making it a config knob
(possibly just auto-selected based on $ARCH) whether to use
get_timer() or get_ticks(), but that becomes quite ugly.
I hesitate a bit, moving with this generic code from get_timer()
to get_ticks() for all boards. Did you test this patch on other
platforms, like some ARM boards?
Please note that I don't reject it - just asking.
Yeah, I'm not really too happy about it myself, exactly because it
affects all arches/platforms. And no, I don't have other hardware handy
unfortunately. So it's very much an RFC where I hope someone with
knowledge of the various arches can say whether one can expect
get_ticks() to be at least as widely available as get_timer().
I'll try to test it on a non-powerpc platform soon'ish.
I've tested it on an MIPS based platform (gardena-smart-gateway-mt7688)
and it works there without any issues AFAICT.
Thanks.
Two more remarks:
Could you please run a build test with this patch applied for all
boards (Travis, Azure...)?
I may need a few more pointers than that. What am I supposed to do exactly?
Not sure if there is some documentation on how to use the Travis,
Gitlab or Azure build. It mainly uses buildman [1] internally, so you
can use buildman locally if you don't want to push the build into
the cloud.
And do you see an issue, that the timer-wrap fixed by Chris with this
patch [1] will not re-occur with your "ticks" approach?
No, it will not re-occur, because this does the comparison the right
way, namely by subtracting the absolute times and comparing that delta
to the threshold (that's also what the time_after does under the hood).
The wrong way is saying now >= last_reset + threshold (or as I think the
code used to do, computing next_reset = now + threshold and then asking
now >= next_reset).
Ah, good to know. Thanks for confirming.
Thanks,
Stefan
[1] tools/buildman/README
