On 10/27/2025 1:26 PM, Wang, Yang(Kevin) wrote:
[AMD Official Use Only - AMD Internal Distribution Only]Probably the fix needs to be in smu_v13_0_6_print_clks?I would like to keep the current code design where there are centralized exit points for called functions to match the kernel coding style requirement.. https://www.kernel.org/doc/html/v6.16/process/coding-style.html 7) Centralized exiting of functions
Not seeing a case for cleanup or multiple unwind steps in common exit point.
Anyway, if you prefer to keep this style, below one also needs a fix inside smu_v13_0_6_print_clks()
if (curr_clk < SMU_13_0_6_DSCLK_THRESHOLD) {
size = sysfs_emit_at(buf, size, "S: %uMhz *\n", curr_clk);
Thanks,
Lijo
Best Regards, Kevin -----Original Message----- From: Lazar, Lijo <[email protected]> Sent: Monday, October 27, 2025 15:46 To: Wang, Yang(Kevin) <[email protected]>; [email protected] Cc: Zhang, Hawking <[email protected]>; Deucher, Alexander <[email protected]>; Kamal, Asad <[email protected]> Subject: Re: [PATCH] drm/amd/pm: Fix incomplete null pointer issue for smu v13.0.6 On 10/27/2025 1:13 PM, Lazar, Lijo wrote:On 10/27/2025 12:57 PM, Yang Wang wrote:the smu v13.0.6 driver should handle return value of smu_v13_0_6_print_clks() to avoid null pointer issue. Fixes: 0354cd650daa ("drm/amd/pm: Avoid writing nulls into `pp_od_clk_voltage`") Signed-off-by: Yang Wang <[email protected]> --- .../drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c | 41 ++++++++++++++----- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c b/ drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c index 39ae6701147c..22fe4d3508fd 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c @@ -1514,9 +1514,14 @@ static int smu_v13_0_6_print_clk_levels(struct smu_context *smu, single_dpm_table = &(dpm_context->dpm_tables.uclk_table); - return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table, - now, "mclk"); + ret = smu_v13_0_6_print_clks(smu, buf, size, +single_dpm_table, + now, "mclk");Probably the fix needs to be in smu_v13_0_6_print_clks? size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i, clk1, (level == i) ? "*" : ""); 'size += to size =' so that it returns only the total size emitted by the function.Never mind, this is not going to work. The function may return the total size it emitted, or it also needs to adjust the below condition. Thanks, LijoThat is the case for this condition now - if (curr_clk < SMU_13_0_6_DSCLK_THRESHOLD) Thanks, Lijo+ if (ret < 0) + return ret; + + size += ret; + break; case SMU_SOCCLK: ret = smu_v13_0_6_get_current_clk_freq_by_table(smu, SMU_SOCCLK, &now); @@ -1528,9 +1533,13 @@ static int smu_v13_0_6_print_clk_levels(struct smu_context *smu, single_dpm_table = &(dpm_context->dpm_tables.soc_table); - return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table, - now, "socclk"); + ret = smu_v13_0_6_print_clks(smu, buf, size, +single_dpm_table, + now, "socclk"); + if (ret < 0) + return ret; + size += ret; + break; case SMU_FCLK: ret = smu_v13_0_6_get_current_clk_freq_by_table(smu, SMU_FCLK, &now); @@ -1542,9 +1551,13 @@ static int smu_v13_0_6_print_clk_levels(struct smu_context *smu, single_dpm_table = &(dpm_context->dpm_tables.fclk_table); - return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table, - now, "fclk"); + ret = smu_v13_0_6_print_clks(smu, buf, size, +single_dpm_table, + now, "fclk"); + if (ret < 0) + return ret; + size += ret; + break; case SMU_VCLK: ret = smu_v13_0_6_get_current_clk_freq_by_table(smu, SMU_VCLK, &now); @@ -1556,9 +1569,13 @@ static int smu_v13_0_6_print_clk_levels(struct smu_context *smu, single_dpm_table = &(dpm_context->dpm_tables.vclk_table); - return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table, - now, "vclk"); + ret = smu_v13_0_6_print_clks(smu, buf, size, +single_dpm_table, + now, "vclk"); + if (ret < 0) + return ret; + size += ret; + break; case SMU_DCLK: ret = smu_v13_0_6_get_current_clk_freq_by_table(smu, SMU_DCLK, &now); @@ -1570,9 +1587,13 @@ static int smu_v13_0_6_print_clk_levels(struct smu_context *smu, single_dpm_table = &(dpm_context->dpm_tables.dclk_table); - return smu_v13_0_6_print_clks(smu, buf, size, single_dpm_table, - now, "dclk"); + ret = smu_v13_0_6_print_clks(smu, buf, size, +single_dpm_table, + now, "dclk"); + if (ret < 0) + return ret; + size += ret; + break; default: break; }
