Re: [PATCH v2 10/13] hw/ppc/mac.h: Move grackle-pcihost declaration out from shared header

[Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>]

On 29/09/2022 12:42, BALATON Zoltan wrote:

> On Thu, 29 Sep 2022, Mark Cave-Ayland wrote:
>> On 25/09/2022 13:38, BALATON Zoltan wrote:
>>
>>> It is only used by mac_oldworld anyway and it already instantiates
>>> a few devices by name so this allows reducing the shared header further.
>>>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>>   hw/pci-host/grackle.c | 1 +
>>>   hw/ppc/mac.h          | 3 ---
>>>   hw/ppc/mac_oldworld.c | 2 +-
>>>   3 files changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
>>> index b05facf463..5282123004 100644
>>> --- a/hw/pci-host/grackle.c
>>> +++ b/hw/pci-host/grackle.c
>>> @@ -34,6 +34,7 @@
>>>   #include "trace.h"
>>>   #include "qom/object.h"
>>>   +#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost"
>>>   OBJECT_DECLARE_SIMPLE_TYPE(GrackleState, GRACKLE_PCI_HOST_BRIDGE)
>>>     struct GrackleState {
>>> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
>>> index 55cb02c990..fe77a6c6db 100644
>>> --- a/hw/ppc/mac.h
>>> +++ b/hw/ppc/mac.h
>>> @@ -35,9 +35,6 @@
>>>   #define KERNEL_LOAD_ADDR 0x01000000
>>>   #define KERNEL_GAP       0x00100000
>>>   -/* Grackle PCI */
>>> -#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost"
>>> -
>>>   /* Mac NVRAM */
>>>   #define TYPE_MACIO_NVRAM "macio-nvram"
>>>   OBJECT_DECLARE_SIMPLE_TYPE(MacIONVRAMState, MACIO_NVRAM)
>>> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
>>> index 1fa7b770b7..1355d032ff 100644
>>> --- a/hw/ppc/mac_oldworld.c
>>> +++ b/hw/ppc/mac_oldworld.c
>>> @@ -214,7 +214,7 @@ static void ppc_heathrow_init(MachineState *machine)
>>>       }
>>>         /* Grackle PCI host bridge */
>>> -    grackle_dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
>>> +    grackle_dev = qdev_new("grackle-pcihost");
>>>       qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000);
>>>       s = SYS_BUS_DEVICE(grackle_dev);
>>>       sysbus_realize_and_unref(s, &error_fatal);
>>
>> Why did you include this patch again in v2 when I nacked it in v1?
> 
> You did not nack it just said you'd prefer a header instead. As a reviwer you express 
> your opinion not an absolute decision that can't be discussed. I've replied to that 
> but could not drop this patch as it's needed for later patches to get mac.h cleaned 
> so until we agree on something this has left unchanged. I'm not a fan of one line 
> headers and splitting up files into separate directories that is harder to work with 
> and also think reorganising grackle is a separate clean up out of scope for this 
> series but OK, I'll move the TYPE define in a new header in the next version. I'll 
> wait for your reply on sysbus_mmio_map before sending a new version.

I should add this obviously also includes moving the GrackleState struct to the new 
header file at the same time, as per typical QOM practice.


ATB,

Mark.