Hi,
On 8/22/22 08:29, Shyam Sundar S K wrote:
> variable 'mode' is used uninitialized whenever switch default is taken
> in sps.c which leads to the following clang warning.
>
> ---
> drivers/platform/x86/amd/pmf/sps.c:96:2: error: variable 'mode' is used uninitialized whenever switch default is taken [-Werror,-Wsometimes-uninitialized]
> default:
> ^~~~~~~
> drivers/platform/x86/amd/pmf/sps.c:101:9: note: uninitialized use occurs here
> return mode;
> ^~~~
> drivers/platform/x86/amd/pmf/sps.c:84:9: note: initialize the variable 'mode' to silence this warning
> u8 mode;
> ^
> = '\0'
> 1 error generated.
> ---
>
> Fix it by returning -EOPNOTSUPP in default case and also change the return
> type of the function amd_pmf_get_pprof_modes() to keep it similar like
> other drivers which implement platform_profile.
>
> Fixes: 4c71ae414474 ("platform/x86/amd/pmf: Add support SPS PMF feature")
> Reported-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> Reviewed-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
Thank you for your patch, I've applied this patch to
the platform-drivers-x86-amd-pmf branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=platform-drivers-x86-amd-pmf
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
> ---
> v2:
> - Handle return codes for all amd_pmf_get_pprof_modes() callers.
>
> drivers/platform/x86/amd/pmf/auto-mode.c | 8 ++++++--
> drivers/platform/x86/amd/pmf/core.c | 3 +++
> drivers/platform/x86/amd/pmf/pmf.h | 4 ++--
> drivers/platform/x86/amd/pmf/sps.c | 11 +++++++----
> 4 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/auto-mode.c b/drivers/platform/x86/amd/pmf/auto-mode.c
> index 368964d885a2..644af42e07cf 100644
> --- a/drivers/platform/x86/amd/pmf/auto-mode.c
> +++ b/drivers/platform/x86/amd/pmf/auto-mode.c
> @@ -264,7 +264,7 @@ static void amd_pmf_load_defaults_auto_mode(struct amd_pmf_dev *dev)
> dev->socket_power_history_idx = -1;
> }
>
> -void amd_pmf_reset_amt(struct amd_pmf_dev *dev)
> +int amd_pmf_reset_amt(struct amd_pmf_dev *dev)
> {
> /*
> * OEM BIOS implementation guide says that if the auto mode is enabled
> @@ -275,11 +275,15 @@ void amd_pmf_reset_amt(struct amd_pmf_dev *dev)
> */
>
> if (is_apmf_func_supported(dev, APMF_FUNC_STATIC_SLIDER_GRANULAR)) {
> - u8 mode = amd_pmf_get_pprof_modes(dev);
> + int mode = amd_pmf_get_pprof_modes(dev);
> +
> + if (mode < 0)
> + return mode;
>
> dev_dbg(dev->dev, "resetting AMT thermals\n");
> amd_pmf_update_slider(dev, SLIDER_OP_SET, mode, NULL);
> }
> + return 0;
> }
>
> void amd_pmf_handle_amt(struct amd_pmf_dev *dev)
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index e46d63aa51b8..a675ca969331 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -65,6 +65,9 @@ static int current_power_limits_show(struct seq_file *seq, void *unused)
> int mode, src = 0;
>
> mode = amd_pmf_get_pprof_modes(dev);
> + if (mode < 0)
> + return mode;
> +
> src = amd_pmf_get_power_source();
> amd_pmf_update_slider(dev, SLIDER_OP_GET, mode, &table);
> seq_printf(seq, "spl:%u fppt:%u sppt:%u sppt_apu_only:%u stt_min:%u stt[APU]:%u stt[HS2]: %u\n",
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 7613ed2ef6e3..0a72a395c2ef 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -303,7 +303,7 @@ int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev);
> int amd_pmf_get_power_source(void);
>
> /* SPS Layer */
> -u8 amd_pmf_get_pprof_modes(struct amd_pmf_dev *pmf);
> +int amd_pmf_get_pprof_modes(struct amd_pmf_dev *pmf);
> void amd_pmf_update_slider(struct amd_pmf_dev *dev, bool op, int idx,
> struct amd_pmf_static_slider_granular *table);
> int amd_pmf_init_sps(struct amd_pmf_dev *dev);
> @@ -322,6 +322,6 @@ void amd_pmf_trans_automode(struct amd_pmf_dev *dev, int socket_power, ktime_t t
> int apmf_get_sbios_requests(struct amd_pmf_dev *pdev, struct apmf_sbios_req *req);
>
> void amd_pmf_update_2_cql(struct amd_pmf_dev *dev, bool is_cql_event);
> -void amd_pmf_reset_amt(struct amd_pmf_dev *dev);
> +int amd_pmf_reset_amt(struct amd_pmf_dev *dev);
> void amd_pmf_handle_amt(struct amd_pmf_dev *dev);
> #endif /* PMF_H */
> diff --git a/drivers/platform/x86/amd/pmf/sps.c b/drivers/platform/x86/amd/pmf/sps.c
> index 8923e29cc6ca..dba7e36962dc 100644
> --- a/drivers/platform/x86/amd/pmf/sps.c
> +++ b/drivers/platform/x86/amd/pmf/sps.c
> @@ -79,9 +79,9 @@ static int amd_pmf_profile_get(struct platform_profile_handler *pprof,
> return 0;
> }
>
> -u8 amd_pmf_get_pprof_modes(struct amd_pmf_dev *pmf)
> +int amd_pmf_get_pprof_modes(struct amd_pmf_dev *pmf)
> {
> - u8 mode;
> + int mode;
>
> switch (pmf->current_profile) {
> case PLATFORM_PROFILE_PERFORMANCE:
> @@ -95,7 +95,7 @@ u8 amd_pmf_get_pprof_modes(struct amd_pmf_dev *pmf)
> break;
> default:
> dev_err(pmf->dev, "Unknown Platform Profile.\n");
> - break;
> + return -EOPNOTSUPP;
> }
>
> return mode;
> @@ -105,10 +105,13 @@ static int amd_pmf_profile_set(struct platform_profile_handler *pprof,
> enum platform_profile_option profile)
> {
> struct amd_pmf_dev *pmf = container_of(pprof, struct amd_pmf_dev, pprof);
> - u8 mode;
> + int mode;
>
> pmf->current_profile = profile;
> mode = amd_pmf_get_pprof_modes(pmf);
> + if (mode < 0)
> + return mode;
> +
> amd_pmf_update_slider(pmf, SLIDER_OP_SET, mode, NULL);
> return 0;
> }
