[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/6] include/hw/southbridge/piix: Aggregate all PIIX sough
From: |
Bernhard Beschow |
Subject: |
Re: [PATCH v2 1/6] include/hw/southbridge/piix: Aggregate all PIIX soughbridge type names |
Date: |
Sun, 29 May 2022 11:23:35 +0200 |
Am 22. Mai 2022 22:32:06 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Sun, 22 May 2022, Bernhard Beschow wrote:
>> TYPE_PIIX3_PCI_DEVICE resides there as already, so add the remaining
>> ones, too.
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> hw/isa/piix3.c | 3 ---
>> include/hw/isa/isa.h | 2 --
>> include/hw/southbridge/piix.h | 4 ++++
>> 3 files changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
>> index dab901c9ad..d96ce2b788 100644
>> --- a/hw/isa/piix3.c
>> +++ b/hw/isa/piix3.c
>> @@ -35,9 +35,6 @@
>>
>> #define XEN_PIIX_NUM_PIRQS 128ULL
>>
>> -#define TYPE_PIIX3_DEVICE "PIIX3"
>> -#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
>> -
>> static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
>> {
>> qemu_set_irq(piix3->pic[pic_irq],
>> diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
>> index 034d706ba1..e9fa2f5cea 100644
>> --- a/include/hw/isa/isa.h
>> +++ b/include/hw/isa/isa.h
>> @@ -144,6 +144,4 @@ static inline ISABus *isa_bus_from_device(ISADevice *d)
>> return ISA_BUS(qdev_get_parent_bus(DEVICE(d)));
>> }
>>
>> -#define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
>> -
>> #endif
>> diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
>> index f63f83e5c6..4d17400dfd 100644
>> --- a/include/hw/southbridge/piix.h
>> +++ b/include/hw/southbridge/piix.h
>> @@ -70,6 +70,10 @@ typedef struct PIIXState PIIX3State;
>> DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
>> TYPE_PIIX3_PCI_DEVICE)
>>
>> +#define TYPE_PIIX3_DEVICE "PIIX3"
>> +#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
>> +#define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
>
>This naming seems to go back to 9b74b190d6c so it's not directly related to
>this series but there seems to be a bit of a confusion here that I wonder
>could be cleaned up now. We have:
>
>#define TYPE_PIIX3_PCI_DEVICE "pci-piix3"
>and
>#define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
>
>and both of these have mismatched #define and device name. These are not user
>creatable so so the device names don't appear anywhere (except maybe in info
>qtree output) but this could still be confusing as normally type defines
>should match device names. If these are the ISA bridges then maybe piix?-isa
>is a good name so maybe we should have
>
>#define TYPE_PIIX3_ISA "piix3-isa"
>#defint TYPE_PIIX4_ISA "piix4-isa"
>
>(then also matching e.g. "via-isa") but I'm not sure changing "pci-piix3" to
>"piix3-isa" could break anything (I don't expect changing the defines
>themselces could cause any problem).
>
>It's just something I've noticed and brought up for consideration but I have
>no strong opinion on it so up to you what you do with it.
Hi Zoltan,
yeah, I noticed the naming when moving the defines. I couldn't come up
with better names which were worth the effort because I can imagine
where the names are coming from. I also didn't want to go down the
rabbithole of backwards compatibility (which is a topic I haven't
covered yet). I think that *-isa is too narrow a name for the
container device since it bridges a couple of buses to PCI, but other
than that I don't have strong opinions about the names.
Best regards,
Bernhard
>
>Regards,
>BALATON Zoltan
>
>> +
>> PIIX3State *piix3_create(PCIBus *pci_bus, ISABus **isa_bus);
>>
>> DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus);
>>
- [PATCH v2 0/6] QOM'ify PIIX southbridge creation, Bernhard Beschow, 2022/05/22
- [PATCH v2 2/6] hw/isa/piix{3, 4}: Move pci_map_irq_fn's near pci_set_irq_fn's, Bernhard Beschow, 2022/05/22
- [PATCH v2 3/6] hw/isa/piix{3, 4}: QOM'ify PCI device creation and wiring, Bernhard Beschow, 2022/05/22
- [PATCH v2 4/6] hw/isa/piix{3, 4}: Factor out ISABus retrieval from create() functions, Bernhard Beschow, 2022/05/22
- [PATCH v2 6/6] hw/isa/piix{3, 4}: Inline and remove create() functions, Bernhard Beschow, 2022/05/22
- [PATCH v2 5/6] hw/isa/piix4: QOM'ify PIIX4 PM creation, Bernhard Beschow, 2022/05/22
- Re: [PATCH v2 0/6] QOM'ify PIIX southbridge creation, Philippe Mathieu-Daudé, 2022/05/22