On 12/02/2020 10:55, Lukasz Luba wrote: >> Because, as the commit message outlines it, such a frequency is terribly >> out of spec? > > I don't see in the RM that < 1MHz is terribly out of spec. > 'Frequency > Increments at a fixed frequency, typically in the range 1-50MHz. > Can support one or more alternative operating modes in which it increments by larger amounts at a > lower frequency, typically for power-saving.' > > There is even an example how to operate at 20kHz and increment by 500. > > I don't know the code if it's supported, thought. > For that one case the value reported by CNTFRQ shouldn't change - it's still a timer that looks like is operating at 10MHz, but under the hood is doing bigger increments at lower freq. As I was trying to get to, this patch isn't validating the actual frequency the timer operates on, rather that whatever is reported by CNTFRQ is somewhat sane (which here means [1, 50]MHz, although we just check the lower bound). [...] >> And? It seems to address a potential issue where the time frequency >> is out of spec, and makes sure we don't end up with additional problems >> in the AMU code. > > This patch just prints warning, does not change anything in booting or > in any code related to AMU. > Right, but it should still be worth having - at least it shows up in dmesg, and when someone reports something fishy we get a hint that we can blame the hardware. >> >> On its own, it is perfectly sensible and could be merged as part of this >> series with my >> >> Acked-by: Marc Zyngier <maz@xxxxxxxxxx> >> >> M. > > Regards, > Lukasz
