Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 20 Jul 2020 10:00:57 +1000
From:   Nicholas Piggin <npiggin@gmail.com>
Subject: Re: [PATCH v3 1/3] powerpc/powernv/idle: Replace CPU features checks
 with PVR checks
To:     benh@kernel.crashing.org, ego@linux.vnet.ibm.com,
        linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
        mikey@neuling.org, mpe@ellerman.id.au, paulus@samba.org,
        pratik.r.sampat@gmail.com,
        Pratik Rajesh Sampat <psampat@linux.ibm.com>,
        svaidy@linux.ibm.com
References: <20200717185306.60607-1-psampat@linux.ibm.com>
        <20200717185306.60607-2-psampat@linux.ibm.com>
In-Reply-To: <20200717185306.60607-2-psampat@linux.ibm.com>
MIME-Version: 1.0
Message-Id: <1595203067.oropk0x5c8.astroid@bobo.none>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Excerpts from Pratik Rajesh Sampat's message of July 18, 2020 4:53 am:
> As the idle framework's architecture is incomplete, hence instead of
> checking for just the processor type advertised in the device tree CPU
> features; check for the Processor Version Register (PVR) so that finer
> granularity can be leveraged while making processor checks.
>=20
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
> ---
>  arch/powerpc/platforms/powernv/idle.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>=20
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platfor=
ms/powernv/idle.c
> index 2dd467383a88..f62904f70fc6 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -92,7 +92,7 @@ static int pnv_save_sprs_for_deep_states(void)
>  		if (rc !=3D 0)
>  			return rc;
> =20
> -		if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> +		if (pvr_version_is(PVR_POWER9)) {
>  			rc =3D opal_slw_set_reg(pir, P9_STOP_SPR_MSR, msr_val);
>  			if (rc)
>  				return rc;
> @@ -116,7 +116,7 @@ static int pnv_save_sprs_for_deep_states(void)
>  				return rc;
> =20
>  			/* Only p8 needs to set extra HID regiters */
> -			if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
> +			if (!pvr_version_is(PVR_POWER9)) {
> =20
>  				rc =3D opal_slw_set_reg(pir, SPRN_HID1, hid1_val);
>  				if (rc !=3D 0)

What I think you should do is keep using CPU_FTR_ARCH_300 for this stuff=20
which is written for power9 and we know is running on power9, because=20
that's a faster test (static branch and does not have to read PVR. And
then...

> @@ -1205,7 +1205,7 @@ static void __init pnv_probe_idle_states(void)
>  		return;
>  	}
> =20
> -	if (cpu_has_feature(CPU_FTR_ARCH_300))
> +	if (pvr_version_is(PVR_POWER9))
>  		pnv_power9_idle_init();
> =20
>  	for (i =3D 0; i < nr_pnv_idle_states; i++)

Here is where you would put the version check. Once we have code that=20
can also handle P10 (either by testing CPU_FTR_ARCH_31, or by adding
an entirely new power10 idle function), then you can add the P10 version
check here.

Thanks,
Nick