[PATCH] firmware: imx: scu-irq: accumulate wakeup sources via sysfs_emit_at()

[PATCH] firmware: imx: scu-irq: accumulate wakeup sources via sysfs_emit_at()

[Stepan Ionichev]

wakeup_source_show() walks all IMX_SC_IRQ_NUM_GROUP groups and, for
every group with a wakeup_src set, writes a line into the sysfs
output buffer:

	for (i = 0; i < IMX_SC_IRQ_NUM_GROUP; i++) {
		if (!scu_irq_wakeup[i].wakeup_src)
			continue;

		if (scu_irq_wakeup[i].valid)
			sprintf(buf, "Wakeup source group = %d, ...", ...);
		else
			sprintf(buf, "Spurious SCU wakeup, group = %d, ...", ...);
	}

	return strlen(buf);

Each iteration calls sprintf(buf, ...) starting at buf[0], so the
previous group's line is overwritten. When several groups have
wakeup_src set simultaneously, userspace reading
/sys/.../wakeup_src sees only the last group reported, not the full
set. The trailing return strlen(buf) reports only that last line's
length for the same reason.

sprintf() also has no buffer length argument; sysfs callbacks must
not write past PAGE_SIZE.

Convert to sysfs_emit_at() with a running offset so each group's
line is appended after the previous one, and bound the writes to
the PAGE_SIZE sysfs limit. Return the accumulated length directly
instead of strlen(buf).

Fixes: c081197a33a2 ("firmware: imx: scu-irq: support identifying SCU wakeup source from sysfs")
Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
 drivers/firmware/imx/imx-scu-irq.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/imx/imx-scu-irq.c b/drivers/firmware/imx/imx-scu-irq.c
index a68d38f89..d1fb20d95 100644
--- a/drivers/firmware/imx/imx-scu-irq.c
+++ b/drivers/firmware/imx/imx-scu-irq.c
@@ -179,6 +179,7 @@ static void imx_scu_irq_callback(struct mbox_client *c, void *msg)
 
 static ssize_t wakeup_source_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
 {
+	ssize_t len = 0;
 	int i;
 
 	for (i = 0; i < IMX_SC_IRQ_NUM_GROUP; i++) {
@@ -186,14 +187,16 @@ static ssize_t wakeup_source_show(struct kobject *kobj, struct kobj_attribute *a
 			continue;
 
 		if (scu_irq_wakeup[i].valid)
-			sprintf(buf, "Wakeup source group = %d, irq = 0x%x\n",
+			len += sysfs_emit_at(buf, len,
+				"Wakeup source group = %d, irq = 0x%x\n",
 				i, scu_irq_wakeup[i].wakeup_src);
 		else
-			sprintf(buf, "Spurious SCU wakeup, group = %d, irq = 0x%x\n",
+			len += sysfs_emit_at(buf, len,
+				"Spurious SCU wakeup, group = %d, irq = 0x%x\n",
 				i, scu_irq_wakeup[i].wakeup_src);
 	}
 
-	return strlen(buf);
+	return len;
 }
 
 int imx_scu_enable_general_irq_channel(struct device *dev)
-- 
2.43.0

Re: [PATCH] firmware: imx: scu-irq: accumulate wakeup sources via sysfs_emit_at()

[Greg KH]

On Fri, May 15, 2026 at 10:50:01PM +0500, Stepan Ionichev wrote:
> wakeup_source_show() walks all IMX_SC_IRQ_NUM_GROUP groups and, for
> every group with a wakeup_src set, writes a line into the sysfs
> output buffer:
> 
> 	for (i = 0; i < IMX_SC_IRQ_NUM_GROUP; i++) {
> 		if (!scu_irq_wakeup[i].wakeup_src)
> 			continue;
> 
> 		if (scu_irq_wakeup[i].valid)
> 			sprintf(buf, "Wakeup source group = %d, ...", ...);
> 		else
> 			sprintf(buf, "Spurious SCU wakeup, group = %d, ...", ...);

This is a horrible sysfs file, and breaks all the rules.  Why not just
delete it and use the proper api for it instead?

thanks,
greg k-h

Re: [PATCH] firmware: imx: scu-irq: accumulate wakeup sources via sysfs_emit_at()

[Stepan Ionichev]

On Sat, May 16, 2026 at 09:15:54AM +0200, Greg Kroah-Hartman wrote:
> This is a horrible sysfs file, and breaks all the rules.  Why not just
> delete it and use the proper api for it instead?

Yeah, fair. The sprintf change just papers over the real issue, which
is the interface itself.

Quick check: no Documentation/ABI/ entry, github code search returns
nothing outside the kernel tree itself
(https://github.com/search?q=scu_wakeup_irqs&type=code), and
codesearch.debian.net also shows zero hits. So removing the attribute
looks doable.

Frank, Sascha -- any out-of-tree readers of
/sys/firmware/scu_wakeup_irqs/wakeup_src I should worry about?  If
not, I'll send v2 that drops the attribute and logs the wakeup source
via dev_info() instead.

Stepan

Re: [PATCH] firmware: imx: scu-irq: accumulate wakeup sources via sysfs_emit_at()

[Greg KH]

On Sat, May 16, 2026 at 02:07:26PM +0500, Stepan Ionichev wrote:
> On Sat, May 16, 2026 at 09:15:54AM +0200, Greg Kroah-Hartman wrote:
> > This is a horrible sysfs file, and breaks all the rules.  Why not just
> > delete it and use the proper api for it instead?
> 
> Yeah, fair. The sprintf change just papers over the real issue, which
> is the interface itself.
> 
> Quick check: no Documentation/ABI/ entry, github code search returns
> nothing outside the kernel tree itself
> (https://github.com/search?q=scu_wakeup_irqs&type=code), and
> codesearch.debian.net also shows zero hits. So removing the attribute
> looks doable.
> 
> Frank, Sascha -- any out-of-tree readers of
> /sys/firmware/scu_wakeup_irqs/wakeup_src I should worry about?  If
> not, I'll send v2 that drops the attribute and logs the wakeup source
> via dev_info() instead.

Close, but no, don't use dev_info(), when drivers work properly, they
are quiet.  Make it dev_dbg() so that if anyone wants to see it, they
can dynamically turn it on.  Or make it a debugfs file.

thanks,

greg k-h