[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 08/30] ide/ich: QOM Upcast Sweep
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH v2 08/30] ide/ich: QOM Upcast Sweep |
Date: |
Mon, 01 Jul 2013 12:03:57 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6 |
Am 01.07.2013 01:36, schrieb Alexander Graf:
>
> On 30.06.2013, at 10:21, Andreas Färber wrote:
>
>> Am 24.06.2013 08:55, schrieb address@hidden:
>>> From: Peter Crosthwaite <address@hidden>
>>>
>>> Define and use standard QOM cast macro. Remove usages of DO_UPCAST
>>> and direct -> style upcasting.
>>>
>>> Signed-off-by: Peter Crosthwaite <address@hidden>
>>> ---
>>>
>>> hw/ide/ahci.h | 5 +++++
>>> hw/ide/ich.c | 10 +++++-----
>>> 2 files changed, 10 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
>>> index 341a571..916bef0 100644
>>> --- a/hw/ide/ahci.h
>>> +++ b/hw/ide/ahci.h
>>> @@ -305,6 +305,11 @@ typedef struct AHCIPCIState {
>>> AHCIState ahci;
>>> } AHCIPCIState;
>>>
>>> +#define TYPE_ICH_AHCI "ich9-ahci"
>>
>> Let's be as precise as for the LSI SCSI HBA and name this ICH9. :)
>
> No, please. ICH9 is a controller hub. This device really is only about the
> AHCI part of it.
>
>>
>>> +
>>> +#define ICH_AHCI(obj) \
>>> + OBJECT_CHECK(AHCIPCIState, (obj), TYPE_ICH_AHCI)
>>
>> Wondering if this is specific to ICH(9)? Alex?
>> Leaving it as is for now, renaming is an easy follow-up.
>
> This is not an ICH9 device. It's an ICH9-AHCI device. And for that the check
> looks sane from what I can tell.
I don't see how either of your answers relate to my question? Maybe you
were just tired? ;)
So as you can see above, the type is called ich9-ahci, therefore I have
done s/TYPE_ICH_AHCI/TYPE_ICH9_AHCI/g, and I still don't see anything
wrong with it, Peter just ack'ed it. There's a link to the modified
patch below to verify.
What does a controller hub vs. AHCI have to do with ICH vs. ICH9?
The implied question really is, how specific is AHCIPCIState to
ich9-ahci (its sole user AFAICS)? Would an ich6-ahci or ich10-ahci use
the same struct or not? Is it even specific to ICH? If not, we might
want to name the cast macro after the struct PCI_AHCI() rather than
ICH_AHCI(). Compare EHCI where we have a base struct and one derived
class having its own extended state/class structs.
Andreas
>>> +
>>> extern const VMStateDescription vmstate_ahci;
>>>
>>> #define VMSTATE_AHCI(_field, _state) { \
>>> diff --git a/hw/ide/ich.c b/hw/ide/ich.c
>>> index 6c0c0c2..c3cbf2a 100644
>>> --- a/hw/ide/ich.c
>>> +++ b/hw/ide/ich.c
>>> @@ -92,7 +92,7 @@ static const VMStateDescription vmstate_ich9_ahci = {
>>>
>>> static void pci_ich9_reset(DeviceState *dev)
>>> {
>>> - struct AHCIPCIState *d = DO_UPCAST(struct AHCIPCIState, card.qdev,
>>> dev);
>>> + struct AHCIPCIState *d = ICH_AHCI(dev);
>>
>> Let's drop the "struct" while touching the line.
>>
>> Thanks, applied to qom-next:
>> https://github.com/afaerber/qemu-cpu/commits/qom-next
>>
>> Andreas
>>
>>>
>>> ahci_reset(&d->ahci);
>>> }
>>> @@ -102,9 +102,9 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
>>> struct AHCIPCIState *d;
>>> int sata_cap_offset;
>>> uint8_t *sata_cap;
>>> - d = DO_UPCAST(struct AHCIPCIState, card, dev);
>>> + d = ICH_AHCI(dev);
>>>
>>> - ahci_init(&d->ahci, &dev->qdev, pci_get_address_space(dev), 6);
>>> + ahci_init(&d->ahci, DEVICE(dev), pci_get_address_space(dev), 6);
>>>
>>> pci_config_set_prog_interface(d->card.config,
>>> AHCI_PROGMODE_MAJOR_REV_1);
>>>
>>> @@ -141,7 +141,7 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
>>> static void pci_ich9_uninit(PCIDevice *dev)
>>> {
>>> struct AHCIPCIState *d;
>>> - d = DO_UPCAST(struct AHCIPCIState, card, dev);
>>> + d = ICH_AHCI(dev);
>>>
>>> msi_uninit(dev);
>>> ahci_uninit(&d->ahci);
>>> @@ -163,7 +163,7 @@ static void ich_ahci_class_init(ObjectClass *klass,
>>> void *data)
>>> }
>>>
>>> static const TypeInfo ich_ahci_info = {
>>> - .name = "ich9-ahci",
>>> + .name = TYPE_ICH_AHCI,
>>> .parent = TYPE_PCI_DEVICE,
>>> .instance_size = sizeof(AHCIPCIState),
>>> .class_init = ich_ahci_class_init,
>>>
>>
>>
>> --
>> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
- Re: [Qemu-devel] [PATCH v2 08/30] ide/ich: QOM Upcast Sweep,
Andreas Färber <=