Re: [PATCH rc2 12/25] hw/timer: Add limited support for Atmel 16 bit timer peripheral

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 1/24/20 9:16 AM, Thomas Huth wrote:
> On 24/01/2020 01.51, Philippe Mathieu-Daudé wrote:
>> From: Michael Rolnik <mrolnik@gmail.com>
>>
>> These were designed to facilitate testing but should provide enough
>> function to be useful in other contexts.  Only a subset of the functions
>> of each peripheral is implemented, mainly due to the lack of a standard
>> way to handle electrical connections (like GPIO pins).
>>
>> Signed-off-by: Sarah Harris <S.E.Harris@kent.ac.uk>
>> Message-Id: <20200118191416.19934-13-mrolnik@gmail.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> [rth: Squash info mtree fixes and a file rename from f4bug, which was:]
>> Suggested-by: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> [PMD: Use qemu_log_mask(LOG_UNIMP), replace goto by return]
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> rc2: Use qemu_log_mask(LOG_UNIMP), replace goto by return (thuth)
>> ---
>>   include/hw/timer/atmel_timer16.h |  94 +++++
>>   hw/timer/atmel_timer16.c         | 605 +++++++++++++++++++++++++++++++
>>   hw/timer/Kconfig                 |   3 +
>>   hw/timer/Makefile.objs           |   2 +
>>   4 files changed, 704 insertions(+)
>>   create mode 100644 include/hw/timer/atmel_timer16.h
>>   create mode 100644 hw/timer/atmel_timer16.c
> [...]
>> +static void avr_timer16_clksrc_update(AVRTimer16State *t16)
>> +{
>> +    uint16_t divider = 0;
>> +    switch (CLKSRC(t16)) {
>> +    case T16_CLKSRC_EXT_FALLING:
>> +    case T16_CLKSRC_EXT_RISING:
>> +        qemu_log_mask(LOG_UNIMP, "%s: external clock source unsupported\n",
>> +                      __func__);
>> +        break;
>> +    case T16_CLKSRC_STOPPED:
>> +        break;
>> +    case T16_CLKSRC_DIV1:
>> +        divider = 1;
>> +        break;
>> +    case T16_CLKSRC_DIV8:
>> +        divider = 8;
>> +        break;
>> +    case T16_CLKSRC_DIV64:
>> +        divider = 64;
>> +        break;
>> +    case T16_CLKSRC_DIV256:
>> +        divider = 256;
>> +        break;
>> +    case T16_CLKSRC_DIV1024:
>> +        divider = 1024;
>> +        break;
>> +    default:
>> +        break;
>> +    }
>> +    if (divider) {
>> +        t16->freq_hz = t16->cpu_freq_hz / divider;
>> +        t16->period_ns = NANOSECONDS_PER_SECOND / t16->freq_hz;
>> +        DB_PRINT("Timer frequency %" PRIu64 " hz, period %" PRIu64 " ns (%f s)",
>> +                 t16->freq_hz, t16->period_ns, 1 / (double)t16->freq_hz);
>> +    }
>> +    return;
>> +}
> 
> You can remove the "return;" at the end of the function now, too.

Oops I didn't notice.

Richard, do you mind to remove this single line?

Thanks both :)