drivers/firmware/imx/imx-scu-irq.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
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.0On 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
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
Hi Stepan, 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 I think it is safe to drop, thanks for working on this. Thanks Peng >via dev_info() instead. > >Stepan
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
wakeup_source_show() walks all IMX_SC_IRQ_NUM_GROUP groups and
rewrites buf from offset 0 each iteration, so userspace reading
/sys/firmware/scu_wakeup_source/wakeup_src only ever sees the last
group reported, and the trailing strlen(buf) reports only that
line length. sprintf() is also unbounded against the sysfs
PAGE_SIZE buffer.
The attribute is not documented under Documentation/ABI/ and a code
search across GitHub and Debian found no out-of-tree consumers.
Peng Fan (NXP) confirmed it is safe to drop.
Remove the attribute, the supporting kobject and the per-group
valid/wakeup_src state used only to feed it. Log the same wakeup
information via dev_dbg() from imx_scu_irq_work_handler() so it
can be enabled dynamically when needed.
Fixes: c081197a33a2 ("firmware: imx: scu-irq: support identifying SCU wakeup source from sysfs")
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Link: https://lore.kernel.org/all/2026051656-corral-edgy-290c@gregkh/
Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
v2:
- Drop the sysfs file entirely instead of patching it (Greg)
- Log wakeup info via dev_dbg() in the IRQ work handler (Greg)
- Peng Fan (NXP) confirmed no out-of-tree consumers
v1: https://lore.kernel.org/all/20260515175002.34853-1-sozdayvek@gmail.com/
drivers/firmware/imx/imx-scu-irq.c | 84 ++++++------------------------
1 file changed, 16 insertions(+), 68 deletions(-)
diff --git a/drivers/firmware/imx/imx-scu-irq.c b/drivers/firmware/imx/imx-scu-irq.c
index a68d38f89..5969460ba 100644
--- a/drivers/firmware/imx/imx-scu-irq.c
+++ b/drivers/firmware/imx/imx-scu-irq.c
@@ -9,11 +9,9 @@
#include <dt-bindings/firmware/imx/rsrc.h>
#include <linux/firmware/imx/ipc.h>
#include <linux/firmware/imx/sci.h>
-#include <linux/kobject.h>
#include <linux/mailbox_client.h>
#include <linux/of.h>
#include <linux/suspend.h>
-#include <linux/sysfs.h>
#define IMX_SC_IRQ_FUNC_ENABLE 1
#define IMX_SC_IRQ_FUNC_STATUS 2
@@ -43,19 +41,8 @@ struct imx_sc_msg_irq_enable {
u8 enable;
} __packed;
-struct scu_wakeup {
- u32 mask;
- u32 wakeup_src;
- bool valid;
-};
-
-/* Sysfs functions */
-static struct kobject *wakeup_obj;
-static ssize_t wakeup_source_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf);
-static struct kobj_attribute wakeup_source_attr =
- __ATTR(wakeup_src, 0660, wakeup_source_show, NULL);
-
-static struct scu_wakeup scu_irq_wakeup[IMX_SC_IRQ_NUM_GROUP];
+static u32 scu_irq_wakeup_mask[IMX_SC_IRQ_NUM_GROUP];
+static struct device *imx_sc_irq_dev;
static struct imx_sc_ipc *imx_sc_irq_ipc_handle;
static struct work_struct imx_sc_irq_work;
@@ -88,11 +75,6 @@ static void imx_scu_irq_work_handler(struct work_struct *work)
u8 i;
for (i = 0; i < IMX_SC_IRQ_NUM_GROUP; i++) {
- if (scu_irq_wakeup[i].mask) {
- scu_irq_wakeup[i].valid = false;
- scu_irq_wakeup[i].wakeup_src = 0;
- }
-
ret = imx_scu_irq_get_status(i, &irq_status);
if (ret) {
pr_err("get irq group %d status failed, ret %d\n",
@@ -102,12 +84,15 @@ static void imx_scu_irq_work_handler(struct work_struct *work)
if (!irq_status)
continue;
- if (scu_irq_wakeup[i].mask & irq_status) {
- scu_irq_wakeup[i].valid = true;
- scu_irq_wakeup[i].wakeup_src = irq_status & scu_irq_wakeup[i].mask;
- } else {
- scu_irq_wakeup[i].wakeup_src = irq_status;
- }
+
+ if (scu_irq_wakeup_mask[i] & irq_status)
+ dev_dbg(imx_sc_irq_dev,
+ "Wakeup source group = %d, irq = 0x%x\n",
+ i, irq_status & scu_irq_wakeup_mask[i]);
+ else
+ dev_dbg(imx_sc_irq_dev,
+ "Spurious SCU wakeup, group = %d, irq = 0x%x\n",
+ i, irq_status);
pm_system_wakeup();
imx_scu_irq_notifier_call_chain(irq_status, &i);
@@ -164,9 +149,9 @@ int imx_scu_irq_group_enable(u8 group, u32 mask, u8 enable)
group, mask, ret);
if (enable)
- scu_irq_wakeup[group].mask |= mask;
+ scu_irq_wakeup_mask[group] |= mask;
else
- scu_irq_wakeup[group].mask &= ~mask;
+ scu_irq_wakeup_mask[group] &= ~mask;
return ret;
}
@@ -177,25 +162,6 @@ static void imx_scu_irq_callback(struct mbox_client *c, void *msg)
schedule_work(&imx_sc_irq_work);
}
-static ssize_t wakeup_source_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
-{
- int i;
-
- 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, irq = 0x%x\n",
- i, scu_irq_wakeup[i].wakeup_src);
- else
- sprintf(buf, "Spurious SCU wakeup, group = %d, irq = 0x%x\n",
- i, scu_irq_wakeup[i].wakeup_src);
- }
-
- return strlen(buf);
-}
-
int imx_scu_enable_general_irq_channel(struct device *dev)
{
struct of_phandle_args spec;
@@ -233,29 +199,11 @@ int imx_scu_enable_general_irq_channel(struct device *dev)
if (IS_ERR(ch)) {
ret = PTR_ERR(ch);
dev_err(dev, "failed to request mbox chan gip3, ret %d\n", ret);
- goto free_cl;
- }
-
- /* Create directory under /sysfs/firmware */
- wakeup_obj = kobject_create_and_add("scu_wakeup_source", firmware_kobj);
- if (!wakeup_obj) {
- ret = -ENOMEM;
- goto free_ch;
+ devm_kfree(dev, cl);
+ return ret;
}
- ret = sysfs_create_file(wakeup_obj, &wakeup_source_attr.attr);
- if (ret) {
- dev_err(dev, "Cannot create wakeup source src file......\n");
- kobject_put(wakeup_obj);
- goto free_ch;
- }
+ imx_sc_irq_dev = dev;
return 0;
-
-free_ch:
- mbox_free_channel(ch);
-free_cl:
- devm_kfree(dev, cl);
-
- return ret;
}
--
2.43.0© 2016 - 2026 Red Hat, Inc.