| Message ID | 20231122125643.1717160-1-adeep@lexina.in (mailing list archive) |
|---|---|
| Headers | show |
| Series | soc: amlogic: add new meson-gx-socinfo-sm driver | expand |
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
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
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 >> linux-amlogic@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-amlogic >
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