[PATCH V5 0/9] firmware: imx: scu/scu-irq: misc update

[PATCH V5 0/9] firmware: imx: scu/scu-irq: misc update

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V5 1/9] firmware: imx: scu: change init level to subsys_initcall_sync

[Peng Fan (OSS)]

From: Dong Aisheng <aisheng.dong@nxp.com>

Change firmware init level to subsys_initcall_sync to ensure it's
probed before most devices to avoid unnecessary defer probe.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/firmware/imx/imx-scu.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c
index 47db49911e7b..2d24359420d8 100644
--- a/drivers/firmware/imx/imx-scu.c
+++ b/drivers/firmware/imx/imx-scu.c
@@ -353,7 +353,12 @@ static struct platform_driver imx_scu_driver = {
 	},
 	.probe = imx_scu_probe,
 };
-builtin_platform_driver(imx_scu_driver);
+
+static int __init imx_scu_driver_init(void)
+{
+	return platform_driver_register(&imx_scu_driver);
+}
+subsys_initcall_sync(imx_scu_driver_init);
 
 MODULE_AUTHOR("Dong Aisheng <aisheng.dong@nxp.com>");
 MODULE_DESCRIPTION("IMX SCU firmware protocol driver");
-- 
2.37.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V5 2/9] firmware: imx: scu: increase RPC timeout

[Peng Fan (OSS)]

From: Dong Aisheng <aisheng.dong@nxp.com>

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 <aisheng.dong@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 2/9] firmware: imx: scu: increase RPC timeout

[Shawn Guo]

On Mon, Jul 31, 2023 at 05:04:42PM +0800, Peng Fan (OSS) wrote:
> From: Dong Aisheng <aisheng.dong@nxp.com>
> 
> 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 <aisheng.dong@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V5 3/9] firmware: imx: scu: drop return value check

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

For IMX_SC_MISC_FUNC_UNIQUE_ID, the scfw has a return value,
and the imx-scu always return 0. So drop the return value check.

