* [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@stwcx.xyz, 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@stwcx.xyz, 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).