On Tue, Oct 8, 2019 at 2:43 AM Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
>
> Hi Alistair,
>
> On 9/27/19 11:42 PM, Alistair Francis wrote:
> >   On Thu, Sep 26, 2019 at 10:44 AM Philippe Mathieu-Daudé
> > <f4...@amsat.org> wrote:
> >>
> >> Base addresses and sizes taken from the "BCM2835 ARM Peripherals"
> >> datasheet from February 06 2012:
> >> https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
> >>
> >> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
> >> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> >> ---
> >>   hw/arm/bcm2835_peripherals.c         | 31 ++++++++++++++++++++++++++++
> >>   include/hw/arm/bcm2835_peripherals.h | 15 ++++++++++++++
> >>   include/hw/arm/raspi_platform.h      |  8 +++++++
> >>   3 files changed, 54 insertions(+)
> >>
> >> diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
> >> index 1bd2ff41d5..fdcf616c56 100644
> >> --- a/hw/arm/bcm2835_peripherals.c
> >> +++ b/hw/arm/bcm2835_peripherals.c
> >> @@ -22,6 +22,20 @@
> >>   /* Capabilities for SD controller: no DMA, high-speed, default clocks 
> >> etc. */
> >>   #define BCM2835_SDHC_CAPAREG 0x52134b4
> >>
> >> +static void create_unimp(BCM2835PeripheralState *ps,
> >> +                         UnimplementedDeviceState *uds,
> >> +                         const char *name, hwaddr ofs, hwaddr size)
> >> +{
> >> +    sysbus_init_child_obj(OBJECT(ps), name, uds,
> >> +                          sizeof(UnimplementedDeviceState),
> >> +                          TYPE_UNIMPLEMENTED_DEVICE);
> >> +    qdev_prop_set_string(DEVICE(uds), "name", name);
> >> +    qdev_prop_set_uint64(DEVICE(uds), "size", size);
> >> +    object_property_set_bool(OBJECT(uds), true, "realized", &error_fatal);
> >> +    memory_region_add_subregion_overlap(&ps->peri_mr, ofs,
> >> +                    sysbus_mmio_get_region(SYS_BUS_DEVICE(uds), 0), 
> >> -1000);
> >
> > Why not just use create_unimplemented_device() and not bother saving
> > the UnimplementedDeviceState members in the struct?
>
> create_unimplemented_device() calls
>   -> sysbus_mmio_map_overlap()
>      -> sysbus_mmio_map_common()
>        -> memory_region_del_subregion(get_system_memory())
>
> So it maps the device at *absolute* offset in the system memory.
>
> create_unimp() maps the device at offset *relative* to peri_mr.
>
> Patch 8 of this series maps the PERI (container) block at peri_base
> (fixed at BCM2836_PERI_BASE=0x3F000000 for the 2836/2837), then patch 12
> adds the 2838 which has PERI mapped at 0xfe000000. So we have the same
> "container" block mapped at different addresses.
> Not the PERI block itself doesn't know its base address, all offsets are
> relative.
>
> So using create_unimp() allow to use the same block in two different SoCs.

Ah, makes sense.

Maybe that is worth adding in the commit message, unless it's just
obvious and I am missing something.

Either way:

Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>

Alistair

>
> 8:  https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg00678.html
> 12: https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg00684.html
>
> >> +}
> >> +
> >>   static void bcm2835_peripherals_init(Object *obj)
> >>   {
> >>       BCM2835PeripheralState *s = BCM2835_PERIPHERALS(obj);
> >> @@ -323,6 +337,23 @@ static void bcm2835_peripherals_realize(DeviceState 
> >> *dev, Error **errp)
> >>           error_propagate(errp, err);
> >>           return;
> >>       }
> >> +
> >> +    create_unimp(s, &s->armtmr, "bcm2835-sp804", ARMCTRL_TIMER0_1_OFFSET, 
> >> 0x40);
> >> +    create_unimp(s, &s->systmr, "bcm2835-systimer", ST_OFFSET, 0x20);
> >> +    create_unimp(s, &s->cprman, "bcm2835-cprman", CPRMAN_OFFSET, 0x1000);
> >> +    create_unimp(s, &s->a2w, "bcm2835-a2w", A2W_OFFSET, 0x1000);
> >> +    create_unimp(s, &s->i2s, "bcm2835-i2s", I2S_OFFSET, 0x100);
> >> +    create_unimp(s, &s->smi, "bcm2835-smi", SMI_OFFSET, 0x100);
> >> +    create_unimp(s, &s->spi[0], "bcm2835-spi0", SPI0_OFFSET, 0x20);
> >> +    create_unimp(s, &s->bscsl, "bcm2835-spis", BSC_SL_OFFSET, 0x100);
> >> +    create_unimp(s, &s->i2c[0], "bcm2835-i2c0", BSC0_OFFSET, 0x20);
> >> +    create_unimp(s, &s->i2c[1], "bcm2835-i2c1", BSC1_OFFSET, 0x20);
> >> +    create_unimp(s, &s->i2c[2], "bcm2835-i2c2", BSC2_OFFSET, 0x20);
> >> +    create_unimp(s, &s->otp, "bcm2835-otp", OTP_OFFSET, 0x80);
> >> +    create_unimp(s, &s->dbus, "bcm2835-dbus", DBUS_OFFSET, 0x8000);
> >> +    create_unimp(s, &s->ave0, "bcm2835-ave0", AVE0_OFFSET, 0x8000);
> >> +    create_unimp(s, &s->dwc2, "dwc-usb2", USB_OTG_OFFSET, 0x1000);
> >> +    create_unimp(s, &s->sdramc, "bcm2835-sdramc", SDRAMC_OFFSET, 0x100);
> >>   }
> >>
> >>   static void bcm2835_peripherals_class_init(ObjectClass *oc, void *data)
> >> diff --git a/include/hw/arm/bcm2835_peripherals.h 
> >> b/include/hw/arm/bcm2835_peripherals.h
> >> index 6b17f6a382..62a4c7b559 100644
> >> --- a/include/hw/arm/bcm2835_peripherals.h
> >> +++ b/include/hw/arm/bcm2835_peripherals.h
> >> @@ -23,6 +23,7 @@
> >>   #include "hw/sd/sdhci.h"
> >>   #include "hw/sd/bcm2835_sdhost.h"
> >>   #include "hw/gpio/bcm2835_gpio.h"
> >> +#include "hw/misc/unimp.h"
> >>
> >>   #define TYPE_BCM2835_PERIPHERALS "bcm2835-peripherals"
> >>   #define BCM2835_PERIPHERALS(obj) \
> >> @@ -37,6 +38,10 @@ typedef struct BCM2835PeripheralState {
> >>       MemoryRegion ram_alias[4];
> >>       qemu_irq irq, fiq;
> >>
> >> +    UnimplementedDeviceState systmr;
> >> +    UnimplementedDeviceState armtmr;
> >> +    UnimplementedDeviceState cprman;
> >> +    UnimplementedDeviceState a2w;
> >>       PL011State uart0;
> >>       BCM2835AuxState aux;
> >>       BCM2835FBState fb;
> >> @@ -48,6 +53,16 @@ typedef struct BCM2835PeripheralState {
> >>       SDHCIState sdhci;
> >>       BCM2835SDHostState sdhost;
> >>       BCM2835GpioState gpio;
> >> +    UnimplementedDeviceState i2s;
> >> +    UnimplementedDeviceState spi[1];
> >> +    UnimplementedDeviceState i2c[3];
> >> +    UnimplementedDeviceState otp;
> >> +    UnimplementedDeviceState dbus;
> >> +    UnimplementedDeviceState ave0;
> >> +    UnimplementedDeviceState bscsl;
> >> +    UnimplementedDeviceState smi;
> >> +    UnimplementedDeviceState dwc2;
> >> +    UnimplementedDeviceState sdramc;
> >>   } BCM2835PeripheralState;
> >>
> >>   #endif /* BCM2835_PERIPHERALS_H */
> >> diff --git a/include/hw/arm/raspi_platform.h 
> >> b/include/hw/arm/raspi_platform.h
> >> index 66969fac5d..cdcbca943f 100644
> >> --- a/include/hw/arm/raspi_platform.h
> >> +++ b/include/hw/arm/raspi_platform.h
> >> @@ -38,6 +38,8 @@
> >>                                                         * Doorbells & 
> >> Mailboxes */
> >>   #define CPRMAN_OFFSET           0x100000 /* Power Management, Watchdog */
> >>   #define CM_OFFSET               0x101000 /* Clock Management */
> >> +#define A2W_OFFSET              0x102000 /* Reset controller */
> >> +#define AVS_OFFSET              0x103000 /* Audio Video Standard */
> >>   #define RNG_OFFSET              0x104000
> >>   #define GPIO_OFFSET             0x200000
> >>   #define UART0_OFFSET            0x201000
> >> @@ -45,11 +47,17 @@
> >>   #define I2S_OFFSET              0x203000
> >>   #define SPI0_OFFSET             0x204000
> >>   #define BSC0_OFFSET             0x205000 /* BSC0 I2C/TWI */
> >> +#define OTP_OFFSET              0x20f000
> >> +#define BSC_SL_OFFSET           0x214000 /* SPI slave */
> >>   #define AUX_OFFSET              0x215000 /* AUX: UART1/SPI1/SPI2 */
> >>   #define EMMC1_OFFSET            0x300000
> >>   #define SMI_OFFSET              0x600000
> >>   #define BSC1_OFFSET             0x804000 /* BSC1 I2C/TWI */
> >> +#define BSC2_OFFSET             0x805000 /* BSC2 I2C/TWI */
> >> +#define DBUS_OFFSET             0x900000
> >> +#define AVE0_OFFSET             0x910000
> >>   #define USB_OTG_OFFSET          0x980000 /* DTC_OTG USB controller */
> >> +#define SDRAMC_OFFSET           0xe00000
> >>   #define DMA15_OFFSET            0xE05000 /* DMA controller, channel 15 */
> >>
> >>   /* GPU interrupts */
> >> --
> >> 2.20.1
> >>
> >>

Reply via email to