* [PATCH 2/2] firmware: imx: MU IRQ group number should be 7
2020-04-23 23:07 [PATCH 1/2] firmware: imx: make sure MU irq can wake up system from suspend mode Anson Huang
@ 2020-04-23 23:07 ` Anson Huang
2020-04-24 2:32 ` Aisheng Dong
2020-04-24 2:38 ` [PATCH 1/2] firmware: imx: make sure MU irq can wake up system from suspend mode Aisheng Dong
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Anson Huang @ 2020-04-23 23:07 UTC (permalink / raw)
To: shawnguo, s.hauer, kernel, festevam, ben.dooks, linux-arm-kernel,
linux-kernel
Cc: Linux-imx
The MU IRQ group number should be 7 instead of 4.
Signed-off-by: Anson Huang <Anson.Huang@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 d9dcc20..66fef88 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 7
static u32 mu_resource_id;
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* RE: [PATCH 2/2] firmware: imx: MU IRQ group number should be 7
2020-04-23 23:07 ` [PATCH 2/2] firmware: imx: MU IRQ group number should be 7 Anson Huang
@ 2020-04-24 2:32 ` Aisheng Dong
2020-04-24 2:36 ` Anson Huang
0 siblings, 1 reply; 10+ messages in thread
From: Aisheng Dong @ 2020-04-24 2:32 UTC (permalink / raw)
To: Anson Huang, shawnguo, s.hauer, kernel, festevam, ben.dooks,
linux-arm-kernel, linux-kernel
Cc: dl-linux-imx
> From: Anson Huang <Anson.Huang@nxp.com>
> Sent: Friday, April 24, 2020 7:07 AM
>
> The MU IRQ group number should be 7 instead of 4.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Are we using others IRQ group?
If not, this change may slow down the irq handling speed.
Regards
Aisheng
> ---
> 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 d9dcc20..66fef88 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 7
>
> static u32 mu_resource_id;
>
> --
> 2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 2/2] firmware: imx: MU IRQ group number should be 7
2020-04-24 2:32 ` Aisheng Dong
@ 2020-04-24 2:36 ` Anson Huang
2020-04-24 2:51 ` Aisheng Dong
0 siblings, 1 reply; 10+ messages in thread
From: Anson Huang @ 2020-04-24 2:36 UTC (permalink / raw)
To: Aisheng Dong, shawnguo, s.hauer, kernel, festevam, ben.dooks,
linux-arm-kernel, linux-kernel
Cc: dl-linux-imx
> -----Original Message-----
> From: Aisheng Dong <aisheng.dong@nxp.com>
> Sent: 2020年4月24日 10:33
> To: Anson Huang <anson.huang@nxp.com>; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> ben.dooks@codethink.co.uk; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org
> Cc: dl-linux-imx <linux-imx@nxp.com>
> Subject: RE: [PATCH 2/2] firmware: imx: MU IRQ group number should be 7
>
> > From: Anson Huang <Anson.Huang@nxp.com>
> > Sent: Friday, April 24, 2020 7:07 AM
> >
> > The MU IRQ group number should be 7 instead of 4.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>
> Are we using others IRQ group?
> If not, this change may slow down the irq handling speed.
The irq handling is using work queue, NOT in ISR, so the speed is NOT that sensitive.
The scu group irq driver should provide full functions, as other drivers using it may enable
the group they want.
Anson
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 2/2] firmware: imx: MU IRQ group number should be 7
2020-04-24 2:36 ` Anson Huang
@ 2020-04-24 2:51 ` Aisheng Dong
2020-04-24 2:53 ` Anson Huang
0 siblings, 1 reply; 10+ messages in thread
From: Aisheng Dong @ 2020-04-24 2:51 UTC (permalink / raw)
To: Anson Huang, shawnguo, s.hauer, kernel, festevam, ben.dooks,
linux-arm-kernel, linux-kernel
Cc: dl-linux-imx
> From: Anson Huang <anson.huang@nxp.com>
> Sent: Friday, April 24, 2020 10:36 AM
>
> > -----Original Message-----
> > From: Aisheng Dong <aisheng.dong@nxp.com>
> > Sent: 2020年4月24日 10:33
> > To: Anson Huang <anson.huang@nxp.com>; shawnguo@kernel.org;
> > s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> > ben.dooks@codethink.co.uk; linux-arm-kernel@lists.infradead.org;
> > linux-kernel@vger.kernel.org
> > Cc: dl-linux-imx <linux-imx@nxp.com>
> > Subject: RE: [PATCH 2/2] firmware: imx: MU IRQ group number should be
> > 7
> >
> > > From: Anson Huang <Anson.Huang@nxp.com>
> > > Sent: Friday, April 24, 2020 7:07 AM
> > >
> > > The MU IRQ group number should be 7 instead of 4.
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> >
> > Are we using others IRQ group?
> > If not, this change may slow down the irq handling speed.
>
> The irq handling is using work queue, NOT in ISR, so the speed is NOT that
> sensitive.
SCU IPC is shared by the whole system, each SCU transfer takes about 10~20 us.
Here you may waste 30~60us if not really used.
> The scu group irq driver should provide full functions, as other drivers using it
> may enable the group they want.
Below are extra GROUPs you're going to add:
#define SC_IRQ_GROUP_SYSCTR 4U /*!< System counter interrupts */
#define SC_IRQ_GROUP_REBOOTED 5U /*!< Partition reboot complete */
#define SC_IRQ_GROUP_REBOOT 6U /*!< Partition reboot starting */
Are we really going to use it? It seems I also didn't see any users in downstream tree.
Some functions provided by SCFW may not really used by Linux.
I think I's better to add them when we really need them, otherwise we benefit nothing
But wasting CPU mips.
Regards
Aisheng
>
> Anson
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 2/2] firmware: imx: MU IRQ group number should be 7
2020-04-24 2:51 ` Aisheng Dong
@ 2020-04-24 2:53 ` Anson Huang
2020-04-24 3:15 ` Aisheng Dong
0 siblings, 1 reply; 10+ messages in thread
From: Anson Huang @ 2020-04-24 2:53 UTC (permalink / raw)
To: Aisheng Dong, shawnguo, s.hauer, kernel, festevam, ben.dooks,
linux-arm-kernel, linux-kernel
Cc: dl-linux-imx
> Subject: RE: [PATCH 2/2] firmware: imx: MU IRQ group number should be 7
>
> > From: Anson Huang <anson.huang@nxp.com>
> > Sent: Friday, April 24, 2020 10:36 AM
> >
> > > -----Original Message-----
> > > From: Aisheng Dong <aisheng.dong@nxp.com>
> > > Sent: 2020年4月24日 10:33
> > > To: Anson Huang <anson.huang@nxp.com>; shawnguo@kernel.org;
> > > s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> > > ben.dooks@codethink.co.uk; linux-arm-kernel@lists.infradead.org;
> > > linux-kernel@vger.kernel.org
> > > Cc: dl-linux-imx <linux-imx@nxp.com>
> > > Subject: RE: [PATCH 2/2] firmware: imx: MU IRQ group number should
> > > be
> > > 7
> > >
> > > > From: Anson Huang <Anson.Huang@nxp.com>
> > > > Sent: Friday, April 24, 2020 7:07 AM
> > > >
> > > > The MU IRQ group number should be 7 instead of 4.
> > > >
> > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > >
> > > Are we using others IRQ group?
> > > If not, this change may slow down the irq handling speed.
> >
> > The irq handling is using work queue, NOT in ISR, so the speed is NOT
> > that sensitive.
>
> SCU IPC is shared by the whole system, each SCU transfer takes about 10~20
> us.
> Here you may waste 30~60us if not really used.
>
> > The scu group irq driver should provide full functions, as other
> > drivers using it may enable the group they want.
>
> Below are extra GROUPs you're going to add:
> #define SC_IRQ_GROUP_SYSCTR 4U /*!< System counter interrupts */
> #define SC_IRQ_GROUP_REBOOTED 5U /*!< Partition reboot complete
> */
> #define SC_IRQ_GROUP_REBOOT 6U /*!< Partition reboot starting */
> Are we really going to use it? It seems I also didn't see any users in
> downstream tree.
>
> Some functions provided by SCFW may not really used by Linux.
> I think I's better to add them when we really need them, otherwise we benefit
> nothing But wasting CPU mips.
I don't agree this, if SCFW NOT support it, it should fix from SCFW. This is aligned with our
internal tree.
Anson
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 2/2] firmware: imx: MU IRQ group number should be 7
2020-04-24 2:53 ` Anson Huang
@ 2020-04-24 3:15 ` Aisheng Dong
0 siblings, 0 replies; 10+ messages in thread
From: Aisheng Dong @ 2020-04-24 3:15 UTC (permalink / raw)
To: Anson Huang, shawnguo, s.hauer, kernel, festevam, ben.dooks,
linux-arm-kernel, linux-kernel
Cc: dl-linux-imx
> From: Anson Huang <anson.huang@nxp.com>
> Sent: Friday, April 24, 2020 10:54 AM
>
> > Subject: RE: [PATCH 2/2] firmware: imx: MU IRQ group number should be
> > 7
> >
> > > From: Anson Huang <anson.huang@nxp.com>
> > > Sent: Friday, April 24, 2020 10:36 AM
> > >
> > > > -----Original Message-----
> > > > From: Aisheng Dong <aisheng.dong@nxp.com>
> > > > Sent: 2020年4月24日 10:33
> > > > To: Anson Huang <anson.huang@nxp.com>; shawnguo@kernel.org;
> > > > s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> > > > ben.dooks@codethink.co.uk; linux-arm-kernel@lists.infradead.org;
> > > > linux-kernel@vger.kernel.org
> > > > Cc: dl-linux-imx <linux-imx@nxp.com>
> > > > Subject: RE: [PATCH 2/2] firmware: imx: MU IRQ group number should
> > > > be
> > > > 7
> > > >
> > > > > From: Anson Huang <Anson.Huang@nxp.com>
> > > > > Sent: Friday, April 24, 2020 7:07 AM
> > > > >
> > > > > The MU IRQ group number should be 7 instead of 4.
> > > > >
> > > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > >
> > > > Are we using others IRQ group?
> > > > If not, this change may slow down the irq handling speed.
> > >
> > > The irq handling is using work queue, NOT in ISR, so the speed is
> > > NOT that sensitive.
> >
> > SCU IPC is shared by the whole system, each SCU transfer takes about
> > 10~20 us.
> > Here you may waste 30~60us if not really used.
> >
> > > The scu group irq driver should provide full functions, as other
> > > drivers using it may enable the group they want.
> >
> > Below are extra GROUPs you're going to add:
> > #define SC_IRQ_GROUP_SYSCTR 4U /*!< System counter interrupts */
> > #define SC_IRQ_GROUP_REBOOTED 5U /*!< Partition reboot complete
> > */
> > #define SC_IRQ_GROUP_REBOOT 6U /*!< Partition reboot starting */
> > Are we really going to use it? It seems I also didn't see any users in
> > downstream tree.
> >
> > Some functions provided by SCFW may not really used by Linux.
> > I think I's better to add them when we really need them, otherwise we
> > benefit nothing But wasting CPU mips.
>
> I don't agree this, if SCFW NOT support it, it should fix from SCFW. This is aligned
> with our internal tree.
Hmm, internal tree does not decide upstream tree. That's a lesson we've already learned
for many years.
For example, we only upstream SCU API really used by Linux.
Not all of them as we simply did for local 4.19 release.
Regards
Aisheng
>
> Anson
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 1/2] firmware: imx: make sure MU irq can wake up system from suspend mode
2020-04-23 23:07 [PATCH 1/2] firmware: imx: make sure MU irq can wake up system from suspend mode Anson Huang
2020-04-23 23:07 ` [PATCH 2/2] firmware: imx: MU IRQ group number should be 7 Anson Huang
@ 2020-04-24 2:38 ` Aisheng Dong
2020-05-28 1:54 ` Anson Huang
2020-06-17 13:48 ` Shawn Guo
3 siblings, 0 replies; 10+ messages in thread
From: Aisheng Dong @ 2020-04-24 2:38 UTC (permalink / raw)
To: Anson Huang, shawnguo, s.hauer, kernel, festevam, ben.dooks,
linux-arm-kernel, linux-kernel
Cc: dl-linux-imx
> From: Anson Huang <Anson.Huang@nxp.com>
> Sent: Friday, April 24, 2020 7:07 AM
>
> IRQF_NO_SUSPEND flag is set for MU IRQ of IPC work, but with this flag set,
> IRQD_WAKEUP_ARMED flag will NOT be set during
> suspend_device_irq() phase, then when MU IRQ arrives, it will NOT wake up
> system from suspend.
>
> To fix this issue, pm_system_wakeup() is called in general MU IRQ handler to
> make sure system can be waked up when MU IRQ arrives.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
Regards
Aisheng
> ---
> drivers/firmware/imx/imx-scu-irq.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/firmware/imx/imx-scu-irq.c
> b/drivers/firmware/imx/imx-scu-irq.c
> index db655e8..d9dcc20 100644
> --- a/drivers/firmware/imx/imx-scu-irq.c
> +++ b/drivers/firmware/imx/imx-scu-irq.c
> @@ -10,6 +10,7 @@
> #include <linux/firmware/imx/ipc.h>
> #include <linux/firmware/imx/sci.h>
> #include <linux/mailbox_client.h>
> +#include <linux/suspend.h>
>
> #define IMX_SC_IRQ_FUNC_ENABLE 1
> #define IMX_SC_IRQ_FUNC_STATUS 2
> @@ -91,6 +92,7 @@ static void imx_scu_irq_work_handler(struct work_struct
> *work)
> if (!irq_status)
> continue;
>
> + pm_system_wakeup();
> imx_scu_irq_notifier_call_chain(irq_status, &i);
> }
> }
> --
> 2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 1/2] firmware: imx: make sure MU irq can wake up system from suspend mode
2020-04-23 23:07 [PATCH 1/2] firmware: imx: make sure MU irq can wake up system from suspend mode Anson Huang
2020-04-23 23:07 ` [PATCH 2/2] firmware: imx: MU IRQ group number should be 7 Anson Huang
2020-04-24 2:38 ` [PATCH 1/2] firmware: imx: make sure MU irq can wake up system from suspend mode Aisheng Dong
@ 2020-05-28 1:54 ` Anson Huang
2020-06-17 13:48 ` Shawn Guo
3 siblings, 0 replies; 10+ messages in thread
From: Anson Huang @ 2020-05-28 1:54 UTC (permalink / raw)
To: Anson Huang, shawnguo, s.hauer, kernel, festevam, ben.dooks,
linux-arm-kernel, linux-kernel
Cc: dl-linux-imx
Gentle ping...
> Subject: [PATCH 1/2] firmware: imx: make sure MU irq can wake up system
> from suspend mode
>
> IRQF_NO_SUSPEND flag is set for MU IRQ of IPC work, but with this flag set,
> IRQD_WAKEUP_ARMED flag will NOT be set during
> suspend_device_irq() phase, then when MU IRQ arrives, it will NOT wake up
> system from suspend.
>
> To fix this issue, pm_system_wakeup() is called in general MU IRQ handler to
> make sure system can be waked up when MU IRQ arrives.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> drivers/firmware/imx/imx-scu-irq.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/firmware/imx/imx-scu-irq.c
> b/drivers/firmware/imx/imx-scu-irq.c
> index db655e8..d9dcc20 100644
> --- a/drivers/firmware/imx/imx-scu-irq.c
> +++ b/drivers/firmware/imx/imx-scu-irq.c
> @@ -10,6 +10,7 @@
> #include <linux/firmware/imx/ipc.h>
> #include <linux/firmware/imx/sci.h>
> #include <linux/mailbox_client.h>
> +#include <linux/suspend.h>
>
> #define IMX_SC_IRQ_FUNC_ENABLE 1
> #define IMX_SC_IRQ_FUNC_STATUS 2
> @@ -91,6 +92,7 @@ static void imx_scu_irq_work_handler(struct work_struct
> *work)
> if (!irq_status)
> continue;
>
> + pm_system_wakeup();
> imx_scu_irq_notifier_call_chain(irq_status, &i);
> }
> }
> --
> 2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] firmware: imx: make sure MU irq can wake up system from suspend mode
2020-04-23 23:07 [PATCH 1/2] firmware: imx: make sure MU irq can wake up system from suspend mode Anson Huang
` (2 preceding siblings ...)
2020-05-28 1:54 ` Anson Huang
@ 2020-06-17 13:48 ` Shawn Guo
3 siblings, 0 replies; 10+ messages in thread
From: Shawn Guo @ 2020-06-17 13:48 UTC (permalink / raw)
To: Anson Huang
Cc: s.hauer, kernel, festevam, ben.dooks, linux-arm-kernel,
linux-kernel, Linux-imx
On Fri, Apr 24, 2020 at 07:07:10AM +0800, Anson Huang wrote:
> IRQF_NO_SUSPEND flag is set for MU IRQ of IPC work, but with this
> flag set, IRQD_WAKEUP_ARMED flag will NOT be set during
> suspend_device_irq() phase, then when MU IRQ arrives, it will NOT
> wake up system from suspend.
>
> To fix this issue, pm_system_wakeup() is called in general MU IRQ
> handler to make sure system can be waked up when MU IRQ arrives.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
I see you guys haven't got agreement on patch #2, so applied #1 only.
Shawn
^ permalink raw reply [flat|nested] 10+ messages in thread