BTW, also initialize msg to 0.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/firmware/imx/imx-scu-soc.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/firmware/imx/imx-scu-soc.c b/drivers/firmware/imx/imx-scu-soc.c
index 2f32353de2c9..ef9103987e76 100644
--- a/drivers/firmware/imx/imx-scu-soc.c
+++ b/drivers/firmware/imx/imx-scu-soc.c
@@ -33,20 +33,15 @@ struct imx_sc_msg_misc_get_soc_uid {
 
 static int imx_scu_soc_uid(u64 *soc_uid)
 {
-	struct imx_sc_msg_misc_get_soc_uid msg;
+	struct imx_sc_msg_misc_get_soc_uid msg = { 0 };
 	struct imx_sc_rpc_msg *hdr = &msg.hdr;
-	int ret;
 
 	hdr->ver = IMX_SC_RPC_VERSION;
 	hdr->svc = IMX_SC_RPC_SVC_MISC;
 	hdr->func = IMX_SC_MISC_FUNC_UNIQUE_ID;
 	hdr->size = 1;
 
-	ret = imx_scu_call_rpc(imx_sc_soc_ipc_handle, &msg, true);
-	if (ret) {
-		pr_err("%s: get soc uid failed, ret %d\n", __func__, ret);
-		return ret;
-	}
+	imx_scu_call_rpc(imx_sc_soc_ipc_handle, &msg, true);
 
 	*soc_uid = msg.uid_high;
 	*soc_uid <<= 32;
-- 
2.37.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 3/9] firmware: imx: scu: drop return value check

[Shawn Guo]

On Mon, Jul 31, 2023 at 05:04:43PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> For IMX_SC_MISC_FUNC_UNIQUE_ID, the scfw has a return value,
> and the imx-scu always return 0. So drop the return value check.

imx_scu_call_rpc() could fail for a few reason.  Are we not want to
check those possible failures?

Shawn

> 
> BTW, also initialize msg to 0.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/firmware/imx/imx-scu-soc.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/firmware/imx/imx-scu-soc.c b/drivers/firmware/imx/imx-scu-soc.c
> index 2f32353de2c9..ef9103987e76 100644
> --- a/drivers/firmware/imx/imx-scu-soc.c
> +++ b/drivers/firmware/imx/imx-scu-soc.c
> @@ -33,20 +33,15 @@ struct imx_sc_msg_misc_get_soc_uid {
>  
>  static int imx_scu_soc_uid(u64 *soc_uid)
>  {
> -	struct imx_sc_msg_misc_get_soc_uid msg;
> +	struct imx_sc_msg_misc_get_soc_uid msg = { 0 };
>  	struct imx_sc_rpc_msg *hdr = &msg.hdr;
> -	int ret;
>  
>  	hdr->ver = IMX_SC_RPC_VERSION;
>  	hdr->svc = IMX_SC_RPC_SVC_MISC;
>  	hdr->func = IMX_SC_MISC_FUNC_UNIQUE_ID;
>  	hdr->size = 1;
>  
> -	ret = imx_scu_call_rpc(imx_sc_soc_ipc_handle, &msg, true);
> -	if (ret) {
> -		pr_err("%s: get soc uid failed, ret %d\n", __func__, ret);
> -		return ret;
> -	}
> +	imx_scu_call_rpc(imx_sc_soc_ipc_handle, &msg, true);
>  
>  	*soc_uid = msg.uid_high;
>  	*soc_uid <<= 32;
> -- 
> 2.37.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH V5 3/9] firmware: imx: scu: drop return value check

[Peng Fan]

Hi Shawn,

> Subject: Re: [PATCH V5 3/9] firmware: imx: scu: drop return value check
> 
> On Mon, Jul 31, 2023 at 05:04:43PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > For IMX_SC_MISC_FUNC_UNIQUE_ID, the scfw has a return value, and the
> > imx-scu always return 0. So drop the return value check.
> 
> imx_scu_call_rpc() could fail for a few reason.  Are we not want to check
> those possible failures?

Yes, I overlooked this, drop this patch.

Thanks,
Peng.

> 
> Shawn
> 
> >
> > BTW, also initialize msg to 0.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  drivers/firmware/imx/imx-scu-soc.c | 9 ++-------
> >  1 file changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/firmware/imx/imx-scu-soc.c
> > b/drivers/firmware/imx/imx-scu-soc.c
> > index 2f32353de2c9..ef9103987e76 100644
> > --- a/drivers/firmware/imx/imx-scu-soc.c
> > +++ b/drivers/firmware/imx/imx-scu-soc.c
> > @@ -33,20 +33,15 @@ struct imx_sc_msg_misc_get_soc_uid {
> >
> >  static int imx_scu_soc_uid(u64 *soc_uid)  {
> > -	struct imx_sc_msg_misc_get_soc_uid msg;
> > +	struct imx_sc_msg_misc_get_soc_uid msg = { 0 };
> >  	struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > -	int ret;
> >
> >  	hdr->ver = IMX_SC_RPC_VERSION;
> >  	hdr->svc = IMX_SC_RPC_SVC_MISC;
> >  	hdr->func = IMX_SC_MISC_FUNC_UNIQUE_ID;
> >  	hdr->size = 1;
> >
> > -	ret = imx_scu_call_rpc(imx_sc_soc_ipc_handle, &msg, true);
> > -	if (ret) {
> > -		pr_err("%s: get soc uid failed, ret %d\n", __func__, ret);
> > -		return ret;
> > -	}
> > +	imx_scu_call_rpc(imx_sc_soc_ipc_handle, &msg, true);
> >
> >  	*soc_uid = msg.uid_high;
> >  	*soc_uid <<= 32;
> > --
> > 2.37.1
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V5 4/9] firmware: imx: scu: use soc name for soc_id

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Same as soc-imx8m and soc-imx driver, use soc name for soc_id

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 4/9] firmware: imx: scu: use soc name for soc_id

[Shawn Guo]

On Mon, Jul 31, 2023 at 05:04:44PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Same as soc-imx8m and soc-imx driver, use soc name for soc_id
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V5 5/9] firmware: imx: scu: use EOPNOTSUPP

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

EOPNOTSUPP is preferred than ENOTSUPP.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 5/9] firmware: imx: scu: use EOPNOTSUPP

