Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
 hw/timer/mc146818rtc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index acee47d..808a212 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -70,6 +70,7 @@ typedef struct RTCState {
     ISADevice parent_obj;
 
     MemoryRegion io;
+    MemoryRegion coalesced_io;
     uint8_t cmos_data[128];
     uint8_t cmos_index;
     int32_t base_year;
@@ -990,6 +991,13 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
     memory_region_init_io(&s->io, OBJECT(s), &cmos_ops, s, "rtc", 2);
     isa_register_ioport(isadev, &s->io, base);
 
+    /* register rtc 0x70 port as coalesced_pio */
+    memory_region_set_flush_coalesced(&s->io);
+    memory_region_init_io(&s->coalesced_io, OBJECT(s), &cmos_ops,
+                          s, "rtc1", 1);
+    isa_register_ioport(isadev, &s->coalesced_io, base);
+    memory_region_add_coalescing(&s->coalesced_io, 0, 1);
+
     qdev_set_legacy_instance_id(dev, base, 3);
     qemu_register_reset(rtc_reset, s);
 
-- 
1.8.3.1

On 17/10/2018 18:52, Peng Hao wrote:
> +    /* register rtc 0x70 port as coalesced_pio */
> +    memory_region_set_flush_coalesced(&s->io);
> +    memory_region_init_io(&s->coalesced_io, OBJECT(s), &cmos_ops,
> +                          s, "rtc1", 1);
> +    isa_register_ioport(isadev, &s->coalesced_io, base);

I think instead of isa_register_ioport you can use
memory_region_add_subregion, so that s->coalesced_io is added below s->io?

This way, you don't rely on the behavior of overlapping regions.

Paolo

> +    memory_region_add_coalescing(&s->coalesced_io, 0, 1);
> +

>On 17/10/2018 18:52, Peng Hao wrote:
>> +    /* register rtc 0x70 port as coalesced_pio */
>> +    memory_region_set_flush_coalesced(&s->io);
>> +    memory_region_init_io(&s->coalesced_io, OBJECT(s), &cmos_ops,
>> +                          s, "rtc1", 1);
>> +    isa_register_ioport(isadev, &s->coalesced_io, base);
>
>I think instead of isa_register_ioport you can use
>memory_region_add_subregion, so that s->coalesced_io is added below s->io?
>
isa_register_ioport also called memory_region_add_subregion.
modify code  like this:
  //isa_register_ioport(isadev, &s->coalesced_io, base);
    memory_region_add_subregion(get_system_io(),0x70,&s->coalesced_io);
The regional distribution before and after code modification is consistent.

   0000000000000070-0000000000000070 (prio 0, i/o): rtc1
    0000000000000070-0000000000000071 (prio 0, i/o): rtc
>This way, you don't rely on the behavior of overlapping regions.
>
>Paolo
>
>> +    memory_region_add_coalescing(&s->coalesced_io, 0, 1);
>> +

On 18/10/2018 10:03, peng.hao2@zte.com.cn wrote:
>>> +    /* register rtc 0x70 port as coalesced_pio */
>>> +    memory_region_set_flush_coalesced(&s->io);
>>> +    memory_region_init_io(&s->coalesced_io, OBJECT(s), &cmos_ops,
>>> +                          s, "rtc1", 1);
>>> +    isa_register_ioport(isadev, &s->coalesced_io, base);
>> I think instead of isa_register_ioport you can use
>> memory_region_add_subregion, so that s->coalesced_io is added below s->io?
>>
> isa_register_ioport also called memory_region_add_subregion.
> modify code  like this:
>   //isa_register_ioport(isadev, &s->coalesced_io, base);
>     memory_region_add_subregion(get_system_io(),0x70,&s->coalesced_io);
> The regional distribution before and after code modification is consistent.

Right, but I'd rather add s->coalesced_io subregion as a subregion of
s->io at offset 0.

Paolo

>    0000000000000070-0000000000000070 (prio 0, i/o): rtc1
>     0000000000000070-0000000000000071 (prio 0, i/o): rtc

>On 18/10/2018 10:03, peng.hao2@zte.com.cn wrote:
>>>> +    /* register rtc 0x70 port as coalesced_pio */
>>>> +    memory_region_set_flush_coalesced(&s->io);
>>>> +    memory_region_init_io(&s->coalesced_io, OBJECT(s), &cmos_ops,
>>>> +                          s, "rtc1", 1);
>>>> +    isa_register_ioport(isadev, &s->coalesced_io, base);
>>> I think instead of isa_register_ioport you can use
>>> memory_region_add_subregion, so that s->coalesced_io is added below s->io?
>>>
>> isa_register_ioport also called memory_region_add_subregion.
>> modify code  like this:
>>   //isa_register_ioport(isadev, &s->coalesced_io, base);
>>     memory_region_add_subregion(get_system_io(),0x70,&s->coalesced_io);
>> The regional distribution before and after code modification is consistent.
>
>Right, but I'd rather add s->coalesced_io subregion as a subregion of
>s->io at offset 0.
Ok， I will modify it and resubmit the patch.
By the way , I find that  when I adjusted the format of the patch "PATCH V6 1/5", I made a mistake to modify a code character. I also resubmit the patch.
Thanks.
>
>Paolo
>
>>    0000000000000070-0000000000000070 (prio 0, i/o): rtc1
>>     0000000000000070-0000000000000071 (prio 0, i/o): rtc