[prev in list] [next in list] [prev in thread] [next in thread] 

List:       qemu-ppc
Subject:    Re: [PATCH v2 05/13] mac_oldworld: Do not open code sysbus_mmio_map()
From:       BALATON Zoltan <balaton () eik ! bme ! hu>
Date:       2022-09-29 11:32:20
Message-ID: 41707493-e27f-53b3-dcde-4543670b7c () eik ! bme ! hu
[Download RAW message or body]

On Thu, 29 Sep 2022, Mark Cave-Ayland wrote:
> On 25/09/2022 13:38, BALATON Zoltan wrote:
>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>   hw/ppc/mac_oldworld.c | 8 ++------
>>   1 file changed, 2 insertions(+), 6 deletions(-)
>> 
>> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
>> index cb67e44081..75fbd2a7df 100644
>> --- a/hw/ppc/mac_oldworld.c
>> +++ b/hw/ppc/mac_oldworld.c
>> @@ -218,16 +218,12 @@ static void ppc_heathrow_init(MachineState *machine)
>>       qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000);
>>       s = SYS_BUS_DEVICE(grackle_dev);
>>       sysbus_realize_and_unref(s, &error_fatal);
>> -
>>       sysbus_mmio_map(s, 0, GRACKLE_BASE);
>>       sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x200000);
>>       /* PCI hole */
>> -    memory_region_add_subregion(get_system_memory(), 0x80000000ULL,
>> -                                sysbus_mmio_get_region(s, 2));
>> +    sysbus_mmio_map(s, 2, 0x80000000);
>>       /* Register 2 MB of ISA IO space */
>> -    memory_region_add_subregion(get_system_memory(), 0xfe000000,
>> -                                sysbus_mmio_get_region(s, 3));
>> -
>> +    sysbus_mmio_map(s, 3, 0xfe000000);
>>       pci_bus = PCI_HOST_BRIDGE(grackle_dev)->bus;
>>         /* MacIO */
>
> Please drop this patch for now. The code was written on assumption that both 
> sysbus and sysbus devices would be going away soon, and there are certainly 
> discussions under way about coming up with a migration strategy to allow them 
> to be completely removed.

This patch actually simplifies transition from sysbus to whatever else 
will be decided because then you'll surely have a way to replace 
sysbus_mmio_map() that's used everywhere else with something. This file 
now has both sysbus_mmio_map and sysbus mmio_get_region so using only one 
will make it easier to convert it and until then it's easier to read so I 
don't agree with this suggestion and want to stick to these patches (same 
with uninorth). Please reconsider your decision.

Regards,
BALATON Zoltan

[prev in list] [next in list] [prev in thread] [next in thread]