Hi Cédric, Thanks for the feedback.
I will update the code accordingly in the next version of the patch series. Best Regards, Kane > -----Original Message----- > From: Cédric Le Goater <c...@kaod.org> > Sent: Tuesday, July 22, 2025 5:55 PM > To: Kane Chen <kane_c...@aspeedtech.com>; Peter Maydell > <peter.mayd...@linaro.org>; Steven Lee <steven_...@aspeedtech.com>; Troy > Lee <leet...@gmail.com>; Jamin Lin <jamin_...@aspeedtech.com>; Andrew > Jeffery <and...@codeconstruct.com.au>; Joel Stanley <j...@jms.id.au>; > open list:ASPEED BMCs <qemu-...@nongnu.org>; open list:All patches CC > here <qemu-devel@nongnu.org> > Cc: Troy Lee <troy_...@aspeedtech.com> > Subject: Re: [SPAM] [PATCH v4 1/5] hw/misc/aspeed_otp: Add ASPEED OTP > memory device model > > On 7/8/25 07:57, Kane Chen wrote: > > From: Kane-Chen-AS <kane_c...@aspeedtech.com> > > > > Introduce a QEMU device model for ASPEED's One-Time Programmable > (OTP) > > memory. > > > > This model simulates a word-addressable OTP region used for secure > > fuse storage. The OTP memory can operate with an internal memory > > buffer. > > > > The OTP model provides a memory-like interface through a dedicated > > AddressSpace, allowing other device models (e.g., SBC) to issue > > transactions as if accessing a memory-mapped region. > > > > Signed-off-by: Kane-Chen-AS <kane_c...@aspeedtech.com> > > --- > > include/hw/nvram/aspeed_otp.h | 33 ++++++++++++ > > hw/nvram/aspeed_otp.c | 99 > +++++++++++++++++++++++++++++++++++ > > hw/nvram/meson.build | 4 ++ > > 3 files changed, 136 insertions(+) > > create mode 100644 include/hw/nvram/aspeed_otp.h > > create mode 100644 hw/nvram/aspeed_otp.c > > > > diff --git a/include/hw/nvram/aspeed_otp.h > > b/include/hw/nvram/aspeed_otp.h new file mode 100644 index > > 0000000000..3752353860 > > --- /dev/null > > +++ b/include/hw/nvram/aspeed_otp.h > > @@ -0,0 +1,33 @@ > > +/* > > + * ASPEED OTP (One-Time Programmable) memory > > + * > > + * Copyright (C) 2025 Aspeed > > + * > > + * SPDX-License-Identifier: GPL-2.0-or-later */ > > + > > +#ifndef ASPEED_OTP_H > > +#define ASPEED_OTP_H > > + > > +#include "system/memory.h" > > +#include "hw/block/block.h" > > +#include "system/address-spaces.h" > > + > > +#define TYPE_ASPEED_OTP "aspeed-otp" > > +OBJECT_DECLARE_SIMPLE_TYPE(AspeedOTPState, ASPEED_OTP) > > + > > +typedef struct AspeedOTPState { > > + DeviceState parent_obj; > > + > > + BlockBackend *blk; > > + > > + uint64_t size; > > + > > + AddressSpace as; > > + > > + MemoryRegion mmio; > > + > > + uint8_t *storage; > > +} AspeedOTPState; > > + > > +#endif /* ASPEED_OTP_H */ > > diff --git a/hw/nvram/aspeed_otp.c b/hw/nvram/aspeed_otp.c new file > > mode 100644 index 0000000000..e41481d9bb > > --- /dev/null > > +++ b/hw/nvram/aspeed_otp.c > > @@ -0,0 +1,99 @@ > > +/* > > + * ASPEED OTP (One-Time Programmable) memory > > + * > > + * Copyright (C) 2025 Aspeed > > + * > > + * SPDX-License-Identifier: GPL-2.0-or-later */ > > + > > +#include "qemu/osdep.h" > > +#include "qemu/log.h" > > +#include "qapi/error.h" > > +#include "system/block-backend-io.h" > > Please replace with : > > #include "system/block-backend.h" > > > Thanks, > > C. > > > > > +#include "hw/qdev-properties.h" > > +#include "hw/nvram/aspeed_otp.h" > > + > > +static uint64_t aspeed_otp_read(void *opaque, hwaddr offset, unsigned > > +size) { > > + AspeedOTPState *s = opaque; > > + uint64_t val = 0; > > + > > + memcpy(&val, s->storage + offset, size); > > + > > + return val; > > +} > > + > > +static void aspeed_otp_write(void *opaque, hwaddr offset, > > + uint64_t val, unsigned size) { > > + AspeedOTPState *s = opaque; > > + > > + memcpy(s->storage + offset, &val, size); } > > + > > +static bool aspeed_otp_init_storage(AspeedOTPState *s, Error **errp) > > +{ > > + uint32_t *p; > > + int i, num; > > + > > + num = s->size / sizeof(uint32_t); > > + p = (uint32_t *)s->storage; > > + for (i = 0; i < num; i++) { > > + p[i] = (i % 2 == 0) ? 0x00000000 : 0xFFFFFFFF; > > + } > > + > > + return true; > > +} > > + > > +static const MemoryRegionOps aspeed_otp_ops = { > > + .read = aspeed_otp_read, > > + .write = aspeed_otp_write, > > + .endianness = DEVICE_LITTLE_ENDIAN, > > + .valid.min_access_size = 1, > > + .valid.max_access_size = 4, > > +}; > > + > > +static void aspeed_otp_realize(DeviceState *dev, Error **errp) { > > + AspeedOTPState *s = ASPEED_OTP(dev); > > + > > + if (s->size == 0) { > > + error_setg(errp, "aspeed.otp: 'size' property must be set"); > > + return; > > + } > > + > > + s->storage = blk_blockalign(s->blk, s->size); > > + > > + if (!aspeed_otp_init_storage(s, errp)) { > > + return; > > + } > > + > > + memory_region_init_io(&s->mmio, OBJECT(dev), &aspeed_otp_ops, > > + s, "aspeed.otp", s->size); > > + address_space_init(&s->as, &s->mmio, NULL); } > > + > > +static const Property aspeed_otp_properties[] = { > > + DEFINE_PROP_UINT64("size", AspeedOTPState, size, 0), }; > > + > > +static void aspeed_otp_class_init(ObjectClass *klass, const void > > +*data) { > > + DeviceClass *dc = DEVICE_CLASS(klass); > > + dc->realize = aspeed_otp_realize; > > + device_class_set_props(dc, aspeed_otp_properties); } > > + > > +static const TypeInfo aspeed_otp_info = { > > + .name = TYPE_ASPEED_OTP, > > + .parent = TYPE_DEVICE, > > + .instance_size = sizeof(AspeedOTPState), > > + .class_init = aspeed_otp_class_init, > > +}; > > + > > +static void aspeed_otp_register_types(void) { > > + type_register_static(&aspeed_otp_info); > > +} > > + > > +type_init(aspeed_otp_register_types) > > diff --git a/hw/nvram/meson.build b/hw/nvram/meson.build index > > 10f3639db6..b66f23605b 100644 > > --- a/hw/nvram/meson.build > > +++ b/hw/nvram/meson.build > > @@ -19,3 +19,7 @@ system_ss.add(when: 'CONFIG_XLNX_BBRAM', > if_true: > > files('xlnx-bbram.c')) > > > > specific_ss.add(when: 'CONFIG_PSERIES', if_true: files('spapr_nvram.c')) > > specific_ss.add(when: 'CONFIG_ACPI', if_true: > > files('fw_cfg-acpi.c')) > > + > > +system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files( > > + 'aspeed_otp.c', > > + )) > > \ No newline at end of file