On Thu, Jan 16, 2025 at 04:01:08PM +0800, zhenglifeng (A) wrote:
> > @@ -399,6 +399,7 @@ static int shmem_init_perf(struct amd_cpudata *cpudata)
> > {
> > struct cppc_perf_caps cppc_perf;
> > u64 numerator;
> > + bool auto_sel; <--- Not needed.
> >
> > int ret = cppc_get_perf_caps(cpudata->cpu, &cppc_perf);
> > if (ret)
> > @@ -420,7 +421,7 @@ static int shmem_init_perf(struct amd_cpudata *cpudata)
> > if (cppc_state == AMD_PSTATE_ACTIVE)
> > return 0;
> >
> > - ret = cppc_get_auto_sel_caps(cpudata->cpu, &cppc_perf); <--- Not needed.
> > + ret = cppc_get_auto_sel(cpudata->cpu, &auto_sel); <--- Not needed.
> > if (ret) { <--- Not needed.
> > pr_warn("failed to get auto_sel, ret: %d\n", ret); <--- Not needed.
> >
>
> If auto_sel is not supported, this function will return 0 after getting
> fail. But after removing cppc_get_auto_sel(), this function will return
> -EOPNOTSUPP by setting. Is this alright?
This is not ok. The shmem_init_perf() function shouldn't error out if
the auto-selection register is not supported.
Also, looking at the function, there may be a few additional cleanups
required in that one. For now, let us just retain the
cppc_get_auto_sel() and cppc_set_auto_sel() functions as you have done
in this patchset.
--
Thanks and Regards
gautham.
