On 10/29/21 14:15, BALATON Zoltan wrote:
On Fri, 29 Oct 2021, Philippe Mathieu-Daudé wrote:
On 10/28/21 21:27, BALATON Zoltan wrote:
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/char/sh_serial.c | 107 +++++++++++++++++++++++++++-----------------
hw/sh4/sh7750.c | 62 ++++++++++++++++++-------
include/hw/sh4/sh.h | 9 +---
3 files changed, 114 insertions(+), 64 deletions(-)
+OBJECT_DECLARE_SIMPLE_TYPE(SHSerialState, SH_SERIAL)
+
+struct SHSerialState {
+ SysBusDevice parent;
[...]
-} SHSerialState;
+};
+
+typedef struct {} SHSerialStateClass;
OBJECT_DECLARE_TYPE()?
From include/qom/object.h:
* OBJECT_DECLARE_SIMPLE_TYPE:
[...]
* This does the same as OBJECT_DECLARE_TYPE(), but with no class struct
* declared.
*
* This macro should be used unless the class struct needs to have
* virtual methods declared.
I think we're rather missing OBJECT_DEFINE_SIMPLE_TYPE. A lot of current
object definitions are open coded because of that and could be replaced
if we had that simple variant but we don't, so this is the shortest way
for now.
-void sh_serial_init(MemoryRegion *sysmem,
- hwaddr base, int feat,
- uint32_t freq, Chardev *chr,
- qemu_irq eri_source,
- qemu_irq rxi_source,
- qemu_irq txi_source,
- qemu_irq tei_source,
- qemu_irq bri_source)
+static void sh_serial_reset(DeviceState *dev)
Can you extract sh_serial_reset() in a previous patch?
I could.
{
- SHSerialState *s = g_malloc0(sizeof(*s));
+ SHSerialState *s = SH_SERIAL(dev);
- s->feat = feat;
s->flags = SH_SERIAL_FLAG_TEND | SH_SERIAL_FLAG_TDE;
s->rtrg = 1;
@@ -397,38 +396,64 @@ void sh_serial_init(MemoryRegion *sysmem,
s->scr = 1 << 5; /* pretend that TX is enabled so early printk
works */
s->sptr = 0;
- if (feat & SH_SERIAL_FEAT_SCIF) {
+ if (s->feat & SH_SERIAL_FEAT_SCIF) {
s->fcr = 0;
} else {
s->dr = 0xff;
}
sh_serial_clear_fifo(s);
+}
- memory_region_init_io(&s->iomem, NULL, &sh_serial_ops, s,
- "serial", 0x100000000ULL);
Keep that, ...
- memory_region_init_alias(&s->iomem_p4, NULL, "serial-p4",
&s->iomem,
- 0, 0x28);
- memory_region_add_subregion(sysmem, P4ADDR(base), &s->iomem_p4);
-
- memory_region_init_alias(&s->iomem_a7, NULL, "serial-a7",
&s->iomem,
- 0, 0x28);
- memory_region_add_subregion(sysmem, A7ADDR(base), &s->iomem_a7);
... and these lines become one single sysbus_init_mmio() ...
Not sure about that. The device doesn't really have two io regions, they
just appear twice due to how the CPU maps them. So I'd keep a single
MMIO region here but could map one directly and use only one alias for
the other instead. (That would get rid of either serial-a7 or serial-p4
with the other just called serial or actually sci/scif after this series).
Looking at the current mapping:
memory-region: system
0000000000000000-ffffffffffffffff (prio 0, i/o): system
0000000000000000-0000000000ffffff (prio 0, romd): r2d.flash
0000000004000000-000000000400003f (prio 0, i/o): r2d-fpga
000000000c000000-000000000fffffff (prio 0, ram): r2d.sdram
0000000010000000-00000000107fffff (prio 0, ram): sm501.local
0000000013e00000-0000000013ffffff (prio 0, i/o): sm501.mmio
0000000013e00000-0000000013e0006b (prio 0, i/o): sm501-system-config
0000000013e10040-0000000013e10053 (prio 0, i/o): sm501-i2c
0000000013e30000-0000000013e3001f (prio 0, i/o): serial
0000000013e40000-0000000013e400ff (prio 0, i/o): ohci
0000000013e80000-0000000013e80fff (prio 0, i/o): sm501-disp-ctrl
0000000013f00000-0000000013f00053 (prio 0, i/o): sm501-2d-engine
000000001400080c-000000001400080f (prio 0, i/o): ide-mmio.2
0000000014001000-000000001400101f (prio 0, i/o): ide-mmio.1
000000001e080000-000000001e080003 (prio 0, i/o): alias
interrupt-controller-prio-set-a7 @interrupt-controller
000000001e080000-000000001e080003
000000001e080040-000000001e080043 (prio 0, i/o): alias
interrupt-controller-mask-set-a7 @interrupt-controller
000000001e080040-000000001e080043
000000001e080060-000000001e080063 (prio 0, i/o): alias
interrupt-controller-mask-clr-a7 @interrupt-controller
000000001e080060-000000001e080063
000000001e100000-000000001e100fff (prio 0, i/o): alias timer-a7
@timer 0000000000000000-0000000000000fff
000000001e200000-000000001e200223 (prio 0, i/o): alias sh_pci.2
@sh_pci 0000000000000000-0000000000000223
000000001f000000-000000001f000fff (prio 0, i/o): alias memory-1f0
@memory 000000001f000000-000000001f000fff
000000001f800000-000000001f800fff (prio 0, i/o): alias memory-1f8
@memory 000000001f800000-000000001f800fff
000000001fc00000-000000001fc00fff (prio 0, i/o): alias memory-1fc
@memory 000000001fc00000-000000001fc00fff
000000001fd00004-000000001fd00007 (prio 0, i/o): alias
interrupt-controller-prio-set-a7 @interrupt-controller
000000001fd00004-000000001fd00007
000000001fd00008-000000001fd0000b (prio 0, i/o): alias
interrupt-controller-prio-set-a7 @interrupt-controller
000000001fd00008-000000001fd0000b
000000001fd0000c-000000001fd0000f (prio 0, i/o): alias
interrupt-controller-prio-set-a7 @interrupt-controller
000000001fd0000c-000000001fd0000f
000000001fd00010-000000001fd00013 (prio 0, i/o): alias
interrupt-controller-prio-set-a7 @interrupt-controller
000000001fd00010-000000001fd00013
000000001fd80000-000000001fd80fff (prio 0, i/o): alias timer-a7
@timer 0000000000000000-0000000000000fff
000000001fe00000-000000001fe00027 (prio 0, i/o): alias serial-a7
@serial 0000000000000000-0000000000000027
000000001fe80000-000000001fe80027 (prio 0, i/o): alias serial-a7
@serial 0000000000000000-0000000000000027
00000000f0000000-00000000f7ffffff (prio 0, i/o): cache-and-tlb
00000000fe080000-00000000fe080003 (prio 0, i/o): alias
interrupt-controller-prio-set-p4 @interrupt-controller
000000001e080000-000000001e080003
00000000fe080040-00000000fe080043 (prio 0, i/o): alias
interrupt-controller-mask-set-p4 @interrupt-controller
000000001e080040-000000001e080043
00000000fe080060-00000000fe080063 (prio 0, i/o): alias
interrupt-controller-mask-clr-p4 @interrupt-controller
000000001e080060-000000001e080063
00000000fe100000-00000000fe100fff (prio 0, i/o): alias timer-p4
@timer 0000000000000000-0000000000000fff
00000000fe200000-00000000fe200223 (prio 0, i/o): sh_pci
00000000fe240000-00000000fe27ffff (prio 0, i/o): alias sh_pci.isa
@io 0000000000000000-000000000003ffff
00000000ff000000-00000000ff000fff (prio 0, i/o): alias memory-ff0
@memory 000000001f000000-000000001f000fff
00000000ff800000-00000000ff800fff (prio 0, i/o): alias memory-ff8
@memory 000000001f800000-000000001f800fff
00000000ffc00000-00000000ffc00fff (prio 0, i/o): alias memory-ffc
@memory 000000001fc00000-000000001fc00fff
00000000ffd00004-00000000ffd00007 (prio 0, i/o): alias
interrupt-controller-prio-set-p4 @interrupt-controller
000000001fd00004-000000001fd00007
00000000ffd00008-00000000ffd0000b (prio 0, i/o): alias
interrupt-controller-prio-set-p4 @interrupt-controller
000000001fd00008-000000001fd0000b
00000000ffd0000c-00000000ffd0000f (prio 0, i/o): alias
interrupt-controller-prio-set-p4 @interrupt-controller
000000001fd0000c-000000001fd0000f
00000000ffd00010-00000000ffd00013 (prio 0, i/o): alias
interrupt-controller-prio-set-p4 @interrupt-controller
000000001fd00010-000000001fd00013
00000000ffd80000-00000000ffd80fff (prio 0, i/o): alias timer-p4
@timer 0000000000000000-0000000000000fff
00000000ffe00000-00000000ffe00027 (prio 0, i/o): alias serial-p4
@serial 0000000000000000-0000000000000027
00000000ffe80000-00000000ffe80027 (prio 0, i/o): alias serial-p4
@serial 0000000000000000-0000000000000027
It seems the 32MiB container region in 0x1e000000-0x1fffffff is
aliased to 0xfe000000-0xffffffff. But I haven't looked at the
datasheet (and don't have time until next week).