Re: [Qemu-devel] [PATCH v2 08/30] ide/ich: QOM Upcast Sweep

[Alexander Graf]

On 01.07.2013, at 12:03, Andreas Färber wrote:

> 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? ;)

Ah, sorry. I thought you wanted to just name it "ICH" instead of "ich9-ahci".

> 
> 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.

ich9-ahci is-a pci-ahci is-a ahci makes sense I would say. Different 
controllers can have different numbers of connectors, but apart from that they 
should look pretty similar.


Alex