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

[Peter Maydell]

On Sat, 5 Jan 2019 at 15:02, Stefan Hajnoczi <address@hidden> wrote:
>
> From: Steffen Görtz <address@hidden>
>
> 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.
>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> Based-on: <address@hidden>
> ---
> Steffen: Please post your Signed-off-by.  I did some minor cleanups so
> your patch can be merged.  Thanks!
>
>  include/hw/arm/nrf51.h     |  1 +
>  include/hw/arm/nrf51_soc.h |  1 +
>  hw/arm/nrf51_soc.c         | 62 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 64 insertions(+)

>
> +/* Two-Wire Interface (TWI) is not implemented but the microbit-dal firmware
> + * panics if the TWI accelerometer and magnetometer WHO_AM_I registers cannot
> + * be read.  Stub out this read sequence so microbit-dal starts up.
> + */
> +static uint32_t twi_read_sequence[] = {0x5A, 0x5A, 0x40};
> +static uint32_t twi_regs[0x1000 / 4];
> +
> +static uint64_t twi_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> +    static int i;
> +    uint64_t data = 0x00;
> +
> +    switch (addr) {
> +    case NRF51_TWI_EVENT_STOPPED:
> +        data = 0x01;
> +        break;
> +    case NRF51_TWI_EVENT_RXDREADY:
> +        data = 0x01;
> +        break;
> +    case NRF51_TWI_EVENT_TXDSENT:
> +        data = 0x01;
> +        break;
> +    case NRF51_TWI_REG_RXD:
> +        data = twi_read_sequence[i];
> +        if (i < ARRAY_SIZE(twi_read_sequence)) {
> +            i++;
> +        }
> +        break;
> +    default:
> +        data = twi_regs[addr / 4];
> +        break;
> +    }
> +
> +    qemu_log_mask(LOG_UNIMP, "%s: 0x%" HWADDR_PRIx " [%u] = %" PRIx32 "\n",
> +                  __func__, addr, size, (uint32_t)data);
> +
> +
> +    return data;
> +}
> +
> +static void twi_write(void *opaque, hwaddr addr, uint64_t data,
> +                      unsigned int size)
> +{
> +    qemu_log_mask(LOG_UNIMP, "%s: 0x%" HWADDR_PRIx " <- 0x%" PRIx64 " 
> [%u]\n",
> +                  __func__, addr, data, size);
> +    twi_regs[addr / 4] = data;
> +}
> +
> +static const MemoryRegionOps twi_ops = {
> +    .read = twi_read,
> +    .write = twi_write
> +};

Can you put this in its own device in its own source file,
please? This is too much device-specific code to have
in the SoC's source file.

thanks
-- PMM