Re: [Qemu-devel] [PATCH pic32 v2 4/5] Two new processor variants: M4K and microAptivP.

[Maciej W. Rozycki]

On Wed, 1 Jul 2015, Aurelien Jarno wrote:

> > diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
> > index ddfaff8..430a547 100644
> > --- a/target-mips/translate_init.c
> > +++ b/target-mips/translate_init.c
> > @@ -232,6 +232,52 @@ static const mips_def_t mips_defs[] =
> >          .mmu_type = MMU_TYPE_FMT,
> >      },
> >      {
> > +        /* Configuration for Microchip PIC32MX microcontroller. */
> > +        .name = "M4K",
> > +        .CP0_PRid = 0x00018765,

 Hmm, does it make sense to set the Revision field here?  We keep it at 0 
for other templates, so why not 0x00018700?

 Also I suggest to move the template earlier on so that entries remain 
sorted by PRId, at least within the same vendor.  So this would go between 
"4KEmR1" and "4KEc" (the M4K is an MTI RTL, quite an old one actually).

> > +    {
> > +        /* Configuration for Microchip PIC32MZ microcontroller. */
> > +        .name = "microAptivP",
> > +        .CP0_PRid = 0x00019e28,

 Same question here, why not 0x00019e00?  Also why not "microAptivUP" as 
documentation calls it (vs "microAptivUC")?

 And again, it looks to me like the entry better followed "M14Kc".

> Otherwise it looks ok, though I haven't look at the PIC32 manual to
> check the values.

 I haven't checked if the bit patterns for configuration registers are 
sane either.  These RTLs are configurable, so (within some limits) real 
hardware will have different values anyway.

  Maciej