From: Franck LENORMAND <[email protected]>
This patch adds the APIs:
- imx_sc_seco_build_info: get commit and sha of SECO
- imx_sc_seco_secvio_enable: enable SNVS IRQ handling
- imx_sc_seco_secvio_config: configure SNVS register
- imx_sc_seco_secvio_dgo_config: configure SNVS DGO register
Signed-off-by: Franck LENORMAND <[email protected]>
---
drivers/firmware/imx/Makefile | 2 +-
drivers/firmware/imx/imx-scu.c | 3 +
drivers/firmware/imx/seco.c | 275 ++++++++++++++++++++++++++++++++++
include/linux/firmware/imx/ipc.h | 1 +
include/linux/firmware/imx/sci.h | 1 +
include/linux/firmware/imx/svc/seco.h | 73 +++++++++
6 files changed, 354 insertions(+), 1 deletion(-)
create mode 100644 drivers/firmware/imx/seco.c
create mode 100644 include/linux/firmware/imx/svc/seco.h
diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
index b76acba..f23e2b0 100644
--- a/drivers/firmware/imx/Makefile
+++ b/drivers/firmware/imx/Makefile
@@ -1,4 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_IMX_DSP) += imx-dsp.o
-obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o imx-scu-soc.o
+obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o imx-scu-soc.o seco.o
obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c
index dca79ca..ed7b87b 100644
--- a/drivers/firmware/imx/imx-scu.c
+++ b/drivers/firmware/imx/imx-scu.c
@@ -245,6 +245,9 @@ int imx_scu_call_rpc(struct imx_sc_ipc *sc_ipc, void *msg, bool have_resp)
(saved_func == IMX_SC_MISC_FUNC_UNIQUE_ID ||
saved_func == IMX_SC_MISC_FUNC_GET_BUTTON_STATUS))
ret = 0;
+ if (saved_svc == IMX_SC_RPC_SVC_SECO &&
+ saved_func == IMX_SC_SECO_FUNC_BUILD_INFO)
+ ret = 0;
}
out:
diff --git a/drivers/firmware/imx/seco.c b/drivers/firmware/imx/seco.c
new file mode 100644
index 0000000..9047a75
--- /dev/null
+++ b/drivers/firmware/imx/seco.c
@@ -0,0 +1,275 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2020 NXP
+ *
+ * File containing client-side RPC functions for the SECO service. These
+ * functions are ported to clients that communicate to the SC.
+ */
+
+#include <linux/firmware/imx/sci.h>
+
+struct imx_sc_msg_seco_get_build_id {
+ struct imx_sc_rpc_msg hdr;
+ u32 version;
+ u32 commit;
+};
+
+/**
+ * imx_sc_seco_build_info() - Get version and coomit ID of the SECO
+ *
+ * @ipc: IPC handle
+ * @version: Version of the SECO
+ * @commit: Commit ID of the SECO
+ *
+ * Return:
+ * 0 - OK
+ * < 0 - error.
+ */
+int imx_sc_seco_build_info(struct imx_sc_ipc *ipc, uint32_t *version,
+ uint32_t *commit)
+{
+ int ret;
+ struct imx_sc_msg_seco_get_build_id msg = {0};
+ struct imx_sc_rpc_msg *hdr = &msg.hdr;
+
+ hdr->ver = IMX_SC_RPC_VERSION;
+ hdr->svc = IMX_SC_RPC_SVC_SECO;
+ hdr->func = IMX_SC_SECO_FUNC_BUILD_INFO;
+ hdr->size = 1;
+
+ ret = imx_scu_call_rpc(ipc, &msg, true);
+ if (ret)
+ return ret;
+
+ if (version)
+ *version = msg.version;
+ if (commit)
+ *commit = msg.commit;
+
+ return 0;
+}
+EXPORT_SYMBOL(imx_sc_seco_build_info);
+
+struct imx_sc_msg_seco_sab_msg {
+ struct imx_sc_rpc_msg hdr;
+ u32 smsg_addr_hi;
+ u32 smsg_addr_lo;
+};
+
+/**
+ * imx_sc_seco_secvio_enable() - Enable the processing of secvio IRQ from the
+ * SNVS by the SECO
+ *
+ * @ipc: IPC handle
+ *
+ * Return:
+ * 0 - OK
+ * < 0 - error.
+ */
+int imx_sc_seco_secvio_enable(struct imx_sc_ipc *ipc)
+{
+ struct imx_sc_rpc_msg msg;
+ struct imx_sc_rpc_msg *hdr = &msg;
+ int ret;
+
+ hdr->ver = IMX_SC_RPC_VERSION;
+ hdr->svc = IMX_SC_RPC_SVC_SECO;
+ hdr->func = IMX_SC_SECO_FUNC_SECVIO_ENABLE;
+ hdr->size = 1;
+
+ ret = imx_scu_call_rpc(ipc, &msg, true);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+EXPORT_SYMBOL(imx_sc_seco_secvio_enable);
+
+struct imx_sc_msg_req_seco_config {
+ struct imx_sc_rpc_msg hdr;
+ u32 data0;
+ u32 data1;
+ u32 data2;
+ u32 data3;
+ u32 data4;
+ u8 id;
+ u8 access;
+ u8 size;
+} __packed __aligned(4);
+
+struct imx_sc_msg_resp_seco_config {
+ struct imx_sc_rpc_msg hdr;
+ u32 data0;
+ u32 data1;
+ u32 data2;
+ u32 data3;
+ u32 data4;
+} __packed;
+
+/**
+ * imx_sc_seco_secvio_config() - Configure a set of SNVS registers for SECure
+ * VIOlation
+ *
+ * Some registers are extended by others registers, they configure the same
+ * kind of behavior, it constitutes a set
+ *
+ * @ipc: IPC handle
+ * @id: ID of the register, ie the offset of the first register of the set
+ * @access: Write (1) or Read (0) the registers
+ * @data0: Data for the first register
+ * @data1: Data for the second register
+ * @data2: Data for the third register
+ * @data3: Data for the fourth register
+ * @data4: Data for the fifth register
+ * @size: Number of register to configure
+ *
+ * Return:
+ * 0 - OK
+ * < 0 - error.
+ */
+int imx_sc_seco_secvio_config(struct imx_sc_ipc *ipc, u8 id, u8 access,
+ u32 *data0, u32 *data1, u32 *data2, u32 *data3,
+ u32 *data4, u8 size)
+{
+ struct imx_sc_msg_req_seco_config msg;
+ struct imx_sc_msg_resp_seco_config *resp;
+ struct imx_sc_rpc_msg *hdr = &msg.hdr;
+ int ret;
+
+ if (!ipc)
+ return -EINVAL;
+
+ hdr->ver = IMX_SC_RPC_VERSION;
+ hdr->svc = IMX_SC_RPC_SVC_SECO;
+ hdr->func = IMX_SC_SECO_FUNC_SECVIO_CONFIG;
+ hdr->size = 7;
+
+ /* Check the pointers on data are valid and set it if doing a write */
+ switch (size) {
+ case 5:
+ if (!data4)
+ return -EINVAL;
+ if (access)
+ msg.data4 = *data4;
+ fallthrough;
+ case 4:
+ if (!data3)
+ return -EINVAL;
+ if (access)
+ msg.data3 = *data3;
+ fallthrough;
+ case 3:
+ if (!data2)
+ return -EINVAL;
+ if (access)
+ msg.data2 = *data2;
+ fallthrough;
+ case 2:
+ if (!data1)
+ return -EINVAL;
+ if (access)
+ msg.data1 = *data1;
+ fallthrough;
+ case 1:
+ if (!data0)
+ return -EINVAL;
+ if (access)
+ msg.data0 = *data0;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ msg.id = id;
+ msg.access = access;
+ msg.size = size;
+
+ ret = imx_scu_call_rpc(ipc, &msg, true);
+ if (ret)
+ return ret;
+
+ resp = (struct imx_sc_msg_resp_seco_config *)&msg;
+
+ /* Pointers already checked so we just copy the data if reading */
+ if (!access)
+ switch (size) {
+ case 5:
+ *data4 = resp->data4;
+ fallthrough;
+ case 4:
+ *data3 = resp->data3;
+ fallthrough;
+ case 3:
+ *data2 = resp->data2;
+ fallthrough;
+ case 2:
+ *data1 = resp->data1;
+ fallthrough;
+ case 1:
+ *data0 = resp->data0;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(imx_sc_seco_secvio_config);
+
+struct imx_sc_msg_req_seco_dgo_config {
+ struct imx_sc_rpc_msg hdr;
+ u32 data;
+ u8 id;
+ u8 access;
+} __packed __aligned(4);
+
+struct imx_sc_msg_resp_seco_dgo_config {
+ struct imx_sc_rpc_msg hdr;
+ u32 data;
+} __packed;
+
+/**
+ * imx_sc_seco_secvio_enable() - Configure the DGO module
+ *
+ * @ipc: IPC handle
+ * @id: ID of the register, ie the offset of the register
+ * @access: Write (1) or Read (0) the registers
+ * @data: Data for the register
+ *
+ * Return:
+ * 0 - OK
+ * < 0 - error.
+ */
+int imx_sc_seco_secvio_dgo_config(struct imx_sc_ipc *ipc, u8 id, u8 access,
+ u32 *data)
+{
+ struct imx_sc_msg_req_seco_dgo_config msg;
+ struct imx_sc_msg_resp_seco_dgo_config *resp;
+ struct imx_sc_rpc_msg *hdr = &msg.hdr;
+ int ret;
+
+ if (!ipc)
+ return -EINVAL;
+
+ hdr->ver = IMX_SC_RPC_VERSION;
+ hdr->svc = IMX_SC_RPC_SVC_SECO;
+ hdr->func = IMX_SC_SECO_FUNC_SECVIO_DGO_CONFIG;
+ hdr->size = 3;
+
+ if (!data)
+ return -EINVAL;
+ if (access)
+ msg.data = *data;
+
+ msg.access = access;
+ msg.id = id;
+
+ ret = imx_scu_call_rpc(ipc, &msg, true);
+ if (ret)
+ return ret;
+
+ resp = (struct imx_sc_msg_resp_seco_dgo_config *)&msg;
+
+ if (!access && data)
+ *data = resp->data;
+
+ return 0;
+}
+EXPORT_SYMBOL(imx_sc_seco_secvio_dgo_config);
diff --git a/include/linux/firmware/imx/ipc.h b/include/linux/firmware/imx/ipc.h
index 89105743..6924359 100644
--- a/include/linux/firmware/imx/ipc.h
+++ b/include/linux/firmware/imx/ipc.h
@@ -25,6 +25,7 @@ enum imx_sc_rpc_svc {
IMX_SC_RPC_SVC_PAD = 6,
IMX_SC_RPC_SVC_MISC = 7,
IMX_SC_RPC_SVC_IRQ = 8,
+ IMX_SC_RPC_SVC_SECO = 9,
};
struct imx_sc_rpc_msg {
diff --git a/include/linux/firmware/imx/sci.h b/include/linux/firmware/imx/sci.h
index 22c7657..914dce1 100644
--- a/include/linux/firmware/imx/sci.h
+++ b/include/linux/firmware/imx/sci.h
@@ -15,6 +15,7 @@
#include <linux/firmware/imx/svc/misc.h>
#include <linux/firmware/imx/svc/pm.h>
#include <linux/firmware/imx/svc/rm.h>
+#include <linux/firmware/imx/svc/seco.h>
int imx_scu_enable_general_irq_channel(struct device *dev);
int imx_scu_irq_register_notifier(struct notifier_block *nb);
diff --git a/include/linux/firmware/imx/svc/seco.h b/include/linux/firmware/imx/svc/seco.h
new file mode 100644
index 0000000..25450ad
--- /dev/null
+++ b/include/linux/firmware/imx/svc/seco.h
@@ -0,0 +1,73 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2020 NXP
+ *
+ * Header file containing the public API for the System Controller (SC)
+ * Security Controller (SECO) function.
+ *
+ * SECO_SVC (SVC) Security Controller Service
+ *
+ * Module for the Security Controller (SECO) service.
+ */
+
+#ifndef _SC_SECO_API_H
+#define _SC_SECO_API_H
+
+#include <linux/errno.h>
+#include <linux/firmware/imx/sci.h>
+
+/*
+ * This type is used to indicate RPC RM function calls.
+ */
+enum imx_sc_seco_func {
+ IMX_SC_SECO_FUNC_UNKNOWN = 0,
+ IMX_SC_SECO_FUNC_BUILD_INFO = 16,
+ IMX_SC_SECO_FUNC_SECVIO_ENABLE = 25,
+ IMX_SC_SECO_FUNC_SECVIO_CONFIG = 26,
+ IMX_SC_SECO_FUNC_SECVIO_DGO_CONFIG = 27,
+};
+
+#if IS_ENABLED(CONFIG_IMX_SCU)
+int imx_sc_seco_build_info(struct imx_sc_ipc *ipc, uint32_t *version,
+ uint32_t *commit);
+
+int imx_sc_seco_secvio_enable(struct imx_sc_ipc *ipc);
+
+int imx_sc_seco_secvio_config(struct imx_sc_ipc *ipc, u8 id, u8 access,
+ u32 *data0, u32 *data1, u32 *data2, u32 *data3,
+ u32 *data4, u8 size);
+
+int imx_sc_seco_secvio_dgo_config(struct imx_sc_ipc *ipc, u8 id, u8 access,
+ u32 *data);
+
+#else /* IS_ENABLED(CONFIG_IMX_SCU) */
+static inline
+int imx_sc_seco_build_info(struct imx_sc_ipc *ipc, uint32_t *version,
+ uint32_t *commit)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline
+int imx_sc_seco_secvio_enable(struct imx_sc_ipc *ipc)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline
+int imx_sc_seco_secvio_config(struct imx_sc_ipc *ipc, u8 id, u8 access,
+ u32 *data0, u32 *data1, u32 *data2, u32 *data3,
+ u32 *data4, u8 size)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline
+int imx_sc_seco_secvio_dgo_config(struct imx_sc_ipc *ipc, u8 id, u8 access,
+ u32 *data)
+{
+ return -EOPNOTSUPP;
+}
+#endif /* IS_ENABLED(CONFIG_IMX_SCU) */
+
+#endif /* _SC_SECO_API_H */
--
2.7.4
> From: Franck Lenormand (OSS) <[email protected]>
> Sent: Tuesday, July 21, 2020 11:21 PM
> Subject: [PATCH v2 1/5] firmware: imx: scu-seco: Add SEcure Controller APIS
Is 'Secure' intended? Not 'Secure'?
>
> This patch adds the APIs:
> - imx_sc_seco_build_info: get commit and sha of SECO
> - imx_sc_seco_secvio_enable: enable SNVS IRQ handling
> - imx_sc_seco_secvio_config: configure SNVS register
> - imx_sc_seco_secvio_dgo_config: configure SNVS DGO register
>
> Signed-off-by: Franck LENORMAND <[email protected]>
> ---
> drivers/firmware/imx/Makefile | 2 +-
> drivers/firmware/imx/imx-scu.c | 3 +
> drivers/firmware/imx/seco.c | 275
> ++++++++++++++++++++++++++++++++++
> include/linux/firmware/imx/ipc.h | 1 +
> include/linux/firmware/imx/sci.h | 1 +
> include/linux/firmware/imx/svc/seco.h | 73 +++++++++
> 6 files changed, 354 insertions(+), 1 deletion(-) create mode 100644
> drivers/firmware/imx/seco.c create mode 100644
> include/linux/firmware/imx/svc/seco.h
>
> diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
> index b76acba..f23e2b0 100644
> --- a/drivers/firmware/imx/Makefile
> +++ b/drivers/firmware/imx/Makefile
> @@ -1,4 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o
> imx-scu-soc.o
> +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o
> imx-scu-soc.o seco.o
> obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c
> index dca79ca..ed7b87b 100644
> --- a/drivers/firmware/imx/imx-scu.c
> +++ b/drivers/firmware/imx/imx-scu.c
> @@ -245,6 +245,9 @@ int imx_scu_call_rpc(struct imx_sc_ipc *sc_ipc, void
> *msg, bool have_resp)
> (saved_func == IMX_SC_MISC_FUNC_UNIQUE_ID ||
> saved_func == IMX_SC_MISC_FUNC_GET_BUTTON_STATUS))
> ret = 0;
> + if (saved_svc == IMX_SC_RPC_SVC_SECO &&
> + saved_func == IMX_SC_SECO_FUNC_BUILD_INFO)
> + ret = 0;
> }
Pls combined into the previous if statement.
>
> out:
> diff --git a/drivers/firmware/imx/seco.c b/drivers/firmware/imx/seco.c new file
> mode 100644 index 0000000..9047a75
> --- /dev/null
> +++ b/drivers/firmware/imx/seco.c
> @@ -0,0 +1,275 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2020 NXP
> + *
> + * File containing client-side RPC functions for the SECO service.
> +These
> + * functions are ported to clients that communicate to the SC.
> + */
> +
> +#include <linux/firmware/imx/sci.h>
> +
> +struct imx_sc_msg_seco_get_build_id {
> + struct imx_sc_rpc_msg hdr;
> + u32 version;
> + u32 commit;
> +};
> +
> +/**
> + * imx_sc_seco_build_info() - Get version and coomit ID of the SECO
s/coomit/commit
> + *
> + * @ipc: IPC handle
> + * @version: Version of the SECO
> + * @commit: Commit ID of the SECO
> + *
> + * Return:
> + * 0 - OK
> + * < 0 - error.
Can we follow the exist style?
@return Returns 0 for success and < 0 for errors
> + */
> +int imx_sc_seco_build_info(struct imx_sc_ipc *ipc, uint32_t *version,
> + uint32_t *commit)
> +{
> + int ret;
> + struct imx_sc_msg_seco_get_build_id msg = {0};
Unnecessary default to 0
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_SECO;
> + hdr->func = IMX_SC_SECO_FUNC_BUILD_INFO;
> + hdr->size = 1;
> +
> + ret = imx_scu_call_rpc(ipc, &msg, true);
> + if (ret)
> + return ret;
> +
> + if (version)
> + *version = msg.version;
> + if (commit)
> + *commit = msg.commit;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(imx_sc_seco_build_info);
> +
> +struct imx_sc_msg_seco_sab_msg {
> + struct imx_sc_rpc_msg hdr;
> + u32 smsg_addr_hi;
> + u32 smsg_addr_lo;
> +};
This can be added when used
> +
> +/**
> + * imx_sc_seco_secvio_enable() - Enable the processing of secvio IRQ
> +from the
> + * SNVS by the SECO
> + *
> + * @ipc: IPC handle
> + *
> + * Return:
> + * 0 - OK
> + * < 0 - error.
Ditto
> + */
> +int imx_sc_seco_secvio_enable(struct imx_sc_ipc *ipc) {
> + struct imx_sc_rpc_msg msg;
> + struct imx_sc_rpc_msg *hdr = &msg;
Unnecessary
> + int ret;
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_SECO;
> + hdr->func = IMX_SC_SECO_FUNC_SECVIO_ENABLE;
> + hdr->size = 1;
s/hdr->xxx/msg.xxx
> +
> + ret = imx_scu_call_rpc(ipc, &msg, true);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(imx_sc_seco_secvio_enable);
> +
> +struct imx_sc_msg_req_seco_config {
> + struct imx_sc_rpc_msg hdr;
> + u32 data0;
> + u32 data1;
> + u32 data2;
> + u32 data3;
> + u32 data4;
> + u8 id;
> + u8 access;
> + u8 size;
> +} __packed __aligned(4);
> +
> +struct imx_sc_msg_resp_seco_config {
> + struct imx_sc_rpc_msg hdr;
> + u32 data0;
> + u32 data1;
> + u32 data2;
> + u32 data3;
> + u32 data4;
> +} __packed;
Can we use the following style for better readability?
struct imx_sc_msg_get_clock_parent {
struct imx_sc_rpc_msg hdr;
union {
struct req_get_clock_parent {
__le16 resource;
u8 clk;
} __packed __aligned(4) req;
struct resp_get_clock_parent {
u8 parent;
} resp;
} data;
};
> +
> +/**
> + * imx_sc_seco_secvio_config() - Configure a set of SNVS registers for
> +SECure
> + * VIOlation
Secure Violation?
> + *
> + * Some registers are extended by others registers, they configure the
> +same
> + * kind of behavior, it constitutes a set
> + *
> + * @ipc: IPC handle
> + * @id: ID of the register, ie the offset of the first register of the
> +set
> + * @access: Write (1) or Read (0) the registers
> + * @data0: Data for the first register
> + * @data1: Data for the second register
> + * @data2: Data for the third register
> + * @data3: Data for the fourth register
> + * @data4: Data for the fifth register
> + * @size: Number of register to configure
> + *
> + * Return:
> + * 0 - OK
> + * < 0 - error.
> + */
> +int imx_sc_seco_secvio_config(struct imx_sc_ipc *ipc, u8 id, u8 access,
> + u32 *data0, u32 *data1, u32 *data2, u32 *data3,
> + u32 *data4, u8 size)
> +{
> + struct imx_sc_msg_req_seco_config msg;
> + struct imx_sc_msg_resp_seco_config *resp;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> + int ret;
> +
> + if (!ipc)
> + return -EINVAL;
Remove the unnecessary check
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_SECO;
> + hdr->func = IMX_SC_SECO_FUNC_SECVIO_CONFIG;
> + hdr->size = 7;
> +
> + /* Check the pointers on data are valid and set it if doing a write */
> + switch (size) {
> + case 5:
> + if (!data4)
> + return -EINVAL;
> + if (access)
> + msg.data4 = *data4;
> + fallthrough;
> + case 4:
> + if (!data3)
> + return -EINVAL;
> + if (access)
> + msg.data3 = *data3;
> + fallthrough;
> + case 3:
> + if (!data2)
> + return -EINVAL;
> + if (access)
> + msg.data2 = *data2;
> + fallthrough;
> + case 2:
> + if (!data1)
> + return -EINVAL;
> + if (access)
> + msg.data1 = *data1;
> + fallthrough;
> + case 1:
> + if (!data0)
> + return -EINVAL;
> + if (access)
> + msg.data0 = *data0;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + msg.id = id;
> + msg.access = access;
> + msg.size = size;
> +
> + ret = imx_scu_call_rpc(ipc, &msg, true);
> + if (ret)
> + return ret;
> +
> + resp = (struct imx_sc_msg_resp_seco_config *)&msg;
> +
> + /* Pointers already checked so we just copy the data if reading */
> + if (!access)
> + switch (size) {
> + case 5:
> + *data4 = resp->data4;
> + fallthrough;
> + case 4:
> + *data3 = resp->data3;
> + fallthrough;
> + case 3:
> + *data2 = resp->data2;
> + fallthrough;
> + case 2:
> + *data1 = resp->data1;
> + fallthrough;
> + case 1:
> + *data0 = resp->data0;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(imx_sc_seco_secvio_config);
> +
> +struct imx_sc_msg_req_seco_dgo_config {
> + struct imx_sc_rpc_msg hdr;
> + u32 data;
> + u8 id;
> + u8 access;
> +} __packed __aligned(4);
> +
> +struct imx_sc_msg_resp_seco_dgo_config {
> + struct imx_sc_rpc_msg hdr;
> + u32 data;
> +} __packed;
I may suggest you to follow the same style as I showed above
struct imx_sc_msg_get_clock_parent
> +
> +/**
> + * imx_sc_seco_secvio_enable() - Configure the DGO module
Wrong function name
> + *
> + * @ipc: IPC handle
> + * @id: ID of the register, ie the offset of the register
> + * @access: Write (1) or Read (0) the registers
> + * @data: Data for the register
> + *
> + * Return:
> + * 0 - OK
> + * < 0 - error.
> + */
> +int imx_sc_seco_secvio_dgo_config(struct imx_sc_ipc *ipc, u8 id, u8 access,
> + u32 *data)
> +{
> + struct imx_sc_msg_req_seco_dgo_config msg;
> + struct imx_sc_msg_resp_seco_dgo_config *resp;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> + int ret;
> +
> + if (!ipc)
> + return -EINVAL;
Remove unnecessary check
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_SECO;
> + hdr->func = IMX_SC_SECO_FUNC_SECVIO_DGO_CONFIG;
> + hdr->size = 3;
> +
> + if (!data)
> + return -EINVAL;
> + if (access)
> + msg.data = *data;
> +
> + msg.access = access;
> + msg.id = id;
> +
> + ret = imx_scu_call_rpc(ipc, &msg, true);
> + if (ret)
> + return ret;
> +
> + resp = (struct imx_sc_msg_resp_seco_dgo_config *)&msg;
> +
> + if (!access && data)
> + *data = resp->data;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(imx_sc_seco_secvio_dgo_config);
> diff --git a/include/linux/firmware/imx/ipc.h b/include/linux/firmware/imx/ipc.h
> index 89105743..6924359 100644
> --- a/include/linux/firmware/imx/ipc.h
> +++ b/include/linux/firmware/imx/ipc.h
> @@ -25,6 +25,7 @@ enum imx_sc_rpc_svc {
> IMX_SC_RPC_SVC_PAD = 6,
> IMX_SC_RPC_SVC_MISC = 7,
> IMX_SC_RPC_SVC_IRQ = 8,
> + IMX_SC_RPC_SVC_SECO = 9,
> };
>
> struct imx_sc_rpc_msg {
> diff --git a/include/linux/firmware/imx/sci.h b/include/linux/firmware/imx/sci.h
> index 22c7657..914dce1 100644
> --- a/include/linux/firmware/imx/sci.h
> +++ b/include/linux/firmware/imx/sci.h
> @@ -15,6 +15,7 @@
> #include <linux/firmware/imx/svc/misc.h> #include
> <linux/firmware/imx/svc/pm.h> #include <linux/firmware/imx/svc/rm.h>
> +#include <linux/firmware/imx/svc/seco.h>
>
> int imx_scu_enable_general_irq_channel(struct device *dev); int
> imx_scu_irq_register_notifier(struct notifier_block *nb); diff --git
> a/include/linux/firmware/imx/svc/seco.h
> b/include/linux/firmware/imx/svc/seco.h
> new file mode 100644
> index 0000000..25450ad
> --- /dev/null
> +++ b/include/linux/firmware/imx/svc/seco.h
> @@ -0,0 +1,73 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright 2020 NXP
> + *
> + * Header file containing the public API for the System Controller (SC)
> + * Security Controller (SECO) function.
> + *
> + * SECO_SVC (SVC) Security Controller Service
> + *
> + * Module for the Security Controller (SECO) service.
> + */
> +
> +#ifndef _SC_SECO_API_H
> +#define _SC_SECO_API_H
> +
> +#include <linux/errno.h>
> +#include <linux/firmware/imx/sci.h>
> +
> +/*
> + * This type is used to indicate RPC RM function calls.
s/RM/SECO
Regards
Aisheng
> + */
> +enum imx_sc_seco_func {
> + IMX_SC_SECO_FUNC_UNKNOWN = 0,
> + IMX_SC_SECO_FUNC_BUILD_INFO = 16,
> + IMX_SC_SECO_FUNC_SECVIO_ENABLE = 25,
> + IMX_SC_SECO_FUNC_SECVIO_CONFIG = 26,
> + IMX_SC_SECO_FUNC_SECVIO_DGO_CONFIG = 27, };
> +
> +#if IS_ENABLED(CONFIG_IMX_SCU)
> +int imx_sc_seco_build_info(struct imx_sc_ipc *ipc, uint32_t *version,
> + uint32_t *commit);
> +
> +int imx_sc_seco_secvio_enable(struct imx_sc_ipc *ipc);
> +
> +int imx_sc_seco_secvio_config(struct imx_sc_ipc *ipc, u8 id, u8 access,
> + u32 *data0, u32 *data1, u32 *data2, u32 *data3,
> + u32 *data4, u8 size);
> +
> +int imx_sc_seco_secvio_dgo_config(struct imx_sc_ipc *ipc, u8 id, u8 access,
> + u32 *data);
> +
> +#else /* IS_ENABLED(CONFIG_IMX_SCU) */
> +static inline
> +int imx_sc_seco_build_info(struct imx_sc_ipc *ipc, uint32_t *version,
> + uint32_t *commit)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline
> +int imx_sc_seco_secvio_enable(struct imx_sc_ipc *ipc) {
> + return -EOPNOTSUPP;
> +}
> +
> +static inline
> +int imx_sc_seco_secvio_config(struct imx_sc_ipc *ipc, u8 id, u8 access,
> + u32 *data0, u32 *data1, u32 *data2, u32 *data3,
> + u32 *data4, u8 size)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline
> +int imx_sc_seco_secvio_dgo_config(struct imx_sc_ipc *ipc, u8 id, u8 access,
> + u32 *data)
> +{
> + return -EOPNOTSUPP;
> +}
> +#endif /* IS_ENABLED(CONFIG_IMX_SCU) */
> +
> +#endif /* _SC_SECO_API_H */
> --
> 2.7.4