From: Peng Fan <[email protected]>
V5:
Address sparse checking in patch 9
Add a new patch 5
V4:
Add static inline for irq get status when SCU not defined
Drop two patches wrongly included
V3:
Fix build error reported by kernel robot
Add patch subject-prefix
V2:
Fix build warning
Dong Aisheng (2):
firmware: imx: scu: change init level to subsys_initcall_sync
firmware: imx: scu: increase RPC timeout
Peng Fan (5):
firmware: imx: scu: drop return value check
firmware: imx: scu: use soc name for soc_id
firmware: imx: scu: use EOPNOTSUPP
firmware: imx: scu-irq: export imx_scu_irq_get_status
firmware: imx: scu-irq: enlarge the IMX_SC_IRQ_NUM_GROUP
Ranjani Vaidyanathan (1):
firmware: imx: scu-irq: support identifying SCU wakeup source from
sysfs
Robin Gong (1):
firmware: imx: scu-irq: fix RCU complains after M4 partition reset
drivers/firmware/imx/imx-scu-irq.c | 116 +++++++++++++++++++++++------
drivers/firmware/imx/imx-scu-soc.c | 19 ++---
drivers/firmware/imx/imx-scu.c | 9 ++-
include/linux/firmware/imx/sci.h | 16 ++--
4 files changed, 119 insertions(+), 41 deletions(-)
--
2.37.1
From: Peng Fan <[email protected]>
Per SCFW update, update the IMX_SC_IRQ_NUM_GROUP to 9.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/firmware/imx/imx-scu-irq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/imx/imx-scu-irq.c b/drivers/firmware/imx/imx-scu-irq.c
index 6549f3792a0f..8d902db1daf2 100644
--- a/drivers/firmware/imx/imx-scu-irq.c
+++ b/drivers/firmware/imx/imx-scu-irq.c
@@ -14,7 +14,7 @@
#define IMX_SC_IRQ_FUNC_ENABLE 1
#define IMX_SC_IRQ_FUNC_STATUS 2
-#define IMX_SC_IRQ_NUM_GROUP 4
+#define IMX_SC_IRQ_NUM_GROUP 9
static u32 mu_resource_id;
--
2.37.1
From: Peng Fan <[email protected]>
EOPNOTSUPP is preferred than ENOTSUPP.
Signed-off-by: Peng Fan <[email protected]>
---
include/linux/firmware/imx/sci.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/linux/firmware/imx/sci.h b/include/linux/firmware/imx/sci.h
index 5cc63fe7e84d..7fa0f3b329b5 100644
--- a/include/linux/firmware/imx/sci.h
+++ b/include/linux/firmware/imx/sci.h
@@ -25,27 +25,27 @@ int imx_scu_soc_init(struct device *dev);
#else
static inline int imx_scu_soc_init(struct device *dev)
{
- return -ENOTSUPP;
+ return -EOPNOTSUPP;
}
static inline int imx_scu_enable_general_irq_channel(struct device *dev)
{
- return -ENOTSUPP;
+ return -EOPNOTSUPP;
}
static inline int imx_scu_irq_register_notifier(struct notifier_block *nb)
{
- return -ENOTSUPP;
+ return -EOPNOTSUPP;
}
static inline int imx_scu_irq_unregister_notifier(struct notifier_block *nb)
{
- return -ENOTSUPP;
+ return -EOPNOTSUPP;
}
static inline int imx_scu_irq_group_enable(u8 group, u32 mask, u8 enable)
{
- return -ENOTSUPP;
+ return -EOPNOTSUPP;
}
#endif
#endif /* _SC_SCI_H */
--
2.37.1
From: Dong Aisheng <[email protected]>
When system loading is high, we can met some command timeout
issue occasionaly, so increase the timeout to a more safe value.
Signed-off-by: Dong Aisheng <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
drivers/firmware/imx/imx-scu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c
index 2d24359420d8..14ff9d3504bf 100644
--- a/drivers/firmware/imx/imx-scu.c
+++ b/drivers/firmware/imx/imx-scu.c
@@ -20,7 +20,7 @@
#include <linux/platform_device.h>
#define SCU_MU_CHAN_NUM 8
-#define MAX_RX_TIMEOUT (msecs_to_jiffies(30))
+#define MAX_RX_TIMEOUT (msecs_to_jiffies(3000))
struct imx_sc_chan {
struct imx_sc_ipc *sc_ipc;
--
2.37.1
From: Peng Fan <[email protected]>
Same as soc-imx8m and soc-imx driver, use soc name for soc_id
Signed-off-by: Peng Fan <[email protected]>
---
drivers/firmware/imx/imx-scu-soc.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/firmware/imx/imx-scu-soc.c b/drivers/firmware/imx/imx-scu-soc.c
index ef9103987e76..cb8377670a7d 100644
--- a/drivers/firmware/imx/imx-scu-soc.c
+++ b/drivers/firmware/imx/imx-scu-soc.c
@@ -107,10 +107,12 @@ int imx_scu_soc_init(struct device *dev)
return -EINVAL;
/* format soc_id value passed from SCU firmware */
- val = id & 0x1f;
- soc_dev_attr->soc_id = devm_kasprintf(dev, GFP_KERNEL, "0x%x", val);
- if (!soc_dev_attr->soc_id)
- return -ENOMEM;
+ if (of_machine_is_compatible("fsl,imx8qm"))
+ soc_dev_attr->soc_id = "i.MX8QM";
+ else if (of_machine_is_compatible("fsl,imx8qxp"))
+ soc_dev_attr->soc_id = "i.MX8QXP";
+ else if (of_machine_is_compatible("fsl,imx8dxl"))
+ soc_dev_attr->soc_id = "i.MX8DXL";
/* format revision value passed from SCU firmware */
val = (id >> 5) & 0xf;
--
2.37.1
From: Peng Fan <[email protected]>
Cleanup code to export imx_scu_irq_get_status API to make it could
be used by others, such as SECO.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/firmware/imx/imx-scu-irq.c | 40 ++++++++++++++++++++----------
include/linux/firmware/imx/sci.h | 6 +++++
2 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/drivers/firmware/imx/imx-scu-irq.c b/drivers/firmware/imx/imx-scu-irq.c
index 4408f150b3d5..6549f3792a0f 100644
--- a/drivers/firmware/imx/imx-scu-irq.c
+++ b/drivers/firmware/imx/imx-scu-irq.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0+
/*
- * Copyright 2019 NXP
+ * Copyright 2019,2023 NXP
*
* Implementation of the SCU IRQ functions using MU.
*
@@ -66,29 +66,18 @@ static int imx_scu_irq_notifier_call_chain(unsigned long status, u8 *group)
static void imx_scu_irq_work_handler(struct work_struct *work)
{
- struct imx_sc_msg_irq_get_status msg;
- struct imx_sc_rpc_msg *hdr = &msg.hdr;
u32 irq_status;
int ret;
u8 i;
for (i = 0; i < IMX_SC_IRQ_NUM_GROUP; i++) {
- hdr->ver = IMX_SC_RPC_VERSION;
- hdr->svc = IMX_SC_RPC_SVC_IRQ;
- hdr->func = IMX_SC_IRQ_FUNC_STATUS;
- hdr->size = 2;
-
- msg.data.req.resource = mu_resource_id;
- msg.data.req.group = i;
-
- ret = imx_scu_call_rpc(imx_sc_irq_ipc_handle, &msg, true);
+ ret = imx_scu_irq_get_status(i, &irq_status);
if (ret) {
pr_err("get irq group %d status failed, ret %d\n",
i, ret);
return;
}
- irq_status = msg.data.resp.status;
if (!irq_status)
continue;
@@ -97,6 +86,31 @@ static void imx_scu_irq_work_handler(struct work_struct *work)
}
}
+int imx_scu_irq_get_status(u8 group, u32 *irq_status)
+{
+ struct imx_sc_msg_irq_get_status msg;
+ struct imx_sc_rpc_msg *hdr = &msg.hdr;
+ int ret;
+
+ hdr->ver = IMX_SC_RPC_VERSION;
+ hdr->svc = IMX_SC_RPC_SVC_IRQ;
+ hdr->func = IMX_SC_IRQ_FUNC_STATUS;
+ hdr->size = 2;
+
+ msg.data.req.resource = mu_resource_id;
+ msg.data.req.group = group;
+
+ ret = imx_scu_call_rpc(imx_sc_irq_ipc_handle, &msg, true);
+ if (ret)
+ return ret;
+
+ if (irq_status)
+ *irq_status = msg.data.resp.status;
+
+ return 0;
+}
+EXPORT_SYMBOL(imx_scu_irq_get_status);
+
int imx_scu_irq_group_enable(u8 group, u32 mask, u8 enable)
{
struct imx_sc_msg_irq_enable msg;
diff --git a/include/linux/firmware/imx/sci.h b/include/linux/firmware/imx/sci.h
index 7fa0f3b329b5..df17196df5ff 100644
--- a/include/linux/firmware/imx/sci.h
+++ b/include/linux/firmware/imx/sci.h
@@ -21,6 +21,7 @@ int imx_scu_enable_general_irq_channel(struct device *dev);
int imx_scu_irq_register_notifier(struct notifier_block *nb);
int imx_scu_irq_unregister_notifier(struct notifier_block *nb);
int imx_scu_irq_group_enable(u8 group, u32 mask, u8 enable);
+int imx_scu_irq_get_status(u8 group, u32 *irq_status);
int imx_scu_soc_init(struct device *dev);
#else
static inline int imx_scu_soc_init(struct device *dev)
@@ -47,5 +48,10 @@ static inline int imx_scu_irq_group_enable(u8 group, u32 mask, u8 enable)
{
return -EOPNOTSUPP;
}
+
+static inline int imx_scu_irq_get_status(u8 group, u32 *irq_status)
+{
+ return -EOPNOTSUPP;
+}
#endif
#endif /* _SC_SCI_H */
--
2.37.1
From: Ranjani Vaidyanathan <[email protected]>
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 <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
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>
#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;
+ }
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);
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;
+ }
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))
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);
+
+ if (sysfs_create_file(wakeup_obj, &wakeup_source_attr.attr)) {
+ pr_err("Cannot create sysfs file......\n");
+ kobject_put(wakeup_obj);
+ sysfs_remove_file(firmware_kobj, &wakeup_source_attr.attr);
+ }
+
return ret;
}
EXPORT_SYMBOL(imx_scu_enable_general_irq_channel);
--
2.37.1
On Mon, Jul 31, 2023 at 05:04:42PM +0800, Peng Fan (OSS) wrote:
> From: Dong Aisheng <[email protected]>
>
> When system loading is high, we can met some command timeout
s/met/meet
> issue occasionaly, so increase the timeout to a more safe value.
s/occasionaly/occasionally
Shawn
>
> Signed-off-by: Dong Aisheng <[email protected]>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/firmware/imx/imx-scu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c
> index 2d24359420d8..14ff9d3504bf 100644
> --- a/drivers/firmware/imx/imx-scu.c
> +++ b/drivers/firmware/imx/imx-scu.c
> @@ -20,7 +20,7 @@
> #include <linux/platform_device.h>
>
> #define SCU_MU_CHAN_NUM 8
> -#define MAX_RX_TIMEOUT (msecs_to_jiffies(30))
> +#define MAX_RX_TIMEOUT (msecs_to_jiffies(3000))
>
> struct imx_sc_chan {
> struct imx_sc_ipc *sc_ipc;
> --
> 2.37.1
>
On Mon, Jul 31, 2023 at 05:04:45PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> EOPNOTSUPP is preferred than ENOTSUPP.
Could you elaborate why?
Shawn
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> include/linux/firmware/imx/sci.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/firmware/imx/sci.h b/include/linux/firmware/imx/sci.h
> index 5cc63fe7e84d..7fa0f3b329b5 100644
> --- a/include/linux/firmware/imx/sci.h
> +++ b/include/linux/firmware/imx/sci.h
> @@ -25,27 +25,27 @@ int imx_scu_soc_init(struct device *dev);
> #else
> static inline int imx_scu_soc_init(struct device *dev)
> {
> - return -ENOTSUPP;
> + return -EOPNOTSUPP;
> }
>
> static inline int imx_scu_enable_general_irq_channel(struct device *dev)
> {
> - return -ENOTSUPP;
> + return -EOPNOTSUPP;
> }
>
> static inline int imx_scu_irq_register_notifier(struct notifier_block *nb)
> {
> - return -ENOTSUPP;
> + return -EOPNOTSUPP;
> }
>
> static inline int imx_scu_irq_unregister_notifier(struct notifier_block *nb)
> {
> - return -ENOTSUPP;
> + return -EOPNOTSUPP;
> }
>
> static inline int imx_scu_irq_group_enable(u8 group, u32 mask, u8 enable)
> {
> - return -ENOTSUPP;
> + return -EOPNOTSUPP;
> }
> #endif
> #endif /* _SC_SCI_H */
> --
> 2.37.1
>
On Mon, Jul 31, 2023 at 05:04:44PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> Same as soc-imx8m and soc-imx driver, use soc name for soc_id
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/firmware/imx/imx-scu-soc.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/firmware/imx/imx-scu-soc.c b/drivers/firmware/imx/imx-scu-soc.c
> index ef9103987e76..cb8377670a7d 100644
> --- a/drivers/firmware/imx/imx-scu-soc.c
> +++ b/drivers/firmware/imx/imx-scu-soc.c
> @@ -107,10 +107,12 @@ int imx_scu_soc_init(struct device *dev)
> return -EINVAL;
>
> /* format soc_id value passed from SCU firmware */
> - val = id & 0x1f;
> - soc_dev_attr->soc_id = devm_kasprintf(dev, GFP_KERNEL, "0x%x", val);
> - if (!soc_dev_attr->soc_id)
> - return -ENOMEM;
> + if (of_machine_is_compatible("fsl,imx8qm"))
> + soc_dev_attr->soc_id = "i.MX8QM";
> + else if (of_machine_is_compatible("fsl,imx8qxp"))
> + soc_dev_attr->soc_id = "i.MX8QXP";
> + else if (of_machine_is_compatible("fsl,imx8dxl"))
> + soc_dev_attr->soc_id = "i.MX8DXL";
Is it possible to deduce SoC name from the id retrieved from SCU firmware?
IMO, device tree should be the last resort when there is no other
sources for the such information.
Shawn
>
> /* format revision value passed from SCU firmware */
> val = (id >> 5) & 0xf;
> --
> 2.37.1
>
On Mon, Aug 07, 2023 at 02:57:17AM +0000, Peng Fan wrote:
> Hi Shawn,
>
> > Subject: Re: [PATCH V5 5/9] firmware: imx: scu: use EOPNOTSUPP
> >
> > On Mon, Jul 31, 2023 at 05:04:45PM +0800, Peng Fan (OSS) wrote:
> > > From: Peng Fan <[email protected]>
> > >
> > > EOPNOTSUPP is preferred than ENOTSUPP.
> >
> > Could you elaborate why?
>
> From checkpatch.pl:
> "ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP"
> I could add this in commit message if you prefer.
Yes, please.
Shawn
On Mon, Jul 31, 2023 at 05:04:47PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> Cleanup code to export imx_scu_irq_get_status API to make it could
> be used by others, such as SECO.
The first read on subject and commit log gets me the impression that
imx_scu_irq_get_status() is an existing function. Please improve to
make it clear that this is a new function.
Shawn
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/firmware/imx/imx-scu-irq.c | 40 ++++++++++++++++++++----------
> include/linux/firmware/imx/sci.h | 6 +++++
> 2 files changed, 33 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/firmware/imx/imx-scu-irq.c b/drivers/firmware/imx/imx-scu-irq.c
> index 4408f150b3d5..6549f3792a0f 100644
> --- a/drivers/firmware/imx/imx-scu-irq.c
> +++ b/drivers/firmware/imx/imx-scu-irq.c
> @@ -1,6 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0+
> /*
> - * Copyright 2019 NXP
> + * Copyright 2019,2023 NXP
> *
> * Implementation of the SCU IRQ functions using MU.
> *
> @@ -66,29 +66,18 @@ static int imx_scu_irq_notifier_call_chain(unsigned long status, u8 *group)
>
> static void imx_scu_irq_work_handler(struct work_struct *work)
> {
> - struct imx_sc_msg_irq_get_status msg;
> - struct imx_sc_rpc_msg *hdr = &msg.hdr;
> u32 irq_status;
> int ret;
> u8 i;
>
> for (i = 0; i < IMX_SC_IRQ_NUM_GROUP; i++) {
> - hdr->ver = IMX_SC_RPC_VERSION;
> - hdr->svc = IMX_SC_RPC_SVC_IRQ;
> - hdr->func = IMX_SC_IRQ_FUNC_STATUS;
> - hdr->size = 2;
> -
> - msg.data.req.resource = mu_resource_id;
> - msg.data.req.group = i;
> -
> - ret = imx_scu_call_rpc(imx_sc_irq_ipc_handle, &msg, true);
> + ret = imx_scu_irq_get_status(i, &irq_status);
> if (ret) {
> pr_err("get irq group %d status failed, ret %d\n",
> i, ret);
> return;
> }
>
> - irq_status = msg.data.resp.status;
> if (!irq_status)
> continue;
>
> @@ -97,6 +86,31 @@ static void imx_scu_irq_work_handler(struct work_struct *work)
> }
> }
>
> +int imx_scu_irq_get_status(u8 group, u32 *irq_status)
> +{
> + struct imx_sc_msg_irq_get_status msg;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> + int ret;
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_IRQ;
> + hdr->func = IMX_SC_IRQ_FUNC_STATUS;
> + hdr->size = 2;
> +
> + msg.data.req.resource = mu_resource_id;
> + msg.data.req.group = group;
> +
> + ret = imx_scu_call_rpc(imx_sc_irq_ipc_handle, &msg, true);
> + if (ret)
> + return ret;
> +
> + if (irq_status)
> + *irq_status = msg.data.resp.status;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(imx_scu_irq_get_status);
> +
> int imx_scu_irq_group_enable(u8 group, u32 mask, u8 enable)
> {
> struct imx_sc_msg_irq_enable msg;
> diff --git a/include/linux/firmware/imx/sci.h b/include/linux/firmware/imx/sci.h
> index 7fa0f3b329b5..df17196df5ff 100644
> --- a/include/linux/firmware/imx/sci.h
> +++ b/include/linux/firmware/imx/sci.h
> @@ -21,6 +21,7 @@ int imx_scu_enable_general_irq_channel(struct device *dev);
> int imx_scu_irq_register_notifier(struct notifier_block *nb);
> int imx_scu_irq_unregister_notifier(struct notifier_block *nb);
> int imx_scu_irq_group_enable(u8 group, u32 mask, u8 enable);
> +int imx_scu_irq_get_status(u8 group, u32 *irq_status);
> int imx_scu_soc_init(struct device *dev);
> #else
> static inline int imx_scu_soc_init(struct device *dev)
> @@ -47,5 +48,10 @@ static inline int imx_scu_irq_group_enable(u8 group, u32 mask, u8 enable)
> {
> return -EOPNOTSUPP;
> }
> +
> +static inline int imx_scu_irq_get_status(u8 group, u32 *irq_status)
> +{
> + return -EOPNOTSUPP;
> +}
> #endif
> #endif /* _SC_SCI_H */
> --
> 2.37.1
>
On Mon, Jul 31, 2023 at 05:04:49PM +0800, Peng Fan (OSS) wrote:
> From: Ranjani Vaidyanathan <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> 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
>
Hi Shawn,
> Subject: Re: [PATCH V5 5/9] firmware: imx: scu: use EOPNOTSUPP
>
> On Mon, Jul 31, 2023 at 05:04:45PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <[email protected]>
> >
> > EOPNOTSUPP is preferred than ENOTSUPP.
>
> Could you elaborate why?
From checkpatch.pl:
"ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP"
I could add this in commit message if you prefer.
Thanks,
Peng.
>
> Shawn
>
> >
> > Signed-off-by: Peng Fan <[email protected]>
> > ---
> > include/linux/firmware/imx/sci.h | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/firmware/imx/sci.h
> > b/include/linux/firmware/imx/sci.h
> > index 5cc63fe7e84d..7fa0f3b329b5 100644
> > --- a/include/linux/firmware/imx/sci.h
> > +++ b/include/linux/firmware/imx/sci.h
> > @@ -25,27 +25,27 @@ int imx_scu_soc_init(struct device *dev); #else
> > static inline int imx_scu_soc_init(struct device *dev) {
> > - return -ENOTSUPP;
> > + return -EOPNOTSUPP;
> > }
> >
> > static inline int imx_scu_enable_general_irq_channel(struct device
> > *dev) {
> > - return -ENOTSUPP;
> > + return -EOPNOTSUPP;
> > }
> >
> > static inline int imx_scu_irq_register_notifier(struct notifier_block
> > *nb) {
> > - return -ENOTSUPP;
> > + return -EOPNOTSUPP;
> > }
> >
> > static inline int imx_scu_irq_unregister_notifier(struct
> > notifier_block *nb) {
> > - return -ENOTSUPP;
> > + return -EOPNOTSUPP;
> > }
> >
> > static inline int imx_scu_irq_group_enable(u8 group, u32 mask, u8
> > enable) {
> > - return -ENOTSUPP;
> > + return -EOPNOTSUPP;
> > }
> > #endif
> > #endif /* _SC_SCI_H */
> > --
> > 2.37.1
> >