On Wed, 26 Jul 2023 at 14:48, Sergey Kambalin <serg.o...@gmail.com> wrote: > > Signed-off-by: Sergey Kambalin <sergey.kamba...@auriga.com> > --- > hw/arm/bcm2838_peripherals.c | 12 ++++ > hw/arm/raspi4b.c | 1 - > hw/misc/bcm2838_thermal.c | 96 ++++++++++++++++++++++++++++ > hw/misc/meson.build | 3 +- > include/hw/arm/bcm2838_peripherals.h | 2 + > include/hw/misc/bcm2838_thermal.h | 24 +++++++ > 6 files changed, 136 insertions(+), 2 deletions(-) > create mode 100644 hw/misc/bcm2838_thermal.c > create mode 100644 include/hw/misc/bcm2838_thermal.h > > diff --git a/hw/arm/bcm2838_peripherals.c b/hw/arm/bcm2838_peripherals.c > index df97953bbe..e7a9db97ab 100644 > --- a/hw/arm/bcm2838_peripherals.c > +++ b/hw/arm/bcm2838_peripherals.c > @@ -37,6 +37,9 @@ static void bcm2838_peripherals_init(Object *obj) > /* Random Number Generator */ > object_initialize_child(obj, "rng200", &s->rng200, TYPE_BCM2838_RNG200); > > + /* Thermal */ > + object_initialize_child(obj, "thermal", &s->thermal, > TYPE_BCM2838_THERMAL); > + > /* PCIe Host Bridge */ > object_initialize_child(obj, "pcie-host", &s->pcie_host, > TYPE_BCM2838_PCIE_HOST); > @@ -98,6 +101,14 @@ static void bcm2838_peripherals_realize(DeviceState *dev, > Error **errp) > qdev_get_gpio_in_named(DEVICE(&s_base->ic), BCM2835_IC_GPU_IRQ, > INTERRUPT_RNG)); > > + /* THERMAL */ > + if (!sysbus_realize(SYS_BUS_DEVICE(&s->thermal), errp)) { > + return; > + } > + memory_region_add_subregion( > + &s->peri_low_mr, 0x15D2000, > + sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->thermal), 0));
(more odd indentation here) > + > /* Extended Mass Media Controller 2 */ > object_property_set_uint(OBJECT(&s->emmc2), "sd-spec-version", 3, > &error_abort); > @@ -197,6 +208,7 @@ static void bcm2838_peripherals_realize(DeviceState *dev, > Error **errp) > BCM2838_MPHI_SIZE); > memory_region_add_subregion(&s_base->peri_mr, BCM2838_MPHI_OFFSET, > &s->mphi_mr_alias); > + > /* PCIe Root Complex */ > if (!sysbus_realize(SYS_BUS_DEVICE(&s->pcie_host), errp)) { > return; > diff --git a/hw/arm/raspi4b.c b/hw/arm/raspi4b.c > index 82b6b48d3c..c7fdf791d5 100644 > --- a/hw/arm/raspi4b.c > +++ b/hw/arm/raspi4b.c > @@ -66,7 +66,6 @@ static void raspi4_modify_dtb(const struct arm_boot_info > *info, void *fdt) > > /* Temporary disable following devices until they are implemented*/ > const char *to_be_removed_from_dt_as_wa[] = { > - "brcm,bcm2711-thermal", > "brcm,bcm2711-genet-v5", > }; > > diff --git a/hw/misc/bcm2838_thermal.c b/hw/misc/bcm2838_thermal.c > new file mode 100644 > index 0000000000..a488dc3da1 > --- /dev/null > +++ b/hw/misc/bcm2838_thermal.c > @@ -0,0 +1,96 @@ > +/* > + * BCM2838 dummy thermal sensor > + * > + * Copyright (C) 2022 Maksim Kopusov <maksim.kopu...@auriga.com> > + * > + * SPDX-License-Identifier: GPL-2.0-or-later > + */ > + > +#include "qemu/osdep.h" > +#include "qemu/log.h" > +#include "qapi/error.h" > +#include "hw/misc/bcm2838_thermal.h" > +#include "hw/registerfields.h" > +#include "migration/vmstate.h" > +#include "qemu/error-report.h" > + > +REG32(STAT, 0x200) > +FIELD(STAT, DATA, 0, 10) > +FIELD(STAT, VALID_1, 10, 1) > +FIELD(STAT, VALID_2, 16, 1) > + > +#define BCM2838_THERMAL_SIZE 0xf00 > + > +#define THERMAL_OFFSET_C 410040 > +#define THERMAL_COEFF (-487.0f) > +#define MILLIDEGREE_COEFF 1000 > + > +static uint16_t bcm2838_thermal_temp2adc(int temp_C) > +{ > + return (temp_C * MILLIDEGREE_COEFF - THERMAL_OFFSET_C) / THERMAL_COEFF; > +} > + > +static uint64_t bcm2838_thermal_read(void *opaque, hwaddr addr, unsigned > size) > +{ > + uint32_t val = 0; > + > + switch (addr) { > + case A_STAT: > + /* Temperature is always 25°C */ > + val = FIELD_DP32(val, STAT, DATA, bcm2838_thermal_temp2adc(25)); > + val = FIELD_DP32(val, STAT, VALID_1, 1); > + val = FIELD_DP32(val, STAT, VALID_2, 1); > + > + break; > + default: > + /* MemoryRegionOps are aligned, so this can not happen. */ This comment doesn't seem right. BCM2838_THERMAL_SIZE is larger than 4, so we can have more than one possible address accessed, even if the address must be aligned. So the warning is OK, but the g_assert_reached() and the comment are not. > + warn_report("%s can't access addr: 0x%"PRIx64, > + TYPE_BCM2838_THERMAL, addr); We usually use a LOG_GUEST_ERROR for this kind of "access with an address that doesn't correspond to a register". > + g_assert_not_reached(); > + } > + return val; > +} > + > +static const MemoryRegionOps bcm2838_thermal_ops = { > + .read = bcm2838_thermal_read, You need to provide a .write, even if it just does a LOG_GUEST_ERROR for every access. Otherwise if the guest tries to write then QEMU will crash trying to call the NULL pointer. > + .impl.max_access_size = 4, > + .valid.min_access_size = 4, > + .endianness = DEVICE_NATIVE_ENDIAN, > +}; > + > +static void bcm2838_thermal_realize(DeviceState *dev, Error **errp) > +{ > + Bcm2838ThermalState *s = BCM2838_THERMAL(dev); > + > + memory_region_init_io(&s->iomem, OBJECT(s), &bcm2838_thermal_ops, > + s, TYPE_BCM2838_THERMAL, BCM2838_THERMAL_SIZE); > + sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem); > +} > + > +static const VMStateDescription bcm2838_thermal_vmstate = { > + .name = TYPE_BCM2838_THERMAL, > + .version_id = 1, > + .minimum_version_id = 1, > +}; You don't need to set up a vmstate if it has no data. > + > +static void bcm2838_thermal_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + > + dc->realize = bcm2838_thermal_realize; > + dc->vmsd = &bcm2838_thermal_vmstate; You can add a comment here: /* This device has no state: no need for vmstate or reset */ to tell the reader you haven't forgotten it. > +} > + > +static const TypeInfo bcm2838_thermal_info = { > + .name = TYPE_BCM2838_THERMAL, > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(Bcm2838ThermalState), > + .class_init = bcm2838_thermal_class_init, > +}; > + > +static void bcm2838_thermal_register_types(void) > +{ > + type_register_static(&bcm2838_thermal_info); > +} > + thanks -- PMM