Now that all of the callers of this function have been switched to use qdev
properties, this legacy init function can now be removed.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/rtc/m48t59.c | 35 -----------------------------------
include/hw/rtc/m48t59.h | 4 ----
2 files changed, 39 deletions(-)
diff --git a/hw/rtc/m48t59.c b/hw/rtc/m48t59.c
index 6525206976..d54929e861 100644
--- a/hw/rtc/m48t59.c
+++ b/hw/rtc/m48t59.c
@@ -564,41 +564,6 @@ const MemoryRegionOps m48t59_io_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
};
-/* Initialisation routine */
-Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
- uint32_t io_base, uint16_t size, int base_year,
- int model)
-{
- DeviceState *dev;
- SysBusDevice *s;
- int i;
-
- for (i = 0; i < ARRAY_SIZE(m48txx_sysbus_info); i++) {
- if (m48txx_sysbus_info[i].size != size ||
- m48txx_sysbus_info[i].model != model) {
- continue;
- }
-
- dev = qdev_new(m48txx_sysbus_info[i].bus_name);
- qdev_prop_set_int32(dev, "base-year", base_year);
- s = SYS_BUS_DEVICE(dev);
- sysbus_realize_and_unref(s, &error_fatal);
- sysbus_connect_irq(s, 0, IRQ);
- if (io_base != 0) {
- memory_region_add_subregion(get_system_io(), io_base,
- sysbus_mmio_get_region(s, 1));
- }
- if (mem_base != 0) {
- sysbus_mmio_map(s, 0, mem_base);
- }
-
- return NVRAM(s);
- }
-
- assert(false);
- return NULL;
-}
-
void m48t59_realize_common(M48t59State *s, Error **errp)
{
s->buffer = g_malloc0(s->size);
diff --git a/include/hw/rtc/m48t59.h b/include/hw/rtc/m48t59.h
index 9defe578d1..d9b45eb161 100644
--- a/include/hw/rtc/m48t59.h
+++ b/include/hw/rtc/m48t59.h
@@ -47,8 +47,4 @@ struct NvramClass {
void (*toggle_lock)(Nvram *obj, int lock);
};
-Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
- uint32_t io_base, uint16_t size, int base_year,
- int type);
-
#endif /* HW_M48T59_H */
--
2.20.1
On 10/16/20 8:27 PM, Mark Cave-Ayland wrote:
> Now that all of the callers of this function have been switched to use qdev
> properties, this legacy init function can now be removed.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/rtc/m48t59.c | 35 -----------------------------------
> include/hw/rtc/m48t59.h | 4 ----
> 2 files changed, 39 deletions(-)
In the PoC I started after your suggestion, I see:
#define TYPE_M48T02_SRAM "sysbus-m48t02"
#define TYPE_M48T08_SRAM "sysbus-m48t08"
#define TYPE_M48T59_SRAM "sysbus-m48t59"
static void m48t02_class_init(ObjectClass *oc, void *data)
{
M48txxSysBusDeviceClass *amc = M48TXX_SYS_BUS_CLASS(oc);
amc->model = 2;
amc->size = 2 * KiB;
};
static void m48t08_class_init(ObjectClass *oc, void *data)
{
M48txxSysBusDeviceClass *amc = M48TXX_SYS_BUS_CLASS(oc);
amc->model = 8;
amc->size = 8 * KiB;
};
static void m48t59_class_init(ObjectClass *oc, void *data)
{
M48txxSysBusDeviceClass *amc = M48TXX_SYS_BUS_CLASS(oc);
amc->model = 59;
amc->size = 8 * KiB;
};
static const TypeInfo m48t59_register_types[] = {
{
.name = TYPE_M48T02_SRAM,
.parent = TYPE_M48TXX_SYSBUS,
.class_init = m48t02_class_init,
}, {
.name = TYPE_M48T08_SRAM,
.parent = TYPE_M48TXX_SYSBUS,
.class_init = m48t08_class_init,
}, {
.name = TYPE_M48T59_SRAM,
.parent = TYPE_M48TXX_SYSBUS,
.class_init = m48t59_class_init,
}, {
.name = TYPE_M48TXX_SYSBUS,
.parent = TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(M48txxSysBusState),
.instance_init = m48t59_init1,
.class_size = sizeof(M48txxSysBusDeviceClass),
.class_init = m48txx_sysbus_class_init,
.abstract = true,
.interfaces = (InterfaceInfo[]) {
{ TYPE_NVRAM },
{ }
}
}
};
and:
#define TYPE_M48T59_SRAM "isa-m48t59"
static void m48t59_class_init(ObjectClass *oc, void *data)
{
M48txxISADeviceClass *midc = M48TXX_ISA_CLASS(oc);
midc->model = 59;
midc->size = 8 * KiB;
};
static const TypeInfo m48t59_isa_register_types[] = {
{
.name = TYPE_M48T59_SRAM,
.parent = TYPE_M48TXX_ISA,
.class_init = m48t59_class_init,
}, {
.name = TYPE_M48TXX_ISA,
.parent = TYPE_ISA_DEVICE,
.instance_size = sizeof(M48txxISAState),
.class_size = sizeof(M48txxISADeviceClass),
.class_init = m48txx_isa_class_init,
.abstract = true,
.interfaces = (InterfaceInfo[]) {
{ TYPE_NVRAM },
{ }
}
}
};
I guess I didn't pursue because I wondered what was the
best way to have the same model usable by sysbus/isa.
IIRC I wanted to proceed as having TYPE_M48T59_SRAM being
an abstract qdev parent, and then TYPE_M48TXX_SYSBUS /
TYPE_M48TXX_ISA implementing the SYSBUS/ISA interfaces.
As it need some thinking I postponed that for after 5.2.
Anyhow back to this patch:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
On 17/10/2020 10:53, Philippe Mathieu-Daudé wrote:
> On 10/16/20 8:27 PM, Mark Cave-Ayland wrote:
>> Now that all of the callers of this function have been switched to use qdev
>> properties, this legacy init function can now be removed.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> hw/rtc/m48t59.c | 35 -----------------------------------
>> include/hw/rtc/m48t59.h | 4 ----
>> 2 files changed, 39 deletions(-)
>
> In the PoC I started after your suggestion, I see:
>
> #define TYPE_M48T02_SRAM "sysbus-m48t02"
> #define TYPE_M48T08_SRAM "sysbus-m48t08"
> #define TYPE_M48T59_SRAM "sysbus-m48t59"
>
> static void m48t02_class_init(ObjectClass *oc, void *data)
> {
> M48txxSysBusDeviceClass *amc = M48TXX_SYS_BUS_CLASS(oc);
>
> amc->model = 2;
> amc->size = 2 * KiB;
> };
>
> static void m48t08_class_init(ObjectClass *oc, void *data)
> {
> M48txxSysBusDeviceClass *amc = M48TXX_SYS_BUS_CLASS(oc);
>
> amc->model = 8;
> amc->size = 8 * KiB;
> };
>
> static void m48t59_class_init(ObjectClass *oc, void *data)
> {
> M48txxSysBusDeviceClass *amc = M48TXX_SYS_BUS_CLASS(oc);
>
> amc->model = 59;
> amc->size = 8 * KiB;
> };
>
> static const TypeInfo m48t59_register_types[] = {
> {
> .name = TYPE_M48T02_SRAM,
> .parent = TYPE_M48TXX_SYSBUS,
> .class_init = m48t02_class_init,
> }, {
> .name = TYPE_M48T08_SRAM,
> .parent = TYPE_M48TXX_SYSBUS,
> .class_init = m48t08_class_init,
> }, {
> .name = TYPE_M48T59_SRAM,
> .parent = TYPE_M48TXX_SYSBUS,
> .class_init = m48t59_class_init,
> }, {
> .name = TYPE_M48TXX_SYSBUS,
> .parent = TYPE_SYS_BUS_DEVICE,
> .instance_size = sizeof(M48txxSysBusState),
> .instance_init = m48t59_init1,
> .class_size = sizeof(M48txxSysBusDeviceClass),
> .class_init = m48txx_sysbus_class_init,
> .abstract = true,
> .interfaces = (InterfaceInfo[]) {
> { TYPE_NVRAM },
> { }
> }
> }
> };
>
> and:
>
> #define TYPE_M48T59_SRAM "isa-m48t59"
>
> static void m48t59_class_init(ObjectClass *oc, void *data)
> {
> M48txxISADeviceClass *midc = M48TXX_ISA_CLASS(oc);
>
> midc->model = 59;
> midc->size = 8 * KiB;
> };
>
> static const TypeInfo m48t59_isa_register_types[] = {
> {
> .name = TYPE_M48T59_SRAM,
> .parent = TYPE_M48TXX_ISA,
> .class_init = m48t59_class_init,
> }, {
> .name = TYPE_M48TXX_ISA,
> .parent = TYPE_ISA_DEVICE,
> .instance_size = sizeof(M48txxISAState),
> .class_size = sizeof(M48txxISADeviceClass),
> .class_init = m48txx_isa_class_init,
> .abstract = true,
> .interfaces = (InterfaceInfo[]) {
> { TYPE_NVRAM },
> { }
> }
> }
> };
>
> I guess I didn't pursue because I wondered what was the
> best way to have the same model usable by sysbus/isa.
>
> IIRC I wanted to proceed as having TYPE_M48T59_SRAM being
> an abstract qdev parent, and then TYPE_M48TXX_SYSBUS /
> TYPE_M48TXX_ISA implementing the SYSBUS/ISA interfaces.
>
> As it need some thinking I postponed that for after 5.2.
>
> Anyhow back to this patch:
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Ha indeed, I think you also came to the same conclusion that I did in my previous
email :)
I'm also not convinced by the dynamic generation of various M48TXX types using
class_data - this seems overly complex, and there's nothing there I can see that
can't be just as easily handled using qdev properties using an abstract parent as you
suggest above.
ATB,
Mark.
On 10/17/20 1:19 PM, Mark Cave-Ayland wrote:
> On 17/10/2020 10:53, Philippe Mathieu-Daudé wrote:
>
>> On 10/16/20 8:27 PM, Mark Cave-Ayland wrote:
>>> Now that all of the callers of this function have been switched to
>>> use qdev
>>> properties, this legacy init function can now be removed.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> ---
>>> hw/rtc/m48t59.c | 35 -----------------------------------
>>> include/hw/rtc/m48t59.h | 4 ----
>>> 2 files changed, 39 deletions(-)
...
>> static void m48t59_class_init(ObjectClass *oc, void *data)
>> {
>> M48txxISADeviceClass *midc = M48TXX_ISA_CLASS(oc);
>>
>> midc->model = 59;
>> midc->size = 8 * KiB;
>> };
>>
>> static const TypeInfo m48t59_isa_register_types[] = {
>> {
>> .name = TYPE_M48T59_SRAM,
>> .parent = TYPE_M48TXX_ISA,
>> .class_init = m48t59_class_init,
>> }, {
>> .name = TYPE_M48TXX_ISA,
>> .parent = TYPE_ISA_DEVICE,
>> .instance_size = sizeof(M48txxISAState),
>> .class_size = sizeof(M48txxISADeviceClass),
>> .class_init = m48txx_isa_class_init,
>> .abstract = true,
>> .interfaces = (InterfaceInfo[]) {
>> { TYPE_NVRAM },
>> { }
>> }
>> }
>> };
>>
>> I guess I didn't pursue because I wondered what was the
>> best way to have the same model usable by sysbus/isa.
>>
>> IIRC I wanted to proceed as having TYPE_M48T59_SRAM being
>> an abstract qdev parent, and then TYPE_M48TXX_SYSBUS /
>> TYPE_M48TXX_ISA implementing the SYSBUS/ISA interfaces.
>>
>> As it need some thinking I postponed that for after 5.2.
>>
>> Anyhow back to this patch:
>>
>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> Ha indeed, I think you also came to the same conclusion that I did in my
> previous email :)
>
> I'm also not convinced by the dynamic generation of various M48TXX types
> using class_data - this seems overly complex, and there's nothing there
> I can see that can't be just as easily handled using qdev properties
> using an abstract parent as you suggest above.
Yeah, no advantage except having uniform code style that serves
as example.
>
>
> ATB,
>
> Mark.
>
© 2016 - 2026 Red Hat, Inc.