Re: [PATCH RFC 1/1] hw/riscv/k230: add IOMUX register block model

On Fri, 2026-07-10 at 12:18 +0800, Kangjie Huang wrote:
> Add a sysbus model for the K230 IOMUX MMIO range at
> 0x91105000-0x911057ff. The model exposes the Function IO registers
> with
> the reset defaults documented by the K230 Technical Reference Manual
> and
> keeps 32-bit guest writes visible to firmware and the operating
> system.
> 
> This is enough for SDK U-Boot's pinctrl-single driver, which programs
> the board pin configuration with 32-bit read-modify-write accesses.
> 
> Wire the device into the K230 SoC and add qtest coverage for reset
> values, read/write storage and system reset.
> 
> Signed-off-by: Kangjie Huang <[email protected]>
> ---
> RFC notes:
> - This is my first QEMU upstream submission; feedback on patch
> structure

Thanks for the patch

>   and modelling boundaries would be appreciated.
> - The model covers the IOMUX MMIO range 0x91105000-0x911057ff.
> - It is intended to satisfy SDK U-Boot pinctrl-single register
> accesses.
> - PMU IOMUX and physical pin routing are intentionally not modeled.
> - Tested with qtest and SDK U-Boot smoke test.
> 
>  docs/system/riscv/k230.rst    |   1 +
>  hw/misc/Kconfig               |   3 +
>  hw/misc/k230_iomux.c          | 138
> ++++++++++++++++++++++++++++++++++
>  hw/misc/meson.build           |   2 +
>  hw/misc/trace-events          |   4 +
>  hw/riscv/Kconfig              |   1 +
>  hw/riscv/k230.c               |  11 ++-
>  include/hw/misc/k230_iomux.h  |  34 +++++++++
>  include/hw/riscv/k230.h       |   2 +
>  tests/qtest/k230-iomux-test.c | 107 ++++++++++++++++++++++++++
>  tests/qtest/meson.build       |   5 +-
>  11 files changed, 304 insertions(+), 4 deletions(-)
>  create mode 100644 hw/misc/k230_iomux.c
>  create mode 100644 include/hw/misc/k230_iomux.h
>  create mode 100644 tests/qtest/k230-iomux-test.c

Generally you will want to split this up more. Usually one patch to add
the model and then a second patch to connect it and a third patch for
tests.

> 
> diff --git a/docs/system/riscv/k230.rst b/docs/system/riscv/k230.rst
> index cea8202e55..b160c401fd 100644
> --- a/docs/system/riscv/k230.rst
> +++ b/docs/system/riscv/k230.rst
> @@ -20,6 +20,7 @@ The ``k230`` machine supports the following
> devices:
>  * Platform-Level Interrupt Controller (PLIC)
>  * 2 K230 Watchdog Timer
>  * 5 UART
> +* K230 IOMUX register block
>  
>  Boot options
>  ------------
> diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> index 1543ee6653..573d24a9df 100644
> --- a/hw/misc/Kconfig
> +++ b/hw/misc/Kconfig
> @@ -257,4 +257,7 @@ config XLNX_VERSAL_TRNG
>  config XLNX_ZYNQ_DDRC
>      bool
>  
> +config K230_IOMUX
> +    bool
> +
>  source macio/Kconfig
> diff --git a/hw/misc/k230_iomux.c b/hw/misc/k230_iomux.c
> new file mode 100644
> index 0000000000..5f5e2aac51
> --- /dev/null
> +++ b/hw/misc/k230_iomux.c
> @@ -0,0 +1,138 @@
> +/*
> + * K230 IOMUX register block compatible with Kendryte K230 SDK
> + *
> + * Copyright (c) 2026 Kangjie Huang <[email protected]>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * Provides the low IOMUX register block at 0x91105000 with the
> reset
> + * defaults documented by the K230 Technical Reference Manual. The
> model keeps
> + * guest 32-bit register writes visible to firmware and the
> operating system,
> + * but does not model physical pin routing, pad electrical effects,
> or the
> + * separate PMU IOMUX block.
> + *
> + * K230 Technical Reference Manual V0.3.1 (2024-11-18):
> + *
> https://github.com/revyos/external-docs/blob/master/K230/en-us/K230_Technical_Reference_Manual_V0.3.1_20241118.pdf
> + *
> + * For more information, see
> <https://www.kendryte.com/en/proDetail/230>
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/misc/k230_iomux.h"
> +#include "migration/vmstate.h"
> +#include "qemu/module.h"
> +#include "qemu/log.h"
> +#include "trace.h"
> +
> +/* TRM V0.3.1 section 12.9.2.1 Function IO register list. */
> +static const uint32_t k230_iomux_reset_values[] = {
> +    0x944, 0x944, 0x929, 0x908, 0x888, 0x908, 0x948, 0x890,
> +    0x890, 0x890, 0x910, 0x890, 0x890, 0x8a9, 0xa9e, 0xabf,
> +    0xb9e, 0xb9e, 0xb9e, 0xb9e, 0xb9e, 0xb9e, 0xb9e, 0xb9e,
> +    0xb1e, 0xa90, 0xabf, 0xb9e, 0xb9e, 0xb9e, 0xb9e, 0xb9e,
> +    0xbd0, 0xbd0, 0xbd0, 0xbd0, 0xbd0, 0xbd0, 0x890, 0x910,
> +    0x890, 0x910, 0x890, 0x910, 0x890, 0x910, 0x890, 0x910,
> +    0x890, 0x910, 0x890, 0x910, 0x890, 0x910, 0x89e, 0x89f,
> +    0x99e, 0x99e, 0x99e, 0x99e, 0x890, 0x890, 0x8a9, 0x8a9,
> +};
> +
> +static void k230_iomux_reset(DeviceState *dev)
> +{
> +    K230IomuxState *s = K230_IOMUX(dev);
> +
> +    memset(s->regs, 0, sizeof(s->regs));
> +    memcpy(s->regs, k230_iomux_reset_values,
> +           sizeof(k230_iomux_reset_values));
> +}
> +
> +static uint64_t k230_iomux_read(void *opaque, hwaddr offset,
> unsigned size)
> +{
> +    K230IomuxState *s = K230_IOMUX(opaque);
> +    uint32_t value;
> +
> +    if (offset + size > K230_IOMUX_MMIO_SIZE || (offset & 3)) {

You don't need to check the offset or offset + size here, that is
enforced by your min_access_size/max_access_size

> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: invalid access offset 0x%" HWADDR_PRIx
> +                      " size %u\n", __func__, offset, size);
> +        return 0;
> +    }
> +
> +    value = s->regs[offset >> 2];
> +    trace_k230_iomux_read(offset, value);
> +    return value;
> +}
> +
> +static void k230_iomux_write(void *opaque, hwaddr offset,
> +                             uint64_t value, unsigned size)
> +{
> +    K230IomuxState *s = K230_IOMUX(opaque);
> +
> +    if (offset + size > K230_IOMUX_MMIO_SIZE || (offset & 3)) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: invalid access offset 0x%" HWADDR_PRIx
> +                      " size %u\n", __func__, offset, size);
> +        return;
> +    }
> +
> +    trace_k230_iomux_write(offset, value);
> +    s->regs[offset >> 2] = value;

Do you only ever expect to just access an array? So there is no other
functionality required here?

Alistair