* [RFC PATCH 0/1] hw: aspeed_adc: Add initial Aspeed ADC support
@ 2021-09-30 0:42 pdel
2021-09-30 0:42 ` [RFC PATCH 1/1] " pdel
0 siblings, 1 reply; 5+ messages in thread
From: pdel @ 2021-09-30 0:42 UTC (permalink / raw)
Cc: clg, patrick, amithash, zhdaniel, qemu-devel, Peter Delevoryas
From: Peter Delevoryas <pdel@fb.com>
Hey everyone,
This patch mostly just does the basic, boilerplate setup for the ADC, so
that we can start adding more ADC feature emulation in the future.
The only device behavior that I tried to add was emulating the control
initialization sequence and the sequence for enabling "Auto compensating
sensing mode". I didn't even use something like a timer to delay the
response for each register write by a few cycles, it just immediately
updates the register.
I was looking at the Nuvoton ADC model, "hw/adc/npcm7xx_adc.c", and I
noticed that it has "enter_reset" and "hold_reset" methods, should I
implement the initialization sequence as part of the "Resettable"
interface? (It seems like I probably should, I'm just not really sure).
I could also add a timer to emulate the time it takes to initialize the
ADC, I see the Nuvoton module also has a constant
"NPCM7XX_ADC_RESET_CYCLES" but it's not used anywhere, but I imagine I
could do something similar to the conversion timer in that module? I
think the upstream drivers poll every 0.5 ms and timeout after 500 ms,
so I could use that as a guide.
This patch, even without any conversion emulation, is useful to me
because some OpenBMC platforms use Aspeed SDK ADC drivers, and they
don't have error handling if the ADC doesn't respond, so they can't boot
in QEMU without some basic emulation like this.
Thanks,
Peter
Peter Delevoryas (1):
hw: aspeed_adc: Add initial Aspeed ADC support
hw/adc/aspeed_adc.c | 205 ++++++++++++++++++++++++++++++++++++
hw/adc/meson.build | 1 +
hw/adc/trace-events | 4 +
hw/arm/aspeed_ast2600.c | 18 ++++
hw/arm/aspeed_soc.c | 17 +++
include/hw/adc/aspeed_adc.h | 48 +++++++++
include/hw/arm/aspeed_soc.h | 5 +
7 files changed, 298 insertions(+)
create mode 100644 hw/adc/aspeed_adc.c
create mode 100644 include/hw/adc/aspeed_adc.h
--
2.30.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [RFC PATCH 1/1] hw: aspeed_adc: Add initial Aspeed ADC support
2021-09-30 0:42 [RFC PATCH 0/1] hw: aspeed_adc: Add initial Aspeed ADC support pdel
@ 2021-09-30 0:42 ` pdel
2021-09-30 6:22 ` Cédric Le Goater
0 siblings, 1 reply; 5+ messages in thread
From: pdel @ 2021-09-30 0:42 UTC (permalink / raw)
Cc: clg, patrick, amithash, zhdaniel, qemu-devel, Peter Delevoryas
From: Peter Delevoryas <pdel@fb.com>
This change sets up Aspeed SoC ADC emulation, so that most ADC drivers
will pass the initialization sequence and load successfully. In the
future, we can extend this to emulate more features.
The initialization sequence is:
1. Set `ADC00` to `0xF`.
2. Wait for bit 8 of `ADC00` to be set.
I also added the sequence for enabling "Auto compensating sensing mode":
1. Set `ADC00` to `0x2F` (set bit 5).
2. Wait for bit 5 of `ADC00` to be reset (to zero).
3. ...
4. ...
Fuji (AST2600):
Before:
[ 56.185778] aspeed_adc: probe of 1e6e9000.adc failed with error -110
[ 56.687936] aspeed_adc: probe of 1e6e9100.adc failed with error -110
After:
aspeed_adc_read 0x0c read 0x0000
aspeed_adc_read 0x0c read 0x0000
aspeed_adc_write 0x00 write 0x000f
aspeed_adc_read 0x00 read 0x010f
aspeed_adc_read 0x00 read 0x010f
[ 55.885164] aspeed_adc 1e6e9000.adc: trim 8
aspeed_adc_read 0xc4 read 0x0000
aspeed_adc_write 0xc4 write 0x0008
aspeed_adc_write 0x00 write 0x011f
aspeed_adc_write 0x00 write 0x1011f
aspeed_adc_read 0x10 read 0x0000
aspeed_adc_write 0x00 write 0x010f
[ 55.886509] aspeed_adc 1e6e9000.adc: cv 512
aspeed_adc_write 0x00 write 0xffff010f
aspeed_adc_read 0x0c read 0x0000
aspeed_adc_read 0x0c read 0x0000
aspeed_adc_write 0x00 write 0x000f
aspeed_adc_read 0x00 read 0x010f
aspeed_adc_read 0x00 read 0x010f
[ 55.890609] aspeed_adc 1e6e9100.adc: trim 8
aspeed_adc_read 0xc4 read 0x0000
aspeed_adc_write 0xc4 write 0x0008
aspeed_adc_write 0x00 write 0x011f
aspeed_adc_write 0x00 write 0x1011f
aspeed_adc_read 0x10 read 0x0000
aspeed_adc_write 0x00 write 0x010f
[ 55.891863] aspeed_adc 1e6e9100.adc: cv 512
aspeed_adc_write 0x00 write 0xffff010f
YosemiteV2 (AST2500):
Before:
[ 20.561588] ast_adc ast_adc.0: ast_adc_probe
[ 20.563741] hwmon hwmon0: write offset: c4, val: 8
[ 20.563925] hwmon hwmon0: write offset: c, val: 40
[ 20.564099] hwmon hwmon0: write offset: 0, val: f
[ 21.066110] ast_adc: driver init failed (ret=-110)!
[ 21.066635] ast_adc: probe of ast_adc.0 failed with error -110
After:
aspeed_adc_write 0xc4 write 0x0008
aspeed_adc_write 0x0c write 0x0040
aspeed_adc_write 0x00 write 0x000f
aspeed_adc_read 0x00 read 0x010f
aspeed_adc_write 0x00 write 0x002f
aspeed_adc_read 0x00 read 0x000f
aspeed_adc_read 0xc4 read 0x0008
[ 19.602033] ast_adc: driver successfully loaded.
Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
hw/adc/aspeed_adc.c | 205 ++++++++++++++++++++++++++++++++++++
hw/adc/meson.build | 1 +
hw/adc/trace-events | 4 +
hw/arm/aspeed_ast2600.c | 18 ++++
hw/arm/aspeed_soc.c | 17 +++
include/hw/adc/aspeed_adc.h | 48 +++++++++
include/hw/arm/aspeed_soc.h | 5 +
7 files changed, 298 insertions(+)
create mode 100644 hw/adc/aspeed_adc.c
create mode 100644 include/hw/adc/aspeed_adc.h
diff --git a/hw/adc/aspeed_adc.c b/hw/adc/aspeed_adc.c
new file mode 100644
index 0000000000..590936148b
--- /dev/null
+++ b/hw/adc/aspeed_adc.c
@@ -0,0 +1,205 @@
+/*
+ * Aspeed ADC Controller
+ *
+ * Copyright 2021 Facebook, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/adc/aspeed_adc.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+#include "trace.h"
+#include "qemu/log.h"
+
+#define TO_REG(offset) ((offset) >> 2)
+#define ENGINE_CONTROL TO_REG(0x00)
+
+static uint64_t aspeed_adc_read(void *opaque, hwaddr offset, unsigned size)
+{
+ AspeedADCState *s = ASPEED_ADC(opaque);
+ int reg = TO_REG(offset);
+
+ if (reg >= ASPEED_ADC_MAX_REGS) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Out-of-bounds read 0x%04" HWADDR_PRIX "\n",
+ __func__, offset);
+ return 0;
+ }
+
+ int value = s->regs[reg];
+
+ trace_aspeed_adc_read(offset, value);
+ return value;
+}
+
+static void aspeed_adc_write(void *opaque, hwaddr offset, uint64_t data,
+ unsigned size)
+{
+ AspeedADCState *s = ASPEED_ADC(opaque);
+ int reg = TO_REG(offset);
+
+ if (reg >= ASPEED_ADC_MAX_REGS) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Out-of-bounds write 0x%04" HWADDR_PRIX "\n",
+ __func__, offset);
+ return;
+ }
+
+ trace_aspeed_adc_write(offset, data);
+
+ switch (reg) {
+ case ENGINE_CONTROL:
+ switch (data) {
+ case 0xF:
+ s->regs[ENGINE_CONTROL] = 0x10F;
+ return;
+ case 0x2F:
+ s->regs[ENGINE_CONTROL] = 0xF;
+ return;
+ }
+ break;
+ }
+
+ s->regs[reg] = data;
+}
+
+static const MemoryRegionOps aspeed_adc_ops = {
+ .read = aspeed_adc_read,
+ .write = aspeed_adc_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+ .valid.min_access_size = 4,
+ .valid.max_access_size = 4,
+ .valid.unaligned = false,
+};
+
+static void aspeed_adc_realize(DeviceState *dev, Error **errp)
+{
+ SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+ AspeedADCState *s = ASPEED_ADC(dev);
+
+ sysbus_init_irq(sbd, &s->irq);
+ // The memory region is actually 4KB (0x1000), but there's 2 ADC's in the
+ // AST2600 that are offset by 0x100.
+ memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_adc_ops, s,
+ TYPE_ASPEED_ADC, 0x100);
+ sysbus_init_mmio(sbd, &s->mmio);
+}
+
+static void aspeed_adc_reset(DeviceState *dev)
+{
+ AspeedADCState *s = ASPEED_ADC(dev);
+ AspeedADCClass *aac = ASPEED_ADC_GET_CLASS(dev);
+
+ memcpy(s->regs, aac->resets, aac->nr_regs << 2);
+}
+
+static const uint32_t aspeed_2400_resets[ASPEED_2400_ADC_NR_REGS] = {
+ [ENGINE_CONTROL] = 0x00000000,
+};
+
+static const uint32_t aspeed_2500_resets[ASPEED_2500_ADC_NR_REGS] = {
+ [ENGINE_CONTROL] = 0x00000000,
+};
+
+static const uint32_t aspeed_2600_resets[ASPEED_2600_ADC_NR_REGS] = {
+ [ENGINE_CONTROL] = 0x00000000,
+};
+
+static const VMStateDescription aspeed_adc_vmstate = {
+ .name = TYPE_ASPEED_ADC,
+ .version_id = 0,
+ .minimum_version_id = 0,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32_ARRAY(regs, AspeedADCState, ASPEED_ADC_MAX_REGS),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static void aspeed_adc_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ dc->realize = aspeed_adc_realize;
+ dc->reset = aspeed_adc_reset;
+ dc->desc = "Aspeed Analog-to-Digital Converter";
+ dc->vmsd = &aspeed_adc_vmstate;
+}
+
+static void aspeed_2400_adc_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ AspeedADCClass *aac = ASPEED_ADC_CLASS(klass);
+
+ dc->desc = "Aspeed 2400 Analog-to-Digital Converter";
+ aac->resets = aspeed_2400_resets;
+ aac->nr_regs = ASPEED_2400_ADC_NR_REGS;
+}
+
+static void aspeed_2500_adc_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ AspeedADCClass *aac = ASPEED_ADC_CLASS(klass);
+
+ dc->desc = "Aspeed 2500 Analog-to-Digital Converter";
+ aac->resets = aspeed_2500_resets;
+ aac->nr_regs = ASPEED_2500_ADC_NR_REGS;
+}
+
+static void aspeed_2600_adc_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ AspeedADCClass *aac = ASPEED_ADC_CLASS(klass);
+
+ dc->desc = "Aspeed 2600 Analog-to-Digital Converter";
+ aac->resets = aspeed_2600_resets;
+ aac->nr_regs = ASPEED_2600_ADC_NR_REGS;
+}
+
+static const TypeInfo aspeed_adc_info = {
+ .name = TYPE_ASPEED_ADC,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(AspeedADCState),
+ .class_init = aspeed_adc_class_init,
+ .class_size = sizeof(AspeedADCClass),
+ .abstract = true,
+};
+
+static const TypeInfo aspeed_2400_adc_info = {
+ .name = TYPE_ASPEED_2400_ADC,
+ .parent = TYPE_ASPEED_ADC,
+ .instance_size = sizeof(AspeedADCState),
+ .class_init = aspeed_2400_adc_class_init,
+};
+
+static const TypeInfo aspeed_2500_adc_info = {
+ .name = TYPE_ASPEED_2500_ADC,
+ .parent = TYPE_ASPEED_ADC,
+ .instance_size = sizeof(AspeedADCState),
+ .class_init = aspeed_2500_adc_class_init,
+};
+
+static const TypeInfo aspeed_2600_adc_info = {
+ .name = TYPE_ASPEED_2600_ADC,
+ .parent = TYPE_ASPEED_ADC,
+ .instance_size = sizeof(AspeedADCState),
+ .class_init = aspeed_2600_adc_class_init,
+};
+
+static void aspeed_adc_register_types(void)
+{
+ type_register_static(&aspeed_adc_info);
+ type_register_static(&aspeed_2400_adc_info);
+ type_register_static(&aspeed_2500_adc_info);
+ type_register_static(&aspeed_2600_adc_info);
+}
+
+type_init(aspeed_adc_register_types);
diff --git a/hw/adc/meson.build b/hw/adc/meson.build
index ac4f093fea..65e1dd73c1 100644
--- a/hw/adc/meson.build
+++ b/hw/adc/meson.build
@@ -2,3 +2,4 @@ softmmu_ss.add(when: 'CONFIG_STM32F2XX_ADC', if_true: files('stm32f2xx_adc.c'))
softmmu_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_adc.c'))
softmmu_ss.add(when: 'CONFIG_ZYNQ', if_true: files('zynq-xadc.c'))
softmmu_ss.add(when: 'CONFIG_MAX111X', if_true: files('max111x.c'))
+softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_adc.c'))
diff --git a/hw/adc/trace-events b/hw/adc/trace-events
index 456f21c8f4..c23d7bb6d7 100644
--- a/hw/adc/trace-events
+++ b/hw/adc/trace-events
@@ -3,3 +3,7 @@
# npcm7xx_adc.c
npcm7xx_adc_read(const char *id, uint64_t offset, uint32_t value) " %s offset: 0x%04" PRIx64 " value 0x%04" PRIx32
npcm7xx_adc_write(const char *id, uint64_t offset, uint32_t value) "%s offset: 0x%04" PRIx64 " value 0x%04" PRIx32
+
+# aspeed_adc.c
+aspeed_adc_read(uint8_t offset, uint32_t value) "0x%02" PRIx8 " read 0x%04" PRIx32
+aspeed_adc_write(uint8_t offset, uint32_t value) "0x%02" PRIx8 " write 0x%04" PRIx32
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 9d70e8e060..a582e882f2 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -44,6 +44,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
[ASPEED_DEV_SCU] = 0x1E6E2000,
[ASPEED_DEV_XDMA] = 0x1E6E7000,
[ASPEED_DEV_ADC] = 0x1E6E9000,
+ [ASPEED_DEV_ADC2] = 0x1E6E9100,
[ASPEED_DEV_VIDEO] = 0x1E700000,
[ASPEED_DEV_SDHCI] = 0x1E740000,
[ASPEED_DEV_EMMC] = 0x1E750000,
@@ -77,6 +78,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
[ASPEED_DEV_SDMC] = 0,
[ASPEED_DEV_SCU] = 12,
[ASPEED_DEV_ADC] = 78,
+ [ASPEED_DEV_ADC2] = 78,
[ASPEED_DEV_XDMA] = 6,
[ASPEED_DEV_SDHCI] = 43,
[ASPEED_DEV_EHCI1] = 5,
@@ -216,6 +218,11 @@ static void aspeed_soc_ast2600_init(Object *obj)
snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
object_initialize_child(obj, "hace", &s->hace, typename);
+
+ snprintf(typename, sizeof(typename), "aspeed.adc-%s", socname);
+ for (i = 0; i < sc->adcs_num; i++) {
+ object_initialize_child(obj, "adc[*]", &s->adc[i], typename);
+ }
}
/*
@@ -507,6 +514,16 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
sysbus_mmio_map(SYS_BUS_DEVICE(&s->hace), 0, sc->memmap[ASPEED_DEV_HACE]);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->hace), 0,
aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
+
+ /* ADC */
+ for (int i = 0; i < sc->adcs_num; i++) {
+ SysBusDevice *bus = SYS_BUS_DEVICE(&s->adc[i]);
+ if (!sysbus_realize(bus, errp)) {
+ return;
+ }
+ sysbus_mmio_map(bus, 0, sc->memmap[ASPEED_DEV_ADC + i]);
+ sysbus_connect_irq(bus, 0, aspeed_soc_get_irq(s, ASPEED_DEV_ADC + i));
+ }
}
static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
@@ -524,6 +541,7 @@ static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
sc->ehcis_num = 2;
sc->wdts_num = 4;
sc->macs_num = 4;
+ sc->adcs_num = 2;
sc->irqmap = aspeed_soc_ast2600_irqmap;
sc->memmap = aspeed_soc_ast2600_memmap;
sc->num_cpus = 2;
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index ed84502e23..412c557e40 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -216,6 +216,11 @@ static void aspeed_soc_init(Object *obj)
snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
object_initialize_child(obj, "hace", &s->hace, typename);
+
+ snprintf(typename, sizeof(typename), "aspeed.adc-%s", socname);
+ for (i = 0; i < sc->adcs_num; i++) {
+ object_initialize_child(obj, "adc[*]", &s->adc[i], typename);
+ }
}
static void aspeed_soc_realize(DeviceState *dev, Error **errp)
@@ -435,6 +440,16 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
sysbus_mmio_map(SYS_BUS_DEVICE(&s->hace), 0, sc->memmap[ASPEED_DEV_HACE]);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->hace), 0,
aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
+
+ /* ADC */
+ for (int i = 0; i < sc->adcs_num; i++) {
+ SysBusDevice *bus = SYS_BUS_DEVICE(&s->adc[i]);
+ if (!sysbus_realize(bus, errp)) {
+ return;
+ }
+ sysbus_mmio_map(bus, 0, sc->memmap[ASPEED_DEV_ADC + i]);
+ sysbus_connect_irq(bus, 0, aspeed_soc_get_irq(s, ASPEED_DEV_ADC + i));
+ }
}
static Property aspeed_soc_properties[] = {
DEFINE_PROP_LINK("dram", AspeedSoCState, dram_mr, TYPE_MEMORY_REGION,
@@ -475,6 +490,7 @@ static void aspeed_soc_ast2400_class_init(ObjectClass *oc, void *data)
sc->ehcis_num = 1;
sc->wdts_num = 2;
sc->macs_num = 2;
+ sc->adcs_num = 1;
sc->irqmap = aspeed_soc_ast2400_irqmap;
sc->memmap = aspeed_soc_ast2400_memmap;
sc->num_cpus = 1;
@@ -500,6 +516,7 @@ static void aspeed_soc_ast2500_class_init(ObjectClass *oc, void *data)
sc->ehcis_num = 2;
sc->wdts_num = 3;
sc->macs_num = 2;
+ sc->adcs_num = 1;
sc->irqmap = aspeed_soc_ast2500_irqmap;
sc->memmap = aspeed_soc_ast2500_memmap;
sc->num_cpus = 1;
diff --git a/include/hw/adc/aspeed_adc.h b/include/hw/adc/aspeed_adc.h
new file mode 100644
index 0000000000..5528781be0
--- /dev/null
+++ b/include/hw/adc/aspeed_adc.h
@@ -0,0 +1,48 @@
+/*
+ * Aspeed ADC Controller
+ *
+ * Copyright 2021 Facebook, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#ifndef ASPEED_ADC_H
+#define ASPEED_ADC_H
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "qom/object.h"
+
+#define TYPE_ASPEED_ADC "aspeed.adc"
+OBJECT_DECLARE_TYPE(AspeedADCState, AspeedADCClass, ASPEED_ADC)
+#define TYPE_ASPEED_2400_ADC TYPE_ASPEED_ADC "-ast2400"
+#define TYPE_ASPEED_2500_ADC TYPE_ASPEED_ADC "-ast2500"
+#define TYPE_ASPEED_2600_ADC TYPE_ASPEED_ADC "-ast2600"
+
+#define ASPEED_2400_ADC_NR_REGS (0xC4 >> 2)
+#define ASPEED_2500_ADC_NR_REGS (0xC8 >> 2)
+#define ASPEED_2600_ADC_NR_REGS (0xD0 >> 2)
+#define ASPEED_ADC_MAX_REGS ASPEED_2600_ADC_NR_REGS
+
+struct AspeedADCState {
+ SysBusDevice parent;
+ MemoryRegion mmio;
+ qemu_irq irq;
+ uint32_t regs[ASPEED_ADC_MAX_REGS];
+};
+
+struct AspeedADCClass {
+ SysBusDeviceClass parent;
+ const uint32_t *resets;
+ uint32_t nr_regs;
+};
+
+#endif
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 87d76c9259..4503f08870 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -30,12 +30,14 @@
#include "hw/usb/hcd-ehci.h"
#include "qom/object.h"
#include "hw/misc/aspeed_lpc.h"
+#include "hw/adc/aspeed_adc.h"
#define ASPEED_SPIS_NUM 2
#define ASPEED_EHCIS_NUM 2
#define ASPEED_WDTS_NUM 4
#define ASPEED_CPUS_NUM 2
#define ASPEED_MACS_NUM 4
+#define ASPEED_ADCS_NUM 2
struct AspeedSoCState {
/*< private >*/
@@ -65,6 +67,7 @@ struct AspeedSoCState {
AspeedSDHCIState sdhci;
AspeedSDHCIState emmc;
AspeedLPCState lpc;
+ AspeedADCState adc[ASPEED_ADCS_NUM];
uint32_t uart_default;
};
@@ -82,6 +85,7 @@ struct AspeedSoCClass {
int ehcis_num;
int wdts_num;
int macs_num;
+ int adcs_num;
const int *irqmap;
const hwaddr *memmap;
uint32_t num_cpus;
@@ -105,6 +109,7 @@ enum {
ASPEED_DEV_SDMC,
ASPEED_DEV_SCU,
ASPEED_DEV_ADC,
+ ASPEED_DEV_ADC2,
ASPEED_DEV_VIDEO,
ASPEED_DEV_SRAM,
ASPEED_DEV_SDHCI,
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC PATCH 1/1] hw: aspeed_adc: Add initial Aspeed ADC support
2021-09-30 0:42 ` [RFC PATCH 1/1] " pdel
@ 2021-09-30 6:22 ` Cédric Le Goater
2021-09-30 15:24 ` Peter Delevoryas
0 siblings, 1 reply; 5+ messages in thread
From: Cédric Le Goater @ 2021-09-30 6:22 UTC (permalink / raw)
To: pdel
Cc: Peter Maydell, Andrew Jeffery, amithash, qemu-devel, patrick,
qemu-arm, Joel Stanley, zhdaniel, Philippe Mathieu-Daudé
Hello Peter,
If you run ./scripts/get_maintainer.pl on the patch, it will build
the list of persons and mailing list to send to.
On 9/30/21 02:42, pdel@fb.com wrote:
> From: Peter Delevoryas <pdel@fb.com>
>
> This change sets up Aspeed SoC ADC emulation, so that most ADC drivers
> will pass the initialization sequence and load successfully. In the
> future, we can extend this to emulate more features.
>
> The initialization sequence is:
>
> 1. Set `ADC00` to `0xF`.
> 2. Wait for bit 8 of `ADC00` to be set.
>
> I also added the sequence for enabling "Auto compensating sensing mode":
>
> 1. Set `ADC00` to `0x2F` (set bit 5).
> 2. Wait for bit 5 of `ADC00` to be reset (to zero).
> 3. ...
> 4. ...
>
> Fuji (AST2600):
> Before:
> [ 56.185778] aspeed_adc: probe of 1e6e9000.adc failed with error -110
> [ 56.687936] aspeed_adc: probe of 1e6e9100.adc failed with error -110
>
> After:
> aspeed_adc_read 0x0c read 0x0000
> aspeed_adc_read 0x0c read 0x0000
> aspeed_adc_write 0x00 write 0x000f
> aspeed_adc_read 0x00 read 0x010f
> aspeed_adc_read 0x00 read 0x010f
> [ 55.885164] aspeed_adc 1e6e9000.adc: trim 8
> aspeed_adc_read 0xc4 read 0x0000
> aspeed_adc_write 0xc4 write 0x0008
> aspeed_adc_write 0x00 write 0x011f
> aspeed_adc_write 0x00 write 0x1011f
> aspeed_adc_read 0x10 read 0x0000
> aspeed_adc_write 0x00 write 0x010f
> [ 55.886509] aspeed_adc 1e6e9000.adc: cv 512
> aspeed_adc_write 0x00 write 0xffff010f
> aspeed_adc_read 0x0c read 0x0000
> aspeed_adc_read 0x0c read 0x0000
> aspeed_adc_write 0x00 write 0x000f
> aspeed_adc_read 0x00 read 0x010f
> aspeed_adc_read 0x00 read 0x010f
> [ 55.890609] aspeed_adc 1e6e9100.adc: trim 8
> aspeed_adc_read 0xc4 read 0x0000
> aspeed_adc_write 0xc4 write 0x0008
> aspeed_adc_write 0x00 write 0x011f
> aspeed_adc_write 0x00 write 0x1011f
> aspeed_adc_read 0x10 read 0x0000
> aspeed_adc_write 0x00 write 0x010f
> [ 55.891863] aspeed_adc 1e6e9100.adc: cv 512
> aspeed_adc_write 0x00 write 0xffff010f
>
> YosemiteV2 (AST2500):
> Before:
> [ 20.561588] ast_adc ast_adc.0: ast_adc_probe
> [ 20.563741] hwmon hwmon0: write offset: c4, val: 8
> [ 20.563925] hwmon hwmon0: write offset: c, val: 40
> [ 20.564099] hwmon hwmon0: write offset: 0, val: f
> [ 21.066110] ast_adc: driver init failed (ret=-110)!
> [ 21.066635] ast_adc: probe of ast_adc.0 failed with error -110
>
> After:
> aspeed_adc_write 0xc4 write 0x0008
> aspeed_adc_write 0x0c write 0x0040
> aspeed_adc_write 0x00 write 0x000f
> aspeed_adc_read 0x00 read 0x010f
> aspeed_adc_write 0x00 write 0x002f
> aspeed_adc_read 0x00 read 0x000f
> aspeed_adc_read 0xc4 read 0x0008
> [ 19.602033] ast_adc: driver successfully loaded.
FYI, these series was sent by Andrew in 2017 and I have been keeping
it alive since in the aspeed-x.y branches :
* memory: Support unaligned accesses on aligned-only models
https://github.com/legoater/qemu/commit/1960ba6bde27b91edb5336985a9210260a4c8938
That was requested by Phil I think.
* hw/adc: Add basic Aspeed ADC model
https://github.com/legoater/qemu/commit/1eff7b1cf10d1777635f7d2cef8ecb441cc607c4
This is the initial patch. I added multi-engine support recently
for the fuji.
* hw/arm: Integrate ADC model into Aspeed SoC
https://github.com/legoater/qemu/commit/3052f9d8ccdaf78b753e53574b7e8cc2ee01429f
That one is trivial.
Overall comments :
I prefer the 'regs' array approach of your proposal.
I think the AspeedADCEngine should appear as a QOM object. Check
the patches above.
To move on, maybe, you could rework the initial series and take
ownership ?
Some more below,
> Signed-off-by: Peter Delevoryas <pdel@fb.com>
> ---
> hw/adc/aspeed_adc.c | 205 ++++++++++++++++++++++++++++++++++++
> hw/adc/meson.build | 1 +
> hw/adc/trace-events | 4 +
> hw/arm/aspeed_ast2600.c | 18 ++++
> hw/arm/aspeed_soc.c | 17 +++
> include/hw/adc/aspeed_adc.h | 48 +++++++++
> include/hw/arm/aspeed_soc.h | 5 +
> 7 files changed, 298 insertions(+)
> create mode 100644 hw/adc/aspeed_adc.c
> create mode 100644 include/hw/adc/aspeed_adc.h
>
> diff --git a/hw/adc/aspeed_adc.c b/hw/adc/aspeed_adc.c
> new file mode 100644
> index 0000000000..590936148b
> --- /dev/null
> +++ b/hw/adc/aspeed_adc.c
> @@ -0,0 +1,205 @@
> +/*
> + * Aspeed ADC Controller
> + *
> + * Copyright 2021 Facebook, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> + * for more details.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/adc/aspeed_adc.h"
> +#include "hw/qdev-properties.h"
> +#include "migration/vmstate.h"
> +#include "trace.h"
> +#include "qemu/log.h"
> +
> +#define TO_REG(offset) ((offset) >> 2)
> +#define ENGINE_CONTROL TO_REG(0x00)
> +
> +static uint64_t aspeed_adc_read(void *opaque, hwaddr offset, unsigned size)
> +{
> + AspeedADCState *s = ASPEED_ADC(opaque);
> + int reg = TO_REG(offset);
> +
> + if (reg >= ASPEED_ADC_MAX_REGS) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Out-of-bounds read 0x%04" HWADDR_PRIX "\n",
> + __func__, offset);
> + return 0;
> + }
> +
> + int value = s->regs[reg];
> +
> + trace_aspeed_adc_read(offset, value);
> + return value;
> +}
> +
> +static void aspeed_adc_write(void *opaque, hwaddr offset, uint64_t data,
> + unsigned size)
> +{
> + AspeedADCState *s = ASPEED_ADC(opaque);
> + int reg = TO_REG(offset);
> +
> + if (reg >= ASPEED_ADC_MAX_REGS) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Out-of-bounds write 0x%04" HWADDR_PRIX "\n",
> + __func__, offset);
> + return;
> + }
> +
> + trace_aspeed_adc_write(offset, data);
> +
> + switch (reg) {
> + case ENGINE_CONTROL:
> + switch (data) {
> + case 0xF:
> + s->regs[ENGINE_CONTROL] = 0x10F;
> + return;
> + case 0x2F:
> + s->regs[ENGINE_CONTROL] = 0xF;
> + return;
> + }
> + break;
> + }
> +
> + s->regs[reg] = data;
> +}
> +
> +static const MemoryRegionOps aspeed_adc_ops = {
> + .read = aspeed_adc_read,
> + .write = aspeed_adc_write,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> + .valid.min_access_size = 4,
> + .valid.max_access_size = 4,
> + .valid.unaligned = false,
That's THE question that has been blocking the patches from being
merged since 2017.
> +};
> +
> +static void aspeed_adc_realize(DeviceState *dev, Error **errp)
> +{
> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> + AspeedADCState *s = ASPEED_ADC(dev);
> +
> + sysbus_init_irq(sbd, &s->irq);
> + // The memory region is actually 4KB (0x1000), but there's 2 ADC's in the
> + // AST2600 that are offset by 0x100.
No C++ comments. Please run ./scripts/checkpatch.pl.
> + memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_adc_ops, s,
> + TYPE_ASPEED_ADC, 0x100);
> + sysbus_init_mmio(sbd, &s->mmio);
> +}
> +
> +static void aspeed_adc_reset(DeviceState *dev)
> +{
> + AspeedADCState *s = ASPEED_ADC(dev);
> + AspeedADCClass *aac = ASPEED_ADC_GET_CLASS(dev);
> +
> + memcpy(s->regs, aac->resets, aac->nr_regs << 2);
> +}
> +
> +static const uint32_t aspeed_2400_resets[ASPEED_2400_ADC_NR_REGS] = {
> + [ENGINE_CONTROL] = 0x00000000,
> +};
> +
> +static const uint32_t aspeed_2500_resets[ASPEED_2500_ADC_NR_REGS] = {
> + [ENGINE_CONTROL] = 0x00000000,
> +};
> +
> +static const uint32_t aspeed_2600_resets[ASPEED_2600_ADC_NR_REGS] = {
> + [ENGINE_CONTROL] = 0x00000000,
> +};
> +
> +static const VMStateDescription aspeed_adc_vmstate = {
> + .name = TYPE_ASPEED_ADC,
> + .version_id = 0,
> + .minimum_version_id = 0,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32_ARRAY(regs, AspeedADCState, ASPEED_ADC_MAX_REGS),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void aspeed_adc_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + dc->realize = aspeed_adc_realize;
> + dc->reset = aspeed_adc_reset;
> + dc->desc = "Aspeed Analog-to-Digital Converter";
> + dc->vmsd = &aspeed_adc_vmstate;
> +}
> +
> +static void aspeed_2400_adc_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + AspeedADCClass *aac = ASPEED_ADC_CLASS(klass);
> +
> + dc->desc = "Aspeed 2400 Analog-to-Digital Converter";
> + aac->resets = aspeed_2400_resets;
> + aac->nr_regs = ASPEED_2400_ADC_NR_REGS;
> +}
> +
> +static void aspeed_2500_adc_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + AspeedADCClass *aac = ASPEED_ADC_CLASS(klass);
> +
> + dc->desc = "Aspeed 2500 Analog-to-Digital Converter";
> + aac->resets = aspeed_2500_resets;
> + aac->nr_regs = ASPEED_2500_ADC_NR_REGS;
> +}
> +
> +static void aspeed_2600_adc_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + AspeedADCClass *aac = ASPEED_ADC_CLASS(klass);
> +
> + dc->desc = "Aspeed 2600 Analog-to-Digital Converter";
> + aac->resets = aspeed_2600_resets;
> + aac->nr_regs = ASPEED_2600_ADC_NR_REGS;
> +}
> +
> +static const TypeInfo aspeed_adc_info = {
> + .name = TYPE_ASPEED_ADC,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(AspeedADCState),
> + .class_init = aspeed_adc_class_init,
> + .class_size = sizeof(AspeedADCClass),
> + .abstract = true,
> +};
> +
> +static const TypeInfo aspeed_2400_adc_info = {
> + .name = TYPE_ASPEED_2400_ADC,
> + .parent = TYPE_ASPEED_ADC,
> + .instance_size = sizeof(AspeedADCState),
> + .class_init = aspeed_2400_adc_class_init,
> +};
> +
> +static const TypeInfo aspeed_2500_adc_info = {
> + .name = TYPE_ASPEED_2500_ADC,
> + .parent = TYPE_ASPEED_ADC,
> + .instance_size = sizeof(AspeedADCState),
> + .class_init = aspeed_2500_adc_class_init,
> +};
> +
> +static const TypeInfo aspeed_2600_adc_info = {
> + .name = TYPE_ASPEED_2600_ADC,
> + .parent = TYPE_ASPEED_ADC,
> + .instance_size = sizeof(AspeedADCState),
> + .class_init = aspeed_2600_adc_class_init,
> +};
> +
> +static void aspeed_adc_register_types(void)
> +{
> + type_register_static(&aspeed_adc_info);
> + type_register_static(&aspeed_2400_adc_info);
> + type_register_static(&aspeed_2500_adc_info);
> + type_register_static(&aspeed_2600_adc_info);
> +}
> +
> +type_init(aspeed_adc_register_types);
> diff --git a/hw/adc/meson.build b/hw/adc/meson.build
> index ac4f093fea..65e1dd73c1 100644
> --- a/hw/adc/meson.build
> +++ b/hw/adc/meson.build
> @@ -2,3 +2,4 @@ softmmu_ss.add(when: 'CONFIG_STM32F2XX_ADC', if_true: files('stm32f2xx_adc.c'))
> softmmu_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_adc.c'))
> softmmu_ss.add(when: 'CONFIG_ZYNQ', if_true: files('zynq-xadc.c'))
> softmmu_ss.add(when: 'CONFIG_MAX111X', if_true: files('max111x.c'))
> +softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_adc.c'))
> diff --git a/hw/adc/trace-events b/hw/adc/trace-events
> index 456f21c8f4..c23d7bb6d7 100644
> --- a/hw/adc/trace-events
> +++ b/hw/adc/trace-events
> @@ -3,3 +3,7 @@
> # npcm7xx_adc.c
> npcm7xx_adc_read(const char *id, uint64_t offset, uint32_t value) " %s offset: 0x%04" PRIx64 " value 0x%04" PRIx32
> npcm7xx_adc_write(const char *id, uint64_t offset, uint32_t value) "%s offset: 0x%04" PRIx64 " value 0x%04" PRIx32
> +
> +# aspeed_adc.c
> +aspeed_adc_read(uint8_t offset, uint32_t value) "0x%02" PRIx8 " read 0x%04" PRIx32
> +aspeed_adc_write(uint8_t offset, uint32_t value) "0x%02" PRIx8 " write 0x%04" PRIx32
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index 9d70e8e060..a582e882f2 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -44,6 +44,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
> [ASPEED_DEV_SCU] = 0x1E6E2000,
> [ASPEED_DEV_XDMA] = 0x1E6E7000,
> [ASPEED_DEV_ADC] = 0x1E6E9000,
> + [ASPEED_DEV_ADC2] = 0x1E6E9100,
Look at the AspeedADCEngine approach introduced in the initial
series. It simplifies the integration in the machine.
Thanks,
C.
> [ASPEED_DEV_VIDEO] = 0x1E700000,
> [ASPEED_DEV_SDHCI] = 0x1E740000,
> [ASPEED_DEV_EMMC] = 0x1E750000,
> @@ -77,6 +78,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
> [ASPEED_DEV_SDMC] = 0,
> [ASPEED_DEV_SCU] = 12,
> [ASPEED_DEV_ADC] = 78,
> + [ASPEED_DEV_ADC2] = 78,
> [ASPEED_DEV_XDMA] = 6,
> [ASPEED_DEV_SDHCI] = 43,
> [ASPEED_DEV_EHCI1] = 5,
> @@ -216,6 +218,11 @@ static void aspeed_soc_ast2600_init(Object *obj)
>
> snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
> object_initialize_child(obj, "hace", &s->hace, typename);
> +
> + snprintf(typename, sizeof(typename), "aspeed.adc-%s", socname);
> + for (i = 0; i < sc->adcs_num; i++) {
> + object_initialize_child(obj, "adc[*]", &s->adc[i], typename);
> + }
> }
>
> /*
> @@ -507,6 +514,16 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
> sysbus_mmio_map(SYS_BUS_DEVICE(&s->hace), 0, sc->memmap[ASPEED_DEV_HACE]);
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->hace), 0,
> aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
> +
> + /* ADC */
> + for (int i = 0; i < sc->adcs_num; i++) {
> + SysBusDevice *bus = SYS_BUS_DEVICE(&s->adc[i]);
> + if (!sysbus_realize(bus, errp)) {
> + return;
> + }
> + sysbus_mmio_map(bus, 0, sc->memmap[ASPEED_DEV_ADC + i]);
> + sysbus_connect_irq(bus, 0, aspeed_soc_get_irq(s, ASPEED_DEV_ADC + i));
> + }
> }
>
> static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
> @@ -524,6 +541,7 @@ static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
> sc->ehcis_num = 2;
> sc->wdts_num = 4;
> sc->macs_num = 4;
> + sc->adcs_num = 2;
> sc->irqmap = aspeed_soc_ast2600_irqmap;
> sc->memmap = aspeed_soc_ast2600_memmap;
> sc->num_cpus = 2;
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index ed84502e23..412c557e40 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -216,6 +216,11 @@ static void aspeed_soc_init(Object *obj)
>
> snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
> object_initialize_child(obj, "hace", &s->hace, typename);
> +
> + snprintf(typename, sizeof(typename), "aspeed.adc-%s", socname);
> + for (i = 0; i < sc->adcs_num; i++) {
> + object_initialize_child(obj, "adc[*]", &s->adc[i], typename);
> + }
> }
>
> static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> @@ -435,6 +440,16 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> sysbus_mmio_map(SYS_BUS_DEVICE(&s->hace), 0, sc->memmap[ASPEED_DEV_HACE]);
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->hace), 0,
> aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
> +
> + /* ADC */
> + for (int i = 0; i < sc->adcs_num; i++) {
> + SysBusDevice *bus = SYS_BUS_DEVICE(&s->adc[i]);
> + if (!sysbus_realize(bus, errp)) {
> + return;
> + }
> + sysbus_mmio_map(bus, 0, sc->memmap[ASPEED_DEV_ADC + i]);
> + sysbus_connect_irq(bus, 0, aspeed_soc_get_irq(s, ASPEED_DEV_ADC + i));
> + }
> }
> static Property aspeed_soc_properties[] = {
> DEFINE_PROP_LINK("dram", AspeedSoCState, dram_mr, TYPE_MEMORY_REGION,
> @@ -475,6 +490,7 @@ static void aspeed_soc_ast2400_class_init(ObjectClass *oc, void *data)
> sc->ehcis_num = 1;
> sc->wdts_num = 2;
> sc->macs_num = 2;
> + sc->adcs_num = 1;
> sc->irqmap = aspeed_soc_ast2400_irqmap;
> sc->memmap = aspeed_soc_ast2400_memmap;
> sc->num_cpus = 1;
> @@ -500,6 +516,7 @@ static void aspeed_soc_ast2500_class_init(ObjectClass *oc, void *data)
> sc->ehcis_num = 2;
> sc->wdts_num = 3;
> sc->macs_num = 2;
> + sc->adcs_num = 1;
> sc->irqmap = aspeed_soc_ast2500_irqmap;
> sc->memmap = aspeed_soc_ast2500_memmap;
> sc->num_cpus = 1;
> diff --git a/include/hw/adc/aspeed_adc.h b/include/hw/adc/aspeed_adc.h
> new file mode 100644
> index 0000000000..5528781be0
> --- /dev/null
> +++ b/include/hw/adc/aspeed_adc.h
> @@ -0,0 +1,48 @@
> +/*
> + * Aspeed ADC Controller
> + *
> + * Copyright 2021 Facebook, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> + * for more details.
> + */
> +
> +#ifndef ASPEED_ADC_H
> +#define ASPEED_ADC_H
> +
> +#include "qemu/osdep.h"
> +#include "hw/sysbus.h"
> +#include "qom/object.h"
> +
> +#define TYPE_ASPEED_ADC "aspeed.adc"
> +OBJECT_DECLARE_TYPE(AspeedADCState, AspeedADCClass, ASPEED_ADC)
> +#define TYPE_ASPEED_2400_ADC TYPE_ASPEED_ADC "-ast2400"
> +#define TYPE_ASPEED_2500_ADC TYPE_ASPEED_ADC "-ast2500"
> +#define TYPE_ASPEED_2600_ADC TYPE_ASPEED_ADC "-ast2600"
> +
> +#define ASPEED_2400_ADC_NR_REGS (0xC4 >> 2)
> +#define ASPEED_2500_ADC_NR_REGS (0xC8 >> 2)
> +#define ASPEED_2600_ADC_NR_REGS (0xD0 >> 2)
> +#define ASPEED_ADC_MAX_REGS ASPEED_2600_ADC_NR_REGS
> +
> +struct AspeedADCState {
> + SysBusDevice parent;
> + MemoryRegion mmio;
> + qemu_irq irq;
> + uint32_t regs[ASPEED_ADC_MAX_REGS];
> +};
> +
> +struct AspeedADCClass {
> + SysBusDeviceClass parent;
> + const uint32_t *resets;
> + uint32_t nr_regs;
> +};
> +
> +#endif
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 87d76c9259..4503f08870 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -30,12 +30,14 @@
> #include "hw/usb/hcd-ehci.h"
> #include "qom/object.h"
> #include "hw/misc/aspeed_lpc.h"
> +#include "hw/adc/aspeed_adc.h"
>
> #define ASPEED_SPIS_NUM 2
> #define ASPEED_EHCIS_NUM 2
> #define ASPEED_WDTS_NUM 4
> #define ASPEED_CPUS_NUM 2
> #define ASPEED_MACS_NUM 4
> +#define ASPEED_ADCS_NUM 2
>
> struct AspeedSoCState {
> /*< private >*/
> @@ -65,6 +67,7 @@ struct AspeedSoCState {
> AspeedSDHCIState sdhci;
> AspeedSDHCIState emmc;
> AspeedLPCState lpc;
> + AspeedADCState adc[ASPEED_ADCS_NUM];
> uint32_t uart_default;
> };
>
> @@ -82,6 +85,7 @@ struct AspeedSoCClass {
> int ehcis_num;
> int wdts_num;
> int macs_num;
> + int adcs_num;
> const int *irqmap;
> const hwaddr *memmap;
> uint32_t num_cpus;
> @@ -105,6 +109,7 @@ enum {
> ASPEED_DEV_SDMC,
> ASPEED_DEV_SCU,
> ASPEED_DEV_ADC,
> + ASPEED_DEV_ADC2,
> ASPEED_DEV_VIDEO,
> ASPEED_DEV_SRAM,
> ASPEED_DEV_SDHCI,
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH 1/1] hw: aspeed_adc: Add initial Aspeed ADC support
2021-09-30 6:22 ` Cédric Le Goater
@ 2021-09-30 15:24 ` Peter Delevoryas
2021-09-30 16:21 ` Cédric Le Goater
0 siblings, 1 reply; 5+ messages in thread
From: Peter Delevoryas @ 2021-09-30 15:24 UTC (permalink / raw)
Cc: patrick, Amithash Prasad, Dan Zhang, Cameron Esfahani via,
qemu-arm, Joel Stanley, Andrew Jeffery, Peter Maydell,
Philippe Mathieu-Daudé,
Cédric Le Goater
> On Sep 29, 2021, at 11:22 PM, Cédric Le Goater <clg@kaod.org> wrote:
>
> Hello Peter,
>
> If you run ./scripts/get_maintainer.pl on the patch, it will build
> the list of persons and mailing list to send to.
Oh, sorry about that, I’ll cc everyone properly when I resubmit this.
>
> On 9/30/21 02:42, pdel@fb.com wrote:
>> From: Peter Delevoryas <pdel@fb.com>
>> This change sets up Aspeed SoC ADC emulation, so that most ADC drivers
>> will pass the initialization sequence and load successfully. In the
>> future, we can extend this to emulate more features.
>> The initialization sequence is:
>> 1. Set `ADC00` to `0xF`.
>> 2. Wait for bit 8 of `ADC00` to be set.
>> I also added the sequence for enabling "Auto compensating sensing mode":
>> 1. Set `ADC00` to `0x2F` (set bit 5).
>> 2. Wait for bit 5 of `ADC00` to be reset (to zero).
>> 3. ...
>> 4. ...
>> Fuji (AST2600):
>> Before:
>> [ 56.185778] aspeed_adc: probe of 1e6e9000.adc failed with error -110
>> [ 56.687936] aspeed_adc: probe of 1e6e9100.adc failed with error -110
>> After:
>> aspeed_adc_read 0x0c read 0x0000
>> aspeed_adc_read 0x0c read 0x0000
>> aspeed_adc_write 0x00 write 0x000f
>> aspeed_adc_read 0x00 read 0x010f
>> aspeed_adc_read 0x00 read 0x010f
>> [ 55.885164] aspeed_adc 1e6e9000.adc: trim 8
>> aspeed_adc_read 0xc4 read 0x0000
>> aspeed_adc_write 0xc4 write 0x0008
>> aspeed_adc_write 0x00 write 0x011f
>> aspeed_adc_write 0x00 write 0x1011f
>> aspeed_adc_read 0x10 read 0x0000
>> aspeed_adc_write 0x00 write 0x010f
>> [ 55.886509] aspeed_adc 1e6e9000.adc: cv 512
>> aspeed_adc_write 0x00 write 0xffff010f
>> aspeed_adc_read 0x0c read 0x0000
>> aspeed_adc_read 0x0c read 0x0000
>> aspeed_adc_write 0x00 write 0x000f
>> aspeed_adc_read 0x00 read 0x010f
>> aspeed_adc_read 0x00 read 0x010f
>> [ 55.890609] aspeed_adc 1e6e9100.adc: trim 8
>> aspeed_adc_read 0xc4 read 0x0000
>> aspeed_adc_write 0xc4 write 0x0008
>> aspeed_adc_write 0x00 write 0x011f
>> aspeed_adc_write 0x00 write 0x1011f
>> aspeed_adc_read 0x10 read 0x0000
>> aspeed_adc_write 0x00 write 0x010f
>> [ 55.891863] aspeed_adc 1e6e9100.adc: cv 512
>> aspeed_adc_write 0x00 write 0xffff010f
>> YosemiteV2 (AST2500):
>> Before:
>> [ 20.561588] ast_adc ast_adc.0: ast_adc_probe
>> [ 20.563741] hwmon hwmon0: write offset: c4, val: 8
>> [ 20.563925] hwmon hwmon0: write offset: c, val: 40
>> [ 20.564099] hwmon hwmon0: write offset: 0, val: f
>> [ 21.066110] ast_adc: driver init failed (ret=-110)!
>> [ 21.066635] ast_adc: probe of ast_adc.0 failed with error -110
>> After:
>> aspeed_adc_write 0xc4 write 0x0008
>> aspeed_adc_write 0x0c write 0x0040
>> aspeed_adc_write 0x00 write 0x000f
>> aspeed_adc_read 0x00 read 0x010f
>> aspeed_adc_write 0x00 write 0x002f
>> aspeed_adc_read 0x00 read 0x000f
>> aspeed_adc_read 0xc4 read 0x0008
>> [ 19.602033] ast_adc: driver successfully loaded.
>
>
> FYI, these series was sent by Andrew in 2017 and I have been keeping
> it alive since in the aspeed-x.y branches :
>
> * memory: Support unaligned accesses on aligned-only models
> https://github.com/legoater/qemu/commit/1960ba6bde27b91edb5336985a9210260a4c8938
>
> That was requested by Phil I think.
>
> * hw/adc: Add basic Aspeed ADC model
> https://github.com/legoater/qemu/commit/1eff7b1cf10d1777635f7d2cef8ecb441cc607c4
>
> This is the initial patch. I added multi-engine support recently
> for the fuji.
>
> * hw/arm: Integrate ADC model into Aspeed SoC
> https://github.com/legoater/qemu/commit/3052f9d8ccdaf78b753e53574b7e8cc2ee01429f
>
> That one is trivial.
>
>
> Overall comments :
>
> I prefer the 'regs' array approach of your proposal.
Ok (I was just following patterns from aspeed_scu.c), I’ll keep that aspect.
>
> I think the AspeedADCEngine should appear as a QOM object. Check
> the patches above.
I see, I’ll make sure to test this.
>
> To move on, maybe, you could rework the initial series and take
> ownership ?
>
Yeah definitely! I’ll resubmit once I’ve reworked it. I don’t intend
to include the unaligned-access support though, at least not w/ the ADC
changes.
>
> Some more below,
>
>
>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
>> ---
>> hw/adc/aspeed_adc.c | 205 ++++++++++++++++++++++++++++++++++++
>> hw/adc/meson.build | 1 +
>> hw/adc/trace-events | 4 +
>> hw/arm/aspeed_ast2600.c | 18 ++++
>> hw/arm/aspeed_soc.c | 17 +++
>> include/hw/adc/aspeed_adc.h | 48 +++++++++
>> include/hw/arm/aspeed_soc.h | 5 +
>> 7 files changed, 298 insertions(+)
>> create mode 100644 hw/adc/aspeed_adc.c
>> create mode 100644 include/hw/adc/aspeed_adc.h
>> diff --git a/hw/adc/aspeed_adc.c b/hw/adc/aspeed_adc.c
>> new file mode 100644
>> index 0000000000..590936148b
>> --- /dev/null
>> +++ b/hw/adc/aspeed_adc.c
>> @@ -0,0 +1,205 @@
>> +/*
>> + * Aspeed ADC Controller
>> + *
>> + * Copyright 2021 Facebook, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License as published by the
>> + * Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
>> + * for more details.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/adc/aspeed_adc.h"
>> +#include "hw/qdev-properties.h"
>> +#include "migration/vmstate.h"
>> +#include "trace.h"
>> +#include "qemu/log.h"
>> +
>> +#define TO_REG(offset) ((offset) >> 2)
>> +#define ENGINE_CONTROL TO_REG(0x00)
>> +
>> +static uint64_t aspeed_adc_read(void *opaque, hwaddr offset, unsigned size)
>> +{
>> + AspeedADCState *s = ASPEED_ADC(opaque);
>> + int reg = TO_REG(offset);
>> +
>> + if (reg >= ASPEED_ADC_MAX_REGS) {
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: Out-of-bounds read 0x%04" HWADDR_PRIX "\n",
>> + __func__, offset);
>> + return 0;
>> + }
>> +
>> + int value = s->regs[reg];
>> +
>> + trace_aspeed_adc_read(offset, value);
>> + return value;
>> +}
>> +
>> +static void aspeed_adc_write(void *opaque, hwaddr offset, uint64_t data,
>> + unsigned size)
>> +{
>> + AspeedADCState *s = ASPEED_ADC(opaque);
>> + int reg = TO_REG(offset);
>> +
>> + if (reg >= ASPEED_ADC_MAX_REGS) {
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: Out-of-bounds write 0x%04" HWADDR_PRIX "\n",
>> + __func__, offset);
>> + return;
>> + }
>> +
>> + trace_aspeed_adc_write(offset, data);
>> +
>> + switch (reg) {
>> + case ENGINE_CONTROL:
>> + switch (data) {
>> + case 0xF:
>> + s->regs[ENGINE_CONTROL] = 0x10F;
>> + return;
>> + case 0x2F:
>> + s->regs[ENGINE_CONTROL] = 0xF;
>> + return;
>> + }
>> + break;
>> + }
>> +
>> + s->regs[reg] = data;
>> +}
>> +
>> +static const MemoryRegionOps aspeed_adc_ops = {
>> + .read = aspeed_adc_read,
>> + .write = aspeed_adc_write,
>> + .endianness = DEVICE_LITTLE_ENDIAN,
>> + .valid.min_access_size = 4,
>> + .valid.max_access_size = 4,
>> + .valid.unaligned = false,
>
> That's THE question that has been blocking the patches from being
> merged since 2017.
Oh I see, because the ADC supports unaligned access and some
drivers actually take advantage of that? Maybe I can just leave
a comment or add a qemu_log_mask that says it’s not supported yet?
I could also resubmit that patch Andrew made for unaligned access too.
>
>> +};
>> +
>> +static void aspeed_adc_realize(DeviceState *dev, Error **errp)
>> +{
>> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> + AspeedADCState *s = ASPEED_ADC(dev);
>> +
>> + sysbus_init_irq(sbd, &s->irq);
>> + // The memory region is actually 4KB (0x1000), but there's 2 ADC's in the
>> + // AST2600 that are offset by 0x100.
>
> No C++ comments. Please run ./scripts/checkpatch.pl.
Erg, yeah sorry about that. I forgot about checkpatch.pl, I was
looking at “make help” and tried running sparse cause I couldn’t
remember where/what the linter script was.
>
>
>> + memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_adc_ops, s,
>> + TYPE_ASPEED_ADC, 0x100);
>> + sysbus_init_mmio(sbd, &s->mmio);
>> +}
>> +
>> +static void aspeed_adc_reset(DeviceState *dev)
>> +{
>> + AspeedADCState *s = ASPEED_ADC(dev);
>> + AspeedADCClass *aac = ASPEED_ADC_GET_CLASS(dev);
>> +
>> + memcpy(s->regs, aac->resets, aac->nr_regs << 2);
>> +}
>> +
>> +static const uint32_t aspeed_2400_resets[ASPEED_2400_ADC_NR_REGS] = {
>> + [ENGINE_CONTROL] = 0x00000000,
>> +};
>> +
>> +static const uint32_t aspeed_2500_resets[ASPEED_2500_ADC_NR_REGS] = {
>> + [ENGINE_CONTROL] = 0x00000000,
>> +};
>> +
>> +static const uint32_t aspeed_2600_resets[ASPEED_2600_ADC_NR_REGS] = {
>> + [ENGINE_CONTROL] = 0x00000000,
>> +};
>> +
>> +static const VMStateDescription aspeed_adc_vmstate = {
>> + .name = TYPE_ASPEED_ADC,
>> + .version_id = 0,
>> + .minimum_version_id = 0,
>> + .fields = (VMStateField[]) {
>> + VMSTATE_UINT32_ARRAY(regs, AspeedADCState, ASPEED_ADC_MAX_REGS),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> +static void aspeed_adc_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> + dc->realize = aspeed_adc_realize;
>> + dc->reset = aspeed_adc_reset;
>> + dc->desc = "Aspeed Analog-to-Digital Converter";
>> + dc->vmsd = &aspeed_adc_vmstate;
>> +}
>> +
>> +static void aspeed_2400_adc_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> + AspeedADCClass *aac = ASPEED_ADC_CLASS(klass);
>> +
>> + dc->desc = "Aspeed 2400 Analog-to-Digital Converter";
>> + aac->resets = aspeed_2400_resets;
>> + aac->nr_regs = ASPEED_2400_ADC_NR_REGS;
>> +}
>> +
>> +static void aspeed_2500_adc_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> + AspeedADCClass *aac = ASPEED_ADC_CLASS(klass);
>> +
>> + dc->desc = "Aspeed 2500 Analog-to-Digital Converter";
>> + aac->resets = aspeed_2500_resets;
>> + aac->nr_regs = ASPEED_2500_ADC_NR_REGS;
>> +}
>> +
>> +static void aspeed_2600_adc_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> + AspeedADCClass *aac = ASPEED_ADC_CLASS(klass);
>> +
>> + dc->desc = "Aspeed 2600 Analog-to-Digital Converter";
>> + aac->resets = aspeed_2600_resets;
>> + aac->nr_regs = ASPEED_2600_ADC_NR_REGS;
>> +}
>> +
>> +static const TypeInfo aspeed_adc_info = {
>> + .name = TYPE_ASPEED_ADC,
>> + .parent = TYPE_SYS_BUS_DEVICE,
>> + .instance_size = sizeof(AspeedADCState),
>> + .class_init = aspeed_adc_class_init,
>> + .class_size = sizeof(AspeedADCClass),
>> + .abstract = true,
>> +};
>> +
>> +static const TypeInfo aspeed_2400_adc_info = {
>> + .name = TYPE_ASPEED_2400_ADC,
>> + .parent = TYPE_ASPEED_ADC,
>> + .instance_size = sizeof(AspeedADCState),
>> + .class_init = aspeed_2400_adc_class_init,
>> +};
>> +
>> +static const TypeInfo aspeed_2500_adc_info = {
>> + .name = TYPE_ASPEED_2500_ADC,
>> + .parent = TYPE_ASPEED_ADC,
>> + .instance_size = sizeof(AspeedADCState),
>> + .class_init = aspeed_2500_adc_class_init,
>> +};
>> +
>> +static const TypeInfo aspeed_2600_adc_info = {
>> + .name = TYPE_ASPEED_2600_ADC,
>> + .parent = TYPE_ASPEED_ADC,
>> + .instance_size = sizeof(AspeedADCState),
>> + .class_init = aspeed_2600_adc_class_init,
>> +};
>> +
>> +static void aspeed_adc_register_types(void)
>> +{
>> + type_register_static(&aspeed_adc_info);
>> + type_register_static(&aspeed_2400_adc_info);
>> + type_register_static(&aspeed_2500_adc_info);
>> + type_register_static(&aspeed_2600_adc_info);
>> +}
>> +
>> +type_init(aspeed_adc_register_types);
>> diff --git a/hw/adc/meson.build b/hw/adc/meson.build
>> index ac4f093fea..65e1dd73c1 100644
>> --- a/hw/adc/meson.build
>> +++ b/hw/adc/meson.build
>> @@ -2,3 +2,4 @@ softmmu_ss.add(when: 'CONFIG_STM32F2XX_ADC', if_true: files('stm32f2xx_adc.c'))
>> softmmu_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_adc.c'))
>> softmmu_ss.add(when: 'CONFIG_ZYNQ', if_true: files('zynq-xadc.c'))
>> softmmu_ss.add(when: 'CONFIG_MAX111X', if_true: files('max111x.c'))
>> +softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_adc.c'))
>> diff --git a/hw/adc/trace-events b/hw/adc/trace-events
>> index 456f21c8f4..c23d7bb6d7 100644
>> --- a/hw/adc/trace-events
>> +++ b/hw/adc/trace-events
>> @@ -3,3 +3,7 @@
>> # npcm7xx_adc.c
>> npcm7xx_adc_read(const char *id, uint64_t offset, uint32_t value) " %s offset: 0x%04" PRIx64 " value 0x%04" PRIx32
>> npcm7xx_adc_write(const char *id, uint64_t offset, uint32_t value) "%s offset: 0x%04" PRIx64 " value 0x%04" PRIx32
>> +
>> +# aspeed_adc.c
>> +aspeed_adc_read(uint8_t offset, uint32_t value) "0x%02" PRIx8 " read 0x%04" PRIx32
>> +aspeed_adc_write(uint8_t offset, uint32_t value) "0x%02" PRIx8 " write 0x%04" PRIx32
>> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
>> index 9d70e8e060..a582e882f2 100644
>> --- a/hw/arm/aspeed_ast2600.c
>> +++ b/hw/arm/aspeed_ast2600.c
>> @@ -44,6 +44,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
>> [ASPEED_DEV_SCU] = 0x1E6E2000,
>> [ASPEED_DEV_XDMA] = 0x1E6E7000,
>> [ASPEED_DEV_ADC] = 0x1E6E9000,
>> + [ASPEED_DEV_ADC2] = 0x1E6E9100,
>
> Look at the AspeedADCEngine approach introduced in the initial
> series. It simplifies the integration in the machine.
Oh yeah, I’ll use the AspeedADCEngine approach instead.
>
> Thanks,
>
> C.
>
>> [ASPEED_DEV_VIDEO] = 0x1E700000,
>> [ASPEED_DEV_SDHCI] = 0x1E740000,
>> [ASPEED_DEV_EMMC] = 0x1E750000,
>> @@ -77,6 +78,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
>> [ASPEED_DEV_SDMC] = 0,
>> [ASPEED_DEV_SCU] = 12,
>> [ASPEED_DEV_ADC] = 78,
>> + [ASPEED_DEV_ADC2] = 78,
>> [ASPEED_DEV_XDMA] = 6,
>> [ASPEED_DEV_SDHCI] = 43,
>> [ASPEED_DEV_EHCI1] = 5,
>> @@ -216,6 +218,11 @@ static void aspeed_soc_ast2600_init(Object *obj)
>> snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
>> object_initialize_child(obj, "hace", &s->hace, typename);
>> +
>> + snprintf(typename, sizeof(typename), "aspeed.adc-%s", socname);
>> + for (i = 0; i < sc->adcs_num; i++) {
>> + object_initialize_child(obj, "adc[*]", &s->adc[i], typename);
>> + }
>> }
>> /*
>> @@ -507,6 +514,16 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>> sysbus_mmio_map(SYS_BUS_DEVICE(&s->hace), 0, sc->memmap[ASPEED_DEV_HACE]);
>> sysbus_connect_irq(SYS_BUS_DEVICE(&s->hace), 0,
>> aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
>> +
>> + /* ADC */
>> + for (int i = 0; i < sc->adcs_num; i++) {
>> + SysBusDevice *bus = SYS_BUS_DEVICE(&s->adc[i]);
>> + if (!sysbus_realize(bus, errp)) {
>> + return;
>> + }
>> + sysbus_mmio_map(bus, 0, sc->memmap[ASPEED_DEV_ADC + i]);
>> + sysbus_connect_irq(bus, 0, aspeed_soc_get_irq(s, ASPEED_DEV_ADC + i));
>> + }
>> }
>> static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
>> @@ -524,6 +541,7 @@ static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
>> sc->ehcis_num = 2;
>> sc->wdts_num = 4;
>> sc->macs_num = 4;
>> + sc->adcs_num = 2;
>> sc->irqmap = aspeed_soc_ast2600_irqmap;
>> sc->memmap = aspeed_soc_ast2600_memmap;
>> sc->num_cpus = 2;
>> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
>> index ed84502e23..412c557e40 100644
>> --- a/hw/arm/aspeed_soc.c
>> +++ b/hw/arm/aspeed_soc.c
>> @@ -216,6 +216,11 @@ static void aspeed_soc_init(Object *obj)
>> snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
>> object_initialize_child(obj, "hace", &s->hace, typename);
>> +
>> + snprintf(typename, sizeof(typename), "aspeed.adc-%s", socname);
>> + for (i = 0; i < sc->adcs_num; i++) {
>> + object_initialize_child(obj, "adc[*]", &s->adc[i], typename);
>> + }
>> }
>> static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>> @@ -435,6 +440,16 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>> sysbus_mmio_map(SYS_BUS_DEVICE(&s->hace), 0, sc->memmap[ASPEED_DEV_HACE]);
>> sysbus_connect_irq(SYS_BUS_DEVICE(&s->hace), 0,
>> aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
>> +
>> + /* ADC */
>> + for (int i = 0; i < sc->adcs_num; i++) {
>> + SysBusDevice *bus = SYS_BUS_DEVICE(&s->adc[i]);
>> + if (!sysbus_realize(bus, errp)) {
>> + return;
>> + }
>> + sysbus_mmio_map(bus, 0, sc->memmap[ASPEED_DEV_ADC + i]);
>> + sysbus_connect_irq(bus, 0, aspeed_soc_get_irq(s, ASPEED_DEV_ADC + i));
>> + }
>> }
>> static Property aspeed_soc_properties[] = {
>> DEFINE_PROP_LINK("dram", AspeedSoCState, dram_mr, TYPE_MEMORY_REGION,
>> @@ -475,6 +490,7 @@ static void aspeed_soc_ast2400_class_init(ObjectClass *oc, void *data)
>> sc->ehcis_num = 1;
>> sc->wdts_num = 2;
>> sc->macs_num = 2;
>> + sc->adcs_num = 1;
>> sc->irqmap = aspeed_soc_ast2400_irqmap;
>> sc->memmap = aspeed_soc_ast2400_memmap;
>> sc->num_cpus = 1;
>> @@ -500,6 +516,7 @@ static void aspeed_soc_ast2500_class_init(ObjectClass *oc, void *data)
>> sc->ehcis_num = 2;
>> sc->wdts_num = 3;
>> sc->macs_num = 2;
>> + sc->adcs_num = 1;
>> sc->irqmap = aspeed_soc_ast2500_irqmap;
>> sc->memmap = aspeed_soc_ast2500_memmap;
>> sc->num_cpus = 1;
>> diff --git a/include/hw/adc/aspeed_adc.h b/include/hw/adc/aspeed_adc.h
>> new file mode 100644
>> index 0000000000..5528781be0
>> --- /dev/null
>> +++ b/include/hw/adc/aspeed_adc.h
>> @@ -0,0 +1,48 @@
>> +/*
>> + * Aspeed ADC Controller
>> + *
>> + * Copyright 2021 Facebook, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License as published by the
>> + * Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
>> + * for more details.
>> + */
>> +
>> +#ifndef ASPEED_ADC_H
>> +#define ASPEED_ADC_H
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/sysbus.h"
>> +#include "qom/object.h"
>> +
>> +#define TYPE_ASPEED_ADC "aspeed.adc"
>> +OBJECT_DECLARE_TYPE(AspeedADCState, AspeedADCClass, ASPEED_ADC)
>> +#define TYPE_ASPEED_2400_ADC TYPE_ASPEED_ADC "-ast2400"
>> +#define TYPE_ASPEED_2500_ADC TYPE_ASPEED_ADC "-ast2500"
>> +#define TYPE_ASPEED_2600_ADC TYPE_ASPEED_ADC "-ast2600"
>> +
>> +#define ASPEED_2400_ADC_NR_REGS (0xC4 >> 2)
>> +#define ASPEED_2500_ADC_NR_REGS (0xC8 >> 2)
>> +#define ASPEED_2600_ADC_NR_REGS (0xD0 >> 2)
>> +#define ASPEED_ADC_MAX_REGS ASPEED_2600_ADC_NR_REGS
>> +
>> +struct AspeedADCState {
>> + SysBusDevice parent;
>> + MemoryRegion mmio;
>> + qemu_irq irq;
>> + uint32_t regs[ASPEED_ADC_MAX_REGS];
>> +};
>> +
>> +struct AspeedADCClass {
>> + SysBusDeviceClass parent;
>> + const uint32_t *resets;
>> + uint32_t nr_regs;
>> +};
>> +
>> +#endif
>> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
>> index 87d76c9259..4503f08870 100644
>> --- a/include/hw/arm/aspeed_soc.h
>> +++ b/include/hw/arm/aspeed_soc.h
>> @@ -30,12 +30,14 @@
>> #include "hw/usb/hcd-ehci.h"
>> #include "qom/object.h"
>> #include "hw/misc/aspeed_lpc.h"
>> +#include "hw/adc/aspeed_adc.h"
>> #define ASPEED_SPIS_NUM 2
>> #define ASPEED_EHCIS_NUM 2
>> #define ASPEED_WDTS_NUM 4
>> #define ASPEED_CPUS_NUM 2
>> #define ASPEED_MACS_NUM 4
>> +#define ASPEED_ADCS_NUM 2
>> struct AspeedSoCState {
>> /*< private >*/
>> @@ -65,6 +67,7 @@ struct AspeedSoCState {
>> AspeedSDHCIState sdhci;
>> AspeedSDHCIState emmc;
>> AspeedLPCState lpc;
>> + AspeedADCState adc[ASPEED_ADCS_NUM];
>> uint32_t uart_default;
>> };
>> @@ -82,6 +85,7 @@ struct AspeedSoCClass {
>> int ehcis_num;
>> int wdts_num;
>> int macs_num;
>> + int adcs_num;
>> const int *irqmap;
>> const hwaddr *memmap;
>> uint32_t num_cpus;
>> @@ -105,6 +109,7 @@ enum {
>> ASPEED_DEV_SDMC,
>> ASPEED_DEV_SCU,
>> ASPEED_DEV_ADC,
>> + ASPEED_DEV_ADC2,
>> ASPEED_DEV_VIDEO,
>> ASPEED_DEV_SRAM,
>> ASPEED_DEV_SDHCI,
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH 1/1] hw: aspeed_adc: Add initial Aspeed ADC support
2021-09-30 15:24 ` Peter Delevoryas
@ 2021-09-30 16:21 ` Cédric Le Goater
0 siblings, 0 replies; 5+ messages in thread
From: Cédric Le Goater @ 2021-09-30 16:21 UTC (permalink / raw)
To: Peter Delevoryas
Cc: Peter Maydell, Andrew Jeffery, Amithash Prasad,
Cameron Esfahani via, patrick, qemu-arm, Joel Stanley, Dan Zhang,
Philippe Mathieu-Daudé
>> FYI, these series was sent by Andrew in 2017 and I have been keeping
>> it alive since in the aspeed-x.y branches :
>>
>> * memory: Support unaligned accesses on aligned-only models
>> https://github.com/legoater/qemu/commit/1960ba6bde27b91edb5336985a9210260a4c8938
>>
>> That was requested by Phil I think.
>>
>> * hw/adc: Add basic Aspeed ADC model
>> https://github.com/legoater/qemu/commit/1eff7b1cf10d1777635f7d2cef8ecb441cc607c4
>>
>> This is the initial patch. I added multi-engine support recently
>> for the fuji.
>>
>> * hw/arm: Integrate ADC model into Aspeed SoC
>> https://github.com/legoater/qemu/commit/3052f9d8ccdaf78b753e53574b7e8cc2ee01429f
>>
>> That one is trivial.
>>
>>
>> Overall comments :
>>
>> I prefer the 'regs' array approach of your proposal.
>
> Ok (I was just following patterns from aspeed_scu.c), I’ll keep that aspect.
>
>>
>> I think the AspeedADCEngine should appear as a QOM object. Check
>> the patches above.
>
> I see, I’ll make sure to test this.
The engines are behind the same BAR and they share the IRQ. So
it makes sense I think. And it shows up nicely under the monitor :
...
000000001e6e7000-000000001e6e7fff (prio 0, i/o): aspeed.xdma
000000001e6e9000-000000001e6e9fff (prio 0, i/o): aspeed.adc
000000001e6e9000-000000001e6e90ff (prio 0, i/o): aspeed.adc.engine.0
000000001e6e9100-000000001e6e91ff (prio 0, i/o): aspeed.adc.engine.1
000000001e700000-000000001e700fff (prio -1000, i/o): aspeed.video
...
/adc (aspeed.adc-ast2600)
/aspeed.adc[0] (memory-region)
/engine[0] (aspeed.adc.engine)
/aspeed.adc.engine.0[0] (memory-region)
/engine[1] (aspeed.adc.engine)
/aspeed.adc.engine.1[0] (memory-region)
/unnamed-gpio-in[0] (irq)
/unnamed-gpio-in[1] (irq)
>> To move on, maybe, you could rework the initial series and take
>> ownership ?
>>
>
> Yeah definitely! I’ll resubmit once I’ve reworked it. I don’t intend
> to include the unaligned-access support though, at least not w/ the ADC
> changes.
Yeah. This can come later.
Thanks,
C.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-09-30 16:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-30 0:42 [RFC PATCH 0/1] hw: aspeed_adc: Add initial Aspeed ADC support pdel
2021-09-30 0:42 ` [RFC PATCH 1/1] " pdel
2021-09-30 6:22 ` Cédric Le Goater
2021-09-30 15:24 ` Peter Delevoryas
2021-09-30 16:21 ` Cédric Le Goater
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.