[PATCH 5/5] test: dm: add scmi command test

2023-10-24 Thread AKASHI Takahiro
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

2023-10-24 Thread AKASHI Takahiro
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

2023-10-24 Thread AKASHI Takahiro
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

2023-10-24 Thread AKASHI Takahiro
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

2023-10-24 Thread AKASHI Takahiro
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

2023-10-24 Thread AKASHI Takahiro
"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)

2023-10-24 Thread Simon Glass
+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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread niek.nooij...@omron.com
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)

2023-10-24 Thread target
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

2023-10-24 Thread Heinrich Schuchardt

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

2023-10-24 Thread AKASHI Takahiro
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

2023-10-24 Thread Heinrich Schuchardt



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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Heinrich Schuchardt



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

2023-10-24 Thread 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.

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

2023-10-24 Thread 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.

Regards,
Simon


Re: [PATCH] .gitignore: ignore misc include, simple-bin, and tools/generated build artifacts

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tony Dinh
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Igor Prusov
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

2023-10-24 Thread Igor Prusov
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

2023-10-24 Thread Igor Prusov
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Andre Przywara
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Andre Przywara
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

2023-10-24 Thread Heinrich Schuchardt
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Caleb Connolly
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

2023-10-24 Thread Caleb Connolly
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()

2023-10-24 Thread Caleb Connolly
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

2023-10-24 Thread Caleb Connolly
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

2023-10-24 Thread Caleb Connolly
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

2023-10-24 Thread Caleb Connolly
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

2023-10-24 Thread Caleb Connolly
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

2023-10-24 Thread Caleb Connolly
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

2023-10-24 Thread Caleb Connolly
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Caleb Connolly



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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Ilias Apalodimas
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Ilias Apalodimas
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Simon Glass
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()

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Caleb Connolly



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

2023-10-24 Thread Tom Rini
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]

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Simon Glass
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()

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Tom Rini
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Simon Glass
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

2023-10-24 Thread Caleb Connolly
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

2023-10-24 Thread Sean Edmond



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

2023-10-24 Thread Rob Herring
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

2023-10-24 Thread Rob Herring
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
> 


  1   2   >