[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 04/11] hw/arm: Add NPCM730 and NPCM750 SoC models
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v5 04/11] hw/arm: Add NPCM730 and NPCM750 SoC models |
Date: |
Tue, 14 Jul 2020 13:37:48 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
+Markus
On 7/14/20 2:44 AM, Havard Skinnemoen wrote:
> On Mon, Jul 13, 2020 at 8:02 AM Cédric Le Goater <clg@kaod.org> wrote:
>>
>> On 7/9/20 2:36 AM, Havard Skinnemoen wrote:
>>> The Nuvoton NPCM7xx SoC family are used to implement Baseboard
>>> Management Controllers in servers. While the family includes four SoCs,
>>> this patch implements limited support for two of them: NPCM730 (targeted
>>> for Data Center applications) and NPCM750 (targeted for Enterprise
>>> applications).
>>>
>>> This patch includes little more than the bare minimum needed to boot a
>>> Linux kernel built with NPCM7xx support in direct-kernel mode:
>>>
>>> - Two Cortex-A9 CPU cores with built-in periperhals.
>>> - Global Configuration Registers.
>>> - Clock Management.
>>> - 3 Timer Modules with 5 timers each.
>>> - 4 serial ports.
>>>
>>> The chips themselves have a lot more features, some of which will be
>>> added to the model at a later stage.
>>>
>>> Reviewed-by: Tyrone Ting <kfting@nuvoton.com>
>>> Reviewed-by: Joel Stanley <joel@jms.id.au>
>>> Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com>
>>> ---
...
>>> +static void npcm7xx_realize(DeviceState *dev, Error **errp)
>>> +{
>>> + NPCM7xxState *s = NPCM7XX(dev);
>>> + NPCM7xxClass *nc = NPCM7XX_GET_CLASS(s);
>>> + int i;
>>> +
>>> + /* CPUs */
>>> + for (i = 0; i < nc->num_cpus; i++) {
>>> + object_property_set_int(OBJECT(&s->cpu[i]),
>>> + arm_cpu_mp_affinity(i,
>>> NPCM7XX_MAX_NUM_CPUS),
>>> + "mp-affinity", &error_abort);
>>> + object_property_set_int(OBJECT(&s->cpu[i]),
>>> NPCM7XX_GIC_CPU_IF_ADDR,
>>> + "reset-cbar", &error_abort);
>>> + object_property_set_bool(OBJECT(&s->cpu[i]), true,
>>> + "reset-hivecs", &error_abort);
>>> +
>>> + /* Disable security extensions. */
>>> + object_property_set_bool(OBJECT(&s->cpu[i]), false, "has_el3",
>>> + &error_abort);
>>> +
>>> + qdev_realize(DEVICE(&s->cpu[i]), NULL, &error_abort);
>>
>> I would check the error:
>>
>> if (!qdev_realize(DEVICE(&s->cpu[i]), NULL, errp)) {
>> return;
>> }
>>
>> same for the sysbus_realize() below.
>
> Hmm, I used to propagate these errors until Philippe told me not to
> (or at least that's how I understood it).
It was before Markus simplification API were merged, you had to
propagate after each call, since this is a non hot-pluggable SoC
I suggested to use &error_abort to simplify.
> I'll be happy to do it
> either way (and the new API makes it really easy to propagate errors),
> but I worry that I don't fully understand when to propagate errors and
> when not to.
Markus explained it on the mailing list recently (as I found the doc
not obvious). I can't find the thread. I suppose once the work result
after the "Questionable aspects of QEMU Error's design" discussion is
merged, the documentation will be clarified.
My rule of thumb so far is:
- programming error (can't happen) -> &error_abort
- everything triggerable by user or management layer (via QMP command)
-> &error_fatal, as we can't risk loose the user data, we need to
shutdown gracefully.
>
> It makes sense to me to propagate errors from *_realize() and
> error_abort on failure to set simple properties, but I'd like to know
> if Philippe is on board with that.
>
>>> + }
>>> +
>>> + /* A9MPCORE peripherals */
>>> + object_property_set_int(OBJECT(&s->a9mpcore), nc->num_cpus, "num-cpu",
>>> + &error_abort);
>>> + object_property_set_int(OBJECT(&s->a9mpcore), NPCM7XX_NUM_IRQ,
>>> "num-irq",
>>> + &error_abort);
>>> + sysbus_realize(SYS_BUS_DEVICE(&s->a9mpcore), &error_abort);
>>> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->a9mpcore), 0, NPCM7XX_CPUP_BA);
>>> +
>>> + for (i = 0; i < nc->num_cpus; i++) {
>>> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->a9mpcore), i,
>>> + qdev_get_gpio_in(DEVICE(&s->cpu[i]),
>>> ARM_CPU_IRQ));
>>> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->a9mpcore), i + nc->num_cpus,
>>> + qdev_get_gpio_in(DEVICE(&s->cpu[i]),
>>> ARM_CPU_FIQ));
>>> + }
...
- [PATCH v5 02/11] hw/misc: Add NPCM7xx Clock Controller device model, (continued)
- [PATCH v5 04/11] hw/arm: Add NPCM730 and NPCM750 SoC models, Havard Skinnemoen, 2020/07/08
- Re: [PATCH v5 04/11] hw/arm: Add NPCM730 and NPCM750 SoC models, Philippe Mathieu-Daudé, 2020/07/09
- Re: [PATCH v5 04/11] hw/arm: Add NPCM730 and NPCM750 SoC models, Cédric Le Goater, 2020/07/13
- Re: [PATCH v5 04/11] hw/arm: Add NPCM730 and NPCM750 SoC models, Havard Skinnemoen, 2020/07/13
- Re: [PATCH v5 04/11] hw/arm: Add NPCM730 and NPCM750 SoC models,
Philippe Mathieu-Daudé <=
- Re: [PATCH v5 04/11] hw/arm: Add NPCM730 and NPCM750 SoC models, Markus Armbruster, 2020/07/14
- Re: [PATCH v5 04/11] hw/arm: Add NPCM730 and NPCM750 SoC models, Philippe Mathieu-Daudé, 2020/07/14
- Re: [PATCH v5 04/11] hw/arm: Add NPCM730 and NPCM750 SoC models, Havard Skinnemoen, 2020/07/14
- Re: [PATCH v5 04/11] hw/arm: Add NPCM730 and NPCM750 SoC models, Markus Armbruster, 2020/07/15
[PATCH v5 06/11] hw/arm: Load -bios image as a boot ROM for npcm7xx, Havard Skinnemoen, 2020/07/08
[PATCH v5 05/11] hw/arm: Add two NPCM7xx-based machines, Havard Skinnemoen, 2020/07/08
[PATCH v5 08/11] hw/mem: Stubbed out NPCM7xx Memory Controller model, Havard Skinnemoen, 2020/07/08