Re: [PATCH v4 09/45] Add GPIO and SD to BCM2838 periph

[Peter Maydell]

On Fri, 8 Dec 2023 at 02:37, Sergey Kambalin <serg.oker@gmail.com> wrote:
>
> Signed-off-by: Sergey Kambalin <sergey.kambalin@auriga.com>
> ---
>  hw/arm/bcm2838_peripherals.c         | 140 +++++++++++++++++++++++++++
>  include/hw/arm/bcm2838_peripherals.h |   9 ++
>  2 files changed, 149 insertions(+)


> @@ -42,6 +73,115 @@ static void bcm2838_peripherals_realize(DeviceState *dev, 
> Error **errp)
>                                          BCM2838_VC_PERI_LOW_BASE,
>                                          &s->peri_low_mr_alias, 1);
>
> +    /* Extended Mass Media Controller 2 */
> +    object_property_set_uint(OBJECT(&s->emmc2), "sd-spec-version", 3,
> +                             &error_abort);
> +    object_property_set_uint(OBJECT(&s->emmc2), "capareg",
> +                             BCM2835_SDHC_CAPAREG, &error_abort);
> +    object_property_set_bool(OBJECT(&s->emmc2), "pending-insert-quirk", true,
> +                             &error_abort);
> +    if (!sysbus_realize(SYS_BUS_DEVICE(&s->emmc2), errp)) {
> +        return;
> +    }
> +
> +    memory_region_add_subregion(
> +        &s_base->peri_mr, EMMC2_OFFSET,
> +        sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->emmc2), 0));

Odd indent again here...

> +
> +    /* According to DTS, EMMC and EMMC2 share one irq */
> +    if (!qdev_realize(DEVICE(&s->mmc_irq_orgate), NULL, errp)) {
> +        return;
> +    }
> +
> +    DeviceState *mmc_irq_orgate = DEVICE(&s->mmc_irq_orgate);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->emmc2), 0,
> +                        qdev_get_gpio_in(mmc_irq_orgate, 0));
> +
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s_base->sdhci), 0,
> +                        qdev_get_gpio_in(mmc_irq_orgate, 1));

...and here.

> +
> +   /* Connect EMMC and EMMC2 to the interrupt controller */
> +    qdev_connect_gpio_out(mmc_irq_orgate, 0,
> +                          qdev_get_gpio_in_named(DEVICE(&s_base->ic),
> +                                                 BCM2835_IC_GPU_IRQ,
> +                                                 INTERRUPT_ARASANSDIO));
> +
> +    /* Connect DMA 0-6 to the interrupt controller */
> +    for (n = 0; n < 7; n++) {
> +        sysbus_connect_irq(SYS_BUS_DEVICE(&s_base->dma), n,
> +                           qdev_get_gpio_in_named(DEVICE(&s_base->ic),
> +                                                  BCM2835_IC_GPU_IRQ,
> +                                                  GPU_INTERRUPT_DMA0 + n));
> +    }
> +
> +   /* According to DTS, DMA 7 and 8 share one irq */
> +    if (!qdev_realize(DEVICE(&s->dma_7_8_irq_orgate), NULL, errp)) {
> +        return;
> +    }
> +    DeviceState *dma_7_8_irq_orgate = DEVICE(&s->dma_7_8_irq_orgate);

Declaration not at top of code block (here and below).


Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM