The Amlogic Meson SoC Secure Monitor implements a call to retrieve an
unique SoC ID starting from the GX Family and all new families.
But GX-family chips (e.g. GXB, GXL and newer) supports also 128-bit
chip ID. 128-bit chip ID consists 32-bit SoC version and 96-bit OTP data.
This is the second attempt to publish data from the Amlogic secure monitor
chipid call. After discussions with Neil Armstrong, it was decided to
publish the chipid call results through the soc driver. Since
soc_device_match cannot wait for the soc driver to load, and the secure
monitor calls in turn depend on the sm driver, it was necessary to create
a new driver rather than expand an existing one.
In the patches, in addition to writing the driver:
- convert commonly used structures and functions of the meson-gx-socinfo
driver to a header file.
- transfer the chipid sm call constants to a header file (perhaps they
need renaming?).
- add secure-monitor references for amlogic,meson-gx-ao-secure sections
in dts files of the a1, axg, g12, gx families.
Viacheslav Bocharov (5):
soc: amlogic: meson-gx-socinfo: move common code to header file
soc: amlogic: meson-gx-socinfo: move common code to exported function
firmware: meson_sm: move common definitions to header file
soc: amlogic: Add Amlogic secure-monitor SoC Information driver
arm64: dts: meson: add dts links to secure-monitor for soc driver in
a1, axg, gx, g12
arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 1 +
arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 1 +
.../boot/dts/amlogic/meson-g12-common.dtsi | 1 +
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 1 +
drivers/firmware/meson/meson_sm.c | 4 -
drivers/soc/amlogic/Kconfig | 10 +
drivers/soc/amlogic/Makefile | 1 +
.../soc/amlogic/meson-gx-socinfo-internal.h | 102 ++++++++++
drivers/soc/amlogic/meson-gx-socinfo-sm.c | 178 ++++++++++++++++++
drivers/soc/amlogic/meson-gx-socinfo.c | 106 ++---------
include/linux/firmware/meson/meson_sm.h | 4 +
11 files changed, 317 insertions(+), 92 deletions(-)
create mode 100644 drivers/soc/amlogic/meson-gx-socinfo-internal.h
create mode 100644 drivers/soc/amlogic/meson-gx-socinfo-sm.c
--
2.34.1
Move common constants and inline functions from meson-gx-socinfo driver
to header file
Signed-off-by: Viacheslav Bocharov <[email protected]>
---
.../soc/amlogic/meson-gx-socinfo-internal.h | 99 +++++++++++++++++++
drivers/soc/amlogic/meson-gx-socinfo.c | 80 +--------------
2 files changed, 100 insertions(+), 79 deletions(-)
create mode 100644 drivers/soc/amlogic/meson-gx-socinfo-internal.h
diff --git a/drivers/soc/amlogic/meson-gx-socinfo-internal.h b/drivers/soc/amlogic/meson-gx-socinfo-internal.h
new file mode 100644
index 000000000000..884cf8fb580f
--- /dev/null
+++ b/drivers/soc/amlogic/meson-gx-socinfo-internal.h
@@ -0,0 +1,99 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2017 BayLibre, SAS
+ * Copyright (C) 2023 JetHome
+ *
+ * Author: Neil Armstrong <[email protected]>
+ * Author: Viacheslav Bocharov <[email protected]>
+ */
+
+#ifndef _MESON_GX_SOCINFO_INTERNAL_H_
+#define _MESON_GX_SOCINFO_INTERNAL_H_
+
+#include <linux/bitfield.h>
+
+#define AO_SEC_SD_CFG8 0xe0
+#define AO_SEC_SOCINFO_OFFSET AO_SEC_SD_CFG8
+
+#define SOCINFO_MAJOR GENMASK(31, 24)
+#define SOCINFO_PACK GENMASK(23, 16)
+#define SOCINFO_MINOR GENMASK(15, 8)
+#define SOCINFO_MISC GENMASK(7, 0)
+
+static const struct meson_gx_soc_id {
+ const char *name;
+ unsigned int id;
+} soc_ids[] = {
+ { "GXBB", 0x1f },
+ { "GXTVBB", 0x20 },
+ { "GXL", 0x21 },
+ { "GXM", 0x22 },
+ { "TXL", 0x23 },
+ { "TXLX", 0x24 },
+ { "AXG", 0x25 },
+ { "GXLX", 0x26 },
+ { "TXHD", 0x27 },
+ { "G12A", 0x28 },
+ { "G12B", 0x29 },
+ { "SM1", 0x2b },
+ { "A1", 0x2c },
+};
+
+
+static const struct meson_gx_package_id {
+ const char *name;
+ unsigned int major_id;
+ unsigned int pack_id;
+ unsigned int pack_mask;
+} soc_packages[] = {
+ { "S905", 0x1f, 0, 0x20 }, /* pack_id != 0x20 */
+ { "S905H", 0x1f, 0x3, 0xf }, /* pack_id & 0xf == 0x3 */
+ { "S905M", 0x1f, 0x20, 0xf0 }, /* pack_id == 0x20 */
+ { "S905D", 0x21, 0, 0xf0 },
+ { "S905X", 0x21, 0x80, 0xf0 },
+ { "S905W", 0x21, 0xa0, 0xf0 },
+ { "S905L", 0x21, 0xc0, 0xf0 },
+ { "S905M2", 0x21, 0xe0, 0xf0 },
+ { "S805X", 0x21, 0x30, 0xf0 },
+ { "S805Y", 0x21, 0xb0, 0xf0 },
+ { "S912", 0x22, 0, 0x0 }, /* Only S912 is known for GXM */
+ { "962X", 0x24, 0x10, 0xf0 },
+ { "962E", 0x24, 0x20, 0xf0 },
+ { "A113X", 0x25, 0x37, 0xff },
+ { "A113X", 0x25, 0x43, 0xff },
+ { "A113D", 0x25, 0x22, 0xff },
+ { "S905D2", 0x28, 0x10, 0xf0 },
+ { "S905Y2", 0x28, 0x30, 0xf0 },
+ { "S905X2", 0x28, 0x40, 0xf0 },
+ { "A311D", 0x29, 0x10, 0xf0 },
+ { "S922X", 0x29, 0x40, 0xf0 },
+ { "S905D3", 0x2b, 0x4, 0xf5 },
+ { "S905X3", 0x2b, 0x5, 0xf5 },
+ { "S905X3", 0x2b, 0x10, 0x3f },
+ { "S905D3", 0x2b, 0x30, 0x3f },
+ { "A113L", 0x2c, 0x0, 0xf8 },
+};
+
+
+static inline unsigned int socinfo_to_major(u32 socinfo)
+{
+ return FIELD_GET(SOCINFO_MAJOR, socinfo);
+}
+
+static inline unsigned int socinfo_to_minor(u32 socinfo)
+{
+ return FIELD_GET(SOCINFO_MINOR, socinfo);
+}
+
+static inline unsigned int socinfo_to_pack(u32 socinfo)
+{
+ return FIELD_GET(SOCINFO_PACK, socinfo);
+}
+
+static inline unsigned int socinfo_to_misc(u32 socinfo)
+{
+ return FIELD_GET(SOCINFO_MISC, socinfo);
+}
+
+#endif /* _MESON_GX_SOCINFO_INTERNAL_H_ */
+
diff --git a/drivers/soc/amlogic/meson-gx-socinfo.c b/drivers/soc/amlogic/meson-gx-socinfo.c
index 6abb730344ab..9d7921c0fb91 100644
--- a/drivers/soc/amlogic/meson-gx-socinfo.c
+++ b/drivers/soc/amlogic/meson-gx-socinfo.c
@@ -16,85 +16,7 @@
#include <linux/regmap.h>
#include <linux/mfd/syscon.h>
-#define AO_SEC_SD_CFG8 0xe0
-#define AO_SEC_SOCINFO_OFFSET AO_SEC_SD_CFG8
-
-#define SOCINFO_MAJOR GENMASK(31, 24)
-#define SOCINFO_PACK GENMASK(23, 16)
-#define SOCINFO_MINOR GENMASK(15, 8)
-#define SOCINFO_MISC GENMASK(7, 0)
-
-static const struct meson_gx_soc_id {
- const char *name;
- unsigned int id;
-} soc_ids[] = {
- { "GXBB", 0x1f },
- { "GXTVBB", 0x20 },
- { "GXL", 0x21 },
- { "GXM", 0x22 },
- { "TXL", 0x23 },
- { "TXLX", 0x24 },
- { "AXG", 0x25 },
- { "GXLX", 0x26 },
- { "TXHD", 0x27 },
- { "G12A", 0x28 },
- { "G12B", 0x29 },
- { "SM1", 0x2b },
- { "A1", 0x2c },
-};
-
-static const struct meson_gx_package_id {
- const char *name;
- unsigned int major_id;
- unsigned int pack_id;
- unsigned int pack_mask;
-} soc_packages[] = {
- { "S905", 0x1f, 0, 0x20 }, /* pack_id != 0x20 */
- { "S905H", 0x1f, 0x3, 0xf }, /* pack_id & 0xf == 0x3 */
- { "S905M", 0x1f, 0x20, 0xf0 }, /* pack_id == 0x20 */
- { "S905D", 0x21, 0, 0xf0 },
- { "S905X", 0x21, 0x80, 0xf0 },
- { "S905W", 0x21, 0xa0, 0xf0 },
- { "S905L", 0x21, 0xc0, 0xf0 },
- { "S905M2", 0x21, 0xe0, 0xf0 },
- { "S805X", 0x21, 0x30, 0xf0 },
- { "S805Y", 0x21, 0xb0, 0xf0 },
- { "S912", 0x22, 0, 0x0 }, /* Only S912 is known for GXM */
- { "962X", 0x24, 0x10, 0xf0 },
- { "962E", 0x24, 0x20, 0xf0 },
- { "A113X", 0x25, 0x37, 0xff },
- { "A113D", 0x25, 0x22, 0xff },
- { "S905D2", 0x28, 0x10, 0xf0 },
- { "S905Y2", 0x28, 0x30, 0xf0 },
- { "S905X2", 0x28, 0x40, 0xf0 },
- { "A311D", 0x29, 0x10, 0xf0 },
- { "S922X", 0x29, 0x40, 0xf0 },
- { "S905D3", 0x2b, 0x4, 0xf5 },
- { "S905X3", 0x2b, 0x5, 0xf5 },
- { "S905X3", 0x2b, 0x10, 0x3f },
- { "S905D3", 0x2b, 0x30, 0x3f },
- { "A113L", 0x2c, 0x0, 0xf8 },
-};
-
-static inline unsigned int socinfo_to_major(u32 socinfo)
-{
- return FIELD_GET(SOCINFO_MAJOR, socinfo);
-}
-
-static inline unsigned int socinfo_to_minor(u32 socinfo)
-{
- return FIELD_GET(SOCINFO_MINOR, socinfo);
-}
-
-static inline unsigned int socinfo_to_pack(u32 socinfo)
-{
- return FIELD_GET(SOCINFO_PACK, socinfo);
-}
-
-static inline unsigned int socinfo_to_misc(u32 socinfo)
-{
- return FIELD_GET(SOCINFO_MISC, socinfo);
-}
+#include "meson-gx-socinfo-internal.h"
static const char *socinfo_to_package_id(u32 socinfo)
{
--
2.34.1
Move SM_CHIPID_* constants from firmware meson sm driver to
header file.
Signed-off-by: Viacheslav Bocharov <[email protected]>
---
drivers/firmware/meson/meson_sm.c | 4 ----
include/linux/firmware/meson/meson_sm.h | 4 ++++
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
index 402851ed4ac0..96e50811336f 100644
--- a/drivers/firmware/meson/meson_sm.c
+++ b/drivers/firmware/meson/meson_sm.c
@@ -240,10 +240,6 @@ struct meson_sm_firmware *meson_sm_get(struct device_node *sm_node)
}
EXPORT_SYMBOL_GPL(meson_sm_get);
-#define SM_CHIP_ID_LENGTH 119
-#define SM_CHIP_ID_OFFSET 4
-#define SM_CHIP_ID_SIZE 12
-
static ssize_t serial_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
diff --git a/include/linux/firmware/meson/meson_sm.h b/include/linux/firmware/meson/meson_sm.h
index 8eaf8922ab02..f62acd2bf52a 100644
--- a/include/linux/firmware/meson/meson_sm.h
+++ b/include/linux/firmware/meson/meson_sm.h
@@ -7,6 +7,10 @@
#ifndef _MESON_SM_FW_H_
#define _MESON_SM_FW_H_
+#define SM_CHIP_ID_LENGTH 119
+#define SM_CHIP_ID_OFFSET 4
+#define SM_CHIP_ID_SIZE 12
+
enum {
SM_EFUSE_READ,
SM_EFUSE_WRITE,
--
2.34.1
Add links to secure-monitor in soc driver section for A1, AXG, GX, G12
Amlogic family.
Signed-off-by: Viacheslav Bocharov <[email protected]>
---
arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 1 +
arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 1 +
arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi | 1 +
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 1 +
4 files changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index 648e7f49424f..449b328d62b1 100644
--- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
@@ -407,6 +407,7 @@ hwrng: rng@5118 {
sec_AO: ao-secure@5a20 {
compatible = "amlogic,meson-gx-ao-secure", "syscon";
reg = <0x0 0x5a20 0x0 0x140>;
+ secure-monitor = <&sm>;
amlogic,has-chip-id;
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index a49aa62e3f9f..6e80bdc525e5 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -1660,6 +1660,7 @@ mux {
sec_AO: ao-secure@140 {
compatible = "amlogic,meson-gx-ao-secure", "syscon";
reg = <0x0 0x140 0x0 0x140>;
+ secure-monitor = <&sm>;
amlogic,has-chip-id;
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
index ff68b911b729..0a6b703b0dc0 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
@@ -2026,6 +2026,7 @@ cec_AO: cec@100 {
sec_AO: ao-secure@140 {
compatible = "amlogic,meson-gx-ao-secure", "syscon";
reg = <0x0 0x140 0x0 0x140>;
+ secure-monitor = <&sm>;
amlogic,has-chip-id;
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index 2673f0dbafe7..656e08b3d872 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -471,6 +471,7 @@ cec_AO: cec@100 {
sec_AO: ao-secure@140 {
compatible = "amlogic,meson-gx-ao-secure", "syscon";
reg = <0x0 0x140 0x0 0x140>;
+ secure-monitor = <&sm>;
amlogic,has-chip-id;
};
--
2.34.1
Hello Viacheslav,
On Wed, Nov 22, 2023 at 03:56:43PM +0300, Viacheslav Bocharov wrote:
> Add links to secure-monitor in soc driver section for A1, AXG, GX, G12
> Amlogic family.
>
> Signed-off-by: Viacheslav Bocharov <[email protected]>
> ---
> arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 1 +
> arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 1 +
> arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi | 1 +
> arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 1 +
> 4 files changed, 4 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> index 648e7f49424f..449b328d62b1 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> @@ -407,6 +407,7 @@ hwrng: rng@5118 {
> sec_AO: ao-secure@5a20 {
> compatible = "amlogic,meson-gx-ao-secure", "syscon";
> reg = <0x0 0x5a20 0x0 0x140>;
> + secure-monitor = <&sm>;
I suppose it's better and more consistent to use the same hierarchy
pattern as Secure PWRC uses: to be a child of Secure Monitor.
Please see the example below from meson-a1.dtsi:
sm: secure-monitor {
compatible = "amlogic,meson-gxbb-sm";
pwrc: power-controller {
compatible = "amlogic,meson-a1-pwrc";
#power-domain-cells = <1>;
};
};
> amlogic,has-chip-id;
> };
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> index a49aa62e3f9f..6e80bdc525e5 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
> @@ -1660,6 +1660,7 @@ mux {
> sec_AO: ao-secure@140 {
> compatible = "amlogic,meson-gx-ao-secure", "syscon";
> reg = <0x0 0x140 0x0 0x140>;
> + secure-monitor = <&sm>;
> amlogic,has-chip-id;
> };
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
> index ff68b911b729..0a6b703b0dc0 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
> @@ -2026,6 +2026,7 @@ cec_AO: cec@100 {
> sec_AO: ao-secure@140 {
> compatible = "amlogic,meson-gx-ao-secure", "syscon";
> reg = <0x0 0x140 0x0 0x140>;
> + secure-monitor = <&sm>;
> amlogic,has-chip-id;
> };
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> index 2673f0dbafe7..656e08b3d872 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> @@ -471,6 +471,7 @@ cec_AO: cec@100 {
> sec_AO: ao-secure@140 {
> compatible = "amlogic,meson-gx-ao-secure", "syscon";
> reg = <0x0 0x140 0x0 0x140>;
> + secure-monitor = <&sm>;
> amlogic,has-chip-id;
> };
>
> --
> 2.34.1
>
>
> _______________________________________________
> linux-amlogic mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
--
Thank you,
Dmitry
Hello Neil,
May I put in my two cents on this patch series?
There appears to be a misunderstanding regarding the terminology used.
Allow me to clarify my perspective.
The original Amlogic chipid has the following format:
4 bytes 12 bytes
+-------+-------------------+
| | |
| CPUID | SOC SERIAL NUMBER |
| | |
+-------+-------------------+
0 15
In the current uboot [1] and kernel [2] upstream, only the SOC SERIAL
NUMBER bytes are read from efuse OTP storage (and it isn't swapped, as
Amlogic reference code does [3]). We refer to these bytes as "serial".
The original chipid value is utilized in several algorithms (for
example, in the Amlogic boot protocols), making it crucial to have the
ability to read the original chipid value as intended by the vendor.
In my opinion, we should align our naming terminology as follows:
- "chipid" - Represents the complete Amlogic SoC ID, includes
"cpuid" and "serial"
- "serial" - 12 byte unique SoC number, identifying particular die
We strongly believe that this patch series is essential and are highly interested in seeing it applied to the upstream linux-amlogic, for the following reasons:
- We use chipid for device identification in our boards
- The Amlogic boot protocols utilize the full version of chipid
(ADNL, Optimus). As I mentioned in the IRC, we are preparing a
patch series for uboot incorporating them.
- in OPTEE: for generation of SSK (Secure Storage Key) [4]
- RPMB: for generation of RPMB key, further provisioned into RPMB
controller (thus particular SoC is bound to particular eMMC
Therefore, I propose that we rename "soc_id" in the Viacheslav patch
series to "chipid" and subsequently port this patch series to uboot.
What are your thoughts on this matter?
Links:
[1] - https://elixir.bootlin.com/u-boot/v2024.01/source/arch/arm/mach-meson/sm.c#L84
[2] - https://elixir.bootlin.com/linux/v6.7.4/source/drivers/firmware/meson/meson_sm.c#L268
[3] - https://github.com/CoreELEC/u-boot/blob/3efc85a8370796bcec3bcadcdecec9aed973f4a9/arch/arm/mach-meson/g12a/bl31_apis.c#L398-L417
[4] - https://github.com/OP-TEE/optee_os/blob/5df2a985b2ffd0b6f1107f12ca2a88203bf31328/core/tee/tee_fs_key_manager.c#L152
On Wed, Nov 22, 2023 at 03:56:38PM +0300, Viacheslav Bocharov wrote:
> The Amlogic Meson SoC Secure Monitor implements a call to retrieve an
> unique SoC ID starting from the GX Family and all new families.
> But GX-family chips (e.g. GXB, GXL and newer) supports also 128-bit
> chip ID. 128-bit chip ID consists 32-bit SoC version and 96-bit OTP data.
>
> This is the second attempt to publish data from the Amlogic secure monitor
> chipid call. After discussions with Neil Armstrong, it was decided to
> publish the chipid call results through the soc driver. Since
> soc_device_match cannot wait for the soc driver to load, and the secure
> monitor calls in turn depend on the sm driver, it was necessary to create
> a new driver rather than expand an existing one.
>
> In the patches, in addition to writing the driver:
> - convert commonly used structures and functions of the meson-gx-socinfo
> driver to a header file.
> - transfer the chipid sm call constants to a header file (perhaps they
> need renaming?).
> - add secure-monitor references for amlogic,meson-gx-ao-secure sections
> in dts files of the a1, axg, g12, gx families.
>
> Viacheslav Bocharov (5):
> soc: amlogic: meson-gx-socinfo: move common code to header file
> soc: amlogic: meson-gx-socinfo: move common code to exported function
> firmware: meson_sm: move common definitions to header file
> soc: amlogic: Add Amlogic secure-monitor SoC Information driver
> arm64: dts: meson: add dts links to secure-monitor for soc driver in
> a1, axg, gx, g12
>
> arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 1 +
> arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 1 +
> .../boot/dts/amlogic/meson-g12-common.dtsi | 1 +
> arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 1 +
> drivers/firmware/meson/meson_sm.c | 4 -
> drivers/soc/amlogic/Kconfig | 10 +
> drivers/soc/amlogic/Makefile | 1 +
> .../soc/amlogic/meson-gx-socinfo-internal.h | 102 ++++++++++
> drivers/soc/amlogic/meson-gx-socinfo-sm.c | 178 ++++++++++++++++++
> drivers/soc/amlogic/meson-gx-socinfo.c | 106 ++---------
> include/linux/firmware/meson/meson_sm.h | 4 +
> 11 files changed, 317 insertions(+), 92 deletions(-)
> create mode 100644 drivers/soc/amlogic/meson-gx-socinfo-internal.h
> create mode 100644 drivers/soc/amlogic/meson-gx-socinfo-sm.c
>
> --
> 2.34.1
>
>
> _______________________________________________
> linux-amlogic mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
--
Thank you,
Dmitry
On Wed, Nov 22, 2023 at 03:56:41PM +0300, Viacheslav Bocharov wrote:
> Move SM_CHIPID_* constants from firmware meson sm driver to
> header file.
>
> Signed-off-by: Viacheslav Bocharov <[email protected]>
> ---
> drivers/firmware/meson/meson_sm.c | 4 ----
> include/linux/firmware/meson/meson_sm.h | 4 ++++
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
> index 402851ed4ac0..96e50811336f 100644
> --- a/drivers/firmware/meson/meson_sm.c
> +++ b/drivers/firmware/meson/meson_sm.c
> @@ -240,10 +240,6 @@ struct meson_sm_firmware *meson_sm_get(struct device_node *sm_node)
> }
> EXPORT_SYMBOL_GPL(meson_sm_get);
>
> -#define SM_CHIP_ID_LENGTH 119
> -#define SM_CHIP_ID_OFFSET 4
> -#define SM_CHIP_ID_SIZE 12
> -
> static ssize_t serial_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> diff --git a/include/linux/firmware/meson/meson_sm.h b/include/linux/firmware/meson/meson_sm.h
> index 8eaf8922ab02..f62acd2bf52a 100644
> --- a/include/linux/firmware/meson/meson_sm.h
> +++ b/include/linux/firmware/meson/meson_sm.h
> @@ -7,6 +7,10 @@
> #ifndef _MESON_SM_FW_H_
> #define _MESON_SM_FW_H_
>
> +#define SM_CHIP_ID_LENGTH 119
Does anybody know why this value is 119 bytes?
if in-shmem data, arrived from BL31, contains only up to 20 bytes
(in case of chipID v2):
+-----------+---------+---------------------------------+
| chipID ver| cpu_id | 12b-serial |
| 4 bytes | 4 bytes |(per particular die unique data) |
+-----------+---------+---------------------------------+
> +#define SM_CHIP_ID_OFFSET 4
> +#define SM_CHIP_ID_SIZE 12
It seems that either the value (12) is incorrect or the current naming
is misleading. This is because the chipID is 16 bytes. Of course,
likely the SoC serial was meant here...
Furthermore, it appears that SM_CHIP_ID_SIZE is an unused symbol. It
has been unused since its creation in
0789724f86a5 ('firmware: meson_sm: Add serial number sysfs entry')
> +
> enum {
> SM_EFUSE_READ,
> SM_EFUSE_WRITE,
> --
> 2.34.1
>
>
--
Best Regards,
Evgeny Bachinin
On 16/02/2024 08:47, Dmitry Rokosov wrote:
> Hello Neil,
>
> May I put in my two cents on this patch series?
>
> There appears to be a misunderstanding regarding the terminology used.
> Allow me to clarify my perspective.
>
> The original Amlogic chipid has the following format:
>
> 4 bytes 12 bytes
> +-------+-------------------+
> | | |
> | CPUID | SOC SERIAL NUMBER |
> | | |
> +-------+-------------------+
> 0 15
>
>
> In the current uboot [1] and kernel [2] upstream, only the SOC SERIAL
> NUMBER bytes are read from efuse OTP storage (and it isn't swapped, as
> Amlogic reference code does [3]). We refer to these bytes as "serial".
>
> The original chipid value is utilized in several algorithms (for
> example, in the Amlogic boot protocols), making it crucial to have the
> ability to read the original chipid value as intended by the vendor.
>
> In my opinion, we should align our naming terminology as follows:
> - "chipid" - Represents the complete Amlogic SoC ID, includes
> "cpuid" and "serial"
> - "serial" - 12 byte unique SoC number, identifying particular die
>
> We strongly believe that this patch series is essential and are highly interested in seeing it applied to the upstream linux-amlogic, for the following reasons:
> - We use chipid for device identification in our boards
> - The Amlogic boot protocols utilize the full version of chipid
> (ADNL, Optimus). As I mentioned in the IRC, we are preparing a
> patch series for uboot incorporating them.
> - in OPTEE: for generation of SSK (Secure Storage Key) [4]
> - RPMB: for generation of RPMB key, further provisioned into RPMB
> controller (thus particular SoC is bound to particular eMMC
>
> Therefore, I propose that we rename "soc_id" in the Viacheslav patch
> series to "chipid" and subsequently port this patch series to uboot.
>
> What are your thoughts on this matter?
I'm perfectly fine with that, but I don't like the shared functions, the only
shared stuff are the soc id tables, the shared functions is not important enough
to be shared.
Neil
>
> Links:
> [1] - https://elixir.bootlin.com/u-boot/v2024.01/source/arch/arm/mach-meson/sm.c#L84
> [2] - https://elixir.bootlin.com/linux/v6.7.4/source/drivers/firmware/meson/meson_sm.c#L268
> [3] - https://github.com/CoreELEC/u-boot/blob/3efc85a8370796bcec3bcadcdecec9aed973f4a9/arch/arm/mach-meson/g12a/bl31_apis.c#L398-L417
> [4] - https://github.com/OP-TEE/optee_os/blob/5df2a985b2ffd0b6f1107f12ca2a88203bf31328/core/tee/tee_fs_key_manager.c#L152
>
> On Wed, Nov 22, 2023 at 03:56:38PM +0300, Viacheslav Bocharov wrote:
>> The Amlogic Meson SoC Secure Monitor implements a call to retrieve an
>> unique SoC ID starting from the GX Family and all new families.
>> But GX-family chips (e.g. GXB, GXL and newer) supports also 128-bit
>> chip ID. 128-bit chip ID consists 32-bit SoC version and 96-bit OTP data.
>>
>> This is the second attempt to publish data from the Amlogic secure monitor
>> chipid call. After discussions with Neil Armstrong, it was decided to
>> publish the chipid call results through the soc driver. Since
>> soc_device_match cannot wait for the soc driver to load, and the secure
>> monitor calls in turn depend on the sm driver, it was necessary to create
>> a new driver rather than expand an existing one.
>>
>> In the patches, in addition to writing the driver:
>> - convert commonly used structures and functions of the meson-gx-socinfo
>> driver to a header file.
>> - transfer the chipid sm call constants to a header file (perhaps they
>> need renaming?).
>> - add secure-monitor references for amlogic,meson-gx-ao-secure sections
>> in dts files of the a1, axg, g12, gx families.
>>
>> Viacheslav Bocharov (5):
>> soc: amlogic: meson-gx-socinfo: move common code to header file
>> soc: amlogic: meson-gx-socinfo: move common code to exported function
>> firmware: meson_sm: move common definitions to header file
>> soc: amlogic: Add Amlogic secure-monitor SoC Information driver
>> arm64: dts: meson: add dts links to secure-monitor for soc driver in
>> a1, axg, gx, g12
>>
>> arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 1 +
>> arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 1 +
>> .../boot/dts/amlogic/meson-g12-common.dtsi | 1 +
>> arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 1 +
>> drivers/firmware/meson/meson_sm.c | 4 -
>> drivers/soc/amlogic/Kconfig | 10 +
>> drivers/soc/amlogic/Makefile | 1 +
>> .../soc/amlogic/meson-gx-socinfo-internal.h | 102 ++++++++++
>> drivers/soc/amlogic/meson-gx-socinfo-sm.c | 178 ++++++++++++++++++
>> drivers/soc/amlogic/meson-gx-socinfo.c | 106 ++---------
>> include/linux/firmware/meson/meson_sm.h | 4 +
>> 11 files changed, 317 insertions(+), 92 deletions(-)
>> create mode 100644 drivers/soc/amlogic/meson-gx-socinfo-internal.h
>> create mode 100644 drivers/soc/amlogic/meson-gx-socinfo-sm.c
>>
>> --
>> 2.34.1
>>
>>
>> _______________________________________________
>> linux-amlogic mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
>