Re: [PATCH 05/19] hw/arm/bcm2835: Add various unimplemented peripherals

["Philippe Mathieu-Daudé" <f4bug@amsat.org>]

Hi Alistair,

On 9/27/19 11:42 PM, Alistair Francis wrote:
>   On Thu, Sep 26, 2019 at 10:44 AM Philippe Mathieu-Daudé
> <f4bug@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.maydell@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@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.

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
>>
>>