| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20230807034729.GR151430@dragon>
Date: Mon, 7 Aug 2023 11:47:29 +0800
From: Shawn Guo <shawnguo@...nel.org>
To: "Peng Fan (OSS)" <peng.fan@....nxp.com>
Cc: s.hauer@...gutronix.de, kernel@...gutronix.de, festevam@...il.com,
linux-imx@....com, aisheng.dong@....com,
alexander.stein@...tq-group.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Ranjani Vaidyanathan <ranjani.vaidyanathan@....com>,
Peng Fan <peng.fan@....com>
Subject: Re: [PATCH V5 9/9] firmware: imx: scu-irq: support identifying SCU
wakeup source from sysfs
On Mon, Jul 31, 2023 at 05:04:49PM +0800, Peng Fan (OSS) wrote:
> From: Ranjani Vaidyanathan <ranjani.vaidyanathan@....com>
>
> Record SCU wakeup interrupt in /sys/power/pm_wakeup_irq
> The user can further identify the exact wakeup source by using the
> following interface:
> cat /sys/firmware/scu_wakeup_source/wakeup_src
>
> The above will print the wake groups and the irqs that could have
> contributed to waking up the kernel. For example if ON/OFF button was the
> wakeup source:
> cat /sys/firmware/scu_wakeup_source/wakeup_src
> Wakeup source group = 3, irq = 0x1
>
> The user can refer to the SCFW API documentation to identify all the
> wake groups and irqs.
>
> Signed-off-by: Ranjani Vaidyanathan <ranjani.vaidyanathan@....com>
> Signed-off-by: Peng Fan <peng.fan@....com>
> ---
> drivers/firmware/imx/imx-scu-irq.c | 66 +++++++++++++++++++++++++++---
> 1 file changed, 61 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/imx/imx-scu-irq.c b/drivers/firmware/imx/imx-scu-irq.c
> index 8d902db1daf2..fcbaa393897c 100644
> --- a/drivers/firmware/imx/imx-scu-irq.c
> +++ b/drivers/firmware/imx/imx-scu-irq.c
> @@ -11,6 +11,8 @@
> #include <linux/firmware/imx/sci.h>
> #include <linux/mailbox_client.h>
> #include <linux/suspend.h>
> +#include <linux/sysfs.h>
> +#include <linux/kobject.h>
Can we keep the list alphabetically sorted?
>
> #define IMX_SC_IRQ_FUNC_ENABLE 1
> #define IMX_SC_IRQ_FUNC_STATUS 2
> @@ -40,6 +42,20 @@ 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 struct imx_sc_ipc *imx_sc_irq_ipc_handle;
> static struct work_struct imx_sc_irq_work;
> static BLOCKING_NOTIFIER_HEAD(imx_scu_irq_notifier_chain);
> @@ -71,16 +87,24 @@ 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;
> + }
Can we have a newline here?
> ret = imx_scu_irq_get_status(i, &irq_status);
> if (ret) {
> - pr_err("get irq group %d status failed, ret %d\n",
> - i, ret);
> + pr_err("get irq group %d status failed, ret %d\n", i, ret);
Unrelated change?
> return;
> }
>
> 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;
> + }
Can we have a newline here?
> pm_system_wakeup();
> imx_scu_irq_notifier_call_chain(irq_status, &i);
> }
> @@ -135,6 +159,11 @@ int imx_scu_irq_group_enable(u8 group, u32 mask, u8 enable)
> pr_err("enable irq failed, group %d, mask %d, ret %d\n",
> group, mask, ret);
>
> + if (enable)
> + scu_irq_wakeup[group].mask |= mask;
> + else
> + scu_irq_wakeup[group].mask &= ~mask;
> +
> return ret;
> }
> EXPORT_SYMBOL(imx_scu_irq_group_enable);
> @@ -144,6 +173,25 @@ 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;
> @@ -173,8 +221,7 @@ int imx_scu_enable_general_irq_channel(struct device *dev)
>
> INIT_WORK(&imx_sc_irq_work, imx_scu_irq_work_handler);
>
> - if (!of_parse_phandle_with_args(dev->of_node, "mboxes",
> - "#mbox-cells", 0, &spec))
> + if (!of_parse_phandle_with_args(dev->of_node, "mboxes", "#mbox-cells", 0, &spec))
Unrelated change?
> i = of_alias_get_id(spec.np, "mu");
>
> /* use mu1 as general mu irq channel if failed */
> @@ -183,6 +230,15 @@ int imx_scu_enable_general_irq_channel(struct device *dev)
>
> mu_resource_id = IMX_SC_R_MU_0A + i;
>
> + /* Create directory under /sysfs/firmware */
> + wakeup_obj = kobject_create_and_add("scu_wakeup_source", firmware_kobj);
Should we check error condition here?
> +
> + if (sysfs_create_file(wakeup_obj, &wakeup_source_attr.attr)) {
Should we populate 'ret' here, or is the sysfs optional?
> + pr_err("Cannot create sysfs file......\n");
Can we use dev_err() here?
> + kobject_put(wakeup_obj);
> + sysfs_remove_file(firmware_kobj, &wakeup_source_attr.attr);
The sysfs_remove_file() is the undo of sysfs_create_file() which hasn't
been successful, right?
Shawn
> + }
> +
> return ret;
> }
> EXPORT_SYMBOL(imx_scu_enable_general_irq_channel);
> --
> 2.37.1
>
Powered by blists - more mailing lists