Re: [Qemu-devel] [PATCH v14 2/9] target-avr: adding AVR CPU features/flavors

[Michael Rolnik]

Richard. Please stop reviewing it. It seems to be a wrong review.

On Mon, Aug 15, 2016, 6:31 PM Peter Maydell <address@hidden>
wrote:

> On 29 July 2016 at 16:32, Michael Rolnik <address@hidden> wrote:
> > Signed-off-by: Michael Rolnik <address@hidden>
> > ---
> >  target-avr/cpu.c     | 319
> +++++++++++++++++++++++++++++++++++++++++++++++++--
> >  target-avr/cpu.h     |  52 ++++++++-
> >  target-avr/machine.c |  28 ++---
> >  3 files changed, 373 insertions(+), 26 deletions(-)
> >
> > diff --git a/target-avr/cpu.c b/target-avr/cpu.c
> > index c5ee2e9..fa15727 100644
> > --- a/target-avr/cpu.c
> > +++ b/target-avr/cpu.c
> > @@ -203,9 +203,300 @@ static void avr_cpu_class_init(ObjectClass *oc,
> void *data)
> >      dc->cannot_destroy_with_object_finalize_yet = true;
> >  }
> >
> > -static void avr_any_initfn(Object *obj)
>
> Please don't add code in one patch and then remove it in the next.
>
>
> > @@ -226,13 +531,7 @@ static gint avr_cpu_list_compare(gconstpointer a,
> gconstpointer b)
> >
> >      name_a = object_class_get_name(class_a);
> >      name_b = object_class_get_name(class_b);
> > -    if (strcmp(name_a, "any-" TYPE_AVR_CPU) == 0) {
> > -        return 1;
> > -    } else if (strcmp(name_b, "any-" TYPE_AVR_CPU) == 0) {
> > -        return -1;
> > -    } else {
> > -        return strcmp(name_a, name_b);
> > -    }
> > +    return strcmp(name_a, name_b);
> >  }
>
> Ditto.
>
> >
> >  static void avr_cpu_list_entry(gpointer data, gpointer user_data)
>
> > diff --git a/target-avr/machine.c b/target-avr/machine.c
> > index 9f20aed..2b005cc 100644
> > --- a/target-avr/machine.c
> > +++ b/target-avr/machine.c
> > @@ -44,8 +44,8 @@ static void put_sreg(QEMUFile *f, void *opaque, size_t
> size)
> >
> >  static const VMStateInfo vms_sreg = {
> >      .name = "sreg",
> > -    .get  = get_sreg,
> > -    .put  = put_sreg,
> > +    .get = get_sreg,
> > +    .put = put_sreg,
> >  };
>
> Please don't add code in one patch and then change its formatting
> in the next patch; just format it correctly to start with.
>
> Otherwise this patch looks OK.
>
> thanks
> -- PMM
>