[Shawn Guo]

On Mon, Jul 31, 2023 at 05:04:45PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> EOPNOTSUPP is preferred than ENOTSUPP.

Could you elaborate why?

Shawn

> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH V5 5/9] firmware: imx: scu: use EOPNOTSUPP

[Peng Fan]

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 <peng.fan@nxp.com>
> >
> > 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 <peng.fan@nxp.com>
> > ---
> >  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
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 5/9] firmware: imx: scu: use EOPNOTSUPP

[Shawn Guo]

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 <peng.fan@nxp.com>
> > >
> > > 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V5 6/9] firmware: imx: scu-irq: fix RCU complains after M4 partition reset

[Peng Fan (OSS)]

From: Robin Gong <yibin.gong@nxp.com>

Use blocking_notifier_chain instead of atomic_notifier_chain, otherwise
below RCU complains would come out since unregister/register_virtio_device
() will issue mbox message (mbox_send_message() is blocking) again after
received M4 partition reset. Actually, no need atomic for notifier which
is so tough for user since this notifier is called in worker instead of
interrupt handler directly.

[  389.706645] i2c-rpmsg virtio0.rpmsg-i2c-channel.-1.2: i2c rpmsg driver is removed
[  389.767362] Wait for remote ready timeout, use first_notify.
[  389.774084] ------------[ cut here ]------------
[  389.778729] WARNING: CPU: 0 PID: 397 at kernel/rcu/tree_plugin.h:293 rcu_note_context_switch+0x34/0x338
[  389.788131] Modules linked in:
[  389.791195] CPU: 0 PID: 397 Comm: kworker/0:13 Not tainted 5.4.0-rc5-02977-g08f78722f07b #26
[  389.799633] Hardware name: Freescale i.MX8DXL Phantom MEK (DT)
[  389.805481] Workqueue: events imx_scu_irq_work_handler

Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/firmware/imx/imx-scu-irq.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/imx/imx-scu-irq.c b/drivers/firmware/imx/imx-scu-irq.c
index d9dcc20945c6..4408f150b3d5 100644
--- a/drivers/firmware/imx/imx-scu-irq.c
+++ b/drivers/firmware/imx/imx-scu-irq.c
@@ -42,25 +42,25 @@ struct imx_sc_msg_irq_enable {
 
 static struct imx_sc_ipc *imx_sc_irq_ipc_handle;
 static struct work_struct imx_sc_irq_work;
-static ATOMIC_NOTIFIER_HEAD(imx_scu_irq_notifier_chain);
+static BLOCKING_NOTIFIER_HEAD(imx_scu_irq_notifier_chain);
 
 int imx_scu_irq_register_notifier(struct notifier_block *nb)
 {
-	return atomic_notifier_chain_register(
+	return blocking_notifier_chain_register(
 		&imx_scu_irq_notifier_chain, nb);
 }
 EXPORT_SYMBOL(imx_scu_irq_register_notifier);
 
 int imx_scu_irq_unregister_notifier(struct notifier_block *nb)
 {
-	return atomic_notifier_chain_unregister(
+	return blocking_notifier_chain_unregister(
 		&imx_scu_irq_notifier_chain, nb);
 }
 EXPORT_SYMBOL(imx_scu_irq_unregister_notifier);
 
 static int imx_scu_irq_notifier_call_chain(unsigned long status, u8 *group)
 {
-	return atomic_notifier_call_chain(&imx_scu_irq_notifier_chain,
+	return blocking_notifier_call_chain(&imx_scu_irq_notifier_chain,
 		status, (void *)group);
 }
 
-- 
2.37.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 6/9] firmware: imx: scu-irq: fix RCU complains after M4 partition reset

[Shawn Guo]

On Mon, Jul 31, 2023 at 05:04:46PM +0800, Peng Fan (OSS) wrote:
> From: Robin Gong <yibin.gong@nxp.com>
> 
> Use blocking_notifier_chain instead of atomic_notifier_chain, otherwise
> below RCU complains would come out since unregister/register_virtio_device

s/complains/complaint, and in subject too.

> () will issue mbox message (mbox_send_message() is blocking) again after

The () should be on the same line as register_virtio_device.

> received M4 partition reset. Actually, no need atomic for notifier which

there is no need of atomic for ...?

Shawn

> is so tough for user since this notifier is called in worker instead of
> interrupt handler directly.
> 
> [  389.706645] i2c-rpmsg virtio0.rpmsg-i2c-channel.-1.2: i2c rpmsg driver is removed
> [  389.767362] Wait for remote ready timeout, use first_notify.
> [  389.774084] ------------[ cut here ]------------
> [  389.778729] WARNING: CPU: 0 PID: 397 at kernel/rcu/tree_plugin.h:293 rcu_note_context_switch+0x34/0x338
> [  389.788131] Modules linked in:
> [  389.791195] CPU: 0 PID: 397 Comm: kworker/0:13 Not tainted 5.4.0-rc5-02977-g08f78722f07b #26
> [  389.799633] Hardware name: Freescale i.MX8DXL Phantom MEK (DT)
> [  389.805481] Workqueue: events imx_scu_irq_work_handler
> 
> Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/firmware/imx/imx-scu-irq.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/firmware/imx/imx-scu-irq.c b/drivers/firmware/imx/imx-scu-irq.c
> index d9dcc20945c6..4408f150b3d5 100644
> --- a/drivers/firmware/imx/imx-scu-irq.c
> +++ b/drivers/firmware/imx/imx-scu-irq.c
> @@ -42,25 +42,25 @@ struct imx_sc_msg_irq_enable {
>  
>  static struct imx_sc_ipc *imx_sc_irq_ipc_handle;
>  static struct work_struct imx_sc_irq_work;
> -static ATOMIC_NOTIFIER_HEAD(imx_scu_irq_notifier_chain);
> +static BLOCKING_NOTIFIER_HEAD(imx_scu_irq_notifier_chain);
>  
>  int imx_scu_irq_register_notifier(struct notifier_block *nb)
>  {
> -	return atomic_notifier_chain_register(
> +	return blocking_notifier_chain_register(
>  		&imx_scu_irq_notifier_chain, nb);
>  }
>  EXPORT_SYMBOL(imx_scu_irq_register_notifier);
>  
>  int imx_scu_irq_unregister_notifier(struct notifier_block *nb)
>  {
> -	return atomic_notifier_chain_unregister(
> +	return blocking_notifier_chain_unregister(
>  		&imx_scu_irq_notifier_chain, nb);
>  }
>  EXPORT_SYMBOL(imx_scu_irq_unregister_notifier);
>  
>  static int imx_scu_irq_notifier_call_chain(unsigned long status, u8 *group)
>  {
> -	return atomic_notifier_call_chain(&imx_scu_irq_notifier_chain,
> +	return blocking_notifier_call_chain(&imx_scu_irq_notifier_chain,
>  		status, (void *)group);
>  }
>  
> -- 
> 2.37.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH V5 6/9] firmware: imx: scu-irq: fix RCU complains after M4 partition reset

[Peng Fan]

> Subject: Re: [PATCH V5 6/9] firmware: imx: scu-irq: fix RCU complains after
> M4 partition reset
> 
> On Mon, Jul 31, 2023 at 05:04:46PM +0800, Peng Fan (OSS) wrote:
> > From: Robin Gong <yibin.gong@nxp.com>
> >
> > Use blocking_notifier_chain instead of atomic_notifier_chain,
> > otherwise below RCU complains would come out since
> > unregister/register_virtio_device
> 
> s/complains/complaint, and in subject too.
> 
> > () will issue mbox message (mbox_send_message() is blocking) again
> > after
> 
> The () should be on the same line as register_virtio_device.
> 
> > received M4 partition reset. Actually, no need atomic for notifier
> > which
> 
> there is no need of atomic for ...?

I'll rephase the commit log in v6. There is no need to use atomic
notifier for scu irq notification.

Thanks,
Peng.

> 
> Shawn
> 
> > is so tough for user since this notifier is called in worker instead
> > of interrupt handler directly.
> >
> > [  389.706645] i2c-rpmsg virtio0.rpmsg-i2c-channel.-1.2: i2c rpmsg
> > driver is removed [  389.767362] Wait for remote ready timeout, use
> first_notify.
> > [  389.774084] ------------[ cut here ]------------ [  389.778729]
> > WARNING: CPU: 0 PID: 397 at kernel/rcu/tree_plugin.h:293
> > rcu_note_context_switch+0x34/0x338
> > [  389.788131] Modules linked in:
> > [  389.791195] CPU: 0 PID: 397 Comm: kworker/0:13 Not tainted
> > 5.4.0-rc5-02977-g08f78722f07b #26 [  389.799633] Hardware name:
> > Freescale i.MX8DXL Phantom MEK (DT) [  389.805481] Workqueue: events
> > imx_scu_irq_work_handler
> >
> > Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> > Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  drivers/firmware/imx/imx-scu-irq.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/firmware/imx/imx-scu-irq.c
> > b/drivers/firmware/imx/imx-scu-irq.c
> > index d9dcc20945c6..4408f150b3d5 100644
> > --- a/drivers/firmware/imx/imx-scu-irq.c
> > +++ b/drivers/firmware/imx/imx-scu-irq.c
> > @@ -42,25 +42,25 @@ struct imx_sc_msg_irq_enable {
> >
> >  static struct imx_sc_ipc *imx_sc_irq_ipc_handle;  static struct
> > work_struct imx_sc_irq_work; -static
> > ATOMIC_NOTIFIER_HEAD(imx_scu_irq_notifier_chain);
> > +static BLOCKING_NOTIFIER_HEAD(imx_scu_irq_notifier_chain);
> >
> >  int imx_scu_irq_register_notifier(struct notifier_block *nb)  {
> > -	return atomic_notifier_chain_register(
> > +	return blocking_notifier_chain_register(
> >  		&imx_scu_irq_notifier_chain, nb);
> >  }
> >  EXPORT_SYMBOL(imx_scu_irq_register_notifier);
> >
> >  int imx_scu_irq_unregister_notifier(struct notifier_block *nb)  {
> > -	return atomic_notifier_chain_unregister(
> > +	return blocking_notifier_chain_unregister(
> >  		&imx_scu_irq_notifier_chain, nb);
> >  }
> >  EXPORT_SYMBOL(imx_scu_irq_unregister_notifier);
> >
> >  static int imx_scu_irq_notifier_call_chain(unsigned long status, u8
> > *group)  {
> > -	return atomic_notifier_call_chain(&imx_scu_irq_notifier_chain,
> > +	return blocking_notifier_call_chain(&imx_scu_irq_notifier_chain,
> >  		status, (void *)group);
> >  }
> >
> > --
> > 2.37.1
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V5 7/9] firmware: imx: scu-irq: export imx_scu_irq_get_status

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

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 <peng.fan@nxp.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 7/9] firmware: imx: scu-irq: export imx_scu_irq_get_status

[Shawn Guo]

On Mon, Jul 31, 2023 at 05:04:47PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> 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 <peng.fan@nxp.com>
> ---
>  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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V5 8/9] firmware: imx: scu-irq: enlarge the IMX_SC_IRQ_NUM_GROUP

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Per SCFW update, update the IMX_SC_IRQ_NUM_GROUP to 9.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V5 9/9] firmware: imx: scu-irq: support identifying SCU wakeup source from sysfs

[Peng Fan (OSS)]

From: Ranjani Vaidyanathan <ranjani.vaidyanathan@nxp.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@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.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>
 
 #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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V5 9/9] firmware: imx: scu-irq: support identifying SCU wakeup source from sysfs

[Shawn Guo]

On Mon, Jul 31, 2023 at 05:04:49PM +0800, Peng Fan (OSS) wrote:
> From: Ranjani Vaidyanathan <ranjani.vaidyanathan@nxp.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@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel