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

[Aditya Gupta]

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