Add scu general interrupt function support.
Signed-off-by: Anson Huang <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Reviewed-by: Dong Aisheng <[email protected]>
---
Changes since V4:
- add mu aliase info and example in binding doc.
---
.../devicetree/bindings/arm/freescale/fsl,scu.txt | 29 +++++++++++++++++-----
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
index 72d481c..5d7dbab 100644
--- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
+++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
@@ -22,9 +22,11 @@ Required properties:
-------------------
- compatible: should be "fsl,imx-scu".
- mbox-names: should include "tx0", "tx1", "tx2", "tx3",
- "rx0", "rx1", "rx2", "rx3".
-- mboxes: List of phandle of 4 MU channels for tx and 4 MU channels
- for rx. All 8 MU channels must be in the same MU instance.
+ "rx0", "rx1", "rx2", "rx3";
+ include "gip3" if want to support general MU interrupt.
+- mboxes: List of phandle of 4 MU channels for tx, 4 MU channels for
+ rx, and 1 optional MU channel for general interrupt.
+ All MU channels must be in the same MU instance.
Cross instances are not allowed. The MU instance can only
be one of LSIO MU0~M4 for imx8qxp and imx8qm. Users need
to make sure use the one which is not conflict with other
@@ -34,6 +36,7 @@ Required properties:
Channel 1 must be "tx1" or "rx1".
Channel 2 must be "tx2" or "rx2".
Channel 3 must be "tx3" or "rx3".
+ General interrupt rx channel must be "gip3".
e.g.
mboxes = <&lsio_mu1 0 0
&lsio_mu1 0 1
@@ -42,10 +45,18 @@ Required properties:
&lsio_mu1 1 0
&lsio_mu1 1 1
&lsio_mu1 1 2
- &lsio_mu1 1 3>;
+ &lsio_mu1 1 3
+ &lsio_mu1 3 3>;
See Documentation/devicetree/bindings/mailbox/fsl,mu.txt
for detailed mailbox binding.
+Note: Each mu which supports general interrupt should have an alias correctly
+numbered in "aliases" node.
+e.g.
+aliases {
+ mu1 = &lsio_mu1;
+};
+
i.MX SCU Client Device Node:
============================================================
@@ -124,6 +135,10 @@ Required properties:
Example (imx8qxp):
-------------
+aliases {
+ mu1 = &lsio_mu1;
+};
+
lsio_mu1: mailbox@5d1c0000 {
...
#mbox-cells = <2>;
@@ -133,7 +148,8 @@ firmware {
scu {
compatible = "fsl,imx-scu";
mbox-names = "tx0", "tx1", "tx2", "tx3",
- "rx0", "rx1", "rx2", "rx3";
+ "rx0", "rx1", "rx2", "rx3",
+ "gip3";
mboxes = <&lsio_mu1 0 0
&lsio_mu1 0 1
&lsio_mu1 0 2
@@ -141,7 +157,8 @@ firmware {
&lsio_mu1 1 0
&lsio_mu1 1 1
&lsio_mu1 1 2
- &lsio_mu1 1 3>;
+ &lsio_mu1 1 3
+ &lsio_mu1 3 3>;
clk: clk {
compatible = "fsl,imx8qxp-clk", "fsl,scu-clk";
--
2.7.4
On i.MX8QXP, SCU uses MU1 general interrupt channel #3 to notify
user for IRQs of RTC alarm, thermal alarm and WDOG etc., mailbox
RX doorbell mode is used for this function, this patch adds
support for it.
Signed-off-by: Anson Huang <[email protected]>
Reviewed-by: Dong Aisheng <[email protected]>
---
No changes.
---
arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 4c3dd95..f0a9224 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -21,6 +21,7 @@
mmc1 = &usdhc2;
mmc2 = &usdhc3;
serial0 = &adma_lpuart0;
+ mu1 = &lsio_mu1;
};
cpus {
@@ -87,7 +88,8 @@
scu {
compatible = "fsl,imx-scu";
mbox-names = "tx0", "tx1", "tx2", "tx3",
- "rx0", "rx1", "rx2", "rx3";
+ "rx0", "rx1", "rx2", "rx3",
+ "gip3";
mboxes = <&lsio_mu1 0 0
&lsio_mu1 0 1
&lsio_mu1 0 2
@@ -95,7 +97,8 @@
&lsio_mu1 1 0
&lsio_mu1 1 1
&lsio_mu1 1 2
- &lsio_mu1 1 3>;
+ &lsio_mu1 1 3
+ &lsio_mu1 3 3>;
clk: clock-controller {
compatible = "fsl,imx8qxp-clk";
--
2.7.4
Add i.MX system controller RTC alarm support, the RTC alarm
is implemented via SIP(silicon provider) runtime service call
and ARM-Trusted-Firmware will communicate with system controller
via MU(message unit) IPC to set RTC alarm. When RTC alarm fires,
system controller will generate a common MU irq event and notify
system controller RTC driver to handle the irq event.
Signed-off-by: Anson Huang <[email protected]>
---
No function changes, just update imx_scu_irq_register_notifier() function name
according to SCU IRQ function name change.
---
drivers/rtc/rtc-imx-sc.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 112 insertions(+)
diff --git a/drivers/rtc/rtc-imx-sc.c b/drivers/rtc/rtc-imx-sc.c
index 19642bf..9df4990 100644
--- a/drivers/rtc/rtc-imx-sc.c
+++ b/drivers/rtc/rtc-imx-sc.c
@@ -3,6 +3,7 @@
* Copyright 2018 NXP.
*/
+#include <dt-bindings/firmware/imx/rsrc.h>
#include <linux/arm-smccc.h>
#include <linux/firmware/imx/sci.h>
#include <linux/module.h>
@@ -11,11 +12,17 @@
#include <linux/rtc.h>
#define IMX_SC_TIMER_FUNC_GET_RTC_SEC1970 9
+#define IMX_SC_TIMER_FUNC_SET_RTC_ALARM 8
#define IMX_SC_TIMER_FUNC_SET_RTC_TIME 6
+#define IMX_SC_IRQ_FUNC_ENABLE 1
+
#define IMX_SIP_SRTC 0xC2000002
#define IMX_SIP_SRTC_SET_TIME 0x0
+#define SC_IRQ_GROUP_RTC 2
+#define SC_IRQ_RTC 1
+
static struct imx_sc_ipc *rtc_ipc_handle;
static struct rtc_device *imx_sc_rtc;
@@ -24,6 +31,24 @@ struct imx_sc_msg_timer_get_rtc_time {
u32 time;
} __packed;
+struct imx_sc_msg_timer_enable_irq {
+ struct imx_sc_rpc_msg hdr;
+ u32 mask;
+ u16 resource;
+ u8 group;
+ u8 enable;
+} __packed;
+
+struct imx_sc_msg_timer_rtc_set_alarm {
+ struct imx_sc_rpc_msg hdr;
+ u16 year;
+ u8 mon;
+ u8 day;
+ u8 hour;
+ u8 min;
+ u8 sec;
+} __packed;
+
static int imx_sc_rtc_read_time(struct device *dev, struct rtc_time *tm)
{
struct imx_sc_msg_timer_get_rtc_time msg;
@@ -60,9 +85,92 @@ static int imx_sc_rtc_set_time(struct device *dev, struct rtc_time *tm)
return res.a0;
}
+static int imx_sc_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
+{
+ struct imx_sc_msg_timer_enable_irq 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_ENABLE;
+ hdr->size = 3;
+
+ msg.resource = IMX_SC_R_MU_1A;
+ msg.group = SC_IRQ_GROUP_RTC;
+ msg.mask = SC_IRQ_RTC;
+ msg.enable = enable;
+
+ ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
+ if (ret) {
+ dev_err(dev, "enable rtc irq failed, ret %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int imx_sc_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+ return 0;
+}
+
+static int imx_sc_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+ struct imx_sc_msg_timer_rtc_set_alarm msg;
+ struct imx_sc_rpc_msg *hdr = &msg.hdr;
+ int ret;
+ struct rtc_time *alrm_tm = &alrm->time;
+
+ hdr->ver = IMX_SC_RPC_VERSION;
+ hdr->svc = IMX_SC_RPC_SVC_TIMER;
+ hdr->func = IMX_SC_TIMER_FUNC_SET_RTC_ALARM;
+ hdr->size = 3;
+
+ msg.year = alrm_tm->tm_year + 1900;
+ msg.mon = alrm_tm->tm_mon + 1;
+ msg.day = alrm_tm->tm_mday;
+ msg.hour = alrm_tm->tm_hour;
+ msg.min = alrm_tm->tm_min;
+ msg.sec = alrm_tm->tm_sec;
+
+ ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
+ if (ret) {
+ dev_err(dev, "set rtc alarm failed, ret %d\n", ret);
+ return ret;
+ }
+
+ ret = imx_sc_rtc_alarm_irq_enable(dev, alrm->enabled);
+ if (ret) {
+ dev_err(dev, "enable rtc alarm failed, ret %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
static const struct rtc_class_ops imx_sc_rtc_ops = {
.read_time = imx_sc_rtc_read_time,
.set_time = imx_sc_rtc_set_time,
+ .read_alarm = imx_sc_rtc_read_alarm,
+ .set_alarm = imx_sc_rtc_set_alarm,
+ .alarm_irq_enable = imx_sc_rtc_alarm_irq_enable,
+};
+
+static int imx_sc_rtc_alarm_sc_notify(struct notifier_block *nb,
+ unsigned long event, void *group)
+{
+ /* ignore non-rtc irq */
+ if (!((event & SC_IRQ_RTC) && (*(u8 *)group == SC_IRQ_GROUP_RTC)))
+ return 0;
+
+ rtc_update_irq(imx_sc_rtc, 1, RTC_IRQF | RTC_AF);
+
+ return 0;
+}
+
+static struct notifier_block imx_sc_rtc_alarm_sc_notifier = {
+ .notifier_call = imx_sc_rtc_alarm_sc_notify,
};
static int imx_sc_rtc_probe(struct platform_device *pdev)
@@ -73,6 +181,8 @@ static int imx_sc_rtc_probe(struct platform_device *pdev)
if (ret)
return ret;
+ device_init_wakeup(&pdev->dev, true);
+
imx_sc_rtc = devm_rtc_allocate_device(&pdev->dev);
if (IS_ERR(imx_sc_rtc))
return PTR_ERR(imx_sc_rtc);
@@ -87,6 +197,8 @@ static int imx_sc_rtc_probe(struct platform_device *pdev)
return ret;
}
+ imx_scu_irq_register_notifier(&imx_sc_rtc_alarm_sc_notifier);
+
return 0;
}
--
2.7.4
The System Controller Firmware (SCFW) controls RTC, thermal
and WDOG etc., these resources' interrupt function are managed
by SCU. When any IRQ pending, SCU will notify Linux via MU general
interrupt channel #3, and Linux kernel needs to call SCU APIs
to get IRQ status and notify each module to handle the interrupt.
Since there is no data transmission for SCU IRQ notification, so
doorbell mode is used for this MU channel, and SCU driver will
use notifier mechanism to broadcast to every module which registers
the SCU block notifier.
Signed-off-by: Anson Huang <[email protected]>
---
Changes since V4:
- move scu irq support to a new file imx-scu-irq.c;
- improve the function name with "imx_scu_irq_" as prefix for irq related functions;
- move MU IRQ sources id out of sc ipc struct;
- add IRQ group info message print when get irq status fail;
- move MU ID parse into imx_scu_enable_general_irq_channel() function and ONLY do it
when irq channel enable successfully.
- change max IRQ group supported to 4, ONLY support for those kernel features.
---
drivers/firmware/imx/Makefile | 2 +-
drivers/firmware/imx/imx-scu-irq.c | 133 +++++++++++++++++++++++++++++++++++++
drivers/firmware/imx/imx-scu.c | 6 ++
include/linux/firmware/imx/sci.h | 4 ++
4 files changed, 144 insertions(+), 1 deletion(-)
create mode 100644 drivers/firmware/imx/imx-scu-irq.c
diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
index 1b2e15b..802c4ad 100644
--- a/drivers/firmware/imx/Makefile
+++ b/drivers/firmware/imx/Makefile
@@ -1,3 +1,3 @@
# SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o
+obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
diff --git a/drivers/firmware/imx/imx-scu-irq.c b/drivers/firmware/imx/imx-scu-irq.c
new file mode 100644
index 0000000..0e20aa7
--- /dev/null
+++ b/drivers/firmware/imx/imx-scu-irq.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2019 NXP
+ *
+ * Implementation of the SCU IRQ functions using MU.
+ *
+ */
+
+#include <dt-bindings/firmware/imx/rsrc.h>
+#include <linux/firmware/imx/ipc.h>
+#include <linux/mailbox_client.h>
+
+#define IMX_SC_IRQ_FUNC_STATUS 2
+#define IMX_SC_IRQ_NUM_GROUP 4
+
+static u32 mu_resource_id;
+
+struct imx_sc_msg_irq_get_status {
+ struct imx_sc_rpc_msg hdr;
+ union {
+ struct {
+ u16 resource;
+ u8 group;
+ u8 reserved;
+ } __packed req;
+ struct {
+ u32 status;
+ } resp;
+ } data;
+};
+
+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);
+
+int imx_scu_irq_register_notifier(struct notifier_block *nb)
+{
+ 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 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 blocking_notifier_call_chain(&imx_scu_irq_notifier_chain,
+ status, (void *)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);
+ 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;
+
+ imx_scu_irq_notifier_call_chain(irq_status, &i);
+ }
+}
+
+static void imx_scu_irq_callback(struct mbox_client *c, void *msg)
+{
+ schedule_work(&imx_sc_irq_work);
+}
+
+int imx_scu_enable_general_irq_channel(struct device *dev)
+{
+ struct of_phandle_args spec;
+ struct mbox_client *cl;
+ struct mbox_chan *ch;
+ int ret = 0, i = 0;
+
+ cl = devm_kzalloc(dev, sizeof(*cl), GFP_KERNEL);
+ if (!cl)
+ return -ENOMEM;
+
+ cl->dev = dev;
+ cl->rx_callback = imx_scu_irq_callback;
+
+ /* SCU general IRQ uses general interrupt channel 3 */
+ ch = mbox_request_channel_byname(cl, "gip3");
+ if (IS_ERR(ch)) {
+ ret = PTR_ERR(ch);
+ dev_err(dev, "failed to request mbox chan gip3, ret %d\n", ret);
+ return ret;
+ }
+
+ ret = imx_scu_get_handle(&imx_sc_irq_ipc_handle);
+ if (ret)
+ return ret;
+
+ 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))
+ i = of_alias_get_id(spec.np, "mu");
+
+ /* use mu1 as general mu irq channel if failed */
+ if (i < 0)
+ i = 1;
+
+ mu_resource_id = IMX_SC_R_MU_0A + i;
+
+ return ret;
+}
+EXPORT_SYMBOL(imx_scu_enable_general_irq_channel);
diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c
index 2bb1a19..04a24a8 100644
--- a/drivers/firmware/imx/imx-scu.c
+++ b/drivers/firmware/imx/imx-scu.c
@@ -10,6 +10,7 @@
#include <linux/err.h>
#include <linux/firmware/imx/types.h>
#include <linux/firmware/imx/ipc.h>
+#include <linux/firmware/imx/sci.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
#include <linux/kernel.h>
@@ -246,6 +247,11 @@ static int imx_scu_probe(struct platform_device *pdev)
imx_sc_ipc_handle = sc_ipc;
+ ret = imx_scu_enable_general_irq_channel(dev);
+ if (ret)
+ dev_warn(dev,
+ "failed to enable general irq channel: %d\n", ret);
+
dev_info(dev, "NXP i.MX SCU Initialized\n");
return devm_of_platform_populate(dev);
diff --git a/include/linux/firmware/imx/sci.h b/include/linux/firmware/imx/sci.h
index ebc5509..918fa16 100644
--- a/include/linux/firmware/imx/sci.h
+++ b/include/linux/firmware/imx/sci.h
@@ -15,4 +15,8 @@
#include <linux/firmware/imx/svc/misc.h>
#include <linux/firmware/imx/svc/pm.h>
+
+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);
#endif /* _SC_SCI_H */
--
2.7.4
Ping...
Best Regards!
Anson Huang
> -----Original Message-----
> From: Anson Huang [mailto:[email protected]]
> Sent: 2019??3??18?? 11:10
> To: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Aisheng Dong
> <[email protected]>; [email protected]; Daniel Baluta
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]
> Cc: dl-linux-imx <[email protected]>
> Subject: [PATCH V5 1/4] dt-bindings: fsl: scu: add general interrupt support
>
> Add scu general interrupt function support.
>
> Signed-off-by: Anson Huang <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> Reviewed-by: Dong Aisheng <[email protected]>
> ---
> Changes since V4:
> - add mu aliase info and example in binding doc.
> ---
> .../devicetree/bindings/arm/freescale/fsl,scu.txt | 29 +++++++++++++++++--
> ---
> 1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> index 72d481c..5d7dbab 100644
> --- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> @@ -22,9 +22,11 @@ Required properties:
> -------------------
> - compatible: should be "fsl,imx-scu".
> - mbox-names: should include "tx0", "tx1", "tx2", "tx3",
> - "rx0", "rx1", "rx2", "rx3".
> -- mboxes: List of phandle of 4 MU channels for tx and 4 MU channels
> - for rx. All 8 MU channels must be in the same MU instance.
> + "rx0", "rx1", "rx2", "rx3";
> + include "gip3" if want to support general MU interrupt.
> +- mboxes: List of phandle of 4 MU channels for tx, 4 MU channels for
> + rx, and 1 optional MU channel for general interrupt.
> + All MU channels must be in the same MU instance.
> Cross instances are not allowed. The MU instance can only
> be one of LSIO MU0~M4 for imx8qxp and imx8qm. Users
> need
> to make sure use the one which is not conflict with other @@
> -34,6 +36,7 @@ Required properties:
> Channel 1 must be "tx1" or "rx1".
> Channel 2 must be "tx2" or "rx2".
> Channel 3 must be "tx3" or "rx3".
> + General interrupt rx channel must be "gip3".
> e.g.
> mboxes = <&lsio_mu1 0 0
> &lsio_mu1 0 1
> @@ -42,10 +45,18 @@ Required properties:
> &lsio_mu1 1 0
> &lsio_mu1 1 1
> &lsio_mu1 1 2
> - &lsio_mu1 1 3>;
> + &lsio_mu1 1 3
> + &lsio_mu1 3 3>;
> See Documentation/devicetree/bindings/mailbox/fsl,mu.txt
> for detailed mailbox binding.
>
> +Note: Each mu which supports general interrupt should have an alias
> +correctly numbered in "aliases" node.
> +e.g.
> +aliases {
> + mu1 = &lsio_mu1;
> +};
> +
> i.MX SCU Client Device Node:
> ============================================================
>
> @@ -124,6 +135,10 @@ Required properties:
>
> Example (imx8qxp):
> -------------
> +aliases {
> + mu1 = &lsio_mu1;
> +};
> +
> lsio_mu1: mailbox@5d1c0000 {
> ...
> #mbox-cells = <2>;
> @@ -133,7 +148,8 @@ firmware {
> scu {
> compatible = "fsl,imx-scu";
> mbox-names = "tx0", "tx1", "tx2", "tx3",
> - "rx0", "rx1", "rx2", "rx3";
> + "rx0", "rx1", "rx2", "rx3",
> + "gip3";
> mboxes = <&lsio_mu1 0 0
> &lsio_mu1 0 1
> &lsio_mu1 0 2
> @@ -141,7 +157,8 @@ firmware {
> &lsio_mu1 1 0
> &lsio_mu1 1 1
> &lsio_mu1 1 2
> - &lsio_mu1 1 3>;
> + &lsio_mu1 1 3
> + &lsio_mu1 3 3>;
>
> clk: clk {
> compatible = "fsl,imx8qxp-clk", "fsl,scu-clk";
> --
> 2.7.4
On 18/03/2019 03:10:06+0000, Anson Huang wrote:
> Add i.MX system controller RTC alarm support, the RTC alarm
> is implemented via SIP(silicon provider) runtime service call
> and ARM-Trusted-Firmware will communicate with system controller
> via MU(message unit) IPC to set RTC alarm. When RTC alarm fires,
> system controller will generate a common MU irq event and notify
> system controller RTC driver to handle the irq event.
>
> Signed-off-by: Anson Huang <[email protected]>
> ---
> No function changes, just update imx_scu_irq_register_notifier() function name
> according to SCU IRQ function name change.
> ---
> drivers/rtc/rtc-imx-sc.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 112 insertions(+)
>
Applied, thanks.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On 01/04/2019 15:11:30+0200, Alexandre Belloni wrote:
> On 18/03/2019 03:10:06+0000, Anson Huang wrote:
> > Add i.MX system controller RTC alarm support, the RTC alarm
> > is implemented via SIP(silicon provider) runtime service call
> > and ARM-Trusted-Firmware will communicate with system controller
> > via MU(message unit) IPC to set RTC alarm. When RTC alarm fires,
> > system controller will generate a common MU irq event and notify
> > system controller RTC driver to handle the irq event.
> >
> > Signed-off-by: Anson Huang <[email protected]>
> > ---
> > No function changes, just update imx_scu_irq_register_notifier() function name
> > according to SCU IRQ function name change.
> > ---
> > drivers/rtc/rtc-imx-sc.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 112 insertions(+)
> >
> Applied, thanks.
>
Actually, that is not the one I applied.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On 18/03/2019 03:10:06+0000, Anson Huang wrote:
> Add i.MX system controller RTC alarm support, the RTC alarm
> is implemented via SIP(silicon provider) runtime service call
> and ARM-Trusted-Firmware will communicate with system controller
> via MU(message unit) IPC to set RTC alarm. When RTC alarm fires,
> system controller will generate a common MU irq event and notify
> system controller RTC driver to handle the irq event.
>
> Signed-off-by: Anson Huang <[email protected]>
Acked-by: Alexandre Belloni <[email protected]>
> ---
> No function changes, just update imx_scu_irq_register_notifier() function name
> according to SCU IRQ function name change.
> ---
> drivers/rtc/rtc-imx-sc.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 112 insertions(+)
>
> diff --git a/drivers/rtc/rtc-imx-sc.c b/drivers/rtc/rtc-imx-sc.c
> index 19642bf..9df4990 100644
> --- a/drivers/rtc/rtc-imx-sc.c
> +++ b/drivers/rtc/rtc-imx-sc.c
> @@ -3,6 +3,7 @@
> * Copyright 2018 NXP.
> */
>
> +#include <dt-bindings/firmware/imx/rsrc.h>
> #include <linux/arm-smccc.h>
> #include <linux/firmware/imx/sci.h>
> #include <linux/module.h>
> @@ -11,11 +12,17 @@
> #include <linux/rtc.h>
>
> #define IMX_SC_TIMER_FUNC_GET_RTC_SEC1970 9
> +#define IMX_SC_TIMER_FUNC_SET_RTC_ALARM 8
> #define IMX_SC_TIMER_FUNC_SET_RTC_TIME 6
>
> +#define IMX_SC_IRQ_FUNC_ENABLE 1
> +
> #define IMX_SIP_SRTC 0xC2000002
> #define IMX_SIP_SRTC_SET_TIME 0x0
>
> +#define SC_IRQ_GROUP_RTC 2
> +#define SC_IRQ_RTC 1
> +
> static struct imx_sc_ipc *rtc_ipc_handle;
> static struct rtc_device *imx_sc_rtc;
>
> @@ -24,6 +31,24 @@ struct imx_sc_msg_timer_get_rtc_time {
> u32 time;
> } __packed;
>
> +struct imx_sc_msg_timer_enable_irq {
> + struct imx_sc_rpc_msg hdr;
> + u32 mask;
> + u16 resource;
> + u8 group;
> + u8 enable;
> +} __packed;
> +
> +struct imx_sc_msg_timer_rtc_set_alarm {
> + struct imx_sc_rpc_msg hdr;
> + u16 year;
> + u8 mon;
> + u8 day;
> + u8 hour;
> + u8 min;
> + u8 sec;
> +} __packed;
> +
> static int imx_sc_rtc_read_time(struct device *dev, struct rtc_time *tm)
> {
> struct imx_sc_msg_timer_get_rtc_time msg;
> @@ -60,9 +85,92 @@ static int imx_sc_rtc_set_time(struct device *dev, struct rtc_time *tm)
> return res.a0;
> }
>
> +static int imx_sc_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
> +{
> + struct imx_sc_msg_timer_enable_irq 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_ENABLE;
> + hdr->size = 3;
> +
> + msg.resource = IMX_SC_R_MU_1A;
> + msg.group = SC_IRQ_GROUP_RTC;
> + msg.mask = SC_IRQ_RTC;
> + msg.enable = enable;
> +
> + ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
> + if (ret) {
> + dev_err(dev, "enable rtc irq failed, ret %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int imx_sc_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> + return 0;
> +}
> +
> +static int imx_sc_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> + struct imx_sc_msg_timer_rtc_set_alarm msg;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> + int ret;
> + struct rtc_time *alrm_tm = &alrm->time;
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_TIMER;
> + hdr->func = IMX_SC_TIMER_FUNC_SET_RTC_ALARM;
> + hdr->size = 3;
> +
> + msg.year = alrm_tm->tm_year + 1900;
> + msg.mon = alrm_tm->tm_mon + 1;
> + msg.day = alrm_tm->tm_mday;
> + msg.hour = alrm_tm->tm_hour;
> + msg.min = alrm_tm->tm_min;
> + msg.sec = alrm_tm->tm_sec;
> +
> + ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
> + if (ret) {
> + dev_err(dev, "set rtc alarm failed, ret %d\n", ret);
> + return ret;
> + }
> +
> + ret = imx_sc_rtc_alarm_irq_enable(dev, alrm->enabled);
> + if (ret) {
> + dev_err(dev, "enable rtc alarm failed, ret %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static const struct rtc_class_ops imx_sc_rtc_ops = {
> .read_time = imx_sc_rtc_read_time,
> .set_time = imx_sc_rtc_set_time,
> + .read_alarm = imx_sc_rtc_read_alarm,
> + .set_alarm = imx_sc_rtc_set_alarm,
> + .alarm_irq_enable = imx_sc_rtc_alarm_irq_enable,
> +};
> +
> +static int imx_sc_rtc_alarm_sc_notify(struct notifier_block *nb,
> + unsigned long event, void *group)
> +{
> + /* ignore non-rtc irq */
> + if (!((event & SC_IRQ_RTC) && (*(u8 *)group == SC_IRQ_GROUP_RTC)))
> + return 0;
> +
> + rtc_update_irq(imx_sc_rtc, 1, RTC_IRQF | RTC_AF);
> +
> + return 0;
> +}
> +
> +static struct notifier_block imx_sc_rtc_alarm_sc_notifier = {
> + .notifier_call = imx_sc_rtc_alarm_sc_notify,
> };
>
> static int imx_sc_rtc_probe(struct platform_device *pdev)
> @@ -73,6 +181,8 @@ static int imx_sc_rtc_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> + device_init_wakeup(&pdev->dev, true);
> +
> imx_sc_rtc = devm_rtc_allocate_device(&pdev->dev);
> if (IS_ERR(imx_sc_rtc))
> return PTR_ERR(imx_sc_rtc);
> @@ -87,6 +197,8 @@ static int imx_sc_rtc_probe(struct platform_device *pdev)
> return ret;
> }
>
> + imx_scu_irq_register_notifier(&imx_sc_rtc_alarm_sc_notifier);
> +
> return 0;
> }
>
> --
> 2.7.4
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Mon, Mar 18, 2019 at 03:09:55AM +0000, Anson Huang wrote:
> The System Controller Firmware (SCFW) controls RTC, thermal
> and WDOG etc., these resources' interrupt function are managed
> by SCU. When any IRQ pending, SCU will notify Linux via MU general
> interrupt channel #3, and Linux kernel needs to call SCU APIs
> to get IRQ status and notify each module to handle the interrupt.
>
> Since there is no data transmission for SCU IRQ notification, so
> doorbell mode is used for this MU channel, and SCU driver will
> use notifier mechanism to broadcast to every module which registers
> the SCU block notifier.
>
> Signed-off-by: Anson Huang <[email protected]>
@Aisheng,
Are you fine with this?
Shawn
> ---
> Changes since V4:
> - move scu irq support to a new file imx-scu-irq.c;
> - improve the function name with "imx_scu_irq_" as prefix for irq related functions;
> - move MU IRQ sources id out of sc ipc struct;
> - add IRQ group info message print when get irq status fail;
> - move MU ID parse into imx_scu_enable_general_irq_channel() function and ONLY do it
> when irq channel enable successfully.
> - change max IRQ group supported to 4, ONLY support for those kernel features.
> ---
> drivers/firmware/imx/Makefile | 2 +-
> drivers/firmware/imx/imx-scu-irq.c | 133 +++++++++++++++++++++++++++++++++++++
> drivers/firmware/imx/imx-scu.c | 6 ++
> include/linux/firmware/imx/sci.h | 4 ++
> 4 files changed, 144 insertions(+), 1 deletion(-)
> create mode 100644 drivers/firmware/imx/imx-scu-irq.c
>
> diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
> index 1b2e15b..802c4ad 100644
> --- a/drivers/firmware/imx/Makefile
> +++ b/drivers/firmware/imx/Makefile
> @@ -1,3 +1,3 @@
> # SPDX-License-Identifier: GPL-2.0
> -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o
> +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
> obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> diff --git a/drivers/firmware/imx/imx-scu-irq.c b/drivers/firmware/imx/imx-scu-irq.c
> new file mode 100644
> index 0000000..0e20aa7
> --- /dev/null
> +++ b/drivers/firmware/imx/imx-scu-irq.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2019 NXP
> + *
> + * Implementation of the SCU IRQ functions using MU.
> + *
> + */
> +
> +#include <dt-bindings/firmware/imx/rsrc.h>
> +#include <linux/firmware/imx/ipc.h>
> +#include <linux/mailbox_client.h>
> +
> +#define IMX_SC_IRQ_FUNC_STATUS 2
> +#define IMX_SC_IRQ_NUM_GROUP 4
> +
> +static u32 mu_resource_id;
> +
> +struct imx_sc_msg_irq_get_status {
> + struct imx_sc_rpc_msg hdr;
> + union {
> + struct {
> + u16 resource;
> + u8 group;
> + u8 reserved;
> + } __packed req;
> + struct {
> + u32 status;
> + } resp;
> + } data;
> +};
> +
> +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);
> +
> +int imx_scu_irq_register_notifier(struct notifier_block *nb)
> +{
> + 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 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 blocking_notifier_call_chain(&imx_scu_irq_notifier_chain,
> + status, (void *)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);
> + 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;
> +
> + imx_scu_irq_notifier_call_chain(irq_status, &i);
> + }
> +}
> +
> +static void imx_scu_irq_callback(struct mbox_client *c, void *msg)
> +{
> + schedule_work(&imx_sc_irq_work);
> +}
> +
> +int imx_scu_enable_general_irq_channel(struct device *dev)
> +{
> + struct of_phandle_args spec;
> + struct mbox_client *cl;
> + struct mbox_chan *ch;
> + int ret = 0, i = 0;
> +
> + cl = devm_kzalloc(dev, sizeof(*cl), GFP_KERNEL);
> + if (!cl)
> + return -ENOMEM;
> +
> + cl->dev = dev;
> + cl->rx_callback = imx_scu_irq_callback;
> +
> + /* SCU general IRQ uses general interrupt channel 3 */
> + ch = mbox_request_channel_byname(cl, "gip3");
> + if (IS_ERR(ch)) {
> + ret = PTR_ERR(ch);
> + dev_err(dev, "failed to request mbox chan gip3, ret %d\n", ret);
> + return ret;
> + }
> +
> + ret = imx_scu_get_handle(&imx_sc_irq_ipc_handle);
> + if (ret)
> + return ret;
> +
> + 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))
> + i = of_alias_get_id(spec.np, "mu");
> +
> + /* use mu1 as general mu irq channel if failed */
> + if (i < 0)
> + i = 1;
> +
> + mu_resource_id = IMX_SC_R_MU_0A + i;
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(imx_scu_enable_general_irq_channel);
> diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c
> index 2bb1a19..04a24a8 100644
> --- a/drivers/firmware/imx/imx-scu.c
> +++ b/drivers/firmware/imx/imx-scu.c
> @@ -10,6 +10,7 @@
> #include <linux/err.h>
> #include <linux/firmware/imx/types.h>
> #include <linux/firmware/imx/ipc.h>
> +#include <linux/firmware/imx/sci.h>
> #include <linux/interrupt.h>
> #include <linux/irq.h>
> #include <linux/kernel.h>
> @@ -246,6 +247,11 @@ static int imx_scu_probe(struct platform_device *pdev)
>
> imx_sc_ipc_handle = sc_ipc;
>
> + ret = imx_scu_enable_general_irq_channel(dev);
> + if (ret)
> + dev_warn(dev,
> + "failed to enable general irq channel: %d\n", ret);
> +
> dev_info(dev, "NXP i.MX SCU Initialized\n");
>
> return devm_of_platform_populate(dev);
> diff --git a/include/linux/firmware/imx/sci.h b/include/linux/firmware/imx/sci.h
> index ebc5509..918fa16 100644
> --- a/include/linux/firmware/imx/sci.h
> +++ b/include/linux/firmware/imx/sci.h
> @@ -15,4 +15,8 @@
>
> #include <linux/firmware/imx/svc/misc.h>
> #include <linux/firmware/imx/svc/pm.h>
> +
> +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);
> #endif /* _SC_SCI_H */
> --
> 2.7.4
>
> From: Anson Huang
> Sent: Monday, March 18, 2019 11:10 AM
>
> The System Controller Firmware (SCFW) controls RTC, thermal and WDOG etc.,
> these resources' interrupt function are managed by SCU. When any IRQ
> pending, SCU will notify Linux via MU general interrupt channel #3, and Linux
> kernel needs to call SCU APIs to get IRQ status and notify each module to
> handle the interrupt.
>
> Since there is no data transmission for SCU IRQ notification, so doorbell mode
> is used for this MU channel, and SCU driver will use notifier mechanism to
> broadcast to every module which registers the SCU block notifier.
>
> Signed-off-by: Anson Huang <[email protected]>
This patch mostly looks good me, except a few minor comments.
Otherwise, you can add my tag.
Reviewed-by: Dong Aisheng <[email protected]>
> ---
> Changes since V4:
> - move scu irq support to a new file imx-scu-irq.c;
> - improve the function name with "imx_scu_irq_" as prefix for irq related
> functions;
> - move MU IRQ sources id out of sc ipc struct;
> - add IRQ group info message print when get irq status fail;
> - move MU ID parse into imx_scu_enable_general_irq_channel() function
> and ONLY do it
> when irq channel enable successfully.
> - change max IRQ group supported to 4, ONLY support for those kernel
> features.
> ---
> drivers/firmware/imx/Makefile | 2 +-
> drivers/firmware/imx/imx-scu-irq.c | 133
> +++++++++++++++++++++++++++++++++++++
> drivers/firmware/imx/imx-scu.c | 6 ++
> include/linux/firmware/imx/sci.h | 4 ++
> 4 files changed, 144 insertions(+), 1 deletion(-) create mode 100644
> drivers/firmware/imx/imx-scu-irq.c
>
> diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
> index 1b2e15b..802c4ad 100644
> --- a/drivers/firmware/imx/Makefile
> +++ b/drivers/firmware/imx/Makefile
> @@ -1,3 +1,3 @@
> # SPDX-License-Identifier: GPL-2.0
> -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o
> +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
> obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> diff --git a/drivers/firmware/imx/imx-scu-irq.c
> b/drivers/firmware/imx/imx-scu-irq.c
> new file mode 100644
> index 0000000..0e20aa7
> --- /dev/null
> +++ b/drivers/firmware/imx/imx-scu-irq.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2019 NXP
> + *
> + * Implementation of the SCU IRQ functions using MU.
> + *
> + */
> +
> +#include <dt-bindings/firmware/imx/rsrc.h> #include
> +<linux/firmware/imx/ipc.h> #include <linux/mailbox_client.h>
> +
> +#define IMX_SC_IRQ_FUNC_STATUS 2
> +#define IMX_SC_IRQ_NUM_GROUP 4
> +
> +static u32 mu_resource_id;
> +
> +struct imx_sc_msg_irq_get_status {
> + struct imx_sc_rpc_msg hdr;
> + union {
> + struct {
> + u16 resource;
> + u8 group;
> + u8 reserved;
> + } __packed req;
> + struct {
> + u32 status;
> + } resp;
> + } data;
> +};
> +
> +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);
> +
> +int imx_scu_irq_register_notifier(struct notifier_block *nb) {
> + return blocking_notifier_chain_register(
We probably can use atomic_notifier_call_chain as it's used for irq function.
> + &imx_scu_irq_notifier_chain, nb);
> +}
> +EXPORT_SYMBOL(imx_scu_irq_register_notifier);
> +
> +int imx_scu_irq_unregister_notifier(struct notifier_block *nb) {
> + 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 blocking_notifier_call_chain(&imx_scu_irq_notifier_chain,
> + status, (void *)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);
> + 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;
> +
> + imx_scu_irq_notifier_call_chain(irq_status, &i);
> + }
> +}
> +
> +static void imx_scu_irq_callback(struct mbox_client *c, void *msg) {
> + schedule_work(&imx_sc_irq_work);
> +}
> +
> +int imx_scu_enable_general_irq_channel(struct device *dev) {
> + struct of_phandle_args spec;
> + struct mbox_client *cl;
> + struct mbox_chan *ch;
> + int ret = 0, i = 0;
> +
> + cl = devm_kzalloc(dev, sizeof(*cl), GFP_KERNEL);
> + if (!cl)
> + return -ENOMEM;
> +
> + cl->dev = dev;
> + cl->rx_callback = imx_scu_irq_callback;
> +
> + /* SCU general IRQ uses general interrupt channel 3 */
> + ch = mbox_request_channel_byname(cl, "gip3");
> + if (IS_ERR(ch)) {
> + ret = PTR_ERR(ch);
> + dev_err(dev, "failed to request mbox chan gip3, ret %d\n", ret);
There may be memory leak as this function is optional in imx-scu.c.
So here seems to miss a devm_kfree
Regards
Dong Aisheng
> + return ret;
> + }
> +
> + ret = imx_scu_get_handle(&imx_sc_irq_ipc_handle);
> + if (ret)
> + return ret;
> +
> + 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))
> + i = of_alias_get_id(spec.np, "mu");
> +
> + /* use mu1 as general mu irq channel if failed */
> + if (i < 0)
> + i = 1;
> +
> + mu_resource_id = IMX_SC_R_MU_0A + i;
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(imx_scu_enable_general_irq_channel);
> diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c
> index 2bb1a19..04a24a8 100644
> --- a/drivers/firmware/imx/imx-scu.c
> +++ b/drivers/firmware/imx/imx-scu.c
> @@ -10,6 +10,7 @@
> #include <linux/err.h>
> #include <linux/firmware/imx/types.h>
> #include <linux/firmware/imx/ipc.h>
> +#include <linux/firmware/imx/sci.h>
> #include <linux/interrupt.h>
> #include <linux/irq.h>
> #include <linux/kernel.h>
> @@ -246,6 +247,11 @@ static int imx_scu_probe(struct platform_device
> *pdev)
>
> imx_sc_ipc_handle = sc_ipc;
>
> + ret = imx_scu_enable_general_irq_channel(dev);
> + if (ret)
> + dev_warn(dev,
> + "failed to enable general irq channel: %d\n", ret);
> +
> dev_info(dev, "NXP i.MX SCU Initialized\n");
>
> return devm_of_platform_populate(dev); diff --git
> a/include/linux/firmware/imx/sci.h b/include/linux/firmware/imx/sci.h
> index ebc5509..918fa16 100644
> --- a/include/linux/firmware/imx/sci.h
> +++ b/include/linux/firmware/imx/sci.h
> @@ -15,4 +15,8 @@
>
> #include <linux/firmware/imx/svc/misc.h> #include
> <linux/firmware/imx/svc/pm.h>
> +
> +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);
> #endif /* _SC_SCI_H */
> --
> 2.7.4
> From: Anson Huang
> Sent: Monday, March 18, 2019 11:10 AM
>
> Add i.MX system controller RTC alarm support, the RTC alarm is implemented
> via SIP(silicon provider) runtime service call and ARM-Trusted-Firmware will
> communicate with system controller via MU(message unit) IPC to set RTC
> alarm. When RTC alarm fires, system controller will generate a common MU
> irq event and notify system controller RTC driver to handle the irq event.
>
> Signed-off-by: Anson Huang <[email protected]>
> ---
> No function changes, just update imx_scu_irq_register_notifier() function
> name according to SCU IRQ function name change.
> ---
> drivers/rtc/rtc-imx-sc.c | 112
> +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 112 insertions(+)
>
> diff --git a/drivers/rtc/rtc-imx-sc.c b/drivers/rtc/rtc-imx-sc.c index
> 19642bf..9df4990 100644
> --- a/drivers/rtc/rtc-imx-sc.c
> +++ b/drivers/rtc/rtc-imx-sc.c
> @@ -3,6 +3,7 @@
> * Copyright 2018 NXP.
> */
>
> +#include <dt-bindings/firmware/imx/rsrc.h>
> #include <linux/arm-smccc.h>
> #include <linux/firmware/imx/sci.h>
> #include <linux/module.h>
> @@ -11,11 +12,17 @@
> #include <linux/rtc.h>
>
> #define IMX_SC_TIMER_FUNC_GET_RTC_SEC1970 9
> +#define IMX_SC_TIMER_FUNC_SET_RTC_ALARM 8
> #define IMX_SC_TIMER_FUNC_SET_RTC_TIME 6
>
> +#define IMX_SC_IRQ_FUNC_ENABLE 1
> +
> #define IMX_SIP_SRTC 0xC2000002
> #define IMX_SIP_SRTC_SET_TIME 0x0
>
> +#define SC_IRQ_GROUP_RTC 2
> +#define SC_IRQ_RTC 1
> +
> static struct imx_sc_ipc *rtc_ipc_handle; static struct rtc_device
> *imx_sc_rtc;
>
> @@ -24,6 +31,24 @@ struct imx_sc_msg_timer_get_rtc_time {
> u32 time;
> } __packed;
>
> +struct imx_sc_msg_timer_enable_irq {
> + struct imx_sc_rpc_msg hdr;
> + u32 mask;
> + u16 resource;
> + u8 group;
> + u8 enable;
> +} __packed;
> +
> +struct imx_sc_msg_timer_rtc_set_alarm {
> + struct imx_sc_rpc_msg hdr;
> + u16 year;
> + u8 mon;
> + u8 day;
> + u8 hour;
> + u8 min;
> + u8 sec;
> +} __packed;
> +
> static int imx_sc_rtc_read_time(struct device *dev, struct rtc_time *tm) {
> struct imx_sc_msg_timer_get_rtc_time msg; @@ -60,9 +85,92 @@ static
> int imx_sc_rtc_set_time(struct device *dev, struct rtc_time *tm)
> return res.a0;
> }
>
> +static int imx_sc_rtc_alarm_irq_enable(struct device *dev, unsigned int
> +enable) {
I think you shouldn't implement this generic function in rtc driver instead of
Imx-scu-irq driver.
> + struct imx_sc_msg_timer_enable_irq 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_ENABLE;
> + hdr->size = 3;
> +
> + msg.resource = IMX_SC_R_MU_1A;
Here may be wrong as it is not align with what you did in Patch 2
that MU resource is dynamically detected.
> + msg.group = SC_IRQ_GROUP_RTC;
> + msg.mask = SC_IRQ_RTC;
> + msg.enable = enable;
> +
> + ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
> + if (ret) {
> + dev_err(dev, "enable rtc irq failed, ret %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int imx_sc_rtc_read_alarm(struct device *dev, struct rtc_wkalrm
> +*alrm) {
> + return 0;
> +}
Can't avoid define NULL function?
> +
> +static int imx_sc_rtc_set_alarm(struct device *dev, struct rtc_wkalrm
> +*alrm) {
> + struct imx_sc_msg_timer_rtc_set_alarm msg;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> + int ret;
> + struct rtc_time *alrm_tm = &alrm->time;
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_TIMER;
> + hdr->func = IMX_SC_TIMER_FUNC_SET_RTC_ALARM;
> + hdr->size = 3;
> +
> + msg.year = alrm_tm->tm_year + 1900;
> + msg.mon = alrm_tm->tm_mon + 1;
> + msg.day = alrm_tm->tm_mday;
> + msg.hour = alrm_tm->tm_hour;
> + msg.min = alrm_tm->tm_min;
> + msg.sec = alrm_tm->tm_sec;
> +
> + ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
> + if (ret) {
> + dev_err(dev, "set rtc alarm failed, ret %d\n", ret);
> + return ret;
> + }
> +
> + ret = imx_sc_rtc_alarm_irq_enable(dev, alrm->enabled);
Just curious we already have .alarm_irq_enable().
Why do we need call it again here?
Regards
Dong Aisheng
> + if (ret) {
> + dev_err(dev, "enable rtc alarm failed, ret %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static const struct rtc_class_ops imx_sc_rtc_ops = {
> .read_time = imx_sc_rtc_read_time,
> .set_time = imx_sc_rtc_set_time,
> + .read_alarm = imx_sc_rtc_read_alarm,
> + .set_alarm = imx_sc_rtc_set_alarm,
> + .alarm_irq_enable = imx_sc_rtc_alarm_irq_enable, };
> +
> +static int imx_sc_rtc_alarm_sc_notify(struct notifier_block *nb,
> + unsigned long event, void *group)
> +{
> + /* ignore non-rtc irq */
> + if (!((event & SC_IRQ_RTC) && (*(u8 *)group == SC_IRQ_GROUP_RTC)))
> + return 0;
> +
> + rtc_update_irq(imx_sc_rtc, 1, RTC_IRQF | RTC_AF);
> +
> + return 0;
> +}
> +
> +static struct notifier_block imx_sc_rtc_alarm_sc_notifier = {
> + .notifier_call = imx_sc_rtc_alarm_sc_notify,
> };
>
> static int imx_sc_rtc_probe(struct platform_device *pdev) @@ -73,6 +181,8
> @@ static int imx_sc_rtc_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> + device_init_wakeup(&pdev->dev, true);
> +
> imx_sc_rtc = devm_rtc_allocate_device(&pdev->dev);
> if (IS_ERR(imx_sc_rtc))
> return PTR_ERR(imx_sc_rtc);
> @@ -87,6 +197,8 @@ static int imx_sc_rtc_probe(struct platform_device
> *pdev)
> return ret;
> }
>
> + imx_scu_irq_register_notifier(&imx_sc_rtc_alarm_sc_notifier);
> +
> return 0;
> }
>
> --
> 2.7.4
Hi, Aisheng
Best Regards!
Anson Huang
> -----Original Message-----
> From: Aisheng Dong
> Sent: 2019??4??8?? 18:35
> To: Anson Huang <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Daniel Baluta
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]
> Cc: dl-linux-imx <[email protected]>
> Subject: RE: [PATCH V5 2/4] firmware: imx: enable imx scu general irq
> function
>
> > From: Anson Huang
> > Sent: Monday, March 18, 2019 11:10 AM
> >
> > The System Controller Firmware (SCFW) controls RTC, thermal and WDOG
> > etc., these resources' interrupt function are managed by SCU. When any
> > IRQ pending, SCU will notify Linux via MU general interrupt channel
> > #3, and Linux kernel needs to call SCU APIs to get IRQ status and
> > notify each module to handle the interrupt.
> >
> > Since there is no data transmission for SCU IRQ notification, so
> > doorbell mode is used for this MU channel, and SCU driver will use
> > notifier mechanism to broadcast to every module which registers the SCU
> block notifier.
> >
> > Signed-off-by: Anson Huang <[email protected]>
>
> This patch mostly looks good me, except a few minor comments.
> Otherwise, you can add my tag.
>
> Reviewed-by: Dong Aisheng <[email protected]>
>
> > ---
> > Changes since V4:
> > - move scu irq support to a new file imx-scu-irq.c;
> > - improve the function name with "imx_scu_irq_" as prefix for irq
> > related functions;
> > - move MU IRQ sources id out of sc ipc struct;
> > - add IRQ group info message print when get irq status fail;
> > - move MU ID parse into imx_scu_enable_general_irq_channel()
> function
> > and ONLY do it
> > when irq channel enable successfully.
> > - change max IRQ group supported to 4, ONLY support for those
> kernel
> > features.
> > ---
> > drivers/firmware/imx/Makefile | 2 +-
> > drivers/firmware/imx/imx-scu-irq.c | 133
> > +++++++++++++++++++++++++++++++++++++
> > drivers/firmware/imx/imx-scu.c | 6 ++
> > include/linux/firmware/imx/sci.h | 4 ++
> > 4 files changed, 144 insertions(+), 1 deletion(-) create mode 100644
> > drivers/firmware/imx/imx-scu-irq.c
> >
> > diff --git a/drivers/firmware/imx/Makefile
> > b/drivers/firmware/imx/Makefile index 1b2e15b..802c4ad 100644
> > --- a/drivers/firmware/imx/Makefile
> > +++ b/drivers/firmware/imx/Makefile
> > @@ -1,3 +1,3 @@
> > # SPDX-License-Identifier: GPL-2.0
> > -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o
> > +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
> > obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> > diff --git a/drivers/firmware/imx/imx-scu-irq.c
> > b/drivers/firmware/imx/imx-scu-irq.c
> > new file mode 100644
> > index 0000000..0e20aa7
> > --- /dev/null
> > +++ b/drivers/firmware/imx/imx-scu-irq.c
> > @@ -0,0 +1,133 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2019 NXP
> > + *
> > + * Implementation of the SCU IRQ functions using MU.
> > + *
> > + */
> > +
> > +#include <dt-bindings/firmware/imx/rsrc.h> #include
> > +<linux/firmware/imx/ipc.h> #include <linux/mailbox_client.h>
> > +
> > +#define IMX_SC_IRQ_FUNC_STATUS 2
> > +#define IMX_SC_IRQ_NUM_GROUP 4
> > +
> > +static u32 mu_resource_id;
> > +
> > +struct imx_sc_msg_irq_get_status {
> > + struct imx_sc_rpc_msg hdr;
> > + union {
> > + struct {
> > + u16 resource;
> > + u8 group;
> > + u8 reserved;
> > + } __packed req;
> > + struct {
> > + u32 status;
> > + } resp;
> > + } data;
> > +};
> > +
> > +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);
> > +
> > +int imx_scu_irq_register_notifier(struct notifier_block *nb) {
> > + return blocking_notifier_chain_register(
>
> We probably can use atomic_notifier_call_chain as it's used for irq function.
Agree, it makes more sense to use it in interrupt context. Will update it in V6.
>
> > + &imx_scu_irq_notifier_chain, nb);
> > +}
> > +EXPORT_SYMBOL(imx_scu_irq_register_notifier);
> > +
> > +int imx_scu_irq_unregister_notifier(struct notifier_block *nb) {
> > + 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 blocking_notifier_call_chain(&imx_scu_irq_notifier_chain,
> > + status, (void *)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);
> > + 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;
> > +
> > + imx_scu_irq_notifier_call_chain(irq_status, &i);
> > + }
> > +}
> > +
> > +static void imx_scu_irq_callback(struct mbox_client *c, void *msg) {
> > + schedule_work(&imx_sc_irq_work);
> > +}
> > +
> > +int imx_scu_enable_general_irq_channel(struct device *dev) {
> > + struct of_phandle_args spec;
> > + struct mbox_client *cl;
> > + struct mbox_chan *ch;
> > + int ret = 0, i = 0;
> > +
> > + cl = devm_kzalloc(dev, sizeof(*cl), GFP_KERNEL);
> > + if (!cl)
> > + return -ENOMEM;
> > +
> > + cl->dev = dev;
> > + cl->rx_callback = imx_scu_irq_callback;
> > +
> > + /* SCU general IRQ uses general interrupt channel 3 */
> > + ch = mbox_request_channel_byname(cl, "gip3");
> > + if (IS_ERR(ch)) {
> > + ret = PTR_ERR(ch);
> > + dev_err(dev, "failed to request mbox chan gip3, ret %d\n",
> ret);
>
> There may be memory leak as this function is optional in imx-scu.c.
> So here seems to miss a devm_kfree
Correct, I need to handle it, will add it in V6.
Thanks,
Anson.
>
> Regards
> Dong Aisheng
>
> > + return ret;
> > + }
> > +
> > + ret = imx_scu_get_handle(&imx_sc_irq_ipc_handle);
> > + if (ret)
> > + return ret;
> > +
> > + 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))
> > + i = of_alias_get_id(spec.np, "mu");
> > +
> > + /* use mu1 as general mu irq channel if failed */
> > + if (i < 0)
> > + i = 1;
> > +
> > + mu_resource_id = IMX_SC_R_MU_0A + i;
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL(imx_scu_enable_general_irq_channel);
> > diff --git a/drivers/firmware/imx/imx-scu.c
> > b/drivers/firmware/imx/imx-scu.c index 2bb1a19..04a24a8 100644
> > --- a/drivers/firmware/imx/imx-scu.c
> > +++ b/drivers/firmware/imx/imx-scu.c
> > @@ -10,6 +10,7 @@
> > #include <linux/err.h>
> > #include <linux/firmware/imx/types.h> #include
> > <linux/firmware/imx/ipc.h>
> > +#include <linux/firmware/imx/sci.h>
> > #include <linux/interrupt.h>
> > #include <linux/irq.h>
> > #include <linux/kernel.h>
> > @@ -246,6 +247,11 @@ static int imx_scu_probe(struct platform_device
> > *pdev)
> >
> > imx_sc_ipc_handle = sc_ipc;
> >
> > + ret = imx_scu_enable_general_irq_channel(dev);
> > + if (ret)
> > + dev_warn(dev,
> > + "failed to enable general irq channel: %d\n", ret);
> > +
> > dev_info(dev, "NXP i.MX SCU Initialized\n");
> >
> > return devm_of_platform_populate(dev); diff --git
> > a/include/linux/firmware/imx/sci.h b/include/linux/firmware/imx/sci.h
> > index ebc5509..918fa16 100644
> > --- a/include/linux/firmware/imx/sci.h
> > +++ b/include/linux/firmware/imx/sci.h
> > @@ -15,4 +15,8 @@
> >
> > #include <linux/firmware/imx/svc/misc.h> #include
> > <linux/firmware/imx/svc/pm.h>
> > +
> > +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);
> > #endif /* _SC_SCI_H */
> > --
> > 2.7.4
Hi, Aisheng
Best Regards!
Anson Huang
> -----Original Message-----
> From: Aisheng Dong
> Sent: 2019??4??8?? 19:09
> To: Anson Huang <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Daniel Baluta
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]
> Cc: dl-linux-imx <[email protected]>
> Subject: RE: [PATCH V5 4/4] rtc: imx-sc: add rtc alarm support
>
> > From: Anson Huang
> > Sent: Monday, March 18, 2019 11:10 AM
> >
> > Add i.MX system controller RTC alarm support, the RTC alarm is
> > implemented via SIP(silicon provider) runtime service call and
> > ARM-Trusted-Firmware will communicate with system controller via
> > MU(message unit) IPC to set RTC alarm. When RTC alarm fires, system
> > controller will generate a common MU irq event and notify system
> controller RTC driver to handle the irq event.
> >
> > Signed-off-by: Anson Huang <[email protected]>
> > ---
> > No function changes, just update imx_scu_irq_register_notifier()
> > function name according to SCU IRQ function name change.
> > ---
> > drivers/rtc/rtc-imx-sc.c | 112
> > +++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 112 insertions(+)
> >
> > diff --git a/drivers/rtc/rtc-imx-sc.c b/drivers/rtc/rtc-imx-sc.c index
> > 19642bf..9df4990 100644
> > --- a/drivers/rtc/rtc-imx-sc.c
> > +++ b/drivers/rtc/rtc-imx-sc.c
> > @@ -3,6 +3,7 @@
> > * Copyright 2018 NXP.
> > */
> >
> > +#include <dt-bindings/firmware/imx/rsrc.h>
> > #include <linux/arm-smccc.h>
> > #include <linux/firmware/imx/sci.h>
> > #include <linux/module.h>
> > @@ -11,11 +12,17 @@
> > #include <linux/rtc.h>
> >
> > #define IMX_SC_TIMER_FUNC_GET_RTC_SEC1970 9
> > +#define IMX_SC_TIMER_FUNC_SET_RTC_ALARM 8
> > #define IMX_SC_TIMER_FUNC_SET_RTC_TIME 6
> >
> > +#define IMX_SC_IRQ_FUNC_ENABLE 1
> > +
> > #define IMX_SIP_SRTC 0xC2000002
> > #define IMX_SIP_SRTC_SET_TIME 0x0
> >
> > +#define SC_IRQ_GROUP_RTC 2
> > +#define SC_IRQ_RTC 1
> > +
> > static struct imx_sc_ipc *rtc_ipc_handle; static struct rtc_device
> > *imx_sc_rtc;
> >
> > @@ -24,6 +31,24 @@ struct imx_sc_msg_timer_get_rtc_time {
> > u32 time;
> > } __packed;
> >
> > +struct imx_sc_msg_timer_enable_irq {
> > + struct imx_sc_rpc_msg hdr;
> > + u32 mask;
> > + u16 resource;
> > + u8 group;
> > + u8 enable;
> > +} __packed;
> > +
> > +struct imx_sc_msg_timer_rtc_set_alarm {
> > + struct imx_sc_rpc_msg hdr;
> > + u16 year;
> > + u8 mon;
> > + u8 day;
> > + u8 hour;
> > + u8 min;
> > + u8 sec;
> > +} __packed;
> > +
> > static int imx_sc_rtc_read_time(struct device *dev, struct rtc_time *tm) {
> > struct imx_sc_msg_timer_get_rtc_time msg; @@ -60,9 +85,92 @@
> static
> > int imx_sc_rtc_set_time(struct device *dev, struct rtc_time *tm)
> > return res.a0;
> > }
> >
> > +static int imx_sc_rtc_alarm_irq_enable(struct device *dev, unsigned
> > +int
> > +enable) {
>
> I think you shouldn't implement this generic function in rtc driver instead of
> Imx-scu-irq driver.
RTC driver has capability to enable/disable IRQ, so we have to implement this
callback, and considering the MU resource ID has to be parsed from DT, it does
NOT make sense to do it for all driver, so I will add another API in imx-scu-irq driver
to provide function of enabling/disabling irq, each driver can just call the API to
enable/disable its own IRQ, ONLY need to pass the corresponding arguments:
+void imx_scu_irq_enable(u8 group, u32 mask, u8 enable)
+{
+ struct imx_sc_msg_irq_enable 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_ENABLE;
+ hdr->size = 3;
+
+ msg.resource = mu_resource_id;
+ msg.group = group;
+ msg.mask = mask;
+ msg.enable = enable;
+
+ ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
+ if (ret)
+ dev_err(dev, "enable irq failed, group %d, mask %d, ret %d\n",
+ group, mask, ret);
+}
>
> > + struct imx_sc_msg_timer_enable_irq 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_ENABLE;
> > + hdr->size = 3;
> > +
> > + msg.resource = IMX_SC_R_MU_1A;
>
> Here may be wrong as it is not align with what you did in Patch 2 that MU
> resource is dynamically detected.
Fixed by upper comment using new API inside imx-scu-irq driver.
>
> > + msg.group = SC_IRQ_GROUP_RTC;
> > + msg.mask = SC_IRQ_RTC;
> > + msg.enable = enable;
> > +
> > + ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
> > + if (ret) {
> > + dev_err(dev, "enable rtc irq failed, ret %d\n", ret);
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int imx_sc_rtc_read_alarm(struct device *dev, struct
> > +rtc_wkalrm
> > +*alrm) {
> > + return 0;
> > +}
>
> Can't avoid define NULL function?
We have to implement it as NULL function, as SCFW does NOT provide such feature,
But rtc alarm ONLY available when .read_alarm ops is implemented:
147 static ssize_t
148 wakealarm_store(struct device *dev, struct device_attribute *attr,
149 const char *buf, size_t n)
150 {
...
187 retval = rtc_read_alarm(rtc, &alm);
188 if (retval < 0)
189 return retval;
>
> > +
> > +static int imx_sc_rtc_set_alarm(struct device *dev, struct rtc_wkalrm
> > +*alrm) {
> > + struct imx_sc_msg_timer_rtc_set_alarm msg;
> > + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > + int ret;
> > + struct rtc_time *alrm_tm = &alrm->time;
> > +
> > + hdr->ver = IMX_SC_RPC_VERSION;
> > + hdr->svc = IMX_SC_RPC_SVC_TIMER;
> > + hdr->func = IMX_SC_TIMER_FUNC_SET_RTC_ALARM;
> > + hdr->size = 3;
> > +
> > + msg.year = alrm_tm->tm_year + 1900;
> > + msg.mon = alrm_tm->tm_mon + 1;
> > + msg.day = alrm_tm->tm_mday;
> > + msg.hour = alrm_tm->tm_hour;
> > + msg.min = alrm_tm->tm_min;
> > + msg.sec = alrm_tm->tm_sec;
> > +
> > + ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
> > + if (ret) {
> > + dev_err(dev, "set rtc alarm failed, ret %d\n", ret);
> > + return ret;
> > + }
> > +
> > + ret = imx_sc_rtc_alarm_irq_enable(dev, alrm->enabled);
>
> Just curious we already have .alarm_irq_enable().
> Why do we need call it again here?
That is because the set_alarm function pass alarm time and alarm->enabled argument,
it could be to enable alarm or to disable alarm, so we have to control the alarm
function here, all other rtc drivers also do it this way. The .alarm_irq_enable() callback
is for just enable or disable alarm.
Thanks,
Anson
>
> Regards
> Dong Aisheng
>
> > + if (ret) {
> > + dev_err(dev, "enable rtc alarm failed, ret %d\n", ret);
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static const struct rtc_class_ops imx_sc_rtc_ops = {
> > .read_time = imx_sc_rtc_read_time,
> > .set_time = imx_sc_rtc_set_time,
> > + .read_alarm = imx_sc_rtc_read_alarm,
> > + .set_alarm = imx_sc_rtc_set_alarm,
> > + .alarm_irq_enable = imx_sc_rtc_alarm_irq_enable, };
> > +
> > +static int imx_sc_rtc_alarm_sc_notify(struct notifier_block *nb,
> > + unsigned long event, void *group) {
> > + /* ignore non-rtc irq */
> > + if (!((event & SC_IRQ_RTC) && (*(u8 *)group ==
> SC_IRQ_GROUP_RTC)))
> > + return 0;
> > +
> > + rtc_update_irq(imx_sc_rtc, 1, RTC_IRQF | RTC_AF);
> > +
> > + return 0;
> > +}
> > +
> > +static struct notifier_block imx_sc_rtc_alarm_sc_notifier = {
> > + .notifier_call = imx_sc_rtc_alarm_sc_notify,
> > };
> >
> > static int imx_sc_rtc_probe(struct platform_device *pdev) @@ -73,6
> > +181,8 @@ static int imx_sc_rtc_probe(struct platform_device *pdev)
> > if (ret)
> > return ret;
> >
> > + device_init_wakeup(&pdev->dev, true);
> > +
> > imx_sc_rtc = devm_rtc_allocate_device(&pdev->dev);
> > if (IS_ERR(imx_sc_rtc))
> > return PTR_ERR(imx_sc_rtc);
> > @@ -87,6 +197,8 @@ static int imx_sc_rtc_probe(struct platform_device
> > *pdev)
> > return ret;
> > }
> >
> > + imx_scu_irq_register_notifier(&imx_sc_rtc_alarm_sc_notifier);
> > +
> > return 0;
> > }
> >
> > --
> > 2.7.4
[...]
> so I will add another API in imx-scu-irq
> driver to provide function of enabling/disabling irq, each driver can just call the
> API to enable/disable its own IRQ, ONLY need to pass the corresponding
> arguments:
>
That's exactly what I mean.
> >
> > > + msg.group = SC_IRQ_GROUP_RTC;
> > > + msg.mask = SC_IRQ_RTC;
> > > + msg.enable = enable;
> > > +
> > > + ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
> > > + if (ret) {
> > > + dev_err(dev, "enable rtc irq failed, ret %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int imx_sc_rtc_read_alarm(struct device *dev, struct
> > > +rtc_wkalrm
> > > +*alrm) {
> > > + return 0;
> > > +}
> >
> > Can't avoid define NULL function?
>
> We have to implement it as NULL function, as SCFW does NOT provide such
> feature, But rtc alarm ONLY available when .read_alarm ops is implemented:
>
If the framework mandantorily requires it, then we'd probably better to add
doc before this function and explain why it's safe to be NULL.
> 147 static ssize_t
> 148 wakealarm_store(struct device *dev, struct device_attribute *attr,
> 149 const char *buf, size_t n)
> 150 {
>
> ...
>
> 187 retval = rtc_read_alarm(rtc, &alm);
> 188 if (retval < 0)
> 189 return retval;
>
>
> >
> > > +
> > > +static int imx_sc_rtc_set_alarm(struct device *dev, struct
> > > +rtc_wkalrm
> > > +*alrm) {
> > > + struct imx_sc_msg_timer_rtc_set_alarm msg;
> > > + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > > + int ret;
> > > + struct rtc_time *alrm_tm = &alrm->time;
> > > +
> > > + hdr->ver = IMX_SC_RPC_VERSION;
> > > + hdr->svc = IMX_SC_RPC_SVC_TIMER;
> > > + hdr->func = IMX_SC_TIMER_FUNC_SET_RTC_ALARM;
> > > + hdr->size = 3;
> > > +
> > > + msg.year = alrm_tm->tm_year + 1900;
> > > + msg.mon = alrm_tm->tm_mon + 1;
> > > + msg.day = alrm_tm->tm_mday;
> > > + msg.hour = alrm_tm->tm_hour;
> > > + msg.min = alrm_tm->tm_min;
> > > + msg.sec = alrm_tm->tm_sec;
> > > +
> > > + ret = imx_scu_call_rpc(rtc_ipc_handle, &msg, true);
> > > + if (ret) {
> > > + dev_err(dev, "set rtc alarm failed, ret %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ret = imx_sc_rtc_alarm_irq_enable(dev, alrm->enabled);
> >
> > Just curious we already have .alarm_irq_enable().
> > Why do we need call it again here?
>
> That is because the set_alarm function pass alarm time and alarm->enabled
> argument, it could be to enable alarm or to disable alarm, so we have to
> control the alarm function here, all other rtc drivers also do it this way.
> The .alarm_irq_enable() callback is for just enable or disable alarm.
>
Got it, thanks for the explanation.
Regards
Dong Aisheng
> Thanks,
> Anson
>
>
> >
> > Regards
> > Dong Aisheng
> >
> > > + if (ret) {
> > > + dev_err(dev, "enable rtc alarm failed, ret %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static const struct rtc_class_ops imx_sc_rtc_ops = {
> > > .read_time = imx_sc_rtc_read_time,
> > > .set_time = imx_sc_rtc_set_time,
> > > + .read_alarm = imx_sc_rtc_read_alarm,
> > > + .set_alarm = imx_sc_rtc_set_alarm,
> > > + .alarm_irq_enable = imx_sc_rtc_alarm_irq_enable, };
> > > +
> > > +static int imx_sc_rtc_alarm_sc_notify(struct notifier_block *nb,
> > > + unsigned long event, void *group) {
> > > + /* ignore non-rtc irq */
> > > + if (!((event & SC_IRQ_RTC) && (*(u8 *)group ==
> > SC_IRQ_GROUP_RTC)))
> > > + return 0;
> > > +
> > > + rtc_update_irq(imx_sc_rtc, 1, RTC_IRQF | RTC_AF);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static struct notifier_block imx_sc_rtc_alarm_sc_notifier = {
> > > + .notifier_call = imx_sc_rtc_alarm_sc_notify,
> > > };
> > >
> > > static int imx_sc_rtc_probe(struct platform_device *pdev) @@ -73,6
> > > +181,8 @@ static int imx_sc_rtc_probe(struct platform_device *pdev)
> > > if (ret)
> > > return ret;
> > >
> > > + device_init_wakeup(&pdev->dev, true);
> > > +
> > > imx_sc_rtc = devm_rtc_allocate_device(&pdev->dev);
> > > if (IS_ERR(imx_sc_rtc))
> > > return PTR_ERR(imx_sc_rtc);
> > > @@ -87,6 +197,8 @@ static int imx_sc_rtc_probe(struct
> > > platform_device
> > > *pdev)
> > > return ret;
> > > }
> > >
> > > + imx_scu_irq_register_notifier(&imx_sc_rtc_alarm_sc_notifier);
> > > +
> > > return 0;
> > > }
> > >
> > > --
> > > 2.7.4