On Thu, 30 Jan 2025 at 18:27, Philippe Mathieu-Daudé <phi...@linaro.org> wrote:
>
> Implicit default values are often hard to figure out, better
> be explicit. Now that all boards explicitly set the number of
> GIC external IRQs, remove the default values (displaying an
> error message if it is not set).
> diff --git a/hw/cpu/a15mpcore.c b/hw/cpu/a15mpcore.c
> index 3b0897e54ee..372b615178f 100644
> --- a/hw/cpu/a15mpcore.c
> +++ b/hw/cpu/a15mpcore.c
> @@ -58,6 +58,11 @@ static void a15mp_priv_realize(DeviceState *dev, Error
> **errp)
> bool has_el2 = false;
> Object *cpuobj;
>
> + if (!s->num_irq) {
> + error_setg(errp, "Property 'num-irq' not set");
> + return;
> + }
> +
> gicdev = DEVICE(&s->gic);
> qdev_prop_set_uint32(gicdev, "num-cpu", s->num_cpu);
> qdev_prop_set_uint32(gicdev, "num-irq", s->num_irq);
> @@ -146,13 +151,7 @@ static void a15mp_priv_realize(DeviceState *dev, Error
> **errp)
>
> static const Property a15mp_priv_properties[] = {
> DEFINE_PROP_UINT32("num-cpu", A15MPPrivState, num_cpu, 1),
> - /* The Cortex-A15MP may have anything from 0 to 224 external interrupt
> - * IRQ lines (with another 32 internal). We default to 128+32, which
> - * is the number provided by the Cortex-A15MP test chip in the
> - * Versatile Express A15 development board.
> - * Other boards may differ and should set this property appropriately.
> - */
> - DEFINE_PROP_UINT32("num-irq", A15MPPrivState, num_irq, 160),
> + DEFINE_PROP_UINT32("num-irq", A15MPPrivState, num_irq, 0),
I think it's worth keeping at least some form of comment here
to document the valid range and that the value is internal + external
interrupts. Something like:
/*
* The Cortex-A15MP may have anything from 0 to 224 external interrupt
* lines, plus always 32 internal IRQs. This property sets the total
* of internal + external, so the valid range is from 32 to 256.
* The board model must set this to whatever the configuration
* used for the CPU on that board or SoC is.
*/
?
(I suppose we could also actually check the property really
is between 32 and 256, but a simple "did the board actually set
it?" check like you have here is fine.)
> @@ -160,13 +166,7 @@ static void a9mp_priv_realize(DeviceState *dev, Error
> **errp)
>
> static const Property a9mp_priv_properties[] = {
> DEFINE_PROP_UINT32("num-cpu", A9MPPrivState, num_cpu, 1),
> - /* The Cortex-A9MP may have anything from 0 to 224 external interrupt
> - * IRQ lines (with another 32 internal). We default to 64+32, which
> - * is the number provided by the Cortex-A9MP test chip in the
> - * Realview PBX-A9 and Versatile Express A9 development boards.
> - * Other boards may differ and should set this property appropriately.
> - */
Similarly here.
thanks
-- PMM
