Re: [PATCH 5/5] m48t59: remove legacy m48t59_init() function
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
---
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é
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.
Re: [PATCH 5/5] m48t59: remove legacy m48t59_init() function
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
---
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é
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.
Re: [PATCH 5/5] m48t59: remove legacy m48t59_init() function
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
---
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é
