[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aH3LEmB3hafb3T8E@vaman>
Date: Mon, 21 Jul 2025 10:37:30 +0530
From: Vinod Koul <vkoul@...nel.org>
To: Suresh K C <suresh.k.chandrappa@...il.com>
Cc: yung-chuan.liao@...ux.intel.com, pierre-louis.bossart@...ux.dev,
sanyog.r.kale@...el.com, shuah@...nel.org,
linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] soundwire: replace scnprintf() with sysfs_emit() for
sysfs consistency
On 12-07-25, 10:13, Suresh K C wrote:
> From: Suresh K C <suresh.k.chandrappa@...il.com>
>
> Replace scnprintf() with sysfs_emit() or sysfs_emit_at() in SoundWire driver files
> to align with the guidelines outlined in Documentation/filesystems/sysfs.rst.
>
> This change improves the safety and correctness of sysfs attribute handling,
> ensures consistency across the kernel codebase, and simplifies future maintenance.
>
> Tested by enabling CONFIG_DEBUG_FS and confirming that /sys/kernel/debug/soundwire is correctly populated
There is trailing whitespace error, pls run checkpatch before
sending patches
>
> Signed-off-by: Suresh K C <suresh.k.chandrappa@...il.com>
> ---
> drivers/soundwire/cadence_master.c | 23 +++++++++++------------
> drivers/soundwire/debugfs.c | 22 +++++++++++-----------
> drivers/soundwire/intel.c | 17 ++++++++---------
> drivers/soundwire/intel_ace2x_debugfs.c | 14 +++++++-------
> 4 files changed, 37 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
> index 21bb491d026b..668a111c66c8 100644
> --- a/drivers/soundwire/cadence_master.c
> +++ b/drivers/soundwire/cadence_master.c
> @@ -319,8 +319,7 @@ EXPORT_SYMBOL(sdw_cdns_config_update_set_wait);
> static ssize_t cdns_sprintf(struct sdw_cdns *cdns,
> char *buf, size_t pos, unsigned int reg)
> {
> - return scnprintf(buf + pos, RD_BUF - pos,
> - "%4x\t%8x\n", reg, cdns_readl(cdns, reg));
> + return sysfs_emit_at(buf, pos,"%4x\t%8x\n", reg, cdns_readl(cdns, reg));
> }
>
> static int cdns_reg_show(struct seq_file *s, void *data)
> @@ -334,42 +333,42 @@ static int cdns_reg_show(struct seq_file *s, void *data)
> if (!buf)
> return -ENOMEM;
>
> - ret = scnprintf(buf, RD_BUF, "Register Value\n");
> - ret += scnprintf(buf + ret, RD_BUF - ret, "\nMCP Registers\n");
> + ret = sysfs_emit(buf, "Register Value\n");
> + ret += sysfs_emit_at(buf, ret,"\nMCP Registers\n");
> /* 8 MCP registers */
> for (i = CDNS_MCP_CONFIG; i <= CDNS_MCP_PHYCTRL; i += sizeof(u32))
> ret += cdns_sprintf(cdns, buf, ret, i);
>
> - ret += scnprintf(buf + ret, RD_BUF - ret,
> + ret += sysfs_emit_at(buf, ret,
> "\nStatus & Intr Registers\n");
> /* 13 Status & Intr registers (offsets 0x70 and 0x74 not defined) */
> for (i = CDNS_MCP_STAT; i <= CDNS_MCP_FIFOSTAT; i += sizeof(u32))
> ret += cdns_sprintf(cdns, buf, ret, i);
>
> - ret += scnprintf(buf + ret, RD_BUF - ret,
> + ret += sysfs_emit_at(buf, ret,
> "\nSSP & Clk ctrl Registers\n");
> ret += cdns_sprintf(cdns, buf, ret, CDNS_MCP_SSP_CTRL0);
> ret += cdns_sprintf(cdns, buf, ret, CDNS_MCP_SSP_CTRL1);
> ret += cdns_sprintf(cdns, buf, ret, CDNS_MCP_CLK_CTRL0);
> ret += cdns_sprintf(cdns, buf, ret, CDNS_MCP_CLK_CTRL1);
>
> - ret += scnprintf(buf + ret, RD_BUF - ret,
> + ret += sysfs_emit_at(buf, ret,
> "\nDPn B0 Registers\n");
>
> num_ports = cdns->num_ports;
>
> for (i = 0; i < num_ports; i++) {
> - ret += scnprintf(buf + ret, RD_BUF - ret,
> + ret += sysfs_emit_at(buf, ret,
> "\nDP-%d\n", i);
> for (j = CDNS_DPN_B0_CONFIG(i);
> j < CDNS_DPN_B0_ASYNC_CTRL(i); j += sizeof(u32))
> ret += cdns_sprintf(cdns, buf, ret, j);
> }
>
> - ret += scnprintf(buf + ret, RD_BUF - ret,
> + ret += sysfs_emit_at(buf, ret,
> "\nDPn B1 Registers\n");
> for (i = 0; i < num_ports; i++) {
> - ret += scnprintf(buf + ret, RD_BUF - ret,
> + ret += sysfs_emit_at(buf, ret,
> "\nDP-%d\n", i);
>
> for (j = CDNS_DPN_B1_CONFIG(i);
> @@ -377,13 +376,13 @@ static int cdns_reg_show(struct seq_file *s, void *data)
> ret += cdns_sprintf(cdns, buf, ret, j);
> }
>
> - ret += scnprintf(buf + ret, RD_BUF - ret,
> + ret += sysfs_emit_at(buf, ret,
> "\nDPn Control Registers\n");
> for (i = 0; i < num_ports; i++)
> ret += cdns_sprintf(cdns, buf, ret,
> CDNS_PORTCTRL + i * CDNS_PORT_OFFSET);
>
> - ret += scnprintf(buf + ret, RD_BUF - ret,
> + ret += sysfs_emit_at(buf, ret,
> "\nPDIn Config Registers\n");
>
> /* number of PDI and ports is interchangeable */
> diff --git a/drivers/soundwire/debugfs.c b/drivers/soundwire/debugfs.c
> index 3099ea074f10..ccc98f449658 100644
> --- a/drivers/soundwire/debugfs.c
> +++ b/drivers/soundwire/debugfs.c
> @@ -42,9 +42,9 @@ static ssize_t sdw_sprintf(struct sdw_slave *slave,
> value = sdw_read_no_pm(slave, reg);
>
> if (value < 0)
> - return scnprintf(buf + pos, RD_BUF - pos, "%3x\tXX\n", reg);
> + return sysfs_emit_at(buf , pos, "%3x\tXX\n", reg);
> else
> - return scnprintf(buf + pos, RD_BUF - pos,
> + return sysfs_emit_at(buf , pos,
> "%3x\t%2x\n", reg, value);
> }
>
> @@ -64,21 +64,21 @@ static int sdw_slave_reg_show(struct seq_file *s_file, void *data)
> return ret;
> }
>
> - ret = scnprintf(buf, RD_BUF, "Register Value\n");
> + ret = sysfs_emit(buf, "Register Value\n");
>
> /* DP0 non-banked registers */
> - ret += scnprintf(buf + ret, RD_BUF - ret, "\nDP0\n");
> + ret += sysfs_emit_at(buf , ret, "\nDP0\n");
> for (i = SDW_DP0_INT; i <= SDW_DP0_PREPARECTRL; i++)
> ret += sdw_sprintf(slave, buf, ret, i);
>
> /* DP0 Bank 0 registers */
> - ret += scnprintf(buf + ret, RD_BUF - ret, "Bank0\n");
> + ret += sysfs_emit_at(buf, ret, "Bank0\n");
> ret += sdw_sprintf(slave, buf, ret, SDW_DP0_CHANNELEN);
> for (i = SDW_DP0_SAMPLECTRL1; i <= SDW_DP0_LANECTRL; i++)
> ret += sdw_sprintf(slave, buf, ret, i);
>
> /* DP0 Bank 1 registers */
> - ret += scnprintf(buf + ret, RD_BUF - ret, "Bank1\n");
> + ret += sysfs_emit_at(buf, ret, "Bank1\n");
> ret += sdw_sprintf(slave, buf, ret,
> SDW_DP0_CHANNELEN + SDW_BANK1_OFFSET);
> for (i = SDW_DP0_SAMPLECTRL1 + SDW_BANK1_OFFSET;
> @@ -86,7 +86,7 @@ static int sdw_slave_reg_show(struct seq_file *s_file, void *data)
> ret += sdw_sprintf(slave, buf, ret, i);
>
> /* SCP registers */
> - ret += scnprintf(buf + ret, RD_BUF - ret, "\nSCP\n");
> + ret += sysfs_emit_at(buf, ret, "\nSCP\n");
> for (i = SDW_SCP_INT1; i <= SDW_SCP_BUS_CLOCK_BASE; i++)
> ret += sdw_sprintf(slave, buf, ret, i);
> for (i = SDW_SCP_DEVID_0; i <= SDW_SCP_DEVID_5; i++)
> @@ -110,18 +110,18 @@ static int sdw_slave_reg_show(struct seq_file *s_file, void *data)
> for (i = 1; SDW_VALID_PORT_RANGE(i); i++) {
>
> /* DPi registers */
> - ret += scnprintf(buf + ret, RD_BUF - ret, "\nDP%d\n", i);
> + ret += sysfs_emit_at(buf , ret, "\nDP%d\n", i);
> for (j = SDW_DPN_INT(i); j <= SDW_DPN_PREPARECTRL(i); j++)
> ret += sdw_sprintf(slave, buf, ret, j);
>
> /* DPi Bank0 registers */
> - ret += scnprintf(buf + ret, RD_BUF - ret, "Bank0\n");
> + ret += sysfs_emit_at(buf , ret, "Bank0\n");
> for (j = SDW_DPN_CHANNELEN_B0(i);
> j <= SDW_DPN_LANECTRL_B0(i); j++)
> ret += sdw_sprintf(slave, buf, ret, j);
>
> /* DPi Bank1 registers */
> - ret += scnprintf(buf + ret, RD_BUF - ret, "Bank1\n");
> + ret += sysfs_emit_at(buf , ret, "Bank1\n");
> for (j = SDW_DPN_CHANNELEN_B1(i);
> j <= SDW_DPN_LANECTRL_B1(i); j++)
> ret += sdw_sprintf(slave, buf, ret, j);
> @@ -317,7 +317,7 @@ static int read_buffer_show(struct seq_file *s_file, void *data)
> return -EINVAL;
>
> for (i = 0; i < num_bytes; i++) {
> - scnprintf(buf, MAX_LINE_LEN, "address %#x val 0x%02x\n",
> + sysfs_emit(buf, "address %#x val 0x%02x\n",
> start_addr + i, read_buffer[i]);
> seq_printf(s_file, "%s", buf);
> }
> diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
> index 9db78f3d7615..aa7aa7126c47 100644
> --- a/drivers/soundwire/intel.c
> +++ b/drivers/soundwire/intel.c
> @@ -65,8 +65,7 @@ static ssize_t intel_sprintf(void __iomem *mem, bool l,
> value = intel_readl(mem, reg);
> else
> value = intel_readw(mem, reg);
> -
> - return scnprintf(buf + pos, RD_BUF - pos, "%4x\t%4x\n", reg, value);
> + return sysfs_emit_at(buf, pos, "%4x\t%4x\n", reg, value);;
double colon?
--
~Vinod
Powered by blists - more mailing lists