Re: [PATCH v5 7/8] ppc/pnv: Add a Power11 Pnv11Chip, and a Power11 Machine

[Aditya Gupta <adityag@linux.ibm.com>]

Hi Cedric,

On 09/03/25 19:38, Cédric Le Goater wrote:
> On 3/8/25 21:51, Aditya Gupta wrote:
>> <...snip...>
>>
>>   static void pnv_chip_power8_instance_init(Object *obj)
>>   {
>>       Pnv8Chip *chip8 = PNV8_CHIP(obj);
>> @@ -1966,6 +1996,20 @@ static void 
>> pnv_chip_power10_instance_init(Object *obj)
>>       PnvChipClass *pcc = PNV_CHIP_GET_CLASS(obj);
>>       int i;
>>   +    /*
>> +     * Power11 declares Power10 as it's parent class, to be able to 
>> reuse
>> +     * most of the Power10 code.
>> +     * But this causes Power10 and Power11's both instance init to be
>> +     * called for PowerNV11 chip
>> +     *
>> +     * Skip initialising Power10 specific child objects, if the chip is
>> +     * Power11 chip, in which case power11's instance init will 
>> initialise
>> +     * the child objects
>> +     */
>> +    if (!strcmp(object_get_typename(obj), TYPE_PNV_CHIP_POWER11)) {
>> +        return;
>> +    }
>> +
>
> This is a hack !  Please duplicate the code like done for other chips.
>
>
>> <...snip...>
>>
>>   +    /*
>> +     * P11 chip and variants
>> +     */
>> +    {
>> +        .name          = TYPE_PNV11_CHIP,
>> +        .parent        = TYPE_PNV10_CHIP,
>
> why is the parent not TYPE_PNV_CHIP like the other chips ?
>
> This is a hack which can be easily avoided with more work :)


When I do this, every function using Pnv10Chip* will need to be 
duplicated to use Pnv11Chip*.

I will do that.

But what do you say about changing Pnv10 chip to Pnv1x chip ? It may 
only be used by Pnv10 and Pnv11, not future chips. But is that 
unnecessary for just two versions of the chip ?

Just tried to think of some ways to reduce duplication, or having 
different files such as pnv10.c, pnv11.c ? I am not sure because these 
might be unnecessary things too.


Thanks,

- Aditya G