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

[Philippe Mathieu-Daudé]

Hello Sarah,

On 1/24/20 11:42 AM, Alex Bennée wrote:
> Philippe Mathieu-Daudé <address@hidden> writes:
>
> > From: Michael Rolnik <address@hidden>
> >
> > 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 <address@hidden>
> > Message-Id: <address@hidden>
> > Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> > [rth: Squash info mtree fixes and a file rename from f4bug, which was:]
> > Suggested-by: Aleksandar Markovic <address@hidden>
> > Signed-off-by: Richard Henderson <address@hidden>
> > [PMD: Use qemu_log_mask(LOG_UNIMP), replace goto by return]
> > Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> > ---
> <snip>
> > --- /dev/null
> > +++ b/include/hw/timer/atmel_timer16.h
> > @@ -0,0 +1,94 @@
> > +/*
> > + * Atmel AVR 16 bit timer
> > + *
> > + * Copyright (c) 2018 University of Kent
> > + * Author: Ed Robbins
>
> No sign off from the author here?

Hmmm I Sarah Harris's one, who is from the University of Kent, isn't it 
enough? (I remember patched from Xilinx with Edgar S-o-b but from other 
authors, Edgar vouched for Xilinx).

Sarah, can you get Ed Signed-off-by?

> > --- /dev/null
> > +++ b/hw/timer/atmel_timer16.c
> > @@ -0,0 +1,605 @@
> <snip>
> > +
> > +/* Helper macros */
> > +#define VAL16(l, h) ((h << 8) | l)
> > +#define DB_PRINT(fmt, args...) /* Nothing */
> > +/*#define DB_PRINT(fmt, args...) printf("%s: " fmt "\n", __func__, ##
> > args)*/
>
> Format strings are likely to bitrot. Either use a if (GATE) or
> tracepoints.

Yes...

> <snip>
>
> Otherwise:
>
> Reviewed-by: Alex Bennée <address@hidden>

Thanks!