Received-SPF: pass (google.com: domain of linux-kernel+bounces-145405-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Mon, 15 Apr 2024 22:59:14 +0800
From: Huang Rui <ray.huang@amd.com>
To: "Yuan, Perry" <Perry.Yuan@amd.com>
Cc: "rafael.j.wysocki@intel.com" <rafael.j.wysocki@intel.com>,
	"Limonciello, Mario" <Mario.Limonciello@amd.com>,
	"viresh.kumar@linaro.org" <viresh.kumar@linaro.org>,
	"Shenoy, Gautham Ranjal" <gautham.shenoy@amd.com>,
	"Petkov, Borislav" <Borislav.Petkov@amd.com>,
	"Deucher, Alexander" <Alexander.Deucher@amd.com>,
	"Huang, Shimmer" <Shimmer.Huang@amd.com>,
	"oleksandr@natalenko.name" <oleksandr@natalenko.name>,
	"Du, Xiaojian" <Xiaojian.Du@amd.com>,
	"Meng, Li (Jassmine)" <Li.Meng@amd.com>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v10 5/8] cpufreq: amd-pstate: Bail out if
 min/max/nominal_freq is 0
Message-ID: <Zh1AwtnewmweX3zr@amd.com>
References: <cover.1711335714.git.perry.yuan@amd.com>
 <c2809391c877dd5842389aaf87bf2b5fce5dc866.1711335714.git.perry.yuan@amd.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <c2809391c877dd5842389aaf87bf2b5fce5dc866.1711335714.git.perry.yuan@amd.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?NdKtTBljv7zQ2IJwjOTN6O9GjUhQQc5WVdGG9sx2RsnwKYMV7/vmyxH01/vI?=
 =?us-ascii?Q?FOoKLBi1BvT4g8Yv8WUn8NuAG40UIGko4zGzEqdfe1CJ4C9id4DQ2v64WZKA?=
 =?us-ascii?Q?gPex1L9QQRDVPI1w7uJy8uqdMfjizAd/vzvcObTo20pXR8zoROGJiwOBm+t+?=
 =?us-ascii?Q?/H0cB2wdvJrr01WilmcEgekXqn3Gqdsvs/TsMPd6f3bTimJ4LvhjRqTrVPZk?=
 =?us-ascii?Q?xFH75kBJ62uZCQuMuab1UD/EKd2W4HO5C+IQylA7dWAEFC94JilJ8wMW2XvX?=
 =?us-ascii?Q?kMwuDJLduZixx9eeeVc6UFBlQZYyvzfUgQbur6sD2dvsVpMP0VKKLTR9izzq?=
 =?us-ascii?Q?ZwrRf7b/HUyQ22cQwaFyE7hxcHzTtbBeEzZvnZW2zZ9u6dGNjC99/W3Bj65O?=
 =?us-ascii?Q?XNrhu044FJG9UExBeRf2TYmakq5A1TWIhK9Hk9NHc3HFNp2MyYgK9d0f9cug?=
 =?us-ascii?Q?peQacdjHjKj68639Oki6xXcFQ/l8bpBxyLWrTtaqfyhqs/z27P9JBxoHe20T?=
 =?us-ascii?Q?iH427j6cY36YdG5/BR0GhtjTZXnWZzRMgc/lAuAcex9ObCFBRNBFlNuKr+A7?=
 =?us-ascii?Q?rZdR+4YWdRBe9qhHUzhZWPu2pCsVSKnCG0p/vJTi8bFBAgBN5ce2hDt5s7o3?=
 =?us-ascii?Q?+fDr10iqGI60osV9fWQbSB3QrM3+PxjZDcQJeNNjFDm6q7L/5ow0Pk25/HAI?=
 =?us-ascii?Q?ICune0qwumKzKymkrIURAZZWQkXS0ERcPYDzmp9c0Thkhd1QAS+fPWhlSWbu?=
 =?us-ascii?Q?kX+tTRKN0GcmT7j1SEJtOPNA/isRC9DSK2zuVL5JhtN+M0SC9S34oZ95NJ1a?=
 =?us-ascii?Q?OpuyefZ8GVCcIMc03dcZnR4ijnVdrRwhaR9LG1Oa7g666k/qEGLvYgWCAWPD?=
 =?us-ascii?Q?J6WtR5E7ihwVaF8MeYVnWFFLR27DTwJUJKu/nMOBJQ+/hRqW98TJmtedju4T?=
 =?us-ascii?Q?cdlRuLnCfW1s1c7gitxxd5MwBVBDQwDWa7tCxNmtq2tGG6acEvTq2BYgTpmt?=
 =?us-ascii?Q?QAuJQz/b7mn+kjcgHloydDWatuZz+NFNykLuwlmHYV5dblgr0TM9oQ6t1Nmn?=
 =?us-ascii?Q?jF7kw+4km1lPtTciYNgQrrWm+E875EaZxGWwy0zuT8HadPl1qkypNr7ba+4p?=
 =?us-ascii?Q?a+TjNUiHSGt5McmnESTfl2gj4fExQf0bFzna4t62ZOsJ3h0N5aTiZW5WYzkd?=
 =?us-ascii?Q?6JKonZFKtkx7jD1yDm8oHkj9fDgfjNaC5PAyuw5egvZMwdhhVdXl5uqFlNOn?=
 =?us-ascii?Q?QFpzognV3caT2fcmoYUzSnJU91lTTTVRE6n94wgEBEedSDm5p53m9fyzmcx4?=
 =?us-ascii?Q?eOqtnLXLDJfezTsEW4DUKc3BMBCFTsR3c03slgDmteZ4yOHZmZE4tRAOUfv8?=
 =?us-ascii?Q?esT2vbljgfG7WPGDlr4RH2+rSpkLZ3fratBR2l4tRRIVbW5Fl04KpnPwaa3j?=
 =?us-ascii?Q?a1+aJq8vR48yQXjWiPuzGkb1MV67wI3wPO+Nj3qUuKlDw7poYq3/T8XUNXBV?=
 =?us-ascii?Q?0oB5fV7lbBa8ccWGcz1Xp5x3Iqp7Rbme4Bz7WI3QvADHtbb+oZmvzc1Qe26J?=
 =?us-ascii?Q?YL0QQD9tCFZ3/DcgAKCHHjN1MreeAFog5r9tunzb?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 07eef7e4-0122-46c1-4c73-08dc5d5caafa
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR12MB8690.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Apr 2024 14:59:36.8664
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 4O4i67jA9E6zHSBNRBTlfawZtYzMWrrhLawSDjv/DWX18ne3q+dEpjXhJh9w7wTIQkvQH+mctc1iSSCp2HqvrA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR12MB6964

