Re: [PATCH] riscv: Fix alignment of RELA sections in the linker scripts
Hi Rick, Bin,
On Tue, Jun 27, 2023 at 09:12:35AM +0800, Bin Meng wrote:
> On Tue, Jun 27, 2023 at 8:50 AM Rick Chen wrote:
> >
> > > From: Bin Meng
> > > Sent: Wednesday, June 21, 2023 11:07 PM
> > > To: u-boot@lists.denx.de
> > > Cc: Andrew Scull ; Leo Yu-Chi Liang(梁育齊)
> > > ; Rick Jian-Zhi Chen(陳建志) ;
> > > Simon Glass
> > > Subject: [PATCH] riscv: Fix alignment of RELA sections in the linker
> > > scripts
> > >
> > > In current linker script both .efi_runtime_rel and .rela.dyn sections are
> > > of RELA type whose entry size is either 12 (RV32) or 24 (RV64).
> > > These two are arranged as an continuous region on purpose so that the
> > > prelink-riscv executable can fix up the PIE addresses in one loop.
> > >
> > > However there is an 'ALIGN(8)' between these 2 sections which might cause
> > > a gap to be inserted between thesse 2 sections to satify the alignment
> > > requirement on RV32. This would break the assumption of the prelink
> > > process and generate an unbootable image.
> > >
> > > Fixes: 9a6569a043d3 ("riscv: Update alignment for some sections in linker
> > > scripts")
> > > Signed-off-by: Bin Meng
> > >
> > > ---
> > > This fix should go into the v2023.07 release.
> > >
> > > arch/riscv/cpu/u-boot.lds | 4 +---
> > > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > Reviewed-by: Rick Chen
> >
> > Hi Leo,
> >
> > Please help to push this patch ASAP.
> >
>
> Thanks Rick. I will respin a v2 to fix the typos in the commit message.
>
> Regards,
> Bin
Thanks for the catch and the reminder.
The patch has been merged.
Best regards,
Leo
[PATCH 08/10] cmd: add scmi command for SCMI firmware
This command, "scmi", provides a command line interface to various SCMI
protocols. It supports at least initially SCMI base protocol with the sub
command, "base", and is intended mainly for debug purpose.
Signed-off-by: AKASHI Takahiro
---
cmd/Kconfig | 8 ++
cmd/Makefile | 1 +
cmd/scmi.c | 373 +++
3 files changed, 382 insertions(+)
create mode 100644 cmd/scmi.c
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 02e54f1e50fe..065470a76295 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2504,6 +2504,14 @@ config CMD_CROS_EC
a number of sub-commands for performing EC tasks such as
updating its flash, accessing a small saved context area
and talking to the I2C bus behind the EC (if there is one).
+
+config CMD_SCMI
+ bool "Enable scmi command"
+ depends on SCMI_FIRMWARE
+ default n
+ help
+ This command provides user interfaces to several SCMI protocols,
+ including Base protocol.
endmenu
menu "Filesystem commands"
diff --git a/cmd/Makefile b/cmd/Makefile
index 6c37521b4e2b..826e0b74b587 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -156,6 +156,7 @@ obj-$(CONFIG_CMD_SATA) += sata.o
obj-$(CONFIG_CMD_NVME) += nvme.o
obj-$(CONFIG_SANDBOX) += sb.o
obj-$(CONFIG_CMD_SF) += sf.o
+obj-$(CONFIG_CMD_SCMI) += scmi.o
obj-$(CONFIG_CMD_SCSI) += scsi.o disk.o
obj-$(CONFIG_CMD_SHA1SUM) += sha1sum.o
obj-$(CONFIG_CMD_SEAMA) += seama.o
diff --git a/cmd/scmi.c b/cmd/scmi.c
new file mode 100644
index ..c97f77e97d95
--- /dev/null
+++ b/cmd/scmi.c
@@ -0,0 +1,373 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * SCMI utility command
+ *
+ * Copyright (c) 2023 Linaro Limited
+ * Author: AKASHI Takahiro
+ */
+
+#include
+#include
+#include
+#include /* uclass_get_device */
+#include
+#include
+#include
+#include
+#include
+#include
+
+static struct udevice *agent;
+static struct udevice *base_proto;
+static const struct scmi_base_ops *ops;
+
+struct {
+ enum scmi_std_protocol id;
+ const char *name;
+} protocol_name[] = {
+ {SCMI_PROTOCOL_ID_BASE, "Base"},
+ {SCMI_PROTOCOL_ID_POWER_DOMAIN, "Power domain management"},
+ {SCMI_PROTOCOL_ID_SYSTEM, "System power management"},
+ {SCMI_PROTOCOL_ID_PERF, "Performance domain management"},
+ {SCMI_PROTOCOL_ID_CLOCK, "Clock management"},
+ {SCMI_PROTOCOL_ID_SENSOR, "Sensor management"},
+ {SCMI_PROTOCOL_ID_RESET_DOMAIN, "Reset domain management"},
+ {SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, "Voltage domain management"},
+};
+
+/**
+ * scmi_convert_id_to_string() - get the name of SCMI protocol
+ *
+ * @id:Protocol ID
+ *
+ * Get the printable name of the protocol, @id
+ *
+ * Return: Name string on success, NULL on failure
+ */
+static const char *scmi_convert_id_to_string(enum scmi_std_protocol id)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(protocol_name); i++)
+ if (id == protocol_name[i].id)
+ return protocol_name[i].name;
+
+ return NULL;
+}
+
+/**
+ * do_scmi_base_info() - get the information of SCMI services
+ *
+ * @cmdtp: Command table
+ * @flag: Command flag
+ * @argc: Number of arguments
+ * @argv: Argument array
+ *
+ * Get the information of SCMI services using various interfaces
+ * provided by the Base protocol.
+ *
+ * Return: CMD_RET_SUCCESS on success, CMD_RET_RET_FAILURE on failure
+ */
+static int do_scmi_base_info(struct cmd_tbl *cmdtp, int flag, int argc,
+char * const argv[])
+{
+ u32 agent_id, num_protocols;
+ u8 agent_name[SCMI_BASE_NAME_LENGTH_MAX], *protocols;
+ int i, ret;
+
+ if (argc != 1)
+ return CMD_RET_USAGE;
+
+ printf("SCMI device: %s\n", agent->name);
+ printf(" protocol version: 0x%x\n", scmi_version(agent));
+ printf(" # of agents: %d\n", scmi_num_agents(agent));
+ for (i = 0; i < scmi_num_agents(agent); i++) {
+ ret = (*ops->base_discover_agent)(base_proto, i, &agent_id,
+ agent_name);
+ if (ret) {
+ if (ret != -EOPNOTSUPP)
+ printf("base_discover_agent() failed for id: %d
(%d)\n",
+ i, ret);
+ break;
+ }
+ printf("%c%2d: %s\n", i == scmi_agent_id(agent) ? '>' : ' ',
+ i, agent_name);
+ }
+ printf(" # of protocols: %d\n", scmi_num_protocols(agent));
+ num_protocols = scmi_num_protocols(agent);
+ protocols = scmi_protocols(agent);
+ if (protocols)
+ for (i = 0; i < num_protocols; i++)
+ printf(" %s\n",
+ scmi_convert_id_to_string(protocols[i]));
+ printf(" vendor: %s\n", scmi_vendor(agent));
+ printf("
[PATCH 09/10] doc: cmd: add documentation for scmi
This is a help text for scmi command. Signed-off-by: AKASHI Takahiro --- doc/usage/cmd/scmi.rst | 98 ++ 1 file changed, 98 insertions(+) create mode 100644 doc/usage/cmd/scmi.rst diff --git a/doc/usage/cmd/scmi.rst b/doc/usage/cmd/scmi.rst new file mode 100644 index ..20cdae4b877d --- /dev/null +++ b/doc/usage/cmd/scmi.rst @@ -0,0 +1,98 @@ +.. SPDX-License-Identifier: GPL-2.0+: + +scmi command + + +Synopsis + + +:: + +scmi base info +scmi base perm_dev +scmi base perm_proto +scmi base reset + +Description +--- + +The scmi command is used to access and operate on SCMI server. + +scmi base info +~~ +Show base information about SCMI server and supported protocols + +scmi base perm_dev +~~ +Allow or deny access permission to the device + +scmi base perm_proto + +Allow or deny access to the protocol on the device + +scmi base reset +~~~ +Reset the existing configurations + +Parameters are used as follows: + + +Agent ID + + +Device ID + + +Protocol ID, should not be 0x10 (base protocol) + + +Flags to control the action. See SCMI specification for +defined values. + +Example +--- + +Obtain basic information about SCMI server: + +:: + +=> scmi base info +SCMI device: scmi + protocol version: 0x2 + # of agents: 3 + 0: platform +> 1: OSPM + 2: PSCI + # of protocols: 4 + Power domain management + Performance domain management + Clock management + Sensor management + vendor: Linaro + sub vendor: PMWG + impl version: 0x20b + +Ask for access permission to device#0: + +:: + +=> scmi base perm_dev 1 0 1 + +Reset configurations with all access permission settings retained: + +:: + +=> scmi base reset 1 0 + +Configuration +- + +The scmi command is only available if CONFIG_CMD_SCMI=y. +Default n because this command is mainly for debug purpose. + +Return value + + +The return value ($?) is set to 0 if the operation succeeded, +1 if the operation failed or -1 if the operation failed due to +a syntax error. -- 2.41.0
[PATCH 10/10] test: dm: add scmi command test
In this test, "scmi" with different sub-commands is tested.
Please note that scmi command is for debug purpose and is not
intended in production system.
Signed-off-by: AKASHI Takahiro
---
test/dm/scmi.c | 57 ++
1 file changed, 57 insertions(+)
diff --git a/test/dm/scmi.c b/test/dm/scmi.c
index 563017bb63e0..b40c84011295 100644
--- a/test/dm/scmi.c
+++ b/test/dm/scmi.c
@@ -207,6 +207,63 @@ static int dm_test_scmi_base(struct unit_test_state *uts)
DM_TEST(dm_test_scmi_base, UT_TESTF_SCAN_FDT);
+static int dm_test_scmi_cmd(struct unit_test_state *uts)
+{
+ struct udevice *agent_dev, *base;
+ struct scmi_agent_priv *priv;
+
+ /* preparation */
+ ut_assertok(uclass_get_device_by_name(UCLASS_SCMI_AGENT, "scmi",
+ &agent_dev));
+ ut_assertnonnull(agent_dev);
+ ut_assertnonnull(priv = dev_get_uclass_plat(agent_dev));
+ ut_assertnonnull(base = scmi_get_protocol(agent_dev,
+ SCMI_PROTOCOL_ID_BASE));
+
+ /* scmi base info */
+ ut_assertok(run_command("scmi base info", 0));
+
+ ut_assert_nextline("SCMI device: scmi");
+ ut_assert_nextline(" protocol version: 0x2");
+ ut_assert_nextline(" # of agents: 2");
+ ut_assert_nextline(" 0: platform");
+ ut_assert_nextline("> 1: OSPM");
+ ut_assert_nextline(" # of protocols: 3");
+ ut_assert_nextline(" Clock management");
+ ut_assert_nextline(" Reset domain management");
+ ut_assert_nextline(" Voltage domain management");
+ ut_assert_nextline(" vendor: U-Boot");
+ ut_assert_nextline(" sub vendor: Sandbox");
+ ut_assert_nextline(" impl version: 0x1");
+
+ ut_assert_console_end();
+
+ /* scmi base perm_dev */
+ ut_assertok(run_command("scmi base perm_dev 1 0 1", 0));
+ ut_assert_console_end();
+
+ ut_assert(run_command("scmi base perm_dev 1 0 0", 0));
+ ut_assert_nextline("Denying access to device:0 failed (-13)");
+ ut_assert_console_end();
+
+ /* scmi base perm_proto */
+ ut_assertok(run_command("scmi base perm_proto 1 0 14 1", 0));
+ ut_assert_console_end();
+
+ ut_assert(run_command("scmi base perm_proto 1 0 14 0", 0));
+ ut_assert_nextline("Denying access to protocol:0x14 on device:0 failed
(-13)");
+ ut_assert_console_end();
+
+ /* scmi base reset */
+ ut_assert(run_command("scmi base reset 1 1", 0));
+ ut_assert_nextline("Reset failed (-13)");
+ ut_assert_console_end();
+
+ return 0;
+}
+
+DM_TEST(dm_test_scmi_cmd, UT_TESTF_SCAN_FDT);
+
static int dm_test_scmi_clocks(struct unit_test_state *uts)
{
struct sandbox_scmi_agent *agent;
--
2.41.0
[PATCH 06/10] test: dm: simplify SCMI unit test on sandbox
Adding SCMI base protocol makes it inconvenient to hold the agent instance
(udevice) locally since the agent device will be re-created per each test.
Just remove it and simply the test flows.
The test scenario is not changed at all.
Signed-off-by: AKASHI Takahiro
---
arch/sandbox/include/asm/scmi_test.h | 7 ++-
drivers/firmware/scmi/sandbox-scmi_agent.c | 20 +--
drivers/firmware/scmi/scmi_agent-uclass.c | 14 -
test/dm/scmi.c | 64 +++---
4 files changed, 26 insertions(+), 79 deletions(-)
diff --git a/arch/sandbox/include/asm/scmi_test.h
b/arch/sandbox/include/asm/scmi_test.h
index c72ec1e1cb25..2718336a9a50 100644
--- a/arch/sandbox/include/asm/scmi_test.h
+++ b/arch/sandbox/include/asm/scmi_test.h
@@ -89,10 +89,11 @@ struct sandbox_scmi_devices {
#ifdef CONFIG_SCMI_FIRMWARE
/**
- * sandbox_scmi_service_ctx - Get the simulated SCMI services context
+ * sandbox_scmi_agent_ctx - Get the simulated SCMI agent context
+ * @dev: Reference to the test agent
* @return:Reference to backend simulated resources state
*/
-struct sandbox_scmi_service *sandbox_scmi_service_ctx(void);
+struct sandbox_scmi_agent *sandbox_scmi_agent_ctx(struct udevice *dev);
/**
* sandbox_scmi_devices_ctx - Get references to devices accessed through SCMI
@@ -101,7 +102,7 @@ struct sandbox_scmi_service *sandbox_scmi_service_ctx(void);
*/
struct sandbox_scmi_devices *sandbox_scmi_devices_ctx(struct udevice *dev);
#else
-static inline struct sandbox_scmi_service *sandbox_scmi_service_ctx(void)
+static struct sandbox_scmi_agent *sandbox_scmi_agent_ctx(struct udevice *dev)
{
return NULL;
}
diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c
b/drivers/firmware/scmi/sandbox-scmi_agent.c
index 1f0261ea5c94..ab8afb01de40 100644
--- a/drivers/firmware/scmi/sandbox-scmi_agent.c
+++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
@@ -66,11 +66,9 @@ static struct sandbox_scmi_voltd scmi_voltd[] = {
{ .id = 1, .voltage_uv = 180 },
};
-static struct sandbox_scmi_service sandbox_scmi_service_state;
-
-struct sandbox_scmi_service *sandbox_scmi_service_ctx(void)
+struct sandbox_scmi_agent *sandbox_scmi_agent_ctx(struct udevice *dev)
{
- return &sandbox_scmi_service_state;
+ return dev_get_priv(dev);
}
static void debug_print_agent_state(struct udevice *dev, char *str)
@@ -898,16 +896,8 @@ static int sandbox_scmi_test_process_msg(struct udevice
*dev,
static int sandbox_scmi_test_remove(struct udevice *dev)
{
- struct sandbox_scmi_agent *agent = dev_get_priv(dev);
-
- if (agent != sandbox_scmi_service_ctx()->agent)
- return -EINVAL;
-
debug_print_agent_state(dev, "removed");
- /* We only need to dereference the agent in the context */
- sandbox_scmi_service_ctx()->agent = NULL;
-
return 0;
}
@@ -915,9 +905,6 @@ static int sandbox_scmi_test_probe(struct udevice *dev)
{
struct sandbox_scmi_agent *agent = dev_get_priv(dev);
- if (sandbox_scmi_service_ctx()->agent)
- return -EINVAL;
-
*agent = (struct sandbox_scmi_agent){
.clk = scmi_clk,
.clk_count = ARRAY_SIZE(scmi_clk),
@@ -929,9 +916,6 @@ static int sandbox_scmi_test_probe(struct udevice *dev)
debug_print_agent_state(dev, "probed");
- /* Save reference for tests purpose */
- sandbox_scmi_service_ctx()->agent = agent;
-
return 0;
};
diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c
b/drivers/firmware/scmi/scmi_agent-uclass.c
index ec15580cc947..4526f75486ed 100644
--- a/drivers/firmware/scmi/scmi_agent-uclass.c
+++ b/drivers/firmware/scmi/scmi_agent-uclass.c
@@ -37,12 +37,6 @@ static const struct error_code scmi_linux_errmap[] = {
{ .scmi = SCMI_PROTOCOL_ERROR, .errno = -EPROTO, },
};
-/*
- * NOTE: The only one instance should exist according to
- * the current specification and device tree bindings.
- */
-struct udevice *scmi_agent;
-
struct udevice *scmi_get_protocol(struct udevice *dev,
enum scmi_std_protocol id)
{
@@ -150,11 +144,6 @@ static int scmi_bind_protocols(struct udevice *dev)
struct driver *drv;
struct udevice *proto;
- if (scmi_agent) {
- dev_err(dev, "SCMI agent already exists: %s\n", dev->name);
- return -EBUSY;
- }
-
drv = DM_DRIVER_GET(scmi_base_drv);
name = "scmi-base.0";
ret = device_bind(dev, drv, name, NULL, ofnode_null(), &proto);
@@ -222,9 +211,6 @@ static int scmi_bind_protocols(struct udevice *dev)
}
}
- if (!ret)
- scmi_agent = dev;
-
return ret;
}
diff --git a/test/dm/scmi.c b/test/dm/scmi.c
index d87e2731ce42..881be3171b7c 100644
--- a/test/dm/scmi.c
+++ b/test/dm/scmi.c
@@ -23,22 +23,11 @@
#include
#include
-static int ut_assert_scmi_state_preprobe(struct unit_test_state *uts)
-{
-
[PATCH 05/10] firmware: scmi: fake base protocol commands on sandbox
This is a simple implementation of SCMI base protocol for sandbox.
The main use is in SCMI unit test.
Signed-off-by: AKASHI Takahiro
---
drivers/firmware/scmi/sandbox-scmi_agent.c | 359 -
1 file changed, 358 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c
b/drivers/firmware/scmi/sandbox-scmi_agent.c
index 031882998dfa..1f0261ea5c94 100644
--- a/drivers/firmware/scmi/sandbox-scmi_agent.c
+++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
@@ -14,11 +14,14 @@
#include
#include
#include
+#include
+#include
/*
* The sandbox SCMI agent driver simulates to some extend a SCMI message
* processing. It simulates few of the SCMI services for some of the
* SCMI protocols embedded in U-Boot. Currently:
+ * - SCMI base protocol
* - SCMI clock protocol emulates an agent exposing 2 clocks
* - SCMI reset protocol emulates an agent exposing a reset controller
* - SCMI voltage domain protocol emulates an agent exposing 2 regulators
@@ -33,6 +36,21 @@
* various uclass devices, as clocks and reset controllers.
*/
+#define SANDBOX_SCMI_BASE_PROTOCOL_VERSION SCMI_BASE_PROTOCOL_VERSION
+#define SANDBOX_SCMI_VENDOR "U-Boot"
+#define SANDBOX_SCMI_SUB_VENDOR "Sandbox"
+#define SANDBOX_SCMI_IMPL_VERSION 0x1
+#define SANDBOX_SCMI_AGENT_NAME "OSPM"
+#define SANDBOX_SCMI_PLATFORM_NAME "platform"
+
+static u8 protocols[] = {
+ SCMI_PROTOCOL_ID_CLOCK,
+ SCMI_PROTOCOL_ID_RESET_DOMAIN,
+ SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN,
+};
+
+#define NUM_PROTOCOLS ARRAY_SIZE(protocols)
+
static struct sandbox_scmi_clk scmi_clk[] = {
{ .rate = 333 },
{ .rate = 200 },
@@ -114,6 +132,316 @@ static struct sandbox_scmi_voltd
*get_scmi_voltd_state(uint domain_id)
* Sandbox SCMI agent ops
*/
+/* Base Protocol */
+
+/**
+ * sandbox_scmi_base_protocol_version - implement SCMI_BASE_PROTOCOL_VERSION
+ * @udevice: SCMI device
+ * @msg: SCMI message
+ *
+ * Implement SCMI_BASE_PROTOCOL_VERSION command.
+ */
+static int sandbox_scmi_base_protocol_version(struct udevice *dev,
+ struct scmi_msg *msg)
+{
+ struct scmi_protocol_version_out *out = NULL;
+
+ if (!msg->out_msg || msg->out_msg_sz < sizeof(*out))
+ return -EINVAL;
+
+ out = (struct scmi_protocol_version_out *)msg->out_msg;
+ out->version = SANDBOX_SCMI_BASE_PROTOCOL_VERSION;
+ out->status = SCMI_SUCCESS;
+
+ return 0;
+}
+
+/**
+ * sandbox_scmi_base_protocol_attrs - implement SCMI_BASE_PROTOCOL_ATTRIBUTES
+ * @udevice: SCMI device
+ * @msg: SCMI message
+ *
+ * Implement SCMI_BASE_PROTOCOL_ATTRIBUTES command.
+ */
+static int sandbox_scmi_base_protocol_attrs(struct udevice *dev,
+ struct scmi_msg *msg)
+{
+ struct scmi_protocol_attrs_out *out = NULL;
+
+ if (!msg->out_msg || msg->out_msg_sz < sizeof(*out))
+ return -EINVAL;
+
+ out = (struct scmi_protocol_attrs_out *)msg->out_msg;
+ out->attributes = FIELD_PREP(0xff00, 2) | NUM_PROTOCOLS;
+ out->status = SCMI_SUCCESS;
+
+ return 0;
+}
+
+/**
+ * sandbox_scmi_base_message_attrs - implement
+ * SCMI_BASE_PROTOCOL_MESSAGE_ATTRIBUTES
+ * @udevice: SCMI device
+ * @msg: SCMI message
+ *
+ * Implement SCMI_BASE_PROTOCOL_MESSAGE_ATTRIBUTES command.
+ */
+static int sandbox_scmi_base_message_attrs(struct udevice *dev,
+ struct scmi_msg *msg)
+{
+ u32 message_id;
+ struct scmi_protocol_msg_attrs_out *out = NULL;
+
+ if (!msg->in_msg || msg->in_msg_sz < sizeof(message_id) ||
+ !msg->out_msg || msg->out_msg_sz < sizeof(*out))
+ return -EINVAL;
+
+ message_id = *(u32 *)msg->in_msg;
+ out = (struct scmi_protocol_msg_attrs_out *)msg->out_msg;
+
+ if (message_id >= SCMI_PROTOCOL_VERSION &&
+ message_id <= SCMI_BASE_RESET_AGENT_CONFIGURATION &&
+ message_id != SCMI_BASE_NOTIFY_ERRORS) {
+ out->attributes = 0;
+ out->status = SCMI_SUCCESS;
+ } else {
+ out->status = SCMI_NOT_FOUND;
+ }
+
+ return 0;
+}
+
+/**
+ * sandbox_scmi_base_discover_vendor - implement SCMI_BASE_DISCOVER_VENDOR
+ * @udevice: SCMI device
+ * @msg: SCMI message
+ *
+ * Implement SCMI_BASE_DISCOVER_VENDOR command
+ */
+static int sandbox_scmi_base_discover_vendor(struct udevice *dev,
+struct scmi_msg *msg)
+{
+ struct scmi_base_discover_vendor_out *out = NULL;
+
+ if (!msg->out_msg || msg->out_msg_sz < sizeof(*out))
+ return -EINVAL;
+
+ out = (struct scmi_base_discover_vendor_out *)msg->out_msg;
+ strcpy(out->vendor_identifier, SANDBOX_SCMI_VENDOR);
+ out->status = SCMI_SUCCESS;
+
+ return 0;
+}
+
+/**
+ * sandbox_scmi_base_discover_sub_vendor - implem
[PATCH 07/10] test: dm: add SCMI base protocol test
Added is a new unit test for SCMI base protocol, which will exercise all
the commands provided by the protocol, except SCMI_BASE_NOTIFY_ERRORS.
$ ut dm scmi_base
It is assumed that test.dtb is used as sandbox's device tree.
Signed-off-by: AKASHI Takahiro
---
test/dm/scmi.c | 112 +
1 file changed, 112 insertions(+)
diff --git a/test/dm/scmi.c b/test/dm/scmi.c
index 881be3171b7c..563017bb63e0 100644
--- a/test/dm/scmi.c
+++ b/test/dm/scmi.c
@@ -16,6 +16,9 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
#include
@@ -95,6 +98,115 @@ static int dm_test_scmi_sandbox_agent(struct
unit_test_state *uts)
}
DM_TEST(dm_test_scmi_sandbox_agent, UT_TESTF_SCAN_FDT);
+static int dm_test_scmi_base(struct unit_test_state *uts)
+{
+ struct udevice *agent_dev, *base;
+ struct scmi_agent_priv *priv;
+ const struct scmi_base_ops *ops;
+ u32 version, num_agents, num_protocols, impl_version;
+ u32 attributes, agent_id;
+ char vendor[SCMI_BASE_NAME_LENGTH_MAX],
+agent_name[SCMI_BASE_NAME_LENGTH_MAX];
+ u8 *protocols;
+ int ret;
+
+ /* preparation */
+ ut_assertok(uclass_get_device_by_name(UCLASS_SCMI_AGENT, "scmi",
+ &agent_dev));
+ ut_assertnonnull(agent_dev);
+ ut_assertnonnull(priv = dev_get_uclass_plat(agent_dev));
+ ut_assertnonnull(base = scmi_get_protocol(agent_dev,
+ SCMI_PROTOCOL_ID_BASE));
+ ut_assertnonnull(ops = dev_get_driver_ops(base));
+
+ /* version */
+ ret = (*ops->protocol_version)(base, &version);
+ ut_assertok(ret);
+ ut_asserteq(priv->version, version);
+
+ /* protocol attributes */
+ ret = (*ops->protocol_attrs)(base, &num_agents, &num_protocols);
+ ut_assertok(ret);
+ ut_asserteq(priv->num_agents, num_agents);
+ ut_asserteq(priv->num_protocols, num_protocols);
+
+ /* discover vendor */
+ ret = (*ops->base_discover_vendor)(base, vendor);
+ ut_assertok(ret);
+ ut_asserteq_str(priv->vendor, vendor);
+
+ /* message attributes */
+ ret = (*ops->protocol_message_attrs)(base,
+SCMI_BASE_DISCOVER_SUB_VENDOR,
+&attributes);
+ ut_assertok(ret);
+ ut_assertok(attributes);
+
+ /* discover sub vendor */
+ ret = (*ops->base_discover_sub_vendor)(base, vendor);
+ ut_assertok(ret);
+ ut_asserteq_str(priv->sub_vendor, vendor);
+
+ /* impl version */
+ ret = (*ops->base_discover_impl_version)(base, &impl_version);
+ ut_assertok(ret);
+ ut_asserteq(priv->impl_version, impl_version);
+
+ /* discover agent (my self) */
+ ret = (*ops->base_discover_agent)(base, 0x,
+ &agent_id, agent_name);
+ ut_assertok(ret);
+ ut_asserteq(priv->agent_id, agent_id);
+ ut_asserteq_str(priv->agent_name, agent_name);
+
+ /* discover protocols */
+ ret = (*ops->base_discover_list_protocols)(base, &protocols);
+ ut_asserteq(num_protocols, ret);
+ ut_asserteq_mem(priv->protocols, protocols, sizeof(u8) * num_protocols);
+ free(protocols);
+
+ /*
+* NOTE: Sandbox SCMI driver handles device-0 only. It supports setting
+* access and protocol permissions, but doesn't allow unsetting them nor
+* resetting the configurations.
+*/
+ /* set device permissions */
+ ret = (*ops->base_set_device_permissions)(base, agent_id, 0,
+
SCMI_BASE_SET_DEVICE_PERMISSIONS_ACCESS);
+ ut_assertok(ret); /* SCMI_SUCCESS */
+ ret = (*ops->base_set_device_permissions)(base, agent_id, 1,
+
SCMI_BASE_SET_DEVICE_PERMISSIONS_ACCESS);
+ ut_asserteq(-ENOENT, ret); /* SCMI_NOT_FOUND */
+ ret = (*ops->base_set_device_permissions)(base, agent_id, 0, 0);
+ ut_asserteq(-EACCES, ret); /* SCMI_DENIED */
+
+ /* set protocol permissions */
+ ret = (*ops->base_set_protocol_permissions)(base, agent_id, 0,
+ SCMI_PROTOCOL_ID_CLOCK,
+
SCMI_BASE_SET_PROTOCOL_PERMISSIONS_ACCESS);
+ ut_assertok(ret); /* SCMI_SUCCESS */
+ ret = (*ops->base_set_protocol_permissions)(base, agent_id, 1,
+ SCMI_PROTOCOL_ID_CLOCK,
+
SCMI_BASE_SET_PROTOCOL_PERMISSIONS_ACCESS);
+ ut_asserteq(-ENOENT, ret); /* SCMI_NOT_FOUND */
+ ret = (*ops->base_set_protocol_permissions)(base, agent_id, 0,
+ SCMI_PROTOCOL_ID_CLOCK, 0);
+ ut_asserteq(-E
[PATCH 03/10] firmware: scmi: install base protocol to SCMI agent
SCMI base protocol is mandatory, and once SCMI node is found in a device
tree, the protocol handle (udevice) is unconditionally installed to
the agent. Then basic information will be retrieved from SCMI server via
the protocol and saved into the agent instance's local storage.
Signed-off-by: AKASHI Takahiro
---
drivers/firmware/scmi/base.c | 6 ++
drivers/firmware/scmi/scmi_agent-uclass.c | 104 ++
include/scmi_agent-uclass.h | 78 +++-
include/scmi_agent.h | 10 +++
4 files changed, 195 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/scmi/base.c b/drivers/firmware/scmi/base.c
index 04018eb6ecf3..224e0ca7a915 100644
--- a/drivers/firmware/scmi/base.c
+++ b/drivers/firmware/scmi/base.c
@@ -484,6 +484,12 @@ static int scmi_base_probe(struct udevice *dev)
return ret;
}
+ ret = scmi_fill_base_info(dev->parent);
+ if (ret) {
+ dev_err(dev, "get information failed\n");
+ return ret;
+ }
+
return ret;
}
diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c
b/drivers/firmware/scmi/scmi_agent-uclass.c
index 6fd948ae6ddf..ec15580cc947 100644
--- a/drivers/firmware/scmi/scmi_agent-uclass.c
+++ b/drivers/firmware/scmi/scmi_agent-uclass.c
@@ -56,6 +56,9 @@ struct udevice *scmi_get_protocol(struct udevice *dev,
}
switch (id) {
+ case SCMI_PROTOCOL_ID_BASE:
+ proto = priv->base_dev;
+ break;
case SCMI_PROTOCOL_ID_CLOCK:
proto = priv->clock_dev;
break;
@@ -100,6 +103,9 @@ static int scmi_add_protocol(struct udevice *dev,
}
switch (proto_id) {
+ case SCMI_PROTOCOL_ID_BASE:
+ priv->base_dev = proto;
+ break;
case SCMI_PROTOCOL_ID_CLOCK:
priv->clock_dev = proto;
break;
@@ -149,6 +155,20 @@ static int scmi_bind_protocols(struct udevice *dev)
return -EBUSY;
}
+ drv = DM_DRIVER_GET(scmi_base_drv);
+ name = "scmi-base.0";
+ ret = device_bind(dev, drv, name, NULL, ofnode_null(), &proto);
+ if (ret) {
+ dev_err(dev, "failed to bind base protocol\n");
+ return ret;
+ }
+ ret = scmi_add_protocol(dev, SCMI_PROTOCOL_ID_BASE, proto);
+ if (ret) {
+ dev_err(dev, "failed to add protocol: %s, ret: %d\n",
+ proto->name, ret);
+ return ret;
+ }
+
dev_for_each_subnode(node, dev) {
u32 protocol_id;
@@ -262,6 +282,90 @@ int devm_scmi_process_msg(struct udevice *dev, struct
scmi_channel *channel,
return -EPROTONOSUPPORT;
}
+int scmi_fill_base_info(struct udevice *dev)
+{
+ struct scmi_agent_priv *priv;
+ struct udevice *base;
+ const struct scmi_base_ops *ops;
+ int ret;
+
+ priv = dev_get_uclass_plat(dev);
+
+ base = priv->base_dev;
+ if (!base) {
+ dev_err(dev, "Base protocol not found\n");
+ return -EPROTO;
+ }
+
+ ops = dev_get_driver_ops(base);
+ if (!ops) {
+ dev_err(base, "Operations in Base protocol not found\n");
+ return -EPROTO;
+ }
+ ret = (*ops->protocol_version)(base, &priv->version);
+ if (ret) {
+ dev_err(base, "protocol_version() failed (%d)\n", ret);
+ return ret;
+ }
+ /* check for required version */
+ if (priv->version < SCMI_BASE_PROTOCOL_VERSION) {
+ dev_err(base, "base protocol version (%d) lower than
expected\n",
+ priv->version);
+ return -EPROTO;
+ }
+
+ ret = (*ops->protocol_attrs)(base, &priv->num_agents,
+&priv->num_protocols);
+ if (ret) {
+ dev_err(base, "protocol_attrs() failed (%d)\n", ret);
+ return ret;
+ }
+ ret = (*ops->base_discover_vendor)(base, priv->vendor);
+ if (ret) {
+ dev_err(base, "base_discover_vendor() failed (%d)\n", ret);
+ return ret;
+ }
+ ret = (*ops->base_discover_sub_vendor)(base, priv->sub_vendor);
+ if (ret) {
+ if (ret != -EOPNOTSUPP) {
+ dev_err(base, "base_discover_sub_vendor() failed
(%d)\n",
+ ret);
+ return ret;
+ }
+ strcpy(priv->sub_vendor, "NA");
+ }
+ ret = (*ops->base_discover_impl_version)(base,
+&priv->impl_version);
+ if (ret) {
+ dev_err(base, "base_discover_impl_version() failed (%d)\n",
+ ret);
+ return ret;
+ }
+
+ priv->agent_id = 0x; /* to avoid false claim */
+ ret = (*ops->base_discover_agent)
[PATCH 04/10] sandbox: remove SCMI base node definition from test.dts
SCMI base protocol is mandatory and doesn't need to be listed in a device
tree.
Signed-off-by: AKASHI Takahiro
---
arch/sandbox/dts/test.dts | 4
1 file changed, 4 deletions(-)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index ff9f9222e6f9..2aad94681148 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -682,10 +682,6 @@
#address-cells = <1>;
#size-cells = <0>;
- protocol@10 {
- reg = <0x10>;
- };
-
clk_scmi: protocol@14 {
reg = <0x14>;
#clock-cells = <1>;
--
2.41.0
[PATCH 01/10] firmware: scmi: implement SCMI base protocol
SCMI base protocol is mandatory according to the SCMI specification.
With this patch, SCMI base protocol can be accessed via SCMI transport
layers. All the commands, except SCMI_BASE_NOTIFY_ERRORS, are supported.
This is because U-Boot doesn't support interrupts and the current transport
layers are not able to handle asynchronous messages properly.
Signed-off-by: AKASHI Takahiro
---
drivers/firmware/scmi/Makefile | 1 +
drivers/firmware/scmi/base.c | 517 +
include/dm/uclass-id.h | 1 +
include/scmi_protocols.h | 201 -
4 files changed, 718 insertions(+), 2 deletions(-)
create mode 100644 drivers/firmware/scmi/base.c
diff --git a/drivers/firmware/scmi/Makefile b/drivers/firmware/scmi/Makefile
index b2ff483c75a1..1a23d4981709 100644
--- a/drivers/firmware/scmi/Makefile
+++ b/drivers/firmware/scmi/Makefile
@@ -1,4 +1,5 @@
obj-y += scmi_agent-uclass.o
+obj-y += base.o
obj-y += smt.o
obj-$(CONFIG_SCMI_AGENT_SMCCC) += smccc_agent.o
obj-$(CONFIG_SCMI_AGENT_MAILBOX) += mailbox_agent.o
diff --git a/drivers/firmware/scmi/base.c b/drivers/firmware/scmi/base.c
new file mode 100644
index ..04018eb6ecf3
--- /dev/null
+++ b/drivers/firmware/scmi/base.c
@@ -0,0 +1,517 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * SCMI Base protocol as U-Boot device
+ *
+ * Copyright (C) 2023 Linaro Limited
+ * author: AKASHI Takahiro
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/**
+ * struct scmi_base_priv - Private data for SCMI base protocol
+ * @channel: Reference to the SCMI channel to use
+ */
+struct scmi_base_priv {
+ struct scmi_channel *channel;
+};
+
+/**
+ * scmi_protocol_version - get protocol version
+ * @dev: Device
+ * @version: Pointer to protocol version
+ *
+ * Obtain the protocol version number in @version.
+ *
+ * Return: 0 on success, error code otherwise
+ */
+static int scmi_protocol_version(struct udevice *dev, u32 *version)
+{
+ struct scmi_base_priv *priv = dev_get_priv(dev);
+ struct scmi_protocol_version_out out;
+ struct scmi_msg msg = {
+ .protocol_id = SCMI_PROTOCOL_ID_BASE,
+ .message_id = SCMI_PROTOCOL_VERSION,
+ .out_msg = (u8 *)&out,
+ .out_msg_sz = sizeof(out),
+ };
+ int ret;
+
+ ret = devm_scmi_process_msg(dev, priv->channel, &msg);
+ if (ret)
+ return ret;
+ if (out.status)
+ return scmi_to_linux_errno(out.status);
+
+ *version = out.version;
+
+ return 0;
+}
+
+/**
+ * scmi_protocol_attrs - get protocol attributes
+ * @dev: Device
+ * @num_agents:Number of agents
+ * @num_protocols: Number of protocols
+ *
+ * Obtain the protocol attributes, the number of agents and the number
+ * of protocols, in @num_agents and @num_protocols respectively, that
+ * the device provides.
+ *
+ * Return: 0 on success, error code otherwise
+ */
+static int scmi_protocol_attrs(struct udevice *dev, u32 *num_agents,
+ u32 *num_protocols)
+{
+ struct scmi_base_priv *priv = dev_get_priv(dev);
+ struct scmi_protocol_attrs_out out;
+ struct scmi_msg msg = {
+ .protocol_id = SCMI_PROTOCOL_ID_BASE,
+ .message_id = SCMI_PROTOCOL_ATTRIBUTES,
+ .out_msg = (u8 *)&out,
+ .out_msg_sz = sizeof(out),
+ };
+ int ret;
+
+ ret = devm_scmi_process_msg(dev, priv->channel, &msg);
+ if (ret)
+ return ret;
+ if (out.status)
+ return scmi_to_linux_errno(out.status);
+
+ *num_agents = SCMI_PROTOCOL_ATTRS_NUM_AGENTS(out.attributes);
+ *num_protocols = SCMI_PROTOCOL_ATTRS_NUM_PROTOCOLS(out.attributes);
+
+ return 0;
+}
+
+/**
+ * scmi_protocol_message_attrs - get message-specific attributes
+ * @dev: Device
+ * @message_id:Identifier of message
+ * @attributes:Message-specific attributes
+ *
+ * Obtain the message-specific attributes in @attributes.
+ * This command succeeds if the message is implemented and available.
+ *
+ * Return: 0 on success, error code otherwise
+ */
+static int scmi_protocol_message_attrs(struct udevice *dev, u32 message_id,
+ u32 *attributes)
+{
+ struct scmi_base_priv *priv = dev_get_priv(dev);
+ struct scmi_protocol_msg_attrs_out out;
+ struct scmi_msg msg = {
+ .protocol_id = SCMI_PROTOCOL_ID_BASE,
+ .message_id = SCMI_PROTOCOL_MESSAGE_ATTRIBUTES,
+ .in_msg = (u8 *)&message_id,
+ .in_msg_sz = sizeof(message_id),
+ .out_msg = (u8 *)&out,
+ .out_msg_sz = sizeof(out),
+ };
+ int ret;
+
+ ret = devm_scmi_process_msg(dev, priv->channel, &msg);
+ if (ret)
+ return ret;
+
[PATCH 02/10] firmware: scmi: framework for installing additional protocols
This framework allows SCMI protocols to be installed and bound to the agent
so that the agent can manage and utilize them later.
Signed-off-by: AKASHI Takahiro
---
drivers/firmware/scmi/scmi_agent-uclass.c | 105 +-
include/scmi_agent-uclass.h | 12 +++
include/scmi_agent.h | 14 +++
3 files changed, 128 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c
b/drivers/firmware/scmi/scmi_agent-uclass.c
index 02de692d66f3..6fd948ae6ddf 100644
--- a/drivers/firmware/scmi/scmi_agent-uclass.c
+++ b/drivers/firmware/scmi/scmi_agent-uclass.c
@@ -37,6 +37,86 @@ static const struct error_code scmi_linux_errmap[] = {
{ .scmi = SCMI_PROTOCOL_ERROR, .errno = -EPROTO, },
};
+/*
+ * NOTE: The only one instance should exist according to
+ * the current specification and device tree bindings.
+ */
+struct udevice *scmi_agent;
+
+struct udevice *scmi_get_protocol(struct udevice *dev,
+ enum scmi_std_protocol id)
+{
+ struct scmi_agent_priv *priv;
+ struct udevice *proto;
+
+ priv = dev_get_uclass_plat(dev);
+ if (!priv) {
+ dev_err(dev, "No priv data found\n");
+ return NULL;
+ }
+
+ switch (id) {
+ case SCMI_PROTOCOL_ID_CLOCK:
+ proto = priv->clock_dev;
+ break;
+ case SCMI_PROTOCOL_ID_RESET_DOMAIN:
+ proto = priv->resetdom_dev;
+ break;
+ case SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN:
+ proto = priv->voltagedom_dev;
+ break;
+ default:
+ dev_err(dev, "Protocol not supported\n");
+ proto = NULL;
+ break;
+ }
+ if (proto && device_probe(proto))
+ dev_err(dev, "Probe failed\n");
+
+ return proto;
+}
+
+/**
+ * scmi_add_protocol - add protocol to agent
+ * @dev: SCMI device
+ * @proto_id: Identifier of protocol
+ * @proto: Device of protocol instance
+ *
+ * Associate the protocol instance, @proto, to the agent, @dev,
+ * for later use.
+ *
+ * Return: 0 on success, error code otherwise
+ */
+static int scmi_add_protocol(struct udevice *dev,
+enum scmi_std_protocol proto_id,
+struct udevice *proto)
+{
+ struct scmi_agent_priv *priv;
+
+ priv = dev_get_uclass_plat(dev);
+ if (!priv) {
+ dev_err(dev, "No priv data found\n");
+ return -ENODEV;
+ }
+
+ switch (proto_id) {
+ case SCMI_PROTOCOL_ID_CLOCK:
+ priv->clock_dev = proto;
+ break;
+ case SCMI_PROTOCOL_ID_RESET_DOMAIN:
+ priv->resetdom_dev = proto;
+ break;
+ case SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN:
+ priv->voltagedom_dev = proto;
+ break;
+ default:
+ dev_err(dev, "Protocol not supported\n");
+ return -EPROTO;
+ }
+
+ return 0;
+}
+
int scmi_to_linux_errno(s32 scmi_code)
{
int n;
@@ -61,9 +141,15 @@ static int scmi_bind_protocols(struct udevice *dev)
int ret = 0;
ofnode node;
const char *name;
+ struct driver *drv;
+ struct udevice *proto;
+
+ if (scmi_agent) {
+ dev_err(dev, "SCMI agent already exists: %s\n", dev->name);
+ return -EBUSY;
+ }
dev_for_each_subnode(node, dev) {
- struct driver *drv = NULL;
u32 protocol_id;
if (!ofnode_is_enabled(node))
@@ -72,6 +158,7 @@ static int scmi_bind_protocols(struct udevice *dev)
if (ofnode_read_u32(node, "reg", &protocol_id))
continue;
+ drv = NULL;
name = ofnode_get_name(node);
switch (protocol_id) {
case SCMI_PROTOCOL_ID_CLOCK:
@@ -102,11 +189,22 @@ static int scmi_bind_protocols(struct udevice *dev)
continue;
}
- ret = device_bind(dev, drv, name, NULL, node, NULL);
- if (ret)
+ ret = device_bind(dev, drv, name, NULL, node, &proto);
+ if (ret) {
+ dev_err(dev, "failed to bind %s protocol\n", drv->name);
break;
+ }
+ ret = scmi_add_protocol(dev, protocol_id, proto);
+ if (ret) {
+ dev_err(dev, "failed to add protocol: %s, ret: %d\n",
+ proto->name, ret);
+ break;
+ }
}
+ if (!ret)
+ scmi_agent = dev;
+
return ret;
}
@@ -168,4 +266,5 @@ UCLASS_DRIVER(scmi_agent) = {
.id = UCLASS_SCMI_AGENT,
.name = "scmi_agent",
.post_bind = scmi_bind_protocols,
+ .per_device_plat_auto = sizeof(struct scmi
[PATCH 00/10] firmware: scmi: add SCMI base protocol support
This patch series allows users to access SCMI base protocol provided by SCMI server (platform). It will also be utilized in separate patches in the future to add sanity/validity checks for other protocols. See SCMI specification document v3.2 beta[1] for more details about SCMI base protocol. What is currently not implemented is - SCMI_BASE_NOTIFY_ERRORS command and notification callback mechanism This feature won't be very useful in the current U-Boot environment. [1] https://developer.arm.com/documentation/den0056/e/?lang=en Test The patch series was tested on the following platforms: * sandbox * qemu-arm64 with OPTEE as SCMI server Prerequisite: = * This patch series is based on v2023.07-rc and relies on the following patch: [2] https://lists.denx.de/pipermail/u-boot/2023-June/520083.html Patches: Patch#1-#4: Add SCMI base protocol driver Patch#5-#7: Add SCMI base protocol device unit test Patch#8-#10: Add scmi command Change history: === v1 (Jun, 28, 2023) * initial release AKASHI Takahiro (10): firmware: scmi: implement SCMI base protocol firmware: scmi: framework for installing additional protocols firmware: scmi: install base protocol to SCMI agent sandbox: remove SCMI base node definition from test.dts firmware: scmi: fake base protocol commands on sandbox test: dm: simplify SCMI unit test on sandbox test: dm: add SCMI base protocol test cmd: add scmi command for SCMI firmware doc: cmd: add documentation for scmi test: dm: add scmi command test arch/sandbox/dts/test.dts | 4 - arch/sandbox/include/asm/scmi_test.h | 7 +- cmd/Kconfig| 8 + cmd/Makefile | 1 + cmd/scmi.c | 373 +++ doc/usage/cmd/scmi.rst | 98 drivers/firmware/scmi/Makefile | 1 + drivers/firmware/scmi/base.c | 523 + drivers/firmware/scmi/sandbox-scmi_agent.c | 379 ++- drivers/firmware/scmi/scmi_agent-uclass.c | 195 +++- include/dm/uclass-id.h | 1 + include/scmi_agent-uclass.h| 84 include/scmi_agent.h | 24 + include/scmi_protocols.h | 201 +++- test/dm/scmi.c | 233 +++-- 15 files changed, 2057 insertions(+), 75 deletions(-) create mode 100644 cmd/scmi.c create mode 100644 doc/usage/cmd/scmi.rst create mode 100644 drivers/firmware/scmi/base.c -- 2.41.0
[PATCH 4/5] rockchip: rk3399-roc-pc: Fix SPL max size and SPI flash payload offset
TPL max size is limited to 184 KB, SPL is loaded to 0x0 and TF-A is
loaded to 0x4, this limit SPL max size to 256 KB. With BootRom only
reading first 2 KB per 4 KB page of SPI flash, 880 KB may be needed for
TPL+SPL in a worst-case scenario. (184 KB + 256 KB) x 2 = 880 KB
Use 0xE (896 KB) as the payload offset in SPI flash, this allows
for a payload of 3168 KB before env offset start to overlap.
Also add CONFIG_ROCKCHIP_SPI_IMAGE=y to build a bootable SPI flash
image, u-boot-rockchip-spi.bin.
Signed-off-by: Jonas Karlman
---
arch/arm/dts/rk3399-roc-pc-u-boot.dtsi| 4
configs/roc-pc-mezzanine-rk3399_defconfig | 4 +++-
configs/roc-pc-rk3399_defconfig | 4 +++-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm/dts/rk3399-roc-pc-u-boot.dtsi
b/arch/arm/dts/rk3399-roc-pc-u-boot.dtsi
index f85e7b62d9ad..c8f4418a7389 100644
--- a/arch/arm/dts/rk3399-roc-pc-u-boot.dtsi
+++ b/arch/arm/dts/rk3399-roc-pc-u-boot.dtsi
@@ -11,10 +11,6 @@
u-boot,spl-boot-order = "same-as-spl", &spi_flash, &sdhci,
&sdmmc;
};
- config {
- u-boot,spl-payload-offset = <0x6>; /* @ 384KB */
- };
-
vcc_hub_en: vcc_hub_en-regulator {
compatible = "regulator-fixed";
enable-active-high;
diff --git a/configs/roc-pc-mezzanine-rk3399_defconfig
b/configs/roc-pc-mezzanine-rk3399_defconfig
index 9321292ca526..6cacff77d4ad 100644
--- a/configs/roc-pc-mezzanine-rk3399_defconfig
+++ b/configs/roc-pc-mezzanine-rk3399_defconfig
@@ -13,6 +13,7 @@ CONFIG_ENV_SECT_SIZE=0x1000
CONFIG_DEFAULT_DEVICE_TREE="rk3399-roc-pc-mezzanine"
CONFIG_DM_RESET=y
CONFIG_ROCKCHIP_RK3399=y
+CONFIG_ROCKCHIP_SPI_IMAGE=y
CONFIG_TARGET_ROC_PC_RK3399=y
CONFIG_SPL_STACK=0x40
CONFIG_DEBUG_UART_BASE=0xFF1A
@@ -26,7 +27,7 @@ CONFIG_DEBUG_UART=y
CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-roc-pc-mezzanine.dtb"
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_MISC_INIT_R=y
-CONFIG_SPL_MAX_SIZE=0x2e000
+CONFIG_SPL_MAX_SIZE=0x4
CONFIG_SPL_PAD_TO=0x7f8000
CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
CONFIG_SPL_BSS_START_ADDR=0x40
@@ -37,6 +38,7 @@ CONFIG_SPL_STACK_R=y
CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x2
CONFIG_SPL_ENV_SUPPORT=y
CONFIG_SPL_SPI_LOAD=y
+CONFIG_SYS_SPI_U_BOOT_OFFS=0xE
CONFIG_TPL=y
CONFIG_CMD_BOOTZ=y
CONFIG_CMD_GPT=y
diff --git a/configs/roc-pc-rk3399_defconfig b/configs/roc-pc-rk3399_defconfig
index 824b4041c208..199081ac8414 100644
--- a/configs/roc-pc-rk3399_defconfig
+++ b/configs/roc-pc-rk3399_defconfig
@@ -14,6 +14,7 @@ CONFIG_ENV_SECT_SIZE=0x1000
CONFIG_DEFAULT_DEVICE_TREE="rk3399-roc-pc"
CONFIG_DM_RESET=y
CONFIG_ROCKCHIP_RK3399=y
+CONFIG_ROCKCHIP_SPI_IMAGE=y
CONFIG_TARGET_ROC_PC_RK3399=y
CONFIG_SPL_STACK=0x40
CONFIG_DEBUG_UART_BASE=0xFF1A
@@ -27,7 +28,7 @@ CONFIG_USE_PREBOOT=y
CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-roc-pc.dtb"
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_MISC_INIT_R=y
-CONFIG_SPL_MAX_SIZE=0x2e000
+CONFIG_SPL_MAX_SIZE=0x4
CONFIG_SPL_PAD_TO=0x7f8000
CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
CONFIG_SPL_BSS_START_ADDR=0x40
@@ -38,6 +39,7 @@ CONFIG_SPL_STACK_R=y
CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x2
CONFIG_SPL_ENV_SUPPORT=y
CONFIG_SPL_SPI_LOAD=y
+CONFIG_SYS_SPI_U_BOOT_OFFS=0xE
CONFIG_TPL=y
CONFIG_CMD_BOOTZ=y
CONFIG_CMD_GPT=y
--
2.41.0
[PATCH 5/5] doc: rockchip: Update SPI flashing instruction
Update documentation on how to write a bootable u-boot-rockchip-spi.bin
image into SPI flash. This removes the reference to a hardcoded and now
obsolete 0x6 payload offset.
Also remove an obsolete reference to pad_cat.
Signed-off-by: Jonas Karlman
---
doc/board/rockchip/rockchip.rst | 26 +-
1 file changed, 5 insertions(+), 21 deletions(-)
diff --git a/doc/board/rockchip/rockchip.rst b/doc/board/rockchip/rockchip.rst
index 99376fb54c90..5471bb928205 100644
--- a/doc/board/rockchip/rockchip.rst
+++ b/doc/board/rockchip/rockchip.rst
@@ -211,7 +211,7 @@ SD Card
^^^
All Rockchip platforms (except rk3128 which doesn't use SPL) are now
-supporting a single boot image using binman and pad_cat.
+supporting a single boot image using binman.
To write an image that boots from a SD card (assumed to be /dev/sda):
@@ -262,31 +262,15 @@ is u-boot-dtb.img
SPI
^^^
-The SPI boot method requires the generation of idbloader.img with help of the
mkimage tool.
+Write u-boot-rockchip-spi.bin to offset 0 of SPI flash.
-SPL-alone SPI boot image:
-
-.. code-block:: bash
-
-./tools/mkimage -n rk3399 -T rkspi -d spl/u-boot-spl.bin idbloader.img
-
-TPL+SPL SPI boot image:
-
-.. code-block:: bash
-
-./tools/mkimage -n rk3399 -T rkspi -d
tpl/u-boot-tpl.bin:spl/u-boot-spl.bin idbloader.img
-
-Copy SPI boot images into SD card and boot from SD:
+Copy u-boot-rockchip-spi.bin into SD card and boot from SD:
.. code-block:: bash
sf probe
-load mmc 1:1 $kernel_addr_r idbloader.img
-sf erase 0 +$filesize
-sf write $kernel_addr_r 0 ${filesize}
-load mmc 1:1 ${kernel_addr_r} u-boot.itb
-sf erase 0x6 +$filesize
-sf write $kernel_addr_r 0x6 ${filesize}
+load mmc 1:1 $kernel_addr_r u-boot-rockchip-spi.bin
+sf update $fileaddr 0 $filesize
2. Package the image with Rockchip miniloader
-
--
2.41.0
[PATCH 3/5] rockchip: rk3399-pinephone-pro: Fix SPL max size and SPI flash payload offset
TPL max size is limited to 184 KB, SPL is loaded to 0x0 and TF-A is
loaded to 0x4, this limit SPL max size to 256 KB. With BootRom only
reading first 2 KB per 4 KB page of SPI flash, 880 KB may be needed for
TPL+SPL in a worst-case scenario. (184 KB + 256 KB) x 2 = 880 KB
Use 0xE (896 KB) as the payload offset in SPI flash, this allows
for a payload of 3168 KB before env offset start to overlap.
Also add CONFIG_ROCKCHIP_SPI_IMAGE=y to build a bootable SPI flash
image, u-boot-rockchip-spi.bin.
Signed-off-by: Jonas Karlman
---
arch/arm/dts/rk3399-pinephone-pro-u-boot.dtsi | 4
configs/pinephone-pro-rk3399_defconfig| 4 +++-
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/arm/dts/rk3399-pinephone-pro-u-boot.dtsi
b/arch/arm/dts/rk3399-pinephone-pro-u-boot.dtsi
index 347243fe4793..cabf0a9dae89 100644
--- a/arch/arm/dts/rk3399-pinephone-pro-u-boot.dtsi
+++ b/arch/arm/dts/rk3399-pinephone-pro-u-boot.dtsi
@@ -10,10 +10,6 @@
chosen {
u-boot,spl-boot-order = "same-as-spl", &sdhci, &sdmmc;
};
-
- config {
- u-boot,spl-payload-offset = <0x6>; /* @ 384KB */
- };
};
&rng {
diff --git a/configs/pinephone-pro-rk3399_defconfig
b/configs/pinephone-pro-rk3399_defconfig
index e4a8beeb1abb..b09211f4eb6e 100644
--- a/configs/pinephone-pro-rk3399_defconfig
+++ b/configs/pinephone-pro-rk3399_defconfig
@@ -12,6 +12,7 @@ CONFIG_ENV_OFFSET=0x3F8000
CONFIG_DEFAULT_DEVICE_TREE="rk3399-pinephone-pro"
CONFIG_DM_RESET=y
CONFIG_ROCKCHIP_RK3399=y
+CONFIG_ROCKCHIP_SPI_IMAGE=y
CONFIG_TARGET_PINEPHONE_PRO_RK3399=y
CONFIG_SPL_STACK=0x40
CONFIG_DEBUG_UART_BASE=0xFF1A
@@ -25,7 +26,7 @@ CONFIG_USE_PREBOOT=y
CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-pinephone-pro.dtb"
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_MISC_INIT_R=y
-CONFIG_SPL_MAX_SIZE=0x2e000
+CONFIG_SPL_MAX_SIZE=0x4
CONFIG_SPL_PAD_TO=0x7f8000
CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
CONFIG_SPL_BSS_START_ADDR=0x40
@@ -35,6 +36,7 @@ CONFIG_SPL_BSS_MAX_SIZE=0x2000
CONFIG_SPL_STACK_R=y
CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x1
CONFIG_SPL_SPI_LOAD=y
+CONFIG_SYS_SPI_U_BOOT_OFFS=0xE
CONFIG_TPL=y
CONFIG_CMD_BOOTZ=y
CONFIG_CMD_GPIO=y
--
2.41.0
[PATCH 1/5] rockchip: rk3399-rockpro64: Fix SPL max size and SPI flash payload offset
TPL max size is limited to 184 KB, SPL is loaded to 0x0 and TF-A is
loaded to 0x4, this limit SPL max size to 256 KB. With BootRom only
reading first 2 KB per 4 KB page of SPI flash, 880 KB may be needed for
TPL+SPL in a worst-case scenario. (184 KB + 256 KB) x 2 = 880 KB
Use 0xE (896 KB) as the payload offset in SPI flash, this allows
for a payload of 3168 KB before env offset start to overlap.
Also remove CONFIG_LTO=y now that there is sufficient space for SPL in
SPI flash, and to fix a build issue reported by Peter Robinson.
Fixes: 5713135ecc75 ("rockchip: rockpro64: Build u-boot-rockchip-spi.bin")
Reported-by: Peter Robinson
Signed-off-by: Jonas Karlman
---
arch/arm/dts/rk3399-rockpro64-u-boot.dtsi | 4
configs/rockpro64-rk3399_defconfig| 5 ++---
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
index bd864d067018..732727d9b036 100644
--- a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
+++ b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
@@ -9,10 +9,6 @@
chosen {
u-boot,spl-boot-order = "same-as-spl", &spi_flash, &sdmmc,
&sdhci;
};
-
- config {
- u-boot,spl-payload-offset = <0x6>; /* @ 384KB */
- };
};
&sdhci {
diff --git a/configs/rockpro64-rk3399_defconfig
b/configs/rockpro64-rk3399_defconfig
index dc4392c7451c..2f1ab1c28864 100644
--- a/configs/rockpro64-rk3399_defconfig
+++ b/configs/rockpro64-rk3399_defconfig
@@ -21,7 +21,6 @@ CONFIG_SPL_SPI=y
CONFIG_SYS_LOAD_ADDR=0x800800
CONFIG_PCI=y
CONFIG_DEBUG_UART=y
-CONFIG_LTO=y
CONFIG_SPL_FIT_SIGNATURE=y
CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_BOOTSTAGE=y
@@ -30,7 +29,7 @@ CONFIG_USE_PREBOOT=y
CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-rockpro64.dtb"
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_MISC_INIT_R=y
-CONFIG_SPL_MAX_SIZE=0x2e000
+CONFIG_SPL_MAX_SIZE=0x4
CONFIG_SPL_PAD_TO=0x7f8000
CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
CONFIG_SPL_BSS_START_ADDR=0x40
@@ -40,7 +39,7 @@ CONFIG_SPL_BSS_MAX_SIZE=0x2000
CONFIG_SPL_STACK_R=y
CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x1
CONFIG_SPL_SPI_LOAD=y
-CONFIG_SYS_SPI_U_BOOT_OFFS=0x6
+CONFIG_SYS_SPI_U_BOOT_OFFS=0xE
CONFIG_TPL=y
CONFIG_CMD_BOOTZ=y
CONFIG_CMD_GPT=y
--
2.41.0
[PATCH 0/5] rockchip: Fix SPI flash SPL and payload overlap
This series fixes a build issue introduced in commit 5713135ecc75
("rockchip: rockpro64: Build u-boot-rockchip-spi.bin"), reported by
Peter Robinson.
Closer inspection into how SPI flash is used revealed that current
payload offset, 0x6, is not enough to accommodate a worst-case
scenario and may overlap SPL if written to SPI flash.
This series fixes this by adjusting payload offset to 0xE, adjusting
SPL max size to 0x4, enables build of u-boot-rockchip-spi.bin for
affected boards and update the SPI flashing documentation.
Series depend on [1] for a clean apply.
[1] https://patchwork.ozlabs.org/patch/1800173/
Jonas Karlman (5):
rockchip: rk3399-rockpro64: Fix SPL max size and SPI flash payload
offset
rockchip: rk3399-pinebook-pro: Fix SPL max size and SPI flash payload
offset
rockchip: rk3399-pinephone-pro: Fix SPL max size and SPI flash payload
offset
rockchip: rk3399-roc-pc: Fix SPL max size and SPI flash payload offset
doc: rockchip: Update SPI flashing instruction
arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi | 4 ---
arch/arm/dts/rk3399-pinephone-pro-u-boot.dtsi | 4 ---
arch/arm/dts/rk3399-roc-pc-u-boot.dtsi| 4 ---
arch/arm/dts/rk3399-rockpro64-u-boot.dtsi | 4 ---
configs/pinebook-pro-rk3399_defconfig | 4 ++-
configs/pinephone-pro-rk3399_defconfig| 4 ++-
configs/roc-pc-mezzanine-rk3399_defconfig | 4 ++-
configs/roc-pc-rk3399_defconfig | 4 ++-
configs/rockpro64-rk3399_defconfig| 5 ++--
doc/board/rockchip/rockchip.rst | 26 ---
10 files changed, 19 insertions(+), 44 deletions(-)
--
2.41.0
[PATCH 2/5] rockchip: rk3399-pinebook-pro: Fix SPL max size and SPI flash payload offset
TPL max size is limited to 184 KB, SPL is loaded to 0x0 and TF-A is
loaded to 0x4, this limit SPL max size to 256 KB. With BootRom only
reading first 2 KB per 4 KB page of SPI flash, 880 KB may be needed for
TPL+SPL in a worst-case scenario. (184 KB + 256 KB) x 2 = 880 KB
Use 0xE (896 KB) as the payload offset in SPI flash, this allows
for a payload of 3168 KB before env offset start to overlap.
Also add CONFIG_ROCKCHIP_SPI_IMAGE=y to build a bootable SPI flash
image, u-boot-rockchip-spi.bin.
Signed-off-by: Jonas Karlman
---
arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi | 4
configs/pinebook-pro-rk3399_defconfig| 4 +++-
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi
b/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi
index ea7a5a17ae0f..88a77cad8d43 100644
--- a/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi
+++ b/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi
@@ -10,10 +10,6 @@
chosen {
u-boot,spl-boot-order = "same-as-spl", &sdhci, &spiflash,
&sdmmc;
};
-
- config {
- u-boot,spl-payload-offset = <0x6>; /* @ 384KB */
- };
};
&edp {
diff --git a/configs/pinebook-pro-rk3399_defconfig
b/configs/pinebook-pro-rk3399_defconfig
index 58a8b91aa60f..1109ebb7387b 100644
--- a/configs/pinebook-pro-rk3399_defconfig
+++ b/configs/pinebook-pro-rk3399_defconfig
@@ -12,6 +12,7 @@ CONFIG_ENV_OFFSET=0x3F8000
CONFIG_DEFAULT_DEVICE_TREE="rk3399-pinebook-pro"
CONFIG_DM_RESET=y
CONFIG_ROCKCHIP_RK3399=y
+CONFIG_ROCKCHIP_SPI_IMAGE=y
CONFIG_TARGET_PINEBOOK_PRO_RK3399=y
CONFIG_SPL_STACK=0x40
CONFIG_DEBUG_UART_BASE=0xFF1A
@@ -26,7 +27,7 @@ CONFIG_USE_PREBOOT=y
CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-pinebook-pro.dtb"
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_MISC_INIT_R=y
-CONFIG_SPL_MAX_SIZE=0x2e000
+CONFIG_SPL_MAX_SIZE=0x4
CONFIG_SPL_PAD_TO=0x7f8000
CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
CONFIG_SPL_BSS_START_ADDR=0x40
@@ -36,6 +37,7 @@ CONFIG_SPL_BSS_MAX_SIZE=0x2000
CONFIG_SPL_STACK_R=y
CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x1
CONFIG_SPL_SPI_LOAD=y
+CONFIG_SYS_SPI_U_BOOT_OFFS=0xE
CONFIG_TPL=y
CONFIG_CMD_BOOTZ=y
CONFIG_CMD_GPIO=y
--
2.41.0
Re: [PATCH 5/7] Makefile: Add a target for building capsules
hi Simon, On Tue, 27 Jun 2023 at 17:51, Simon Glass wrote: > > Hi Sughosh, > > On Tue, 27 Jun 2023 at 13:08, Sughosh Ganu wrote: > > > > hi Simon, > > > > On Tue, 27 Jun 2023 at 16:50, Simon Glass wrote: > > > > > > Hi Sughosh, > > > > > > On Tue, 27 Jun 2023 at 05:57, Sughosh Ganu > > > wrote: > > > > > > > > hi Simon, > > > > > > > > On Mon, 26 Jun 2023 at 17:43, Sughosh Ganu > > > > wrote: > > > > > > > > > > hi Simon, > > > > > > > > > > On Mon, 26 Jun 2023 at 14:38, Simon Glass wrote: > > > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > On Wed, 21 Jun 2023 at 05:26, Sughosh Ganu > > > > > > wrote: > > > > > > > > > > > > > > hi Simon, > > > > > > > > > > > > > > On Mon, 19 Jun 2023 at 18:07, Simon Glass > > > > > > > wrote: > > > > > > > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > > > > > On Thu, 15 Jun 2023 at 17:25, Sughosh Ganu > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > hi Simon, > > > > > > > > > > > > > > > > > > On Thu, 15 Jun 2023 at 14:44, Simon Glass > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > > > > > > > > > On Tue, 13 Jun 2023 at 11:39, Sughosh Ganu > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > Add a target for building EFI capsules. The capsule > > > > > > > > > > > parameters are > > > > > > > > > > > specified through a config file, and the path to the > > > > > > > > > > > config file is > > > > > > > > > > > specified through CONFIG_EFI_CAPSULE_CFG_FILE. When the > > > > > > > > > > > config file is > > > > > > > > > > > not specified, the command only builds tools. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu > > > > > > > > > > > --- > > > > > > > > > > > Makefile | 9 + > > > > > > > > > > > 1 file changed, 9 insertions(+) > > > > > > > > > > > > > > > > > > > > > > diff --git a/Makefile b/Makefile > > > > > > > > > > > index 10bfaa52ad..96db29aa77 100644 > > > > > > > > > > > --- a/Makefile > > > > > > > > > > > +++ b/Makefile > > > > > > > > > > > @@ -1151,6 +1151,15 @@ dtbs: dts/dt.dtb > > > > > > > > > > > dts/dt.dtb: u-boot > > > > > > > > > > > $(Q)$(MAKE) $(build)=dts dtbs > > > > > > > > > > > > > > > > > > > > > > +quiet_cmd_mkeficapsule = MKEFICAPSULE $@ > > > > > > > > > > > +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule $@ > > > > > > > > > > > + > > > > > > > > > > > +PHONY += capsule > > > > > > > > > > > +capsule: tools > > > > > > > > > > > +ifneq ($(CONFIG_EFI_CAPSULE_CFG_FILE),"") > > > > > > > > > > > + $(call cmd,mkeficapsule) > > > > > > > > > > > +endif > > > > > > > > > > > + > > > > > > > > > > > quiet_cmd_copy = COPY$@ > > > > > > > > > > >cmd_copy = cp $< $@ > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > 2.34.1 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > We should be using binman to build images...you seem to be > > > > > > > > > > building > > > > > > > > > > something in parallel with that. Can you please take a look > > > > > > > > > > at binman? > > > > > > > > > > > > > > > > > > Again, I had explored using binman for this task. The one > > > > > > > > > issue where > > > > > > > > > I find the above flow better is that I can simply build my > > > > > > > > > payload > > > > > > > > > image(s) followed by 'make capsule' to generate the capsules > > > > > > > > > for > > > > > > > > > earlier generated images. In it's current form, I don't see > > > > > > > > > an easy > > > > > > > > > way to enforce this dependency in binman when I want to build > > > > > > > > > the > > > > > > > > > payload followed by generation of capsules. I did see the > > > > > > > > > mention of > > > > > > > > > encapsulating an entry within another dependent entry, but I > > > > > > > > > think > > > > > > > > > that makes the implementation more complex than it ought to > > > > > > > > > be. > > > > > > > > > > > > > > > > > > I think it is much easier to use the make flow to generate > > > > > > > > > the images > > > > > > > > > followed by capsules, instead of tweaking the binman node to > > > > > > > > > first > > > > > > > > > generate the payload images, followed by enabling the capsule > > > > > > > > > node to > > > > > > > > > build the capsules. If there is an easy way of enforcing this > > > > > > > > > dependency, please let me know. Thanks > > > > > > > > > > > > > > > > Can you share your explorations? I think the capsule should be > > > > > > > > created > > > > > > > > as part of the build, if enabled. Rather than changing the input > > > > > > > > files, binman should produce new output files. > > > > > > > > > > > > > > This is an issue of handling dependencies in binman, and not > > > > > > > changing > > > > > > > input files. We do not have support for telling binman > > > > > > > "build/generate > > > > > > > this particular image first before you proceed to build the > > > > > > > capsules > >
Re: [PATCH] smegw01: Fix wrong symbol override
On Tue, Jun 27, 2023 at 01:57:49PM -0300, Fabio Estevam wrote: > From: Eduard Strehlau > > board_mmc_get_env_part() is not called as the default implementation > of mmc_get_env_part() is used. > > Fix this problem by directly calling mmc_get_env_part() instead. > > Signed-off-by: Eduard Strehlau > Signed-off-by: Fabio Estevam > --- > Tom, > > Stefano is out this week. Could you please apply this one > directly so that it gets included in 2023.07 as well? Yes, I've got a few other things in a bundle now and I want to wait just a little longer. -- Tom signature.asc Description: PGP signature
[PATCH] smegw01: Fix wrong symbol override
From: Eduard Strehlau
board_mmc_get_env_part() is not called as the default implementation
of mmc_get_env_part() is used.
Fix this problem by directly calling mmc_get_env_part() instead.
Signed-off-by: Eduard Strehlau
Signed-off-by: Fabio Estevam
---
Tom,
Stefano is out this week. Could you please apply this one
directly so that it gets included in 2023.07 as well?
Thanks
board/storopack/smegw01/smegw01.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/storopack/smegw01/smegw01.c
b/board/storopack/smegw01/smegw01.c
index 20c09700bf0d..7b2c50a61e43 100644
--- a/board/storopack/smegw01/smegw01.c
+++ b/board/storopack/smegw01/smegw01.c
@@ -102,7 +102,7 @@ int board_late_init(void)
return 0;
}
-uint board_mmc_get_env_part(struct mmc *mmc)
+uint mmc_get_env_part(struct mmc *mmc)
{
uint part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
--
2.34.1
Re: [PULL] u-boot-riscv/riscv-fixes
On Tue, Jun 27, 2023 at 07:27:06AM +, Leo Liang wrote: > Hi Tom, > > The following changes since commit 4f1077bc35f683985ff77e442ada7e8a8a52e3b7: > > Prepare v2023.07-rc5 (2023-06-26 11:44:06 -0400) > > are available in the Git repository at: > > https://source.denx.de/u-boot/custodians/u-boot-riscv.git riscv-fixes > > for you to fetch changes up to 4a3efd71cd858b87527e9478ff51529d39329819: > > riscv: Fix alignment of RELA sections in the linker scripts (2023-06-27 > 10:09:51 +0800) > > CI result shows no issue: > https://source.denx.de/u-boot/custodians/u-boot-riscv/-/pipelines/16707 > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v7 7/7] board: htc: endeavoru: add One X support
Thierry, Endeavoru device tree is not included into Makefile. Will you include
it or should I send v8? It feels like these patches are hanging in here for
ethernity.
23 червня 2023 р. 08:56:00 GMT+03:00, Svyatoslav Ryhel
написав(-ла):
>The HTC One X is a touchscreen-based, slate-sized smartphone
>designed and manufactured by HTC that runs the Android operating
>system. The One X features a 4.7" display, an Nvidia Tegra 3
>quad-core chip, 1 GB of RAM and non-extendable 32 GB of internal
>storage. UART-A is default debug port.
>
>Tested-by: Andreas Westman Dorcsak
>Tested-by: Ion Agorria
>Tested-by: Svyatoslav Ryhel
>Signed-off-by: Svyatoslav Ryhel
>---
> arch/arm/dts/tegra30-htc-endeavoru.dts| 166
> arch/arm/mach-tegra/tegra30/Kconfig | 5 +
> board/htc/endeavoru/Kconfig | 12 +
> board/htc/endeavoru/MAINTAINERS | 7 +
> board/htc/endeavoru/Makefile | 11 +
> board/htc/endeavoru/endeavoru-spl.c | 47 +++
> board/htc/endeavoru/endeavoru.c | 116 ++
> board/htc/endeavoru/pinmux-config-endeavoru.h | 362 ++
> configs/endeavoru_defconfig | 84
> doc/board/htc/endeavoru.rst | 89 +
> doc/board/htc/index.rst | 9 +
> doc/board/index.rst | 1 +
> include/configs/endeavoru.h | 65
> 13 files changed, 974 insertions(+)
> create mode 100644 arch/arm/dts/tegra30-htc-endeavoru.dts
> create mode 100644 board/htc/endeavoru/Kconfig
> create mode 100644 board/htc/endeavoru/MAINTAINERS
> create mode 100644 board/htc/endeavoru/Makefile
> create mode 100644 board/htc/endeavoru/endeavoru-spl.c
> create mode 100644 board/htc/endeavoru/endeavoru.c
> create mode 100644 board/htc/endeavoru/pinmux-config-endeavoru.h
> create mode 100644 configs/endeavoru_defconfig
> create mode 100644 doc/board/htc/endeavoru.rst
> create mode 100644 doc/board/htc/index.rst
> create mode 100644 include/configs/endeavoru.h
>
>diff --git a/arch/arm/dts/tegra30-htc-endeavoru.dts
>b/arch/arm/dts/tegra30-htc-endeavoru.dts
>new file mode 100644
>index 00..c55e193d1d
>--- /dev/null
>+++ b/arch/arm/dts/tegra30-htc-endeavoru.dts
>@@ -0,0 +1,166 @@
>+// SPDX-License-Identifier: GPL-2.0
>+/dts-v1/;
>+
>+/* This dts file describes the HTC One X smartphone */
>+/* CPU Speedo ID 4, Soc Speedo ID 1, CPU Process: 1, Core Process: 0 */
>+
>+#include
>+
>+#include "tegra30.dtsi"
>+
>+/ {
>+ model = "HTC One X";
>+ compatible = "htc,endeavoru", "nvidia,tegra30";
>+
>+ chosen {
>+ stdout-path = &uarta;
>+ };
>+
>+ aliases {
>+ i2c0 = &pwr_i2c;
>+
>+ mmc0 = &sdmmc4; /* eMMC */
>+
>+ rtc0 = &pmic;
>+ rtc1 = "/rtc@7000e000";
>+
>+ usb0 = µ_usb;
>+ };
>+
>+ memory {
>+ device_type = "memory";
>+ reg = <0x8000 0x4000>;
>+ };
>+
>+ host1x@5000 {
>+ dc@5420 {
>+ clocks = <&tegra_car TEGRA30_CLK_DISP1>,
>+ <&tegra_car TEGRA30_CLK_PLL_D_OUT0>;
>+
>+ rgb {
>+ status = "okay";
>+
>+ nvidia,panel = <&dsia>;
>+ };
>+ };
>+
>+ dsia: dsi@5430 {
>+ status = "okay";
>+
>+ avdd-dsi-csi-supply = <&avdd_dsi_csi>;
>+
>+ panel = <&panel>;
>+ };
>+ };
>+
>+ uarta: serial@70006000 {
>+ status = "okay";
>+ };
>+
>+ pwr_i2c: i2c@7000d000 {
>+ status = "okay";
>+ clock-frequency = <10>;
>+
>+ /* Texas Instruments TPS80032 PMIC */
>+ pmic: tps80032@48 {
>+ compatible = "ti,tps80032";
>+ reg = <0x48>;
>+
>+ regulators {
>+ /* DSI VDD */
>+ avdd_dsi_csi: ldo1 {
>+ regulator-name = "avdd_dsi_csi";
>+ regulator-min-microvolt = <120>;
>+ regulator-max-microvolt = <120>;
>+ regulator-always-on;
>+ };
>+ };
>+ };
>+ };
>+
>+ sdmmc4: sdhci@78000600 {
>+ status = "okay";
>+ bus-width = <8>;
>+ non-removable;
>+ };
>+
>+ micro_usb: usb@7d00 {
>+ status = "okay";
>+ dr_mode = "otg";
>+ };
>+
>+ backlight: backlight {
>+ compatible = "nvidia,tegra-pwm-backlight";
>+
>+ nvidia,pwm-source = <1>;
>+ nvidia,default-brightness = <0x8E>;
>+ };
>+
>+ /* PMIC has a built-
Re: ELCE 2023 - Prague
Do you guys wanna go to some pub today? On Fri, 16 Jun 2023 14:13:59 +0200 Michal Simek wrote: > Hi, > > I have had a chat with Marek that ELCE 2023 in Prague is coming. > It is good opportunity to talk to each other in person. > That's why we are interested to know who from U-Boot community is going. > > Me, Marek and Simon are going to be there. Who else is coming? > > Cheers, > Michal
[PATCH] bootstd: USB devtype detection for script boot
Change the device type from "usb_mass_storage" to "usb" when
booting a script.
Before this change:
=> printenv devtype
devtype=usb_mass_storage
After this change:
=> printenv devtype
devtype=usb
Signed-off-by: John Clark
---
boot/bootmeth_script.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/boot/bootmeth_script.c b/boot/bootmeth_script.c
index 225eb18ee6..9fdadb3005 100644
--- a/boot/bootmeth_script.c
+++ b/boot/bootmeth_script.c
@@ -187,10 +187,14 @@ static int script_set_bootflow(struct udevice *dev,
struct bootflow *bflow,
static int script_boot(struct udevice *dev, struct bootflow *bflow)
{
struct blk_desc *desc = dev_get_uclass_plat(bflow->blk);
+ const char *devtype = blk_get_devtype(bflow->blk);
ulong addr;
int ret;
- ret = env_set("devtype", blk_get_devtype(bflow->blk));
+ if (!strcmp("usb_mass_storage", devtype))
+ ret = env_set("devtype", "usb");
+ else
+ ret = env_set("devtype", devtype);
if (!ret)
ret = env_set_hex("devnum", desc->devnum);
if (!ret)
@@ -198,7 +202,7 @@ static int script_boot(struct udevice *dev, struct bootflow
*bflow)
if (!ret)
ret = env_set("prefix", bflow->subdir);
if (!ret && IS_ENABLED(CONFIG_ARCH_SUNXI) &&
- !strcmp("mmc", blk_get_devtype(bflow->blk)))
+ !strcmp("mmc", devtype))
ret = env_set_hex("mmc_bootdev", desc->devnum);
if (ret)
return log_msg_ret("env", ret);
--
2.39.2
Re: [PATCH 5/7] Makefile: Add a target for building capsules
Hi Sughosh, On Tue, 27 Jun 2023 at 13:08, Sughosh Ganu wrote: > > hi Simon, > > On Tue, 27 Jun 2023 at 16:50, Simon Glass wrote: > > > > Hi Sughosh, > > > > On Tue, 27 Jun 2023 at 05:57, Sughosh Ganu wrote: > > > > > > hi Simon, > > > > > > On Mon, 26 Jun 2023 at 17:43, Sughosh Ganu > > > wrote: > > > > > > > > hi Simon, > > > > > > > > On Mon, 26 Jun 2023 at 14:38, Simon Glass wrote: > > > > > > > > > > Hi Sughosh, > > > > > > > > > > On Wed, 21 Jun 2023 at 05:26, Sughosh Ganu > > > > > wrote: > > > > > > > > > > > > hi Simon, > > > > > > > > > > > > On Mon, 19 Jun 2023 at 18:07, Simon Glass wrote: > > > > > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > > > On Thu, 15 Jun 2023 at 17:25, Sughosh Ganu > > > > > > > wrote: > > > > > > > > > > > > > > > > hi Simon, > > > > > > > > > > > > > > > > On Thu, 15 Jun 2023 at 14:44, Simon Glass > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > > > > > > > On Tue, 13 Jun 2023 at 11:39, Sughosh Ganu > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > Add a target for building EFI capsules. The capsule > > > > > > > > > > parameters are > > > > > > > > > > specified through a config file, and the path to the config > > > > > > > > > > file is > > > > > > > > > > specified through CONFIG_EFI_CAPSULE_CFG_FILE. When the > > > > > > > > > > config file is > > > > > > > > > > not specified, the command only builds tools. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu > > > > > > > > > > --- > > > > > > > > > > Makefile | 9 + > > > > > > > > > > 1 file changed, 9 insertions(+) > > > > > > > > > > > > > > > > > > > > diff --git a/Makefile b/Makefile > > > > > > > > > > index 10bfaa52ad..96db29aa77 100644 > > > > > > > > > > --- a/Makefile > > > > > > > > > > +++ b/Makefile > > > > > > > > > > @@ -1151,6 +1151,15 @@ dtbs: dts/dt.dtb > > > > > > > > > > dts/dt.dtb: u-boot > > > > > > > > > > $(Q)$(MAKE) $(build)=dts dtbs > > > > > > > > > > > > > > > > > > > > +quiet_cmd_mkeficapsule = MKEFICAPSULE $@ > > > > > > > > > > +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule $@ > > > > > > > > > > + > > > > > > > > > > +PHONY += capsule > > > > > > > > > > +capsule: tools > > > > > > > > > > +ifneq ($(CONFIG_EFI_CAPSULE_CFG_FILE),"") > > > > > > > > > > + $(call cmd,mkeficapsule) > > > > > > > > > > +endif > > > > > > > > > > + > > > > > > > > > > quiet_cmd_copy = COPY$@ > > > > > > > > > >cmd_copy = cp $< $@ > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > 2.34.1 > > > > > > > > > > > > > > > > > > > > > > > > > > > > We should be using binman to build images...you seem to be > > > > > > > > > building > > > > > > > > > something in parallel with that. Can you please take a look > > > > > > > > > at binman? > > > > > > > > > > > > > > > > Again, I had explored using binman for this task. The one issue > > > > > > > > where > > > > > > > > I find the above flow better is that I can simply build my > > > > > > > > payload > > > > > > > > image(s) followed by 'make capsule' to generate the capsules for > > > > > > > > earlier generated images. In it's current form, I don't see an > > > > > > > > easy > > > > > > > > way to enforce this dependency in binman when I want to build > > > > > > > > the > > > > > > > > payload followed by generation of capsules. I did see the > > > > > > > > mention of > > > > > > > > encapsulating an entry within another dependent entry, but I > > > > > > > > think > > > > > > > > that makes the implementation more complex than it ought to be. > > > > > > > > > > > > > > > > I think it is much easier to use the make flow to generate the > > > > > > > > images > > > > > > > > followed by capsules, instead of tweaking the binman node to > > > > > > > > first > > > > > > > > generate the payload images, followed by enabling the capsule > > > > > > > > node to > > > > > > > > build the capsules. If there is an easy way of enforcing this > > > > > > > > dependency, please let me know. Thanks > > > > > > > > > > > > > > Can you share your explorations? I think the capsule should be > > > > > > > created > > > > > > > as part of the build, if enabled. Rather than changing the input > > > > > > > files, binman should produce new output files. > > > > > > > > > > > > This is an issue of handling dependencies in binman, and not > > > > > > changing > > > > > > input files. We do not have support for telling binman > > > > > > "build/generate > > > > > > this particular image first before you proceed to build the capsules > > > > > > using the earlier built images". I am not sure if this can be done > > > > > > in > > > > > > a generic manner in binman, so that irrespective of the image being > > > > > > generated, it can be specified to build capsules once the capsule > > > > > > input images have been generated. > > > > > > > > > > I'm just not sure what you are getting out here. > > > > > > > >
Re: [PATCH v3] dt-bindings: riscv: deprecate riscv,isa
On Mon, Jun 26, 2023, at 6:10 AM, Conor Dooley wrote: > intro > = > > When the RISC-V dt-bindings were accepted upstream in Linux, the base > ISA etc had yet to be ratified. By the ratification of the base ISA, > incompatible changes had snuck into the specifications - for example the > Zicsr and Zifencei extensions were spun out of the base ISA. > > Fast forward to today, and the reason for this patch. > Currently the riscv,isa dt property permits only a specific subset of > the ISA string - in particular it excludes version numbering. > With the current constraints, it is not possible to discern whether > "rv64i" means that the hart supports the fence.i instruction, for > example. > Future systems may choose to implement their own instruction fencing, > perhaps using a vendor extension, or they may not implement the optional > counter extensions. Software needs a way to determine this. > > versioning schemes > == > > "Use the extension versions that are described in the ISA manual" you > may say, and it's not like this has not been considered. > Firstly, software that parses the riscv,isa property at runtime will > need to contain a lookup table of some sort that maps arbitrary versions > to versions it understands. There is not a consistent application of > version number applied to extensions, with a higgledy-piggledy > collection of tags, "bare" and versioned documents awaiting the reader > on the "recently ratified extensions" page: > https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions > > As an aside, and this is reflected in the patch too, since many > extensions have yet to appear in a release of the ISA specs, > they are defined by commits in their respective "working draft" > repositories. > > Secondly, there is an issue of backwards compatibility, whereby allowing > numbers in the ISA string, some parsers may be broken. This would > require an additional property to be created to even use the versions in > this manner. > > ~boolean properties~ string array property > == > > If a new property is needed, the whole approach may as well be looked at > from the bottom up. A string with limited character choices etc is > hardly the best approach for communicating extension information to > software. > > Switching to using properties that are defined on a per extension basis, > allows us to define explicit meanings for the DT representation of each > extension - rather than the current situation where different operating > systems or other bits of software may impart different meanings to > characters in the string. > Clearly the best source of meanings is the specifications themselves, > this just provides us the ability to choose at what point in time the > meaning is set. If an extension changes incompatibility in the future, > a new property will be required. > > Off-list, some of the RVI folks have committed to shoring up the wording > in either the ISA specifications, the riscv-isa-manual or > so that in the future, modifications to and additions or removals of > features will require a new extension. Codifying that assertion > somewhere would make it quite unlikely that compatibility would be > broken, but we have the tools required to deal with it, if & when it > crops up. > It is in our collective interest, as consumers of extension meanings, to > define a scheme that enforces compatibility. > > The use of individual properties, rather than elements in a single no longer individual properties > string, will also permit validation that the properties have a meaning, > as well as potentially reject mutually exclusive combinations, or > enforce dependencies between extensions. That would not have be possible Under what circumstances is a device tree which declares support for a superset extension (e.g. m) required to also declare support for its subsets (e.g. zmmul)? There are compatibility issues in both directions. Proposal: If an extension X is a superset of an extension Y and X is present in riscv,isa-extensions, Y must also be present if Y was ratified or added to the schema before X, but need not also be present if Y was ratified after or at the same time as X. If X "depends on" Y, then Y must be present in riscv,isa-extensions even if X and Y were ratified at the same time. > with the current dt-schema infrastructure for arbitrary strings, as we > would need to add a riscv,isa parser to dt-validate! > That's not implemented in this patch, but rather left as future work (for > the brave, or the foolish). > > acpi > > > The current ACPI ECR is based on having a single ISA string unfortunately, > but ideally ACPI will move to another method, perhaps GUIDs, that give > explicit meaning to extensions. > > parser simplicity > = > > Many systems that parse DT at runtime already implement an function that > can check for the presence of a string in an array of string, as it
Re: [PATCH v3] dt-bindings: riscv: deprecate riscv,isa
On Mon, 26 Jun 2023 10:38:43 PDT (-0700), apa...@ventanamicro.com wrote: On Mon, Jun 26, 2023 at 3:42 PM Conor Dooley wrote: intro = When the RISC-V dt-bindings were accepted upstream in Linux, the base ISA etc had yet to be ratified. By the ratification of the base ISA, incompatible changes had snuck into the specifications - for example the Zicsr and Zifencei extensions were spun out of the base ISA. Fast forward to today, and the reason for this patch. Currently the riscv,isa dt property permits only a specific subset of the ISA string - in particular it excludes version numbering. With the current constraints, it is not possible to discern whether "rv64i" means that the hart supports the fence.i instruction, for example. Future systems may choose to implement their own instruction fencing, perhaps using a vendor extension, or they may not implement the optional counter extensions. Software needs a way to determine this. versioning schemes == "Use the extension versions that are described in the ISA manual" you may say, and it's not like this has not been considered. Firstly, software that parses the riscv,isa property at runtime will need to contain a lookup table of some sort that maps arbitrary versions to versions it understands. There is not a consistent application of version number applied to extensions, with a higgledy-piggledy collection of tags, "bare" and versioned documents awaiting the reader on the "recently ratified extensions" page: https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions As an aside, and this is reflected in the patch too, since many extensions have yet to appear in a release of the ISA specs, they are defined by commits in their respective "working draft" repositories. Secondly, there is an issue of backwards compatibility, whereby allowing numbers in the ISA string, some parsers may be broken. This would require an additional property to be created to even use the versions in this manner. ~boolean properties~ string array property == If a new property is needed, the whole approach may as well be looked at from the bottom up. A string with limited character choices etc is hardly the best approach for communicating extension information to software. Switching to using properties that are defined on a per extension basis, allows us to define explicit meanings for the DT representation of each extension - rather than the current situation where different operating systems or other bits of software may impart different meanings to characters in the string. Clearly the best source of meanings is the specifications themselves, this just provides us the ability to choose at what point in time the meaning is set. If an extension changes incompatibility in the future, a new property will be required. Off-list, some of the RVI folks have committed to shoring up the wording in either the ISA specifications, the riscv-isa-manual or so that in the future, modifications to and additions or removals of features will require a new extension. Codifying that assertion somewhere would make it quite unlikely that compatibility would be broken, but we have the tools required to deal with it, if & when it crops up. It is in our collective interest, as consumers of extension meanings, to define a scheme that enforces compatibility. The use of individual properties, rather than elements in a single string, will also permit validation that the properties have a meaning, as well as potentially reject mutually exclusive combinations, or enforce dependencies between extensions. That would not have be possible with the current dt-schema infrastructure for arbitrary strings, as we would need to add a riscv,isa parser to dt-validate! That's not implemented in this patch, but rather left as future work (for the brave, or the foolish). acpi The current ACPI ECR is based on having a single ISA string unfortunately, but ideally ACPI will move to another method, perhaps GUIDs, that give explicit meaning to extensions. Drop this paragraph on ACPI. We clearly mentioned previously that ACPI will follow specs defined by RVI. There are scalability issues in using GUIDs for each ISA extension. Which spec are we following for the ACPI ISA string? Regards, Anup parser simplicity = Many systems that parse DT at runtime already implement an function that can check for the presence of a string in an array of string, as it is similar to the process for parsing a list of compatible strings, so a bunch of new, custom, DT parsing should not be needed. Getting rid of "riscv,isa" parsing would be a nice simplification, but unfortunately for backwards compatibility with old dtbs, existing parsers may not be removable - which may greatly simplify dt parsing code. In Linux, for example, checking for whether a hart supports an extension becomes as simple as: of_property
Re: CFP open for RISC-V MC at Linux Plumbers Conference 2023
On Mon, Jun 19, 2023 at 12:55:39PM -0700, Atish Patra wrote: > The CFP for topic proposals for the RISC-V micro conference[1] 2023 is open > now. > Please submit your proposal before it's too late! Thanks for posting. I am looking forward to it! > The Linux plumbers event will be both in person and remote > (hybrid)virtual this year. More details can be found here [2]. > > We will start accepting proposals after 15th August 2023 to avoid last > minute visa/travel hassles. > The conference registration sells out pretty quickly too. Thus, we > would prefer to sort out the schedule sooner than later. > > The final deadline is 15th October 2023 though. > > [1] https://lpc.events/event/16/contributions/1148/ I think that page is from 2022 and that this would be a better link: https://lpc.events/blog/current/index.php/2023/06/23/risc-v-microconference-cfp/ Drew
[PATCH] spl: Add function called after fpga image upload
From: Christian Taedcke
This way custom logic can be implemented per board after the fpga
image is uploaded.
Signed-off-by: Christian Taedcke
---
common/spl/spl_fit.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 730639f756..3a1e3382ba 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -560,6 +560,16 @@ __weak void *spl_load_simple_fit_fix_load(const void *fit)
return (void *)fit;
}
+/*
+ * Weak default function to allow implementing logic after fpga image is
+ * uploaded.
+ */
+__weak void board_spl_fit_post_upload_fpga(const void *fit, int node,
+ struct spl_image_info *fpga_image,
+ int ret)
+{
+}
+
static void warn_deprecated(const char *msg)
{
printf("DEPRECATED: %s\n", msg);
@@ -590,6 +600,9 @@ static int spl_fit_upload_fpga(struct spl_fit_info *ctx,
int node,
ret = fpga_load(devnum, (void *)fpga_image->load_addr,
fpga_image->size, BIT_FULL, flags);
+
+ board_spl_fit_post_upload_fpga(ctx->fit, node, fpga_image, ret);
+
if (ret) {
printf("%s: Cannot load the image to the FPGA\n", __func__);
return ret;
--
2.34.1
[PATCH] xilinx: zynqmp: Extract aes operation into new file
From: Christian Taedcke
This moves the aes operation that is perfomed by the pmu into a
separate file. This way it can be called not just from the shell
command, but also e.g. from board initialization code.
Signed-off-by: Christian Taedcke
---
arch/arm/mach-zynqmp/Makefile | 3 +-
arch/arm/mach-zynqmp/aes.c| 58 +++
.../arm/mach-zynqmp/include/mach/zynqmp_aes.h | 31 ++
board/xilinx/zynqmp/cmds.c| 44 ++
4 files changed, 96 insertions(+), 40 deletions(-)
create mode 100644 arch/arm/mach-zynqmp/aes.c
create mode 100644 arch/arm/mach-zynqmp/include/mach/zynqmp_aes.h
diff --git a/arch/arm/mach-zynqmp/Makefile b/arch/arm/mach-zynqmp/Makefile
index bb1830c846..1a76493bef 100644
--- a/arch/arm/mach-zynqmp/Makefile
+++ b/arch/arm/mach-zynqmp/Makefile
@@ -3,8 +3,7 @@
# (C) Copyright 2014 - 2015 Xilinx, Inc.
# Michal Simek
-obj-y += clk.o
-obj-y += cpu.o
+obj-y += aes.o clk.o cpu.o
obj-$(CONFIG_MP) += mp.o
obj-$(CONFIG_SPL_BUILD) += spl.o handoff.o psu_spl_init.o
obj-$(CONFIG_SPL_ZYNQMP_DRAM_ECC_INIT) += ecc_spl_init.o
diff --git a/arch/arm/mach-zynqmp/aes.c b/arch/arm/mach-zynqmp/aes.c
new file mode 100644
index 00..f508862e57
--- /dev/null
+++ b/arch/arm/mach-zynqmp/aes.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * (C) Copyright 2018 Xilinx, Inc.
+ * Siva Durga Prasad Paladugu
+ *
+ * Copyright (C) 2023 Weidmueller Interface GmbH & Co. KG
+ * Christian Taedcke
+ */
+
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+int zynqmp_aes_operation(struct zynqmp_aes *aes)
+{
+ int ret;
+ u32 ret_payload[PAYLOAD_ARG_CNT];
+
+ if (zynqmp_firmware_version() <= PMUFW_V1_0)
+ return -ENOENT;
+
+ if (aes->srcaddr && aes->ivaddr && aes->dstaddr) {
+ flush_dcache_range(aes->srcaddr,
+ (aes->srcaddr +
+ roundup(aes->len, ARCH_DMA_MINALIGN)));
+ flush_dcache_range(aes->ivaddr,
+ (aes->ivaddr +
+ roundup(IV_SIZE, ARCH_DMA_MINALIGN)));
+ flush_dcache_range(aes->dstaddr,
+ (aes->dstaddr +
+ roundup(aes->len, ARCH_DMA_MINALIGN)));
+ }
+
+ if (aes->keysrc == 0) {
+ if (aes->keyaddr == 0)
+ return -EINVAL;
+
+ flush_dcache_range(aes->keyaddr, (aes->keyaddr +
+ roundup(KEY_PTR_LEN,
ARCH_DMA_MINALIGN)));
+ }
+
+ flush_dcache_range((ulong)aes, (ulong)(aes) +
+ roundup(sizeof(struct zynqmp_aes),
ARCH_DMA_MINALIGN));
+
+ ret = xilinx_pm_request(PM_SECURE_AES, upper_32_bits((ulong)aes),
+ lower_32_bits((ulong)aes), 0, 0, ret_payload);
+ if (ret || ret_payload[1]) {
+ printf("Failed: AES op status:0x%x, errcode:0x%x\n",
+ ret, ret_payload[1]);
+ return -EIO;
+ }
+
+ return 0;
+}
diff --git a/arch/arm/mach-zynqmp/include/mach/zynqmp_aes.h
b/arch/arm/mach-zynqmp/include/mach/zynqmp_aes.h
new file mode 100644
index 00..7c28e377a1
--- /dev/null
+++ b/arch/arm/mach-zynqmp/include/mach/zynqmp_aes.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2023 Weidmueller Interface GmbH & Co. KG
+ * Christian Taedcke
+ *
+ * Declaration of AES operation functionality for ZynqMP.
+ */
+
+#ifndef ZYNQMP_AES_H
+#define ZYNQMP_AES_H
+
+struct zynqmp_aes {
+ u64 srcaddr;
+ u64 ivaddr;
+ u64 keyaddr;
+ u64 dstaddr;
+ u64 len;
+ u64 op;
+ u64 keysrc;
+};
+
+/*
+ * Performs an aes operation using the pmu firmware
+ *
+ * @param aes The aes operation buffer that must have been allocated using
+ *ALLOC_CACHE_ALIGN_BUFFER(struct zynqmp_aes, aes, 1).
+ * @return 0 in case of success, in case of an errer any other value.
+ */
+int zynqmp_aes_operation(struct zynqmp_aes *aes);
+
+#endif /* ZYNQMP_AES_H */
diff --git a/board/xilinx/zynqmp/cmds.c b/board/xilinx/zynqmp/cmds.c
index e20030ecda..76be17e7ae 100644
--- a/board/xilinx/zynqmp/cmds.c
+++ b/board/xilinx/zynqmp/cmds.c
@@ -14,16 +14,7 @@
#include
#include
#include
-
-struct aes {
- u64 srcaddr;
- u64 ivaddr;
- u64 keyaddr;
- u64 dstaddr;
- u64 len;
- u64 op;
- u64 keysrc;
-};
+#include
static int do_zynqmp_verify_secure(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
@@ -121,9 +112,8 @@ static int do_zynqmp_mmio_write(struct cmd_tbl *cmdtp, int
flag, int argc,
static int do_zynqmp_aes(struct cmd_tbl *cmdtp, int flag, int argc,
char * const argv[])
{
- ALLOC_CACHE_ALIGN_BUFFER(struct aes, aes, 1);
+
[PATCH 3/3] binman: Add tests for etype encrypted
From: Christian Taedcke
Add tests to reach 100% code coverage for the added etype encrypted.
Signed-off-by: Christian Taedcke
---
tools/binman/ftest.py | 69 +++
.../binman/test/282_encrypted_no_content.dts | 15
tools/binman/test/283_encrypted_no_algo.dts | 19 +
.../test/284_encrypted_invalid_iv_file.dts| 22 ++
tools/binman/test/285_encrypted.dts | 29
tools/binman/test/286_encrypted_key_file.dts | 30
.../test/287_encrypted_iv_name_hint.dts | 30
7 files changed, 214 insertions(+)
create mode 100644 tools/binman/test/282_encrypted_no_content.dts
create mode 100644 tools/binman/test/283_encrypted_no_algo.dts
create mode 100644 tools/binman/test/284_encrypted_invalid_iv_file.dts
create mode 100644 tools/binman/test/285_encrypted.dts
create mode 100644 tools/binman/test/286_encrypted_key_file.dts
create mode 100644 tools/binman/test/287_encrypted_iv_name_hint.dts
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 43b4f850a6..3fb57e964e 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -94,6 +94,8 @@ ROCKCHIP_TPL_DATA = b'rockchip-tpl'
TEST_FDT1_DATA= b'fdt1'
TEST_FDT2_DATA= b'test-fdt2'
ENV_DATA = b'var1=1\nvar2="2"'
+ENCRYPTED_IV_DATA = b'123456'
+ENCRYPTED_KEY_DATA= b'1234567890123456'
PRE_LOAD_MAGIC= b'UBSH'
PRE_LOAD_VERSION = 0x11223344.to_bytes(4, 'big')
PRE_LOAD_HDR_SIZE = 0x1000.to_bytes(4, 'big')
@@ -226,6 +228,10 @@ class TestFunctional(unittest.TestCase):
# Newer OP_TEE file in v1 binary format
cls.make_tee_bin('tee.bin')
+# test files for encrypted tests
+TestFunctional._MakeInputFile('encrypted-file.iv', ENCRYPTED_IV_DATA)
+TestFunctional._MakeInputFile('encrypted-file.key', ENCRYPTED_KEY_DATA)
+
cls.comp_bintools = {}
for name in COMP_BINTOOLS:
cls.comp_bintools[name] = bintool.Bintool.create(name)
@@ -6676,6 +6682,69 @@ fdt fdtmapExtract the devicetree
blob from the fdtmap
['fit'])
self.assertIn("Node '/fit': Missing tool: 'mkimage'", str(e.exception))
+def testEncryptedNoContent(self):
+with self.assertRaises(ValueError) as e:
+self._DoReadFileDtb('282_encrypted_no_content.dts',
update_dtb=True)
+self.assertIn("Node \'/binman/fit/images/u-boot/encrypted\':
Collection must have a 'content' property", str(e.exception))
+
+def testEncryptedNoAlgo(self):
+with self.assertRaises(ValueError) as e:
+self._DoReadFileDtb('283_encrypted_no_algo.dts', update_dtb=True)
+self.assertIn("Node \'/binman/fit/images/u-boot/encrypted\':
'encrypted' entry is missing properties: algo key-name-hint iv-filename",
str(e.exception))
+
+def testEncryptedInvalidIvfile(self):
+with self.assertRaises(ValueError) as e:
+self._DoReadFileDtb('284_encrypted_invalid_iv_file.dts',
update_dtb=True)
+self.assertIn("Filename 'invalid-iv-file' not found in input path",
+ str(e.exception))
+
+def testEncryptedNoKey(self):
+data = self._DoReadFileDtb('285_encrypted.dts')[0]
+
+dtb = fdt.Fdt.FromData(data)
+dtb.Scan()
+
+node = dtb.GetNode('/images/u-boot/cipher')
+self.assertEqual('algo-name', node.props['algo'].value)
+self.assertEqual('key-name-hint-value',
node.props['key-name-hint'].value)
+self.assertEqual(ENCRYPTED_IV_DATA,
tools.to_bytes(''.join(node.props['iv'].value)))
+self.assertNotIn('iv-name-hint', node.props)
+
+node = dtb.GetNode('/cipher')
+self.assertIsNone(node)
+
+def testEncryptedKeyFile(self):
+data = self._DoReadFileDtb('286_encrypted_key_file.dts')[0]
+
+dtb = fdt.Fdt.FromData(data)
+dtb.Scan()
+
+node = dtb.GetNode('/images/u-boot/cipher')
+self.assertEqual('algo-name', node.props['algo'].value)
+self.assertEqual('key-name-hint-value',
node.props['key-name-hint'].value)
+self.assertEqual(ENCRYPTED_IV_DATA,
tools.to_bytes(''.join(node.props['iv'].value)))
+self.assertNotIn('iv-name-hint', node.props)
+
+node = dtb.GetNode('/cipher/key-algo-name-key-name-hint-value')
+self.assertEqual(ENCRYPTED_KEY_DATA, b''.join(node.props['key'].value))
+self.assertNotIn('iv', node.props)
+
+def testEncryptedIvNameHint(self):
+data = self._DoReadFileDtb('287_encrypted_iv_name_hint.dts')[0]
+
+dtb = fdt.Fdt.FromData(data)
+dtb.Scan()
+
+node = dtb.GetNode('/images/u-boot/cipher')
+self.assertEqual('algo-name', node.props['algo'].value)
+self.assertEqual('iv-name-hint-value',
node.props['iv-name-hint'].value)
+self.assertEqual('key-name-hint-value',
node.props['key-name-hint'].value)
+self.assertNotIn('iv', n
[PATCH 2/3] binman: Allow cipher node as special section
From: Christian Taedcke
The new encrypted etype generates a cipher node in the device tree
that should not be evaluated by binman, but still be kept in the
output device tree.
Signed-off-by: Christian Taedcke
---
tools/binman/etype/section.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index c36edd1350..56abfd5129 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -178,7 +178,7 @@ class Entry_section(Entry):
Returns:
bool: True if the node is a special one, else False
"""
-return node.name.startswith('hash') or
node.name.startswith('signature')
+return node.name.startswith('hash') or
node.name.startswith('signature') or node.name.startswith('cipher')
def ReadNode(self):
"""Read properties from the section node"""
--
2.34.1
[PATCH 1/3] binman: Add support for externally encrypted blobs
From: Christian Taedcke
This adds a new etype encrypted that is derived from collection.
It creates a new cipher node in the related image similar to the
cipher node used by u-boot, see boot/image-cipher.c.
Optionally it creates a global /cipher node containing a key and iv
using the same nameing convention as used in boot/image-cipher.c.
Signed-off-by: Christian Taedcke
---
tools/binman/etype/encrypted.py | 98 +
1 file changed, 98 insertions(+)
create mode 100644 tools/binman/etype/encrypted.py
diff --git a/tools/binman/etype/encrypted.py b/tools/binman/etype/encrypted.py
new file mode 100644
index 00..005ae56acf
--- /dev/null
+++ b/tools/binman/etype/encrypted.py
@@ -0,0 +1,98 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright 2023 Weidmüller Interface GmbH & Co. KG
+# Written by Christian Taedcke
+#
+# Entry-type module for cipher information of encrypted blobs/images
+#
+
+from binman.etype.collection import Entry_collection
+from dtoc import fdt_util
+from u_boot_pylib import tools
+
+# This is imported if needed
+state = None
+
+
+class Entry_encrypted(Entry_collection):
+"""Externally built encrypted binary blob
+
+If the file providing this blob is missing, binman can optionally ignore it
+and produce a broken image with a warning.
+
+In addition to the inherited 'collection' for Properties / Entry arguments:
+- algo: The encryption algorithm
+- iv-name-hint: The name hint for the iv
+- key-name-hint: The name hint for the key
+- iv-filename: The name of the file containing the iv
+- key-filename: The name of the file containing the key
+
+This entry creates a cipher node in the entries' parent node (i.e. the
+image). Optionally it also creates a cipher node in the root of the device
+tree containg key and iv information.
+"""
+
+def __init__(self, section, etype, node):
+# Put this here to allow entry-docs and help to work without libfdt
+global state
+from binman import state
+
+super().__init__(section, etype, node)
+# The property key-filename is not required, because some
implementations use keys that
+# are not embedded in the device tree, but e.g. in the device itself
+self.required_props = ['algo', 'key-name-hint', 'iv-filename']
+self._algo = fdt_util.GetString(self._node, 'algo')
+self._iv_name_hint = fdt_util.GetString(self._node, 'iv-name-hint')
+self._key_name_hint = fdt_util.GetString(self._node, 'key-name-hint')
+self._filename_iv = fdt_util.GetString(self._node, 'iv-filename')
+self._filename_key = fdt_util.GetString(self._node, 'key-filename')
+
+def ReadNode(self):
+super().ReadNode()
+
+iv_filename = tools.get_input_filename(self._filename_iv)
+self._iv = tools.read_file(iv_filename, binary=True)
+
+self._key = None
+if self._filename_key:
+key_filename = tools.get_input_filename(self._filename_key)
+self._key = tools.read_file(key_filename, binary=True)
+
+def gen_entries(self):
+super().gen_entries()
+
+cipher_node = state.AddSubnode(self._node.parent, "cipher")
+cipher_node.AddString("algo", self._algo)
+cipher_node.AddString("key-name-hint", self._key_name_hint)
+if self._iv_name_hint:
+cipher_node.AddString("iv-name-hint", self._iv_name_hint)
+else:
+cipher_node.AddData("iv", self._iv)
+
+if self._key or self._iv_name_hint:
+# add cipher node in root
+root_node = self._node.parent.parent.parent
+name = "cipher"
+cipher_node = root_node.FindNode(name)
+if not cipher_node:
+cipher_node = state.AddSubnode(root_node, name)
+key_node_name = (
+f"key-{self._algo}-{self._key_name_hint}-{self._iv_name_hint}"
+if self._iv_name_hint
+else f"key-{self._algo}-{self._key_name_hint}"
+)
+key_node = cipher_node.FindNode(key_node_name)
+if not key_node:
+key_node = state.AddSubnode(cipher_node, key_node_name)
+if self._key:
+key_node.AddData("key", self._key)
+if self._iv_name_hint:
+key_node.AddData("iv", self._iv)
+
+def ObtainContents(self):
+# ensure that linked content is not added to the device tree again
from this entry
+self.SetContents(b'')
+return True
+
+def ProcessContents(self):
+# ensure that linked content is not added to the device tree again
from this entry
+return self.ProcessContentsUpdate(b'')
--
2.34.1
[PATCH 0/3] binman: Add support for externally encrypted blobs
From: Christian Taedcke
This series adds the functionality to handle externally encrypted
blobs to binman. It includes the functionality itself and the
corresponding unit tests. The generated device tree structure is
similar to the structure used in the already implemented cipher node
in boot/image-cipher.c.
The following block shows an example on how to use this functionality.
In the device tree that is parsed by binman a new node encrypted is
used:
/ {
binman {
filename = "u-boot.itb";
fit {
...
images {
some-bitstream {
...
image_bitstream: blob-ext {
filename = "bitstream.bin";
};
encrypted {
content = <&image_bitstream>;
algo = "aes256-gcm";
key-name-hint = "keyname";
iv-filename =
"bitstream.bin.iv";
key-filename =
"bitstream.bin.key";
};
...
This results in an generated fit image containing the following
information:
\ {
cipher {
key-aes256-gcm-keyname {
key = <0x...>;
iv = <0x...>;
};
};
images {
...
some-bitstream {
...
data = [...]
cipher {
algo = "aes256-gcm";
key-name-hint = "keyname";
};
};
...
Christian Taedcke (3):
binman: Add support for externally encrypted blobs
binman: Allow cipher node as special section
binman: Add tests for etype encrypted
tools/binman/etype/encrypted.py | 98 +++
tools/binman/etype/section.py | 2 +-
tools/binman/ftest.py | 69 +
.../binman/test/282_encrypted_no_content.dts | 15 +++
tools/binman/test/283_encrypted_no_algo.dts | 19
.../test/284_encrypted_invalid_iv_file.dts| 22 +
tools/binman/test/285_encrypted.dts | 29 ++
tools/binman/test/286_encrypted_key_file.dts | 30 ++
.../test/287_encrypted_iv_name_hint.dts | 30 ++
9 files changed, 313 insertions(+), 1 deletion(-)
create mode 100644 tools/binman/etype/encrypted.py
create mode 100644 tools/binman/test/282_encrypted_no_content.dts
create mode 100644 tools/binman/test/283_encrypted_no_algo.dts
create mode 100644 tools/binman/test/284_encrypted_invalid_iv_file.dts
create mode 100644 tools/binman/test/285_encrypted.dts
create mode 100644 tools/binman/test/286_encrypted_key_file.dts
create mode 100644 tools/binman/test/287_encrypted_iv_name_hint.dts
--
2.34.1
Re: [PATCH 5/7] Makefile: Add a target for building capsules
hi Simon, On Tue, 27 Jun 2023 at 16:50, Simon Glass wrote: > > Hi Sughosh, > > On Tue, 27 Jun 2023 at 05:57, Sughosh Ganu wrote: > > > > hi Simon, > > > > On Mon, 26 Jun 2023 at 17:43, Sughosh Ganu wrote: > > > > > > hi Simon, > > > > > > On Mon, 26 Jun 2023 at 14:38, Simon Glass wrote: > > > > > > > > Hi Sughosh, > > > > > > > > On Wed, 21 Jun 2023 at 05:26, Sughosh Ganu > > > > wrote: > > > > > > > > > > hi Simon, > > > > > > > > > > On Mon, 19 Jun 2023 at 18:07, Simon Glass wrote: > > > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > On Thu, 15 Jun 2023 at 17:25, Sughosh Ganu > > > > > > wrote: > > > > > > > > > > > > > > hi Simon, > > > > > > > > > > > > > > On Thu, 15 Jun 2023 at 14:44, Simon Glass > > > > > > > wrote: > > > > > > > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > > > > > On Tue, 13 Jun 2023 at 11:39, Sughosh Ganu > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > Add a target for building EFI capsules. The capsule > > > > > > > > > parameters are > > > > > > > > > specified through a config file, and the path to the config > > > > > > > > > file is > > > > > > > > > specified through CONFIG_EFI_CAPSULE_CFG_FILE. When the > > > > > > > > > config file is > > > > > > > > > not specified, the command only builds tools. > > > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu > > > > > > > > > --- > > > > > > > > > Makefile | 9 + > > > > > > > > > 1 file changed, 9 insertions(+) > > > > > > > > > > > > > > > > > > diff --git a/Makefile b/Makefile > > > > > > > > > index 10bfaa52ad..96db29aa77 100644 > > > > > > > > > --- a/Makefile > > > > > > > > > +++ b/Makefile > > > > > > > > > @@ -1151,6 +1151,15 @@ dtbs: dts/dt.dtb > > > > > > > > > dts/dt.dtb: u-boot > > > > > > > > > $(Q)$(MAKE) $(build)=dts dtbs > > > > > > > > > > > > > > > > > > +quiet_cmd_mkeficapsule = MKEFICAPSULE $@ > > > > > > > > > +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule $@ > > > > > > > > > + > > > > > > > > > +PHONY += capsule > > > > > > > > > +capsule: tools > > > > > > > > > +ifneq ($(CONFIG_EFI_CAPSULE_CFG_FILE),"") > > > > > > > > > + $(call cmd,mkeficapsule) > > > > > > > > > +endif > > > > > > > > > + > > > > > > > > > quiet_cmd_copy = COPY$@ > > > > > > > > >cmd_copy = cp $< $@ > > > > > > > > > > > > > > > > > > -- > > > > > > > > > 2.34.1 > > > > > > > > > > > > > > > > > > > > > > > > > We should be using binman to build images...you seem to be > > > > > > > > building > > > > > > > > something in parallel with that. Can you please take a look at > > > > > > > > binman? > > > > > > > > > > > > > > Again, I had explored using binman for this task. The one issue > > > > > > > where > > > > > > > I find the above flow better is that I can simply build my payload > > > > > > > image(s) followed by 'make capsule' to generate the capsules for > > > > > > > earlier generated images. In it's current form, I don't see an > > > > > > > easy > > > > > > > way to enforce this dependency in binman when I want to build the > > > > > > > payload followed by generation of capsules. I did see the mention > > > > > > > of > > > > > > > encapsulating an entry within another dependent entry, but I think > > > > > > > that makes the implementation more complex than it ought to be. > > > > > > > > > > > > > > I think it is much easier to use the make flow to generate the > > > > > > > images > > > > > > > followed by capsules, instead of tweaking the binman node to first > > > > > > > generate the payload images, followed by enabling the capsule > > > > > > > node to > > > > > > > build the capsules. If there is an easy way of enforcing this > > > > > > > dependency, please let me know. Thanks > > > > > > > > > > > > Can you share your explorations? I think the capsule should be > > > > > > created > > > > > > as part of the build, if enabled. Rather than changing the input > > > > > > files, binman should produce new output files. > > > > > > > > > > This is an issue of handling dependencies in binman, and not changing > > > > > input files. We do not have support for telling binman "build/generate > > > > > this particular image first before you proceed to build the capsules > > > > > using the earlier built images". I am not sure if this can be done in > > > > > a generic manner in binman, so that irrespective of the image being > > > > > generated, it can be specified to build capsules once the capsule > > > > > input images have been generated. > > > > > > > > I'm just not sure what you are getting out here. > > > > > > > > See INPUTS-y for the input files to binman. Then binman uses these to > > > > generate output files. It does not mess with the input files, nor > > > > should it. Please read the top part of the Binman motivation to > > > > understand how all this works. > > > > > > > > At the risk of repeating myself, we want the Makefile to focus on > > > > building U-Boot, with Binman handling the laterprocessing of thos
Re: [PATCH v3] dt-bindings: riscv: deprecate riscv,isa
Hey Atish, Stefan, On Mon, Jun 26, 2023 at 11:35:10PM -0700, Atish Patra wrote: > On Mon, Jun 26, 2023 at 5:40 PM Stefan O'Rear wrote: > > On Mon, Jun 26, 2023, at 6:10 AM, Conor Dooley wrote: > > > Off-list, some of the RVI folks have committed to shoring up the wording > > > in either the ISA specifications, the riscv-isa-manual or > > > so that in the future, modifications to and additions or removals of > > > features will require a new extension. Codifying that assertion > > > somewhere would make it quite unlikely that compatibility would be > > > broken, but we have the tools required to deal with it, if & when it > > > crops up. > > > It is in our collective interest, as consumers of extension meanings, to > > > define a scheme that enforces compatibility. > > > > > > The use of individual properties, rather than elements in a single > > > > no longer individual properties > > > > > string, will also permit validation that the properties have a meaning, > > > as well as potentially reject mutually exclusive combinations, or > > > enforce dependencies between extensions. That would not have be possible > > > > Under what circumstances is a device tree which declares support for a > > superset extension (e.g. m) required to also declare support for its subsets > > (e.g. zmmul)? There are compatibility issues in both directions. > > > > Proposal: If an extension X is a superset of an extension Y and X is present > > in riscv,isa-extensions, Y must also be present if Y was ratified or added > > to the schema before X, but need not also be present if Y was ratified after > > or at the same time as X. If X "depends on" Y, then Y must be present in > > riscv,isa-extensions even if X and Y were ratified at the same time. Yes, I think that this all makes sense from a compatibility point of view. Splitting it up: > > If an extension X is a superset of an extension Y and X is present > > in riscv,isa-extensions, Y must also be present if Y was ratified or added > > to the schema before X This makes total sense from a "being nice to" software point of view. > > but need not also be present if Y was ratified after > > or at the same time as X. It may make sense to reduce this to only after, or not permit the supersets at all where they are ratified alongside their subsets. Cross that bridge when we come to it perhaps. > > If X "depends on" Y, then Y must be present in > > riscv,isa-extensions even if X and Y were ratified at the same tim For Linux, this is already the case for F & D. I think that's a good policy to follow. > > > > > > vendor extensions > > > = > > > > > > Compared to riscv,isa, this proposed scheme promotes vendor extensions, > > > oft touted as the strength of RISC-V, to first-class citizens. > > > At present, extensions are defined as meaning what the RISC-V ISA > > > specifications say they do. There is no realistic way of using that > > > interface to provide cross-platform definitions for what vendor > > > extensions mean. Vendor extensions may also have even less consistency > > > than RVI do in terms of versioning, or no care about backwards > > > compatibility. > > > The new property allows us to assign explicit meanings on a per vendor > > > extension basis, backed up by a description of their meanings. > > > > How are vendor extension names allocated? Will any proposed name for a > > vendor extension pass through linux-riscv@ before it shows up in the wild, > > or are vendors expected to allocate extension names unilaterally? The same way any other dt-binding works, it's no different in that respect to compatible strings. > > Is it > > worth creating an experimental-* namespace for prototype implementations > > of unreleased extensions? IMO, people are free to do whatever they like in their own development trees. I don't really know why we'd introduce stuff in dt-bindings for things that are only in an experimental state. > > > + riscv,isa-extensions: > > > +$ref: /schemas/types.yaml#/definitions/string-array > > > +minItems: 1 > > > +description: Extensions supported by the hart. > > > +items: > > > + anyOf: > > > +# single letter extensions, in canonical order > > > +- const: i > > > + description: | > > > +The base integer instruction set, as ratified in the > > > 20191213 > > > +version of the unprivileged ISA specification, with the > > > exception of > > > +counter access. > > > +Counter access was removed after the ratification of the > > > 20191213 > > > +version of the unprivileged specification and shunted into > > > the > > > +Zicntr and Zihpm extensions. > > > > I think this may belong in the description of zicsr? rdcycle in 20191213 > > is a special case of csrrs, which is in zicsr not the base. Sorry, this is a bit unclear. Do you mean that the sentence you have provided here should be in the Zicsr entry? > > > +
Re: [PATCH 5/7] Makefile: Add a target for building capsules
Hi Sughosh, On Tue, 27 Jun 2023 at 05:57, Sughosh Ganu wrote: > > hi Simon, > > On Mon, 26 Jun 2023 at 17:43, Sughosh Ganu wrote: > > > > hi Simon, > > > > On Mon, 26 Jun 2023 at 14:38, Simon Glass wrote: > > > > > > Hi Sughosh, > > > > > > On Wed, 21 Jun 2023 at 05:26, Sughosh Ganu > > > wrote: > > > > > > > > hi Simon, > > > > > > > > On Mon, 19 Jun 2023 at 18:07, Simon Glass wrote: > > > > > > > > > > Hi Sughosh, > > > > > > > > > > On Thu, 15 Jun 2023 at 17:25, Sughosh Ganu > > > > > wrote: > > > > > > > > > > > > hi Simon, > > > > > > > > > > > > On Thu, 15 Jun 2023 at 14:44, Simon Glass wrote: > > > > > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > > > On Tue, 13 Jun 2023 at 11:39, Sughosh Ganu > > > > > > > wrote: > > > > > > > > > > > > > > > > Add a target for building EFI capsules. The capsule parameters > > > > > > > > are > > > > > > > > specified through a config file, and the path to the config > > > > > > > > file is > > > > > > > > specified through CONFIG_EFI_CAPSULE_CFG_FILE. When the config > > > > > > > > file is > > > > > > > > not specified, the command only builds tools. > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu > > > > > > > > --- > > > > > > > > Makefile | 9 + > > > > > > > > 1 file changed, 9 insertions(+) > > > > > > > > > > > > > > > > diff --git a/Makefile b/Makefile > > > > > > > > index 10bfaa52ad..96db29aa77 100644 > > > > > > > > --- a/Makefile > > > > > > > > +++ b/Makefile > > > > > > > > @@ -1151,6 +1151,15 @@ dtbs: dts/dt.dtb > > > > > > > > dts/dt.dtb: u-boot > > > > > > > > $(Q)$(MAKE) $(build)=dts dtbs > > > > > > > > > > > > > > > > +quiet_cmd_mkeficapsule = MKEFICAPSULE $@ > > > > > > > > +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule $@ > > > > > > > > + > > > > > > > > +PHONY += capsule > > > > > > > > +capsule: tools > > > > > > > > +ifneq ($(CONFIG_EFI_CAPSULE_CFG_FILE),"") > > > > > > > > + $(call cmd,mkeficapsule) > > > > > > > > +endif > > > > > > > > + > > > > > > > > quiet_cmd_copy = COPY$@ > > > > > > > >cmd_copy = cp $< $@ > > > > > > > > > > > > > > > > -- > > > > > > > > 2.34.1 > > > > > > > > > > > > > > > > > > > > > > We should be using binman to build images...you seem to be > > > > > > > building > > > > > > > something in parallel with that. Can you please take a look at > > > > > > > binman? > > > > > > > > > > > > Again, I had explored using binman for this task. The one issue > > > > > > where > > > > > > I find the above flow better is that I can simply build my payload > > > > > > image(s) followed by 'make capsule' to generate the capsules for > > > > > > earlier generated images. In it's current form, I don't see an easy > > > > > > way to enforce this dependency in binman when I want to build the > > > > > > payload followed by generation of capsules. I did see the mention of > > > > > > encapsulating an entry within another dependent entry, but I think > > > > > > that makes the implementation more complex than it ought to be. > > > > > > > > > > > > I think it is much easier to use the make flow to generate the > > > > > > images > > > > > > followed by capsules, instead of tweaking the binman node to first > > > > > > generate the payload images, followed by enabling the capsule node > > > > > > to > > > > > > build the capsules. If there is an easy way of enforcing this > > > > > > dependency, please let me know. Thanks > > > > > > > > > > Can you share your explorations? I think the capsule should be created > > > > > as part of the build, if enabled. Rather than changing the input > > > > > files, binman should produce new output files. > > > > > > > > This is an issue of handling dependencies in binman, and not changing > > > > input files. We do not have support for telling binman "build/generate > > > > this particular image first before you proceed to build the capsules > > > > using the earlier built images". I am not sure if this can be done in > > > > a generic manner in binman, so that irrespective of the image being > > > > generated, it can be specified to build capsules once the capsule > > > > input images have been generated. > > > > > > I'm just not sure what you are getting out here. > > > > > > See INPUTS-y for the input files to binman. Then binman uses these to > > > generate output files. It does not mess with the input files, nor > > > should it. Please read the top part of the Binman motivation to > > > understand how all this works. > > > > > > At the risk of repeating myself, we want the Makefile to focus on > > > building U-Boot, with Binman handling the laterprocessing of those > > > files. Binman may run as part of the U-Boot build, and/or can be run > > > later, with the input files. > > > > > > > > > > > > > > > > > We are trying to remove most of the output logic in Makefile. It > > > > > should just be producing input files for binman. > > > > > > > > I understand. However, like I mentioned above, as of now, we don't
Re: [PATCH 1/7] capsule: authenticate: Embed capsule public key in platform's dtb
hi Simon, On Tue, 27 Jun 2023 at 15:44, Simon Glass wrote: > > Hi, > > On Tue, 27 Jun 2023 at 10:55, Ilias Apalodimas > wrote: > > > > Hi Simon, > > > > On Mon, 26 Jun 2023 at 14:19, Simon Glass wrote: > > > > > > Hi Ilias, > > > > > > On Mon, 26 Jun 2023 at 10:53, Ilias Apalodimas > > > wrote: > > > > > > > > Hi Simon, > > > > > > > > [...] > > > > > > > > > > > > > > + > > > > > > > > > > +gen_capsule_signature_file signature.$$.dts > /dev/null > > > > > > > > > > 2>&1 > > > > > > > > > > +$CPP $dtc_cpp_flags -x assembler-with-cpp -o > > > > > > > > > > signature.$$.tmp signature.$$.dts > /dev/null 2>&1 > > > > > > > > > > +dtc -@ -O dtb -o signature.$$.dtbo signature.$$.tmp > > > > > > > > > > > /dev/null 2>&1 > > > > > > > > > > +fdtoverlay -i $1 -o temp.$$.dtb -v signature.$$.dtbo > > > > > > > > > > > /dev/null 2>&1 > > > > > > > > > > +mv temp.$$.dtb $1 > /dev/null 2>&1 > > > > > > > > > > +rm -f signature.$$.* > /dev/null 2>&1 > > > > > > > > > > -- > > > > > > > > > > 2.34.1 > > > > > > > > > > > > > > > > > > > > > > > > > > > > Can you please add this to binman instead? > > > > > > > > > > > > > > > > I had looked at using binman for this work earlier because I > > > > > > > > very much > > > > > > > > expected this comment from you :). Having said that, I am very > > > > > > > > much > > > > > > > > open to using binman instead if it turns out to be the better > > > > > > > > way of > > > > > > > > achieving this. What this patch does is that, with capsule > > > > > > > > authentication enabled, it embeds the public key esl file into > > > > > > > > the > > > > > > > > dtb's as they get built. As per my understanding, binman gets > > > > > > > > called > > > > > > > > at the end of the u-boot build, once the constituent images( > > > > > > > > e..g > > > > > > > > u-boot.bin = u-boot-no-dtb.bin + dtb) have been generated. So, > > > > > > > > if we > > > > > > > > call binman _after_ the requisite image(s) have been generated, > > > > > > > > we > > > > > > > > would need to 1) identify the dtb's in which the esl needs to be > > > > > > > > embedded, and then 2) generate the final image all over again. > > > > > > > > Don't > > > > > > > > you think this is non optimal? Or is there a way of generating > > > > > > > > the > > > > > > > > constituent images(including the dtb's) through binman instead? > > > > > > > > > > > > > > The best way to do that IMO is to generate a second file, .e.g. > > > > > > > u-boot-capsule.bin > > > > > > > > > > > > > > This make no sense to me whatsoever. Do we have an example in u-boot > > > > generating multiple dtb versions for other reasons/subsystems? > > > > > > > > > > That would break the scripts for platforms which might be using > > > > > > u-boot.bin as the image to boot from. I know that the ST platform > > > > > > which does enable capsule updates uses the u-boot-nodtb.bin as the > > > > > > BL33 image and the u-boot.dtb as BL33_CFG. Hence my question, if we > > > > > > have to use binman, is there a way to 1) modify the u-boot.dtb and > > > > > > then 2) regenerate u-boot.bin image. > > > > > > > > > > > > I know this is software, and everything can be done in a hacky way. > > > > > > But I was exploring using the u-boot node as a section entry, so > > > > > > that > > > > > > the u-boot.dtb can be modified and then binman would > > > > > > repackage/regenerate the u-boot.bin. But this is not working. > > > > > > > > > > NO, please do not do that. You should create a new file, not modify > > > > > u-boot.bin or u-boot.dtb. Please just don't mess around with this, it > > > > > will lead to all sorts of confusion. > > > > > > > > > > I thought we already had this discussion a while back? > > > > > > > > No we haven't. In fact I am struggling to see the confusion part. It's > > > > fine for the u-boot dtb to include all the internal nodes DM needs, but > > > > suddenly having the capsule signature is problematic? > > > > > > > > In the past the .esl file was part of the U-Boot binary and things were > > > > working perfectly fine. In fact you could update/downgrade u-boot and > > > > the > > > > signatures naturally followed along instead of having to update u-boot > > > > *and* the dtb, which we have to do today. You could also build a capsule > > > > way easier without injecting/removing signatures to the dtb. > > > > You were the one that insisted on reverting that and instead adding it > > > > on > > > > the dtb. We explained most of the downsides back then, along with some > > > > security concerns. We also mentioned that the signature in the dtb > > > > makes > > > > little sense since it's difference *per class of boards* and it's not > > > > something we could include in static dtb files, but that lead nowhere... > > > > > > > > As Sughosh already said there are platforms that use the generated > > > > u-boot > > > > dtb and the raw binary to assemble a FIP image. So why exactly adding > > > > the > > > > capsule
Re: [PATCH 1/7] capsule: authenticate: Embed capsule public key in platform's dtb
Hi, On Tue, 27 Jun 2023 at 10:55, Ilias Apalodimas wrote: > > Hi Simon, > > On Mon, 26 Jun 2023 at 14:19, Simon Glass wrote: > > > > Hi Ilias, > > > > On Mon, 26 Jun 2023 at 10:53, Ilias Apalodimas > > wrote: > > > > > > Hi Simon, > > > > > > [...] > > > > > > > > > > > > + > > > > > > > > > +gen_capsule_signature_file signature.$$.dts > /dev/null 2>&1 > > > > > > > > > +$CPP $dtc_cpp_flags -x assembler-with-cpp -o > > > > > > > > > signature.$$.tmp signature.$$.dts > /dev/null 2>&1 > > > > > > > > > +dtc -@ -O dtb -o signature.$$.dtbo signature.$$.tmp > > > > > > > > > > /dev/null 2>&1 > > > > > > > > > +fdtoverlay -i $1 -o temp.$$.dtb -v signature.$$.dtbo > > > > > > > > > > /dev/null 2>&1 > > > > > > > > > +mv temp.$$.dtb $1 > /dev/null 2>&1 > > > > > > > > > +rm -f signature.$$.* > /dev/null 2>&1 > > > > > > > > > -- > > > > > > > > > 2.34.1 > > > > > > > > > > > > > > > > > > > > > > > > > Can you please add this to binman instead? > > > > > > > > > > > > > > I had looked at using binman for this work earlier because I very > > > > > > > much > > > > > > > expected this comment from you :). Having said that, I am very > > > > > > > much > > > > > > > open to using binman instead if it turns out to be the better way > > > > > > > of > > > > > > > achieving this. What this patch does is that, with capsule > > > > > > > authentication enabled, it embeds the public key esl file into the > > > > > > > dtb's as they get built. As per my understanding, binman gets > > > > > > > called > > > > > > > at the end of the u-boot build, once the constituent images( e..g > > > > > > > u-boot.bin = u-boot-no-dtb.bin + dtb) have been generated. So, if > > > > > > > we > > > > > > > call binman _after_ the requisite image(s) have been generated, we > > > > > > > would need to 1) identify the dtb's in which the esl needs to be > > > > > > > embedded, and then 2) generate the final image all over again. > > > > > > > Don't > > > > > > > you think this is non optimal? Or is there a way of generating the > > > > > > > constituent images(including the dtb's) through binman instead? > > > > > > > > > > > > The best way to do that IMO is to generate a second file, .e.g. > > > > > > u-boot-capsule.bin > > > > > > > > > > > This make no sense to me whatsoever. Do we have an example in u-boot > > > generating multiple dtb versions for other reasons/subsystems? > > > > > > > > That would break the scripts for platforms which might be using > > > > > u-boot.bin as the image to boot from. I know that the ST platform > > > > > which does enable capsule updates uses the u-boot-nodtb.bin as the > > > > > BL33 image and the u-boot.dtb as BL33_CFG. Hence my question, if we > > > > > have to use binman, is there a way to 1) modify the u-boot.dtb and > > > > > then 2) regenerate u-boot.bin image. > > > > > > > > > > I know this is software, and everything can be done in a hacky way. > > > > > But I was exploring using the u-boot node as a section entry, so that > > > > > the u-boot.dtb can be modified and then binman would > > > > > repackage/regenerate the u-boot.bin. But this is not working. > > > > > > > > NO, please do not do that. You should create a new file, not modify > > > > u-boot.bin or u-boot.dtb. Please just don't mess around with this, it > > > > will lead to all sorts of confusion. > > > > > > > > I thought we already had this discussion a while back? > > > > > > No we haven't. In fact I am struggling to see the confusion part. It's > > > fine for the u-boot dtb to include all the internal nodes DM needs, but > > > suddenly having the capsule signature is problematic? > > > > > > In the past the .esl file was part of the U-Boot binary and things were > > > working perfectly fine. In fact you could update/downgrade u-boot and the > > > signatures naturally followed along instead of having to update u-boot > > > *and* the dtb, which we have to do today. You could also build a capsule > > > way easier without injecting/removing signatures to the dtb. > > > You were the one that insisted on reverting that and instead adding it on > > > the dtb. We explained most of the downsides back then, along with some > > > security concerns. We also mentioned that the signature in the dtb makes > > > little sense since it's difference *per class of boards* and it's not > > > something we could include in static dtb files, but that lead nowhere... > > > > > > As Sughosh already said there are platforms that use the generated u-boot > > > dtb and the raw binary to assemble a FIP image. So why exactly adding the > > > capsule signature to the default dtb is wrong? Why should we add an > > > *extra* .dtb with one additional node -- which is very much needed by the > > > design you proposed. This will only lead to extra confusion. > > > > 1. I thought a capsule update was going to be a separate file, e.g. > > u-boot-capture.bin ? > > Yes the capsule itself is a different file and I dont think there's > any disagreement on
Re: [PATCH 1/7] capsule: authenticate: Embed capsule public key in platform's dtb
Hi Simon, On Mon, 26 Jun 2023 at 14:19, Simon Glass wrote: > > Hi Ilias, > > On Mon, 26 Jun 2023 at 10:53, Ilias Apalodimas > wrote: > > > > Hi Simon, > > > > [...] > > > > > > > > > > + > > > > > > > > +gen_capsule_signature_file signature.$$.dts > /dev/null 2>&1 > > > > > > > > +$CPP $dtc_cpp_flags -x assembler-with-cpp -o signature.$$.tmp > > > > > > > > signature.$$.dts > /dev/null 2>&1 > > > > > > > > +dtc -@ -O dtb -o signature.$$.dtbo signature.$$.tmp > > > > > > > > > /dev/null 2>&1 > > > > > > > > +fdtoverlay -i $1 -o temp.$$.dtb -v signature.$$.dtbo > > > > > > > > > /dev/null 2>&1 > > > > > > > > +mv temp.$$.dtb $1 > /dev/null 2>&1 > > > > > > > > +rm -f signature.$$.* > /dev/null 2>&1 > > > > > > > > -- > > > > > > > > 2.34.1 > > > > > > > > > > > > > > > > > > > > > > Can you please add this to binman instead? > > > > > > > > > > > > I had looked at using binman for this work earlier because I very > > > > > > much > > > > > > expected this comment from you :). Having said that, I am very much > > > > > > open to using binman instead if it turns out to be the better way of > > > > > > achieving this. What this patch does is that, with capsule > > > > > > authentication enabled, it embeds the public key esl file into the > > > > > > dtb's as they get built. As per my understanding, binman gets called > > > > > > at the end of the u-boot build, once the constituent images( e..g > > > > > > u-boot.bin = u-boot-no-dtb.bin + dtb) have been generated. So, if we > > > > > > call binman _after_ the requisite image(s) have been generated, we > > > > > > would need to 1) identify the dtb's in which the esl needs to be > > > > > > embedded, and then 2) generate the final image all over again. Don't > > > > > > you think this is non optimal? Or is there a way of generating the > > > > > > constituent images(including the dtb's) through binman instead? > > > > > > > > > > The best way to do that IMO is to generate a second file, .e.g. > > > > > u-boot-capsule.bin > > > > > > > > This make no sense to me whatsoever. Do we have an example in u-boot > > generating multiple dtb versions for other reasons/subsystems? > > > > > > That would break the scripts for platforms which might be using > > > > u-boot.bin as the image to boot from. I know that the ST platform > > > > which does enable capsule updates uses the u-boot-nodtb.bin as the > > > > BL33 image and the u-boot.dtb as BL33_CFG. Hence my question, if we > > > > have to use binman, is there a way to 1) modify the u-boot.dtb and > > > > then 2) regenerate u-boot.bin image. > > > > > > > > I know this is software, and everything can be done in a hacky way. > > > > But I was exploring using the u-boot node as a section entry, so that > > > > the u-boot.dtb can be modified and then binman would > > > > repackage/regenerate the u-boot.bin. But this is not working. > > > > > > NO, please do not do that. You should create a new file, not modify > > > u-boot.bin or u-boot.dtb. Please just don't mess around with this, it > > > will lead to all sorts of confusion. > > > > > > I thought we already had this discussion a while back? > > > > No we haven't. In fact I am struggling to see the confusion part. It's > > fine for the u-boot dtb to include all the internal nodes DM needs, but > > suddenly having the capsule signature is problematic? > > > > In the past the .esl file was part of the U-Boot binary and things were > > working perfectly fine. In fact you could update/downgrade u-boot and the > > signatures naturally followed along instead of having to update u-boot > > *and* the dtb, which we have to do today. You could also build a capsule > > way easier without injecting/removing signatures to the dtb. > > You were the one that insisted on reverting that and instead adding it on > > the dtb. We explained most of the downsides back then, along with some > > security concerns. We also mentioned that the signature in the dtb makes > > little sense since it's difference *per class of boards* and it's not > > something we could include in static dtb files, but that lead nowhere... > > > > As Sughosh already said there are platforms that use the generated u-boot > > dtb and the raw binary to assemble a FIP image. So why exactly adding the > > capsule signature to the default dtb is wrong? Why should we add an > > *extra* .dtb with one additional node -- which is very much needed by the > > design you proposed. This will only lead to extra confusion. > > 1. I thought a capsule update was going to be a separate file, e.g. > u-boot-capture.bin ? Yes the capsule itself is a different file and I dont think there's any disagreement on how to generate that. On the u-boot.bin you need to flash on the board though, you need to include the public key you authenticate the incoming capsule against. That's what Sughosh wants to inject. > > 2. You can't put the signature into U-Boot. It needs to be in the > capsule update so that U-Boot can check it. If you ar
[PULL] u-boot-riscv/riscv-fixes
Hi Tom, The following changes since commit 4f1077bc35f683985ff77e442ada7e8a8a52e3b7: Prepare v2023.07-rc5 (2023-06-26 11:44:06 -0400) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-riscv.git riscv-fixes for you to fetch changes up to 4a3efd71cd858b87527e9478ff51529d39329819: riscv: Fix alignment of RELA sections in the linker scripts (2023-06-27 10:09:51 +0800) CI result shows no issue: https://source.denx.de/u-boot/custodians/u-boot-riscv/-/pipelines/16707 - Alignment fixes for RISC-V prelink process - MAINTAINERS file fixes Bin Meng (1): riscv: Fix alignment of RELA sections in the linker scripts Heinrich Schuchardt (1): MAINTAINERS: update SiFive HiFive Unmatched maintainers Yanhong Wang (1): board: starfive: Fixed errors reported when executing get_maintainer.pl arch/riscv/cpu/u-boot.lds | 4 +--- board/sifive/unmatched/MAINTAINERS | 1 - board/starfive/visionfive2/MAINTAINERS | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-) Best regards, Leo
