Re: [PATCH 05/21] hw/arm: Add i.MX 8M Plus EVK board

[Bernhard Beschow]

Am 28. Januar 2025 14:29:53 UTC schrieb Peter Maydell 
<peter.maydell@linaro.org>:
>On Mon, 20 Jan 2025 at 20:38, Bernhard Beschow <shentey@gmail.com> wrote:
>>
>> As a first step, implement the bare minimum: CPUs, RAM, interrupt controller,
>> serial. All other devices of the A53 memory map are represented as
>> TYPE_UNIMPLEMENTED_DEVICE, i.e. the whole memory map is provided. This allows
>> for running Linux without it crashing due to invalid memory accesses.
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>  MAINTAINERS                    |   9 +
>>  docs/system/arm/imx8mp-evk.rst |  47 +++++
>>  docs/system/target-arm.rst     |   1 +
>>  include/hw/arm/fsl-imx8mp.h    | 189 +++++++++++++++++
>>  hw/arm/fsl-imx8mp.c            | 371 +++++++++++++++++++++++++++++++++
>>  hw/arm/imx8mp-evk.c            |  55 +++++
>>  hw/arm/Kconfig                 |  13 ++
>>  hw/arm/meson.build             |   2 +
>>  8 files changed, 687 insertions(+)
>>  create mode 100644 docs/system/arm/imx8mp-evk.rst
>>  create mode 100644 include/hw/arm/fsl-imx8mp.h
>>  create mode 100644 hw/arm/fsl-imx8mp.c
>>  create mode 100644 hw/arm/imx8mp-evk.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 846b81e3ec..cace8cf25b 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -819,6 +819,15 @@ F: hw/pci-host/designware.c
>>  F: include/hw/pci-host/designware.h
>>  F: docs/system/arm/mcimx7d-sabre.rst
>>
>> +MCIMX8MP-EVK / i.MX8MP
>> +M: Bernhard Beschow <shentey@gmail.com>
>> +L: qemu-arm@nongnu.org
>> +S: Odd Fixes
>
>I'd rather we didn't take on a new board model that
>starts out in the "odd fixes" state. If you don't
>have enough time to look after it, it seems unlikely
>that anybody else is going to want to take it over.

I'd look after it, but I'd also not raise expectations regarding commercial 
grade support. What state would you suggest?

>
>> +F: hw/arm/imx8mp-evk.c
>> +F: hw/arm/fsl-imx8mp.c
>> +F: include/hw/arm/fsl-imx8mp.h
>> +F: docs/system/arm/imx8mp-evk.rst
>> +
>>  MPS2 / MPS3
>>  M: Peter Maydell <peter.maydell@linaro.org>
>>  L: qemu-arm@nongnu.org
>
>> +static void fsl_imx8mp_realize(DeviceState *dev, Error **errp)
>> +{
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>> +    FslImx8mpState *s = FSL_IMX8MP(dev);
>> +    DeviceState *gicdev = DEVICE(&s->gic);
>> +    int i;
>> +
>> +    if (ms->smp.cpus > FSL_IMX8MP_NUM_CPUS) {
>> +        error_setg(errp, "%s: Only %d CPUs are supported (%d requested)",
>> +                   TYPE_FSL_IMX8MP, FSL_IMX8MP_NUM_CPUS, ms->smp.cpus);
>> +        return;
>> +    }
>> +
>> +    /* CPUs */
>> +    for (i = 0; i < ms->smp.cpus; i++) {
>> +        /* On uniprocessor, the CBAR is set to 0 */
>> +        if (ms->smp.cpus > 1) {
>> +            object_property_set_int(OBJECT(&s->cpu[i]), "reset-cbar",
>> +                                    
>> fsl_imx8mp_memmap[FSL_IMX8MP_GIC_DIST].addr,
>> +                                    &error_abort);
>> +        }
>> +
>> +        /*
>> +         * Magic value form Linux output: "arch_timer: cp15 timer(s) 
>> running at
>> +         * 8.00MHz (phys)".
>
>"from"

Will fix.

>
>> +         */
>> +        object_property_set_int(OBJECT(&s->cpu[i]), "cntfrq", 8000000,
>> +                                &error_abort);
>> +
>> +        if (i) {
>> +            /*
>> +             * Secondary CPUs start in powered-down state (and can be
>> +             * powered up via the SRC system reset controller)
>> +             */
>> +            object_property_set_bool(OBJECT(&s->cpu[i]), 
>> "start-powered-off",
>> +                                     true, &error_abort);
>> +        }
>> +
>> +        if (!qdev_realize(DEVICE(&s->cpu[i]), NULL, errp)) {
>> +            return;
>> +        }
>> +    }
>> +
>
>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>> index fe47e259b8..adb4ed8076 100644
>> --- a/hw/arm/Kconfig
>> +++ b/hw/arm/Kconfig
>> @@ -577,6 +577,19 @@ config FSL_IMX7
>>      select SDHCI
>>      select UNIMP
>>
>> +config FSL_IMX8MP
>> +    bool
>> +    imply TEST_DEVICES
>
>Why do we need TEST_DEVICES ?

Copied from FSL_IMX7. Will drop.

>
>> +    select ARM_GIC
>> +    select IMX
>> +    select UNIMP
>> +
>> +config FSL_IMX8MP_EVK
>> +    bool
>> +    default y
>> +    depends on TCG && AARCH64
>> +    select FSL_IMX8MP
>> +
>>  config ARM_SMMUV3
>>      bool
>
>Looks good otherwise.

Thanks,
Bernhard

>
>thanks
>-- PMM