Introduce a driver to provide calls into secure monitor mode.
This driver is used for SoCs produced by Samsung Foundry to provide
SMC call. This patch supports register read/write request to secure
monitor. Also, SMC call request which uses shared memory to exchange
the data between kernel and secure monitor.
Signed-off-by: Dongjin Yang <[email protected]>
---
MAINTAINERS | 2 +
drivers/firmware/Kconfig | 11 +++
drivers/firmware/Makefile | 1 +
drivers/firmware/samsung-smc-svc.c | 154 +++++++++++++++++++++++++++++++
include/linux/firmware/samsung-smc-svc.h | 59 ++++++++++++
5 files changed, 227 insertions(+)
create mode 100644 drivers/firmware/samsung-smc-svc.c
create mode 100644 include/linux/firmware/samsung-smc-svc.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 6763746c349f..d173043ffb46 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1868,8 +1868,10 @@ F: arch/arm/boot/dts/artpec6*
F: arch/arm/mach-artpec
F: drivers/clk/axis
F: drivers/crypto/axis
+F: drivers/firmware/samsung-smc-svc.c
F: drivers/mmc/host/usdhi6rol0.c
F: drivers/pinctrl/pinctrl-artpec*
+F: include/linux/firmware/samsung-smc-svc.h
ARM/ASPEED I2C DRIVER
M: Brendan Higgins <[email protected]>
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index e5cfb01353d8..4b0f2d033f58 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -217,6 +217,17 @@ config QCOM_SCM_DOWNLOAD_MODE_DEFAULT
Say Y here to enable "download mode" by default.
+config SAMSUNG_SECURE_SERVICE
+ bool "Samsung Foundry Secure Service Layer"
+ depends on HAVE_ARM_SMCCC
+ default n
+ help
+ Support secure service layer for SoCs which is manufactured by
+ Samsung Foundry.
+
+ This option provide support of secure monitor service call using
+ Trusted Foundations.
+
config SYSFB
bool
default y
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 4e58cb474a68..985e30a9665f 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o
obj-$(CONFIG_FW_CFG_SYSFS) += qemu_fw_cfg.o
obj-$(CONFIG_QCOM_SCM) += qcom-scm.o
qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
+obj-$(CONFIG_SAMSUNG_SECURE_SERVICE) += samsung-smc-svc.o
obj-$(CONFIG_SYSFB) += sysfb.o
obj-$(CONFIG_SYSFB_SIMPLEFB) += sysfb_simplefb.o
obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o
diff --git a/drivers/firmware/samsung-smc-svc.c b/drivers/firmware/samsung-smc-svc.c
new file mode 100644
index 000000000000..eb3a5285cf2b
--- /dev/null
+++ b/drivers/firmware/samsung-smc-svc.c
@@ -0,0 +1,154 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 Samsung Electronics Co., Ltd.
+ * Author: Kihyun Yoon<[email protected]>
+ * Author: Dongjin Yang<[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/firmware/samsung-smc-svc.h>
+
+struct samsung_smc_version {
+ u32 major;
+ u32 minor;
+};
+
+struct samsung_smc_data {
+ struct samsung_smc_version version;
+ u32 svc_cmd_list[SAMSUNG_SIP_NR_SIP];
+};
+
+struct samsung_smc_data *svc_data;
+
+static int samsung_smc_svc_init(void);
+
+static int __samsung_get_svc_cmd(u32 cmd_idx)
+{
+ if (cmd_idx >= SAMSUNG_SIP_NR_SIP)
+ return SAMSUNG_SIP_NOT_SUPPORTED;
+
+ /* Initialize this driver if it is not ready */
+ if (!svc_data && samsung_smc_svc_init()) {
+ pr_err("samsung,smccc_service initialization is failed\n");
+ return SAMSUNG_SIP_NOT_SUPPORTED;
+ }
+
+ return svc_data->svc_cmd_list[cmd_idx];
+}
+
+int samsung_smc_call(u32 cmd_idx,
+ unsigned long a0, unsigned long a1,
+ unsigned long a2, unsigned long a3,
+ u32 *val)
+
+{
+ struct arm_smccc_res res;
+ u32 cmd = __samsung_get_svc_cmd(cmd_idx);
+
+ if (cmd == SAMSUNG_SIP_NOT_SUPPORTED)
+ return -EOPNOTSUPP;
+
+ arm_smccc_smc(cmd, a0, a1, a2, a3, 0, 0, 0, &res);
+ if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
+ return -EINVAL;
+
+ if (val)
+ *val = (u32)res.a1;
+
+ return 0;
+}
+EXPORT_SYMBOL(samsung_smc_call);
+
+int samsung_smc_reg_read(void *base, u32 reg, u32 *val)
+{
+ return samsung_smc_call(SAMSUNG_SIP_READ_REG,
+ (unsigned long)base, reg, 0, 0, val);
+}
+EXPORT_SYMBOL(samsung_smc_reg_read);
+
+int samsung_smc_reg_write(void *base, u32 reg, u32 val)
+{
+ return samsung_smc_call(SAMSUNG_SIP_WRITE_REG,
+ (unsigned long)base, reg, val, 0, NULL);
+}
+EXPORT_SYMBOL(samsung_smc_reg_write);
+
+static int samsung_smc_check_version(struct samsung_smc_version *ver)
+{
+ struct arm_smccc_res res;
+
+ arm_smccc_smc(SAMSUNG_SIP_SVC_VERSION, 0, 0, 0, 0, 0, 0, 0, &res);
+
+ if ((u32)res.a0 != ver->major || (u32)res.a1 != ver->minor)
+ return -EINVAL;
+
+ pr_info("Samsung SVC version [%d.%d] is detected\n",
+ ver->major, ver->minor);
+
+ return 0;
+}
+
+static const struct samsung_smc_data svc_0_1 = {
+ .version = {
+ .major = 0,
+ .minor = 1
+ },
+ .svc_cmd_list = {
+ [SAMSUNG_SIP_READ_REG] = 0x82000020,
+ [SAMSUNG_SIP_WRITE_REG] = 0x82000021,
+ /* TODO: SMC call for clock driver should be added */
+ }
+};
+
+static const struct of_device_id samsung_smc_of_match[] = {
+ { .compatible = "samsung,smccc-svc", .data = &svc_0_1 },
+ { /* sentinel */ }
+};
+
+static int samsung_smc_svc_init(void)
+{
+ struct device_node *fw_np;
+ struct device_node *np;
+ const struct of_device_id *match;
+ int ret;
+
+ if (svc_data) {
+ pr_info("samsung,smccc_service is already initialized\n");
+ return 0;
+ }
+
+ fw_np = of_find_node_by_name(NULL, "firmware");
+ if (!fw_np)
+ return -ENODEV;
+
+ np = of_find_matching_node_and_match(fw_np, samsung_smc_of_match,
+ &match);
+ if (!np)
+ return -ENODEV;
+
+ svc_data = (struct samsung_smc_data *)match->data;
+ if (!svc_data)
+ return -ENODEV;
+
+ ret = samsung_smc_check_version(&svc_data->version);
+ if (ret) {
+ pr_err("samsung svc version is not matched\n");
+ return ret;
+ }
+
+ pr_info("samsung smc svc is initialized\n");
+ return 0;
+}
+
+early_initcall(samsung_smc_svc_init);
diff --git a/include/linux/firmware/samsung-smc-svc.h b/include/linux/firmware/samsung-smc-svc.h
new file mode 100644
index 000000000000..9c94fd3e10a2
--- /dev/null
+++ b/include/linux/firmware/samsung-smc-svc.h
@@ -0,0 +1,59 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __SAMSUNG_SMC_H
+#define __SAMSUNG_SMC_H
+
+#include <linux/arm-smccc.h>
+
+#ifdef CONFIG_SAMSUNG_SECURE_SERVICE
+
+#define SAMSUNG_SIP_CALL_VAL(x) \
+ ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+ ARM_SMCCC_SMC_32, ARM_SMCCC_OWNER_SIP, x)
+
+/* Common SIP SVC number */
+#define SAMSUING_SIP_SVC_CALL_COUNT SAMSUNG_SIP_CALL_VAL(0xff00)
+#define SAMSUING_SIP_SVC_UID SAMSUNG_SIP_CALL_VAL(0xff01)
+#define SAMSUNG_SIP_SVC_VERSION SAMSUNG_SIP_CALL_VAL(0xff03)
+#define SAMSUNG_SIP_NOT_SUPPORTED 0x0
+
+enum {
+ /* SIP enumeration for V0.1 */
+ SAMSUNG_SIP_READ_REG,
+ SAMSUNG_SIP_WRITE_REG,
+ /* Add here for later version */
+ SAMSUNG_SIP_NR_SIP
+};
+
+int samsung_smc_call(u32 cmd_idx,
+ unsigned long a0, unsigned long a1,
+ unsigned long a2, unsigned long a3,
+ u32 *val);
+int samsung_smc_reg_read(void *addr, unsigned int reg, u32 *val);
+int samsung_smc_reg_write(void *addr, unsigned int reg, u32 val);
+
+#else
+
+static inline int samsung_smc_call(u32 cmd_idx,
+ unsigned long a0, unsigned long a1,
+ unsigned long a2, unsigned long a3,
+ u32 *val)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int samsung_smc_reg_read(void *base,
+ unsigned int reg, u32 *val)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int samsung_smc_reg_write(void *base,
+ unsigned int reg, u32 val)
+{
+ return -EOPNOTSUPP;
+}
+
+#endif
+
+#endif /* __SAMSUNG_SMC_H */
--
2.9.5
Hi--
On 7/12/22 21:55, Dongjin Yang wrote:
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index e5cfb01353d8..4b0f2d033f58 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -217,6 +217,17 @@ config QCOM_SCM_DOWNLOAD_MODE_DEFAULT
>
> Say Y here to enable "download mode" by default.
>
> +config SAMSUNG_SECURE_SERVICE
> + bool "Samsung Foundry Secure Service Layer"
> + depends on HAVE_ARM_SMCCC
> + default n
Drop that line, it's the default anyway.
> + help
> + Support secure service layer for SoCs which is manufactured by
which are
> + Samsung Foundry.
> +
> + This option provide support of secure monitor service call using
> + Trusted Foundations.
--
~Randy
On 13/07/2022 06:55, Dongjin Yang wrote:
> Introduce a driver to provide calls into secure monitor mode.
> This driver is used for SoCs produced by Samsung Foundry to provide
> SMC call. This patch supports register read/write request to secure
> monitor. Also, SMC call request which uses shared memory to exchange
> the data between kernel and secure monitor.
>
> Signed-off-by: Dongjin Yang <[email protected]>
> ---
> MAINTAINERS | 2 +
> drivers/firmware/Kconfig | 11 +++
> drivers/firmware/Makefile | 1 +
> drivers/firmware/samsung-smc-svc.c | 154 +++++++++++++++++++++++++++++++
> include/linux/firmware/samsung-smc-svc.h | 59 ++++++++++++
> 5 files changed, 227 insertions(+)
> create mode 100644 drivers/firmware/samsung-smc-svc.c
> create mode 100644 include/linux/firmware/samsung-smc-svc.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6763746c349f..d173043ffb46 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1868,8 +1868,10 @@ F: arch/arm/boot/dts/artpec6*
> F: arch/arm/mach-artpec
> F: drivers/clk/axis
> F: drivers/crypto/axis
> +F: drivers/firmware/samsung-smc-svc.c
> F: drivers/mmc/host/usdhi6rol0.c
> F: drivers/pinctrl/pinctrl-artpec*
> +F: include/linux/firmware/samsung-smc-svc.h
Same as for bindings - not related to Artpec platforms. Either add
dedicated entry or put it under Samsung SoC.
>
> ARM/ASPEED I2C DRIVER
> M: Brendan Higgins <[email protected]>
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index e5cfb01353d8..4b0f2d033f58 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -217,6 +217,17 @@ config QCOM_SCM_DOWNLOAD_MODE_DEFAULT
>
> Say Y here to enable "download mode" by default.
>
> +config SAMSUNG_SECURE_SERVICE
> + bool "Samsung Foundry Secure Service Layer"
> + depends on HAVE_ARM_SMCCC
> + default n
> + help
> + Support secure service layer for SoCs which is manufactured by
> + Samsung Foundry.
> +
> + This option provide support of secure monitor service call using
> + Trusted Foundations.
> +
> config SYSFB
> bool
> default y
> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
> index 4e58cb474a68..985e30a9665f 100644
> --- a/drivers/firmware/Makefile
> +++ b/drivers/firmware/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o
> obj-$(CONFIG_FW_CFG_SYSFS) += qemu_fw_cfg.o
> obj-$(CONFIG_QCOM_SCM) += qcom-scm.o
> qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
> +obj-$(CONFIG_SAMSUNG_SECURE_SERVICE) += samsung-smc-svc.o
> obj-$(CONFIG_SYSFB) += sysfb.o
> obj-$(CONFIG_SYSFB_SIMPLEFB) += sysfb_simplefb.o
> obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o
> diff --git a/drivers/firmware/samsung-smc-svc.c b/drivers/firmware/samsung-smc-svc.c
> new file mode 100644
> index 000000000000..eb3a5285cf2b
> --- /dev/null
> +++ b/drivers/firmware/samsung-smc-svc.c
> @@ -0,0 +1,154 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 Samsung Electronics Co., Ltd.
> + * Author: Kihyun Yoon<[email protected]>
> + * Author: Dongjin Yang<[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
Drop boiler plate with trailing empty '*' comment.
> + */
> +
> +#include <linux/types.h>
> +#include <linux/errno.h>
> +#include <linux/module.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/firmware/samsung-smc-svc.h>
Order the headers by name.
> +
> +struct samsung_smc_version {
> + u32 major;
> + u32 minor;
> +};
> +
> +struct samsung_smc_data {
> + struct samsung_smc_version version;
> + u32 svc_cmd_list[SAMSUNG_SIP_NR_SIP];
> +};
> +
> +struct samsung_smc_data *svc_data;
No, no globals. Even no static variables.
> +
> +static int samsung_smc_svc_init(void);
> +
> +static int __samsung_get_svc_cmd(u32 cmd_idx)
> +{
> + if (cmd_idx >= SAMSUNG_SIP_NR_SIP)
> + return SAMSUNG_SIP_NOT_SUPPORTED;
> +
> + /* Initialize this driver if it is not ready */
> + if (!svc_data && samsung_smc_svc_init()) {
> + pr_err("samsung,smccc_service initialization is failed\n");
> + return SAMSUNG_SIP_NOT_SUPPORTED;
> + }
> +
> + return svc_data->svc_cmd_list[cmd_idx];
> +}
> +
> +int samsung_smc_call(u32 cmd_idx,
> + unsigned long a0, unsigned long a1,
> + unsigned long a2, unsigned long a3,
> + u32 *val)
> +
> +{
> + struct arm_smccc_res res;
> + u32 cmd = __samsung_get_svc_cmd(cmd_idx);
> +
> + if (cmd == SAMSUNG_SIP_NOT_SUPPORTED)
> + return -EOPNOTSUPP;
> +
> + arm_smccc_smc(cmd, a0, a1, a2, a3, 0, 0, 0, &res);
> + if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
> + return -EINVAL;
> +
> + if (val)
> + *val = (u32)res.a1;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(samsung_smc_call);
EXPORT_SYMBOL_GPL
You need kerneldoc for all these.
> +
> +int samsung_smc_reg_read(void *base, u32 reg, u32 *val)
> +{
> + return samsung_smc_call(SAMSUNG_SIP_READ_REG,
> + (unsigned long)base, reg, 0, 0, val);
> +}
> +EXPORT_SYMBOL(samsung_smc_reg_read);
> +
> +int samsung_smc_reg_write(void *base, u32 reg, u32 val)
> +{
> + return samsung_smc_call(SAMSUNG_SIP_WRITE_REG,
> + (unsigned long)base, reg, val, 0, NULL);
> +}
> +EXPORT_SYMBOL(samsung_smc_reg_write);
> +
> +static int samsung_smc_check_version(struct samsung_smc_version *ver)
> +{
> + struct arm_smccc_res res;
> +
> + arm_smccc_smc(SAMSUNG_SIP_SVC_VERSION, 0, 0, 0, 0, 0, 0, 0, &res);
> +
> + if ((u32)res.a0 != ver->major || (u32)res.a1 != ver->minor)
> + return -EINVAL;
> +
> + pr_info("Samsung SVC version [%d.%d] is detected\n",
> + ver->major, ver->minor);
> +
> + return 0;
> +}
> +
> +static const struct samsung_smc_data svc_0_1 = {
> + .version = {
> + .major = 0,
> + .minor = 1
> + },
> + .svc_cmd_list = {
> + [SAMSUNG_SIP_READ_REG] = 0x82000020,
> + [SAMSUNG_SIP_WRITE_REG] = 0x82000021,
> + /* TODO: SMC call for clock driver should be added */
> + }
> +};
> +
> +static const struct of_device_id samsung_smc_of_match[] = {
> + { .compatible = "samsung,smccc-svc", .data = &svc_0_1 },
> + { /* sentinel */ }
> +};
> +
> +static int samsung_smc_svc_init(void)
> +{
> + struct device_node *fw_np;
> + struct device_node *np;
> + const struct of_device_id *match;
> + int ret;
> +
> + if (svc_data) {
> + pr_info("samsung,smccc_service is already initialized\n");
> + return 0;
> + }
> +
> + fw_np = of_find_node_by_name(NULL, "firmware");
> + if (!fw_np)
> + return -ENODEV;
> +
> + np = of_find_matching_node_and_match(fw_np, samsung_smc_of_match,
> + &match);
> + if (!np)
> + return -ENODEV;
> +
> + svc_data = (struct samsung_smc_data *)match->data;
> + if (!svc_data)
> + return -ENODEV;
> +
> + ret = samsung_smc_check_version(&svc_data->version);
> + if (ret) {
> + pr_err("samsung svc version is not matched\n");
> + return ret;
> + }
> +
> + pr_info("samsung smc svc is initialized\n");
No useless success printks. We have tracing and sysfs for that.
> + return 0;
> +}
> +
> +early_initcall(samsung_smc_svc_init);
No. Make it a platform driver.
> diff --git a/include/linux/firmware/samsung-smc-svc.h b/include/linux/firmware/samsung-smc-svc.h
> new file mode 100644
> index 000000000000..9c94fd3e10a2
> --- /dev/null
> +++ b/include/linux/firmware/samsung-smc-svc.h
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __SAMSUNG_SMC_H
> +#define __SAMSUNG_SMC_H
Extend the header gaurd to match path.
> +
> +#include <linux/arm-smccc.h>
> +
> +#ifdef CONFIG_SAMSUNG_SECURE_SERVICE
No ifdefs in headers. This is not a subsystem.
> +
> +#define SAMSUNG_SIP_CALL_VAL(x) \
> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> + ARM_SMCCC_SMC_32, ARM_SMCCC_OWNER_SIP, x)
Need kerneldoc.
> +
> +/* Common SIP SVC number */
> +#define SAMSUING_SIP_SVC_CALL_COUNT SAMSUNG_SIP_CALL_VAL(0xff00)
> +#define SAMSUING_SIP_SVC_UID SAMSUNG_SIP_CALL_VAL(0xff01)
> +#define SAMSUNG_SIP_SVC_VERSION SAMSUNG_SIP_CALL_VAL(0xff03)
> +#define SAMSUNG_SIP_NOT_SUPPORTED 0x0
Need kerneldoc.
> +
> +enum {
> + /* SIP enumeration for V0.1 */
> + SAMSUNG_SIP_READ_REG,
> + SAMSUNG_SIP_WRITE_REG,
> + /* Add here for later version */
> + SAMSUNG_SIP_NR_SIP
> +};
Need kerneldoc.
> +
> +int samsung_smc_call(u32 cmd_idx,
> + unsigned long a0, unsigned long a1,
> + unsigned long a2, unsigned long a3,
> + u32 *val);
> +int samsung_smc_reg_read(void *addr, unsigned int reg, u32 *val);
> +int samsung_smc_reg_write(void *addr, unsigned int reg, u32 val);
> +
Need kerneldoc in the driver.
> +#else
> +
No stubs, no ifdefs. It's not a subsystem.
Best regards,
Krzysztof
On 7/13/22 04:58, Randy Dunlap wrote:
> Hi--
>
> On 7/12/22 21:55, Dongjin Yang wrote:
> > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> > index e5cfb01353d8..4b0f2d033f58 100644
> > --- a/drivers/firmware/Kconfig
> > +++ b/drivers/firmware/Kconfig
> > @@ -217,6 +217,17 @@ config QCOM_SCM_DOWNLOAD_MODE_DEFAULT
> >
> > Say Y here to enable "download mode" by default.
> >
> > +config SAMSUNG_SECURE_SERVICE
> > + bool "Samsung Foundry Secure Service Layer"
> > + depends on HAVE_ARM_SMCCC
> > + default n
>
> Drop that line, it's the default anyway.
Sure thanks.
>
> > + help
> > + Support secure service layer for SoCs which is manufactured by
>
> which are
Thanks for correcting.
>
> > + Samsung Foundry.
> > +
> > + This option provide support of secure monitor service call using
> > + Trusted Foundations.
>
> --
> ~Randy
On 14/07/2022 04:28, Krzysztof Kozlowski wrote:
> On 13/07/2022 06:55, Dongjin Yang wrote:
> > Introduce a driver to provide calls into secure monitor mode.
> > This driver is used for SoCs produced by Samsung Foundry to provide
> > SMC call. This patch supports register read/write request to secure
> > monitor. Also, SMC call request which uses shared memory to exchange
> > the data between kernel and secure monitor.
> >
> > Signed-off-by: Dongjin Yang <[email protected]>
> > ---
> > MAINTAINERS | 2 +
> > drivers/firmware/Kconfig | 11 +++
> > drivers/firmware/Makefile | 1 +
> > drivers/firmware/samsung-smc-svc.c | 154 +++++++++++++++++++++++++++++++
> > include/linux/firmware/samsung-smc-svc.h | 59 ++++++++++++
> > 5 files changed, 227 insertions(+)
> > create mode 100644 drivers/firmware/samsung-smc-svc.c
> > create mode 100644 include/linux/firmware/samsung-smc-svc.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 6763746c349f..d173043ffb46 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1868,8 +1868,10 @@ F: arch/arm/boot/dts/artpec6*
> > F: arch/arm/mach-artpec
> > F: drivers/clk/axis
> > F: drivers/crypto/axis
> > +F: drivers/firmware/samsung-smc-svc.c
> > F: drivers/mmc/host/usdhi6rol0.c
> > F: drivers/pinctrl/pinctrl-artpec*
> > +F: include/linux/firmware/samsung-smc-svc.h
>
> Same as for bindings - not related to Artpec platforms. Either add
> dedicated entry or put it under Samsung SoC.
>
This patch is for Artpec8 SoC.
> >
> > ARM/ASPEED I2C DRIVER
> > M: Brendan Higgins <[email protected]>
> > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> > index e5cfb01353d8..4b0f2d033f58 100644
> > --- a/drivers/firmware/Kconfig
> > +++ b/drivers/firmware/Kconfig
> > @@ -217,6 +217,17 @@ config QCOM_SCM_DOWNLOAD_MODE_DEFAULT
> >
> > Say Y here to enable "download mode" by default.
> >
> > +config SAMSUNG_SECURE_SERVICE
> > + bool "Samsung Foundry Secure Service Layer"
> > + depends on HAVE_ARM_SMCCC
> > + default n
> > + help
> > + Support secure service layer for SoCs which is manufactured by
> > + Samsung Foundry.
> > +
> > + This option provide support of secure monitor service call using
> > + Trusted Foundations.
> > +
> > config SYSFB
> > bool
> > default y
> > diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
> > index 4e58cb474a68..985e30a9665f 100644
> > --- a/drivers/firmware/Makefile
> > +++ b/drivers/firmware/Makefile
> > @@ -19,6 +19,7 @@ obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o
> > obj-$(CONFIG_FW_CFG_SYSFS) += qemu_fw_cfg.o
> > obj-$(CONFIG_QCOM_SCM) += qcom-scm.o
> > qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
> > +obj-$(CONFIG_SAMSUNG_SECURE_SERVICE) += samsung-smc-svc.o
> > obj-$(CONFIG_SYSFB) += sysfb.o
> > obj-$(CONFIG_SYSFB_SIMPLEFB) += sysfb_simplefb.o
> > obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o
> > diff --git a/drivers/firmware/samsung-smc-svc.c b/drivers/firmware/samsung-smc-svc.c
> > new file mode 100644
> > index 000000000000..eb3a5285cf2b
> > --- /dev/null
> > +++ b/drivers/firmware/samsung-smc-svc.c
> > @@ -0,0 +1,154 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2019 Samsung Electronics Co., Ltd.
> > + * Author: Kihyun Yoon<[email protected]>
> > + * Author: Dongjin Yang<[email protected]>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
>
> Drop boiler plate with trailing empty '*' comment.
>
Yes,
> > + */
> > +
> > +#include <linux/types.h>
> > +#include <linux/errno.h>
> > +#include <linux/module.h>
> > +#include <linux/io.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/firmware/samsung-smc-svc.h>
>
> Order the headers by name.
>
Yes I will,
> > +
> > +struct samsung_smc_version {
> > + u32 major;
> > + u32 minor;
> > +};
> > +
> > +struct samsung_smc_data {
> > + struct samsung_smc_version version;
> > + u32 svc_cmd_list[SAMSUNG_SIP_NR_SIP];
> > +};
> > +
> > +struct samsung_smc_data *svc_data;
>
> No, no globals. Even no static variables.
>
Yes, let me change the driver
> > +
> > +static int samsung_smc_svc_init(void);
> > +
> > +static int __samsung_get_svc_cmd(u32 cmd_idx)
> > +{
> > + if (cmd_idx >= SAMSUNG_SIP_NR_SIP)
> > + return SAMSUNG_SIP_NOT_SUPPORTED;
> > +
> > + /* Initialize this driver if it is not ready */
> > + if (!svc_data && samsung_smc_svc_init()) {
> > + pr_err("samsung,smccc_service initialization is failed\n");
> > + return SAMSUNG_SIP_NOT_SUPPORTED;
> > + }
> > +
> > + return svc_data->svc_cmd_list[cmd_idx];
> > +}
> > +
> > +int samsung_smc_call(u32 cmd_idx,
> > + unsigned long a0, unsigned long a1,
> > + unsigned long a2, unsigned long a3,
> > + u32 *val)
> > +
> > +{
> > + struct arm_smccc_res res;
> > + u32 cmd = __samsung_get_svc_cmd(cmd_idx);
> > +
> > + if (cmd == SAMSUNG_SIP_NOT_SUPPORTED)
> > + return -EOPNOTSUPP;
> > +
> > + arm_smccc_smc(cmd, a0, a1, a2, a3, 0, 0, 0, &res);
> > + if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
> > + return -EINVAL;
> > +
> > + if (val)
> > + *val = (u32)res.a1;
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(samsung_smc_call);
>
> EXPORT_SYMBOL_GPL
>
> You need kerneldoc for all these.
>
Yes,
> > +
> > +int samsung_smc_reg_read(void *base, u32 reg, u32 *val)
> > +{
> > + return samsung_smc_call(SAMSUNG_SIP_READ_REG,
> > + (unsigned long)base, reg, 0, 0, val);
> > +}
> > +EXPORT_SYMBOL(samsung_smc_reg_read);
> > +
> > +int samsung_smc_reg_write(void *base, u32 reg, u32 val)
> > +{
> > + return samsung_smc_call(SAMSUNG_SIP_WRITE_REG,
> > + (unsigned long)base, reg, val, 0, NULL);
> > +}
> > +EXPORT_SYMBOL(samsung_smc_reg_write);
> > +
> > +static int samsung_smc_check_version(struct samsung_smc_version *ver)
> > +{
> > + struct arm_smccc_res res;
> > +
> > + arm_smccc_smc(SAMSUNG_SIP_SVC_VERSION, 0, 0, 0, 0, 0, 0, 0, &res);
> > +
> > + if ((u32)res.a0 != ver->major || (u32)res.a1 != ver->minor)
> > + return -EINVAL;
> > +
> > + pr_info("Samsung SVC version [%d.%d] is detected\n",
> > + ver->major, ver->minor);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct samsung_smc_data svc_0_1 = {
> > + .version = {
> > + .major = 0,
> > + .minor = 1
> > + },
> > + .svc_cmd_list = {
> > + [SAMSUNG_SIP_READ_REG] = 0x82000020,
> > + [SAMSUNG_SIP_WRITE_REG] = 0x82000021,
> > + /* TODO: SMC call for clock driver should be added */
> > + }
> > +};
> > +
> > +static const struct of_device_id samsung_smc_of_match[] = {
> > + { .compatible = "samsung,smccc-svc", .data = &svc_0_1 },
> > + { /* sentinel */ }
> > +};
> > +
> > +static int samsung_smc_svc_init(void)
> > +{
> > + struct device_node *fw_np;
> > + struct device_node *np;
> > + const struct of_device_id *match;
> > + int ret;
> > +
> > + if (svc_data) {
> > + pr_info("samsung,smccc_service is already initialized\n");
> > + return 0;
> > + }
> > +
> > + fw_np = of_find_node_by_name(NULL, "firmware");
> > + if (!fw_np)
> > + return -ENODEV;
> > +
> > + np = of_find_matching_node_and_match(fw_np, samsung_smc_of_match,
> > + &match);
> > + if (!np)
> > + return -ENODEV;
> > +
> > + svc_data = (struct samsung_smc_data *)match->data;
> > + if (!svc_data)
> > + return -ENODEV;
> > +
> > + ret = samsung_smc_check_version(&svc_data->version);
> > + if (ret) {
> > + pr_err("samsung svc version is not matched\n");
> > + return ret;
> > + }
> > +
> > + pr_info("samsung smc svc is initialized\n");
>
> No useless success printks. We have tracing and sysfs for that.
>
Ok,
> > + return 0;
> > +}
> > +
> > +early_initcall(samsung_smc_svc_init);
>
> No. Make it a platform driver.
>
Ok, I will change,
> > diff --git a/include/linux/firmware/samsung-smc-svc.h b/include/linux/firmware/samsung-smc-svc.h
> > new file mode 100644
> > index 000000000000..9c94fd3e10a2
> > --- /dev/null
> > +++ b/include/linux/firmware/samsung-smc-svc.h
> > @@ -0,0 +1,59 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#ifndef __SAMSUNG_SMC_H
> > +#define __SAMSUNG_SMC_H
>
> Extend the header gaurd to match path.
>
Yes,
> > +
> > +#include <linux/arm-smccc.h>
> > +
> > +#ifdef CONFIG_SAMSUNG_SECURE_SERVICE
>
> No ifdefs in headers. This is not a subsystem.
>
Yes, I will remove,
> > +
> > +#define SAMSUNG_SIP_CALL_VAL(x) \
> > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> > + ARM_SMCCC_SMC_32, ARM_SMCCC_OWNER_SIP, x)
>
> Need kerneldoc.
>
Yes, I will add kerneldoc.
> > +
> > +/* Common SIP SVC number */
> > +#define SAMSUING_SIP_SVC_CALL_COUNT SAMSUNG_SIP_CALL_VAL(0xff00)
> > +#define SAMSUING_SIP_SVC_UID SAMSUNG_SIP_CALL_VAL(0xff01)
> > +#define SAMSUNG_SIP_SVC_VERSION SAMSUNG_SIP_CALL_VAL(0xff03)
> > +#define SAMSUNG_SIP_NOT_SUPPORTED 0x0
>
> Need kerneldoc.
>
>
> > +
> > +enum {
> > + /* SIP enumeration for V0.1 */
> > + SAMSUNG_SIP_READ_REG,
> > + SAMSUNG_SIP_WRITE_REG,
> > + /* Add here for later version */
> > + SAMSUNG_SIP_NR_SIP
> > +};
>
> Need kerneldoc.
>
>
> > +
> > +int samsung_smc_call(u32 cmd_idx,
> > + unsigned long a0, unsigned long a1,
> > + unsigned long a2, unsigned long a3,
> > + u32 *val);
> > +int samsung_smc_reg_read(void *addr, unsigned int reg, u32 *val);
> > +int samsung_smc_reg_write(void *addr, unsigned int reg, u32 val);
> > +
>
> Need kerneldoc in the driver.
>
>
> > +#else
> > +
>
> No stubs, no ifdefs. It's not a subsystem.
>
>
>
> Best regards,
> Krzysztof
On 26/07/2022 02:04, Dongjin Yang wrote:
>
> On 14/07/2022 04:28, Krzysztof Kozlowski wrote:
>> On 13/07/2022 06:55, Dongjin Yang wrote:
>>> Introduce a driver to provide calls into secure monitor mode.
>>> This driver is used for SoCs produced by Samsung Foundry to provide
>>> SMC call. This patch supports register read/write request to secure
>>> monitor. Also, SMC call request which uses shared memory to exchange
>>> the data between kernel and secure monitor.
>>>
>>> Signed-off-by: Dongjin Yang <[email protected]>
>>> ---
>>> MAINTAINERS | 2 +
>>> drivers/firmware/Kconfig | 11 +++
>>> drivers/firmware/Makefile | 1 +
>>> drivers/firmware/samsung-smc-svc.c | 154 +++++++++++++++++++++++++++++++
>>> include/linux/firmware/samsung-smc-svc.h | 59 ++++++++++++
>>> 5 files changed, 227 insertions(+)
>>> create mode 100644 drivers/firmware/samsung-smc-svc.c
>>> create mode 100644 include/linux/firmware/samsung-smc-svc.h
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 6763746c349f..d173043ffb46 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -1868,8 +1868,10 @@ F: arch/arm/boot/dts/artpec6*
>>> F: arch/arm/mach-artpec
>>> F: drivers/clk/axis
>>> F: drivers/crypto/axis
>>> +F: drivers/firmware/samsung-smc-svc.c
>>> F: drivers/mmc/host/usdhi6rol0.c
>>> F: drivers/pinctrl/pinctrl-artpec*
>>> +F: include/linux/firmware/samsung-smc-svc.h
>>
>> Same as for bindings - not related to Artpec platforms. Either add
>> dedicated entry or put it under Samsung SoC.
>>
>
> This patch is for Artpec8 SoC.
This does not answer my comment at all.
>
Best regards,
Krzysztof