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

[Serge Vakulenko]

On Fri, Jul 3, 2015 at 3:04 PM, Maciej W. Rozycki <address@hidden> wrote:
> 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?

OK, I will zero out the Revision field, to align with the rest of
configurations.

>  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).

Not a problem, I'll reorder it.

>> > +    {
>> > +        /* 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")?

Well... I don't see any reason not to change it to "microAptivUP", for
consistency with MIPS documentation.

>  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