[PATCH 5/5] test: dm: add scmi command test
In this test, "scmi" command is tested against different sub-commands. Please note that scmi command is for debug purpose and is not intended in production system. Signed-off-by: AKASHI Takahiro Reviewed-by: Simon Glass Reviewed-by: Etienne Carriere --- v7 * make test assertions more flexible depending on the number of provided protocols v4 * move 'base'-related changes to the prior commit * add CONFIG_CMD_SCMI to sandbox_defconfig v3 * change char to u8 in vendor/agent names v2 * use helper functions, removing direct uses of ops --- configs/sandbox_defconfig | 1 + test/dm/scmi.c| 81 +++ 2 files changed, 82 insertions(+) diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index e3a2f9eb1708..34e3cc281d7f 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -120,6 +120,7 @@ CONFIG_CMD_REGULATOR=y CONFIG_CMD_AES=y CONFIG_CMD_TPM=y CONFIG_CMD_TPM_TEST=y +CONFIG_CMD_SCMI=y CONFIG_CMD_BTRFS=y CONFIG_CMD_CBFS=y CONFIG_CMD_CRAMFS=y diff --git a/test/dm/scmi.c b/test/dm/scmi.c index 2f63f2da16fb..2bcf7ac6fcc3 100644 --- a/test/dm/scmi.c +++ b/test/dm/scmi.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -206,6 +207,86 @@ 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; + int num_proto = 0; + char cmd_out[30]; + + if (!IS_ENABLED(CONFIG_CMD_SCMI)) + return 0; + + /* preparation */ + ut_assertok(uclass_get_device_by_name(UCLASS_SCMI_AGENT, "scmi", + _dev)); + ut_assertnonnull(agent_dev); + + /* +* Estimate the number of provided protocols. +* This estimation is correct as far as a corresponding +* protocol support is added to sandbox fake serer. +*/ + if (IS_ENABLED(CONFIG_SCMI_POWER_DOMAIN)) + num_proto++; + if (IS_ENABLED(CONFIG_CLK_SCMI)) + num_proto++; + if (IS_ENABLED(CONFIG_RESET_SCMI)) + num_proto++; + if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI)) + num_proto++; + + /* scmi info */ + ut_assertok(run_command("scmi info", 0)); + + ut_assert_nextline("SCMI device: scmi"); + snprintf(cmd_out, 30, " protocol version: 0x%x", +SCMI_BASE_PROTOCOL_VERSION); + ut_assert_nextline(cmd_out); + ut_assert_nextline(" # of agents: 2"); + ut_assert_nextline(" 0: platform"); + ut_assert_nextline("> 1: OSPM"); + snprintf(cmd_out, 30, " # of protocols: %d", num_proto); + ut_assert_nextline(cmd_out); + if (IS_ENABLED(CONFIG_SCMI_POWER_DOMAIN)) + ut_assert_nextline(" Power domain management"); + if (IS_ENABLED(CONFIG_CLK_SCMI)) + ut_assert_nextline(" Clock management"); + if (IS_ENABLED(CONFIG_RESET_SCMI)) + ut_assert_nextline(" Reset domain management"); + if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI)) + 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 perm_dev */ + ut_assertok(run_command("scmi perm_dev 1 0 1", 0)); + ut_assert_console_end(); + + ut_assert(run_command("scmi perm_dev 1 0 0", 0)); + ut_assert_nextline("Denying access to device:0 failed (-13)"); + ut_assert_console_end(); + + /* scmi perm_proto */ + ut_assertok(run_command("scmi perm_proto 1 0 14 1", 0)); + ut_assert_console_end(); + + ut_assert(run_command("scmi 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 reset */ + ut_assert(run_command("scmi 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_power_domains(struct unit_test_state *uts) { struct sandbox_scmi_agent *agent; -- 2.34.1
[PATCH 4/5] doc: cmd: add documentation for scmi
This is a help text for scmi command. Signed-off-by: AKASHI Takahiro Reviewed-by: Simon Glass Reviewed-by: Etienne Carriere --- v6 * add the manual to doc/usage/index.rst v4 * s/tranport/transport/ v2 * add more descriptions about SCMI --- doc/usage/cmd/scmi.rst | 126 + doc/usage/index.rst| 1 + 2 files changed, 127 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 ..9ea7e0e41dad --- /dev/null +++ b/doc/usage/cmd/scmi.rst @@ -0,0 +1,126 @@ +.. SPDX-License-Identifier: GPL-2.0+: + +scmi command + + +Synopsis + + +:: + +scmi info +scmi perm_dev +scmi perm_proto +scmi reset + +Description +--- + +Arm System Control and Management Interface (SCMI hereafter) is a set of +standardised interfaces to manage system resources, like clocks, power +domains, pin controls, reset and so on, in a system-wide manner. + +An entity which provides those services is called a SCMI firmware (or +SCMI server if you like) may be placed/implemented by EL3 software or +by a dedicated system control processor (SCP) or else. + +A user of SCMI interfaces, including U-Boot, is called a SCMI agent and +may issues commands, which are defined in each protocol for specific system +resources, to SCMI server via a communication channel, called a transport. +Those interfaces are independent from the server's implementation thanks to +a transport layer. + +For more details, see the `SCMI specification`_. + +While most of system resources managed under SCMI protocols are implemented +and handled as standard U-Boot devices, for example clk_scmi, scmi command +provides additional management functionality against SCMI server. + +scmi info +~ +Show base information about SCMI server and supported protocols + +scmi perm_dev +~ +Allow or deny access permission to the device + +scmi perm_proto +~~~ +Allow or deny access to the protocol on the device + +scmi reset +~~ +Reset the already-configured permissions against the device + +Parameters are used as follows: + + +SCMI Agent ID, hex value + + +SCMI Device ID, hex value + +Please note that what a device means is not defined +in the specification. + + +SCMI Protocol ID, hex value + +It must not be 0x10 (base protocol) + + +Flags to control the action, hex value + +0 to deny, 1 to allow. The other values are reserved and allowed +values may depend on the implemented version of SCMI server in +the future. See SCMI specification for more details. + +Example +--- + +Obtain basic information about SCMI server: + +:: + +=> scmi 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 perm_dev 1 0 1 + +Reset configurations with all access permission settings retained: + +:: + +=> scmi 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. + +.. _`SCMI specification`: https://developer.arm.com/documentation/den0056/e/?lang=en diff --git a/doc/usage/index.rst b/doc/usage/index.rst index 98b4719c4088..9a65b50aeed7 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -94,6 +94,7 @@ Shell commands cmd/saves cmd/sbi cmd/sf + cmd/scmi cmd/scp03 cmd/seama cmd/setexpr -- 2.34.1
[PATCH 3/5] cmd: add scmi command for SCMI firmware
This command, "scmi", may provide a command line interface to various SCMI protocols. It supports at least initially SCMI base protocol and is intended mainly for debug purpose. Signed-off-by: AKASHI Takahiro Reviewed-by: Simon Glass Reviewed-by: Etienne Carriere --- v3 * describe that arguments are in hex at a help message * modify the code for dynamically allocated agent names v2 * remove sub command category, 'scmi base', for simplicity --- cmd/Kconfig | 9 ++ cmd/Makefile | 1 + cmd/scmi.c | 335 +++ 3 files changed, 345 insertions(+) create mode 100644 cmd/scmi.c diff --git a/cmd/Kconfig b/cmd/Kconfig index 205df2f1fb65..c940051eba91 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -2550,6 +2550,15 @@ 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 (System + Control and Management Interface) protocols available on Arm + platforms to manage system resources. endmenu menu "Filesystem commands" diff --git a/cmd/Makefile b/cmd/Makefile index 9a6790cc1708..320f0b5266eb 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -159,6 +159,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 ..f8f54f84cff8 --- /dev/null +++ b/cmd/scmi.c @@ -0,0 +1,335 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * SCMI (System Control and Management Interface) utility command + * + * Copyright (c) 2023 Linaro Limited + * Author: AKASHI Takahiro + */ + +#include +#include +#include +#include +#include +#include +#include +#include /* uclass_get_device */ +#include +#include + +static struct udevice *agent; +static struct udevice *base_proto; + +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"}, +}; + +/** + * get_proto_name() - get the name of SCMI protocol + * + * @id:SCMI Protocol ID + * + * Get the printable name of the protocol, @id + * + * Return: Name string on success, NULL on failure + */ +static const char *get_proto_name(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_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_info(struct cmd_tbl *cmdtp, int flag, int argc, + char * const argv[]) +{ + u32 agent_id, num_protocols; + u8 *agent_name, *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 = scmi_base_discover_agent(base_proto, i, _id, + _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); + free(agent_name); + } + printf(" # of protocols: %d\n", scmi_num_protocols(agent)); + num_protocols = scmi_num_protocols(agent); + protocols = scmi_protocols(agent); + if
[PATCH 2/5] firmware: scmi: support protocols on sandbox only if enabled
This change will be useful when we manually test SCMI on sandbox by enabling/disabling a specific SCMI protocol. Signed-off-by: AKASHI Takahiro --- drivers/firmware/scmi/sandbox-scmi_agent.c | 27 ++- drivers/firmware/scmi/sandbox-scmi_devices.c | 78 2 files changed, 72 insertions(+), 33 deletions(-) diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c index 9f5f497e0a6c..e1b9b5f5f2d8 100644 --- a/drivers/firmware/scmi/sandbox-scmi_agent.c +++ b/drivers/firmware/scmi/sandbox-scmi_agent.c @@ -66,10 +66,18 @@ struct scmi_channel { }; static u8 protocols[] = { +#if IS_ENABLED(CONFIG_SCMI_POWER_DOMAIN) SCMI_PROTOCOL_ID_POWER_DOMAIN, +#endif +#if IS_ENABLED(CONFIG_CLK_SCMI) SCMI_PROTOCOL_ID_CLOCK, +#endif +#if IS_ENABLED(CONFIG_RESET_SCMI) SCMI_PROTOCOL_ID_RESET_DOMAIN, +#endif +#if IS_ENABLED(CONFIG_DM_REGULATOR_SCMI) SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, +#endif }; #define NUM_PROTOCOLS ARRAY_SIZE(protocols) @@ -1160,6 +1168,9 @@ static int sandbox_scmi_test_process_msg(struct udevice *dev, } break; case SCMI_PROTOCOL_ID_POWER_DOMAIN: + if (!IS_ENABLED(CONFIG_SCMI_POWER_DOMAIN)) + goto not_supported; + switch (msg->message_id) { case SCMI_PROTOCOL_VERSION: return sandbox_scmi_pwd_protocol_version(dev, msg); @@ -1180,6 +1191,9 @@ static int sandbox_scmi_test_process_msg(struct udevice *dev, } break; case SCMI_PROTOCOL_ID_CLOCK: + if (!IS_ENABLED(CONFIG_CLK_SCMI)) + goto not_supported; + switch (msg->message_id) { case SCMI_PROTOCOL_ATTRIBUTES: return sandbox_scmi_clock_protocol_attribs(dev, msg); @@ -1196,6 +1210,9 @@ static int sandbox_scmi_test_process_msg(struct udevice *dev, } break; case SCMI_PROTOCOL_ID_RESET_DOMAIN: + if (!IS_ENABLED(CONFIG_RESET_SCMI)) + goto not_supported; + switch (msg->message_id) { case SCMI_RESET_DOMAIN_ATTRIBUTES: return sandbox_scmi_rd_attribs(dev, msg); @@ -1206,6 +1223,9 @@ static int sandbox_scmi_test_process_msg(struct udevice *dev, } break; case SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN: + if (!IS_ENABLED(CONFIG_DM_REGULATOR_SCMI)) + goto not_supported; + switch (msg->message_id) { case SCMI_VOLTAGE_DOMAIN_ATTRIBUTES: return sandbox_scmi_voltd_attribs(dev, msg); @@ -1224,8 +1244,7 @@ static int sandbox_scmi_test_process_msg(struct udevice *dev, case SCMI_PROTOCOL_ID_SYSTEM: case SCMI_PROTOCOL_ID_PERF: case SCMI_PROTOCOL_ID_SENSOR: - *(u32 *)msg->out_msg = SCMI_NOT_SUPPORTED; - return 0; + goto not_supported; default: break; } @@ -1239,6 +1258,10 @@ static int sandbox_scmi_test_process_msg(struct udevice *dev, /* Intentionnaly report unhandled IDs through the SCMI return code */ *(u32 *)msg->out_msg = SCMI_PROTOCOL_ERROR; return 0; + +not_supported: + *(u32 *)msg->out_msg = SCMI_NOT_SUPPORTED; + return 0; } static int sandbox_scmi_test_remove(struct udevice *dev) diff --git a/drivers/firmware/scmi/sandbox-scmi_devices.c b/drivers/firmware/scmi/sandbox-scmi_devices.c index facb5b06ffb5..0519cf889aa9 100644 --- a/drivers/firmware/scmi/sandbox-scmi_devices.c +++ b/drivers/firmware/scmi/sandbox-scmi_devices.c @@ -62,12 +62,13 @@ static int sandbox_scmi_devices_remove(struct udevice *dev) if (!devices) return 0; - for (n = 0; n < SCMI_TEST_DEVICES_RD_COUNT; n++) { - int ret2 = reset_free(devices->reset + n); + if (IS_ENABLED(CONFIG_RESET_SCMI)) + for (n = 0; n < SCMI_TEST_DEVICES_RD_COUNT; n++) { + int ret2 = reset_free(devices->reset + n); - if (ret2 && !ret) - ret = ret2; - } + if (ret2 && !ret) + ret = ret2; + } return ret; } @@ -89,39 +90,53 @@ static int sandbox_scmi_devices_probe(struct udevice *dev) .regul_count = SCMI_TEST_DEVICES_VOLTD_COUNT, }; - ret = power_domain_get_by_index(dev, priv->devices.pwdom, 0); - if (ret) { - dev_err(dev, "%s: Failed on power domain\n", __func__); - return ret; - } - - for (n = 0; n < SCMI_TEST_DEVICES_CLK_COUNT; n++) { - ret = clk_get_by_index(dev, n, priv->devices.clk + n); + if (IS_ENABLED(CONFIG_SCMI_POWER_DOMAIN)) { + ret = power_domain_get_by_index(dev,
[PATCH 1/5] test: dm: skip scmi tests against disabled protocols
This is a precautious change to make scmi tests workable whether or not a specific protocol be enabled. Signed-off-by: AKASHI Takahiro --- test/dm/scmi.c | 12 1 file changed, 12 insertions(+) diff --git a/test/dm/scmi.c b/test/dm/scmi.c index da45314f2e4c..2f63f2da16fb 100644 --- a/test/dm/scmi.c +++ b/test/dm/scmi.c @@ -217,6 +217,9 @@ static int dm_test_scmi_power_domains(struct unit_test_state *uts) u8 *name; int ret; + if (!IS_ENABLED(CONFIG_SCMI_POWER_DOMAIN)) + return 0; + /* preparation */ ut_assertok(load_sandbox_scmi_test_devices(uts, , )); ut_assertnonnull(agent); @@ -317,6 +320,9 @@ static int dm_test_scmi_clocks(struct unit_test_state *uts) int ret_dev; int ret; + if (!IS_ENABLED(CONFIG_CLK_SCMI)) + return 0; + ret = load_sandbox_scmi_test_devices(uts, , ); if (ret) return ret; @@ -382,6 +388,9 @@ static int dm_test_scmi_resets(struct unit_test_state *uts) struct udevice *agent_dev, *reset_dev, *dev = NULL; int ret; + if (!IS_ENABLED(CONFIG_RESET_SCMI)) + return 0; + ret = load_sandbox_scmi_test_devices(uts, , ); if (ret) return ret; @@ -418,6 +427,9 @@ static int dm_test_scmi_voltage_domains(struct unit_test_state *uts) struct udevice *dev; struct udevice *regul0_dev; + if (!IS_ENABLED(CONFIG_DM_REGULATOR_SCMI)) + return 0; + ut_assertok(load_sandbox_scmi_test_devices(uts, , )); scmi_devices = sandbox_scmi_devices_ctx(dev); -- 2.34.1
[PATCH 0/5] cmd: add scmi command
"Scmi" command will be re-introduced per Michal's request. The functionality is the same as I put it in my patch set of adding SCMI base protocol support, but made some tweak to make UT, "ut dm scmi_cmd," more flexible and tolerable when enabling/disabling a specific SCMI protocol for test purpose. Each commit may have some change history inherited from the preceding patch series. Test The patch series was tested on the following platforms: * sandbox Prerequisite: = * This patch series is based on the latest master. AKASHI Takahiro (5): test: dm: skip scmi tests against disabled protocols firmware: scmi: support protocols on sandbox only if enabled cmd: add scmi command for SCMI firmware doc: cmd: add documentation for scmi test: dm: add scmi command test cmd/Kconfig | 9 + cmd/Makefile | 1 + cmd/scmi.c | 335 +++ configs/sandbox_defconfig| 1 + doc/usage/cmd/scmi.rst | 126 +++ doc/usage/index.rst | 1 + drivers/firmware/scmi/sandbox-scmi_agent.c | 27 +- drivers/firmware/scmi/sandbox-scmi_devices.c | 78 +++-- test/dm/scmi.c | 93 + 9 files changed, 638 insertions(+), 33 deletions(-) create mode 100644 cmd/scmi.c create mode 100644 doc/usage/cmd/scmi.rst -- 2.34.1
Re: Failedto read big file with "Invald FAT entry' from USB disk (FAT16)
+Heinrich Schuchardt On Tue, 24 Oct 2023 at 19:34, target wrote: > > I am using u-boot 2017.03 in my system and enabled the debug in fs/fat/fat.c > I had a USB disk formatted in FAT16 in Linux, I put a bigfile.bin into / in > the USB disk and plugged the disk into by board. > Then I want to read/load the bigfile.bin into RAM with u-boot's fatload > command, I did followings. > > > # usb reset > # fatls usb 0, it showd, > VFAT Support enabled > FAT16, fat_sect: 128, fatlength: 128 > Rootdir begins at cluster: 0, sector: 384, offset: 3 > Data begins at: 256 > Sector size: 512, cluster size: 128 > FAT read(sect=384, cnt:2), clust_size=128, DIRENTSPERBLOCK=16 > 36438016 bigfile.bin > 1012 s01syslogd > 1004 s02klogd > 1876 s02sysctl > 1684 s20urandom > 214 s21mount > END LOOP: buffer_blk_cnt=0 clust_size=128 > FAT read(sect=385, cnt:2), clust_size=128, DIRENTSPERBLOCK=16 > 1635 s30dbus > 438 s40network > RootDentname == NULL - 13 > > > 8 file(s), 0 dir(s) > # fatload usb 0 0x10 bigfile.bin > reading bigile.bin > VFAT Support enabled > FAT16, fat_sect: 128, fatlength: 128 > Rootdir begins at cluster: 0, sector: 384, offset: 3 > Data begins at: 256 > Sector size: 512, cluster size: 128 > FAT read(sect=384, cnt:2), clust_size=128, DIRENTSPERBLOCK=16 > Rootvfatname: |bigfile.bin| > RootName: bigfile.bin, start: 0x3, size: 0x22c > Filesize: 36438016 bytes > 36438016 bytes > FAT16: entry: 0x0003 = 3, offset: 0x0003 = 3 > FAT16: ret: 0004, offset: 0003 > FAT16: entry: 0x0004 = 4, offset: 0x0004 = 4 > FAT16: ret: 0005, offset: 0004 > FAT16: entry: 0x0005 = 5, offset: 0x0005 = 5 > FAT16: ret: 0006, offset: 0005 > FAT16: entry: 0x0006 = 6, offset: 0x0006 = 6 > FAT16: ret: 0007, offset: 0006 > FAT16: entry: 0x0007 = 7, offset: 0x0007 = 7 > FAT16: ret: 0008, offset: 0007 > FAT16: entry: 0x0008 = 8, offset: 0x0008 = 8 > FAT16: ret: 0009, offset: 0008 > FAT16: entry: 0x0009 = 9, offset: 0x0009 = 9 > FAT16: ret: 000a, offset: 0009 > . > . > FAT16: entry: 0x020e = 526, offset: 0x020e = 526 > FAT16: ret: 020f, offset: 020e > FAT16: entry: 0x020f = 527, offset: 0x020f = 527 > FAT16: ret: 0210, offset: 020f > FAT16: entry: 0x0210 = 528, offset: 0x0210 = 528 > FAT16: ret: , offset: 0210 > gc - clustnum: 3, startsect: 640 > FAT16: entry: 0x0210 = 528, offset: 0x0210 = 528 > FAT16: ret: , offset: 0210 > curclust: 0x > Invalid FAT entry > Size: 36438016, got: 34471936 > > > I have tried many times, even with "fatload usb 0 0x10 bigfile.bin > 200", it still failed with above error. > I can not test newer version of u-boot, since there are lots of old drivers I > am using. > > > > >
Re: [PATCH v2] smbios: arm64: Allow table to be written at a fixed addr
Hi Heinrich, On Tue, 24 Oct 2023 at 18:22, Heinrich Schuchardt wrote: > > > > Am 25. Oktober 2023 01:31:19 MESZ schrieb Simon Glass : > >U-Boot typically sets up its malloc() pool near the top of memory. On > >ARM64 systems this can result in an SMBIOS table above 4GB which is > >not supported by SMBIOSv2. > > > >Work around this problem by providing a new option to choose an address > >below 4GB (but as high as possible), if needed. > > You must not overwrite memory controlled by the EFI subsystem without calling > its allocator. We should provide SMBIOS 3. SMBIOS 2 is only a fallback for > outdated tools. That is not my intention and I don't believe this code does that. EFI is not running at this point, is it? The bit I am confused about is that we don't support SMBIOS3 in U-Boot. I am trying to fix an introduced bug... Regards, Simon
Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr
Hi Tom, On Tue, 24 Oct 2023 at 17:44, Tom Rini wrote: > > On Wed, Oct 25, 2023 at 02:19:59AM +0200, Heinrich Schuchardt wrote: > > > > > > Am 25. Oktober 2023 01:28:10 MESZ schrieb Simon Glass : > > >Hi Tom, > > > > > >On Tue, 24 Oct 2023 at 15:34, Tom Rini wrote: > > >> > > >> On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote: > > >> > > From: Simon Glass > > >> > > Date: Mon, 23 Oct 2023 00:04:14 -0700 > > >> > > > > >> > > Hi Caleb, > > >> > > > > >> > > On Sat, 21 Oct 2023 at 01:43, Caleb Connolly > > >> > > wrote: > > >> > > > > > >> > > > Hi Simon, > > >> > > > > > >> > > > On 21/10/2023 01:45, Simon Glass wrote: > > >> > > > > U-Boot typically sets up its malloc() pool near the top of > > >> > > > > memory. On > > >> > > > > ARM64 systems this can result in an SMBIOS table above 4GB which > > >> > > > > is > > >> > > > > not supported by SMBIOSv2. > > >> [snip] > > >> > There is absolutely no guarantee that arm64 machines have memory below > > >> > 4GB. Examples of SoCs that have no memory below 4GB are AMD's Opteron > > >> > A1100 SoC and all the recent Apple SoCs. > > >> > > >> So one thing to resolve here is where does that requirement about the > > >> SMBIOS table needing to be below 4GB come from (standards wise), and in > > >> turn is that obeyed by consumers like say Linux or OpenBSD? Answering my > > >> own question, maybe in part, https://www.dmtf.org/standards/smbios reads > > >> to me like there's a v3 and maybe we should be doing what we need to > > >> support / identify as that, if it doesn't have that restriction? > > > > > >Yes that was my previous patch. However 1) we apparently don't want to > > >use SMBIOS3 and 2) my patch had some sort of bug so that it wasn't > > >read correctly. > > > > > >So my next version is going to be along the lines of what was discussed > > >here. > > > > > >Of course, we cannot solve Mark's problem with SMBIOS2, but I suppose > > >that is obvious. Anyway, those platforms probably don't need SMBIOS. > > > > You are heading in the wrong direction. We need SMBIOS 3. SMBIOS 2 is > > only a fallback for outdated tools. > > > > Upcoming mainline RISC-V platforms will also have memory starting above > > 4GiB. > > I want to echo this because yes, SMBIOS if I'm recalling things right is > one of those tools used to provide user friendly information about the > system, so "everyone" wants it, or at least platforms like ones Mark is > concerned about that have more human users than embedded system users, > would like to show the information. Maybe part of the answer moving > forward is to allow being specific about SMBIOS2 or SMBIOS3 (or none) so > that the issue you had to fix can also be addressed minimally? OK, so perhaps fixing up this patch would work? [1] I got the impression that we wanted to continue to use SMBIOS2 unless we couldn't. So this patch provides for that. It is conceptually similar to the way things worked before, so provides the smallest possible change. The move to SMBIOS3 is really a separate issue, isn't it? In fact, how can writing SMBIOS2 have ever worked on machines without memory below 4GB? That seems like a 'feature request' rather than a bug fix...and the merge window is closed. So...? Regards, Simon [1] https://patchwork.ozlabs.org/project/uboot/patch/20231015024511.3995580-4-...@chromium.org/
Re: quick question about TPM
Hi Simon Driver model is enabled, so it's pretty weird it doesn't show up. CONFIG_DM=y CONFIG_SPL_DM=y CONFIG_DM_WARN=y The TPM menu is there, yet somehow empty. This is just after "make socfpga_cyclone5_defconfig" and "make menuconfig" if I use "make snow_defconfig" the menu is usable like normal, so something is disabling it, yet when grep-ping for TPM in socfpga there are no results. Adding it to .config manually also doesn't seem to work. so I'm a bit confused about what's going on. Anyhow thanks for the help! Niek 差出人: Simon Glass 送信日�r: 2023年10月25日 03:03 宛先: Niek Nooijens / OC-IAB PBD-C DEVEL 1-1 CC: u-boot@lists.denx.de 件名: Re: quick question about TPM Hi Niek, On Tue, 24 Oct 2023 at 04:51, niek.nooij...@omron.com wrote: > > Hi > > Just a quick question. I'm developing a platform using the > socfpga_cyclone5_defconfig > everything is working, linux boots, but we decided to add a TPM to it's SPI > bus. > For some reason the TPM support menu in the menuconfig is disabled and I > can't seem to find out why, or which file disables it. can you point me in > the right direction? The only thing 'config TPM' depends on is DM (driver model). Is that somehow disabled? Once you enable that, it should appear. Regards, Simon
Failedto read big file with "Invald FAT entry' from USB disk (FAT16)
I am using u-boot 2017.03 in my system and enabled the debug in fs/fat/fat.c I had a USB disk formatted in FAT16 in Linux, I put a bigfile.bin into / in the USB disk and plugged the disk into by board. Then I want to read/load the bigfile.bin into RAM with u-boot's fatload command, I did followings. # usb reset # fatls usb 0, it showd, VFAT Support enabled FAT16, fat_sect: 128, fatlength: 128 Rootdir begins at cluster: 0, sector: 384, offset: 3 Data begins at: 256 Sector size: 512, cluster size: 128 FAT read(sect=384, cnt:2), clust_size=128, DIRENTSPERBLOCK=16 36438016 bigfile.bin 1012 s01syslogd 1004 s02klogd 1876 s02sysctl 1684 s20urandom 214 s21mount END LOOP: buffer_blk_cnt=0 clust_size=128 FAT read(sect=385, cnt:2), clust_size=128, DIRENTSPERBLOCK=16 1635 s30dbus 438 s40network RootDentname == NULL - 13 8 file(s), 0 dir(s) # fatload usb 0 0x10 bigfile.bin reading bigile.bin VFAT Support enabled FAT16, fat_sect: 128, fatlength: 128 Rootdir begins at cluster: 0, sector: 384, offset: 3 Data begins at: 256 Sector size: 512, cluster size: 128 FAT read(sect=384, cnt:2), clust_size=128, DIRENTSPERBLOCK=16 Rootvfatname: |bigfile.bin| RootName: bigfile.bin, start: 0x3, size: 0x22c Filesize: 36438016 bytes 36438016 bytes FAT16: entry: 0x0003 = 3, offset: 0x0003 = 3 FAT16: ret: 0004, offset: 0003 FAT16: entry: 0x0004 = 4, offset: 0x0004 = 4 FAT16: ret: 0005, offset: 0004 FAT16: entry: 0x0005 = 5, offset: 0x0005 = 5 FAT16: ret: 0006, offset: 0005 FAT16: entry: 0x0006 = 6, offset: 0x0006 = 6 FAT16: ret: 0007, offset: 0006 FAT16: entry: 0x0007 = 7, offset: 0x0007 = 7 FAT16: ret: 0008, offset: 0007 FAT16: entry: 0x0008 = 8, offset: 0x0008 = 8 FAT16: ret: 0009, offset: 0008 FAT16: entry: 0x0009 = 9, offset: 0x0009 = 9 FAT16: ret: 000a, offset: 0009 . . FAT16: entry: 0x020e = 526, offset: 0x020e = 526 FAT16: ret: 020f, offset: 020e FAT16: entry: 0x020f = 527, offset: 0x020f = 527 FAT16: ret: 0210, offset: 020f FAT16: entry: 0x0210 = 528, offset: 0x0210 = 528 FAT16: ret: , offset: 0210 gc - clustnum: 3, startsect: 640 FAT16: entry: 0x0210 = 528, offset: 0x0210 = 528 FAT16: ret: , offset: 0210 curclust: 0x Invalid FAT entry Size: 36438016, got: 34471936 I have tried many times, even with "fatload usb 0 0x10 bigfile.bin 200", it still failed with above error. I can not test newer version of u-boot, since there are lots of old drivers I am using.
Pull request efi-2024-01-rc2
Dear Tom, The following changes since commit 5cab3515f8c9796015739c1750b8933291c816be: Merge tag 'u-boot-rockchip-20231024' of https://source.denx.de/u-boot/custodians/u-boot-rockchip (2023-10-24 09:39:52 -0400) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-efi.git tags/efi-2024-01-rc2 for you to fetch changes up to 916dad34af0e53181dfe21f5764d3a787cb24bdc: efi_loader: fix EFI_ENTRY point on get_active_pcr_banks (2023-10-24 23:56:43 +0200) Gitlab CI showed no issues: https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/18293 Pull request efi-2024-01-rc2 Documentation: * Bump urllib3 version * Replace references to dm_dump_all() with dm_dump_tree() * Update description of build dependencies for Alpine Linux * Fix typo in gpt example * Fix ordering of shell commands UEFI: * Fix build failure without network * Expose the device-tree file name as UEFI variable * Move misplace EFI_ENTRY macro. Dylan Corrales (1): doc: Replace dm_dump_all() with dm_dump_tree() Heinrich Schuchardt (2): efi_loader: fix efi_dp_from_eth efi_loader: expose the device-tree file name Ilias Apalodimas (1): efi_loader: fix EFI_ENTRY point on get_active_pcr_banks Milan P. Stanić (1): doc: build: update description of build dependencies for Alpine Linux Tom Fitzhenry (2): doc: gpt: fix example of echoing variable doc: usage: fix ordering of shell commands Tom Rini (1): sphinx: Bump urllib3 version doc/build/gcc.rst | 10 -- doc/develop/driver-model/debugging.rst | 2 +- doc/develop/uefi/uefi.rst | 14 ++ doc/sphinx/requirements.txt| 2 +- doc/usage/cmd/gpt.rst | 5 +++-- doc/usage/index.rst| 8 include/efi_loader.h | 5 + lib/efi_loader/efi_device_path.c | 5 - lib/efi_loader/efi_setup.c | 30 ++ lib/efi_loader/efi_tcg2.c | 3 ++- 10 files changed, 72 insertions(+), 12 deletions(-)
Re: [PATCH v5 14/16] cmd: add scmi command for SCMI firmware
Hi Tom, Michal, On Tue, Oct 24, 2023 at 06:24:07PM -0400, Tom Rini wrote: > On Tue, Oct 24, 2023 at 10:27:44AM +0200, Michal Simek wrote: > > Hi Takahiro, > > > > ?t 26. 9. 2023 v 9:00 odes?latel AKASHI Takahiro > > napsal: > > > > > > This command, "scmi", may provide a command line interface to various SCMI > > > protocols. It supports at least initially SCMI base protocol and is > > > intended mainly for debug purpose. > > > > > > Signed-off-by: AKASHI Takahiro > > > Reviewed-by: Simon Glass > > > Reviewed-by: Etienne Carriere > > > --- > > > v3 > > > * describe that arguments are in hex at a help message > > > * modify the code for dynamically allocated agent names > > > v2 > > > * remove sub command category, 'scmi base', for simplicity > > > --- > > > cmd/Kconfig | 9 ++ > > > cmd/Makefile | 1 + > > > cmd/scmi.c | 337 +++ > > > 3 files changed, 347 insertions(+) > > > create mode 100644 cmd/scmi.c > > > > > > diff --git a/cmd/Kconfig b/cmd/Kconfig > > > index 43ca10f69ccf..f46152ace7d8 100644 > > > --- a/cmd/Kconfig > > > +++ b/cmd/Kconfig > > > @@ -2533,6 +2533,15 @@ 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 > > > > This line above is wrong and this was removed from v6 with @Michal, do you mean the default should be 'y'? > > "drop scmi command which was intended to be used for debugging". > > It is fine that it shouldn't be used on production system but it > > doesn't mean that > > it should be actually removed. > > It is useful for bring ups. Can we get this patch merged? It was > > already reviewed > > anyway. > > There was then also some conflict with the follow-up series here. I > would be fine with the command being introduced again after I merge that > second series, which I am testing now. I don't mind neither. My concern, however, was a test, "ut dm scmi_cmd". The output from "scmi info" command varies depending on what SCMI protocols are provided by SCMI firmware (sandbox fake server in this case). I will try to keep it updated. -Takahiro Akashi > > -- > Tom signature.asc Description: PGP signature
Re: [PATCH v2] smbios: arm64: Allow table to be written at a fixed addr
Am 25. Oktober 2023 01:31:19 MESZ schrieb Simon Glass : >U-Boot typically sets up its malloc() pool near the top of memory. On >ARM64 systems this can result in an SMBIOS table above 4GB which is >not supported by SMBIOSv2. > >Work around this problem by providing a new option to choose an address >below 4GB (but as high as possible), if needed. You must not overwrite memory controlled by the EFI subsystem without calling its allocator. We should provide SMBIOS 3. SMBIOS 2 is only a fallback for outdated tools. Best regards Heinrich > >Signed-off-by: Simon Glass >--- > >Changes in v2: >- Update to search for a suitable area automatically, if enabled > > lib/Kconfig | 12 +++ > lib/efi_loader/efi_smbios.c | 63 - > 2 files changed, 74 insertions(+), 1 deletion(-) > >diff --git a/lib/Kconfig b/lib/Kconfig >index f6ca559897e7..a1eec98b392f 100644 >--- a/lib/Kconfig >+++ b/lib/Kconfig >@@ -994,6 +994,18 @@ config GENERATE_SMBIOS_TABLE > See also SMBIOS_SYSINFO which allows SMBIOS values to be provided in > the devicetree. > >+config SMBIOS_TABLE_FIXED >+ bool "Place the SMBIOS table at a special address" >+ depends on GENERATE_SMBIOS_TABLE && ARM64 && SMBIOS && EFI_LOADER >+ default y >+ help >+Use this option to place the SMBIOS table at a special address. >+ >+U-Boot typically sets up its malloc() pool near the top of memory. On >+ARM64 systems this can result in an SMBIOS table above 4GB which is >+not supported by SMBIOSv2. This option works around this problem by >+chosing an address just below 4GB, if needed. >+ > endmenu > > config LIB_RATIONAL >diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c >index 48446f654d9b..bdbce4c4d785 100644 >--- a/lib/efi_loader/efi_smbios.c >+++ b/lib/efi_loader/efi_smbios.c >@@ -47,6 +47,60 @@ efi_status_t efi_smbios_register(void) > map_sysmem(addr, 0)); > } > >+/** >+ * find_addr_below() - Find a usable region below the given max_addr >+ * >+ * Check if *addrp is suitable to define a memory region which finishes below >+ * @max_addr + @req_size. If so, do nothing and return 0 >+ * >+ * As a backup, if CONFIG_SMBIOS_TABLE_FIXED is enabled, search for a >+ * 4KB-aligned DRAM region which is large enough. Make sure it is below >U-Boot's >+ * stack space, assumed to be 64KB. >+ * >+ * @max_addr: Maximum address that can be used (region must finish before >here) >+ * @req:size: Required size of region >+ * @addrp: On entry: Current proposed address; on exit, holds the new address, >+ *on success >+ * Return 0 if OK (existing region was OK, or a new one was found), -ENOSPC if >+ * nothing suitable was found >+ */ >+static int find_addr_below(ulong max_addr, ulong req_size, ulong *addrp) >+{ >+ struct bd_info *bd = gd->bd; >+ ulong max_base; >+ int i; >+ >+ max_base = max_addr - req_size; >+ if (*addrp <= max_base) >+ return 0; >+ >+ if (!IS_ENABLED(CONFIG_SMBIOS_TABLE_FIXED)) >+ return -ENOSPC; >+ >+ /* Make sure that the base is at least 64KB below the stack */ >+ max_base = min(max_base, >+ ALIGN(gd->start_addr_sp - SZ_64K - req_size, SZ_4K)); >+ >+ for (i = CONFIG_NR_DRAM_BANKS - 1; i >= 0; i--) { >+ ulong start = bd->bi_dram[i].start; >+ ulong size = bd->bi_dram[i].size; >+ ulong addr; >+ >+ /* chose an address at most req_size bytes before the end */ >+ addr = min(max_base, start - req_size + size); >+ >+ /* check this is in the range */ >+ if (addr >= start && addr + req_size < start + size) { >+ *addrp = addr; >+ log_warning("Forcing SMBIOS table to address %lx\n", >+ addr); >+ return 0; >+ } >+ } >+ >+ return -ENOSPC; >+} >+ > static int install_smbios_table(void) > { > ulong addr; >@@ -61,7 +115,14 @@ static int install_smbios_table(void) > return log_msg_ret("mem", -ENOMEM); > > addr = map_to_sysmem(buf); >- if (!write_smbios_table(addr)) { >+ >+ /* >+ * Deal with a fixed address if needed. For simplicity we assume that >+ * the SMBIOS-table size is <64KB. If a suitable address cannot be >+ * found, then write_smbios_table() returns an error. >+ */ >+ if (find_addr_below(SZ_4G, SZ_64K, ) || >+ !write_smbios_table(addr)) { > log_err("Failed to write SMBIOS table\n"); > return log_msg_ret("smbios", -EINVAL); > }
Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr
On Wed, Oct 25, 2023 at 02:19:59AM +0200, Heinrich Schuchardt wrote: > > > Am 25. Oktober 2023 01:28:10 MESZ schrieb Simon Glass : > >Hi Tom, > > > >On Tue, 24 Oct 2023 at 15:34, Tom Rini wrote: > >> > >> On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote: > >> > > From: Simon Glass > >> > > Date: Mon, 23 Oct 2023 00:04:14 -0700 > >> > > > >> > > Hi Caleb, > >> > > > >> > > On Sat, 21 Oct 2023 at 01:43, Caleb Connolly > >> > > wrote: > >> > > > > >> > > > Hi Simon, > >> > > > > >> > > > On 21/10/2023 01:45, Simon Glass wrote: > >> > > > > U-Boot typically sets up its malloc() pool near the top of memory. > >> > > > > On > >> > > > > ARM64 systems this can result in an SMBIOS table above 4GB which is > >> > > > > not supported by SMBIOSv2. > >> [snip] > >> > There is absolutely no guarantee that arm64 machines have memory below > >> > 4GB. Examples of SoCs that have no memory below 4GB are AMD's Opteron > >> > A1100 SoC and all the recent Apple SoCs. > >> > >> So one thing to resolve here is where does that requirement about the > >> SMBIOS table needing to be below 4GB come from (standards wise), and in > >> turn is that obeyed by consumers like say Linux or OpenBSD? Answering my > >> own question, maybe in part, https://www.dmtf.org/standards/smbios reads > >> to me like there's a v3 and maybe we should be doing what we need to > >> support / identify as that, if it doesn't have that restriction? > > > >Yes that was my previous patch. However 1) we apparently don't want to > >use SMBIOS3 and 2) my patch had some sort of bug so that it wasn't > >read correctly. > > > >So my next version is going to be along the lines of what was discussed here. > > > >Of course, we cannot solve Mark's problem with SMBIOS2, but I suppose > >that is obvious. Anyway, those platforms probably don't need SMBIOS. > > You are heading in the wrong direction. We need SMBIOS 3. SMBIOS 2 is > only a fallback for outdated tools. > > Upcoming mainline RISC-V platforms will also have memory starting above 4GiB. I want to echo this because yes, SMBIOS if I'm recalling things right is one of those tools used to provide user friendly information about the system, so "everyone" wants it, or at least platforms like ones Mark is concerned about that have more human users than embedded system users, would like to show the information. Maybe part of the answer moving forward is to allow being specific about SMBIOS2 or SMBIOS3 (or none) so that the issue you had to fix can also be addressed minimally? -- Tom signature.asc Description: PGP signature
Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr
Am 25. Oktober 2023 01:28:10 MESZ schrieb Simon Glass : >Hi Tom, > >On Tue, 24 Oct 2023 at 15:34, Tom Rini wrote: >> >> On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote: >> > > From: Simon Glass >> > > Date: Mon, 23 Oct 2023 00:04:14 -0700 >> > > >> > > Hi Caleb, >> > > >> > > On Sat, 21 Oct 2023 at 01:43, Caleb Connolly >> > > wrote: >> > > > >> > > > Hi Simon, >> > > > >> > > > On 21/10/2023 01:45, Simon Glass wrote: >> > > > > U-Boot typically sets up its malloc() pool near the top of memory. On >> > > > > ARM64 systems this can result in an SMBIOS table above 4GB which is >> > > > > not supported by SMBIOSv2. >> [snip] >> > There is absolutely no guarantee that arm64 machines have memory below >> > 4GB. Examples of SoCs that have no memory below 4GB are AMD's Opteron >> > A1100 SoC and all the recent Apple SoCs. >> >> So one thing to resolve here is where does that requirement about the >> SMBIOS table needing to be below 4GB come from (standards wise), and in >> turn is that obeyed by consumers like say Linux or OpenBSD? Answering my >> own question, maybe in part, https://www.dmtf.org/standards/smbios reads >> to me like there's a v3 and maybe we should be doing what we need to >> support / identify as that, if it doesn't have that restriction? > >Yes that was my previous patch. However 1) we apparently don't want to >use SMBIOS3 and 2) my patch had some sort of bug so that it wasn't >read correctly. > >So my next version is going to be along the lines of what was discussed here. > >Of course, we cannot solve Mark's problem with SMBIOS2, but I suppose >that is obvious. Anyway, those platforms probably don't need SMBIOS. You are heading in the wrong direction. We need SMBIOS 3. SMBIOS 2 is only a fallback for outdated tools. Upcoming mainline RISC-V platforms will also have memory starting above 4GiB. Best regards Heinrich > >Regards, >Simon
[PATCH v2] smbios: arm64: Allow table to be written at a fixed addr
U-Boot typically sets up its malloc() pool near the top of memory. On ARM64 systems this can result in an SMBIOS table above 4GB which is not supported by SMBIOSv2. Work around this problem by providing a new option to choose an address below 4GB (but as high as possible), if needed. Signed-off-by: Simon Glass --- Changes in v2: - Update to search for a suitable area automatically, if enabled lib/Kconfig | 12 +++ lib/efi_loader/efi_smbios.c | 63 - 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/lib/Kconfig b/lib/Kconfig index f6ca559897e7..a1eec98b392f 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -994,6 +994,18 @@ config GENERATE_SMBIOS_TABLE See also SMBIOS_SYSINFO which allows SMBIOS values to be provided in the devicetree. +config SMBIOS_TABLE_FIXED + bool "Place the SMBIOS table at a special address" + depends on GENERATE_SMBIOS_TABLE && ARM64 && SMBIOS && EFI_LOADER + default y + help + Use this option to place the SMBIOS table at a special address. + + U-Boot typically sets up its malloc() pool near the top of memory. On + ARM64 systems this can result in an SMBIOS table above 4GB which is + not supported by SMBIOSv2. This option works around this problem by + chosing an address just below 4GB, if needed. + endmenu config LIB_RATIONAL diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c index 48446f654d9b..bdbce4c4d785 100644 --- a/lib/efi_loader/efi_smbios.c +++ b/lib/efi_loader/efi_smbios.c @@ -47,6 +47,60 @@ efi_status_t efi_smbios_register(void) map_sysmem(addr, 0)); } +/** + * find_addr_below() - Find a usable region below the given max_addr + * + * Check if *addrp is suitable to define a memory region which finishes below + * @max_addr + @req_size. If so, do nothing and return 0 + * + * As a backup, if CONFIG_SMBIOS_TABLE_FIXED is enabled, search for a + * 4KB-aligned DRAM region which is large enough. Make sure it is below U-Boot's + * stack space, assumed to be 64KB. + * + * @max_addr: Maximum address that can be used (region must finish before here) + * @req:size: Required size of region + * @addrp: On entry: Current proposed address; on exit, holds the new address, + * on success + * Return 0 if OK (existing region was OK, or a new one was found), -ENOSPC if + * nothing suitable was found + */ +static int find_addr_below(ulong max_addr, ulong req_size, ulong *addrp) +{ + struct bd_info *bd = gd->bd; + ulong max_base; + int i; + + max_base = max_addr - req_size; + if (*addrp <= max_base) + return 0; + + if (!IS_ENABLED(CONFIG_SMBIOS_TABLE_FIXED)) + return -ENOSPC; + + /* Make sure that the base is at least 64KB below the stack */ + max_base = min(max_base, + ALIGN(gd->start_addr_sp - SZ_64K - req_size, SZ_4K)); + + for (i = CONFIG_NR_DRAM_BANKS - 1; i >= 0; i--) { + ulong start = bd->bi_dram[i].start; + ulong size = bd->bi_dram[i].size; + ulong addr; + + /* chose an address at most req_size bytes before the end */ + addr = min(max_base, start - req_size + size); + + /* check this is in the range */ + if (addr >= start && addr + req_size < start + size) { + *addrp = addr; + log_warning("Forcing SMBIOS table to address %lx\n", + addr); + return 0; + } + } + + return -ENOSPC; +} + static int install_smbios_table(void) { ulong addr; @@ -61,7 +115,14 @@ static int install_smbios_table(void) return log_msg_ret("mem", -ENOMEM); addr = map_to_sysmem(buf); - if (!write_smbios_table(addr)) { + + /* +* Deal with a fixed address if needed. For simplicity we assume that +* the SMBIOS-table size is <64KB. If a suitable address cannot be +* found, then write_smbios_table() returns an error. +*/ + if (find_addr_below(SZ_4G, SZ_64K, ) || + !write_smbios_table(addr)) { log_err("Failed to write SMBIOS table\n"); return log_msg_ret("smbios", -EINVAL); } -- 2.42.0.758.gaed0368e0e-goog
Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr
Hi Tom, On Tue, 24 Oct 2023 at 15:34, Tom Rini wrote: > > On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote: > > > From: Simon Glass > > > Date: Mon, 23 Oct 2023 00:04:14 -0700 > > > > > > Hi Caleb, > > > > > > On Sat, 21 Oct 2023 at 01:43, Caleb Connolly > > > wrote: > > > > > > > > Hi Simon, > > > > > > > > On 21/10/2023 01:45, Simon Glass wrote: > > > > > U-Boot typically sets up its malloc() pool near the top of memory. On > > > > > ARM64 systems this can result in an SMBIOS table above 4GB which is > > > > > not supported by SMBIOSv2. > [snip] > > There is absolutely no guarantee that arm64 machines have memory below > > 4GB. Examples of SoCs that have no memory below 4GB are AMD's Opteron > > A1100 SoC and all the recent Apple SoCs. > > So one thing to resolve here is where does that requirement about the > SMBIOS table needing to be below 4GB come from (standards wise), and in > turn is that obeyed by consumers like say Linux or OpenBSD? Answering my > own question, maybe in part, https://www.dmtf.org/standards/smbios reads > to me like there's a v3 and maybe we should be doing what we need to > support / identify as that, if it doesn't have that restriction? Yes that was my previous patch. However 1) we apparently don't want to use SMBIOS3 and 2) my patch had some sort of bug so that it wasn't read correctly. So my next version is going to be along the lines of what was discussed here. Of course, we cannot solve Mark's problem with SMBIOS2, but I suppose that is obvious. Anyway, those platforms probably don't need SMBIOS. Regards, Simon
Re: [PATCH] .gitignore: ignore misc include, simple-bin, and tools/generated build artifacts
On Fri, Oct 13, 2023 at 01:26:16PM +, John Clark wrote: > make rock5b-rk3588_defconfig > make > git status > > before > ~~~ > On branch master > Your branch is ahead of 'origin/master' by 1 commit. > (use "git push" to publish your local commits) > > Untracked files: > (use "git add ..." to include in what will be committed) > include/autoconf.mk > include/autoconf.mk.dep > include/config.h > mkimage-in-simple-bin-spi.mkimage-rockchip-tpl > mkimage-in-simple-bin-spi.mkimage-u-boot-spl > mkimage-in-simple-bin.mkimage-rockchip-tpl > mkimage-in-simple-bin.mkimage-u-boot-spl > simple-bin-spi.map > simple-bin.fit.fit > simple-bin.fit.itb > simple-bin.map > tools/generated/ > > after > ~~~ > On branch master > Your branch is ahead of 'origin/master' by 1 commit. > (use "git push" to publish your local commits) > > nothing to commit, working tree clean > > Signed-off-by: John Clark > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 4/4] test: dm: add SCMI power domain protocol test
On Mon, Oct 16, 2023 at 02:39:46PM +0900, AKASHI Takahiro wrote: > This ut has tests for the SCMI power domain protocol as well as DM > interfaces for power domain devices. > > Signed-off-by: AKASHI Takahiro > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 2/2] serial: introduce CONFIG_CONSOLE_FLUSH_ON_NEWLINE
On Mon, Oct 16, 2023 at 10:35:22AM +0200, Rasmus Villemoes wrote: > When debugging, one sometimes only gets partial output lines or > nothing at all from the last printf, because the uart has a largish > buffer, and the code after the printf() may cause the CPU to hang > before the uart IP has time to actually emit all the characters. That > can be very confusing, because one doesn't then know exactly where the > hang happens. > > Introduce a config knob allowing one to wait for the uart fifo to > drain whenever a newline character is printed, roughly corresponding > to the effect of setvbuf(..., _IOLBF, ...) in ordinary C programs. > > Since this uses IS_ENABLED() instead of cpp ifdef, we can remove the > ifdef around the _serial_flush() definition - if neither > CONSOLE_FLUSH_SUPPORT or CONSOLE_FLUSH_ON_NEWLINE are enabled, the > compiler elides _serial_flush(), but it won't warn about it being > unused. > > Signed-off-by: Rasmus Villemoes > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
[PATCH] arm: kirkwood: Enable bootstd for Pogo V4 board
Enable bootstd for Pogo V4 board, and remove distroboot. Signed-off-by: Tony Dinh --- configs/pogo_v4_defconfig | 3 ++- include/configs/pogo_v4.h | 35 ++- 2 files changed, 4 insertions(+), 34 deletions(-) diff --git a/configs/pogo_v4_defconfig b/configs/pogo_v4_defconfig index 2747c3c72d..ff6411d894 100644 --- a/configs/pogo_v4_defconfig +++ b/configs/pogo_v4_defconfig @@ -17,7 +17,8 @@ CONFIG_IDENT_STRING="\nPogoplug V4" CONFIG_SYS_LOAD_ADDR=0x80 CONFIG_PCI=y CONFIG_LTO=y -CONFIG_DISTRO_DEFAULTS=y +CONFIG_BOOTSTD_FULL=y +CONFIG_BOOTSTD_DEFAULTS=y CONFIG_BOOTSTAGE=y CONFIG_SHOW_BOOT_PROGRESS=y CONFIG_BOOTDELAY=10 diff --git a/include/configs/pogo_v4.h b/include/configs/pogo_v4.h index 3371579023..d5003538da 100644 --- a/include/configs/pogo_v4.h +++ b/include/configs/pogo_v4.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0+ */ /* - * Copyright (C) 2014-2022 Tony Dinh + * Copyright (C) 2014-2023 Tony Dinh * * Based on * Copyright (C) 2012 @@ -21,33 +21,6 @@ */ #include "mv-common.h" -/* Include the common distro boot environment */ -#ifndef CONFIG_SPL_BUILD - -#ifdef CONFIG_MMC -#define BOOT_TARGET_DEVICES_MMC(func) func(MMC, mmc, 0) -#else -#define BOOT_TARGET_DEVICES_MMC(func) -#endif - -#ifdef CONFIG_SATA -#define BOOT_TARGET_DEVICES_SATA(func) func(SATA, sata, 0) -#else -#define BOOT_TARGET_DEVICES_SATA(func) -#endif - -#ifdef CONFIG_USB_STORAGE -#define BOOT_TARGET_DEVICES_USB(func) func(USB, usb, 0) -#else -#define BOOT_TARGET_DEVICES_USB(func) -#endif - -#define BOOT_TARGET_DEVICES(func) \ - BOOT_TARGET_DEVICES_MMC(func) \ - BOOT_TARGET_DEVICES_USB(func) \ - BOOT_TARGET_DEVICES_SATA(func) \ - func(DHCP, dhcp, na) - #define KERNEL_ADDR_R __stringify(0x80) #define FDT_ADDR_R __stringify(0x2c0) #define RAMDISK_ADDR_R __stringify(0x0110) @@ -59,14 +32,10 @@ "ramdisk_addr_r=" RAMDISK_ADDR_R "\0" \ "scriptaddr=" SCRIPT_ADDR_R "\0" -#include - #define CFG_EXTRA_ENV_SETTINGS \ LOAD_ADDRESS_ENV_SETTINGS \ "fdtfile=" CONFIG_DEFAULT_DEVICE_TREE ".dtb\0" \ "mtdparts=" CONFIG_MTDPARTS_DEFAULT "\0" \ - "console=ttyS0,115200\0" \ - BOOTENV -#endif /* CONFIG_SPL_BUILD */ + "console=ttyS0,115200\0" #endif /* _CONFIG_POGO_V4_H */ -- 2.39.2
Re: [PATCH v2 2/4] power: domain: add SCMI driver
On Mon, Oct 16, 2023 at 02:39:44PM +0900, AKASHI Takahiro wrote: > Add power domain driver based on SCMI power domain management protocol. > > Signed-off-by: AKASHI Takahiro Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 1/4] firmware: scmi: add power domain protocol support
On Mon, Oct 16, 2023 at 02:39:43PM +0900, AKASHI Takahiro wrote: > In this patch, added are helper functions to directly manipulate > SCMI power domain management protocol. DM compliant power domain > driver will be implemented on top of those interfaces in a succeeding > patch. > > Signed-off-by: AKASHI Takahiro Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 3/4] sandbox: add SCMI power domain protocol support for testing
On Mon, Oct 16, 2023 at 02:39:45PM +0900, AKASHI Takahiro wrote: > SCMI power domain management protocol is supported on sandbox > for test purpose. Add fake agent interfaces and associated > power domain devices. > > Signed-off-by: AKASHI Takahiro > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v3] cli: Consume invalid escape sequences early
On Tue, Oct 10, 2023 at 11:16:39AM +0300, Yurii Monakov wrote: > Unexpected 'Esc' key presses are accumulated internally, even if it is > already clear that the current escape sequence is invalid. This results > in weird behaviour. For example, the next character after 'Esc' key > simply disappears from input and 'Unknown command' is printed > after 'Enter'. > > This commit fixes some issues with extra 'Esc' keys entered by user: > > 1. Sequence right after autoboot stop gives: > => > nknown command 'ry 'help' > => > 2. Sequence gives: > => ri > Unknown command 'ri' - try 'help' > => > 3. Extra 'Esc' key presses break backspace functionality. > > Signed-off-by: Yurii Monakov > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 1/2] serial: serial-uclass.c: move definition of _serial_flush up a bit
On Mon, Oct 16, 2023 at 10:35:21AM +0200, Rasmus Villemoes wrote: > Preparation for next patch. > > Reviewed-by: Simon Glass > Signed-off-by: Rasmus Villemoes Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] Makefile: remove misc include and simple-bin build artifacts on clean
On Fri, Oct 13, 2023 at 01:23:07PM +, John Clark wrote: > make rock5b-rk3588_defconfig > make > make clean > git status > > before > ~~~ > On branch master > Your branch is up to date with 'origin/master'. > > Untracked files: > (use "git add ..." to include in what will be committed) > include/autoconf.mk > include/autoconf.mk.dep > include/config.h > mkimage-in-simple-bin-spi.mkimage-rockchip-tpl > mkimage-in-simple-bin-spi.mkimage-u-boot-spl > mkimage-in-simple-bin.mkimage-rockchip-tpl > mkimage-in-simple-bin.mkimage-u-boot-spl > simple-bin.fit.fit > simple-bin.fit.itb > > after > ~~~ > On branch master > Your branch is ahead of 'origin/master' by 1 commit. > (use "git push" to publish your local commits) > > nothing to commit, working tree clean > > Signed-off-by: John Clark Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] pstore: Use root address-cells/size-cells as defaults for reserved-memory
On Sat, Aug 26, 2023 at 03:16:52PM +0300, Andrey Skvortsov wrote: > u-boot adds reserve-memory node, if it's missing, with following > properties: > > ``` > reserved-memory { > #address-cells = <2>; > #size-cells = <2>; > ranges; > } > ``` > > But with these default address-cells and size-cells values, pstore > isn't working on A64. Root node for A64 defines 'address-cells' and > 'size-cells' as 1. > > dtc complains if reserved-memory has different address-cells and > size-cells. > > ``` > Warning (ranges_format): /reserved-memory:ranges: empty "ranges" > property but its #address-cells (2) differs from / (1) > ``` > > This patch takes into account address-cells and size-cells of the root > node and uses them as values for new reserved-memory node. > > Signed-off-by: Andrey Skvortsov Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] mbr: Test tweaks
On Mon, Oct 16, 2023 at 04:50:46PM -0600, Simon Glass wrote: > Rather than do another review on [1] I thought it better to add a patch > showing changes. This is intended to be squashed in, if acceptable. > > With the patch as is, various bootstd tests fail. This is because the > test adds a new mmc6 device. There is nothing really wrong with adding > new devices, but it does require quite a few tests to be updated. It is > simpler to have mmc6 disabled by default, then just enable it for the > one test that uses it. > > This patch does that and undoes the changes to blk.c and bootdev.c > > [1] > https://patchwork.ozlabs.org/project/uboot/patch/ZSNWVGTjPEt2jXn5@alg-gentoo/ > > Signed-off-by: Simon Glass I've folded this in to the original patch while applying. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2] cmd: mbr: Allow 4 MBR partitions without need for extended
On Mon, Oct 09, 2023 at 01:24:36AM +, Alexander Gendin wrote: > Current code allows up to 3 MBR partitions without extended one. > If more than 3 partitions are required, then extended partition(s) > must be used. > This commit allows up to 4 primary MBR partitions without the > need for extended partition. > > Add mbr test unit. In order to run the test manually, mmc6.img file > of size 12 MiB or greater is required in the same directory as u-boot. > Test also runs automatically via ./test/py/test.py tool. > Running mbr test is only supported in sandbox mode. > > Signed-off-by: Alex Gendin Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 1/2] checkpatch.pl: Make common.h check boarder
On Fri, Oct 13, 2023 at 09:28:32AM -0700, Simon Glass wrote: > From: Tom Rini > > At this point in time we should not add common.h to any new files, so > make checkpatch.pl complain. > > Signed-off-by: Tom Rini > Signed-off-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 2/2] patman: Add a little documentation on the checkpatch tests
On Fri, Oct 13, 2023 at 09:28:33AM -0700, Simon Glass wrote: > These texts lack comments. Add some so that it is clearer what is going > on. > > Signed-off-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 7/7] riscv: Remove common.h usage
On Thu, Oct 12, 2023 at 07:03:59PM -0400, Tom Rini wrote: > We can remove common.h from most cases of the code here, and only a few > places need an additional header instead. > > Signed-off-by: Tom Rini > Reviewed-by: Rick Chen Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 5/7] microblaze: Remove common.h usage
On Thu, Oct 12, 2023 at 07:03:57PM -0400, Tom Rini wrote: > We can remove common.h from most cases of the code here, and only a few > places need an additional header instead. > > Signed-off-by: Tom Rini > Acked-by: Michal Simek Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 6/7] mips: Remove common.h usage
On Thu, Oct 12, 2023 at 07:03:58PM -0400, Tom Rini wrote: > We can remove common.h from most cases of the code here, and only a few > places need an additional header instead. > > Signed-off-by: Tom Rini Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 3/7] arc: Remove common.h usage
On Thu, Oct 12, 2023 at 07:03:55PM -0400, Tom Rini wrote: > We can remove common.h from most cases of the code here, and only a few > places need an additional header instead. > > Signed-off-by: Tom Rini > Acked-by: Alexey Brodkin Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 4/7] m68k: Remove common.h usage
On Thu, Oct 12, 2023 at 07:03:56PM -0400, Tom Rini wrote: > We can remove common.h from most cases of the code here, and only a few > places need an additional header instead. > > Signed-off-by: Tom Rini > Acked-by: Angelo Dureghello Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 2/7] include: Add in a few places
On Thu, Oct 12, 2023 at 07:03:54PM -0400, Tom Rini wrote: > These files references a number of types that are defined in > (and so forth), so include it here rather than rely on > indirect inclusion. > > Signed-off-by: Tom Rini > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 1/4] spl: atf: Improve comments and code readability
On Sat, Sep 16, 2023 at 10:23:01AM +0200, Massimo Pegorer wrote: > Rename bl31_entry static function to avoid name clash with its first > argument. Fix spaces misuse. Describe code accurately: load address > is used if getting entry point address fails, and not if addresses > differ. Remove not up-to-date comment about BL3-2 usage. > > Signed-off-by: Massimo Pegorer > Reviewed-by: Simon Glass > --- > common/spl/spl_atf.c | 34 +++--- > 1 file changed, 15 insertions(+), 19 deletions(-) Please rebase this on current master if this is still helpful, thanks and sorry for the delay. -- Tom signature.asc Description: PGP signature
Re: [PATCH] misc: i2c_eeprom: consider pagesize when writing to eeprom
On Thu, Sep 21, 2023 at 12:29:45PM +, Michel Alex wrote: > Calculate the maximum length of the buffer when writing > accross the page boundary. If the buffer length exceeds > the page boundary, split it. Use this length instead of > comparing the length with the pagesize, because the write > start address can be somewhere in the middle of a page. > > Signed-off-by: Alex Michel > --- > drivers/misc/i2c_eeprom.c | 13 - > 1 file changed, 12 insertions(+), 1 deletion(-) Please fix the checkpatch.pl errors with this patch, thanks. -- Tom signature.asc Description: PGP signature
[PATCH v2 2/2] ARM: amlogic: ad401: enable SPIFC
Enable Amlogic A1 SPI FLash Controller support. Signed-off-by: Igor Prusov Reviewed-by: Neil Armstrong --- configs/ad401_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/ad401_defconfig b/configs/ad401_defconfig index b9aca3ab0d..b44b9c63e8 100644 --- a/configs/ad401_defconfig +++ b/configs/ad401_defconfig @@ -51,6 +51,7 @@ CONFIG_DEBUG_UART_SKIP_INIT=y CONFIG_MESON_SERIAL=y CONFIG_SPI=y CONFIG_DM_SPI=y +CONFIG_MESON_SPIFC_A1=y CONFIG_USB=y CONFIG_DM_USB_GADGET=y CONFIG_USB_GADGET=y -- 2.34.1
[PATCH v2 1/2] spi: add support for Amlogic A1 SPI Flash Controller
From: Igor Prusov Add A1 SPIFC driver from Linux. Slightly modified to use u-boot driver framework and accommodate to lack of ioread32_rep/iowrite32_rep. Based on Linux version 6.6-rc4 Signed-off-by: Igor Prusov Signed-off-by: Martin Kurbanov Reviewed-by: Simon Glass --- drivers/spi/Kconfig | 9 + drivers/spi/Makefile | 1 + drivers/spi/meson_spifc_a1.c | 384 +++ 3 files changed, 394 insertions(+) create mode 100644 drivers/spi/meson_spifc_a1.c diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 854b8b88da..66cf727113 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -251,6 +251,15 @@ config MICROCHIP_COREQSPI Enable the QSPI driver for Microchip FPGA QSPI controllers. This driver can be used on Polarfire SoC. +config MESON_SPIFC_A1 + bool "Amlogic Meson A1 SPI Flash Controller driver" + depends on ARCH_MESON + help + Enable the Amlogic A1 SPI Flash Controller (SPIFC) driver. + This driver can be used to access the SPI NOR/NAND flash chips + with STR mode frequency up to 98MHz. Dual and quad modes are + supported by controller. + config MPC8XX_SPI bool "MPC8XX SPI Driver" depends on MPC8xx diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index c27b3327c3..14bdb97f18 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -41,6 +41,7 @@ obj-$(CONFIG_ICH_SPI) += ich.o obj-$(CONFIG_IPROC_QSPI) += iproc_qspi.o obj-$(CONFIG_KIRKWOOD_SPI) += kirkwood_spi.o obj-$(CONFIG_MESON_SPIFC) += meson_spifc.o +obj-$(CONFIG_MESON_SPIFC_A1) += meson_spifc_a1.o obj-$(CONFIG_MICROCHIP_COREQSPI) += microchip_coreqspi.o obj-$(CONFIG_MPC8XX_SPI) += mpc8xx_spi.o obj-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o diff --git a/drivers/spi/meson_spifc_a1.c b/drivers/spi/meson_spifc_a1.c new file mode 100644 index 00..7cefb03197 --- /dev/null +++ b/drivers/spi/meson_spifc_a1.c @@ -0,0 +1,384 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Driver for Amlogic A1 SPI flash controller (SPIFC) + * + * Copyright (c) 2023, SberDevices. All Rights Reserved. + * + * Author: Martin Kurbanov + * + * Ported to u-boot: + * Author: Igor Prusov + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define SPIFC_A1_AHB_CTRL_REG 0x0 +#define SPIFC_A1_AHB_BUS_ENBIT(31) + +#define SPIFC_A1_USER_CTRL0_REG0x200 +#define SPIFC_A1_USER_REQUEST_ENABLE BIT(31) +#define SPIFC_A1_USER_REQUEST_FINISH BIT(30) +#define SPIFC_A1_USER_DATA_UPDATED BIT(0) + +#define SPIFC_A1_USER_CTRL1_REG0x204 +#define SPIFC_A1_USER_CMD_ENABLE BIT(30) +#define SPIFC_A1_USER_CMD_MODE GENMASK(29, 28) +#define SPIFC_A1_USER_CMD_CODE GENMASK(27, 20) +#define SPIFC_A1_USER_ADDR_ENABLE BIT(19) +#define SPIFC_A1_USER_ADDR_MODEGENMASK(18, 17) +#define SPIFC_A1_USER_ADDR_BYTES GENMASK(16, 15) +#define SPIFC_A1_USER_DOUT_ENABLE BIT(14) +#define SPIFC_A1_USER_DOUT_MODEGENMASK(11, 10) +#define SPIFC_A1_USER_DOUT_BYTES GENMASK(9, 0) + +#define SPIFC_A1_USER_CTRL2_REG0x208 +#define SPIFC_A1_USER_DUMMY_ENABLE BIT(31) +#define SPIFC_A1_USER_DUMMY_MODE GENMASK(30, 29) +#define SPIFC_A1_USER_DUMMY_CLK_SYCLES GENMASK(28, 23) + +#define SPIFC_A1_USER_CTRL3_REG0x20c +#define SPIFC_A1_USER_DIN_ENABLE BIT(31) +#define SPIFC_A1_USER_DIN_MODE GENMASK(28, 27) +#define SPIFC_A1_USER_DIN_BYTESGENMASK(25, 16) + +#define SPIFC_A1_USER_ADDR_REG 0x210 + +#define SPIFC_A1_AHB_REQ_CTRL_REG 0x214 +#define SPIFC_A1_AHB_REQ_ENABLEBIT(31) + +#define SPIFC_A1_ACTIMING0_REG (0x0088 << 2) +#define SPIFC_A1_TSLCH GENMASK(31, 30) +#define SPIFC_A1_TCLSH GENMASK(29, 28) +#define SPIFC_A1_TSHWL GENMASK(20, 16) +#define SPIFC_A1_TSHSL2GENMASK(15, 12) +#define SPIFC_A1_TSHSL1GENMASK(11, 8) +#define SPIFC_A1_TWHSL GENMASK(7, 0) + +#define SPIFC_A1_DBUF_CTRL_REG 0x240 +#define SPIFC_A1_DBUF_DIR BIT(31) +#define SPIFC_A1_DBUF_AUTO_UPDATE_ADDR BIT(30) +#define SPIFC_A1_DBUF_ADDR GENMASK(7, 0) + +#define SPIFC_A1_DBUF_DATA_REG 0x244 + +#define SPIFC_A1_USER_DBUF_ADDR_REG0x248 + +#define SPIFC_A1_BUFFER_SIZE 512U + +#define SPIFC_A1_MAX_HZ2 +#define SPIFC_A1_MIN_HZ100 + +#define SPIFC_A1_USER_CMD(op) ( \ + SPIFC_A1_USER_CMD_ENABLE | \ + FIELD_PREP(SPIFC_A1_USER_CMD_CODE, (op)->cmd.opcode) | \ + FIELD_PREP(SPIFC_A1_USER_CMD_MODE, ilog2((op)->cmd.buswidth))) + +#define SPIFC_A1_USER_ADDR(op) ( \ + SPIFC_A1_USER_ADDR_ENABLE | \ + FIELD_PREP(SPIFC_A1_USER_ADDR_MODE, ilog2((op)->addr.buswidth)) |
[PATCH v2 0/2] ARM: amlogic: Add A1 SPIFC support
A1 family boards have new version of SPIFC controller, that is incompatible with meson_spifc driver. This series ports A1 SPIFC driver from Linux and enables it for ad401 board. Changes in V2: - more details in Kconfig help message - removed unused field of struct amlogic_spifc_a1 - add missed check for NULL during probe Igor Prusov (2): spi: add support for Amlogic A1 SPI Flash Controller ARM: amlogic: ad401: enable SPIFC configs/ad401_defconfig | 1 + drivers/spi/Kconfig | 9 + drivers/spi/Makefile | 1 + drivers/spi/meson_spifc_a1.c | 384 +++ 4 files changed, 395 insertions(+) create mode 100644 drivers/spi/meson_spifc_a1.c -- 2.34.1
Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr
On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote: > > From: Simon Glass > > Date: Mon, 23 Oct 2023 00:04:14 -0700 > > > > Hi Caleb, > > > > On Sat, 21 Oct 2023 at 01:43, Caleb Connolly > > wrote: > > > > > > Hi Simon, > > > > > > On 21/10/2023 01:45, Simon Glass wrote: > > > > U-Boot typically sets up its malloc() pool near the top of memory. On > > > > ARM64 systems this can result in an SMBIOS table above 4GB which is > > > > not supported by SMBIOSv2. [snip] > There is absolutely no guarantee that arm64 machines have memory below > 4GB. Examples of SoCs that have no memory below 4GB are AMD's Opteron > A1100 SoC and all the recent Apple SoCs. So one thing to resolve here is where does that requirement about the SMBIOS table needing to be below 4GB come from (standards wise), and in turn is that obeyed by consumers like say Linux or OpenBSD? Answering my own question, maybe in part, https://www.dmtf.org/standards/smbios reads to me like there's a v3 and maybe we should be doing what we need to support / identify as that, if it doesn't have that restriction? -- Tom signature.asc Description: PGP signature
Re: [PATCH RESEND v3 1/1] arm: dts: icnova-a20-adb4006: Add board support
On Mon, 9 Oct 2023 13:39:16 +0200 Ludwig Kormann wrote: Hi Ludwig, > Add board support for ICnova A20 SomPi compute module on > ICnova ADB4006 development board. > > Specification: > SoM > - Processor: Allwinner A20 Cortex-A7 Dual Core at 1GHz > - 512MB DDR3 RAM > - Fast Ethernet (Phy: Realtek RTL8201CP) > ADB4006 > - I2C > - 2x USB 2.0 > - 1x Fast Ethernet port > - 1x SATA > - 2x buttons (PWRON, Boot) > - 2x LEDS > - serial console > - HDMI > - µSD-Card slot > - Audio Line-In / Line-Out > - GPIO pinheaders > > https://wiki.in-circuit.de/index.php5?title=ICnova_ADB4006 > https://wiki.in-circuit.de/index.php5?title=ICnova_A20_SODIMM > > devicetree upstreamed with linux 6.5 As you have probably seen, the DT files have been synced into U-Boot's master branch yesterday. So if you rebase this on top of master, so just send the defconfig (with CONFIG_SYS_64BIT_LBA added) and the Makefile change in a new patch, I am happy to take it still this cycle. Cheers, Andre > Signed-off-by: Ludwig Kormann > --- > changes in v3: > - rebase on v2023.10 > > changes in v2: > - rebase on v2023.07-rc2 > - remove pin defines from defconfig > - get dts reviewed on the linux mailing list and > scheduled for kernel 6.5 [1] > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git > commit de2bdfb7f79d5c655eb056d459e02be2c7f13c8b > > --- > arch/arm/dts/Makefile | 1 + > arch/arm/dts/sun7i-a20-icnova-a20-adb4006.dts | 137 ++ > arch/arm/dts/sun7i-a20-icnova-a20.dtsi| 62 > board/sunxi/MAINTAINERS | 5 + > configs/icnova-a20-adb4006_defconfig | 20 +++ > 5 files changed, 225 insertions(+) > create mode 100644 arch/arm/dts/sun7i-a20-icnova-a20-adb4006.dts > create mode 100644 arch/arm/dts/sun7i-a20-icnova-a20.dtsi > create mode 100644 configs/icnova-a20-adb4006_defconfig > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile > index 85fd5b1157..16d5930b78 100644 > --- a/arch/arm/dts/Makefile > +++ b/arch/arm/dts/Makefile > @@ -667,6 +667,7 @@ dtb-$(CONFIG_MACH_SUN7I) += \ > sun7i-a20-haoyu-marsboard.dtb \ > sun7i-a20-hummingbird.dtb \ > sun7i-a20-i12-tvbox.dtb \ > + sun7i-a20-icnova-a20-adb4006.dtb \ > sun7i-a20-icnova-swac.dtb \ > sun7i-a20-itead-ibox.dtb \ > sun7i-a20-lamobo-r1.dtb \ > diff --git a/arch/arm/dts/sun7i-a20-icnova-a20-adb4006.dts > b/arch/arm/dts/sun7i-a20-icnova-a20-adb4006.dts > new file mode 100644 > index 00..577ead1d02 > --- /dev/null > +++ b/arch/arm/dts/sun7i-a20-icnova-a20-adb4006.dts > @@ -0,0 +1,137 @@ > +// SPDX-License-Identifier: (GPL-2.0 OR MIT) > +// Copyright (C) 2023 In-Circuit GmbH > + > +/dts-v1/; > + > +#include "sun7i-a20-icnova-a20.dtsi" > + > +#include > +#include > + > +/ { > + model = "In-Circuit ICnova A20 ADB4006"; > + compatible = "incircuit,icnova-a20-adb4006", "incircuit,icnova-a20", > + "allwinner,sun7i-a20"; > + > + aliases { > + serial0 = > + }; > + > + chosen { > + stdout-path = "serial0:115200n8"; > + }; > + > + hdmi-connector { > + compatible = "hdmi-connector"; > + type = "a"; > + > + port { > + hdmi_con_in: endpoint { > + remote-endpoint = <_out_con>; > + }; > + }; > + }; > + > + leds { > + compatible = "gpio-leds"; > + > + led-0 { > + function = LED_FUNCTION_POWER; > + color = ; > + gpios = < 7 21 GPIO_ACTIVE_HIGH>; /* PH21 */ > + default-state = "on"; > + }; > + > + led-1 { > + function = LED_FUNCTION_HEARTBEAT; > + color = ; > + gpios = < 7 20 GPIO_ACTIVE_HIGH>; /* PH20 */ > + linux,default-trigger = "heartbeat"; > + }; > + }; > +}; > + > + { > + target-supply = <_ahci_5v>; > + status = "okay"; > +}; > + > + { > + status = "okay"; > +}; > + > + { > + status = "okay"; > +}; > + > + { > + status = "okay"; > +}; > + > + { > + status = "okay"; > +}; > + > + { > + status = "okay"; > +}; > + > +_out { > + hdmi_out_con: endpoint { > + remote-endpoint = <_con_in>; > + }; > +}; > + > + { > + vmmc-supply = <_vcc3v3>; > + bus-width = <4>; > + cd-gpios = < 7 1 GPIO_ACTIVE_LOW>; /* PH1 */ > + status = "okay"; > +}; > + > + { > + status = "okay"; > +}; > + > + { > + status = "okay"; > +}; > + > +_sram { > + status = "okay"; > +}; > + > +_ahci_5v { > + status = "okay"; > +}; > + > +_power_supply { > + status = "okay"; > +}; > + > +_usb1_vbus { > + status = "okay"; > +}; > + > +_usb2_vbus { > + status = "okay"; > +}; > + > + { > + pinctrl-names = "default"; > + pinctrl-0 = <_pb_pins>; > + status =
Re: [PATCH v5 14/16] cmd: add scmi command for SCMI firmware
On Tue, Oct 24, 2023 at 10:27:44AM +0200, Michal Simek wrote: > Hi Takahiro, > > út 26. 9. 2023 v 9:00 odesílatel AKASHI Takahiro > napsal: > > > > This command, "scmi", may provide a command line interface to various SCMI > > protocols. It supports at least initially SCMI base protocol and is > > intended mainly for debug purpose. > > > > Signed-off-by: AKASHI Takahiro > > Reviewed-by: Simon Glass > > Reviewed-by: Etienne Carriere > > --- > > v3 > > * describe that arguments are in hex at a help message > > * modify the code for dynamically allocated agent names > > v2 > > * remove sub command category, 'scmi base', for simplicity > > --- > > cmd/Kconfig | 9 ++ > > cmd/Makefile | 1 + > > cmd/scmi.c | 337 +++ > > 3 files changed, 347 insertions(+) > > create mode 100644 cmd/scmi.c > > > > diff --git a/cmd/Kconfig b/cmd/Kconfig > > index 43ca10f69ccf..f46152ace7d8 100644 > > --- a/cmd/Kconfig > > +++ b/cmd/Kconfig > > @@ -2533,6 +2533,15 @@ 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 > > This line above is wrong and this was removed from v6 with > "drop scmi command which was intended to be used for debugging". > It is fine that it shouldn't be used on production system but it > doesn't mean that > it should be actually removed. > It is useful for bring ups. Can we get this patch merged? It was > already reviewed > anyway. There was then also some conflict with the follow-up series here. I would be fine with the command being introduced again after I merge that second series, which I am testing now. -- Tom signature.asc Description: PGP signature
Re: [PATCH v4 4/4] sunxi: psci: implement PSCI on R528
On Sun, 22 Oct 2023 01:49:49 +0100 Andre Przywara wrote: > On Wed, 11 Oct 2023 19:47:56 -0600 > Sam Edwards wrote: > > > This patch adds the necessary code to make nonsec booting and PSCI > > secondary core management functional on the R528/T113. > > > > Signed-off-by: Sam Edwards > > Tested-by: Maksim Kiselev > > Tested-by: Kevin Amadiva > > Thanks, that looks good now. It's debatable whether the Kconfig depends > on should be NCAT2 instead, but we can fix this when the need arises. > > Reviewed-by: Andre Przywara For the records, this patch was missing the definition of SUNXI_R_CPUCFG_BASE, which broke the build of all other 32-bit sunxi boards. I fixed that here: https://lore.kernel.org/u-boot/20231023132449.813863-24-andre.przyw...@arm.com/ and this version was also merged. Cheers, Andre > > > --- > > arch/arm/cpu/armv7/Kconfig | 3 ++- > > arch/arm/cpu/armv7/sunxi/psci.c | 47 - > > arch/arm/mach-sunxi/Kconfig | 4 +++ > > 3 files changed, 52 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/cpu/armv7/Kconfig b/arch/arm/cpu/armv7/Kconfig > > index f015d133cb..4eb34b7b44 100644 > > --- a/arch/arm/cpu/armv7/Kconfig > > +++ b/arch/arm/cpu/armv7/Kconfig > > @@ -61,8 +61,9 @@ config ARMV7_SECURE_MAX_SIZE > > config ARM_GIC_BASE_ADDRESS > > hex > > depends on ARMV7_NONSEC > > - depends on ARCH_EXYNOS5 > > + depends on ARCH_EXYNOS5 || MACH_SUN8I_R528 > > default 0x1048 if ARCH_EXYNOS5 > > + default 0x0302 if MACH_SUN8I_R528 > > help > > Override the GIC base address if the Arm Cortex defined > > CBAR/PERIPHBASE system register holds the wrong value. > > diff --git a/arch/arm/cpu/armv7/sunxi/psci.c > > b/arch/arm/cpu/armv7/sunxi/psci.c > > index 207aa6bc4b..b03a865483 100644 > > --- a/arch/arm/cpu/armv7/sunxi/psci.c > > +++ b/arch/arm/cpu/armv7/sunxi/psci.c > > @@ -47,6 +47,19 @@ > > #define SUN8I_R40_PWR_CLAMP(cpu) (0x120 + (cpu) * 0x4) > > #define SUN8I_R40_SRAMC_SOFT_ENTRY_REG0(0xbc) > > > > +/* > > + * R528 is also different, as it has both cores powered up (but held in > > reset > > + * state) after the SoC is reset. Like the R40, it uses a "soft" entry > > point > > + * address register, but unlike the R40, it uses a newer "CPUX" block to > > manage > > + * CPU state, rather than the older CPUCFG system. > > + */ > > +#define SUN8I_R528_SOFT_ENTRY (0x1c8) > > +#define SUN8I_R528_C0_RST_CTRL (0x) > > +#define SUN8I_R528_C0_CTRL_REG0(0x0010) > > +#define SUN8I_R528_C0_CPU_STATUS (0x0080) > > + > > +#define SUN8I_R528_C0_STATUS_STANDBYWFI(16) > > + > > static void __secure cp15_write_cntp_tval(u32 tval) > > { > > asm volatile ("mcr p15, 0, %0, c14, c2, 0" : : "r" (tval)); > > @@ -103,10 +116,12 @@ static void __secure clamp_set(u32 *clamp) > > > > static void __secure sunxi_cpu_set_entry(int __always_unused cpu, void > > *entry) > > { > > - /* secondary core entry address is programmed differently on R40 */ > > if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) { > > writel((u32)entry, > >SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0); > > + } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) { > > + writel((u32)entry, > > + SUNXI_R_CPUCFG_BASE + SUN8I_R528_SOFT_ENTRY); > > } else { > > writel((u32)entry, SUNXI_CPUCFG_BASE + SUNXI_PRIV0); > > } > > @@ -125,6 +140,9 @@ static void __secure sunxi_cpu_set_power(int cpu, bool > > on) > > } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) { > > clamp = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWR_CLAMP(cpu); > > pwroff = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWROFF; > > + } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) { > > + /* R528 leaves both cores powered up, manages them via reset */ > > + return; > > } else { > > if (IS_ENABLED(CONFIG_MACH_SUN6I) || > > IS_ENABLED(CONFIG_MACH_SUN8I_H3)) > > @@ -152,11 +170,27 @@ static void __secure sunxi_cpu_set_power(int cpu, > > bool on) > > > > static void __secure sunxi_cpu_set_reset(int cpu, bool reset) > > { > > + if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) { > > + if (reset) > > + clrbits_le32(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_RST_CTRL, > > +BIT(cpu)); > > + else > > + setbits_le32(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_RST_CTRL, > > +BIT(cpu)); > > + > > + return; > > + } > > + > > writel(reset ? 0b00 : 0b11, SUNXI_CPUCFG_BASE + SUNXI_CPU_RST(cpu)); > > } > > > > static void __secure sunxi_cpu_set_locking(int cpu, bool lock) > > { > > + if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) { > > + /* Not required on R528 */ > > + return; > > + } > > + > > if (lock) > >
[PATCH 1/1] CI: use OpenSBI 1.3.1 for testing
Use the most recent upstream release of OpenSBI for CI testing. Signed-off-by: Heinrich Schuchardt --- .azure-pipelines.yml | 8 .gitlab-ci.yml | 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 6f91553e86..d3ecb7c802 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -203,12 +203,12 @@ stages: grub-mkimage --prefix=\"\" -o ~/grub_x86.efi -O i386-efi normal echo lsefimmap lsefi lsefisystab efinet tftp minicmd grub-mkimage --prefix=\"\" -o ~/grub_x64.efi -O x86_64-efi normal echo lsefimmap lsefi lsefisystab efinet tftp minicmd if [[ "\${TEST_PY_BD}" == "qemu-riscv32_spl" ]]; then - wget -O - https://github.com/riscv-software-src/opensbi/releases/download/v1.2/opensbi-1.2-rv-bin.tar.xz | tar -C /tmp -xJ; - export OPENSBI=/tmp/opensbi-1.2-rv-bin/share/opensbi/ilp32/generic/firmware/fw_dynamic.bin; + wget -O - https://github.com/riscv-software-src/opensbi/releases/download/v1.3.1/opensbi-1.3.1-rv-bin.tar.xz | tar -C /tmp -xJ; + export OPENSBI=/tmp/opensbi-1.3.1-rv-bin/share/opensbi/ilp32/generic/firmware/fw_dynamic.bin; fi if [[ "\${TEST_PY_BD}" == "qemu-riscv64_spl" ]] || [[ "\${TEST_PY_BD}" == "sifive_unleashed" ]]; then - wget -O - https://github.com/riscv-software-src/opensbi/releases/download/v1.2/opensbi-1.2-rv-bin.tar.xz | tar -C /tmp -xJ; - export OPENSBI=/tmp/opensbi-1.2-rv-bin/share/opensbi/lp64/generic/firmware/fw_dynamic.bin; + wget -O - https://github.com/riscv-software-src/opensbi/releases/download/v1.3.1/opensbi-1.3.1-rv-bin.tar.xz | tar -C /tmp -xJ; + export OPENSBI=/tmp/opensbi-1.3.1-rv-bin/share/opensbi/lp64/generic/firmware/fw_dynamic.bin; fi # the below corresponds to .gitlab-ci.yml "script" cd \${WORK_DIR} diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6decdfdee3..27065a7ce7 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -32,12 +32,12 @@ stages: - grub-mkimage --prefix="" -o ~/grub_x86.efi -O i386-efi normal echo lsefimmap lsefi lsefisystab efinet tftp minicmd - grub-mkimage --prefix="" -o ~/grub_x64.efi -O x86_64-efi normal echo lsefimmap lsefi lsefisystab efinet tftp minicmd - if [[ "${TEST_PY_BD}" == "qemu-riscv32_spl" ]]; then -wget -O - https://github.com/riscv-software-src/opensbi/releases/download/v1.2/opensbi-1.2-rv-bin.tar.xz | tar -C /tmp -xJ; -export OPENSBI=/tmp/opensbi-1.2-rv-bin/share/opensbi/ilp32/generic/firmware/fw_dynamic.bin; +wget -O - https://github.com/riscv-software-src/opensbi/releases/download/v1.3.1/opensbi-1.3.1-rv-bin.tar.xz | tar -C /tmp -xJ; +export OPENSBI=/tmp/opensbi-1.3.1-rv-bin/share/opensbi/ilp32/generic/firmware/fw_dynamic.bin; fi - if [[ "${TEST_PY_BD}" == "qemu-riscv64_spl" ]] || [[ "${TEST_PY_BD}" == "sifive_unleashed" ]]; then -wget -O - https://github.com/riscv-software-src/opensbi/releases/download/v1.2/opensbi-1.2-rv-bin.tar.xz | tar -C /tmp -xJ; -export OPENSBI=/tmp/opensbi-1.2-rv-bin/share/opensbi/lp64/generic/firmware/fw_dynamic.bin; +wget -O - https://github.com/riscv-software-src/opensbi/releases/download/v1.3.1/opensbi-1.3.1-rv-bin.tar.xz | tar -C /tmp -xJ; +export OPENSBI=/tmp/opensbi-1.3.1-rv-bin/share/opensbi/lp64/generic/firmware/fw_dynamic.bin; fi after_script: -- 2.40.1
Re: [v4.1 2/2] CI, pytest: Add a test for sandbox without LTO
On Tue, Oct 24, 2023 at 02:39:49PM -0700, Simon Glass wrote: > Hi Tom, > > On Tue, 24 Oct 2023 at 11:07, Tom Rini wrote: > > > > On Tue, Oct 24, 2023 at 11:02:06AM -0700, Simon Glass wrote: > > > Hi Tom, > > > > > > On Mon, 23 Oct 2023 at 10:28, Tom Rini wrote: > > > > > > > > On Mon, Oct 23, 2023 at 10:13:52AM -0700, Simon Glass wrote: > > > > > > > > [snip] > > > > > BTW buildman supports -L which disabled LTO using the NO_LTO=1 option > > > > > > > > I worry about putting sandbox-specific flags in buildman. Outside of > > > > sandbox, targets that enable LTO require LTO, just like any other CONFIG > > > > option. > > > > > > Some problems with LTO and why I don't normally develop with it enabled: > > > > > > - build time > > > - code moves around all over the place so it is hard to compare size > > > growth > > > > > > At least for my IDE flow, I use -L in most cases. Yes there are some > > > boards which won't fit without LTO, but I don't see them much. > > > > > > So this is mostly a dev convenience / productivity tool. > > > > Yes, it does take longer to link. And yes, a more complex optimization > > does make some size tracking harder to understand (since growth or > > shrinkage allows for different optimizations to be made around it). But > > everything in configs/ that enables LTO needs LTO. > > I thought we were planning to enable it for all of ARM, though? > Clearly most of those boards don't *need* it. Yes, closer to when it was introduced the ideal was to make it default, because there are so many "on the edge" platforms, with respect to size limits. And, we could possibly make it default now and see what that does to CI build times too. But no, today, platforms opt-in for it because they need it, and the most common need is because of size rather than speed (or, speed gained through size reduction). And also yes, that it does make size comparison trickier is another reason I've not aggressively pushed for it globally. But yes, I still don't think disabling LTO is a general feature, and only sandbox has it architecture-wide enabled by default (for testing) and that -L/--no-lto is not a good generic feature for buildman. But I too don't wish to argue points around LTO nor gc-sections either anymore. -- Tom signature.asc Description: PGP signature
[PATCH v5 3/3] dt-bindings: mtd: binman-partitions: Add alignment properties
Add three properties for controlling alignment of partitions, aka 'entries' in binman. For now there is no explicit mention of hierarchy, so a 'section' is just the 'binman' node. These new properties are inputs to the packaging process, but are also needed if the firmware is repacked, to ensure that alignment constraints are not violated. Therefore they are provided as part of the schema. Signed-off-by: Simon Glass --- Changes in v5: - Add value ranges - Consistently mention alignment must be power-of-2 - Mention that alignment refers to bytes Changes in v2: - Fix 'a' typo in commit message .../mtd/partitions/binman-partition.yaml | 54 +++ 1 file changed, 54 insertions(+) diff --git a/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml b/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml index 37a413464b0ce5..cb1edd83ed29a9 100644 --- a/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml +++ b/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml @@ -29,6 +29,57 @@ properties: - u-boot # u-boot.bin from U-Boot project - tfa-bl31 # bl31.bin or bl31.elf from TF-A project + align: +$ref: /schemas/types.yaml#/definitions/uint32 +minimum: 1 +maximum: 0x8000 +multipleOf: 2 +description: + This sets the alignment of the entry in bytes. + + The entry offset is adjusted so that the entry starts on an aligned + boundary within the containing section or image. For example ‘align = + <16>’ means that the entry will start on a 16-byte boundary. This may + mean that padding is added before the entry. The padding is part of + the containing section but is not included in the entry, meaning that + an empty space may be created before the entry starts. Alignment + must be a power of 2. If ‘align’ is not provided, no alignment is + performed. + + align-size: +$ref: /schemas/types.yaml#/definitions/uint32 +minimum: 1 +maximum: 0x8000 +multipleOf: 2 +description: + This sets the alignment of the entry size in bytes. It must be a power + of 2. + + For example, to ensure that the size of an entry is a multiple of 64 + bytes, set this to 64. While this does not affect the contents of the + entry within binman itself (the padding is performed only when its + parent section is assembled), the end result is that the entry ends + with the padding bytes, so may grow. If ‘align-size’ is not provided, + no alignment is performed. + + align-end: +$ref: /schemas/types.yaml#/definitions/uint32 +minimum: 1 +maximum: 0x8000 +multipleOf: 2 +description: + This sets the alignment (in bytes) of the end of an entry with respect + to the containing section. It must be a power of 2. + + Some entries require that they end on an alignment boundary, + regardless of where they start. This does not move the start of the + entry, so the contents of the entry will still start at the beginning. + But there may be padding at the end. While this does not affect the + contents of the entry within binman itself (the padding is performed + only when its parent section is assembled), the end result is that the + entry ends with the padding bytes, so may grow. If ‘align-end’ is not + provided, no alignment is performed. + additionalProperties: false examples: @@ -41,10 +92,13 @@ examples: partition@10 { compatible = "u-boot"; reg = <0x10 0xf0>; +align-size = <0x1000>; +align-end = <0x1>; }; partition@20 { compatible = "tfa-bl31"; reg = <0x20 0x10>; +align = <0x4000>; }; }; -- 2.42.0.758.gaed0368e0e-goog
[PATCH v5 2/3] dt-bindings: mtd: binman-partition: Add binman compatibles
Add two compatible for binman entries, as a starting point for the schema. Note that, after discussion on v2, we decided to keep the existing meaning of label so as not to require changes to existing userspace software when moving to use binman nodes to specify the firmware layout. Signed-off-by: Simon Glass --- Changes in v5: - Add mention of why 'binman' is the vendor - Drop 'select: false' - Tidy up the compatible setings - Use 'tfa-bl31' instead of 'atf-bl31' Changes in v4: - Correct selection of multiple compatible strings Changes in v3: - Drop fixed-partitions from the example - Use compatible instead of label Changes in v2: - Use plain partition@xxx for the node name .../mtd/partitions/binman-partition.yaml | 50 +++ 1 file changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml diff --git a/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml b/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml new file mode 100644 index 00..37a413464b0ce5 --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml @@ -0,0 +1,50 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright 2023 Google LLC + +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mtd/partitions/binman-partition.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Binman partition + +maintainers: + - Simon Glass + +description: | + This corresponds to a binman 'entry'. It is a single partition which holds + data of a defined type. + + The vendor is specified as binman since there are quite a number + of binman-specific entry types, such as section, fill and files, + to be added later. + +allOf: + - $ref: /schemas/mtd/partitions/partition.yaml# + +properties: + compatible: +- enum: +- binman,entry # generic binman entry +- u-boot # u-boot.bin from U-Boot project +- tfa-bl31 # bl31.bin or bl31.elf from TF-A project + +additionalProperties: false + +examples: + - | +partitions { +compatible = "binman"; +#address-cells = <1>; +#size-cells = <1>; + +partition@10 { +compatible = "u-boot"; +reg = <0x10 0xf0>; +}; + +partition@20 { +compatible = "tfa-bl31"; +reg = <0x20 0x10>; +}; +}; -- 2.42.0.758.gaed0368e0e-goog
[PATCH v5 1/3] dt-bindings: mtd: partitions: Add binman compatible
Add a compatible string for binman, so we can extend fixed-partitions in various ways. Signed-off-by: Simon Glass --- Changes in v5: - Add #address/size-cells and parternProperties - Drop $ref to fixed-partitions.yaml - Drop 'select: false' Changes in v4: - Change subject line Changes in v3: - Drop fixed-partition additional compatible string - Drop fixed-partitions from the example - Mention use of compatible instead of label Changes in v2: - Drop mention of 'enhanced features' in fixed-partitions.yaml - Mention Binman input and output properties - Use plain partition@xxx for the node name .../bindings/mtd/partitions/binman.yaml | 68 +++ .../bindings/mtd/partitions/partitions.yaml | 1 + MAINTAINERS | 5 ++ 3 files changed, 74 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/partitions/binman.yaml diff --git a/Documentation/devicetree/bindings/mtd/partitions/binman.yaml b/Documentation/devicetree/bindings/mtd/partitions/binman.yaml new file mode 100644 index 00..329217550a98cc --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/partitions/binman.yaml @@ -0,0 +1,68 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright 2023 Google LLC + +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mtd/partitions/binman.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Binman firmware layout + +maintainers: + - Simon Glass + +description: | + The binman node provides a layout for firmware, used when packaging firmware + from multiple projects. It is based on fixed-partitions, with some + extensions, but uses 'compatible' to indicate the contents of the node, to + avoid perturbing or confusing existing installations which use 'label' for a + particular purpose. + + Binman supports properties used as inputs to the firmware-packaging process, + such as those which control alignment of partitions. This binding addresses + these 'input' properties. For example, it is common for the 'reg' property + (an 'output' property) to be set by Binman, based on the alignment requested + in the input. + + Once processing is complete, input properties have mostly served their + purpose, at least until the firmware is repacked later, e.g. due to a + firmware update. The 'fixed-partitions' binding should provide enough + information to read the firmware at runtime, including decompression if + needed. + + Documentation for Binman is available at: + + https://u-boot.readthedocs.io/en/latest/develop/package/binman.html + + with the current image-description format at: + + https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#image-description-format + +properties: + compatible: +const: binman + + "#address-cells": +const: 1 + + "#size-cells": +const: 1 + +patternProperties: + "^partition(-.+|@[0-9a-f]+)$": +$ref: partition.yaml + +additionalProperties: false + +examples: + - | +partitions { +compatible = "binman"; +#address-cells = <1>; +#size-cells = <1>; + +partition@10 { +label = "u-boot"; +reg = <0x10 0xf0>; +}; +}; diff --git a/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml index 1dda2c80747bd7..849fd15d085ccc 100644 --- a/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml +++ b/Documentation/devicetree/bindings/mtd/partitions/partitions.yaml @@ -15,6 +15,7 @@ maintainers: oneOf: - $ref: arm,arm-firmware-suite.yaml + - $ref: binman.yaml - $ref: brcm,bcm4908-partitions.yaml - $ref: brcm,bcm947xx-cfe-partitions.yaml - $ref: fixed-partitions.yaml diff --git a/MAINTAINERS b/MAINTAINERS index 2d13bbd69adbc1..b9441eec979a42 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3542,6 +3542,11 @@ F: Documentation/filesystems/bfs.rst F: fs/bfs/ F: include/uapi/linux/bfs_fs.h +BINMAN +M: Simon Glass +S: Supported +F: Documentation/devicetree/bindings/mtd/partitions/binman* + BITMAP API M: Yury Norov R: Andy Shevchenko -- 2.42.0.758.gaed0368e0e-goog
Re: [PATCH v4 2/3] dt-bindings: mtd: binman-partition: Add binman compatibles
Hi Rob, On Tue, 24 Oct 2023 at 09:16, Rob Herring wrote: > > On Mon, Oct 09, 2023 at 04:04:14PM -0600, Simon Glass wrote: > > Add two compatible for binman entries, as a starting point for the > > schema. > > > > Note that, after discussion on v2, we decided to keep the existing > > meaning of label so as not to require changes to existing userspace > > software when moving to use binman nodes to specify the firmware > > layout. > > > > Signed-off-by: Simon Glass > > --- > > > > Changes in v4: > > - Correct selection of multiple compatible strings > > > > Changes in v3: > > - Drop fixed-partitions from the example > > - Use compatible instead of label > > > > Changes in v2: > > - Use plain partition@xxx for the node name > > > > .../mtd/partitions/binman-partition.yaml | 49 +++ > > 1 file changed, 49 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml > > b/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml > > new file mode 100644 > > index ..35a320359ec1 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml > > @@ -0,0 +1,49 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +# Copyright 2023 Google LLC > > + > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/mtd/partitions/binman-partition.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Binman partition > > + > > +maintainers: > > + - Simon Glass > > + > > +select: false > > So this schema is never used. 'select: false' is only useful if > something else if referencing the schema. OK. Is there a user guide to this somewhere? I really don't understand it very well. > > > + > > +description: | > > + This corresponds to a binman 'entry'. It is a single partition which > > holds > > + data of a defined type. > > + > > +allOf: > > + - $ref: /schemas/mtd/partitions/partition.yaml# > > + > > +properties: > > + compatible: > > +oneOf: > > + - const: binman,entry # generic binman entry > > 'binman' is not a vendor. You could add it if you think that's useful. > Probably not with only 1 case... I think it is best to use this for generic things implemented by binman, rather than some other project. For example, binman supports a 'fill' region. It also supports sections which are groups of sub-entries. So we will likely start with half a dozen of these and it will likely grow: binman,fill, binman,section, binman,files If we don't use 'binman', what do you suggest? > > > + - items: > > + - const: u-boot # u-boot.bin from U-Boot project > > + - const: atf-bl31 # bl31.bin or bl31.elf from TF-A project > > Probably should use the new 'tfa' rather than old 'atf'. Is this the > only binary for TFA? The naming seems inconsistent in that every image > goes in (or can go in) a bl?? section. Why does TFA have it but u-boot > doesn't? Perhaps BL?? is orthogonal to defining what is in each > partition. Perhaps someone more familar with all this than I am can > comment. >From what I can tell TF-A can produce all sorts of binaries, of which bl31 is one. U-Boot can also produce lots of binaries, but its naming is different (u-boot, u-boot-spl, etc.). Bear in mind that U-Boot is used on ARM, where this terminology is defined, and on x86 (for example), where it is not. > > Once you actually test this, you'll find you are specifying: > > compatible = "u-boot", "atf-bl31"; I don't understand that, sorry. I'll send a v5 and see if the problem goes away. > > > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > +partitions { > > +compatible = "binman"; > > +#address-cells = <1>; > > +#size-cells = <1>; > > + > > +partition@10 { > > +compatible = "u-boot"; > > +reg = <0x10 0xf0>; > > +}; > > + > > +partition@20 { > > +compatible = "atf-bl31"; > > +reg = <0x20 0x10>; > > +}; > > +}; > > -- > > 2.42.0.609.gbb76f46606-goog > > Regards, Simon
Re: [v4.1 2/2] CI, pytest: Add a test for sandbox without LTO
Hi Tom, On Tue, 24 Oct 2023 at 11:07, Tom Rini wrote: > > On Tue, Oct 24, 2023 at 11:02:06AM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Mon, 23 Oct 2023 at 10:28, Tom Rini wrote: > > > > > > On Mon, Oct 23, 2023 at 10:13:52AM -0700, Simon Glass wrote: > > > > > > [snip] > > > > BTW buildman supports -L which disabled LTO using the NO_LTO=1 option > > > > > > I worry about putting sandbox-specific flags in buildman. Outside of > > > sandbox, targets that enable LTO require LTO, just like any other CONFIG > > > option. > > > > Some problems with LTO and why I don't normally develop with it enabled: > > > > - build time > > - code moves around all over the place so it is hard to compare size growth > > > > At least for my IDE flow, I use -L in most cases. Yes there are some > > boards which won't fit without LTO, but I don't see them much. > > > > So this is mostly a dev convenience / productivity tool. > > Yes, it does take longer to link. And yes, a more complex optimization > does make some size tracking harder to understand (since growth or > shrinkage allows for different optimizations to be made around it). But > everything in configs/ that enables LTO needs LTO. I thought we were planning to enable it for all of ARM, though? Clearly most of those boards don't *need* it. Regards, Simon
[PATCH 8/8] clk/qcom: fix rcg divider value
The RCG divider field takes a value of (2*h - 1) where h is the divisor. This allows fractional dividers to be supported by calculating them at compile time using a macro. However, the clk_rcg_set_rate_mnd() function was also performing the calculation. Clean this all up and consistently use the F() macro to calculate these at compile time and properly support fractional divisors. Additionally, improve clk_bcr_update() to timeout with a warning rather than hanging the board, and make the freq_tbl struct and helpers common so that they can be reused by future platforms. Signed-off-by: Caleb Connolly --- drivers/clk/qcom/clock-apq8016.c | 2 +- drivers/clk/qcom/clock-apq8096.c | 2 +- drivers/clk/qcom/clock-qcom.c| 40 drivers/clk/qcom/clock-qcom.h| 11 +++ drivers/clk/qcom/clock-qcs404.c | 16 drivers/clk/qcom/clock-sdm845.c | 26 -- 6 files changed, 53 insertions(+), 44 deletions(-) diff --git a/drivers/clk/qcom/clock-apq8016.c b/drivers/clk/qcom/clock-apq8016.c index 5eba18739cfb..a1481cd5177b 100644 --- a/drivers/clk/qcom/clock-apq8016.c +++ b/drivers/clk/qcom/clock-apq8016.c @@ -53,7 +53,7 @@ static struct vote_clk gcc_blsp1_ahb_clk = { /* SDHCI */ static int clk_init_sdc(struct qcom_cc_priv *priv, int slot, uint rate) { - int div = 8; /* 100MHz default */ + int div = 15; /* 100MHz default */ if (rate == 2) div = 4; diff --git a/drivers/clk/qcom/clock-apq8096.c b/drivers/clk/qcom/clock-apq8096.c index 48cac08eed67..ef81cd16223c 100644 --- a/drivers/clk/qcom/clock-apq8096.c +++ b/drivers/clk/qcom/clock-apq8096.c @@ -44,7 +44,7 @@ static struct vote_clk gcc_blsp2_ahb_clk = { static int clk_init_sdc(struct qcom_cc_priv *priv, uint rate) { - int div = 3; + int div = 5; clk_enable_cbc(priv->base + SDCC2_AHB_CBCR); clk_rcg_set_rate_mnd(priv->base, _regs, div, 0, 0, diff --git a/drivers/clk/qcom/clock-qcom.c b/drivers/clk/qcom/clock-qcom.c index 7a6157bf123f..a83c74cd20ba 100644 --- a/drivers/clk/qcom/clock-qcom.c +++ b/drivers/clk/qcom/clock-qcom.c @@ -68,20 +68,46 @@ void clk_enable_vote_clk(phys_addr_t base, const struct vote_clk *vclk) /* Update clock command via CMD_RCGR */ void clk_bcr_update(phys_addr_t apps_cmd_rcgr) { + u32 count; setbits_le32(apps_cmd_rcgr, APPS_CMD_RCGR_UPDATE); /* Wait for frequency to be updated. */ - while (readl(apps_cmd_rcgr) & APPS_CMD_RCGR_UPDATE) - ; + for (count = 0; count < 5; count++) { + if (!(readl(apps_cmd_rcgr) & APPS_CMD_RCGR_UPDATE)) + break; + udelay(1); + } + WARN(count == 5, "WARNING: RCG @ %#llx [%#010x] stuck at off\n", +apps_cmd_rcgr, readl(apps_cmd_rcgr)); +} + +const struct freq_tbl *qcom_find_freq(const struct freq_tbl *f, uint rate) +{ + if (!f) + return NULL; + + if (!f->freq) + return f; + + for (; f->freq; f++) + if (rate <= f->freq) + return f; + + /* Default to our fastest rate */ + return f - 1; } #define CFG_MODE_DUAL_EDGE (0x2 << 12) /* Counter mode */ -#define CFG_MASK 0x3FFF +// Disable the HW_CLK_CONTROL bit +#define CFG_MASK (0x3FFF | (1 << 20)) #define CFG_DIVIDER_MASK 0x1F -/* root set rate for clocks with half integer and MND divider */ +/* + * root set rate for clocks with half integer and MND divider + * div should be pre-calculated ((div * 2) - 1) + */ void clk_rcg_set_rate_mnd(phys_addr_t base, const struct bcr_regs *regs, int div, int m, int n, int source, u8 mnd_width) { @@ -99,17 +125,15 @@ void clk_rcg_set_rate_mnd(phys_addr_t base, const struct bcr_regs *regs, /* Program MND values */ setbits_le32(base + regs->M, m_val & mask); setbits_le32(base + regs->N, n_val & mask); - setbits_le32(base + regs->D, d_val & mask); + setbits_le32(base + regs->D, (d_val & mask) == mask ? 0 : (d_val & mask)); /* setup src select and divider */ cfg = readl(base + regs->cfg_rcgr); cfg &= ~CFG_MASK; cfg |= source & CFG_CLK_SRC_MASK; /* Select clock source */ - /* Set the divider; HW permits fraction dividers (+0.5), but - for simplicity, we will support integers only */ if (div) - cfg |= (2 * div - 1) & CFG_DIVIDER_MASK; + cfg |= div & CFG_DIVIDER_MASK; if (n_val) cfg |= CFG_MODE_DUAL_EDGE; diff --git a/drivers/clk/qcom/clock-qcom.h b/drivers/clk/qcom/clock-qcom.h index 6fa88fb40af8..f91e9d47dd22 100644 --- a/drivers/clk/qcom/clock-qcom.h +++ b/drivers/clk/qcom/clock-qcom.h @@ -30,6 +30,16 @@ struct bcr_regs { uintptr_t D; }; +struct freq_tbl { + uint freq; + uint src; + u8 pre_div; + u16 m; + u16 n; +}; +
[PATCH 6/8] clk/qcom: use function pointers for enable and set_rate
Currently, it isn't possible to build clock drivers for more than one platform due to how the msm_enable() and msm_set_rate() callbacks are implemented. Extend qcom_cc_data to include function pointers for these and convert all platforms to use them. Previously, clock drivers relied on common.h to include the board specific sysmap header, include those explicitly to further reduce the dependency between the clock driver and a particular target configuration. Signed-off-by: Caleb Connolly --- drivers/clk/qcom/clock-apq8016.c | 12 ++-- drivers/clk/qcom/clock-apq8096.c | 12 ++-- drivers/clk/qcom/clock-ipq4019.c | 6 -- drivers/clk/qcom/clock-qcom.c| 17 - drivers/clk/qcom/clock-qcom.h| 5 + drivers/clk/qcom/clock-qcs404.c | 5 - drivers/clk/qcom/clock-sdm845.c | 12 7 files changed, 45 insertions(+), 24 deletions(-) diff --git a/drivers/clk/qcom/clock-apq8016.c b/drivers/clk/qcom/clock-apq8016.c index e3b9b8c1b91b..f74f7a0f5ad9 100644 --- a/drivers/clk/qcom/clock-apq8016.c +++ b/drivers/clk/qcom/clock-apq8016.c @@ -13,6 +13,7 @@ #include #include #include +#include #include "clock-qcom.h" @@ -94,7 +95,7 @@ static int clk_init_uart(struct qcom_cc_priv *priv) return 0; } -ulong msm_set_rate(struct clk *clk, ulong rate) +static ulong apq8016_set_rate(struct clk *clk, ulong rate) { struct qcom_cc_priv *priv = dev_get_priv(clk->dev); @@ -113,15 +114,14 @@ ulong msm_set_rate(struct clk *clk, ulong rate) } } -int msm_enable(struct clk *clk) -{ - return 0; -} +static struct qcom_cc_data apq8016_data = { + .set_rate = apq8016_set_rate, +}; static const struct udevice_id gcc_apq8016_of_match[] = { { .compatible = "qcom,gcc-apq8016", - /* TODO: add reset map */ + .data = (ulong)_data, }, { } }; diff --git a/drivers/clk/qcom/clock-apq8096.c b/drivers/clk/qcom/clock-apq8096.c index dc64d11ca979..1efb6e2313a5 100644 --- a/drivers/clk/qcom/clock-apq8096.c +++ b/drivers/clk/qcom/clock-apq8096.c @@ -13,6 +13,7 @@ #include #include #include +#include #include "clock-qcom.h" @@ -80,7 +81,7 @@ static int clk_init_uart(struct qcom_cc_priv *priv) return 0; } -ulong msm_set_rate(struct clk *clk, ulong rate) +static ulong apq8096_set_rate(struct clk *clk, ulong rate) { struct qcom_cc_priv *priv = dev_get_priv(clk->dev); @@ -95,15 +96,14 @@ ulong msm_set_rate(struct clk *clk, ulong rate) } } -int msm_enable(struct clk *clk) -{ - return 0; -} +static struct qcom_cc_data apq8096_data = { + .set_rate = apq8096_set_rate, +}; static const struct udevice_id gcc_apq8096_of_match[] = { { .compatible = "qcom,gcc-apq8096", - /* TODO: add reset map */ + .data = (ulong)_data, }, { } }; diff --git a/drivers/clk/qcom/clock-ipq4019.c b/drivers/clk/qcom/clock-ipq4019.c index 6636af98132d..d42b32c3afd3 100644 --- a/drivers/clk/qcom/clock-ipq4019.c +++ b/drivers/clk/qcom/clock-ipq4019.c @@ -17,7 +17,7 @@ #include "clock-qcom.h" -ulong msm_set_rate(struct clk *clk, ulong rate) +static ulong ipq4019_set_rate(struct clk *clk, ulong rate) { switch (clk->id) { case GCC_BLSP1_UART1_APPS_CLK: /*UART1*/ @@ -28,7 +28,7 @@ ulong msm_set_rate(struct clk *clk, ulong rate) } } -int msm_enable(struct clk *clk) +static int ipq4019_enable(struct clk *clk) { switch (clk->id) { case GCC_BLSP1_QUP1_SPI_APPS_CLK: /*SPI1*/ @@ -125,6 +125,8 @@ static const struct qcom_reset_map gcc_ipq4019_resets[] = { }; static struct qcom_cc_data ipq4019_data = { + .enable = ipq4019_enable, + .set_rate = ipq4019_set_rate, .resets = gcc_ipq4019_resets, .num_resets = ARRAY_SIZE(gcc_ipq4019_resets), }; diff --git a/drivers/clk/qcom/clock-qcom.c b/drivers/clk/qcom/clock-qcom.c index b0416a05789d..a9602d0c9ca6 100644 --- a/drivers/clk/qcom/clock-qcom.c +++ b/drivers/clk/qcom/clock-qcom.c @@ -28,9 +28,6 @@ #define CBCR_BRANCH_ENABLE_BIT BIT(0) #define CBCR_BRANCH_OFF_BIT BIT(31) -extern ulong msm_set_rate(struct clk *clk, ulong rate); -extern int msm_enable(struct clk *clk); - /* Enable clock controlled by CBC soft macro */ void clk_enable_cbc(phys_addr_t cbcr) { @@ -160,12 +157,22 @@ static int msm_clk_probe(struct udevice *dev) static ulong msm_clk_set_rate(struct clk *clk, ulong rate) { - return msm_set_rate(clk, rate); + struct qcom_cc_data *data = (struct qcom_cc_data *)dev_get_driver_data(clk->dev); + + if (data->set_rate) + return data->set_rate(clk, rate); + + return 0; } static int msm_clk_enable(struct clk *clk) { - return msm_enable(clk); + struct qcom_cc_data *data = (struct qcom_cc_data *)dev_get_driver_data(clk->dev); + + if (data->enable) + return data->enable(clk); + +
[PATCH 7/8] clk/qcom: add mnd_width to clk_rcg_set_rate_mnd()
This property is needed on some platforms to ensure that only the relevant bits are set in the M/N/D registers. Signed-off-by: Caleb Connolly --- drivers/clk/qcom/clock-apq8016.c | 4 ++-- drivers/clk/qcom/clock-apq8096.c | 4 ++-- drivers/clk/qcom/clock-qcom.c| 11 +++ drivers/clk/qcom/clock-qcom.h| 2 +- drivers/clk/qcom/clock-qcs404.c | 20 ++-- drivers/clk/qcom/clock-sdm845.c | 3 +-- 6 files changed, 23 insertions(+), 21 deletions(-) diff --git a/drivers/clk/qcom/clock-apq8016.c b/drivers/clk/qcom/clock-apq8016.c index f74f7a0f5ad9..5eba18739cfb 100644 --- a/drivers/clk/qcom/clock-apq8016.c +++ b/drivers/clk/qcom/clock-apq8016.c @@ -61,7 +61,7 @@ static int clk_init_sdc(struct qcom_cc_priv *priv, int slot, uint rate) clk_enable_cbc(priv->base + SDCC_AHB_CBCR(slot)); /* 800Mhz/div, gpll0 */ clk_rcg_set_rate_mnd(priv->base, _regs[slot], div, 0, 0, -CFG_CLK_SRC_GPLL0); +CFG_CLK_SRC_GPLL0, 8); clk_enable_gpll0(priv->base, _vote_clk); clk_enable_cbc(priv->base + SDCC_APPS_CBCR(slot)); @@ -84,7 +84,7 @@ static int clk_init_uart(struct qcom_cc_priv *priv) /* 7372800 uart block clock @ GPLL0 */ clk_rcg_set_rate_mnd(priv->base, _regs, 1, 144, 15625, -CFG_CLK_SRC_GPLL0); +CFG_CLK_SRC_GPLL0, 8); /* Vote for gpll0 clock */ clk_enable_gpll0(priv->base, _vote_clk); diff --git a/drivers/clk/qcom/clock-apq8096.c b/drivers/clk/qcom/clock-apq8096.c index 1efb6e2313a5..48cac08eed67 100644 --- a/drivers/clk/qcom/clock-apq8096.c +++ b/drivers/clk/qcom/clock-apq8096.c @@ -48,7 +48,7 @@ static int clk_init_sdc(struct qcom_cc_priv *priv, uint rate) clk_enable_cbc(priv->base + SDCC2_AHB_CBCR); clk_rcg_set_rate_mnd(priv->base, _regs, div, 0, 0, -CFG_CLK_SRC_GPLL0); +CFG_CLK_SRC_GPLL0, 8); clk_enable_gpll0(priv->base, _vote_clk); clk_enable_cbc(priv->base + SDCC2_APPS_CBCR); @@ -70,7 +70,7 @@ static int clk_init_uart(struct qcom_cc_priv *priv) /* 7372800 uart block clock @ GPLL0 */ clk_rcg_set_rate_mnd(priv->base, _regs, 1, 192, 15625, -CFG_CLK_SRC_GPLL0); +CFG_CLK_SRC_GPLL0, 8); /* Vote for gpll0 clock */ clk_enable_gpll0(priv->base, _vote_clk); diff --git a/drivers/clk/qcom/clock-qcom.c b/drivers/clk/qcom/clock-qcom.c index a9602d0c9ca6..7a6157bf123f 100644 --- a/drivers/clk/qcom/clock-qcom.c +++ b/drivers/clk/qcom/clock-qcom.c @@ -83,7 +83,7 @@ void clk_bcr_update(phys_addr_t apps_cmd_rcgr) /* root set rate for clocks with half integer and MND divider */ void clk_rcg_set_rate_mnd(phys_addr_t base, const struct bcr_regs *regs, - int div, int m, int n, int source) + int div, int m, int n, int source, u8 mnd_width) { u32 cfg; /* M value for MND divider. */ @@ -92,11 +92,14 @@ void clk_rcg_set_rate_mnd(phys_addr_t base, const struct bcr_regs *regs, u32 n_val = ~((n) - (m)) * !!(n); /* NOT 2D value for MND divider. */ u32 d_val = ~(n); + u32 mask = BIT(mnd_width) - 1; + + debug("m %#x n %#x d %#x div %#x mask %#x\n", m_val, n_val, d_val, div, mask); /* Program MND values */ - writel(m_val, base + regs->M); - writel(n_val, base + regs->N); - writel(d_val, base + regs->D); + setbits_le32(base + regs->M, m_val & mask); + setbits_le32(base + regs->N, n_val & mask); + setbits_le32(base + regs->D, d_val & mask); /* setup src select and divider */ cfg = readl(base + regs->cfg_rcgr); diff --git a/drivers/clk/qcom/clock-qcom.h b/drivers/clk/qcom/clock-qcom.h index 4c5959b60038..6fa88fb40af8 100644 --- a/drivers/clk/qcom/clock-qcom.h +++ b/drivers/clk/qcom/clock-qcom.h @@ -70,7 +70,7 @@ void clk_bcr_update(phys_addr_t apps_cmd_rgcr); void clk_enable_cbc(phys_addr_t cbcr); void clk_enable_vote_clk(phys_addr_t base, const struct vote_clk *vclk); void clk_rcg_set_rate_mnd(phys_addr_t base, const struct bcr_regs *regs, - int div, int m, int n, int source); + int div, int m, int n, int source, u8 mnd_width); void clk_rcg_set_rate(phys_addr_t base, const struct bcr_regs *regs, int div, int source); diff --git a/drivers/clk/qcom/clock-qcs404.c b/drivers/clk/qcom/clock-qcs404.c index ce83ec741278..d10992ee58bf 100644 --- a/drivers/clk/qcom/clock-qcs404.c +++ b/drivers/clk/qcom/clock-qcs404.c @@ -113,7 +113,7 @@ static const struct bcr_regs blsp1_qup4_i2c_apps_regs = { /* mnd_width = 0 */ }; -ulong msm_set_rate(struct clk *clk, ulong rate) +static ulong qcs404_set_rate(struct clk *clk, ulong rate) { struct qcom_cc_priv *priv = dev_get_priv(clk->dev); @@ -121,7 +121,7 @@
[PATCH 5/8] clk/qcom: sdm845: add register map for simple gate clocks
Many gate clocks can be enabled with a single register write, add support for defining these simple gate clocks and add the ones found on SDM845. While we're here, inline clk_init_uart() into msm_set_rate(). Signed-off-by: Caleb Connolly --- .../mach-snapdragon/include/mach/sysmap-sdm845.h | 3 + drivers/clk/qcom/clock-qcom.h | 22 +++ drivers/clk/qcom/clock-sdm845.c| 163 +++-- 3 files changed, 176 insertions(+), 12 deletions(-) diff --git a/arch/arm/mach-snapdragon/include/mach/sysmap-sdm845.h b/arch/arm/mach-snapdragon/include/mach/sysmap-sdm845.h index 7165985bcd1e..a0010d71594e 100644 --- a/arch/arm/mach-snapdragon/include/mach/sysmap-sdm845.h +++ b/arch/arm/mach-snapdragon/include/mach/sysmap-sdm845.h @@ -39,4 +39,7 @@ #define SE9_UART_APPS_N(0x18154) #define SE9_UART_APPS_D(0x18158) +#define USB30_SEC_GDSCR(0x10004) +#define USB30_PRIM_GDSCR (0xf004) + #endif diff --git a/drivers/clk/qcom/clock-qcom.h b/drivers/clk/qcom/clock-qcom.h index a77a94b6ea06..7b3bcf41f421 100644 --- a/drivers/clk/qcom/clock-qcom.h +++ b/drivers/clk/qcom/clock-qcom.h @@ -30,6 +30,18 @@ struct bcr_regs { uintptr_t D; }; +struct gate_clk { + uintptr_t reg; + u32 en_val; + const char *name; +}; + +#ifdef DEBUG +#define GATE_CLK(clk, reg, val) [clk] = { reg, val, #clk } +#else +#define GATE_CLK(clk, reg, val) [clk] = { reg, val, NULL } +#endif + struct qcom_reset_map { unsigned int reg; u8 bit; @@ -38,6 +50,8 @@ struct qcom_reset_map { struct qcom_cc_data { const struct qcom_reset_map *resets; unsigned long num_resets; + const struct gate_clk *clks; + unsigned long num_clks; }; struct qcom_cc_priv { @@ -55,4 +69,12 @@ void clk_rcg_set_rate_mnd(phys_addr_t base, const struct bcr_regs *regs, void clk_rcg_set_rate(phys_addr_t base, const struct bcr_regs *regs, int div, int source); +static inline void qcom_gate_clk_en(const struct qcom_cc_priv *priv, unsigned long id) +{ + if (id >= priv->data->num_clks || priv->data->clks[id].reg == 0) + return; + + setbits_le32(priv->base + priv->data->clks[id].reg, priv->data->clks[id].en_val); +} + #endif diff --git a/drivers/clk/qcom/clock-sdm845.c b/drivers/clk/qcom/clock-sdm845.c index eab88a40c09d..ccad73b6ff15 100644 --- a/drivers/clk/qcom/clock-sdm845.c +++ b/drivers/clk/qcom/clock-sdm845.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -71,30 +72,166 @@ const struct freq_tbl *qcom_find_freq(const struct freq_tbl *f, uint rate) return f - 1; } -static int clk_init_uart(struct qcom_cc_priv *priv, uint rate) -{ - const struct freq_tbl *freq = qcom_find_freq(ftbl_gcc_qupv3_wrap0_s0_clk_src, rate); - - clk_rcg_set_rate_mnd(priv->base, _regs, - freq->pre_div, freq->m, freq->n, freq->src); - - return 0; -} - ulong msm_set_rate(struct clk *clk, ulong rate) { struct qcom_cc_priv *priv = dev_get_priv(clk->dev); + const struct freq_tbl *freq; switch (clk->id) { - case GCC_QUPV3_WRAP1_S1_CLK: /*UART2*/ - return clk_init_uart(priv, rate); + case GCC_QUPV3_WRAP1_S1_CLK: /* UART9 */ + freq = qcom_find_freq(ftbl_gcc_qupv3_wrap0_s0_clk_src, rate); + clk_rcg_set_rate_mnd(priv->base, _regs, +freq->pre_div, freq->m, freq->n, freq->src); + + return freq->freq; default: return 0; } } +static const struct gate_clk sdm845_clks[] = { + GATE_CLK(GCC_AGGRE_NOC_PCIE_TBU_CLK,0x90014, 0x0001), + GATE_CLK(GCC_AGGRE_UFS_CARD_AXI_CLK,0x82028, 0x0001), + GATE_CLK(GCC_AGGRE_UFS_PHY_AXI_CLK, 0x82024, 0x0001), + GATE_CLK(GCC_AGGRE_USB3_PRIM_AXI_CLK, 0x8201c, 0x0001), + GATE_CLK(GCC_AGGRE_USB3_SEC_AXI_CLK,0x82020, 0x0001), + GATE_CLK(GCC_BOOT_ROM_AHB_CLK, 0x52004, 0x0400), + GATE_CLK(GCC_CAMERA_AHB_CLK,0x0b008, 0x0001), + GATE_CLK(GCC_CAMERA_AXI_CLK,0x0b020, 0x0001), + GATE_CLK(GCC_CAMERA_XO_CLK, 0x0b02c, 0x0001), + GATE_CLK(GCC_CE1_AHB_CLK, 0x52004, 0x0008), + GATE_CLK(GCC_CE1_AXI_CLK, 0x52004, 0x0010), + GATE_CLK(GCC_CE1_CLK, 0x52004, 0x0020), + GATE_CLK(GCC_CFG_NOC_USB3_PRIM_AXI_CLK, 0x0502c, 0x0001), + GATE_CLK(GCC_CFG_NOC_USB3_SEC_AXI_CLK, 0x05030, 0x0001), + GATE_CLK(GCC_CPUSS_AHB_CLK, 0x52004, 0x0020), +
[PATCH 4/8] clk/qcom: handle resets and clocks in one device
From: Konrad Dybcio Qualcomm's clock controller blocks are actually do much more than it says on the tin.. They provide clocks, resets and power domains. Currently, U-Boot required one to spawn 2 separate devices for controlling clocks and resets, both spanning the same register space. Refactor the code to make it work with just a single DT node, making it compatible with upstream Linux bindings and dropping the dedicated reset driver in favour of including it in the clock driver. While at it, take the liberty of slowly renaming 'msm' and 'snapdragon' to 'qcom' and drop unused compatibles. Heavily inspired by Renesas code for a similar hw block. Signed-off-by: Konrad Dybcio [caleb: moved drivers to clk/qcom, added reset driver and adjusted bind logic] Signed-off-by: Caleb Connolly --- arch/arm/Kconfig | 1 + arch/arm/dts/qcom-ipq4019.dtsi | 14 +-- arch/arm/dts/qcs404-evb.dts | 19 ++-- drivers/clk/qcom/clock-apq8016.c | 23 - drivers/clk/qcom/clock-apq8096.c | 22 - drivers/clk/qcom/clock-ipq4019.c | 95 +++ drivers/clk/qcom/clock-qcom.c| 122 drivers/clk/qcom/clock-qcom.h| 22 +++-- drivers/clk/qcom/clock-qcs404.c | 54 ++- drivers/clk/qcom/clock-sdm845.c | 55 ++- drivers/reset/Kconfig| 7 -- drivers/reset/Makefile | 1 - drivers/reset/reset-qcom.c | 195 --- 13 files changed, 370 insertions(+), 260 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 5aaf7e5e32af..faccfaf720a8 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1067,6 +1067,7 @@ config ARCH_SNAPDRAGON select DM select DM_GPIO select DM_SERIAL + select DM_RESET select GPIO_EXTRA_HEADER select MSM_SMEM select OF_CONTROL diff --git a/arch/arm/dts/qcom-ipq4019.dtsi b/arch/arm/dts/qcom-ipq4019.dtsi index 0850ae56e9a8..f9489e42ea2c 100644 --- a/arch/arm/dts/qcom-ipq4019.dtsi +++ b/arch/arm/dts/qcom-ipq4019.dtsi @@ -66,14 +66,6 @@ status = "disabled"; }; - reset: gcc-reset@180 { - compatible = "qcom,gcc-reset-ipq4019"; - reg = <0x180 0x6>; - #clock-cells = <1>; - #reset-cells = <1>; - bootph-all; - }; - soc_gpios: pinctrl@100 { compatible = "qcom,ipq4019-pinctrl"; reg = <0x100 0x30>; @@ -136,7 +128,7 @@ #phy-cells = <0>; reg = <0x9a000 0x800>; reg-names = "phy_base"; - resets = < USB3_UNIPHY_PHY_ARES>; + resets = < USB3_UNIPHY_PHY_ARES>; reset-names = "por_rst"; status = "disabled"; }; @@ -146,7 +138,7 @@ #phy-cells = <0>; reg = <0xa6000 0x40>; reg-names = "phy_base"; - resets = < USB3_HSPHY_POR_ARES>, < USB3_HSPHY_S_ARES>; + resets = < USB3_HSPHY_POR_ARES>, < USB3_HSPHY_S_ARES>; reset-names = "por_rst", "srif_rst"; status = "disabled"; }; @@ -179,7 +171,7 @@ #phy-cells = <0>; reg = <0xa8000 0x40>; reg-names = "phy_base"; - resets = < USB2_HSPHY_POR_ARES>, < USB2_HSPHY_S_ARES>; + resets = < USB2_HSPHY_POR_ARES>, < USB2_HSPHY_S_ARES>; reset-names = "por_rst", "srif_rst"; status = "disabled"; }; diff --git a/arch/arm/dts/qcs404-evb.dts b/arch/arm/dts/qcs404-evb.dts index 8d7893c11695..84224a8a3d39 100644 --- a/arch/arm/dts/qcs404-evb.dts +++ b/arch/arm/dts/qcs404-evb.dts @@ -208,11 +208,6 @@ #address-cells = <0x1>; #size-cells = <0x0>; #clock-cells = <1>; - }; - - reset: gcc-reset@180 { - compatible = "qcom,gcc-reset-qcs404"; - reg = <0x180 0x8>; #reset-cells = <1>; }; @@ -245,8 +240,8 @@ clocks = < GCC_USB_HS_PHY_CFG_AHB_CLK>, < GCC_USB3_PHY_PIPE_CLK>; clock-names = "ahb", "pipe"; - resets = < GCC_USB3_PHY_BCR>, -< GCC_USB3PHY_PHY_BCR>; + resets = < GCC_USB3_PHY_BCR>, +< GCC_USB3PHY_PHY_BCR>; reset-names = "com", "phy"; }; @@ -257,8 +252,8 @@
[PATCH 2/8] clk/qcom: add per-platform configs
Decouple the clock drivers from the mach-snapdragon TARGET configs by introducing CONFIG_CLK_QCOM and associated options to build each SoC. This will make future cleanup easier as we move towards a generic Qualcomm target. Signed-off-by: Caleb Connolly --- arch/arm/mach-snapdragon/Kconfig | 4 drivers/clk/Kconfig | 1 + drivers/clk/Makefile | 2 +- drivers/clk/qcom/Kconfig | 44 drivers/clk/qcom/Makefile| 8 5 files changed, 54 insertions(+), 5 deletions(-) diff --git a/arch/arm/mach-snapdragon/Kconfig b/arch/arm/mach-snapdragon/Kconfig index 0e073045be54..eaf75abf4bd5 100644 --- a/arch/arm/mach-snapdragon/Kconfig +++ b/arch/arm/mach-snapdragon/Kconfig @@ -16,6 +16,7 @@ config SDM845 bool "Qualcomm Snapdragon 845 SoC" default n select LINUX_KERNEL_IMAGE_HEADER + imply CLK_QCOM_SDM845 config LNX_KRNL_IMG_TEXT_OFFSET_BASE default 0x8000 @@ -27,6 +28,7 @@ config TARGET_DRAGONBOARD410C bool "96Boards Dragonboard 410C" select BOARD_LATE_INIT select ENABLE_ARM_SOC_BOOT0_HOOK + imply CLK_QCOM_APQ8016 help Support for 96Boards Dragonboard 410C. This board complies with 96Board Open Platform Specifications. Features: @@ -40,6 +42,7 @@ config TARGET_DRAGONBOARD410C config TARGET_DRAGONBOARD820C bool "96Boards Dragonboard 820C" + imply CLK_QCOM_APQ8096 help Support for 96Boards Dragonboard 820C. This board complies with 96Board Open Platform Specifications. Features: @@ -73,6 +76,7 @@ config TARGET_STARQLTECHN config TARGET_QCS404EVB bool "Qualcomm Technologies, Inc. QCS404 EVB" select LINUX_KERNEL_IMAGE_HEADER + imply CLK_QCOM_QCS404 help Support for Qualcomm Technologies, Inc. QCS404 evaluation board. Features: diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index bfd23a990469..017dd260a544 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -254,6 +254,7 @@ source "drivers/clk/meson/Kconfig" source "drivers/clk/microchip/Kconfig" source "drivers/clk/mvebu/Kconfig" source "drivers/clk/owl/Kconfig" +source "drivers/clk/qcom/Kconfig" source "drivers/clk/renesas/Kconfig" source "drivers/clk/sunxi/Kconfig" source "drivers/clk/sifive/Kconfig" diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index dd90c7672e1b..dd974742b6c8 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -40,7 +40,7 @@ obj-$(CONFIG_CLK_MPFS) += microchip/ obj-$(CONFIG_CLK_MVEBU) += mvebu/ obj-$(CONFIG_CLK_OCTEON) += clk_octeon.o obj-$(CONFIG_CLK_OWL) += owl/ -obj-$(CONFIG_ARCH_SNAPDRAGON) += qcom/ +obj-$(CONFIG_CLK_QCOM) += qcom/ obj-$(CONFIG_CLK_RENESAS) += renesas/ obj-$(CONFIG_$(SPL_TPL_)CLK_SCMI) += clk_scmi.o obj-$(CONFIG_CLK_SIFIVE) += sifive/ diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig new file mode 100644 index ..a884f077d9b9 --- /dev/null +++ b/drivers/clk/qcom/Kconfig @@ -0,0 +1,44 @@ +if ARCH_SNAPDRAGON || ARCH_IPQ40XX + +config CLK_QCOM + bool + depends on CLK && DM_RESET + def_bool n + +menu "Qualcomm clock drivers" + +config CLK_QCOM_APQ8016 + bool "Qualcomm APQ8016 GCC" + select CLK_QCOM + help + Say Y here to enable support for the Global Clock Controller + on the Snapdragon APQ8016 SoC. This driver supports the clocks + and resets exposed by the GCC hardware block. + +config CLK_QCOM_APQ8096 + bool "Qualcomm APQ8096 GCC" + select CLK_QCOM + help + Say Y here to enable support for the Global Clock Controller + on the Snapdragon APQ8096 SoC. This driver supports the clocks + and resets exposed by the GCC hardware block. + +config CLK_QCOM_QCS404 + bool "Qualcomm QCS404 GCC" + select CLK_QCOM + help + Say Y here to enable support for the Global Clock Controller + on the Snapdragon QCS404 SoC. This driver supports the clocks + and resets exposed by the GCC hardware block. + +config CLK_QCOM_SDM845 + bool "Qualcomm SDM845 GCC" + select CLK_QCOM + help + Say Y here to enable support for the Global Clock Controller + on the Snapdragon 845 SoC. This driver supports the clocks + and resets exposed by the GCC hardware block. + +endmenu + +endif diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile index 5f0c7a79d2ab..44d55583596d 100644 --- a/drivers/clk/qcom/Makefile +++ b/drivers/clk/qcom/Makefile @@ -3,7 +3,7 @@ # (C) Copyright 2023 Linaro obj-y += clock-qcom.o -obj-$(CONFIG_SDM845) += clock-sdm845.o -obj-$(CONFIG_TARGET_DRAGONBOARD410C) += clock-apq8016.o -obj-$(CONFIG_TARGET_DRAGONBOARD820C) += clock-apq8096.o -obj-$(CONFIG_TARGET_QCS404EVB) += clock-qcs404.o +obj-$(CONFIG_CLK_QCOM_SDM845) += clock-sdm845.o +obj-$(CONFIG_CLK_QCOM_APQ8016) += clock-apq8016.o
[PATCH 3/8] clk/qcom: move ipq4019 driver from mach-ipq40xx
This driver is just a stub, but it's necessary to support the upcoming reset driver changes. Signed-off-by: Caleb Connolly --- arch/arm/Kconfig | 1 + arch/arm/mach-ipq40xx/Makefile | 1 - drivers/clk/qcom/Kconfig | 8 + drivers/clk/qcom/Makefile | 1 + .../clk/qcom}/clock-ipq4019.c | 41 ++ 5 files changed, 12 insertions(+), 40 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 531b081de996..5aaf7e5e32af 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -766,6 +766,7 @@ config ARCH_IPQ40XX select CLK select SMEM select OF_CONTROL + select CLK_QCOM_IPQ4019 imply CMD_DM config ARCH_KEYSTONE diff --git a/arch/arm/mach-ipq40xx/Makefile b/arch/arm/mach-ipq40xx/Makefile index 08a65b8854d3..b36a935c6f9f 100644 --- a/arch/arm/mach-ipq40xx/Makefile +++ b/arch/arm/mach-ipq40xx/Makefile @@ -4,6 +4,5 @@ # # Author: Robert Marko -obj-y += clock-ipq4019.o obj-y += pinctrl-snapdragon.o obj-y += pinctrl-ipq4019.o diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig index a884f077d9b9..0df0d1881a49 100644 --- a/drivers/clk/qcom/Kconfig +++ b/drivers/clk/qcom/Kconfig @@ -23,6 +23,14 @@ config CLK_QCOM_APQ8096 on the Snapdragon APQ8096 SoC. This driver supports the clocks and resets exposed by the GCC hardware block. +config CLK_QCOM_IPQ4019 + bool "Qualcomm IPQ4019 GCC" + select CLK_QCOM + help + Say Y here to enable support for the Global Clock Controller + on the Snapdragon IPQ4019 SoC. This driver supports the clocks + and resets exposed by the GCC hardware block. + config CLK_QCOM_QCS404 bool "Qualcomm QCS404 GCC" select CLK_QCOM diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile index 44d55583596d..25bd2e1e8bb1 100644 --- a/drivers/clk/qcom/Makefile +++ b/drivers/clk/qcom/Makefile @@ -6,4 +6,5 @@ obj-y += clock-qcom.o obj-$(CONFIG_CLK_QCOM_SDM845) += clock-sdm845.o obj-$(CONFIG_CLK_QCOM_APQ8016) += clock-apq8016.o obj-$(CONFIG_CLK_QCOM_APQ8096) += clock-apq8096.o +obj-$(CONFIG_CLK_QCOM_IPQ4019) += clock-ipa4019.o obj-$(CONFIG_CLK_QCOM_QCS404) += clock-qcs404.o diff --git a/arch/arm/mach-ipq40xx/clock-ipq4019.c b/drivers/clk/qcom/clock-ipq4019.c similarity index 56% rename from arch/arm/mach-ipq40xx/clock-ipq4019.c rename to drivers/clk/qcom/clock-ipq4019.c index c1d5c4ecdd81..04c99964df15 100644 --- a/arch/arm/mach-ipq40xx/clock-ipq4019.c +++ b/drivers/clk/qcom/clock-ipq4019.c @@ -12,12 +12,9 @@ #include #include #include - #include -struct msm_clk_priv { - phys_addr_t base; -}; +#include "clock-qcom.h" ulong msm_set_rate(struct clk *clk, ulong rate) { @@ -30,23 +27,7 @@ ulong msm_set_rate(struct clk *clk, ulong rate) } } -static int msm_clk_probe(struct udevice *dev) -{ - struct msm_clk_priv *priv = dev_get_priv(dev); - - priv->base = dev_read_addr(dev); - if (priv->base == FDT_ADDR_T_NONE) - return -EINVAL; - - return 0; -} - -static ulong msm_clk_set_rate(struct clk *clk, ulong rate) -{ - return msm_set_rate(clk, rate); -} - -static int msm_enable(struct clk *clk) +int msm_enable(struct clk *clk) { switch (clk->id) { case GCC_BLSP1_QUP1_SPI_APPS_CLK: /*SPI1*/ @@ -68,21 +49,3 @@ static int msm_enable(struct clk *clk) } } -static struct clk_ops msm_clk_ops = { - .set_rate = msm_clk_set_rate, - .enable = msm_enable, -}; - -static const struct udevice_id msm_clk_ids[] = { - { .compatible = "qcom,gcc-ipq4019" }, - { } -}; - -U_BOOT_DRIVER(clk_msm) = { - .name = "clk_msm", - .id = UCLASS_CLK, - .of_match = msm_clk_ids, - .ops= _clk_ops, - .priv_auto = sizeof(struct msm_clk_priv), - .probe = msm_clk_probe, -}; -- 2.42.0
[PATCH 1/8] clk/qcom: move from mach-snapdragon
Clock drivers don't belong here, move them to the right place and declutter mach-snapdragon a bit. Signed-off-by: Caleb Connolly --- arch/arm/mach-snapdragon/Makefile| 5 - drivers/clk/Makefile | 1 + drivers/clk/qcom/Makefile| 9 + {arch/arm/mach-snapdragon => drivers/clk/qcom}/clock-apq8016.c | 2 +- {arch/arm/mach-snapdragon => drivers/clk/qcom}/clock-apq8096.c | 3 ++- .../clock-snapdragon.c => drivers/clk/qcom/clock-qcom.c | 2 +- .../clock-snapdragon.h => drivers/clk/qcom/clock-qcom.h | 0 {arch/arm/mach-snapdragon => drivers/clk/qcom}/clock-qcs404.c| 2 +- {arch/arm/mach-snapdragon => drivers/clk/qcom}/clock-sdm845.c| 2 +- 9 files changed, 16 insertions(+), 10 deletions(-) diff --git a/arch/arm/mach-snapdragon/Makefile b/arch/arm/mach-snapdragon/Makefile index cbaaf23f6b56..497ee35cf7d3 100644 --- a/arch/arm/mach-snapdragon/Makefile +++ b/arch/arm/mach-snapdragon/Makefile @@ -2,20 +2,15 @@ # # (C) Copyright 2015 Mateusz Kulikowski -obj-$(CONFIG_SDM845) += clock-sdm845.o obj-$(CONFIG_SDM845) += sysmap-sdm845.o obj-$(CONFIG_SDM845) += init_sdm845.o -obj-$(CONFIG_TARGET_DRAGONBOARD820C) += clock-apq8096.o obj-$(CONFIG_TARGET_DRAGONBOARD820C) += sysmap-apq8096.o -obj-$(CONFIG_TARGET_DRAGONBOARD410C) += clock-apq8016.o obj-$(CONFIG_TARGET_DRAGONBOARD410C) += sysmap-apq8016.o obj-y += misc.o -obj-y += clock-snapdragon.o obj-y += dram.o obj-y += pinctrl-snapdragon.o obj-y += pinctrl-apq8016.o obj-y += pinctrl-apq8096.o obj-y += pinctrl-qcs404.o obj-y += pinctrl-sdm845.o -obj-$(CONFIG_TARGET_QCS404EVB) += clock-qcs404.o obj-$(CONFIG_TARGET_QCS404EVB) += sysmap-qcs404.o diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 26bf429acbc0..dd90c7672e1b 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -40,6 +40,7 @@ obj-$(CONFIG_CLK_MPFS) += microchip/ obj-$(CONFIG_CLK_MVEBU) += mvebu/ obj-$(CONFIG_CLK_OCTEON) += clk_octeon.o obj-$(CONFIG_CLK_OWL) += owl/ +obj-$(CONFIG_ARCH_SNAPDRAGON) += qcom/ obj-$(CONFIG_CLK_RENESAS) += renesas/ obj-$(CONFIG_$(SPL_TPL_)CLK_SCMI) += clk_scmi.o obj-$(CONFIG_CLK_SIFIVE) += sifive/ diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile new file mode 100644 index ..5f0c7a79d2ab --- /dev/null +++ b/drivers/clk/qcom/Makefile @@ -0,0 +1,9 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# (C) Copyright 2023 Linaro + +obj-y += clock-qcom.o +obj-$(CONFIG_SDM845) += clock-sdm845.o +obj-$(CONFIG_TARGET_DRAGONBOARD410C) += clock-apq8016.o +obj-$(CONFIG_TARGET_DRAGONBOARD820C) += clock-apq8096.o +obj-$(CONFIG_TARGET_QCS404EVB) += clock-qcs404.o diff --git a/arch/arm/mach-snapdragon/clock-apq8016.c b/drivers/clk/qcom/clock-apq8016.c similarity index 98% rename from arch/arm/mach-snapdragon/clock-apq8016.c rename to drivers/clk/qcom/clock-apq8016.c index 23a37a1714dc..90f2a93d9ed5 100644 --- a/arch/arm/mach-snapdragon/clock-apq8016.c +++ b/drivers/clk/qcom/clock-apq8016.c @@ -13,7 +13,7 @@ #include #include #include -#include "clock-snapdragon.h" +#include "clock-qcom.h" /* GPLL0 clock control registers */ #define GPLL0_STATUS_ACTIVE BIT(17) diff --git a/arch/arm/mach-snapdragon/clock-apq8096.c b/drivers/clk/qcom/clock-apq8096.c similarity index 98% rename from arch/arm/mach-snapdragon/clock-apq8096.c rename to drivers/clk/qcom/clock-apq8096.c index 66184596d562..d5388c69aefe 100644 --- a/arch/arm/mach-snapdragon/clock-apq8096.c +++ b/drivers/clk/qcom/clock-apq8096.c @@ -13,7 +13,8 @@ #include #include #include -#include "clock-snapdragon.h" + +#include "clock-qcom.h" /* GPLL0 clock control registers */ #define GPLL0_STATUS_ACTIVEBIT(30) diff --git a/arch/arm/mach-snapdragon/clock-snapdragon.c b/drivers/clk/qcom/clock-qcom.c similarity index 99% rename from arch/arm/mach-snapdragon/clock-snapdragon.c rename to drivers/clk/qcom/clock-qcom.c index 0ac45dce9a92..5667abeb89a4 100644 --- a/arch/arm/mach-snapdragon/clock-snapdragon.c +++ b/drivers/clk/qcom/clock-qcom.c @@ -13,7 +13,7 @@ #include #include #include -#include "clock-snapdragon.h" +#include "clock-qcom.h" /* CBCR register fields */ #define CBCR_BRANCH_ENABLE_BIT BIT(0) diff --git a/arch/arm/mach-snapdragon/clock-snapdragon.h b/drivers/clk/qcom/clock-qcom.h similarity index 100% rename from arch/arm/mach-snapdragon/clock-snapdragon.h rename to drivers/clk/qcom/clock-qcom.h diff --git a/arch/arm/mach-snapdragon/clock-qcs404.c b/drivers/clk/qcom/clock-qcs404.c similarity index 99% rename from arch/arm/mach-snapdragon/clock-qcs404.c rename to drivers/clk/qcom/clock-qcs404.c index 3357b54c30c0..80218af73ef6 100644 --- a/arch/arm/mach-snapdragon/clock-qcs404.c +++ b/drivers/clk/qcom/clock-qcs404.c @@ -11,7 +11,7 @@ #include #include #include -#include "clock-snapdragon.h" +#include "clock-qcom.h" #include diff --git
[PATCH 0/8] arm: mach-snapdragon: Qualcomm clock driver cleanup
This series begins making some headway towards cleaning up Qualcomm platform support in u-boot. The following is a rough overview of the changes: * Move the Qualcomm clock drivers out of mach-snapdragon and into clk/qcom * Introduce per-platform clock driver configs to decouple Qualcomm platform support from mach-snapdragon targets. * Add the IPQ4019 clock driver, removing it from mach-ipq40xx and introducing the reset map. * Merge the qcom reset driver is into clk/qcom and rework it to be compatible with upstream devicetrees. * A callback model is added so that multiple clock drivers can be compiled in at once. * SDM845 gains support for enabling/disabling all gate clocks (CBC's) by way of a new "gate_clk" abstraction. * Preperatory cleanup work is done to simplify the bringup process for new platforms. Further details are included in each commit. The primary goal of this series is to prepare for enabling several new Qualcomm platforms in u-boot as well as additional peripherals, while minimising the amount of copy/pasted board-specific code. This series conflicts with a previous series introducing support for the Qualcomm RB2 board [1], I plan to resend this initial support pending acceptance of this series and several other cleanups. [1]: https://lore.kernel.org/u-boot/20230324080418.3856409-1-bhupesh.sha...@linaro.org/ --- Caleb Connolly (7): clk/qcom: move from mach-snapdragon clk/qcom: add per-platform configs clk/qcom: move ipq4019 driver from mach-ipq40xx clk/qcom: sdm845: add register map for simple gate clocks clk/qcom: use function pointers for enable and set_rate clk/qcom: add mnd_width to clk_rcg_set_rate_mnd() clk/qcom: fix rcg divider value Konrad Dybcio (1): clk/qcom: handle resets and clocks in one device arch/arm/Kconfig | 2 + arch/arm/dts/qcom-ipq4019.dtsi | 14 +- arch/arm/dts/qcs404-evb.dts| 19 +- arch/arm/mach-ipq40xx/Makefile | 1 - arch/arm/mach-ipq40xx/clock-ipq4019.c | 88 -- arch/arm/mach-snapdragon/Kconfig | 4 + arch/arm/mach-snapdragon/Makefile | 5 - arch/arm/mach-snapdragon/clock-sdm845.c| 98 --- arch/arm/mach-snapdragon/clock-snapdragon.c| 181 arch/arm/mach-snapdragon/clock-snapdragon.h| 48 .../mach-snapdragon/include/mach/sysmap-sdm845.h | 3 + drivers/clk/Kconfig| 1 + drivers/clk/Makefile | 1 + drivers/clk/qcom/Kconfig | 52 drivers/clk/qcom/Makefile | 10 + .../clk/qcom}/clock-apq8016.c | 41 ++- .../clk/qcom}/clock-apq8096.c | 41 ++- .../reset-qcom.c => clk/qcom/clock-ipq4019.c} | 165 --- drivers/clk/qcom/clock-qcom.c | 303 + drivers/clk/qcom/clock-qcom.h | 96 +++ .../clk/qcom}/clock-qcs404.c | 97 +-- drivers/clk/qcom/clock-sdm845.c| 265 ++ drivers/reset/Kconfig | 7 - drivers/reset/Makefile | 1 - 24 files changed, 938 insertions(+), 605 deletions(-) --- base-commit: 30d01b582f2274eb8c808026c5fb4c33e9f2210d // Caleb (they/them)
Re: [PATCH] arm: init: add an option to use FDT from previous bootloader
On Tue, Oct 24, 2023 at 08:52:22PM +0100, Caleb Connolly wrote: > > > On 24/10/2023 20:25, Tom Rini wrote: > > On Tue, Oct 24, 2023 at 12:32:35PM +0100, Caleb Connolly wrote: > > > >> Add a new config option to allow u-boot to reuse the FDT provided by the > >> previous stage bootloader when available. > >> > >> On some boards the previous stage bootloader can populate > >> platform-specific parts of the devicetree such as the memory node, this > >> allows us to avoid hardcoding it in u-boot and instead determine it > >> dynamically at runtime. > >> > >> Signed-off-by: Caleb Connolly > >> --- > >> This patch will improve generic support for Qualcomm boards by enabling > >> us to configure the memory map at runtime rather than having hardcoded > >> maps on a per-device basis. I've gone for this approach initially to try > >> and avoid introducing board specific code where possible, but I'm happy > >> to rework this into mach-snapdragon if that's preferred. > >> --- > >> base-commit: e65b5d35c9116485366bb08138043d51220551da > >> > >> // Caleb (they/them) > >> --- > >> arch/arm/lib/save_prev_bl_data.c | 7 +++ > >> boot/Kconfig | 10 ++ > >> include/init.h | 9 + > >> lib/fdtdec.c | 7 ++- > > > > So what is different with this instead of using save_boot_params ? I'm > > not saying this isn't needed, but I don't immediately see why > > save_boot_params + (OF_HAS_PRIOR_STAGE=y&_BOARD=y) isn't the solution > > to the problem. > > OF_BOARD would work here by implementing board_fdt_blob_setup() for > mach-snapdragon, my hope was to avoid doing this in a board-specific > way, but if that's preferred then I can totally do that instead. > > I left a comment below the patch about this, although it probably wasn't > super clear. Ah, yes. I guess I don't see what I suggested as "board specific" in that outside of efforts like bloblist that Simon mentioned, every semiconductor is slightly different. So yes please, OF_BOARD and board_fdt_blob_setup and so forth instead, thanks. -- Tom signature.asc Description: PGP signature
Re: [PATCH] arm: init: add an option to use FDT from previous bootloader
On 24/10/2023 20:25, Tom Rini wrote: > On Tue, Oct 24, 2023 at 12:32:35PM +0100, Caleb Connolly wrote: > >> Add a new config option to allow u-boot to reuse the FDT provided by the >> previous stage bootloader when available. >> >> On some boards the previous stage bootloader can populate >> platform-specific parts of the devicetree such as the memory node, this >> allows us to avoid hardcoding it in u-boot and instead determine it >> dynamically at runtime. >> >> Signed-off-by: Caleb Connolly >> --- >> This patch will improve generic support for Qualcomm boards by enabling >> us to configure the memory map at runtime rather than having hardcoded >> maps on a per-device basis. I've gone for this approach initially to try >> and avoid introducing board specific code where possible, but I'm happy >> to rework this into mach-snapdragon if that's preferred. >> --- >> base-commit: e65b5d35c9116485366bb08138043d51220551da >> >> // Caleb (they/them) >> --- >> arch/arm/lib/save_prev_bl_data.c | 7 +++ >> boot/Kconfig | 10 ++ >> include/init.h | 9 + >> lib/fdtdec.c | 7 ++- > > So what is different with this instead of using save_boot_params ? I'm > not saying this isn't needed, but I don't immediately see why > save_boot_params + (OF_HAS_PRIOR_STAGE=y&_BOARD=y) isn't the solution > to the problem. OF_BOARD would work here by implementing board_fdt_blob_setup() for mach-snapdragon, my hope was to avoid doing this in a board-specific way, but if that's preferred then I can totally do that instead. I left a comment below the patch about this, although it probably wasn't super clear. Thanks, > -- // Caleb (they/them)
Re: [PATCH] arm: init: add an option to use FDT from previous bootloader
Hi Caleb, On Tue, 24 Oct 2023 at 11:10, Caleb Connolly wrote: > > > > On 24/10/2023 19:03, Simon Glass wrote: > > Hi Caleb, > > > > On Tue, 24 Oct 2023 at 04:32, Caleb Connolly > > wrote: > >> > >> Add a new config option to allow u-boot to reuse the FDT provided by the > > > > U-Boot (please fix throughout) > > Will do! > > > >> previous stage bootloader when available. > >> > >> On some boards the previous stage bootloader can populate > >> platform-specific parts of the devicetree such as the memory node, this > >> allows us to avoid hardcoding it in u-boot and instead determine it > >> dynamically at runtime. > >> > >> Signed-off-by: Caleb Connolly > >> --- > >> This patch will improve generic support for Qualcomm boards by enabling > >> us to configure the memory map at runtime rather than having hardcoded > >> maps on a per-device basis. I've gone for this approach initially to try > >> and avoid introducing board specific code where possible, but I'm happy > >> to rework this into mach-snapdragon if that's preferred. > > > > Do you think it could use bloblist instead? I did send a patch to use > > the FDT in the bloblist: > > If it would require changes to the previous stage bootloader then > unfortunately not. We're mostly stuck with the behaviour that we've got > for now... Yes it would. What is the previous-stage bootloader? > > This mechanism of retrieving the DTB is also used on the apple M1 I > think, and any other board where we're booting U-Boot as a drop-in for > the kernel on arm64. I believe M1 is an open source project so perhaps they could adopt firmware handoff when it is finalised. Anyway, can this use OF_BOARD instead, perhaps? It already permits board-specific actions and is used by some rpi boards. > > > > https://patchwork.ozlabs.org/project/uboot/patch/20230926141514.2101787-40-...@chromium.org/ > > > > Regards, > > Simon > > Regards, Simon
Re: [PATCH v4 1/1] efi_loader: expose the device-tree file name
On Tue, 24 Oct 2023 at 09:20, Heinrich Schuchardt wrote: > > Forward and backward compatibility of Linux kernel device-trees is > sometimes missing. One solution approach is to load a kernel specific > device-tree. This can either be done via a U-Boot scripts (like the one > generated by Debian package flash-kernel or by a boot loader like GRUB. > The boot loader approach currently requires to know the device-tree name > before first boot which makes it unusable for generic images. > > Expose the device-tree file name as EFI variable FdtFile. > This will allow bootloaders to load a kernel specific device-tree. > > The variable will not be exposed on ACPI based systems or if the > environment variable fdtfile is not defined. > > Signed-off-by: Heinrich Schuchardt > --- > v4: > Generalize the description of the content of $fdtfile. > v3: > Add documentation > v2: > Use a unique GUID to enable future U-Boot independent > standardization. > Do not try to add the variable on ACPI based systems. > --- > doc/develop/uefi/uefi.rst | 14 ++ > include/efi_loader.h | 5 + > lib/efi_loader/efi_setup.c | 30 ++ > 3 files changed, 49 insertions(+) > > diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst > index fb16ac743a..702c490831 100644 > --- a/doc/develop/uefi/uefi.rst > +++ b/doc/develop/uefi/uefi.rst > @@ -916,6 +916,20 @@ So our final format of the FilePathList[] is:: > > Loaded image - end node (0xff) - VenMedia - initrd_1 - [end node (0x01) > - initrd_n ...] - end node (0xff) > > +EFI variable FdtFile > + > + > +Ideally U-Boot would always expose a device-tree that can be used for booting > +any operating systems. Unfortunately operating systems like Linux sometimes > +break forward and backward compatibility. In this case there is a need to > load > +an operating system version specific device-tree. > + > +U-Boot has an environment variable fdtfile identifying the device-tree file > to > +load. The content of this variable is exposed as EFI variable Fdtfile, vendor > +GUID d45dde69-3bd6-40e0-90d5-6b606aa57730. It contains the device-tree path > +name as a NUL terminated ASCII string. On many architectures the file name is > +preceded by a vendor directory ('vendor-directory/board-name.dtb'). > + > Links > - > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index e24410505f..146e7f1bce 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -152,6 +152,11 @@ static inline efi_status_t efi_launch_capsules(void) > EFI_GUID(0x8108ac4e, 0x9f11, 0x4d59, \ > 0x85, 0x0e, 0xe2, 0x1a, 0x52, 0x2c, 0x59, 0xb2) > > +/* Vendor GUID for the FdtFile variable */ > +#define VENDOR_FDTFILE_GUID \ > + EFI_GUID(0xd45dde69, 0x3bd6, 0x40e0, \ > +0x90, 0xd5, 0x6b, 0x60, 0x6a, 0xa5, 0x77, 0x30) > + > /* Use internal device tree when starting UEFI application */ > #define EFI_FDT_USE_INTERNAL NULL > > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c > index e6de685e87..71bcde645b 100644 > --- a/lib/efi_loader/efi_setup.c > +++ b/lib/efi_loader/efi_setup.c > @@ -17,6 +17,8 @@ > > efi_status_t efi_obj_list_initialized = OBJ_LIST_NOT_INITIALIZED; > > +efi_guid_t vendor_fdtfile_guid = VENDOR_FDTFILE_GUID; > + > /* > * Allow unaligned memory access. > * > @@ -26,6 +28,27 @@ void __weak allow_unaligned(void) > { > } > > +/** > + * efi_init_fdtfile() - set EFI variable FdtFile > + * > + * Return: status code > + */ > +static efi_status_t efi_init_fdtfile(void) > +{ > + char *val; > + > + val = env_get("fdtfile"); > + if (!val) > + return EFI_SUCCESS; > + > + return efi_set_variable_int(u"FdtFile", > + _fdtfile_guid, > + EFI_VARIABLE_BOOTSERVICE_ACCESS | > + EFI_VARIABLE_RUNTIME_ACCESS | > + EFI_VARIABLE_READ_ONLY, > + strlen(val) + 1, val, false); > +} > + > /** > * efi_init_platform_lang() - define supported languages > * > @@ -250,6 +273,13 @@ efi_status_t efi_init_obj_list(void) > if (ret != EFI_SUCCESS) > goto out; > > + /* Define EFI variable FdtFile */ > + if (!CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)) { > + ret = efi_init_fdtfile(); > + if (ret != EFI_SUCCESS) > + goto out; > + } > + > /* Indicate supported features */ > ret = efi_init_os_indications(); > if (ret != EFI_SUCCESS) > -- > 2.40.1 > Reviewed-by: Ilias Apalodimas
Re: [PATCH v2] arm: init: add an option to use FDT from previous bootloader
On Tue, Oct 24, 2023 at 06:38:43PM +0100, Caleb Connolly wrote: > Add a new config option to allow u-boot to reuse the FDT provided by the > previous stage bootloader when available. > > On some boards the previous stage bootloader can populate > platform-specific parts of the devicetree such as the memory node, this > allows us to avoid hardcoding it in u-boot and instead determine it > dynamically at runtime. > > Signed-off-by: Caleb Connolly [snip] > +/** > + * get_prev_bl_fdt_addr - When u-boot is chainloaded, get the address > + * of the FDT passed by the previous bootloader. > + * > + * Return: the address of the FDT passed by the previous bootloader > + * or 0 if not found. > + */ > +phys_addr_t get_prev_bl_fdt_addr(void); > +#else > +#define get_prev_bl_fdt_addr() 0LLU > #endif Question in v1 aside, why do you need the dummy define here? -- Tom signature.asc Description: PGP signature
Re: [PATCH v4 1/1] efi_loader: expose the device-tree file name
On Tue, 24 Oct 2023 at 09:20, Heinrich Schuchardt wrote: > > Forward and backward compatibility of Linux kernel device-trees is > sometimes missing. One solution approach is to load a kernel specific > device-tree. This can either be done via a U-Boot scripts (like the one > generated by Debian package flash-kernel or by a boot loader like GRUB. > The boot loader approach currently requires to know the device-tree name > before first boot which makes it unusable for generic images. > > Expose the device-tree file name as EFI variable FdtFile. > This will allow bootloaders to load a kernel specific device-tree. > > The variable will not be exposed on ACPI based systems or if the > environment variable fdtfile is not defined. > > Signed-off-by: Heinrich Schuchardt > --- > v4: > Generalize the description of the content of $fdtfile. > v3: > Add documentation > v2: > Use a unique GUID to enable future U-Boot independent > standardization. > Do not try to add the variable on ACPI based systems. > --- > doc/develop/uefi/uefi.rst | 14 ++ > include/efi_loader.h | 5 + > lib/efi_loader/efi_setup.c | 30 ++ > 3 files changed, 49 insertions(+) > > diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst > index fb16ac743a..702c490831 100644 > --- a/doc/develop/uefi/uefi.rst > +++ b/doc/develop/uefi/uefi.rst > @@ -916,6 +916,20 @@ So our final format of the FilePathList[] is:: > > Loaded image - end node (0xff) - VenMedia - initrd_1 - [end node (0x01) > - initrd_n ...] - end node (0xff) > > +EFI variable FdtFile > + > + > +Ideally U-Boot would always expose a device-tree that can be used for booting > +any operating systems. Unfortunately operating systems like Linux sometimes > +break forward and backward compatibility. In this case there is a need to > load > +an operating system version specific device-tree. > + > +U-Boot has an environment variable fdtfile identifying the device-tree file > to > +load. The content of this variable is exposed as EFI variable Fdtfile, vendor > +GUID d45dde69-3bd6-40e0-90d5-6b606aa57730. It contains the device-tree path > +name as a NUL terminated ASCII string. On many architectures the file name is > +preceded by a vendor directory ('vendor-directory/board-name.dtb'). > + > Links > - > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index e24410505f..146e7f1bce 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -152,6 +152,11 @@ static inline efi_status_t efi_launch_capsules(void) > EFI_GUID(0x8108ac4e, 0x9f11, 0x4d59, \ > 0x85, 0x0e, 0xe2, 0x1a, 0x52, 0x2c, 0x59, 0xb2) > > +/* Vendor GUID for the FdtFile variable */ > +#define VENDOR_FDTFILE_GUID \ > + EFI_GUID(0xd45dde69, 0x3bd6, 0x40e0, \ > +0x90, 0xd5, 0x6b, 0x60, 0x6a, 0xa5, 0x77, 0x30) > + > /* Use internal device tree when starting UEFI application */ > #define EFI_FDT_USE_INTERNAL NULL > > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c > index e6de685e87..71bcde645b 100644 > --- a/lib/efi_loader/efi_setup.c > +++ b/lib/efi_loader/efi_setup.c > @@ -17,6 +17,8 @@ > > efi_status_t efi_obj_list_initialized = OBJ_LIST_NOT_INITIALIZED; > > +efi_guid_t vendor_fdtfile_guid = VENDOR_FDTFILE_GUID; > + > /* > * Allow unaligned memory access. > * > @@ -26,6 +28,27 @@ void __weak allow_unaligned(void) > { > } > > +/** > + * efi_init_fdtfile() - set EFI variable FdtFile > + * > + * Return: status code > + */ > +static efi_status_t efi_init_fdtfile(void) > +{ > + char *val; > + > + val = env_get("fdtfile"); > + if (!val) > + return EFI_SUCCESS; > + > + return efi_set_variable_int(u"FdtFile", > + _fdtfile_guid, > + EFI_VARIABLE_BOOTSERVICE_ACCESS | > + EFI_VARIABLE_RUNTIME_ACCESS | > + EFI_VARIABLE_READ_ONLY, > + strlen(val) + 1, val, false); > +} > + > /** > * efi_init_platform_lang() - define supported languages > * > @@ -250,6 +273,13 @@ efi_status_t efi_init_obj_list(void) > if (ret != EFI_SUCCESS) > goto out; > > + /* Define EFI variable FdtFile */ > + if (!CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)) { > + ret = efi_init_fdtfile(); > + if (ret != EFI_SUCCESS) > + goto out; > + } > + > /* Indicate supported features */ > ret = efi_init_os_indications(); > if (ret != EFI_SUCCESS) > -- > 2.40.1 >
Re: [PATCH] arm: init: add an option to use FDT from previous bootloader
On Tue, Oct 24, 2023 at 12:32:35PM +0100, Caleb Connolly wrote: > Add a new config option to allow u-boot to reuse the FDT provided by the > previous stage bootloader when available. > > On some boards the previous stage bootloader can populate > platform-specific parts of the devicetree such as the memory node, this > allows us to avoid hardcoding it in u-boot and instead determine it > dynamically at runtime. > > Signed-off-by: Caleb Connolly > --- > This patch will improve generic support for Qualcomm boards by enabling > us to configure the memory map at runtime rather than having hardcoded > maps on a per-device basis. I've gone for this approach initially to try > and avoid introducing board specific code where possible, but I'm happy > to rework this into mach-snapdragon if that's preferred. > --- > base-commit: e65b5d35c9116485366bb08138043d51220551da > > // Caleb (they/them) > --- > arch/arm/lib/save_prev_bl_data.c | 7 +++ > boot/Kconfig | 10 ++ > include/init.h | 9 + > lib/fdtdec.c | 7 ++- So what is different with this instead of using save_boot_params ? I'm not saying this isn't needed, but I don't immediately see why save_boot_params + (OF_HAS_PRIOR_STAGE=y&_BOARD=y) isn't the solution to the problem. -- Tom signature.asc Description: PGP signature
Re: [PATCH v8 3/8] power: pmic: add the base MAX77663 PMIC support
On Tue, Oct 24, 2023 at 10:41:05AM +0300, Svyatoslav Ryhel wrote: > Add support to bind the regulators/child nodes with the pmic. > Also adds the pmic i2c based read/write functions to access pmic > registers. > > Signed-off-by: Svyatoslav Ryhel > Reviewed-by: Simon Glass [snip] > diff --git a/drivers/power/pmic/max77663.c b/drivers/power/pmic/max77663.c > new file mode 100644 > index 00..4070235d3c > --- /dev/null > +++ b/drivers/power/pmic/max77663.c > @@ -0,0 +1,86 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright(C) 2023 Svyatoslav Ryhel > + */ > + > +#include Should have noted sooner, sorry. Please don't use in new code, and audit the include list you have as well (this one looks reasonable I suspect, all the same, so general advice). -- Tom signature.asc Description: PGP signature
[PATCH 2/2] bootstd: cros: Correct condition for read method
This has a typo which makes the method inoperable. Correct it so that 'bootflow read' works correctly for ChromeOS. Signed-off-by: Simon Glass --- boot/bootmeth_cros.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/boot/bootmeth_cros.c b/boot/bootmeth_cros.c index 20e0b1e89c36..cd72db8250ce 100644 --- a/boot/bootmeth_cros.c +++ b/boot/bootmeth_cros.c @@ -406,7 +406,7 @@ static int cros_read_file(struct udevice *dev, struct bootflow *bflow, return -ENOSYS; } -#if CONFIG_IS_ENABLED(BOOSTD_FULL) +#if CONFIG_IS_ENABLED(BOOTSTD_FULL) static int cros_read_all(struct udevice *dev, struct bootflow *bflow) { int ret; @@ -419,7 +419,7 @@ static int cros_read_all(struct udevice *dev, struct bootflow *bflow) return 0; } -#endif /* BOOSTD_FULL */ +#endif /* BOOTSTD_FULL */ static int cros_boot(struct udevice *dev, struct bootflow *bflow) { @@ -458,9 +458,9 @@ static struct bootmeth_ops cros_bootmeth_ops = { .read_bootflow = cros_read_bootflow, .read_file = cros_read_file, .boot = cros_boot, -#if CONFIG_IS_ENABLED(BOOSTD_FULL) +#if CONFIG_IS_ENABLED(BOOTSTD_FULL) .read_all = cros_read_all, -#endif /* BOOSTD_FULL */ +#endif /* BOOTSTD_FULL */ }; static const struct udevice_id cros_bootmeth_ids[] = { -- 2.42.0.758.gaed0368e0e-goog
[PATCH 1/2] bootstd: Handle a few special cases in cmdline_set_arg()
Two bugs have appeared: - arguments can have an equals sign embedded in them, which must be considered part of the value - arguments must fully match the name; partial matches should be ignored Fix these and add a test to cover both. Signed-off-by: Simon Glass --- boot/bootflow.c | 5 +++-- test/boot/bootflow.c | 20 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/boot/bootflow.c b/boot/bootflow.c index be543c8588cc..6922e7e0c4e7 100644 --- a/boot/bootflow.c +++ b/boot/bootflow.c @@ -752,7 +752,7 @@ int cmdline_set_arg(char *buf, int maxlen, const char *cmdline, in_quote = false; continue; } - if (*p == '=') { + if (*p == '=' && !arg_end) { arg_end = p; val = p + 1; } else if (*p == '"') { @@ -788,7 +788,8 @@ int cmdline_set_arg(char *buf, int maxlen, const char *cmdline, } /* if this is the target arg, update it */ - if (!strncmp(from, set_arg, arg_end - from)) { + if (arg_end - from == set_arg_len && + !strncmp(from, set_arg, set_arg_len)) { if (!buf) { bool has_quote = val_end[-1] == '"'; diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c index f5b2059140ac..f640db8a2418 100644 --- a/test/boot/bootflow.c +++ b/test/boot/bootflow.c @@ -973,6 +973,26 @@ static int bootflow_cmdline(struct unit_test_state *uts) } BOOTSTD_TEST(bootflow_cmdline, 0); +/* test a few special changes to a long command line */ +static int bootflow_cmdline_special(struct unit_test_state *uts) +{ + char buf[500]; + int pos; + + /* +* check handling of an argument which has an embedded '=', as well as +* handling of a argument which partially matches ("ro" and "root") +*/ + ut_asserteq(32, cmdline_set_arg( + buf, sizeof(buf), + "loglevel=7 root=PARTUUID=d68352e3 rootwait ro noinitrd", + "root", NULL, )); + ut_asserteq_str("loglevel=7 rootwait ro noinitrd", buf); + + return 0; +} +BOOTSTD_TEST(bootflow_cmdline_special, 0); + /* Test ChromiumOS bootmeth */ static int bootflow_cros(struct unit_test_state *uts) { -- 2.42.0.758.gaed0368e0e-goog
[PATCH 0/2] bootstd: Fix a few minor bugs
This series fixes a few bugs noticed recently: - handling of certain command-line updates with 'bootflow cmdline' - support for 'bootflow read' in ChromeOS Simon Glass (2): bootstd: Handle a few special cases in cmdline_set_arg() bootstd: cros: Correct condition for read method boot/bootflow.c | 5 +++-- boot/bootmeth_cros.c | 8 test/boot/bootflow.c | 20 3 files changed, 27 insertions(+), 6 deletions(-) -- 2.42.0.758.gaed0368e0e-goog
Re: [PATCH] arm: init: add an option to use FDT from previous bootloader
On 24/10/2023 19:03, Simon Glass wrote: > Hi Caleb, > > On Tue, 24 Oct 2023 at 04:32, Caleb Connolly > wrote: >> >> Add a new config option to allow u-boot to reuse the FDT provided by the > > U-Boot (please fix throughout) Will do! > >> previous stage bootloader when available. >> >> On some boards the previous stage bootloader can populate >> platform-specific parts of the devicetree such as the memory node, this >> allows us to avoid hardcoding it in u-boot and instead determine it >> dynamically at runtime. >> >> Signed-off-by: Caleb Connolly >> --- >> This patch will improve generic support for Qualcomm boards by enabling >> us to configure the memory map at runtime rather than having hardcoded >> maps on a per-device basis. I've gone for this approach initially to try >> and avoid introducing board specific code where possible, but I'm happy >> to rework this into mach-snapdragon if that's preferred. > > Do you think it could use bloblist instead? I did send a patch to use > the FDT in the bloblist: If it would require changes to the previous stage bootloader then unfortunately not. We're mostly stuck with the behaviour that we've got for now... This mechanism of retrieving the DTB is also used on the apple M1 I think, and any other board where we're booting U-Boot as a drop-in for the kernel on arm64. > > https://patchwork.ozlabs.org/project/uboot/patch/20230926141514.2101787-40-...@chromium.org/ > > Regards, > Simon > >> --- >> base-commit: e65b5d35c9116485366bb08138043d51220551da >> >> // Caleb (they/them) >> --- >> arch/arm/lib/save_prev_bl_data.c | 7 +++ >> boot/Kconfig | 10 ++ >> include/init.h | 9 + >> lib/fdtdec.c | 7 ++- >> 4 files changed, 32 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm/lib/save_prev_bl_data.c >> b/arch/arm/lib/save_prev_bl_data.c >> index f7b23faf0d66..b0608502535e 100644 >> --- a/arch/arm/lib/save_prev_bl_data.c >> +++ b/arch/arm/lib/save_prev_bl_data.c >> @@ -45,6 +45,13 @@ bool is_addr_accessible(phys_addr_t addr) >> return false; >> } >> >> +phys_addr_t get_prev_bl_fdt_addr(void) >> +{ >> + if (!is_addr_accessible((phys_addr_t)reg0)) >> + return 0; >> + return reg0; >> +} >> + >> int save_prev_bl_data(void) >> { >> struct fdt_header *fdt_blob; >> diff --git a/boot/Kconfig b/boot/Kconfig >> index a01e6cb8aafe..c127ba254589 100644 >> --- a/boot/Kconfig >> +++ b/boot/Kconfig >> @@ -1599,6 +1599,16 @@ config SAVE_PREV_BL_INITRAMFS_START_ADDR >> If no initramfs was provided by previous bootloader, no env >> variables >> will be created. >> >> +config USE_PREV_BL_FDT >> + depends on SAVE_PREV_BL_FDT_ADDR && !OF_BOARD >> + bool "Use the FDT provided by the previous stage bootloader" >> + help >> + When u-boot is chain-loaded from a previous bootloader, enable >> this option >> + to use the FDT provided by the previous bootloader instead of any >> built-in >> + to u-boot. >> + >> + If no FDT was available, u-boot will fall back to its internal FDT. >> + >> menu "Configuration editor" >> >> config CEDIT >> diff --git a/include/init.h b/include/init.h >> index 4e7fe26c2004..58604cd98758 100644 >> --- a/include/init.h >> +++ b/include/init.h >> @@ -168,6 +168,15 @@ defined(CONFIG_SAVE_PREV_BL_FDT_ADDR) >> * Return: 0 if ok; -ENODATA on error >> */ >> int save_prev_bl_data(void); >> + >> +/** >> + * get_prev_bl_fdt_addr - When u-boot is chainloaded, get the address >> + * of the FDT passed by the previous bootloader. >> + * >> + * Return: the address of the FDT passed by the previous bootloader >> + * or 0 if not found. >> + */ >> +phys_addr_t get_prev_bl_fdt_addr(void); >> #endif >> >> /** >> diff --git a/lib/fdtdec.c b/lib/fdtdec.c >> index 7a6916764835..85425f2dc1ee 100644 >> --- a/lib/fdtdec.c >> +++ b/lib/fdtdec.c >> @@ -1222,7 +1222,7 @@ static int uncompress_blob(const void *src, ulong >> sz_src, void **dstp) >> */ >> static void *fdt_find_separate(void) >> { >> - void *fdt_blob = NULL; >> + void *fdt_blob = NULL, *prevbl_fdt_blob; >> >> if (IS_ENABLED(CONFIG_SANDBOX)) >> return NULL; >> @@ -1234,6 +1234,11 @@ static void *fdt_find_separate(void) >> else >> fdt_blob = (ulong *)__bss_end; >> #else >> +#if defined(CONFIG_USE_PREV_BL_FDT) >> + prevbl_fdt_blob = (void *)get_prev_bl_fdt_addr(); >> + if (prevbl_fdt_blob) >> + return prevbl_fdt_blob; >> +#endif >> /* FDT is at end of image */ >> fdt_blob = (ulong *)_end; >> >> > > Regards, > SImon -- // Caleb (they/them)
Re: [v4.1 2/2] CI, pytest: Add a test for sandbox without LTO
On Tue, Oct 24, 2023 at 11:02:06AM -0700, Simon Glass wrote: > Hi Tom, > > On Mon, 23 Oct 2023 at 10:28, Tom Rini wrote: > > > > On Mon, Oct 23, 2023 at 10:13:52AM -0700, Simon Glass wrote: > > > > [snip] > > > BTW buildman supports -L which disabled LTO using the NO_LTO=1 option > > > > I worry about putting sandbox-specific flags in buildman. Outside of > > sandbox, targets that enable LTO require LTO, just like any other CONFIG > > option. > > Some problems with LTO and why I don't normally develop with it enabled: > > - build time > - code moves around all over the place so it is hard to compare size growth > > At least for my IDE flow, I use -L in most cases. Yes there are some > boards which won't fit without LTO, but I don't see them much. > > So this is mostly a dev convenience / productivity tool. Yes, it does take longer to link. And yes, a more complex optimization does make some size tracking harder to understand (since growth or shrinkage allows for different optimizations to be made around it). But everything in configs/ that enables LTO needs LTO. -- Tom signature.asc Description: PGP signature
Re: [tom.r...@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
On Tue, Oct 24, 2023 at 08:35:58PM +0530, Sughosh Ganu wrote: > hi Tom, > > On Tue, 24 Oct 2023 at 06:48, Tom Rini wrote: > > > > Here's the latest report > > > > -- Forwarded message - > > From: > > Date: Mon, Oct 23, 2023 at 4:40 PM > > Subject: New Defects reported by Coverity Scan for Das U-Boot > > To: > > > > > > Hi, > > > > Please find the latest report on new defect(s) introduced to Das > > U-Boot found with Coverity Scan. > > > > 16 new defect(s) introduced to Das U-Boot found with Coverity Scan. > > 6 defect(s), reported by Coverity Scan earlier, were marked fixed in > > the recent build analyzed by Coverity Scan. > > > > New defect(s) Reported-by: Coverity Scan > > Showing 16 of 16 defect(s) > > > > > > > > > ** CID 467053:(RESOURCE_LEAK) > > /tools/mkeficapsule.c: 859 in dump_capsule_contents() > > /tools/mkeficapsule.c: 859 in dump_capsule_contents() > > > > > > > > *** CID 467053:(RESOURCE_LEAK) > > /tools/mkeficapsule.c: 859 in dump_capsule_contents() > > 853 empty_capsule_dump(ptr); > > 854 } else { > > 855 fprintf(stderr, "Unable to decode the capsule > > file: %s\n", > > 856 capsule_file); > > 857 exit(EXIT_FAILURE); > > 858 } > > >>> CID 467053:(RESOURCE_LEAK) > > >>> Variable "ptr" going out of scope leaks the storage it points to. > > 859 } > > 860 > > 861 /** > > 862 * main - main entry function of mkeficapsule > > 863 * @argc: Number of arguments > > 864 * @argv: Array of pointers to arguments > > /tools/mkeficapsule.c: 859 in dump_capsule_contents() > > 853 empty_capsule_dump(ptr); > > 854 } else { > > 855 fprintf(stderr, "Unable to decode the capsule > > file: %s\n", > > 856 capsule_file); > > 857 exit(EXIT_FAILURE); > > 858 } > > >>> CID 467053:(RESOURCE_LEAK) > > >>> Variable "ptr" going out of scope leaks the storage it points to. > > 859 } > > 860 > > 861 /** > > 862 * main - main entry function of mkeficapsule > > 863 * @argc: Number of arguments > > 864 * @argv: Array of pointers to arguments > > > > > > > ** CID 467045: Resource leaks (RESOURCE_LEAK) > > /tools/mkeficapsule.c: 859 in dump_capsule_contents() > > > > > > > > *** CID 467045: Resource leaks (RESOURCE_LEAK) > > /tools/mkeficapsule.c: 859 in dump_capsule_contents() > > 853 empty_capsule_dump(ptr); > > 854 } else { > > 855 fprintf(stderr, "Unable to decode the capsule > > file: %s\n", > > 856 capsule_file); > > 857 exit(EXIT_FAILURE); > > 858 } > > >>> CID 467045: Resource leaks (RESOURCE_LEAK) > > >>> Handle variable "fd" going out of scope leaks the handle. > > 859 } > > 860 > > 861 /** > > 862 * main - main entry function of mkeficapsule > > 863 * @argc: Number of arguments > > 864 * @argv: Array of pointers to arguments > > > > > > Both the pointer and file descriptor are not being freed since the > process exits once the dump_capaule_contents() function returns. These > can be marked as false positives. Thanks. I would say that's "intentional" rather than false positive (and perhaps a bad example) but indeed not a fatal problem. Thanks for checking. -- Tom signature.asc Description: PGP signature
Re: [PATCH v4 2/2] DONOTMERGE: arm: dts: k3-j7200-binman: Enable split mode for MCU R5
On Mon, 23 Oct 2023 at 01:01, Neha Malcom Francis wrote: > > Set boot core-opts to enable split mode for MCU R5 cluster by default. > This patch serves to demonstrate how this can be done. > > Signed-off-by: Neha Malcom Francis > --- > No change since v2 > > arch/arm/dts/k3-j7200-binman.dtsi | 3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH 1/1] tools: mkimage: fix sfspl_image_extract_subimage()
On Tue, 24 Oct 2023 at 00:26, Heinrich Schuchardt wrote: > > Do not leak file descriptor if writing fails. > Correct the error text if opening a file fails. > > Addresses-Coverity-ID: 467054 Resource leaks > Fixes: 64fd30d367a1 ("tools: mkimage: Add StarFive SPL image support") > Signed-off-by: Heinrich Schuchardt > --- > tools/sfspl.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH] arm: init: add an option to use FDT from previous bootloader
Hi Caleb, On Tue, 24 Oct 2023 at 04:32, Caleb Connolly wrote: > > Add a new config option to allow u-boot to reuse the FDT provided by the U-Boot (please fix throughout) > previous stage bootloader when available. > > On some boards the previous stage bootloader can populate > platform-specific parts of the devicetree such as the memory node, this > allows us to avoid hardcoding it in u-boot and instead determine it > dynamically at runtime. > > Signed-off-by: Caleb Connolly > --- > This patch will improve generic support for Qualcomm boards by enabling > us to configure the memory map at runtime rather than having hardcoded > maps on a per-device basis. I've gone for this approach initially to try > and avoid introducing board specific code where possible, but I'm happy > to rework this into mach-snapdragon if that's preferred. Do you think it could use bloblist instead? I did send a patch to use the FDT in the bloblist: https://patchwork.ozlabs.org/project/uboot/patch/20230926141514.2101787-40-...@chromium.org/ Regards, Simon > --- > base-commit: e65b5d35c9116485366bb08138043d51220551da > > // Caleb (they/them) > --- > arch/arm/lib/save_prev_bl_data.c | 7 +++ > boot/Kconfig | 10 ++ > include/init.h | 9 + > lib/fdtdec.c | 7 ++- > 4 files changed, 32 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/lib/save_prev_bl_data.c > b/arch/arm/lib/save_prev_bl_data.c > index f7b23faf0d66..b0608502535e 100644 > --- a/arch/arm/lib/save_prev_bl_data.c > +++ b/arch/arm/lib/save_prev_bl_data.c > @@ -45,6 +45,13 @@ bool is_addr_accessible(phys_addr_t addr) > return false; > } > > +phys_addr_t get_prev_bl_fdt_addr(void) > +{ > + if (!is_addr_accessible((phys_addr_t)reg0)) > + return 0; > + return reg0; > +} > + > int save_prev_bl_data(void) > { > struct fdt_header *fdt_blob; > diff --git a/boot/Kconfig b/boot/Kconfig > index a01e6cb8aafe..c127ba254589 100644 > --- a/boot/Kconfig > +++ b/boot/Kconfig > @@ -1599,6 +1599,16 @@ config SAVE_PREV_BL_INITRAMFS_START_ADDR > If no initramfs was provided by previous bootloader, no env > variables > will be created. > > +config USE_PREV_BL_FDT > + depends on SAVE_PREV_BL_FDT_ADDR && !OF_BOARD > + bool "Use the FDT provided by the previous stage bootloader" > + help > + When u-boot is chain-loaded from a previous bootloader, enable this > option > + to use the FDT provided by the previous bootloader instead of any > built-in > + to u-boot. > + > + If no FDT was available, u-boot will fall back to its internal FDT. > + > menu "Configuration editor" > > config CEDIT > diff --git a/include/init.h b/include/init.h > index 4e7fe26c2004..58604cd98758 100644 > --- a/include/init.h > +++ b/include/init.h > @@ -168,6 +168,15 @@ defined(CONFIG_SAVE_PREV_BL_FDT_ADDR) > * Return: 0 if ok; -ENODATA on error > */ > int save_prev_bl_data(void); > + > +/** > + * get_prev_bl_fdt_addr - When u-boot is chainloaded, get the address > + * of the FDT passed by the previous bootloader. > + * > + * Return: the address of the FDT passed by the previous bootloader > + * or 0 if not found. > + */ > +phys_addr_t get_prev_bl_fdt_addr(void); > #endif > > /** > diff --git a/lib/fdtdec.c b/lib/fdtdec.c > index 7a6916764835..85425f2dc1ee 100644 > --- a/lib/fdtdec.c > +++ b/lib/fdtdec.c > @@ -1222,7 +1222,7 @@ static int uncompress_blob(const void *src, ulong > sz_src, void **dstp) > */ > static void *fdt_find_separate(void) > { > - void *fdt_blob = NULL; > + void *fdt_blob = NULL, *prevbl_fdt_blob; > > if (IS_ENABLED(CONFIG_SANDBOX)) > return NULL; > @@ -1234,6 +1234,11 @@ static void *fdt_find_separate(void) > else > fdt_blob = (ulong *)__bss_end; > #else > +#if defined(CONFIG_USE_PREV_BL_FDT) > + prevbl_fdt_blob = (void *)get_prev_bl_fdt_addr(); > + if (prevbl_fdt_blob) > + return prevbl_fdt_blob; > +#endif > /* FDT is at end of image */ > fdt_blob = (ulong *)_end; > > Regards, SImon
Re: [PATCH v4 1/2] binman: openssl: x509: ti_secure_rom: Add support for bootcore_opts
On Mon, 23 Oct 2023 at 01:01, Neha Malcom Francis wrote: > > According to the TRMs of K3 platform of devices, the ROM boot image > format specifies a "Core Options Field" that provides the capability to > set the boot core in lockstep when set to 0 or to split mode when set > to 2. Add support for providing the same from the binman DTS. Also > modify existing test case for ensuring future coverage. > > Signed-off-by: Neha Malcom Francis > --- > Link to J721E TRM: https://www.ti.com/lit/zip/spruil1 > Section 4.5.4.1 Boot Info > > Changes in v4: > - corrected function comments, 0 for lockstep, 2 for split mode > > Changes in v3: > - updated function comments > - removed inconsistency in setting bootcore_opts to 32 > > Changes in v2: > - included TRM link in commit message > > tools/binman/btool/openssl.py | 6 -- > tools/binman/entries.rst| 1 + > tools/binman/etype/ti_secure_rom.py | 11 +-- > tools/binman/etype/x509_cert.py | 3 ++- > tools/binman/test/297_ti_secure_rom.dts | 1 + > 5 files changed, 17 insertions(+), 5 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 1/1] mkimage: do not write incorrect error message
On Tue, 24 Oct 2023 at 00:19, Heinrich Schuchardt wrote: > > When running 'mkimage -l' is called for a valid StarFive file an error > message "Error: invalid marker bytes" is written by the Renesas SPKG > driver. > > mkimage -l may be invoked without specifying an image type. In this case > mkimage iterates over all image type drivers to find the one that matches. > None of the non-matching drivers should write an error message. > > Fix the Renesas SPKG driver. > > Fixes: afdfcb11f97c ("tools: spkgimage: add Renesas SPKG format") > Signed-off-by: Heinrich Schuchardt > --- > tools/renesas_spkgimage.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > Reviewed-by: Simon Glass
Re: [PATCH] doc: gpt: fix example of echoing variable
On Tue, 24 Oct 2023 at 05:00, Tom Fitzhenry wrote: > > --- > doc/usage/cmd/gpt.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Simon Glass It could use 'print' instead, I suppose. > diff --git a/doc/usage/cmd/gpt.rst b/doc/usage/cmd/gpt.rst > index f6115ecb0ee..ea43218fa21 100644 > --- a/doc/usage/cmd/gpt.rst > +++ b/doc/usage/cmd/gpt.rst > @@ -192,7 +192,7 @@ Get the information about the partition named 'rootfs':: > Get the list of partition names on the disk:: > > => gpt enumerate > -=> echo gpt_partition_list > +=> echo ${gpt_partition_list} > boot rootfs system-data [ext] user modules ramdisk > > > -- > 2.42.0 >
Re: quick question about TPM
Hi Niek, On Tue, 24 Oct 2023 at 04:51, niek.nooij...@omron.com wrote: > > Hi > > Just a quick question. I'm developing a platform using the > socfpga_cyclone5_defconfig > everything is working, linux boots, but we decided to add a TPM to it's SPI > bus. > For some reason the TPM support menu in the menuconfig is disabled and I > can't seem to find out why, or which file disables it. can you point me in > the right direction? The only thing 'config TPM' depends on is DM (driver model). Is that somehow disabled? Once you enable that, it should appear. Regards, Simon
Re: [PATCH v4 1/1] efi_loader: expose the device-tree file name
On Tue, Oct 24, 2023 at 08:20:32AM +0200, Heinrich Schuchardt wrote: > Forward and backward compatibility of Linux kernel device-trees is > sometimes missing. One solution approach is to load a kernel specific > device-tree. This can either be done via a U-Boot scripts (like the one > generated by Debian package flash-kernel or by a boot loader like GRUB. > The boot loader approach currently requires to know the device-tree name > before first boot which makes it unusable for generic images. > > Expose the device-tree file name as EFI variable FdtFile. > This will allow bootloaders to load a kernel specific device-tree. > > The variable will not be exposed on ACPI based systems or if the > environment variable fdtfile is not defined. > > Signed-off-by: Heinrich Schuchardt Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 1/1] sandbox: eliminate unused functions from binaries
On Tue, Oct 24, 2023 at 08:30:47AM +0200, Heinrich Schuchardt wrote: > The sandbox should closely mimic other architectures. > > Place each function or data in a separate section and let the linker > eliminate unused ones. This will reduce the binary size. > > In the linker script mark that u_boot_sandbox_getopt are to be kept. > > Signed-off-by: Heinrich Schuchardt Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 3/3] trace: Fix alignment logic in flyrecord header
On Tue, Oct 24, 2023 at 02:33:26PM +0200, Michal Simek wrote: > Hi Tom, > > On 9/25/23 16:33, Tom Rini wrote: > > On Mon, Sep 25, 2023 at 04:21:17PM +0200, Michal Simek wrote: > > > > > > > > > On 9/25/23 16:19, Tom Rini wrote: > > > > On Mon, Sep 25, 2023 at 04:10:38PM +0200, Michal Simek wrote: > > > > > Hi Simon, > > > > > > > > > > On 9/25/23 16:01, Simon Glass wrote: > > > > > > Hi Michal, > > > > > > > > > > > > On Mon, 25 Sept 2023 at 07:38, Michal Simek > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 9/25/23 15:10, Simon Glass wrote: > > > > > > > > Hi Michal, > > > > > > > > > > > > > > > > On Mon, 25 Sept 2023 at 00:06, Michal Simek > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > Hi Simon, > > > > > > > > > > > > > > > > > > > > > > > > > > > On 9/23/23 20:13, Simon Glass wrote: > > > > > > > > > > Current alignment which is using 16 bytes is not correct in > > > > > > > > > > connection to > > > > > > > > > > trace_clocks description and it's length. > > > > > > > > > > That's why use start_addr variable and record proper size > > > > > > > > > > based on used > > > > > > > > > > entries. > > > > > > > > > > > > > > > > > > > > Fixes: be16fc81b2ed ("trace: Update proftool to use new > > > > > > > > > > binary format"). > > > > > > > > > > Signed-off-by: Michal Simek > > > > > > > > > > Reviewed-by: Simon Glass > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > > - s/start_addr/start_ofs/g' > > > > > > > > > > > > > > > > > > > > tools/proftool.c | 31 +-- > > > > > > > > > > 1 file changed, 29 insertions(+), 2 deletions(-) > > > > > > > > > > > > > > > > > > > > Applied to u-boot-dm, thanks! > > > > > > > > > > > > > > > > > > FYI: I have merged it to my tree and already sent pull > > > > > > > > > request to Tom. > > > > > > > > > Without it I couldn't pass CI loop to get all reviewed > > > > > > > > > features in. > > > > > > > > > > > > > > > > > > https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c...@amd.com/ > > > > > > > > > > > > > > > > Ah OK, well that's fine. It was in my patchwork queue still, > > > > > > > > which > > > > > > > > suggests that the patches were not set to 'applied'? > > > > > > > > > > > > > > I am not using patchwork. But I expect my reply to cover letter > > > > > > > was recorded there. > > > > > > > > > > > > Probably. If you reply to each patch, it shows up in the patch, but > > > > > > the cover letter is hidden somewhere else. > > > > > > > > > > I have never started to like patchwork. I installed that client long > > > > > time > > > > > ago, I also have account for quite a long time. > > > > > > > > > > > If you are not using patchwork, how come you are a custodian? Is > > > > > > someone else dealing with patchwork for you? > > > > > > > > > > Not really. I am just keep track on it via emails. > > > > > > > > > > DT folks did wire CI loop on every patch which they get. I am not > > > > > aware > > > > > about any feature like this which would bring me something. That's > > > > > why I am > > > > > considering patchwork as unneeded layer. And I also don't think that > > > > > I have > > > > > read anywhere that all custodians should be using patchwork. > > > > > > > > Right, patchwork isn't required, but can be helpful. Part of how > > > > patchwork is maintained for everyone (in U-Boot) is that I have a script > > > > that will update the status of patches to accepted and add the githash, > > > > based on the "patchwork hash" of a given commit. There's a number of > > > > automated tooling things that other projects use which could be helpful > > > > here, but due to lack of time/resources, we haven't tried them here. > > > > > > Can you share that script? I am happy to run it and pretty much close my > > > list. > > > I am using b4 for applying patches that's why all message-ids are listed > > > in > > > the history which will uniquely identify that patches. > > > > If you like, yes, you can run the following. Note that when I run it > > myself between tags, it will still re-update things. This requires > > having patchwork cloned from git as well and is a slight modification of > > both tools/patchwork-update-commits and tools/post-receive.hook: > > > > #!/bin/bash > > > > # Patchwork - automated patch tracking system > > # Copyright (C) 2010 martin f. krafft > > # > > # SPDX-License-Identifier: GPL-2.0-or-later > > > > # Git post-receive hook to update Patchwork patches after Git pushes > > set -u > > > > PW_DIR=/home/trini/work/u-boot/patchwork/patchwork > > > > #TODO: the state map should really live in the repo's git-config > > STATE_MAP=".git/refs/heads/master:Accepted" > > > > # ignore all commits already present in these refs > > # e.g., > > # EXCLUDE="refs/heads/upstream refs/heads/other-project" > > EXCLUDE="" > > > > do_exit=0 > > trap "do_exit=1" INT
Re: Pull request: u-boot-rockchip-20231024
On Tue, Oct 24, 2023 at 05:12:23PM +0800, Kever Yang wrote: > Hi Tom, > > Please pull the updates for rockchip platform: > - Add Board: rk3588 NanoPC-T6, Orange Pi 5, Orange Pi 5 Plus; > - clk driver fix for rk3568 and rk3588; > - rkmtd cmd support for rockchip nand device; > - dts update and sync from linux; > > CI: > https://source.denx.de/u-boot/custodians/u-boot-rockchip/-/pipelines/18275 > > Thanks, > - Kever > > The following changes since commit 9a3a58396b78b1f9d0c14580dc03f81d29207dd2: > > Merge https://source.denx.de/u-boot/custodians/u-boot-marvell (2023-10-20 > 12:54:33 -0400) > > are available in the Git repository at: > > https://source.denx.de/u-boot/custodians/u-boot-rockchip.git > tags/u-boot-rockchip-20231024 > > for you to fetch changes up to d039b1b551e8dbb2c309d35bb19b7866caa4dcc1: > > rockchip: configs: sandbox: enable rkmtd command (2023-10-24 15:55:17 +0800) > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: Pull request: please pull u-boot-imx-20231024
On Tue, Oct 24, 2023 at 10:20:57AM +0200, Stefano Babic wrote: > Hi Tom, > > please pull from u-boot-imx, thanks ! > > The following changes since commit e65b5d35c9116485366bb08138043d51220551da: > > Merge branch 'master' of > https://source.denx.de/u-boot/custodians/u-boot-sh (2023-10-17 09:15:56 > -0400) > > are available in the Git repository at: > > https://gitlab.denx.de/u-boot/custodians/u-boot-imx.git > tags/u-boot-imx-20231024 > > for you to fetch changes up to 2f96064d0cf78e21a668ad907d41d63e56f9f7bb: > > arm64: dts: imx8mp: Describe M24C32-D write-lockable page in DH i.MX8MP > DHCOM DT (2023-10-18 21:29:59 +0200) > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] doc: usage: fix ordering of shell commands
On Tue, 24 Oct 2023 at 05:03, Tom Fitzhenry wrote: > > I initially didn't find the bootz docs when I went looking for them. :) > --- > doc/usage/index.rst | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v3 1/1] sandbox: eliminate unused functions from binaries
Hi Heinrich, On Mon, 23 Oct 2023 at 23:31, Heinrich Schuchardt wrote: > > The sandbox should closely mimic other architectures. > > Place each function or data in a separate section and let the linker > eliminate unused ones. This will reduce the binary size. > > In the linker script mark that u_boot_sandbox_getopt are to be kept. > > Signed-off-by: Heinrich Schuchardt > --- > v3: > avoid duplicate -Wl,--gc-sections > only use KEEP for unreferenced symbols > v2: > mark _u_boot_sandbox_getopt sections as KEEP() > --- > arch/sandbox/config.mk | 4 ++-- > arch/sandbox/cpu/u-boot.lds | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) I would still rather not do this, I am OK with trying it so long as we can revert it if it causes problems. > > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk > index 2d184c5f65..1d50991f8d 100644 > --- a/arch/sandbox/config.mk > +++ b/arch/sandbox/config.mk > @@ -2,7 +2,7 @@ > # Copyright (c) 2011 The Chromium OS Authors. > > PLATFORM_CPPFLAGS += -D__SANDBOX__ -U_FORTIFY_SOURCE > -PLATFORM_CPPFLAGS += -fPIC > +PLATFORM_CPPFLAGS += -fPIC -ffunction-sections -fdata-sections > PLATFORM_LIBS += -lrt > SDL_CONFIG ?= sdl2-config > > @@ -30,7 +30,7 @@ cmd_u-boot__ = $(CC) -o $@ -Wl,-T u-boot.lds $(u-boot-init) > \ > $(u-boot-main) \ > $(u-boot-keep-syms-lto) \ > -Wl,--no-whole-archive \ > - $(PLATFORM_LIBS) -Wl,-Map -Wl,u-boot.map > + $(PLATFORM_LIBS) -Wl,-Map -Wl,u-boot.map -Wl,--gc-sections > > cmd_u-boot-spl = (cd $(obj) && $(CC) -o $(SPL_BIN) -Wl,-T u-boot-spl.lds \ > $(KBUILD_LDFLAGS:%=-Wl,%) \ > diff --git a/arch/sandbox/cpu/u-boot.lds b/arch/sandbox/cpu/u-boot.lds > index ba8dee50c7..52f13af374 100644 > --- a/arch/sandbox/cpu/u-boot.lds > +++ b/arch/sandbox/cpu/u-boot.lds > @@ -15,7 +15,7 @@ SECTIONS > > _u_boot_sandbox_getopt : { > *(_u_boot_sandbox_getopt_start) > - *(_u_boot_sandbox_getopt) > + KEEP(*(_u_boot_sandbox_getopt)) > *(_u_boot_sandbox_getopt_end) > } > > -- > 2.40.1 > Regards, Simon
Re: [PATCH v4 1/1] efi_loader: expose the device-tree file name
Hi Heinrich, On Mon, 23 Oct 2023 at 23:20, Heinrich Schuchardt wrote: > > Forward and backward compatibility of Linux kernel device-trees is > sometimes missing. One solution approach is to load a kernel specific > device-tree. This can either be done via a U-Boot scripts (like the one > generated by Debian package flash-kernel or by a boot loader like GRUB. > The boot loader approach currently requires to know the device-tree name > before first boot which makes it unusable for generic images. > > Expose the device-tree file name as EFI variable FdtFile. > This will allow bootloaders to load a kernel specific device-tree. kernel-specific > > The variable will not be exposed on ACPI based systems or if the > environment variable fdtfile is not defined. > > Signed-off-by: Heinrich Schuchardt > --- > v4: > Generalize the description of the content of $fdtfile. > v3: > Add documentation > v2: > Use a unique GUID to enable future U-Boot independent > standardization. > Do not try to add the variable on ACPI based systems. > --- > doc/develop/uefi/uefi.rst | 14 ++ > include/efi_loader.h | 5 + > lib/efi_loader/efi_setup.c | 30 ++ > 3 files changed, 49 insertions(+) > > diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst > index fb16ac743a..702c490831 100644 > --- a/doc/develop/uefi/uefi.rst > +++ b/doc/develop/uefi/uefi.rst > @@ -916,6 +916,20 @@ So our final format of the FilePathList[] is:: > > Loaded image - end node (0xff) - VenMedia - initrd_1 - [end node (0x01) > - initrd_n ...] - end node (0xff) > > +EFI variable FdtFile > + > + > +Ideally U-Boot would always expose a device-tree that can be used for booting > +any operating systems. Unfortunately operating systems like Linux sometimes > +break forward and backward compatibility. In this case there is a need to > load > +an operating system version specific device-tree. This seems to be a strong statement. Given the effort that goes into the DT, changes are supposed to be backwards-compatible. Is this generally true, or is it just that we want an up-to-date DT for the kernel to enable new features? > + > +U-Boot has an environment variable fdtfile identifying the device-tree file > to > +load. The content of this variable is exposed as EFI variable Fdtfile, vendor > +GUID d45dde69-3bd6-40e0-90d5-6b606aa57730. It contains the device-tree path > +name as a NUL terminated ASCII string. On many architectures the file name is NUL-terminated > +preceded by a vendor directory ('vendor-directory/board-name.dtb'). > + > Links > - > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index e24410505f..146e7f1bce 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -152,6 +152,11 @@ static inline efi_status_t efi_launch_capsules(void) > EFI_GUID(0x8108ac4e, 0x9f11, 0x4d59, \ > 0x85, 0x0e, 0xe2, 0x1a, 0x52, 0x2c, 0x59, 0xb2) > > +/* Vendor GUID for the FdtFile variable */ > +#define VENDOR_FDTFILE_GUID \ > + EFI_GUID(0xd45dde69, 0x3bd6, 0x40e0, \ > +0x90, 0xd5, 0x6b, 0x60, 0x6a, 0xa5, 0x77, 0x30) > + > /* Use internal device tree when starting UEFI application */ > #define EFI_FDT_USE_INTERNAL NULL > > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c > index e6de685e87..71bcde645b 100644 > --- a/lib/efi_loader/efi_setup.c > +++ b/lib/efi_loader/efi_setup.c > @@ -17,6 +17,8 @@ > > efi_status_t efi_obj_list_initialized = OBJ_LIST_NOT_INITIALIZED; > > +efi_guid_t vendor_fdtfile_guid = VENDOR_FDTFILE_GUID; > + > /* > * Allow unaligned memory access. > * > @@ -26,6 +28,27 @@ void __weak allow_unaligned(void) > { > } > > +/** > + * efi_init_fdtfile() - set EFI variable FdtFile > + * > + * Return: status code > + */ > +static efi_status_t efi_init_fdtfile(void) > +{ > + char *val; > + > + val = env_get("fdtfile"); > + if (!val) > + return EFI_SUCCESS; > + > + return efi_set_variable_int(u"FdtFile", > + _fdtfile_guid, > + EFI_VARIABLE_BOOTSERVICE_ACCESS | > + EFI_VARIABLE_RUNTIME_ACCESS | > + EFI_VARIABLE_READ_ONLY, > + strlen(val) + 1, val, false); > +} > + > /** > * efi_init_platform_lang() - define supported languages > * > @@ -250,6 +273,13 @@ efi_status_t efi_init_obj_list(void) > if (ret != EFI_SUCCESS) > goto out; > > + /* Define EFI variable FdtFile */ > + if (!CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)) { > + ret = efi_init_fdtfile(); > + if (ret != EFI_SUCCESS) > + goto out; > + } > + > /* Indicate supported features */ > ret = efi_init_os_indications(); > if (ret != EFI_SUCCESS) > -- >
Re: [v4.1 2/2] CI, pytest: Add a test for sandbox without LTO
Hi Tom, On Mon, 23 Oct 2023 at 10:28, Tom Rini wrote: > > On Mon, Oct 23, 2023 at 10:13:52AM -0700, Simon Glass wrote: > > [snip] > > BTW buildman supports -L which disabled LTO using the NO_LTO=1 option > > I worry about putting sandbox-specific flags in buildman. Outside of > sandbox, targets that enable LTO require LTO, just like any other CONFIG > option. Some problems with LTO and why I don't normally develop with it enabled: - build time - code moves around all over the place so it is hard to compare size growth At least for my IDE flow, I use -L in most cases. Yes there are some boards which won't fit without LTO, but I don't see them much. So this is mostly a dev convenience / productivity tool. Regards, Simon
Re: [PATCH v3 22/32] efi: Update EFI_LOADER to depend on DM_ETH
Hi Tom, On Mon, 23 Oct 2023 at 10:45, Tom Rini wrote: > > On Mon, Oct 23, 2023 at 10:13:54AM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Mon, 23 Oct 2023 at 06:16, Tom Rini wrote: > > > > > > On Mon, Oct 23, 2023 at 12:05:28AM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Sun, 22 Oct 2023 at 16:45, Tom Rini wrote: > > > > > > > > > > On Sun, Oct 22, 2023 at 02:55:32PM -0700, Simon Glass wrote: > > > > > > Hi, > > > > > > > > > > > > On Sun, 22 Oct 2023 at 07:59, Tom Rini wrote: > > > > > > > > > > > > > > On Sun, Oct 22, 2023 at 10:29:22AM -0400, Tom Rini wrote: > > > > > > > > On Sun, Oct 22, 2023 at 08:08:11AM +0200, Heinrich Schuchardt > > > > > > > > wrote: > > > > > > > > > On 10/21/23 20:26, Tom Rini wrote: > > > > > > > > > > On Sat, Oct 21, 2023 at 08:43:08AM -0700, Simon Glass wrote: > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > > > On Thu, 19 Oct 2023 at 17:30, AKASHI Takahiro > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Oct 19, 2023 at 08:01:11AM -0600, Simon Glass > > > > > > > > > > > > wrote: > > > > > > > > > > > > > Hi Heinrich, > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, 18 Oct 2023 at 06:55, Heinrich Schuchardt > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 10/17/23 16:09, Tom Rini wrote: > > > > > > > > > > > > > > > On Mon, Oct 16, 2023 at 04:28:13PM -0600, Simon > > > > > > > > > > > > > > > Glass wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Since efi_device_path.c calls eth_get_dev() and > > > > > > > > > > > > > > > > assumes that Ethernet is > > > > > > > > > > > > > > > > available, add it as an explicit dependency. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass > > > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > (no changes since v2) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > > > > > > > > - Add new patch to update EFI_LOADER to depend > > > > > > > > > > > > > > > > on DM_ETH > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >lib/efi_loader/Kconfig | 1 + > > > > > > > > > > > > > > > >1 file changed, 1 insertion(+) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/lib/efi_loader/Kconfig > > > > > > > > > > > > > > > > b/lib/efi_loader/Kconfig > > > > > > > > > > > > > > > > index 13cad6342c36..fca4b3eef270 100644 > > > > > > > > > > > > > > > > --- a/lib/efi_loader/Kconfig > > > > > > > > > > > > > > > > +++ b/lib/efi_loader/Kconfig > > > > > > > > > > > > > > > > @@ -11,6 +11,7 @@ config EFI_LOADER > > > > > > > > > > > > > > > > # We need EFI_STUB_32BIT to be set on > > > > > > > > > > > > > > > > x86_32 with EFI_STUB > > > > > > > > > > > > > > > > depends on !EFI_STUB || !X86 || X86_64 || > > > > > > > > > > > > > > > > EFI_STUB_32BIT > > > > > > > > > > > > > > > > depends on BLK > > > > > > > > > > > > > > > > +depends on DM_ETH > > > > > > > > > > > > > > > > depends on !EFI_APP > > > > > > > > > > > > > > > > default y if !ARM || SYS_CPU = armv7 || > > > > > > > > > > > > > > > > SYS_CPU = armv8 > > > > > > > > > > > > > > > > select CHARSET > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Does this work for you Heinrich, or do you want > > > > > > > > > > > > > > > to clarify the > > > > > > > > > > > > > > > dependencies (and re-organize the code as needed) > > > > > > > > > > > > > > > around networking? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > We should be able to boot via EFI on devices > > > > > > > > > > > > > > without U-Boot network support. > > > > > > > > > > > > > > > > > > > > > > > > > > > > We already use IS_ENABLED(CONFIG_NETDEVICES) to > > > > > > > > > > > > > > avoid invoking > > > > > > > > > > > > > > eth_get_dev() if there is no network. > > > > > > > > > > > > > > CONFIG_NETDEVICES=y selects > > > > > > > > > > > > > > CONFIG_DM_ETH. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Why is this not sufficient? > > > > > > > > > > > > > > Is there a configuration that does not build? > > > > > > > > > > > > > > > > > > > > > > > > > > The point of this series is to disable CMDLINE and > > > > > > > > > > > > > fix up what breaks. > > > > > > > > > > > > > > > > > > > > > > > > > > In this case we have some sort of breakage...perhaps > > > > > > > > > > > > > Tom has already > > > > > > > > > > > > > found it, but otherwise could you take a look? > > > > > > > > > > > > > > > > > > > > > > > > > > We should be able to disable NET and LTO in sandbox > > > > > > > > > > > > > and still build. > > > > > > > > > > > > > But this fails at present[1]. You can try it on > > > > > > > > > > > > > -master > > > > > > > > > > > > > > > > > > > > > > > > Obviously, it would be
Re: [PATCH v2 1/1] sandbox: eliminate unused functions from binaries
Hi, On Mon, 23 Oct 2023 at 10:24, Tom Rini wrote: > > On Mon, Oct 23, 2023 at 10:13:55AM -0700, Simon Glass wrote: > > Hi, > > > > On Mon, 23 Oct 2023 at 10:03, Tom Rini wrote: > > > > > > On Mon, Oct 23, 2023 at 01:37:27AM +0200, Heinrich Schuchardt wrote: > > > > The sandbox should closely mimic other architectures. > > > > > > > > Place each function or data in a separate section and let the linker > > > > eliminate unused ones. This will reduce the binary size. > > > > > > > > In the linker script mark that u_boot_sandbox_getopt are to be kept. > > > > > > > > Signed-off-by: Heinrich Schuchardt > > > > --- > > > > v2: > > > > mark _u_boot_sandbox_getopt secitons a KEEP() > > > > --- > > > > arch/sandbox/config.mk | 4 +++- > > > > arch/sandbox/cpu/u-boot.lds | 6 +++--- > > > > 2 files changed, 6 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk > > > > index 2d184c5f65..c02f0229c2 100644 > > > > --- a/arch/sandbox/config.mk > > > > +++ b/arch/sandbox/config.mk > > > > @@ -2,7 +2,7 @@ > > > > # Copyright (c) 2011 The Chromium OS Authors. > > > > > > > > PLATFORM_CPPFLAGS += -D__SANDBOX__ -U_FORTIFY_SOURCE > > > > -PLATFORM_CPPFLAGS += -fPIC > > > > +PLATFORM_CPPFLAGS += -fPIC -ffunction-sections -fdata-sections > > > > PLATFORM_LIBS += -lrt > > > > SDL_CONFIG ?= sdl2-config > > > > > > > > @@ -26,6 +26,7 @@ cmd_u-boot__ = $(CC) -o $@ -Wl,-T u-boot.lds > > > > $(u-boot-init) \ > > > > $(KBUILD_LDFLAGS:%=-Wl,%) \ > > > > $(SANITIZERS) \ > > > > $(LTO_FINAL_LDFLAGS) \ > > > > + -Wl,--gc-section \ > > > > -Wl,--whole-archive \ > > > > $(u-boot-main) \ > > > > $(u-boot-keep-syms-lto) \ > > > > @@ -37,6 +38,7 @@ cmd_u-boot-spl = (cd $(obj) && $(CC) -o $(SPL_BIN) > > > > -Wl,-T u-boot-spl.lds \ > > > > $(SANITIZERS) \ > > > > $(LTO_FINAL_LDFLAGS) \ > > > > $(patsubst $(obj)/%,%,$(u-boot-spl-init)) \ > > > > + -Wl,--gc-section \ > > > > -Wl,--whole-archive \ > > > > $(patsubst $(obj)/%,%,$(u-boot-spl-main)) \ > > > > $(patsubst $(obj)/%,%,$(u-boot-spl-platdata)) \ > > > > > > There's already a call to --gc-sections for u-boot-spl on sandbox (in > > > the last line of the link command). We should probably move the main > > > u-boot call there too, for consistency. > > > > For the moment I really don't want this in sandbox. I think I explain > > that there is very little benefit in terms of size and it takes us > > away from how native apps are normally built. It also seems to mask > > problems in your CMDLINE series > > This fixes problems with sandbox not acting like a proper U-Boot target. > It is not at all about size reduction, it's about being able to use > normal conventions in U-Boot. I've said everything I can say here. I would prefer not to go down this path, but we can always try it and see what happens. Regards, Simon
[PATCH v2] arm: init: add an option to use FDT from previous bootloader
Add a new config option to allow u-boot to reuse the FDT provided by the previous stage bootloader when available. On some boards the previous stage bootloader can populate platform-specific parts of the devicetree such as the memory node, this allows us to avoid hardcoding it in u-boot and instead determine it dynamically at runtime. Signed-off-by: Caleb Connolly --- This patch will improve generic support for Qualcomm boards by enabling us to configure the memory map at runtime rather than having hardcoded maps on a per-device basis. I've gone for this approach initially to try and avoid introducing board specific code where possible, but I'm happy to rework this into mach-snapdragon if that's preferred. --- Changes in v2: - Replace pre-processor conditional with runtime IS_ENABLED() - Link to v1: https://lore.kernel.org/r/20231024-b4-prevbl-fdt-v1-1-541f71dba...@linaro.org --- base-commit: e65b5d35c9116485366bb08138043d51220551da // Caleb (they/them) --- arch/arm/lib/save_prev_bl_data.c | 7 +++ boot/Kconfig | 10 ++ include/init.h | 11 +++ lib/fdtdec.c | 8 +++- 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/arch/arm/lib/save_prev_bl_data.c b/arch/arm/lib/save_prev_bl_data.c index f7b23faf0d66..b0608502535e 100644 --- a/arch/arm/lib/save_prev_bl_data.c +++ b/arch/arm/lib/save_prev_bl_data.c @@ -45,6 +45,13 @@ bool is_addr_accessible(phys_addr_t addr) return false; } +phys_addr_t get_prev_bl_fdt_addr(void) +{ + if (!is_addr_accessible((phys_addr_t)reg0)) + return 0; + return reg0; +} + int save_prev_bl_data(void) { struct fdt_header *fdt_blob; diff --git a/boot/Kconfig b/boot/Kconfig index a01e6cb8aafe..c127ba254589 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -1599,6 +1599,16 @@ config SAVE_PREV_BL_INITRAMFS_START_ADDR If no initramfs was provided by previous bootloader, no env variables will be created. +config USE_PREV_BL_FDT + depends on SAVE_PREV_BL_FDT_ADDR && !OF_BOARD + bool "Use the FDT provided by the previous stage bootloader" + help + When u-boot is chain-loaded from a previous bootloader, enable this option + to use the FDT provided by the previous bootloader instead of any built-in + to u-boot. + + If no FDT was available, u-boot will fall back to its internal FDT. + menu "Configuration editor" config CEDIT diff --git a/include/init.h b/include/init.h index 4e7fe26c2004..ca78ff9f08c4 100644 --- a/include/init.h +++ b/include/init.h @@ -168,6 +168,17 @@ defined(CONFIG_SAVE_PREV_BL_FDT_ADDR) * Return: 0 if ok; -ENODATA on error */ int save_prev_bl_data(void); + +/** + * get_prev_bl_fdt_addr - When u-boot is chainloaded, get the address + * of the FDT passed by the previous bootloader. + * + * Return: the address of the FDT passed by the previous bootloader + * or 0 if not found. + */ +phys_addr_t get_prev_bl_fdt_addr(void); +#else +#define get_prev_bl_fdt_addr() 0LLU #endif /** diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 7a6916764835..b82baf08fc2f 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1222,7 +1222,7 @@ static int uncompress_blob(const void *src, ulong sz_src, void **dstp) */ static void *fdt_find_separate(void) { - void *fdt_blob = NULL; + void *fdt_blob = NULL, *prevbl_fdt_blob; if (IS_ENABLED(CONFIG_SANDBOX)) return NULL; @@ -1234,6 +1234,12 @@ static void *fdt_find_separate(void) else fdt_blob = (ulong *)__bss_end; #else + if (IS_ENABLED(CONFIG_USE_PREV_BL_FDT)) { + prevbl_fdt_blob = (void *)get_prev_bl_fdt_addr(); + if (prevbl_fdt_blob) + return prevbl_fdt_blob; + } + /* FDT is at end of image */ fdt_blob = (ulong *)_end;
Re: [PATCH v2 2/3] net: bootp: BOOTP/DHCPv4 retransmission improvements
On 2023-10-23 11:06 p.m., Heinrich Schuchardt wrote: On 10/24/23 02:21, seanedm...@linux.microsoft.com wrote: From: Sean Edmond This patch introduces 3 improvements to align with RFC 951: - retransmission backoff interval maximum is configurable - initial retranmission backoff interval is configurable - transaction ID is kept the same for each BOOTP/DHCPv4 request In applications where thousands of nodes are serviced by a single DHCP server, maximizing the retransmission backoff interval at 2 seconds (the current u-boot default) exerts high pressure on the DHCP server and network layer. RFC 951 “7.2. Client Retransmission Strategy” states that the retransmission backoff interval should maximize at 60 seconds. This %s/maximize at/be limited to/ patch allows the interval to be configurable using the environment variable "bootpretransmitperiodmax" If there is an RFC defining the value, why do we need an environment variable? The intention with this patch is to provide the option to satisfy RFC, while giving the option to preserve the existing behavior. The retransmission timing parameters in u-boot have been the same for the last ~10 years, so I'm guessing some have gotten used to this behavior (even though it's not correct). The initial retranmission backoff period defaults to 250ms, which is also too small for these scenarios with many clients. This patch makes the initial retransmission interval to be configurable using the environment variable "bootpretransmitperiodinit". Also, on a retransmission it is not expected for the transaction ID to change (only the 'secs' field should be updated). Let's save the transaction ID and use the same transaction ID for each BOOTP/DHCPv4 exchange. Signed-off-by: Sean Edmond --- net/bootp.c | 56 ++--- 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/net/bootp.c b/net/bootp.c index 6800290963..bab17a9ceb 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -42,6 +42,17 @@ */ #define TIMEOUT_MS ((3 + (CONFIG_NET_RETRY_COUNT * 5)) * 1000) +/* + * According to rfc951 : 7.2. Client Retransmission Strategy + * "After the 'average' backoff reaches about 60 seconds, it should be + * increased no further, but still randomized." + * + * U-Boot has saturated this backoff at 2 seconds for a long time. + * To modify, set the environment variable "bootpretransmitperiodmax" + */ +#define RETRANSMIT_PERIOD_MAX_MS 2000 This does not match RFC 951. Please, why shouldn't we use the standard value by default? You're right, better to use the standard value by default. +#define RETRANSMIT_PERIOD_INIT_MS 250 This constant should be described too. Will add a description. + #ifndef CFG_DHCP_MIN_EXT_LEN /* minimal length of extension list */ #define CFG_DHCP_MIN_EXT_LEN 64 #endif @@ -53,6 +64,7 @@ u32 bootp_ids[CFG_BOOTP_ID_CACHE_SIZE]; unsigned int bootp_num_ids; int bootp_try; +u32 bootp_id; ulong bootp_start; ulong bootp_timeout; char net_nis_domain[32] = {0,}; /* Our NIS domain */ @@ -60,6 +72,7 @@ char net_hostname[32] = {0,}; /* Our hostname */ char net_root_path[CONFIG_BOOTP_MAX_ROOT_PATH_LEN] = {0,}; /* Our bootpath */ static ulong time_taken_max; +static u32 retransmit_period_max_ms; #if defined(CONFIG_CMD_DHCP) static dhcp_state_t dhcp_state = INIT; @@ -414,8 +427,8 @@ static void bootp_timeout_handler(void) } } else { bootp_timeout *= 2; - if (bootp_timeout > 2000) - bootp_timeout = 2000; + if (bootp_timeout > retransmit_period_max_ms) + bootp_timeout = retransmit_period_max_ms; RFC 951 requires that the backoff time is randomized. I'll can look into adding randomization to this as well. Best regards Heinrich net_set_timeout_handler(bootp_timeout, bootp_timeout_handler); bootp_request(); } @@ -711,10 +724,14 @@ static int bootp_extended(u8 *e) void bootp_reset(void) { + char *ep; /* Environment pointer */ + bootp_num_ids = 0; bootp_try = 0; bootp_start = get_timer(0); - bootp_timeout = 250; + + bootp_timeout = env_get_ulong("bootpretransmitperiodinit", 10, RETRANSMIT_PERIOD_INIT_MS); + } void bootp_request(void) @@ -726,7 +743,6 @@ void bootp_request(void) #ifdef CONFIG_BOOTP_RANDOM_DELAY ulong rand_ms; #endif - u32 bootp_id; struct in_addr zero_ip; struct in_addr bcast_ip; char *ep; /* Environment pointer */ @@ -742,6 +758,9 @@ void bootp_request(void) else time_taken_max = TIMEOUT_MS; + retransmit_period_max_ms = env_get_ulong("bootpretransmitperiodmax", 10, + RETRANSMIT_PERIOD_MAX_MS); + #ifdef CONFIG_BOOTP_RANDOM_DELAY /* Random BOOTP delay */ if (bootp_try == 0) srand_mac(); @@ -801,18 +820,23 @@ void bootp_request(void) extlen = bootp_extended((u8
Re: [PATCH v4 3/3] dt-bindings: mtd: binman-partitions: Add alignment properties
On Mon, Oct 09, 2023 at 04:04:15PM -0600, Simon Glass wrote: > Add three properties for controlling alignment of partitions, aka > 'entries' in binman. > > For now there is no explicit mention of hierarchy, so a 'section' is > just the 'binman' node. > > These new properties are inputs to the packaging process, but are also > needed if the firmware is repacked, to ensure that alignment > constraints are not violated. Therefore they are provided as part of > the schema. > > Signed-off-by: Simon Glass > --- > > (no changes since v2) > > Changes in v2: > - Fix 'a' typo in commit message > > .../mtd/partitions/binman-partition.yaml | 39 +++ > 1 file changed, 39 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml > b/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml > index 35a320359ec1..8e8a3b6d4d14 100644 > --- a/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml > +++ b/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml > @@ -28,6 +28,42 @@ properties: >- const: u-boot # u-boot.bin from U-Boot project >- const: atf-bl31 # bl31.bin or bl31.elf from TF-A project > > + align: > +$ref: /schemas/types.yaml#/definitions/uint32 > +description: > + This sets the alignment of the entry. The entry offset is adjusted > + so that the entry starts on an aligned boundary within the containing > + section or image. For example ‘align = <16>’ means that the entry will > + start on a 16-byte boundary. This may mean that padding is added before Only your example defines that alignment is in bytes. > + the entry. The padding is part of the containing section but is not > + included in the entry, meaning that an empty space may be created > before > + the entry starts. Alignment should be a power of 2. If ‘align’ is not > + provided, no alignment is performed. Would be nice to have some constraints. Unfortunately, no way to say 'power of 2' in json-schema (we could add something possibly), so the only way is: enum: [ 2, 4, 8, 16, 32, 64, ... ] Kind of verbose if we add all 31 possibilities... Could also do this: minium: 2 maximum: 0x8000 multipleOf: 2 > + > + align-size: > +$ref: /schemas/types.yaml#/definitions/uint32 > +description: > + This sets the alignment of the entry size. For example, to ensure > + that the size of an entry is a multiple of 64 bytes, set this to 64. > + While this does not affect the contents of the entry within binman > + itself (the padding is performed only when its parent section is > + assembled), the end result is that the entry ends with the padding > + bytes, so may grow. If ‘align-size’ is not provided, no alignment is > + performed. > + > + align-end: > +$ref: /schemas/types.yaml#/definitions/uint32 > +description: > + This sets the alignment of the end of an entry with respect to the > + containing section. Some entries require that they end on an alignment > + boundary, regardless of where they start. This does not move the start > + of the entry, so the contents of the entry will still start at the > + beginning. But there may be padding at the end. While this does not > + affect the contents of the entry within binman itself (the padding is > + performed only when its parent section is assembled), the end result is > + that the entry ends with the padding bytes, so may grow. If ‘align-end’ > + is not provided, no alignment is performed. > + > additionalProperties: false > > examples: > @@ -40,10 +76,13 @@ examples: > partition@10 { > compatible = "u-boot"; > reg = <0x10 0xf0>; > +align-size = <0x1000>; > +align-end = <0x1>; > }; > > partition@20 { > compatible = "atf-bl31"; > reg = <0x20 0x10>; > +align = <0x4000>; > }; > }; > -- > 2.42.0.609.gbb76f46606-goog >
Re: [PATCH v4 2/3] dt-bindings: mtd: binman-partition: Add binman compatibles
On Mon, Oct 09, 2023 at 04:04:14PM -0600, Simon Glass wrote: > Add two compatible for binman entries, as a starting point for the > schema. > > Note that, after discussion on v2, we decided to keep the existing > meaning of label so as not to require changes to existing userspace > software when moving to use binman nodes to specify the firmware > layout. > > Signed-off-by: Simon Glass > --- > > Changes in v4: > - Correct selection of multiple compatible strings > > Changes in v3: > - Drop fixed-partitions from the example > - Use compatible instead of label > > Changes in v2: > - Use plain partition@xxx for the node name > > .../mtd/partitions/binman-partition.yaml | 49 +++ > 1 file changed, 49 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml > > diff --git > a/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml > b/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml > new file mode 100644 > index ..35a320359ec1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml > @@ -0,0 +1,49 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +# Copyright 2023 Google LLC > + > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mtd/partitions/binman-partition.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Binman partition > + > +maintainers: > + - Simon Glass > + > +select: false So this schema is never used. 'select: false' is only useful if something else if referencing the schema. > + > +description: | > + This corresponds to a binman 'entry'. It is a single partition which holds > + data of a defined type. > + > +allOf: > + - $ref: /schemas/mtd/partitions/partition.yaml# > + > +properties: > + compatible: > +oneOf: > + - const: binman,entry # generic binman entry 'binman' is not a vendor. You could add it if you think that's useful. Probably not with only 1 case... > + - items: > + - const: u-boot # u-boot.bin from U-Boot project > + - const: atf-bl31 # bl31.bin or bl31.elf from TF-A project Probably should use the new 'tfa' rather than old 'atf'. Is this the only binary for TFA? The naming seems inconsistent in that every image goes in (or can go in) a bl?? section. Why does TFA have it but u-boot doesn't? Perhaps BL?? is orthogonal to defining what is in each partition. Perhaps someone more familar with all this than I am can comment. Once you actually test this, you'll find you are specifying: compatible = "u-boot", "atf-bl31"; > + > +additionalProperties: false > + > +examples: > + - | > +partitions { > +compatible = "binman"; > +#address-cells = <1>; > +#size-cells = <1>; > + > +partition@10 { > +compatible = "u-boot"; > +reg = <0x10 0xf0>; > +}; > + > +partition@20 { > +compatible = "atf-bl31"; > +reg = <0x20 0x10>; > +}; > +}; > -- > 2.42.0.609.gbb76f46606-goog >