Re: [PATCH 00/13] ppc/pnv: Get rid of chip_type attributes

[David Gibson]

On Fri, Dec 13, 2019 at 12:59:28PM +0100, Greg Kurz wrote:
> The PnvChipClass type has a chip_type attribute which identifies various
> POWER CPU chip types that can be used in a powernv machine.
> 
> typedef enum PnvChipType {
>     PNV_CHIP_POWER8E,     /* AKA Murano (default) */
>     PNV_CHIP_POWER8,      /* AKA Venice */
>     PNV_CHIP_POWER8NVL,   /* AKA Naples */
>     PNV_CHIP_POWER9,      /* AKA Nimbus */
>     PNV_CHIP_POWER10,     /* AKA TBD */
> } PnvChipType;
> 
> This attribute is used in many places where we want a different behaviour
> depending on the CPU type, either directly like:
> 
>     switch (PNV_CHIP_GET_CLASS(chip)->chip_type) {
>     case PNV_CHIP_POWER8E:
>     case PNV_CHIP_POWER8:
>     case PNV_CHIP_POWER8NVL:
>         return ((addr >> 4) & ~0xfull) | ((addr >> 3) & 0xf);
>     case PNV_CHIP_POWER9:
>     case PNV_CHIP_POWER10:
>         return addr >> 3;
>     default:
>         g_assert_not_reached();
>     }
> 
> or through various helpers that rely on it:
> 
>         /* Each core has an XSCOM MMIO region */
>         if (pnv_chip_is_power10(chip)) {
>             xscom_core_base = PNV10_XSCOM_EC_BASE(core_hwid);
>         } else if (pnv_chip_is_power9(chip)) {
>             xscom_core_base = PNV9_XSCOM_EC_BASE(core_hwid);
>         } else {
>             xscom_core_base = PNV_XSCOM_EX_BASE(core_hwid);
>         }
> 
> The chip_type is also duplicated in the PnvPsiClass type.
> 
> It looks a bit unfortunate to implement manually something that falls into
> the scope of QOM. Especially since we don't seem to need a finer grain than
> the CPU familly, ie. POWER8, POWER9, POWER10, ..., and we already have
> specialized versions of PnvChipClass and PnvPsiClass for these.
> 
> This series basically QOM-ifies all the places where we check on the chip
> type, and gets rid of the chip_type attributes and the is_powerXX() helpers.
> 
> Patch 1 was recently posted to the list but it isn't available in David's
> ppc-for-5.0 tree yet, so I include it in this series for convenience.

Applied to ppc-for-5.0.  I had been anticipating just replacing a
bunch of tests on ->chip_type with object_dynamic_cast() checks, but
this is better still.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature