Re: [PATCH v4 05/12] hw/arm: Add NPCM730 and NPCM750 SoC models

[Philippe Mathieu-Daudé]

On 7/9/20 2:06 AM, Havard Skinnemoen wrote:
> On Wed, Jul 8, 2020 at 11:13 AM Havard Skinnemoen
> <hskinnemoen@google.com> wrote:
>> On Wed, Jul 8, 2020 at 10:31 AM Philippe Mathieu-Daudé <f4bug@amsat.org> 
>> wrote:
>>> On 7/7/20 8:47 PM, Havard Skinnemoen wrote:
>>>> +typedef struct NPCM7xxClass {
>>>> +    DeviceClass         parent;
>>>
>>> Similar comment that elsewhere on this series, if NPCM7xxClass not used
>>> outside of npcm7xx.c, keep it local.
>>
>> OK, will do.
> 
> Turns out it is used in npcm7xx_boards.c, so it has to stay where it is.

Indeed:

static void npcm7xx_load_kernel(MachineState *machine,
                                NPCM7xxState *soc)
{
    NPCM7xxClass *sc = NPCM7XX_GET_CLASS(soc);

    npcm7xx_binfo.ram_size = machine->ram_size;
    npcm7xx_binfo.nb_cpus = sc->num_cpus;

    arm_load_kernel(&soc->cpu[0], machine, &npcm7xx_binfo);
}

This is fine.

Just thinking loudly, we traditionally add the load_kernel() code
in the machine, because it is often specific to Linux guest, and
the SoC doesn't need to know about the guest OS.

hw/arm/boot.c contains helpers also useful for firmwares.

The SoC has a link to the DRAM so can get its size.
All the arm_boot_info fields are specific to this SoC.
So we could move a lot of code to npcm7xx.c, only declaring:

  void npcm7xx_load_kernel(MachineState *machine,
                           NPCM7xxState *soc);