[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 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, Lijo > That 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; >> } >
