Re: [Qemu-devel] [PATCH pic32 1/7] Speed of MIPS CPU timer made configurable per platform.

[Frederic Konrad <fred.konrad@greensocs.com>]

Hi,

This is nice.

You should use the checkpatch (script/checkpatch) to check the coding 
style of
your patch.
The first is corrupted and there are some coding style issues.
And as Peter suggest use --cover-letter and git send email to send the 
emails.

Fred

On 30/06/2015 07:00, Serge Vakulenko wrote:
> Please find below a set of patches, which allow to simulate Microchip
> PIC32 microcontrollers on QEMU. For examples of real PIC32
> applications running on QEMU, see page:
> https://github.com/sergev/qemu/wiki
>
> (1) Make the CPU clock frequency configurable per platform. Currently
> the clock rate for all MIPS platforms is fixed at 100MHz. Need to make
> it 40MHz for pic32mx7.
>
> (2) When WAIT instruction is executed with interrupts disabled, the
> processor is essentially suspended forever. Terminate the simulation
> in this case.
>
> (3) For TLBWR instruction, the generated random index value has been
> not quite random and did not take into account the Wired register
> value. Fixed.
>
> (4) Added support for external interrupt controller mode (EIC).
> Required for pic32.
>
> (5) Added two processor variants: M4K and microAptivUP. Needed for
> pic32mx and pic32mz simulation.
>
> (6) Added two machine platforms: Microchip pic32mx7 and pic32mz
> microcontrollers. Several board types supported for each platform:
>
>      pic32mx7-explorer16  PIC32MX7 microcontroller on Microchip Explorer-16 board
>      pic32mx7-max32       PIC32MX7 microcontroller on chipKIT Max32 board
>      pic32mx7-maximite    PIC32MX7 microcontroller on Geoff's Maximite board
>      pic32mz-explorer16   PIC32MZ microcontroller on Microchip Explorer-16 board
>      pic32mz-meb2         PIC32MZ microcontroller on Microchip MEB-II board
>      pic32mz-wifire       PIC32MZ microcontroller on chipKIT WiFire board
>
> (7) When "-sd" option is used to set a name of file with SD card
> image, a warning is printed: "WARNING: Image format was not
> specified". Fixed.
>
> Signed-off-by: Serge Vakulenko <serge@vak.ru>
> ---
>   hw/mips/cputimer.c        | 13 +++++++------
>   hw/mips/mips_fulong2e.c   |  2 +-
>   hw/mips/mips_jazz.c       |  2 +-
>   hw/mips/mips_malta.c      |  4 ++--
>   hw/mips/mips_mipssim.c    |  2 +-
>   hw/mips/mips_r4k.c        |  2 +-
>   include/hw/mips/cpudevs.h |  2 +-
>   target-mips/cpu.h         |  1 +
>   8 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/hw/mips/cputimer.c b/hw/mips/cputimer.c
> index 577c9ae..3d23c1b 100644
> --- a/hw/mips/cputimer.c
> +++ b/hw/mips/cputimer.c
> @@ -50,8 +50,8 @@ static void cpu_mips_timer_update(CPUMIPSState *env)
>
>       now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>       wait = env->CP0_Compare - env->CP0_Count -
> -    (uint32_t)muldiv64(now, TIMER_FREQ, get_ticks_per_sec());
> -    next = now + muldiv64(wait, get_ticks_per_sec(), TIMER_FREQ);
> +        (uint32_t)muldiv64(now, env->count_freq, get_ticks_per_sec());
> +    next = now + muldiv64(wait, get_ticks_per_sec(), env->count_freq);
>       timer_mod(env->timer, next);
>   }
>
> @@ -80,7 +80,7 @@ uint32_t cpu_mips_get_count (CPUMIPSState *env)
>           }
>
>           return env->CP0_Count +
> -            (uint32_t)muldiv64(now, TIMER_FREQ, get_ticks_per_sec());
> +            (uint32_t)muldiv64(now, env->count_freq, get_ticks_per_sec());
>       }
>   }
>
> @@ -97,7 +97,7 @@ void cpu_mips_store_count (CPUMIPSState *env, uint32_t count)
>           /* Store new count register */
>           env->CP0_Count =
>               count - (uint32_t)muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> -                                       TIMER_FREQ, get_ticks_per_sec());
> +                                       env->count_freq, get_ticks_per_sec());
>           /* Update timer timer */
>           cpu_mips_timer_update(env);
>       }
> @@ -122,7 +122,7 @@ void cpu_mips_stop_count(CPUMIPSState *env)
>   {
>       /* Store the current value */
>       env->CP0_Count += (uint32_t)muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> -                                         TIMER_FREQ, get_ticks_per_sec());
> +                                         env->count_freq, get_ticks_per_sec());
>   }
>
>   static void mips_timer_cb (void *opaque)
> @@ -145,7 +145,7 @@ static void mips_timer_cb (void *opaque)
>       env->CP0_Count--;
>   }
>
> -void cpu_mips_clock_init (CPUMIPSState *env)
> +void cpu_mips_clock_init(CPUMIPSState *env, unsigned count_freq)
>   {
>       /*
>        * If we're in KVM mode, don't create the periodic timer, that is
> handled in
> @@ -153,5 +153,6 @@ void cpu_mips_clock_init (CPUMIPSState *env)
>        */
>       if (!kvm_enabled()) {
>           env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &mips_timer_cb, env);
> +        env->count_freq = count_freq;
>       }
>   }
> diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
> index dea941a..5decc30 100644
> --- a/hw/mips/mips_fulong2e.c
> +++ b/hw/mips/mips_fulong2e.c
> @@ -343,7 +343,7 @@ static void mips_fulong2e_init(MachineState *machine)
>
>       /* Init internal devices */
>       cpu_mips_irq_init_cpu(env);
> -    cpu_mips_clock_init(env);
> +    cpu_mips_clock_init(env, 100*1000*1000);
>
>       /* North bridge, Bonito --> IP2 */
>       pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));
> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> index 9d60633..02629fa 100644
> --- a/hw/mips/mips_jazz.c
> +++ b/hw/mips/mips_jazz.c
> @@ -210,7 +210,7 @@ static void mips_jazz_init(MachineState *machine,
>
>       /* Init CPU internal devices */
>       cpu_mips_irq_init_cpu(env);
> -    cpu_mips_clock_init(env);
> +    cpu_mips_clock_init(env, 100*1000*1000);
>
>       /* Chipset */
>       rc4030 = rc4030_init(&dmas, &rc4030_dma_mr);
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 3082e75..0393017 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -988,7 +988,7 @@ void mips_malta_init(MachineState *machine)
>
>           /* Init internal devices */
>           cpu_mips_irq_init_cpu(env);
> -        cpu_mips_clock_init(env);
> +        cpu_mips_clock_init(env, 100*1000*1000);
>           qemu_register_reset(main_cpu_reset, cpu);
>       }
>       cpu = MIPS_CPU(first_cpu);
> @@ -1144,7 +1144,7 @@ void mips_malta_init(MachineState *machine)
>
>       /* Init internal devices */
>       cpu_mips_irq_init_cpu(env);
> -    cpu_mips_clock_init(env);
> +    cpu_mips_clock_init(env, 100*1000*1000);
>
>       /*
>        * We have a circular dependency problem: pci_bus depends on isa_irq,
> diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
> index 61f74a6..940ca58 100644
> --- a/hw/mips/mips_mipssim.c
> +++ b/hw/mips/mips_mipssim.c
> @@ -213,7 +213,7 @@ mips_mipssim_init(MachineState *machine)
>
>       /* Init CPU internal devices. */
>       cpu_mips_irq_init_cpu(env);
> -    cpu_mips_clock_init(env);
> +    cpu_mips_clock_init(env, 100*1000*1000);
>
>       /* Register 64 KB of ISA IO space at 0x1fd00000. */
>       memory_region_init_alias(isa, NULL, "isa_mmio",
> diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
> index f4dcacd..5f0b766 100644
> --- a/hw/mips/mips_r4k.c
> +++ b/hw/mips/mips_r4k.c
> @@ -266,7 +266,7 @@ void mips_r4k_init(MachineState *machine)
>
>       /* Init CPU internal devices */
>       cpu_mips_irq_init_cpu(env);
> -    cpu_mips_clock_init(env);
> +    cpu_mips_clock_init(env, 100*1000*1000);
>
>       /* ISA bus: IO space at 0x14000000, mem space at 0x10000000 */
>       memory_region_init_alias(isa_io, NULL, "isa-io",
> diff --git a/include/hw/mips/cpudevs.h b/include/hw/mips/cpudevs.h
> index b2626f2..4d4e62a 100644
> --- a/include/hw/mips/cpudevs.h
> +++ b/include/hw/mips/cpudevs.h
> @@ -12,6 +12,6 @@ uint64_t cpu_mips_kvm_um_phys_to_kseg0(void *opaque,
> uint64_t addr);
>   void cpu_mips_irq_init_cpu(CPUMIPSState *env);
>
>   /* mips_timer.c */
> -void cpu_mips_clock_init(CPUMIPSState *);
> +void cpu_mips_clock_init(CPUMIPSState *, unsigned count_freq);
>
>   #endif
> diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> index 474a0e3..c476166 100644
> --- a/target-mips/cpu.h
> +++ b/target-mips/cpu.h
> @@ -595,6 +595,7 @@ struct CPUMIPSState {
>       const mips_def_t *cpu_model;
>       void *irq[8];
>       QEMUTimer *timer; /* Internal timer */
> +    unsigned count_freq; /* rate of Count register */
>   };
>
>   #include "cpu-qom.h"
> --
> 1.9.1
>