On Mon, Mar 25, 2024 at 11:03:25AM +0800, Yuan, Perry wrote:
> The amd-pstate driver cannot work when the min_freq, nominal_freq or
> the max_freq is zero. When this happens it is prudent to error out
> early on rather than waiting failing at the time of the governor
> initialization.
> 
> Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> Tested-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> ---
>  drivers/cpufreq/amd-pstate.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 132330b4942f..6708c436e1a2 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -839,9 +839,11 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
>  	nominal_freq = READ_ONCE(cpudata->nominal_freq);
>  	lowest_nonlinear_freq = READ_ONCE(cpudata->lowest_nonlinear_freq);
>  
> -	if (min_freq < 0 || max_freq < 0 || min_freq > max_freq) {
> -		dev_err(dev, "min_freq(%d) or max_freq(%d) value is incorrect\n",
> -			min_freq, max_freq);
> +	if (min_freq <= 0 || max_freq <= 0 ||
> +	    nominal_freq <= 0 || min_freq > max_freq) {
> +		dev_err(dev,
> +			"min_freq(%d) or max_freq(%d) or nominal_freq (%d) value is incorrect\n",
> +			min_freq, max_freq, nominal_freq);

I suggest that we add one comment to remind that should be the error of
ACPI table or BIOS.

>  		ret = -EINVAL;
>  		goto free_cpudata1;
>  	}
> @@ -1299,9 +1301,11 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
>  	max_freq = READ_ONCE(cpudata->max_freq);
>  	nominal_freq = READ_ONCE(cpudata->nominal_freq);
>  	lowest_nonlinear_freq = READ_ONCE(cpudata->lowest_nonlinear_freq);
> -	if (min_freq < 0 || max_freq < 0 || min_freq > max_freq) {
> -		dev_err(dev, "min_freq(%d) or max_freq(%d) value is incorrect\n",
> -				min_freq, max_freq);
> +	if (min_freq <= 0 || max_freq <= 0 ||
> +	    nominal_freq <= 0 || min_freq > max_freq) {
> +		dev_err(dev,
> +			"min_freq(%d) or max_freq(%d) or nominal_freq(%d) value is incorrect\n",
> +			min_freq, max_freq, nominal_freq);

The same with above.

With that fixed, patch is Acked-by: Huang Rui <ray.huang@amd.com>

Thanks,
Ray