Gilles Grimaud <[email protected]> writes:

> From: gilles grimaud <[email protected]>
>
> Add shared helpers for reporting unimplemented RP2040 features and register 
> accesses. Keeping the formatting in one place makes shallow peripheral models 
> explicit and gives their diagnostics a consistent component, address, offset 
> and value format.
>
> Add a dedicated Kconfig symbol so the helpers remain disabled until
> selected by the RP2040 SoC.

This seems over-engineered. What is wrong with:

  qemu_log_mask(LOG_UNIMP, "%s: ....", __func__, ...);

In the various call sites. Most of the funcs seem to be prefixed anyway.

In the meantime you can instantiate TYPE_UNIMPLEMENTED_DEVICE for any
memory regions you have no implementation for.

>
> Signed-off-by: gilles grimaud <[email protected]>
> ---
>  MAINTAINERS                  |  7 ++++++
>  hw/misc/Kconfig              |  3 +++
>  hw/misc/meson.build          |  1 +
>  hw/misc/rp2040_nyi.c         | 47 ++++++++++++++++++++++++++++++++++++
>  include/hw/misc/rp2040_nyi.h | 19 +++++++++++++++
>  5 files changed, 77 insertions(+)
>  create mode 100644 hw/misc/rp2040_nyi.c
>  create mode 100644 include/hw/misc/rp2040_nyi.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 55f6f2e3c0..db641d9c4c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1035,6 +1035,13 @@ F: docs/system/arm/raspi.rst
>  F: tests/functional/arm/test_raspi2.py
>  F: tests/functional/aarch64/test_raspi*.py
>  
> +Raspberry Pi Pico / RP2040
> +M: Gilles Grimaud <[email protected]>
> +L: [email protected]
> +S: Maintained
> +F: hw/*/rp2040*
> +F: include/hw/*/rp2040*
> +
>  Real View
>  M: Peter Maydell <[email protected]>
>  L: [email protected]
> diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> index 46e3c03cc8..de7a0f56fa 100644
> --- a/hw/misc/Kconfig
> +++ b/hw/misc/Kconfig
> @@ -101,6 +101,9 @@ config FSL_IMX8MP_ANALOG
>  config FSL_IMX8MP_CCM
>      bool
>  
> +config RP2040_NYI
> +    bool
> +
>  config STM32_RCC
>      bool
>  
> diff --git a/hw/misc/meson.build b/hw/misc/meson.build
> index 54e07aacda..8611d59437 100644
> --- a/hw/misc/meson.build
> +++ b/hw/misc/meson.build
> @@ -98,6 +98,7 @@ system_ss.add(when: 'CONFIG_RASPI', if_true: files(
>    'bcm2835_cprman.c',
>    'bcm2835_powermgt.c',
>  ))
> +system_ss.add(when: 'CONFIG_RP2040_NYI', if_true: files('rp2040_nyi.c'))
>  system_ss.add(when: 'CONFIG_SLAVIO', if_true: files('slavio_misc.c'))
>  system_ss.add(when: 'CONFIG_ZYNQ', if_true: files('zynq_slcr.c'))
>  system_ss.add(when: 'CONFIG_XLNX_ZYNQ_DDRC', if_true: 
> files('xlnx-zynq-ddrc.c'))
> diff --git a/hw/misc/rp2040_nyi.c b/hw/misc/rp2040_nyi.c
> new file mode 100644
> index 0000000000..7e9e4c09a1
> --- /dev/null
> +++ b/hw/misc/rp2040_nyi.c
> @@ -0,0 +1,47 @@
> +/*
> + * RP2040 "not yet implemented" diagnostics
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/misc/rp2040_nyi.h"
> +#include "qemu/log.h"
> +
> +void rp2040_log_nyi(const char *component, const char *feature,
> +                    const char *detail)
> +{
> +    if (g_str_has_prefix(component, "rp2040.")) {
> +        component += strlen("rp2040.");
> +    }
> +
> +    qemu_log_mask(LOG_UNIMP, "Not yet implemented: rp2040.%s: %s%s%s\n",
> +                  component, feature, detail ? ": " : "",
> +                  detail ? detail : "");
> +}
> +
> +void rp2040_log_unimplemented_read(const char *component, unsigned size,
> +                                   uint64_t addr, uint64_t offset,
> +                                   uint64_t value)
> +{
> +    char detail[128];
> +
> +    snprintf(detail, sizeof(detail),
> +             "size %u, addr 0x%08" PRIx64 ", offset 0x%04" PRIx64
> +             " -> 0x%0*" PRIx64,
> +             size, addr, offset, size << 1, value);
> +    rp2040_log_nyi(component, "unimplemented read", detail);
> +}
> +
> +void rp2040_log_unimplemented_write(const char *component, unsigned size,
> +                                    uint64_t addr, uint64_t offset,
> +                                    uint64_t value)
> +{
> +    char detail[128];
> +
> +    snprintf(detail, sizeof(detail),
> +             "size %u, addr 0x%08" PRIx64 ", offset 0x%04" PRIx64
> +             ", value 0x%0*" PRIx64,
> +             size, addr, offset, size << 1, value);
> +    rp2040_log_nyi(component, "unimplemented write", detail);
> +}
> diff --git a/include/hw/misc/rp2040_nyi.h b/include/hw/misc/rp2040_nyi.h
> new file mode 100644
> index 0000000000..c23805973b
> --- /dev/null
> +++ b/include/hw/misc/rp2040_nyi.h
> @@ -0,0 +1,19 @@
> +/*
> + * RP2040 "not yet implemented" diagnostics
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef HW_MISC_RP2040_NYI_H
> +#define HW_MISC_RP2040_NYI_H
> +
> +void rp2040_log_nyi(const char *component, const char *feature,
> +                    const char *detail);
> +void rp2040_log_unimplemented_read(const char *component, unsigned size,
> +                                   uint64_t addr, uint64_t offset,
> +                                   uint64_t value);
> +void rp2040_log_unimplemented_write(const char *component, unsigned size,
> +                                    uint64_t addr, uint64_t offset,
> +                                    uint64_t value);
> +
> +#endif

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Reply via email to