AMD General The "len += sysfs_emit_at(...)" is a common practice in the Linux kernel. Its return value has no adverse impact on subsequent calls, so repeatedly checking the return value serves no practical purpose. You may refer to other usages of sysfs_emit_at() in the kernel source; none of the existing examples check its return value. (this may be a widely adopted convention).
Cc @Koenig, Christian Best Regards, Kevin > -----Original Message----- > From: Kamal, Asad <[email protected]> > Sent: Friday, May 29, 2026 14:19 > To: [email protected] > Cc: Lazar, Lijo <[email protected]>; Zhang, Hawking > <[email protected]>; Ma, Le <[email protected]>; Zhang, Morris > <[email protected]>; Deucher, Alexander <[email protected]>; > Wang, Yang(Kevin) <[email protected]>; Kamal, Asad > <[email protected]> > Subject: [PATCH 1/4] drm/amd/pm: Handle truncation in common clk/pcie printers > > In smu_cmn_print_dpm_clk_levels() and smu_cmn_print_pcie_levels(), use the > sysfs_emit_at() return value, break out of per-level loops when n == 0, and > use a > shared out label before updating offset. > > Signed-off-by: Asad Kamal <[email protected]> > --- > drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c | 77 +++++++++++++++----------- > 1 file changed, 44 insertions(+), 33 deletions(-) > > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c > b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c > index 546e64e3ba9c..0a745afa8552 100644 > --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c > @@ -1384,7 +1384,7 @@ int smu_cmn_print_dpm_clk_levels(struct smu_context > *smu, { > uint32_t min_clk, max_clk, level_index, count; > uint32_t freq_values[3]; > - int size, lvl, i; > + int size, lvl, i, n; > bool is_fine_grained; > bool is_deep_sleep; > bool freq_match; > @@ -1402,7 +1402,10 @@ int smu_cmn_print_dpm_clk_levels(struct smu_context > *smu, > /* Deep sleep - current clock < min_clock/2, TBD: cur_clk = 0 as GFXOFF > */ > is_deep_sleep = cur_clk < min_clk / 2; > if (is_deep_sleep) { > - size += sysfs_emit_at(buf, size, "S: %uMhz *\n", cur_clk); > + n = sysfs_emit_at(buf, size, "S: %uMhz *\n", cur_clk); > + if (!n) > + goto out; > + size += n; > level_index = 1; > } > > @@ -1412,10 +1415,13 @@ int smu_cmn_print_dpm_clk_levels(struct > smu_context *smu, > smu_cmn_freqs_match( > cur_clk, > dpm_table->dpm_levels[i].value); > - size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", > - level_index + i, > - dpm_table->dpm_levels[i].value, > - freq_match ? "*" : ""); > + n = sysfs_emit_at(buf, size, "%d: %uMhz %s\n", > + level_index + i, > + dpm_table->dpm_levels[i].value, > + freq_match ? "*" : ""); > + if (!n) > + break; > + size += n; > } > } else { > count = 2; > @@ -1437,13 +1443,16 @@ int smu_cmn_print_dpm_clk_levels(struct > smu_context *smu, > } > > for (i = 0; i < count; i++) { > - size += sysfs_emit_at( > - buf, size, "%d: %uMhz %s\n", level_index + i, > - freq_values[i], > - (!is_deep_sleep && i == lvl) ? "*" : ""); > + n = sysfs_emit_at(buf, size, "%d: %uMhz %s\n", > + level_index + i, freq_values[i], > + (!is_deep_sleep && i == lvl) ? "*" : > ""); > + if (!n) > + break; > + size += n; > } > } > > +out: > *offset = size; > > return 0; > @@ -1454,7 +1463,7 @@ int smu_cmn_print_pcie_levels(struct smu_context *smu, > uint32_t cur_gen, uint32_t cur_lane, char *buf, > int *offset) > { > - int size, i; > + int size, i, n; > > if (!pcie_table || !buf) > return -EINVAL; > @@ -1462,28 +1471,30 @@ int smu_cmn_print_pcie_levels(struct smu_context > *smu, > size = *offset; > > for (i = 0; i < pcie_table->lclk_levels; i++) { > - size += sysfs_emit_at( > - buf, size, "%d: %s %s %dMhz %s\n", i, > - (pcie_table->pcie_gen[i] == 0) ? "2.5GT/s," : > - (pcie_table->pcie_gen[i] == 1) ? "5.0GT/s," : > - (pcie_table->pcie_gen[i] == 2) ? "8.0GT/s," : > - (pcie_table->pcie_gen[i] == 3) ? "16.0GT/s," : > - (pcie_table->pcie_gen[i] == 4) ? "32.0GT/s," : > - (pcie_table->pcie_gen[i] == 5) ? "64.0GT/s," : > - "", > - (pcie_table->pcie_lane[i] == 1) ? "x1" : > - (pcie_table->pcie_lane[i] == 2) ? "x2" : > - (pcie_table->pcie_lane[i] == 3) ? "x4" : > - (pcie_table->pcie_lane[i] == 4) ? "x8" : > - (pcie_table->pcie_lane[i] == 5) ? "x12" : > - (pcie_table->pcie_lane[i] == 6) ? "x16" : > - (pcie_table->pcie_lane[i] == 7) ? "x32" : > - "", > - pcie_table->lclk_freq[i], > - (cur_gen == pcie_table->pcie_gen[i]) && > - (cur_lane == pcie_table->pcie_lane[i]) ? > - "*" : > - ""); > + n = sysfs_emit_at(buf, size, "%d: %s %s %dMhz %s\n", i, > + (pcie_table->pcie_gen[i] == 0) ? "2.5GT/s," : > + (pcie_table->pcie_gen[i] == 1) ? "5.0GT/s," : > + (pcie_table->pcie_gen[i] == 2) ? "8.0GT/s," : > + (pcie_table->pcie_gen[i] == 3) ? "16.0GT/s," : > + (pcie_table->pcie_gen[i] == 4) ? "32.0GT/s," : > + (pcie_table->pcie_gen[i] == 5) ? "64.0GT/s," : > + "", > + (pcie_table->pcie_lane[i] == 1) ? "x1" : > + (pcie_table->pcie_lane[i] == 2) ? "x2" : > + (pcie_table->pcie_lane[i] == 3) ? "x4" : > + (pcie_table->pcie_lane[i] == 4) ? "x8" : > + (pcie_table->pcie_lane[i] == 5) ? "x12" : > + (pcie_table->pcie_lane[i] == 6) ? "x16" : > + (pcie_table->pcie_lane[i] == 7) ? "x32" : > + "", > + pcie_table->lclk_freq[i], > + (cur_gen == pcie_table->pcie_gen[i]) && > + (cur_lane == pcie_table->pcie_lane[i]) ? > + "*" : > + ""); > + if (!n) > + break; > + size += n; > } > > *offset = size; > -- > 2.46.0
