Re: [PATCH v4 06/45] Add BCM2838 GPIO stub

[Peter Maydell]

On Fri, 8 Dec 2023 at 02:39, Sergey Kambalin <serg.oker@gmail.com> wrote:
>
> Signed-off-by: Sergey Kambalin <sergey.kambalin@auriga.com>
> ---
>  hw/arm/bcm2838.c                     |   4 +-
>  hw/gpio/bcm2838_gpio.c               | 152 +++++++++++++++++++++++++++
>  hw/gpio/meson.build                  |   5 +-
>  include/hw/arm/bcm2838_peripherals.h |   2 -
>  include/hw/gpio/bcm2838_gpio.h       |  40 +++++++
>  5 files changed, 198 insertions(+), 5 deletions(-)
>  create mode 100644 hw/gpio/bcm2838_gpio.c
>  create mode 100644 include/hw/gpio/bcm2838_gpio.h
>
> diff --git a/hw/arm/bcm2838.c b/hw/arm/bcm2838.c
> index 042e543006..8925957c6c 100644
> --- a/hw/arm/bcm2838.c
> +++ b/hw/arm/bcm2838.c
> @@ -14,7 +14,7 @@
>  #include "hw/arm/bcm2838.h"
>  #include "trace.h"
>
> -#define GIC400_MAINTAINANCE_IRQ      9
> +#define GIC400_MAINTENANCE_IRQ      9
>  #define GIC400_TIMER_NS_EL2_IRQ     10
>  #define GIC400_TIMER_VIRT_IRQ       11
>  #define GIC400_LEGACY_FIQ           12
> @@ -163,7 +163,7 @@ static void bcm2838_realize(DeviceState *dev, Error 
> **errp)
>
>          sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), n + 4 * BCM283X_NCPUS,
>                             qdev_get_gpio_in(gicdev,
> -                                            PPI(n, 
> GIC400_MAINTAINANCE_IRQ)));
> +                                            PPI(n, GIC400_MAINTENANCE_IRQ)));
>
>          /* Connect timers from the CPU to the interrupt controller */
>          qdev_connect_gpio_out(cpudev, GTIMER_PHYS,

Squash these changes into the previous patch :-)

> diff --git a/hw/gpio/bcm2838_gpio.c b/hw/gpio/bcm2838_gpio.c
> new file mode 100644
> index 0000000000..15b66cb559
> --- /dev/null
> +++ b/hw/gpio/bcm2838_gpio.c
> @@ -0,0 +1,152 @@
> +/*
> + * Raspberry Pi (BCM2838) GPIO Controller
> + * This implementation is based on bcm2835_gpio (hw/gpio/bcm2835_gpio.c)
> + *
> + * Copyright (c) 2022 Auriga LLC
> + *
> + * Authors:
> + *  Lotosh, Aleksey <aleksey.lotosh@auriga.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.

It would be nice to be consistent about whether you want to use
SPDX-License-Identifier tags or not in the new files you're adding.
(Patch 4's bcm2838.c uses it; this one doesn't.)

> + */

> +#define RESET_VAL_CNTRL_REG0 0xAAA95555;
> +#define RESET_VAL_CNTRL_REG1 0xA0AAAAAA;
> +#define RESET_VAL_CNTRL_REG2 0x50AAA95A;
> +#define RESET_VAL_CNTRL_REG3 0x00055555;

These shouldn't have trailing semicolons.

> +
> +#define BYTES_IN_WORD        4

> diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
> index 066ea96480..8a8d03d885 100644
> --- a/hw/gpio/meson.build
> +++ b/hw/gpio/meson.build
> @@ -9,6 +9,9 @@ system_ss.add(when: 'CONFIG_IMX', if_true: 
> files('imx_gpio.c'))
>  system_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_gpio.c'))
>  system_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_gpio.c'))
>  system_ss.add(when: 'CONFIG_OMAP', if_true: files('omap_gpio.c'))
> -system_ss.add(when: 'CONFIG_RASPI', if_true: files('bcm2835_gpio.c'))
> +system_ss.add(when: 'CONFIG_RASPI', if_true: files(
> +    'bcm2835_gpio.c',
> +    'bcm2838_gpio.c'
> +))
>  system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_gpio.c'))
>  system_ss.add(when: 'CONFIG_SIFIVE_GPIO', if_true: files('sifive_gpio.c'))
> diff --git a/include/hw/arm/bcm2838_peripherals.h 
> b/include/hw/arm/bcm2838_peripherals.h
> index 5a72355183..d07831753a 100644
> --- a/include/hw/arm/bcm2838_peripherals.h
> +++ b/include/hw/arm/bcm2838_peripherals.h
> @@ -11,8 +11,6 @@
>
>  #include "hw/arm/bcm2835_peripherals.h"
>
> -#define GENET_OFFSET            0x1580000
> -

Why does this line get deleted ?

>  /* SPI */
>  #define GIC_SPI_INTERRUPT_MBOX         33
>  #define GIC_SPI_INTERRUPT_MPHI         40

thanks
-- PMM