Re: [Qemu-devel] [PATCH v2 1/2] arm: Stub out NRF51 TWI magnetometer/accelerometer detection

[Stefan Hajnoczi <stefanha@gmail.com>]

On Fri, Jan 18, 2019 at 1:59 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 10 Jan 2019 at 09:40, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > From: Steffen Görtz <contrib@steffen-goertz.de>
> >
> > Recent microbit firmwares panic if the TWI magnetometer/accelerometer
> > devices are not detected during startup.  We don't implement TWI (I2C)
> > so let's stub out these devices just to let the firmware boot.
> >
>
> > --- a/hw/arm/microbit.c
> > +++ b/hw/arm/microbit.c
> > @@ -16,11 +16,13 @@
> >  #include "exec/address-spaces.h"
> >
> >  #include "hw/arm/nrf51_soc.h"
> > +#include "hw/i2c/microbit_i2c.h"
> >
> >  typedef struct {
> >      MachineState parent;
> >
> >      NRF51State nrf51;
> > +    MicrobitI2CState i2c;
> >  } MicrobitMachineState;
> >
> >  #define TYPE_MICROBIT_MACHINE MACHINE_TYPE_NAME("microbit")
> > @@ -32,7 +34,9 @@ static void microbit_init(MachineState *machine)
> >  {
> >      MicrobitMachineState *s = MICROBIT_MACHINE(machine);
> >      MemoryRegion *system_memory = get_system_memory();
> > +    MemoryRegion *mr;
> >      Object *soc = OBJECT(&s->nrf51);
> > +    Object *i2c = OBJECT(&s->i2c);
> >
> >      sysbus_init_child_obj(OBJECT(machine), "nrf51", soc, sizeof(s->nrf51),
> >                            TYPE_NRF51_SOC);
> > @@ -41,6 +45,17 @@ static void microbit_init(MachineState *machine)
> >                               &error_fatal);
> >      object_property_set_bool(soc, true, "realized", &error_fatal);
> >
> > +    /* Overlap the TWI stub device into the SoC.  This is a microbit-specific
> > +     * hack until we implement the nRF51 TWI controller properly and the
> > +     * magnetometer/accelerometer devices.
> > +     */
> > +    sysbus_init_child_obj(OBJECT(machine), "microbit.twi", i2c,
> > +                          sizeof(s->i2c), TYPE_MICROBIT_I2C);
> > +    object_property_set_bool(i2c, true, "realized", &error_fatal);
> > +    mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(i2c), 0);
> > +    memory_region_add_subregion_overlap(&s->nrf51.container, NRF51_TWI_BASE,
> > +                                        mr, -1);
>
> This is an SoC device, right? Why are we creating it here
> in the board code rather than in the SoC object's code ?

Yes, this is a microbit-specific hack.  We don't implement a full
TWI/I2C bus for the nRF51 SoC.  This implementation is just aimed at
stubbing out the microbit-specific accelerometer/magnetometer devices.

Therefore I decided it was cleanest to place it in the microbit
machine type instead of dirtying the nRF51 SoC with a microbit hack.

If the I2C passthrough GSoC 2019 project happens we'll get proper
nRF51 TWI/I2C controller emulation and can eliminate this stub.  But
for now this allows the microbit to boot and doesn't mess up any other
potential machine types that will use the nRF51 SoC.

Stefan