Re: [PATCH v2 1/1] efi_loader: typo mstching

[Ilias Apalodimas]

On Thu, 18 Apr 2024 at 05:32, Heinrich Schuchardt
 wrote:
>
> %s/mstching/matching/
>
> Reported-by: E Shattow 
> Signed-off-by: Heinrich Schuchardt 
> ---
> v2:
> correct Reported-by line
> ---
>  lib/efi_loader/efi_var_common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
> index aa8feffd3ec..ed53bcf3939 100644
> --- a/lib/efi_loader/efi_var_common.c
> +++ b/lib/efi_loader/efi_var_common.c
> @@ -419,7 +419,7 @@ void *efi_get_var(const u16 *name, const efi_guid_t 
> *vendor, efi_uintn_t *size)
>  }
>
>  /**
> - * efi_var_collect() - Copy EFI variables mstching attributes mask
> + * efi_var_collect() - Copy EFI variables matching attributes mask
>   *
>   * @bufp:  buffer containing variable collection
>   * @lenp:  buffer length
> --
> 2.43.0
>

Reviewed-by: Ilias Apalodimas 

Re: [PATCH 1/1] sandbox: don't call os_close with invalid file descriptor

[Tom Rini]

On Wed, 10 Apr 2024 23:50:34 +0200, Heinrich Schuchardt wrote:

> If open() fails it returns -1. Calling close() with this value
> makes no sense. Return -EIO instead.
> 
> 

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH] arm: mach-k3: security: Lower verbosity of cert message for GP

[Tom Rini]

On Wed, 10 Apr 2024 13:38:34 -0500, Andrew Davis wrote:

> When we find a certificate on an image to be booted on a GP device we
> print out a message explaining that the certificate is being skipped.
> This message is rather long and is printed for every image. Shorten
> the message and make the long version into a debug message.
> 
> 

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH] configs: am64x_evm_*_defconfig: Increase offsets for eMMC raw boot

[Tom Rini]

On Wed, 10 Apr 2024 13:20:16 -0500, Judith Mendez wrote:

> EMMC boot can fail due to the size of R5 SPL image growing beyond the
> 500KB of memory allocated in eMMC. Update offsets for eMMMC raw boot
> to load each binary from the correct address in eMMC according to the
> following eMMC layout:
> 
> boot0/1 partition
> 0x0+--+
>| tiboot3.bin (1 MB)   |
>   0x800+--+
>|   tispl.bin (2 MB)   |
> 0x1800+---+
>|   u-boot.img (4 MB)  |
> 0x3800+---+
>|  environment (128 KB)|
> 0x3900+---+
> 
> [...]

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH v1] configs: arbel: Use generic timer and npcm reset driver

[Tom Rini]

On Wed, 10 Apr 2024 13:54:37 +0800, Jim Liu wrote:

> Modify defconfig to use generic timer and npcm reset driver
> 
> 

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH 1/1] fs: fat: fill creation and change date

[Tom Rini]

On Tue, 09 Apr 2024 22:06:07 +0200, Heinrich Schuchardt wrote:

> The FAT specification requires that the change date is set.
> 
> If a DM RTC device exists, set the creation and change date to the current
> date when updating the directory entry. Otherwise use the date 2020-01-01.
> 
> 

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH 1/1] fs: fat: convert change month correctly

[Tom Rini]

On Tue, 09 Apr 2024 20:04:56 +0200, Heinrich Schuchardt wrote:

> The month is stored in 5 - 8. We need to shift it by 5 bits.
> 
> Cf. Microsoft FAT Specification, 2005-08-30
> 
> 

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH v4] tools: copyfile: use 64k instead of 512 buffer

[Tom Rini]

On Tue, 09 Apr 2024 14:14:34 +0200, Ahelenia Ziemiańska wrote:

> This is a trivial but significant optimization:
> mkimage took >200ms (and 49489 writes (of which 49456 512)),
> now it takes  110ms (and   419 writes (of which   386 64k)).
> 
> sendfile is much more appropriate for this and is done in one syscall,
> but doesn't bring any significant speedups over 64k r/w
> at the 13M size ranges, so there's no need to introduce
>   #if __linux__
>   while((size = sendfile(fd_dst, fd_src, NULL, 128 * 1024 * 1024)) > 0)
>   ;
>   if(size != -1) {
>   ret = 0;
>   goto out;
>   }
>   #endif
> 
> [...]

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH] Makefile.lib: find capsule ESL dtsi file with CONFIG_OF_UPSTREAM

[Tom Rini]

On Mon, 08 Apr 2024 16:28:43 -0500, Jonathan Humphreys wrote:

> When CONFIG_OF_UPSTREAM is enabled, DTS files are in SOC subdirectories (vs 
> the
> top level dts directory), but when CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled,
> the dynamically created dtsi file containing the capsule ESL DT node is in the
> parent directory. This results in a build failure because the #include 
> inserted
> in the DTS file is local to the current directory.  Update Makefile to have 
> the
> DT preprocessing of #includes search in the parent (dts top level) directory
> too.
> 
> [...]

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH 1/1] test: typo curren

[Tom Rini]

On Tue, 09 Apr 2024 09:55:49 +0200, Heinrich Schuchardt wrote:

> Fix typos in test_eficonfig.py: %s/curren/current/
> 
> 

Applied to u-boot/master, thanks!

-- 
Tom

[PATCH 1/1] acpi: set creator_revision in acpi_fill_header

[Heinrich Schuchardt]

We should have a single place where we write the default value to the
creator revision field. If we ever will have any table created by another
tool, we can overwrite the value afterwards.

Signed-off-by: Heinrich Schuchardt 
---
 arch/x86/lib/acpi_table.c | 2 --
 lib/acpi/acpi_table.c | 2 +-
 lib/acpi/ssdt.c   | 1 -
 test/dm/acpi.c| 3 +--
 4 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
index a5683132b01..06997948880 100644
--- a/arch/x86/lib/acpi_table.c
+++ b/arch/x86/lib/acpi_table.c
@@ -479,7 +479,6 @@ static int acpi_create_hpet(struct acpi_hpet *hpet)
/* Fill out header fields. */
acpi_fill_header(header, "HPET");
 
-   header->creator_revision = ASL_REVISION;
header->length = sizeof(struct acpi_hpet);
header->revision = acpi_get_table_revision(ACPITAB_HPET);
 
@@ -570,7 +569,6 @@ void acpi_fadt_common(struct acpi_fadt *fadt, struct 
acpi_facs *facs,
memcpy(header->oem_id, OEM_ID, 6);
memcpy(header->oem_table_id, OEM_TABLE_ID, 8);
memcpy(header->creator_id, ASLC_ID, 4);
-   header->creator_revision = 1;
 
fadt->x_firmware_ctrl = map_to_sysmem(facs);
fadt->x_dsdt = map_to_sysmem(dsdt);
diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
index c16ead6a6ec..6dbfdb22dec 100644
--- a/lib/acpi/acpi_table.c
+++ b/lib/acpi/acpi_table.c
@@ -117,6 +117,7 @@ void acpi_fill_header(struct acpi_table_header *header, 
char *signature)
memcpy(header->oem_table_id, OEM_TABLE_ID, 8);
header->oem_revision = OEM_REVISION;
memcpy(header->creator_id, ASLC_ID, 4);
+   header->creator_revision = ASL_REVISION;
 }
 
 void acpi_align(struct acpi_ctx *ctx)
@@ -219,7 +220,6 @@ void acpi_create_dbg2(struct acpi_dbg2_header *dbg2,
 
header->revision = acpi_get_table_revision(ACPITAB_DBG2);
acpi_fill_header(header, "DBG2");
-   header->creator_revision = ASL_REVISION;
 
/* One debug device defined */
dbg2->devices_offset = sizeof(struct acpi_dbg2_header);
diff --git a/lib/acpi/ssdt.c b/lib/acpi/ssdt.c
index e032e266b3c..df1d739d117 100644
--- a/lib/acpi/ssdt.c
+++ b/lib/acpi/ssdt.c
@@ -23,7 +23,6 @@ int acpi_write_ssdt(struct acpi_ctx *ctx, const struct 
acpi_writer *entry)
 
acpi_fill_header(ssdt, "SSDT");
ssdt->revision = acpi_get_table_revision(ACPITAB_SSDT);
-   ssdt->creator_revision = 1;
ssdt->length = sizeof(struct acpi_table_header);
 
acpi_inc(ctx, sizeof(struct acpi_table_header));
diff --git a/test/dm/acpi.c b/test/dm/acpi.c
index f14b3962f84..48cf3af1f8f 100644
--- a/test/dm/acpi.c
+++ b/test/dm/acpi.c
@@ -237,7 +237,6 @@ static int dm_test_acpi_fill_header(struct unit_test_state 
*uts)
hdr.length = 0x11;
hdr.revision = 0x22;
hdr.checksum = 0x33;
-   hdr.creator_revision = 0x44;
acpi_fill_header(, "ABCD");
 
ut_asserteq_mem("ABCD", hdr.signature, sizeof(hdr.signature));
@@ -249,7 +248,7 @@ static int dm_test_acpi_fill_header(struct unit_test_state 
*uts)
sizeof(hdr.oem_table_id));
ut_asserteq(OEM_REVISION, hdr.oem_revision);
ut_asserteq_mem(ASLC_ID, hdr.creator_id, sizeof(hdr.creator_id));
-   ut_asserteq(0x44, hdr.creator_revision);
+   ut_asserteq(ASL_REVISION, hdr.creator_revision);
 
return 0;
 }
-- 
2.43.0

Re: [PATCH 2/3] pci: dw_imx: add support for IMX8MM

[Marek Vasut]

On 4/17/24 10:09 PM, Tim Harvey wrote:

Add support for the IMX8MM SoC by adding driver data with the compatible
string of the GPR controller.

Signed-off-by: Tim Harvey 
---
  drivers/pci/pcie_dw_imx.c | 20 ++--
  1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pcie_dw_imx.c b/drivers/pci/pcie_dw_imx.c
index a2ee228224b5..10d926c30645 100644
--- a/drivers/pci/pcie_dw_imx.c
+++ b/drivers/pci/pcie_dw_imx.c
@@ -45,6 +45,10 @@
  #define IMX8M_GPR_PCIE_CLK_REQ_OVERRIDE_ENBIT(10)
  #define IMX8M_GPR_PCIE_CLK_REQ_OVERRIDE   BIT(11)
  
+struct pcie_chip_info {

+   const char *gpr;
+};
+
  struct pcie_dw_imx {
/* Must be first member of the struct */
struct pcie_dw  dw;
@@ -54,6 +58,15 @@ struct pcie_dw_imx {
struct reset_ctlapps_reset;
struct phy  phy;
struct udevice  *vpcie;
+   struct pcie_chip_info   *info;
+};
+
+static const struct pcie_chip_info imx8mm_chip_info = {
+   .gpr = "fsl,imx8mm-iomuxc-gpr",
+};
+
+static const struct pcie_chip_info imx8mp_chip_info = {
+   .gpr = "fsl,imx8mp-iomuxc-gpr",
  };
  
  static void pcie_dw_configure(struct pcie_dw_imx *priv, u32 cap_speed)

@@ -246,6 +259,8 @@ static int pcie_dw_imx_of_to_plat(struct udevice *dev)
ofnode gpr;
int ret;
  
+	priv->info = (void *)dev_get_driver_data(dev);

+


Does this really have to be cached in priv ?

The priv->info seems used only in this function.

[...]

[PATCH v2 1/1] efi_loader: typo mstching

[Heinrich Schuchardt]

%s/mstching/matching/

Reported-by: E Shattow 
Signed-off-by: Heinrich Schuchardt 
---
v2:
correct Reported-by line
---
 lib/efi_loader/efi_var_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index aa8feffd3ec..ed53bcf3939 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -419,7 +419,7 @@ void *efi_get_var(const u16 *name, const efi_guid_t 
*vendor, efi_uintn_t *size)
 }
 
 /**
- * efi_var_collect() - Copy EFI variables mstching attributes mask
+ * efi_var_collect() - Copy EFI variables matching attributes mask
  *
  * @bufp:  buffer containing variable collection
  * @lenp:  buffer length
-- 
2.43.0

[PATCH 1/1] efi_loader: typo mstching

[Heinrich Schuchardt]

%s/mstching/matching/

Reported-by: Reported-by: E Shattow 
Signed-off-by: Heinrich Schuchardt 
---
 lib/efi_loader/efi_var_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index aa8feffd3ec..ed53bcf3939 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -419,7 +419,7 @@ void *efi_get_var(const u16 *name, const efi_guid_t 
*vendor, efi_uintn_t *size)
 }
 
 /**
- * efi_var_collect() - Copy EFI variables mstching attributes mask
+ * efi_var_collect() - Copy EFI variables matching attributes mask
  *
  * @bufp:  buffer containing variable collection
  * @lenp:  buffer length
-- 
2.43.0

Re: [PATCH v3 0/2] upstream DT compatibility

[Tom Rini]

On Tue, Apr 09, 2024 at 04:49:23PM +0200, Caleb Connolly wrote:

> This is a subset of [1]. With more platform maintainers switching to
> OF_UPSTREAM I didn't want to get in the way (it has also proven more
> difficult than I hoped to remove only the fully compatible header
> files).
> 
> This series removes only the dt-bindings headers which contain generic
> data like GPIO flags, input event codes, etc.
> 
> It then implements support for building all DTBs for a vendor with
> OF_UPSTREAM_BUILD_VENDOR. This removes the need to maintain a set list
> of DTBs that can be built by U-Boot and opens up the possibility of new
> boards becoming supported "by default" just by landing their DT
> upstream.
> 
> [1]: 
> https://lore.kernel.org/u-boot/20240321-b4-upstream-dt-headers-v2-0-1eac0df87...@linaro.org

Currently this breaks a number of imx6 platforms starting with
aristainetos2c.

-- 
Tom


signature.asc
Description: PGP signature

Re: [RESEND PATCH v3 5/5] power: regulator: tps65941: Add TPS65224 PMIC regulator support

[Jaehoon Chung]

On 3/18/24 18:49, Bhargav Raviprakash wrote:
> Reuse TPS65941 regulator driver to adds support for
> TPS65224 PMIC's regulators. 4 BUCKs and 3 LDOs, where
> BUCK1 and BUCK2 can be configured in dual phase mode.
> 
> Signed-off-by: Bhargav Raviprakash 
> ---
>  drivers/power/regulator/tps65941_regulator.c | 283 ++-
>  1 file changed, 270 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/power/regulator/tps65941_regulator.c 
> b/drivers/power/regulator/tps65941_regulator.c
> index d879c2301b..b1b9462fd3 100644
> --- a/drivers/power/regulator/tps65941_regulator.c
> +++ b/drivers/power/regulator/tps65941_regulator.c
> @@ -37,6 +37,8 @@
>  
>  #define TPS65941_BUCK_CONV_OPS_IDX  0
>  #define TPS65941_LDO_CONV_OPS_IDX   0
> +#define TPS65224_LDO_CONV_OPS_IDX   1
> +#define TPS65224_BUCK_CONV_OPS_IDX  1
>  
>  struct tps65941_reg_conv_ops {
>   int volt_mask;
> @@ -55,6 +57,11 @@ static const char tps65941_ldo_ctrl[TPS65941_BUCK_NUM] = 
> {0x1D, 0x1E, 0x1F,
>  static const char tps65941_ldo_vout[TPS65941_BUCK_NUM] = {0x23, 0x24, 0x25,
>   0x26};
>  
> +static inline int tps65941_get_chip_id(struct udevice *dev)
> +{
> + return dev->parent->driver_data;
> +}
> +
>  static int tps65941_buck_enable(struct udevice *dev, int op, bool *enable)
>  {
>   int ret;
> @@ -146,6 +153,112 @@ int tps65941_lookup_slew(int id)
>   }
>  }
>  
> +static int tps65224_buck_volt2val(int idx, int uV)
> +{
> + /* This functions maps a value which is in micro Volts to the VSET 
> value.
> +  * The mapping is as per the datasheet of TPS65224.
> +  */
> +
> + if (uV > TPS65224_BUCK_VOLT_MAX)
> + return -EINVAL;
> +
> + if (idx > 0) {
> + /* Buck2, Buck3 and Buck4 of TPS65224 has a different schema in
> +  * converting b/w micro_volt and VSET hex values
> +  *
> +  * VSET value starts from 0x00 for 0.5V, and for every increment
> +  * in VSET value the output voltage increases by 25mV. This is 
> upto
> +  * 1.15V where VSET is 0x1A.
> +  *
> +  * For 0x1B the output voltage is 1.2V, and for every increment 
> of
> +  * VSET the output voltage increases by 50mV upto the max 
> voltage of
> +  * 3.3V
> +  *
> +  * | Voltage Ranges  | VSET Ranges  | Voltage Step |
> +  * +-+--+--+
> +  * | 0.5V to 1.50V   | 0x00 to 0x1A |  25mV|
> +  * | 1.2V to 3.3V| 0x1B to 0x45 |  50mV|
> +  */
> + if (uV >= 120)
> + return (uV - 120) / 5 + 0x1B;
> + else if (uV >= 50)
> + return (uV - 50) / 25000;
> + else
> + return -EINVAL;
> + }
> +
> + /* Buck1 and Buck12(dual phase) has a different mapping b/w output
> +  * voltage and VSET value.
> +  *
> +  * | Voltage Ranges  | VSET Ranges  | Voltage Step |
> +  * +-+--+--+
> +  * | 0.5V to 0.58V   | 0xA to 0xE   |  20mV|
> +  * | 0.6V to 1.095V  | 0xF to 0x72  |  5mV |
> +  * | 1.1V to 1.65V   | 0x73 to 0xAA |  10mV|
> +  * | 1.6V to 3.3V| 0xAB to 0xFD |  20mV|
> +  *
> +  */
> + if (uV >= 166)
> + return (uV - 166) / 2 + 0xAB;
> + else if (uV >= 110)
> + return (uV - 110) / 1 + 0x73;
> + else if (uV >= 60)
> + return (uV - 60) / 5000 + 0x0F;
> + else if (uV >= 50)
> + return (uV - 50) / 2 + 0x0A;
> + else
> + return -EINVAL;
> +}
> +
> +static int tps65224_buck_val2volt(int idx, int val)
> +{
> + /* This function does the opposite to the tps65224_buck_volt2val 
> function
> +  * described above.
> +  * This maps the VSET value to micro volts. Please refer to the ranges
> +  * mentioned the comments of tps65224_buck_volt2val.
> +  */
> +
> + if (idx > 0) {
> + if (val > TPS65224_BUCK234_VOLT_MAX_HEX)
> + return -EINVAL;
> + else if (val >= 0x1B)
> + return 120 + (val - 0x1B) * 5;
> + else if (val >= 0x00)
> + return 50 + (val - 0x00) * 25000;
> + else
> + return -EINVAL;
> + }
> +
> + if (val > TPS65224_BUCK1_VOLT_MAX_HEX)
> + return -EINVAL;
> + else if (val >= 0xAB)
> + return 166 + (val - 0xAB) * 2;
> + else if (val >= 0x73)
> + return 110 + (val - 0x73) * 1;
> + else if (val >= 0xF)
> + return 60 + (val - 0xF) * 5000;
> + else if (val >= 0xA)
> + return 50 + (val - 0xA) * 2;
> + else
> + 

Re: [RESEND PATCH v3 3/5] power: regulator: tps65941: Added macros for BUCK ID

[Jaehoon Chung]

On 3/18/24 18:49, Bhargav Raviprakash wrote:
> Adds macros for buck and ldo ids and switched to using switch
> case instead of if else in probe functions. Helps in adding
> support for TPS65224 PMIC.
> 
> Signed-off-by: Bhargav Raviprakash 
> Reviewed-by: Dhruva Gole 

Reviewed-by: Jaehoon Chung 

Best Regards,
Jaehoon Chung

> ---
>  drivers/power/regulator/tps65941_regulator.c | 54 +++-
>  1 file changed, 42 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/power/regulator/tps65941_regulator.c 
> b/drivers/power/regulator/tps65941_regulator.c
> index b041126775..cf54e30df5 100644
> --- a/drivers/power/regulator/tps65941_regulator.c
> +++ b/drivers/power/regulator/tps65941_regulator.c
> @@ -16,6 +16,25 @@
>  #include 
>  #include 
>  
> +/* Single Phase Buck IDs */
> +#define TPS65941_BUCK_ID_11
> +#define TPS65941_BUCK_ID_22
> +#define TPS65941_BUCK_ID_33
> +#define TPS65941_BUCK_ID_44
> +#define TPS65941_BUCK_ID_55
> +
> +/* Multi Phase Buck IDs */
> +#define TPS65941_BUCK_ID_12  12
> +#define TPS65941_BUCK_ID_34  34
> +#define TPS65941_BUCK_ID_123123
> +#define TPS65941_BUCK_ID_1234  1234
> +
> +/* LDO IDs */
> +#define TPS65941_LDO_ID_1 1
> +#define TPS65941_LDO_ID_2 2
> +#define TPS65941_LDO_ID_3 3
> +#define TPS65941_LDO_ID_4 4
> +
>  static const char tps65941_buck_ctrl[TPS65941_BUCK_NUM] = {0x4, 0x6, 0x8, 
> 0xA,
>   0xC};
>  static const char tps65941_buck_vout[TPS65941_BUCK_NUM] = {0xE, 0x10, 0x12,
> @@ -270,10 +289,15 @@ static int tps65941_ldo_probe(struct udevice *dev)
>   uc_pdata->type = REGULATOR_TYPE_LDO;
>  
>   idx = dev->driver_data;
> - if (idx == 1 || idx == 2 || idx == 3 || idx == 4) {
> + switch (idx) {
> + case TPS65941_LDO_ID_1:
> + case TPS65941_LDO_ID_2:
> + case TPS65941_LDO_ID_3:
> + case TPS65941_LDO_ID_4:
>   debug("Single phase regulator\n");
> - } else {
> - printf("Wrong ID for regulator\n");
> + break;
> + default:
> + pr_err("Wrong ID for regulator\n");
>   return -EINVAL;
>   }
>  
> @@ -292,18 +316,24 @@ static int tps65941_buck_probe(struct udevice *dev)
>   uc_pdata->type = REGULATOR_TYPE_BUCK;
>  
>   idx = dev->driver_data;
> - if (idx == 1 || idx == 2 || idx == 3 || idx == 4 || idx == 5) {
> + switch (idx) {
> + case TPS65941_BUCK_ID_1:
> + case TPS65941_BUCK_ID_2:
> + case TPS65941_BUCK_ID_3:
> + case TPS65941_BUCK_ID_4:
> + case TPS65941_BUCK_ID_5:
>   debug("Single phase regulator\n");
> - } else if (idx == 12) {
> + break;
> + case TPS65941_BUCK_ID_12:
> + case TPS65941_BUCK_ID_123:
> + case TPS65941_BUCK_ID_1234:
>   idx = 1;
> - } else if (idx == 34) {
> + break;
> + case TPS65941_BUCK_ID_34:
>   idx = 3;
> - } else if (idx == 123) {
> - idx = 1;
> - } else if (idx == 1234) {
> - idx = 1;
> - } else {
> - printf("Wrong ID for regulator\n");
> + break;
> + default:
> + pr_err("Wrong ID for regulator\n");
>   return -EINVAL;
>   }
>  

Re: [RESEND PATCH v3 4/5] power: regulator: tps65941: use function callbacks for conversion ops

[Jaehoon Chung]

On 3/18/24 18:49, Bhargav Raviprakash wrote:
> Use function callbacks for volt2val, val2volt and slewrate lookups.
> This makes it easier to add support for TPS65224 PMIC regulators.
> 
> Signed-off-by: Bhargav Raviprakash 

Reviewed-by: Jaehoon Chung 

Best Regards,
Jaehoon Chung

> ---
>  drivers/power/regulator/tps65941_regulator.c | 61 +++-
>  1 file changed, 48 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/power/regulator/tps65941_regulator.c 
> b/drivers/power/regulator/tps65941_regulator.c
> index cf54e30df5..d879c2301b 100644
> --- a/drivers/power/regulator/tps65941_regulator.c
> +++ b/drivers/power/regulator/tps65941_regulator.c
> @@ -35,6 +35,17 @@
>  #define TPS65941_LDO_ID_3 3
>  #define TPS65941_LDO_ID_4 4
>  
> +#define TPS65941_BUCK_CONV_OPS_IDX  0
> +#define TPS65941_LDO_CONV_OPS_IDX   0
> +
> +struct tps65941_reg_conv_ops {
> + int volt_mask;
> + int (*volt2val)(int idx, int uV);
> + int (*val2volt)(int idx, int volt);
> + int slew_mask;
> + int (*lookup_slew)(int id);
> +};
> +
>  static const char tps65941_buck_ctrl[TPS65941_BUCK_NUM] = {0x4, 0x6, 0x8, 
> 0xA,
>   0xC};
>  static const char tps65941_buck_vout[TPS65941_BUCK_NUM] = {0xE, 0x10, 0x12,
> @@ -79,7 +90,7 @@ static int tps65941_buck_enable(struct udevice *dev, int 
> op, bool *enable)
>   return 0;
>  }
>  
> -static int tps65941_buck_volt2val(int uV)
> +static int tps65941_buck_volt2val(__maybe_unused int idx, int uV)
>  {
>   if (uV > TPS65941_BUCK_VOLT_MAX)
>   return -EINVAL;
> @@ -95,7 +106,7 @@ static int tps65941_buck_volt2val(int uV)
>   return -EINVAL;
>  }
>  
> -static int tps65941_buck_val2volt(int val)
> +static int tps65941_buck_val2volt(__maybe_unused int idx, int val)
>  {
>   if (val > TPS65941_BUCK_VOLT_MAX_HEX)
>   return -EINVAL;
> @@ -135,12 +146,25 @@ int tps65941_lookup_slew(int id)
>   }
>  }
>  
> +static const struct tps65941_reg_conv_ops buck_conv_ops[] = {
> + [TPS65941_BUCK_CONV_OPS_IDX] = {
> + .volt_mask = TPS65941_BUCK_VOLT_MASK,
> + .volt2val = tps65941_buck_volt2val,
> + .val2volt = tps65941_buck_val2volt,
> + .slew_mask = TP65941_BUCK_CONF_SLEW_MASK,
> + .lookup_slew = tps65941_lookup_slew,
> + },
> +};
> +
>  static int tps65941_buck_val(struct udevice *dev, int op, int *uV)
>  {
>   unsigned int hex, adr;
> - int ret, delta, uwait, slew;
> + int ret, delta, uwait, slew, idx;
>   struct dm_regulator_uclass_plat *uc_pdata;
> + const struct tps65941_reg_conv_ops *conv_ops;
>  
> + idx = dev->driver_data;
> + conv_ops = _conv_ops[TPS65941_BUCK_CONV_OPS_IDX];
>   uc_pdata = dev_get_uclass_plat(dev);
>  
>   if (op == PMIC_OP_GET)
> @@ -152,8 +176,8 @@ static int tps65941_buck_val(struct udevice *dev, int op, 
> int *uV)
>   if (ret < 0)
>   return ret;
>  
> - ret &= TPS65941_BUCK_VOLT_MASK;
> - ret = tps65941_buck_val2volt(ret);
> + ret &= conv_ops->volt_mask;
> + ret = conv_ops->val2volt(idx, ret);
>   if (ret < 0)
>   return ret;
>  
> @@ -175,14 +199,14 @@ static int tps65941_buck_val(struct udevice *dev, int 
> op, int *uV)
>   if (slew < 0)
>   return ret;
>  
> - slew &= TP65941_BUCK_CONF_SLEW_MASK;
> - slew = tps65941_lookup_slew(slew);
> + slew &= conv_ops->slew_mask;
> + slew = conv_ops->lookup_slew(slew);
>   if (slew <= 0)
>   return ret;
>  
>   uwait = delta / slew;
>  
> - hex = tps65941_buck_volt2val(*uV);
> + hex = conv_ops->volt2val(idx, *uV);
>   if (hex < 0)
>   return hex;
>  
> @@ -231,7 +255,7 @@ static int tps65941_ldo_enable(struct udevice *dev, int 
> op, bool *enable)
>   return 0;
>  }
>  
> -static int tps65941_ldo_val2volt(int val)
> +static int tps65941_ldo_val2volt(__maybe_unused int idx, int val)
>  {
>   if (val > TPS65941_LDO_VOLT_MAX_HEX || val < TPS65941_LDO_VOLT_MIN_HEX)
>   return -EINVAL;
> @@ -241,12 +265,23 @@ static int tps65941_ldo_val2volt(int val)
>   return -EINVAL;
>  }
>  
> +static const struct tps65941_reg_conv_ops ldo_conv_ops[] = {
> + [TPS65941_LDO_CONV_OPS_IDX] = {
> + .volt_mask = TPS65941_LDO_VOLT_MASK,
> + .volt2val = tps65941_buck_volt2val,
> + .val2volt = tps65941_ldo_val2volt,
> + },
> +};
> +
>  static int tps65941_ldo_val(struct udevice *dev, int op, int *uV)
>  {
>   unsigned int hex, adr;
> - int ret;
> + int ret, idx;
>   struct dm_regulator_uclass_plat *uc_pdata;
> + const struct tps65941_reg_conv_ops *conv_ops;
>  
> + idx = dev->driver_data;
> + conv_ops = _conv_ops[TPS65941_LDO_CONV_OPS_IDX];
>   uc_pdata = dev_get_uclass_plat(dev);
>  
>   if (op == PMIC_OP_GET)
> @@ -258,8 +293,8 @@ static int tps65941_ldo_val(struct 

Re: [RESEND PATCH v3 2/5] power: pmic: tps65941: Add TI TPS65224 PMIC

[Jaehoon Chung]

On 3/18/24 18:49, Bhargav Raviprakash wrote:
> Adds compatible and data field values of TPS65224 driver in
> TPS65941 PMIC driver.
> 
> Signed-off-by: Bhargav Raviprakash 
> Reviewed-by: Dhruva Gole 

Reviewed-by: Jaehoon Chung 

Best Regards,
Jaehoon Chung

> ---
>  drivers/power/pmic/tps65941.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/power/pmic/tps65941.c b/drivers/power/pmic/tps65941.c
> index 727b42747a..ef63eb733a 100644
> --- a/drivers/power/pmic/tps65941.c
> +++ b/drivers/power/pmic/tps65941.c
> @@ -75,6 +75,7 @@ static const struct udevice_id tps65941_ids[] = {
>   { .compatible = "ti,tps659412", .data = TPS659411 },
>   { .compatible = "ti,tps659413", .data = TPS659413 },
>   { .compatible = "ti,lp876441",  .data =  LP876441 },
> + { .compatible = "ti,tps65224",  .data =  TPS65224 },
>   { }
>  };
>  

Re: [RESEND PATCH v3 1/5] power: tps65941: Add macros of TPS65224 PMIC

[Jaehoon Chung]

On 3/18/24 18:49, Bhargav Raviprakash wrote:
> Re-use the TPS65941 PMIC driver for TPS65224 PMIC.
> Add additional macros of TPS65224 to aid in the driver
> re-use.
> 
> Signed-off-by: Bhargav Raviprakash 
> Reviewed-by: Dhruva Gole 

Reviewed-by: Jaehoon Chung 

Best Regards,
Jaehoon Chung

> ---
>  include/power/tps65941.h | 30 ++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/include/power/tps65941.h b/include/power/tps65941.h
> index a2bc6814ba..cec85333f0 100644
> --- a/include/power/tps65941.h
> +++ b/include/power/tps65941.h
> @@ -3,11 +3,14 @@
>  #define TPS6594130x2
>  #define TPS6594140x3
>  #define  LP8764410x4
> +#define  TPS652240x5
>  
>  /* I2C device address for pmic tps65941 */
>  #define TPS65941_I2C_ADDR(0x12 >> 1)
>  #define TPS65941_LDO_NUM 4
>  #define TPS65941_BUCK_NUM5
> +#define TPS65224_LDO_NUM 3
> +#define TPS65224_BUCK_NUM4
>  
>  /* Drivers name */
>  #define TPS65941_LDO_DRIVER  "tps65941_ldo"
> @@ -25,3 +28,30 @@
>  #define TPS65941_LDO_MODE_MASK   0x1
>  #define TPS65941_LDO_BYPASS_EN   0x80
>  #define TP65941_BUCK_CONF_SLEW_MASK  0x7
> +
> +#define TPS65224_BUCK_VOLT_MAX   330
> +#define TPS65224_BUCK1_VOLT_MAX_HEX  0xFD
> +#define TPS65224_BUCK234_VOLT_MAX_HEX0x45
> +
> +#define TPS65224_BUCK_CONF_SLEW_MASK 0x3
> +#define TPS65224_LDO_VOLT_MASK(0x3F << 1)
> +
> +#define TPS65224_LDO1_VOLT_MIN_HEX   0x0C
> +#define TPS65224_LDO23_VOLT_MIN_HEX  0x00
> +#define TPS65224_LDO1_VOLT_MAX_HEX   0x36
> +#define TPS65224_LDO23_VOLT_MAX_HEX  0x38
> +
> +#define TPS65224_LDO1_VOLT_MAX330
> +#define TPS65224_LDO23_VOLT_MAX   340
> +#define TPS65224_LDO1_VOLT_MIN120
> +#define TPS65224_LDO23_VOLT_MIN60
> +
> +#define TPS65224_LDO_STEP   5
> +
> +#define TPS65224_LDO_BYP_CONFIG 7
> +
> +#define TPS65224_LDO1_VOLT_BYP_MIN220
> +#define TPS65224_LDO1_VOLT_BYP_MAX360
> +
> +#define TPS65224_LDO23_VOLT_BYP_MIN   150
> +#define TPS65224_LDO23_VOLT_BYP_MAX   550

Re: [PATCH 00/15] configs: ti: Enable basic settings for SystemReady ACS

[Tom Rini]

On Mon, 08 Apr 2024 16:10:45 -0500, Jonathan Humphreys wrote:

> Set basic settings needed for System Ready IR ACS testing, for several TI SoC
> based platforms: AM64, AM62, AM62p, BeaglePlay, J7, and BeagleboneAI.
> 
> For AM64, AM62, and AM62p, also includes some config cleanup.  Should be no
> functional change.
> 
> Jonathan Humphreys (15):
>   configs: am64x: cosmetic config cleanup
>   configs: am64x: Enable basic EFI CMD support
>   configs: am64x: Enable RTC emulation
>   configs: j721e: Enable basic EFI CMD support
>   configs: j721e: Enable RTC emulation
>   configs: beagleplay: Enable basic EFI CMD support
>   configs: beagleplay: Enable RTC emulation
>   configs: am62px: cosmetic config cleanup
>   configs: am62px: Enable basic EFI CMD support
>   configs: am62px: Enable RTC emulation
>   configs: am62x: cosmetic config cleanup
>   configs: am62x: Enable basic EFI CMD support
>   configs: am62x: Enable RTC emulation
>   configs: beagleboneai64: Enable basic EFI CMD support
>   configs: beagleboneai64: Enable RTC emulation
> 
> [...]

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH 00/23] [RFC] Integrate MbedTLS v3.6 LTS with U-Boot

[Tom Rini]

On Wed, Apr 17, 2024 at 04:48:14PM -0400, Raymond Mao wrote:
> Hi Tom,
> 
> On Tue, 16 Apr 2024 at 15:23, Tom Rini  wrote:
> 
> > On Tue, Apr 16, 2024 at 11:59:56AM -0700, Raymond Mao wrote:
> >
> > > Integrate MbedTLS v3.6 LTS (currently v3.6.0-RC1) with U-Boot.
> > >
> > > Patch 01 and 02 are for introducing MbedTLS release package.
> > > I have to split it into 2 parts due to the size limitation of Gmail.
> >
> > So to be clear, for v2 you need to switch this to subtrees, like we do
> > for upstream dts files now. And a script to automate the "merge a new
> > release" should be done as well. Thanks!
> >
> Just to confirm, for v2, I don't need to send the commit that was generated
> by "git subtree add ***" but just state the command as a prerequisite in the
> commit message, right? Like what was done for the commit id
> "5b825032957c2613ef2f8f639e949ae02cb5bdff".

Yes, correct. For testing, people will be expected to either:
- Manually "git subtree add ..."
- Use the script that your series will add for syncing, to perform the
  initial import.

The script for devicetree-rebasing handles this already so should be a
good model, I believe.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH 00/23] [RFC] Integrate MbedTLS v3.6 LTS with U-Boot

[Raymond Mao]

Hi Tom,

On Tue, 16 Apr 2024 at 15:23, Tom Rini  wrote:

> On Tue, Apr 16, 2024 at 11:59:56AM -0700, Raymond Mao wrote:
>
> > Integrate MbedTLS v3.6 LTS (currently v3.6.0-RC1) with U-Boot.
> >
> > Patch 01 and 02 are for introducing MbedTLS release package.
> > I have to split it into 2 parts due to the size limitation of Gmail.
>
> So to be clear, for v2 you need to switch this to subtrees, like we do
> for upstream dts files now. And a script to automate the "merge a new
> release" should be done as well. Thanks!
>
> Just to confirm, for v2, I don't need to send the commit that was generated
by "git subtree add ***" but just state the command as a prerequisite in the
commit message, right? Like what was done for the commit id
"5b825032957c2613ef2f8f639e949ae02cb5bdff".

Thanks and regards,
Raymond

[PATCH 1/3] clk: imx8mm: Add support for PCIe clocks

[Tim Harvey]

Add support for PCIe clocks required to enable PCIe support on
iMX8MM SoC.

Signed-off-by: Tim Harvey 
---
 drivers/clk/imx/clk-imx8mm.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
index b5c253e49663..c2f01b385201 100644
--- a/drivers/clk/imx/clk-imx8mm.c
+++ b/drivers/clk/imx/clk-imx8mm.c
@@ -66,6 +66,15 @@ static const char *imx8mm_i2c3_sels[] = {"clock-osc-24m", 
"sys_pll1_160m", "sys_
 static const char *imx8mm_i2c4_sels[] = {"clock-osc-24m", "sys_pll1_160m", 
"sys_pll2_50m", "sys_pll3_out", "audio_pll1_out",
 "video_pll1_out", "audio_pll2_out", 
"sys_pll1_133m", };
 
+static const char *imx8mm_pcie1_ctrl_sels[] = {"clock-osc-24m", 
"sys_pll2_250m", "sys_pll2_200m", "sys_pll1_266m",
+  "sys_pll1_800m", 
"sys_pll2_500m", "sys_pll2_333m", "sys_pll3_out", };
+
+static const char *imx8mm_pcie1_phy_sels[] = {"clock-osc-24m", 
"sys_pll2_100m", "sys_pll2_500m", "clk_ext1", "clk_ext2",
+ "clk_ext3", "clk_ext4", 
"sys_pll1_400m", };
+
+static const char *imx8mm_pcie1_aux_sels[] = {"clock-osc-24m", 
"sys_pll2_200m", "sys_pll2_50m", "sys_pll3_out",
+ "sys_pll2_100m", "sys_pll1_80m", 
"sys_pll1_160m", "sys_pll1_200m", };
+
 #ifndef CONFIG_SPL_BUILD
 static const char *imx8mm_pwm1_sels[] = {"clock-osc-24m", "sys_pll2_100m", 
"sys_pll1_160m", "sys_pll1_40m",
 "sys_pll3_out", "clk_ext1", 
"sys_pll1_80m", "video_pll1_out", };
@@ -256,6 +265,15 @@ static int imx8mm_clk_probe(struct udevice *dev)
imx8m_clk_composite("usb_bus", imx8mm_usb_bus_sels, base + 
0x8b80));
 
/* IP */
+   clk_dm(IMX8MM_CLK_PCIE1_CTRL,
+  imx8m_clk_composite("pcie1_ctrl", imx8mm_pcie1_ctrl_sels,
+  base + 0xa300));
+   clk_dm(IMX8MM_CLK_PCIE1_PHY,
+  imx8m_clk_composite("pcie1_phy", imx8mm_pcie1_phy_sels,
+  base + 0xa380));
+   clk_dm(IMX8MM_CLK_PCIE1_AUX,
+  imx8m_clk_composite("pcie1_aux", imx8mm_pcie1_aux_sels,
+  base + 0xa400));
clk_dm(IMX8MM_CLK_USDHC1,
   imx8m_clk_composite("usdhc1", imx8mm_usdhc1_sels,
   base + 0xac00));
@@ -339,6 +357,9 @@ static int imx8mm_clk_probe(struct udevice *dev)
   imx_clk_gate4("pwm4_root_clk", "pwm4", base + 0x42b0, 0));
 #endif
 
+   clk_dm(IMX8MM_CLK_PCIE1_ROOT,
+  imx_clk_gate4("pcie1_root_clk", "pcie1_ctrl", base + 0x4250, 0));
+
 #if CONFIG_IS_ENABLED(DM_SPI)
clk_dm(IMX8MM_CLK_ECSPI1,
   imx8m_clk_composite("ecspi1", imx8mm_ecspi1_sels, base + 
0xb280));
-- 
2.25.1

[PATCH 3/3] imx8mm_venice_defconfig: Enable PCIe/NVMe support

[Tim Harvey]

Enable PCIe/NVMe support. Also, enable the reset
driver which is a prerequisite for PCIe support.

Signed-off-by: Tim Harvey 
---
 configs/imx8mm_venice_defconfig | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/configs/imx8mm_venice_defconfig b/configs/imx8mm_venice_defconfig
index c3fd4c45ab3d..d34dd7cc2bac 100644
--- a/configs/imx8mm_venice_defconfig
+++ b/configs/imx8mm_venice_defconfig
@@ -12,6 +12,7 @@ CONFIG_DEFAULT_DEVICE_TREE="freescale/imx8mm-venice-gw71xx-0x"
 CONFIG_SPL_TEXT_BASE=0x7E1000
 CONFIG_TARGET_IMX8MM_VENICE=y
 CONFIG_OF_LIBFDT_OVERLAY=y
+CONFIG_DM_RESET=y
 CONFIG_SYS_MONITOR_LEN=524288
 CONFIG_SPL_MMC=y
 CONFIG_SPL_SERIAL=y
@@ -20,6 +21,7 @@ CONFIG_SPL_STACK=0x92
 CONFIG_SPL=y
 CONFIG_ENV_OFFSET_REDUND=0x3f8000
 CONFIG_SYS_LOAD_ADDR=0x4820
+CONFIG_PCI=y
 CONFIG_SYS_MEMTEST_START=0x4000
 CONFIG_SYS_MEMTEST_END=0x8000
 CONFIG_FIT=y
@@ -60,6 +62,7 @@ CONFIG_CMD_FUSE=y
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_I2C=y
 CONFIG_CMD_MMC=y
+CONFIG_CMD_PCI=y
 CONFIG_CMD_USB=y
 CONFIG_CMD_USB_MASS_STORAGE=y
 CONFIG_CMD_DHCP6=y
@@ -119,6 +122,9 @@ CONFIG_PHY_GIGE=y
 CONFIG_FEC_MXC=y
 CONFIG_KSZ9477=y
 CONFIG_MII=y
+CONFIG_NVME_PCI=y
+CONFIG_PCIE_DW_IMX=y
+CONFIG_PHY_IMX8M_PCIE=y
 CONFIG_PINCTRL=y
 CONFIG_SPL_PINCTRL=y
 CONFIG_PINCTRL_IMX8M=y
-- 
2.25.1

[PATCH 2/3] pci: dw_imx: add support for IMX8MM

[Tim Harvey]

Add support for the IMX8MM SoC by adding driver data with the compatible
string of the GPR controller.

Signed-off-by: Tim Harvey 
---
 drivers/pci/pcie_dw_imx.c | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pcie_dw_imx.c b/drivers/pci/pcie_dw_imx.c
index a2ee228224b5..10d926c30645 100644
--- a/drivers/pci/pcie_dw_imx.c
+++ b/drivers/pci/pcie_dw_imx.c
@@ -45,6 +45,10 @@
 #define IMX8M_GPR_PCIE_CLK_REQ_OVERRIDE_EN BIT(10)
 #define IMX8M_GPR_PCIE_CLK_REQ_OVERRIDEBIT(11)
 
+struct pcie_chip_info {
+   const char *gpr;
+};
+
 struct pcie_dw_imx {
/* Must be first member of the struct */
struct pcie_dw  dw;
@@ -54,6 +58,15 @@ struct pcie_dw_imx {
struct reset_ctlapps_reset;
struct phy  phy;
struct udevice  *vpcie;
+   struct pcie_chip_info   *info;
+};
+
+static const struct pcie_chip_info imx8mm_chip_info = {
+   .gpr = "fsl,imx8mm-iomuxc-gpr",
+};
+
+static const struct pcie_chip_info imx8mp_chip_info = {
+   .gpr = "fsl,imx8mp-iomuxc-gpr",
 };
 
 static void pcie_dw_configure(struct pcie_dw_imx *priv, u32 cap_speed)
@@ -246,6 +259,8 @@ static int pcie_dw_imx_of_to_plat(struct udevice *dev)
ofnode gpr;
int ret;
 
+   priv->info = (void *)dev_get_driver_data(dev);
+
/* Get the controller base address */
priv->dw.dbi_base = (void *)dev_read_addr_name(dev, "dbi");
if ((fdt_addr_t)priv->dw.dbi_base == FDT_ADDR_T_NONE) {
@@ -287,7 +302,7 @@ static int pcie_dw_imx_of_to_plat(struct udevice *dev)
goto err_phy;
}
 
-   gpr = ofnode_by_compatible(ofnode_null(), "fsl,imx8mp-iomuxc-gpr");
+   gpr = ofnode_by_compatible(ofnode_null(), priv->info->gpr);
if (ofnode_equal(gpr, ofnode_null())) {
dev_err(dev, "unable to find GPR node\n");
ret = -ENODEV;
@@ -322,7 +337,8 @@ static const struct dm_pci_ops pcie_dw_imx_ops = {
 };
 
 static const struct udevice_id pcie_dw_imx_ids[] = {
-   { .compatible = "fsl,imx8mp-pcie" },
+   { .compatible = "fsl,imx8mm-pcie", .data = (ulong)_chip_info, },
+   { .compatible = "fsl,imx8mp-pcie", .data = (ulong)_chip_info, },
{ }
 };
 
-- 
2.25.1

[PATCH] imx8m*_venice_defconfig: enable ipv6, wget and tftpput

[Tim Harvey]

Enable ipv6, wget, and tftpput support

Signed-off-by: Tim Harvey 
---
 configs/imx8mm_venice_defconfig | 5 +
 configs/imx8mn_venice_defconfig | 5 +
 configs/imx8mp_venice_defconfig | 5 +
 3 files changed, 15 insertions(+)

diff --git a/configs/imx8mm_venice_defconfig b/configs/imx8mm_venice_defconfig
index cb6b97d097c1..c3fd4c45ab3d 100644
--- a/configs/imx8mm_venice_defconfig
+++ b/configs/imx8mm_venice_defconfig
@@ -62,7 +62,10 @@ CONFIG_CMD_I2C=y
 CONFIG_CMD_MMC=y
 CONFIG_CMD_USB=y
 CONFIG_CMD_USB_MASS_STORAGE=y
+CONFIG_CMD_DHCP6=y
+CONFIG_CMD_TFTPPUT=y
 CONFIG_SYS_DISABLE_AUTOLOAD=y
+CONFIG_CMD_WGET=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_TIME=y
 CONFIG_CMD_UUID=y
@@ -83,6 +86,8 @@ CONFIG_ETHPRIME="eth0"
 CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_IP_DEFRAG=y
 CONFIG_TFTP_BLOCKSIZE=4096
+CONFIG_PROT_TCP_SACK=y
+CONFIG_IPV6=y
 CONFIG_SPL_DM=y
 CONFIG_SPL_CLK_COMPOSITE_CCF=y
 CONFIG_CLK_COMPOSITE_CCF=y
diff --git a/configs/imx8mn_venice_defconfig b/configs/imx8mn_venice_defconfig
index 0a4fba5f1ca8..21b68e3cc841 100644
--- a/configs/imx8mn_venice_defconfig
+++ b/configs/imx8mn_venice_defconfig
@@ -65,7 +65,10 @@ CONFIG_CMD_I2C=y
 CONFIG_CMD_MMC=y
 CONFIG_CMD_USB=y
 CONFIG_CMD_USB_MASS_STORAGE=y
+CONFIG_CMD_DHCP6=y
+CONFIG_CMD_TFTPPUT=y
 CONFIG_SYS_DISABLE_AUTOLOAD=y
+CONFIG_CMD_WGET=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_TIME=y
 CONFIG_CMD_UUID=y
@@ -86,6 +89,8 @@ CONFIG_ETHPRIME="eth0"
 CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_IP_DEFRAG=y
 CONFIG_TFTP_BLOCKSIZE=4096
+CONFIG_PROT_TCP_SACK=y
+CONFIG_IPV6=y
 CONFIG_SPL_DM=y
 CONFIG_SPL_CLK_IMX8MN=y
 CONFIG_CLK_IMX8MN=y
diff --git a/configs/imx8mp_venice_defconfig b/configs/imx8mp_venice_defconfig
index 6e4addc7728f..187221d2ba63 100644
--- a/configs/imx8mp_venice_defconfig
+++ b/configs/imx8mp_venice_defconfig
@@ -67,7 +67,10 @@ CONFIG_CMD_I2C=y
 CONFIG_CMD_MMC=y
 CONFIG_CMD_PCI=y
 CONFIG_CMD_USB=y
+CONFIG_CMD_DHCP6=y
+CONFIG_CMD_TFTPPUT=y
 CONFIG_SYS_DISABLE_AUTOLOAD=y
+CONFIG_CMD_WGET=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_TIME=y
 CONFIG_CMD_UUID=y
@@ -86,6 +89,8 @@ CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y
 CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_IP_DEFRAG=y
 CONFIG_TFTP_BLOCKSIZE=4096
+CONFIG_PROT_TCP_SACK=y
+CONFIG_IPV6=y
 CONFIG_SPL_DM=y
 CONFIG_REGMAP=y
 CONFIG_SYSCON=y
-- 
2.25.1

[PATCH] board: gateworks: venice: fix dt adjustment for gw73xx baseboard for imx8mp

[Tim Harvey]

The GW73xx baseboard needs a PCI dt adjustment for revC/D based on a
change of the PCIe switch. Make sure we are only doing this for a pci
based ethernet to avoid causing a boot hang when the ethernet1 alias
points to eqos or fec. To know this is a pcie device ensure the alias
begins with the pcie controller.

Signed-off-by: Tim Harvey 
---
 board/gateworks/venice/venice.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/board/gateworks/venice/venice.c b/board/gateworks/venice/venice.c
index f54f1186b686..5b105d7659e4 100644
--- a/board/gateworks/venice/venice.c
+++ b/board/gateworks/venice/venice.c
@@ -230,6 +230,7 @@ uint mmc_get_env_part(struct mmc *mmc)
 int ft_board_setup(void *fdt, struct bd_info *bd)
 {
const char *base_model = eeprom_get_baseboard_model();
+   const char *path;
char pcbrev;
int off;
 
@@ -238,10 +239,10 @@ int ft_board_setup(void *fdt, struct bd_info *bd)
 
if (!strncmp(base_model, "GW73", 4)) {
pcbrev = get_pcb_rev(base_model);
+   path = fdt_get_alias(fdt, "ethernet1");
 
-   if (pcbrev > 'B' && pcbrev < 'E') {
-   printf("adjusting dt for %s\n", base_model);
-
+   if (pcbrev > 'B' && pcbrev < 'E' && path && !strncmp(path, 
"/soc@0/pcie@", 12)) {
+   printf("adjusting %s pcie\n", base_model);
/*
 * revC/D/E has PCIe 4-port switch which changes
 * ethernet1 PCIe GbE:
-- 
2.25.1

Re: [PATCH 4/4] configs: visionfive2: enable SPL_YMODEM_SUPPORT

[E Shattow]

P.S. mis-spoke should have written "Having 'env load' allows to skip
that reboot step
in-between

On Wed, Apr 17, 2024 at 12:28 PM E Shattow  wrote:
>
> On Wed, Apr 17, 2024 at 1:22 AM Heinrich Schuchardt
>  wrote:
> >
> > On 17.04.24 03:41, E Shattow wrote:
> > > Successful in use w/ 'tio' serial tool and Adafruit CP2102N Friend
> > > adapter on Mars CM Lite board in DFRobot mini router carrier.
> > >
> > > While SPL and u-boot.itb payload are sourced via UART the environment
> > > variables are from the environment variable storage as-is. This makes
> > > sense in the use case for development but may have side-effects in the
> > > case of U-Boot as a JH7110 recovery tool. There is now 'env default -f
> > > -a' which does not purge non-default variables and retains the
> > > non-default variables when migrating from vendor firmware. Consider to
> > > also build for U-Boot the commands that can aid in cleaning the stored
> > > environment variable state CONFIG_CMD_ERASEENV=y and in-memory state
> > > CONFIG_CMD_NVEDIT_LOAD=y. With these it can be done easily with: 'env
> > > erase; env load; env save'.
> >
> > Thank you for testing.
> >
> > After erasing there is no need to save the environment. If there is no
> > environment on flash, the default will be loaded anyway:
> >
> >*** Warning - bad CRC, using default environment
> >
> > Instead of 'env erase' you could use 'sf erase 0xf 0x1000' which is
> > already available. As adding CONFIG_CMD_ERASEENV=y is not necessary in
> > the scope of this patch series I would prefer leaving it to future
> > discussion.
> >
> > Best regards
> >
> > Heinrich
> >
>
> The 'env erase; env load; env save' can be done at the same session
> just before recovery write to provide a sanitized environment with the
> defaults of the present U-Boot payload loaded via UART, so is not the
> same as 'sf erase ...' waiting for a reboot (5 minutes more to
> transfer via UART at 115200 baud) and needing to interrupt the boot to
> further 'env save'. Having 'env load' allows to allow that reboot step
> in-between. Point taken though, it's helpful for recovery but not
> required. -E

Re: [PATCH 4/4] configs: visionfive2: enable SPL_YMODEM_SUPPORT

[E Shattow]

On Wed, Apr 17, 2024 at 1:22 AM Heinrich Schuchardt
 wrote:
>
> On 17.04.24 03:41, E Shattow wrote:
> > Successful in use w/ 'tio' serial tool and Adafruit CP2102N Friend
> > adapter on Mars CM Lite board in DFRobot mini router carrier.
> >
> > While SPL and u-boot.itb payload are sourced via UART the environment
> > variables are from the environment variable storage as-is. This makes
> > sense in the use case for development but may have side-effects in the
> > case of U-Boot as a JH7110 recovery tool. There is now 'env default -f
> > -a' which does not purge non-default variables and retains the
> > non-default variables when migrating from vendor firmware. Consider to
> > also build for U-Boot the commands that can aid in cleaning the stored
> > environment variable state CONFIG_CMD_ERASEENV=y and in-memory state
> > CONFIG_CMD_NVEDIT_LOAD=y. With these it can be done easily with: 'env
> > erase; env load; env save'.
>
> Thank you for testing.
>
> After erasing there is no need to save the environment. If there is no
> environment on flash, the default will be loaded anyway:
>
>*** Warning - bad CRC, using default environment
>
> Instead of 'env erase' you could use 'sf erase 0xf 0x1000' which is
> already available. As adding CONFIG_CMD_ERASEENV=y is not necessary in
> the scope of this patch series I would prefer leaving it to future
> discussion.
>
> Best regards
>
> Heinrich
>

The 'env erase; env load; env save' can be done at the same session
just before recovery write to provide a sanitized environment with the
defaults of the present U-Boot payload loaded via UART, so is not the
same as 'sf erase ...' waiting for a reboot (5 minutes more to
transfer via UART at 115200 baud) and needing to interrupt the boot to
further 'env save'. Having 'env load' allows to allow that reboot step
in-between. Point taken though, it's helpful for recovery but not
required. -E

Re: [PATCH 0/5] zfs: Fix zfs support on aarch64

[Tom Rini]

On Sat, 06 Apr 2024 18:47:24 -0700, mwle...@mailtundra.com wrote:

> This patch series is needed to get U-Boot to boot from a ZFS filesystem
> on an aarch64 computer. Some of the patches are not architecture specific
> and would be needed to boot ZFS on other platforms as well. The ZFS
> support in U-Boot hasn't been substantively touched in several years and
> to me it seems like it must have been broken for a long time on all
> platforms, but I have only tested on aarch64.
> 
> [...]

Per Igor's comment and Phaedrus agreement, dropped his Tested-by on the patches
themselves. Then applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH v1] mtd: rawnand: macronix: OTP access for MX30LFxG18AC

[Michael Nazzareno Trimarchi]

Hi

Dario did you add those patches in CI and test them again?

Michael

On Wed, Apr 17, 2024 at 8:44 PM Arseniy Krasnov
 wrote:
>
> Hello,
>
> Sorry, pls ping
>
> Thanks, Arseniy
>
> On 13.03.2024 09:46, Michael Nazzareno Trimarchi wrote:
> > Hi  Dario
> >
> > Can apply this series and put in CI?
> >
> > Michael
> >
> > On Wed, Mar 13, 2024 at 7:43 AM Arseniy Krasnov
> >  wrote:
> >>
> >> Sorry, please ping
> >>
> >> Thanks, Arseniy
> >>
> >> On 11.02.2024 02:16, Arseniy Krasnov wrote:
> >>> Sorry, pls ping
> >>>
> >>> Thanks, Arseniy
> >>>
> >>> On 08.01.2024 21:33, Arseniy Krasnov wrote:
>  Sorry, pls ping
> 
>  Thanks, Arseniy
> >
> >
> >



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH v1] mtd: rawnand: macronix: OTP access for MX30LFxG18AC

[Arseniy Krasnov]

Hello,

Sorry, pls ping

Thanks, Arseniy

On 13.03.2024 09:46, Michael Nazzareno Trimarchi wrote:
> Hi  Dario
> 
> Can apply this series and put in CI?
> 
> Michael
> 
> On Wed, Mar 13, 2024 at 7:43 AM Arseniy Krasnov
>  wrote:
>>
>> Sorry, please ping
>>
>> Thanks, Arseniy
>>
>> On 11.02.2024 02:16, Arseniy Krasnov wrote:
>>> Sorry, pls ping
>>>
>>> Thanks, Arseniy
>>>
>>> On 08.01.2024 21:33, Arseniy Krasnov wrote:
 Sorry, pls ping

 Thanks, Arseniy
> 
> 
> 

Re: [PATCH] imx8m*-venice: enable TPM support

[Fabio Estevam]

Hi Tim,

On Wed, Apr 17, 2024 at 2:00 PM Tim Harvey  wrote:

> Fabio,
>
> This one got tagged as 'changes requested' in patchwork but I'm not
> clear why. Are there any issues with this?

Sorry, this was a mistake. I will apply it in the next batch of patches.

Re: [PATCH] imx8m*-venice: enable TPM support

[Tim Harvey]

On Mon, Mar 25, 2024 at 9:27 AM Tim Harvey  wrote:
>
> Enable support for TPM2 devices. As the ATTPM20P TPM2 used on the
> Gateworks Venice boards hangs off the SPI bus we enable SPI support
> as well.
>
> Signed-off-by: Tim Harvey 
> ---
>  configs/imx8mm_venice_defconfig | 10 ++
>  configs/imx8mn_venice_defconfig | 10 ++
>  configs/imx8mp_venice_defconfig | 10 ++
>  3 files changed, 30 insertions(+)
>
> diff --git a/configs/imx8mm_venice_defconfig b/configs/imx8mm_venice_defconfig
> index cb6b97d097c1..86928687df2d 100644
> --- a/configs/imx8mm_venice_defconfig
> +++ b/configs/imx8mm_venice_defconfig
> @@ -68,6 +68,7 @@ CONFIG_CMD_TIME=y
>  CONFIG_CMD_UUID=y
>  CONFIG_CMD_PMIC=y
>  CONFIG_CMD_REGULATOR=y
> +CONFIG_CMD_TPM=y
>  CONFIG_CMD_EXT4_WRITE=y
>  # CONFIG_ISO_PARTITION is not set
>  # CONFIG_SPL_EFI_PARTITION is not set
> @@ -127,14 +128,20 @@ CONFIG_SPL_DM_PMIC_MP5416=y
>  CONFIG_DM_REGULATOR=y
>  CONFIG_DM_REGULATOR_FIXED=y
>  CONFIG_DM_REGULATOR_GPIO=y
> +# CONFIG_DM_RNG is not set
>  CONFIG_DM_SERIAL=y
>  CONFIG_MXC_UART=y
> +CONFIG_SPI=y
> +CONFIG_DM_SPI=y
> +CONFIG_MXC_SPI=y
>  CONFIG_SYSRESET=y
>  CONFIG_SPL_SYSRESET=y
>  CONFIG_SYSRESET_PSCI=y
>  CONFIG_SYSRESET_WATCHDOG=y
>  CONFIG_DM_THERMAL=y
>  CONFIG_IMX_TMU=y
> +# CONFIG_TPM_V1 is not set
> +CONFIG_TPM2_TIS_SPI=y
>  CONFIG_USB=y
>  CONFIG_USB_EHCI_HCD=y
>  CONFIG_USB_HOST_ETHER=y
> @@ -152,4 +159,7 @@ CONFIG_USB_GADGET_PRODUCT_NUM=0xa4a5
>  CONFIG_CI_UDC=y
>  CONFIG_USB_GADGET_DOWNLOAD=y
>  CONFIG_IMX_WATCHDOG=y
> +CONFIG_TPM=y
> +# CONFIG_SPL_SHA512 is not set
> +# CONFIG_SPL_SHA384 is not set
>  CONFIG_HEXDUMP=y
> diff --git a/configs/imx8mn_venice_defconfig b/configs/imx8mn_venice_defconfig
> index 0a4fba5f1ca8..6bb0f015a9bf 100644
> --- a/configs/imx8mn_venice_defconfig
> +++ b/configs/imx8mn_venice_defconfig
> @@ -71,6 +71,7 @@ CONFIG_CMD_TIME=y
>  CONFIG_CMD_UUID=y
>  CONFIG_CMD_PMIC=y
>  CONFIG_CMD_REGULATOR=y
> +CONFIG_CMD_TPM=y
>  CONFIG_CMD_EXT4_WRITE=y
>  # CONFIG_ISO_PARTITION is not set
>  # CONFIG_SPL_EFI_PARTITION is not set
> @@ -128,14 +129,20 @@ CONFIG_SPL_DM_PMIC_MP5416=y
>  CONFIG_DM_REGULATOR=y
>  CONFIG_DM_REGULATOR_FIXED=y
>  CONFIG_DM_REGULATOR_GPIO=y
> +# CONFIG_DM_RNG is not set
>  CONFIG_DM_SERIAL=y
>  CONFIG_MXC_UART=y
> +CONFIG_SPI=y
> +CONFIG_DM_SPI=y
> +CONFIG_MXC_SPI=y
>  CONFIG_SYSRESET=y
>  CONFIG_SPL_SYSRESET=y
>  CONFIG_SYSRESET_PSCI=y
>  CONFIG_SYSRESET_WATCHDOG=y
>  CONFIG_DM_THERMAL=y
>  CONFIG_IMX_TMU=y
> +# CONFIG_TPM_V1 is not set
> +CONFIG_TPM2_TIS_SPI=y
>  CONFIG_USB=y
>  CONFIG_USB_EHCI_HCD=y
>  CONFIG_USB_HOST_ETHER=y
> @@ -153,4 +160,7 @@ CONFIG_USB_GADGET_PRODUCT_NUM=0xa4a5
>  CONFIG_CI_UDC=y
>  CONFIG_USB_GADGET_DOWNLOAD=y
>  CONFIG_IMX_WATCHDOG=y
> +CONFIG_TPM=y
> +# CONFIG_SPL_SHA512 is not set
> +# CONFIG_SPL_SHA384 is not set
>  CONFIG_HEXDUMP=y
> diff --git a/configs/imx8mp_venice_defconfig b/configs/imx8mp_venice_defconfig
> index 6e4addc7728f..b1055ef2f066 100644
> --- a/configs/imx8mp_venice_defconfig
> +++ b/configs/imx8mp_venice_defconfig
> @@ -73,6 +73,7 @@ CONFIG_CMD_TIME=y
>  CONFIG_CMD_UUID=y
>  CONFIG_CMD_PMIC=y
>  CONFIG_CMD_REGULATOR=y
> +CONFIG_CMD_TPM=y
>  CONFIG_CMD_EXT4_WRITE=y
>  # CONFIG_ISO_PARTITION is not set
>  # CONFIG_SPL_EFI_PARTITION is not set
> @@ -135,14 +136,20 @@ CONFIG_SPL_DM_PMIC_MP5416=y
>  CONFIG_DM_REGULATOR=y
>  CONFIG_DM_REGULATOR_FIXED=y
>  CONFIG_DM_REGULATOR_GPIO=y
> +# CONFIG_DM_RNG is not set
>  CONFIG_DM_SERIAL=y
>  CONFIG_MXC_UART=y
> +CONFIG_SPI=y
> +CONFIG_DM_SPI=y
> +CONFIG_MXC_SPI=y
>  CONFIG_SYSRESET=y
>  CONFIG_SPL_SYSRESET=y
>  CONFIG_SYSRESET_PSCI=y
>  CONFIG_SYSRESET_WATCHDOG=y
>  CONFIG_DM_THERMAL=y
>  CONFIG_IMX_TMU=y
> +# CONFIG_TPM_V1 is not set
> +CONFIG_TPM2_TIS_SPI=y
>  CONFIG_USB=y
>  CONFIG_USB_XHCI_HCD=y
>  CONFIG_USB_XHCI_DWC3=y
> @@ -160,4 +167,7 @@ CONFIG_USB_ETHER_MCS7830=y
>  CONFIG_USB_ETHER_RTL8152=y
>  CONFIG_USB_ETHER_SMSC95XX=y
>  CONFIG_IMX_WATCHDOG=y
> +CONFIG_TPM=y
> +# CONFIG_SPL_SHA512 is not set
> +# CONFIG_SPL_SHA384 is not set
>  CONFIG_HEXDUMP=y
> --
> 2.25.1
>

Fabio,

This one got tagged as 'changes requested' in patchwork but I'm not
clear why. Are there any issues with this?

Best Regards,

Tim

Re: [PATCH v2 00/11] net: dwc_eth_qos: Clean up STM32 glue code and add STM32MP13xx support

[Marek Vasut]

On 3/26/24 1:07 PM, Marek Vasut wrote:

Split off STM32 glue code from the DWMAC driver into separate
file, similar to what other SoCs already do, to avoid mixing
the ST specifics with generic DWMAC core code.

Clean the STM32 DWMAC board code which is currently duplicated
in multiple board files, move it into the newly separated glue
code, since the code is not board specific, it is only generic
DT parsing and generic register programming.

Add STM32MP13xx support based on ST downstream patches on top,
although that part is mostly rewritten from scratch.


Can either of you, Patrice/Patrick, pick this series via ST tree and 
create a MR for Tom (possibly including the other long outstanding 
patches too) ?


Thanks

Re: [Patch] dwc_eth_qos: Revert regression handling fixed phy

[Elmar Psilog]

On 4/17/24 13:15, Marek Vasut wrote:

On 4/17/24 10:41 AM, Nicole Battenfeld wrote:

Subject: [PATCH] dwc_eth_qos: Revert regression handling fixed phy

In imx8mp operation on eqos with fixed phy I get without that patch:
ERROR: no/invalid 

Which commit is being reverted here ?


True. I just checked the mails last year and my confusion grows.

It was right here at the bottom:
https://lists.denx.de/pipermail/u-boot/2023-February/509156.html

But already in PATCH_v2 it seems that line was dropped, my fault.

[PATCH] .gitignore: Add files produced by TI platform

[Andrea Calabrese]

Add files produced by compilation of TI platforms:

*.ti-secure(-rom)
*.map
*-board-config
custMpk.pem
*.bin_*
*.fit
*.itb
tispl.bin_unsigned

Signed-off-by: Andrea Calabrese 
---
 .gitignore | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index be137040a2..870c3f1819 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,6 +10,7 @@
 *.a
 *.asn1.[ch]
 *.bin
+*.bin[_.]*
 *.cfgout
 *.cover
 *.dtb
@@ -17,23 +18,27 @@
 *.dtb.S
 *.elf
 *.exe
+*.fit
 *.gcda
 *.gcno
 *.i
 *.img
+*.itb
 *.lex.c
 *.lst
+*.map
 *.mod.c
 *.mbx
 *.o
 *.o.*
 *.order
 *.patch
+*.pem
 *.s
 *.su
 *.swp
 *.tab.[ch]
-
+*.ti-*
 # Build tree
 /build*
 
-- 
2.34.1

Re: [PATCH 01/10] board: ti: am62x: Init DRAM size in R5/A53 SPL

[Tom Rini]

On Wed, Apr 17, 2024 at 05:48:31PM +0530, Sughosh Ganu wrote:
> hi Chintan,
> 
> On Wed, 17 Apr 2024 at 13:21, Chintan Vankar  wrote:
> >
> >
> >
> > On 16/04/24 22:30, Tom Rini wrote:
> > > On Tue, Apr 16, 2024 at 05:52:58PM +0530, Chintan Vankar wrote:
> > >>
> > >>
> > >> On 12/04/24 03:37, Tom Rini wrote:
> > >>> On Wed, Apr 03, 2024 at 06:18:01PM +0530, Chintan Vankar wrote:
> > 
> > 
> >  On 22/01/24 10:11, Siddharth Vadapalli wrote:
> > >
> > >
> > > On 20/01/24 22:11, Tom Rini wrote:
> > >> On Mon, Jan 15, 2024 at 01:42:51PM +0530, Siddharth Vadapalli wrote:
> > >>> Hello Tom,
> > >>>
> > >>> On 12/01/24 18:56, Tom Rini wrote:
> > >
> > > ...
> > >
> >  The list of conditionals in common/spl/spl.c::board_init_r() 
> >  should be
> >  updated and probably use SPL_NET as the option to check for.
> > >>>
> > >>> Thank you for reviewing the patch and pointing this out. I wasn't 
> > >>> aware of it. I
> > >>> assume that you are referring to the following change:
> > >>>
> > >>>if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || 
> > >>> CONFIG_IS_ENABLED(HANDOFF) ||
> > >>> -   IS_ENABLED(CONFIG_SPL_ATF))
> > >>> +   IS_ENABLED(CONFIG_SPL_ATF) || 
> > >>> IS_ENABLED(CONFIG_SPL_NET))
> > >>>dram_init_banksize();
> > >>>
> > >>> I shall replace the current patch with the above change in the v2 
> > >>> series. Since
> > >>> this is in the common section, is there a generic reason I could 
> > >>> provide in the
> > >>> commit message rather than the existing commit message which seems 
> > >>> to be board
> > >>> specific? Also, I hope that the above change will not cause 
> > >>> regressions for
> > >>> other non-TI devices. Please let me know.
> > >>
> > >> Yes, that's the area, and just note that networking also requires the
> > >> DDR to be initialized.
> > >>
> > >
> > > Thank you for confirming and providing your suggestion for the 
> > > contents of the
> > > commit message.
> > >
> >  Following Tom's Suggestion of adding CONFIG_SPL_NET in common/spl/spl.c
> >  "dram_init_banksize()", the issue of fetching a file at SPL stage 
> >  seemed
> >  to be fixed. However the commit "ba20b2443c29", which sets gd->ram_top
> >  for the very first time in "spl_enable_cache()" results in
> >  "arch_lmb_reserve()" function reserving memory region from Stack 
> >  pointer
> >  at "0x81FFB820" to gd->ram_top pointing to "0x1". Previously
> >  when gd->ram_top was zero "arch_lmb_reserve()" was noop. Now using TFTP
> >  to fetch U-Boot image at SPL stage results in "tftp_init_load_addr()"
> >  function call that invokes "arch_lmb_reserve()" function, which 
> >  reserves
> >  entire memory starting from Stack Pointer to gd->ram_top leaving no
> >  space to load U-Boot image via TFTP since TFTP loads files at pre
> >  configured memory address at "0x8200".
> > 
> >  As a workaround for this issue, one solution we can propose is to
> >  disable the checks "lmb_get_free_size()" at SPL and U-Boot stage. For
> >  that we can define a new config option for LMB reserve checks as
> >  "SPL_LMB". This config will be enable by default for the backword
> >  compatibility and disable for our use case at SPL and U-Boot stage.
> > >>>
> > >>> The problem here is that we need LMB for booting an OS, which is
> > >>> something we'll want in SPL in non-cortex-R cases too, which means this
> > >>> platform, so that's a no-go. I think you need to dig harder and see if
> > >>> you can correct the logic somewhere so that we don't over reserve?
> > >>>
> > >> Since this issue is due to function call "lmb_init_and_reserve()"
> > >> function invoked from "tftp_init_load_addr()" function. This function
> > >> is defined by Simon in commit "a156c47e39ad", which fixes
> > >> "CVE-2018-18439" to prevent overwriting reserved memory. Simon, can you
> > >> explain why do we need to call "lmb_init_and_reserve()" function here ?
> > >
> > > This is indeed a tricky area which is why Sughosh is looking in to
> > > trying to re-work the LMB mechanic and we've had a few long threads
> > > about it as well.
> > >
> > > I've honestly forgotten the use case you have here, can you please
> > > remind us?
> > >
> > We are trying to boot AM62x using Ethernet for which we need to load
> > binary files at SPL and U-Boot stage using TFTP. To store the file we
> > need a free memory in RAM, specifically we are storing these files at
> > 0x8200. But we are facing an issue while loading the file since
> > the memory area having an address 0x8200 is reserved due to
> > "lmb_init_and_reserve()" function call. This function is called in
> > "tftp_init_load_addr()" function which is getting called exactly before
> > we are trying to get 

[PATCH 1/1] cmd: eficonfig: check initrd path allocation

[Heinrich Schuchardt]

After allocating memory for the initrd file path we need to check the
initrd buffer pointer is not NULL.

Fixes: 87d791423ac6 ("eficonfig: menu-driven addition of UEFI boot option")
Signed-off-by: Heinrich Schuchardt 
---
 cmd/eficonfig.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 8234e602b8f..0ba92c60e03 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -1419,7 +1419,7 @@ static efi_status_t eficonfig_edit_boot_option(u16 
*varname, struct eficonfig_bo
}
 
bo->initrd_info.current_path = calloc(1, EFICONFIG_FILE_PATH_BUF_SIZE);
-   if (!bo->file_info.current_path) {
+   if (!bo->initrd_info.current_path) {
ret =  EFI_OUT_OF_RESOURCES;
goto out;
}
-- 
2.43.0

Re: [Patch] dwc_eth_qos: Revert regression handling fixed phy

[Marek Vasut]

On 4/17/24 10:41 AM, Nicole Battenfeld wrote:

Subject: [PATCH] dwc_eth_qos: Revert regression handling fixed phy

In imx8mp operation on eqos with fixed phy I get without that patch:
ERROR: no/invalid 

Which commit is being reverted here ?

Re: [PATCH v2] tpm: display warning if using gpio reset with TPM

[Ilias Apalodimas]

Hi Tim!

> > > > >
> > > > > The current logic expects a reset gpio and bails out if it cannot get
> > > > > it with a (questionable) goto statement.
> > > > >
> > > > > You want to invert that logic, and expect no gpio, but only if there 
> > > > > is
> > > > > one you want to warn.
> > > > >
> > > > > This is perfectly fine but the logic here must be clarified. I'd go 
> > > > > for:
> > > > >
> > > > > ret = gpio_request()
> > > > > if (ret) {
> > > > > ret = gpio_request()
> > > > > if (!ret)
> > > > > warn(deprecated)
> > > > > }
> > > > >
> > > > > if (!ret) {
> > > > > warn(dangerous)
> > > > > toggle_value()
> > > > > }
> > > > >
> > > > > I would ideally replace the 'if (ret)' clauses with 'if (!reset_gpio)'
> > > > > which would make the checks even clearer.
> > > >
> > > > Fair enough. But the code with the proposed change has no functional
> > > > problems right?
> > >
> > > No, this is functionally right, but the code is not clear like that.
> > >
> > > > If so I'll send a PR to Tom as is and rework it as suggested later
> > >
> > > Well, that's not how contribution work usually. Is there an emergency
> > > in getting this merged?
> >
> > Not really, it's a print message. But I don't currently have time to
> > pick this up.
> > Tim, would you mind reworking it as Miquel suggested?
> >
>
> I'm just catching up after being out of the office for a couple of
> weeks - I'll rework it and submit another revision as soon as I have
> some time.

Thanks!
/Ilias
>
> Best Regards,
>
> Tim

Re: [PATCH v2] tpm: display warning if using gpio reset with TPM

[Tim Harvey]

On Wed, Apr 17, 2024 at 12:01 AM Ilias Apalodimas
 wrote:
>
> On Wed, 17 Apr 2024 at 09:48, Miquel Raynal  wrote:
> >
> > Hi Ilias,
> >
> > ilias.apalodi...@linaro.org wrote on Wed, 17 Apr 2024 08:40:14 +0300:
> >
> > > Hi Miquel
> > >
> > > On Mon, 8 Apr 2024 at 10:23, Miquel Raynal  
> > > wrote:
> > > >
> > > > Hello,
> > > >
> > > > ilias.apalodi...@linaro.org wrote on Thu, 28 Mar 2024 09:08:37 +0200:
> > > >
> > > > > Thanks Tim
> > > > >
> > > > > On Thu, 28 Mar 2024 at 00:12, Tim Harvey  
> > > > > wrote:
> > > > > >
> > > > > > Instead of displaying what looks like an error message if a
> > > > > > gpio-reset dt prop is missing for a TPM display a warning that
> > > > > > having a gpio reset on a TPM should not be used for a secure 
> > > > > > production
> > > > > > device.
> > > > > >
> > > > > > TCG TIS spec [1] says:
> > > > > > "The TPM_Init (LRESET#/SPI_RST#) signal MUST be connected to the
> > > > > > platform CPU Reset signal such that it complies with the 
> > > > > > requirements
> > > > > > specified in section 1.2.7 HOST Platform Reset in the PC Client
> > > > > > Implementation Specification for Conventional BIOS."
> > > > > >
> > > > > > The reasoning is that you should not be able to toggle a GPIO and 
> > > > > > reset
> > > > > > the TPM without resetting the CPU as well because if an attacker can
> > > > > > break into your OS via an OS level security flaw they can then 
> > > > > > reset the
> > > > > > TPM via GPIO and replay the measurements required to unseal keys
> > > > > > that you have otherwise protected.
> > > > > >
> > > > > > [1] 
> > > > > > https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClientTPMInterfaceSpecification_TIS__1-3_27_03212013.pdf
> > > > > >
> > > > > > Signed-off-by: Tim Harvey 
> > > > > > ---
> > > > > > v2: change the message to a warning and update commit desc/log
> > > > > > ---
> > > > > >  drivers/tpm/tpm2_tis_spi.c | 8 
> > > > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/tpm/tpm2_tis_spi.c b/drivers/tpm/tpm2_tis_spi.c
> > > > > > index de9cf8f21e07..c9c83f6f0fc8 100644
> > > > > > --- a/drivers/tpm/tpm2_tis_spi.c
> > > > > > +++ b/drivers/tpm/tpm2_tis_spi.c
> > > > > > @@ -237,14 +237,14 @@ static int tpm_tis_spi_probe(struct udevice 
> > > > > > *dev)
> > > > > > /* legacy reset */
> > > > > > ret = gpio_request_by_name(dev, 
> > > > > > "gpio-reset", 0,
> > > > > >_gpio, 
> > > > > > GPIOD_IS_OUT);
> > > > > > -   if (ret) {
> > > > > > -   log(LOGC_NONE, LOGL_NOTICE,
> > > > > > -   "%s: missing reset GPIO\n", 
> > > > > > __func__);
> > > > > > +   if (ret)
> > > > > > goto init;
> > > > > > -   }
> > > > > > log(LOGC_NONE, LOGL_NOTICE,
> > > > > > "%s: gpio-reset is deprecated\n", 
> > > > > > __func__);
> > > > > > }
> > > > > > +   log(LOGC_NONE, LOGL_WARNING,
> > > > > > +   "%s: TPM gpio reset should not be used on 
> > > > > > secure production devices\n",
> > > > > > +   dev->name);
> > > > > > dm_gpio_set_value(_gpio, 1);
> > > > > > mdelay(1);
> > > > > > dm_gpio_set_value(_gpio, 0);
> > > >
> > > > The current logic expects a reset gpio and bails out if it cannot get
> > > > it with a (questionable) goto statement.
> > > >
> > > > You want to invert that logic, and expect no gpio, but only if there is
> > > > one you want to warn.
> > > >
> > > > This is perfectly fine but the logic here must be clarified. I'd go for:
> > > >
> > > > ret = gpio_request()
> > > > if (ret) {
> > > > ret = gpio_request()
> > > > if (!ret)
> > > > warn(deprecated)
> > > > }
> > > >
> > > > if (!ret) {
> > > > warn(dangerous)
> > > > toggle_value()
> > > > }
> > > >
> > > > I would ideally replace the 'if (ret)' clauses with 'if (!reset_gpio)'
> > > > which would make the checks even clearer.
> > >
> > > Fair enough. But the code with the proposed change has no functional
> > > problems right?
> >
> > No, this is functionally right, but the code is not clear like that.
> >
> > > If so I'll send a PR to Tom as is and rework it as suggested later
> >
> > Well, that's not how contribution work usually. Is there an emergency
> > in getting this merged?
>
> Not really, it's a print message. But I don't currently have time to
> pick this up.
> Tim, would you mind reworking it as Miquel suggested?
>

I'm just catching up after being out of the office for a couple of
weeks - I'll rework it and submit another revision as soon as I have
some time.

Best Regards,

Tim

Re: [PATCH] ram: rockchip: px30: Replace misleading log message

[Łukasz Czechowski]

Hi Quentin,

Maybe I was a little too fast with submitting this patch.
My original point of this commit was to get rid of the message "out",
that is printed without any context every time the board boots, or at
least replace it with something more descriptive, similarly like it is
done with other boards.
I've used the `debug` macro as it was used in the same context in e.g.
rk3399, but also already inside the sdram_px30.c file. Indeed I'll try
to revisit this topic once I'm back.

Best regards,
Łukasz


śr., 17 kwi 2024 o 15:58 Quentin Schulz
 napisał(a):
>
> Hi Lukasz,
>
> On 4/17/24 13:16, lukasz.czechow...@thaumatec.com wrote:
> > From: Lukasz Czechowski 
> >
> > Remove the log message "out" from sdram_init function which
> > pollutes the console. It brings no meaningful information and
> > might be unwanted in case silencing the console is required.
> > Instead, add a debug log with a more meaningful message, printed
> > only if DEBUG is set. The same convention is used for other
> > boards, i.e. rk3399.
> >
> > Signed-off-by: Lukasz Czechowski 
> > ---
> >   drivers/ram/rockchip/sdram_px30.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/ram/rockchip/sdram_px30.c 
> > b/drivers/ram/rockchip/sdram_px30.c
> > index 21498e8957..3b429973cd 100644
> > --- a/drivers/ram/rockchip/sdram_px30.c
> > +++ b/drivers/ram/rockchip/sdram_px30.c
> > @@ -712,7 +712,7 @@ int sdram_init(void)
> >
> >   sdram_print_ddr_info(_params->ch.cap_info, _params->base, 
> > 0);
> >
> > - printascii("out\n");
> > + debug("Finish SDRAM initialization...\n");
>
> Mmmm I don't think this is appropriate. debug() is essentially replaced
> with printf() whenever the selected log level permits. The issue is that
> printf() != printascii() and I have a feeling we use printascii
> explicitly because it is a much smaller way to print data than printf.
>
> Additionally, we're extremely size constrained in TPL on PX30, so
> increasing the string size to print may also not be wise. If we are not
> enable to even build the TPL by defining DEBUG, this is basically dead
> code and it's as good to us as removing it entirely.
>
> So... questions:
> 1) does it actually build if you set #define DEBUG 1 or whatever is
> needed for having the debug message printed? If yes, does it boot?
> 2) What's the size of the TPL with this change and with this
> change+DEBUG set?
>
> What I can suggest instead is to guard all printascii (or at least the
> ones only useful for debugging) with the appropriate symbol, e.g. could
> be CONFIG_RAM_ROCKCHIP_DEBUG.
>
> I have not tested it (only build tested on Ringneck PX30 without
> CONFIG_RAM_ROCKCHIP_DEBUG nor CONFIG_TPL_SERIAL), but here's a quick
> attempt:
>
> """
> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> index b7a6f100d41..17656e99a2f 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -11,7 +11,7 @@ config ROCKCHIP_PX30
> select TPL_NEEDS_SEPARATE_STACK if TPL
> imply SPL_SEPARATE_BSS
> select SPL_SERIAL
> -   select TPL_SERIAL
> +   imply TPL_SERIAL
> select DEBUG_UART_BOARD_INIT
> imply ROCKCHIP_COMMON_BOARD
> imply SPL_ROCKCHIP_COMMON_BOARD
> diff --git a/arch/arm/mach-rockchip/px30-board-tpl.c
> b/arch/arm/mach-rockchip/px30-board-tpl.c
> index 637a5e1b18b..8f56147ca7a 100644
> --- a/arch/arm/mach-rockchip/px30-board-tpl.c
> +++ b/arch/arm/mach-rockchip/px30-board-tpl.c
> @@ -36,7 +36,7 @@ void board_init_f(ulong dummy)
>   {
> int ret;
>
> -#ifdef CONFIG_DEBUG_UART
> +#if defined(CONFIG_DEBUG_UART) && defined(CONFIG_TPL_SERIAL)
> debug_uart_init();
> /*
>  * Debug UART can be used from here if required:
> @@ -51,8 +51,10 @@ void board_init_f(ulong dummy)
>
> secure_timer_init();
> ret = sdram_init();
> +#if defined(CONFIG_DEBUG_UART) && defined(CONFIG_TPL_SERIAL)
> if (ret)
> printascii("sdram_init failed\n");
> +#endif
>
> /* return to maskrom */
> back_to_bootrom(BROM_BOOT_NEXTSTAGE);
> diff --git a/drivers/ram/rockchip/Kconfig b/drivers/ram/rockchip/Kconfig
> index 67c63ecba04..cb59bcf0414 100644
> --- a/drivers/ram/rockchip/Kconfig
> +++ b/drivers/ram/rockchip/Kconfig
> @@ -16,6 +16,8 @@ if RAM_ROCKCHIP
>   config RAM_ROCKCHIP_DEBUG
> bool "Rockchip ram drivers debugging"
> default y
> +   depends on TPL_SERIAL if TPL_RAM
> +   depends on SPL_SERIAL if SPL_RAM
> help
>   This enables debugging ram driver API's for the platforms
>   based on Rockchip SoCs.
> diff --git a/drivers/ram/rockchip/sdram_common.c
> b/drivers/ram/rockchip/sdram_common.c
> index 60fc90d0a5c..8076cfa9aad 100644
> --- a/drivers/ram/rockchip/sdram_common.c
> +++ b/drivers/ram/rockchip/sdram_common.c
> @@ -231,7 +231,9 @@ int sdram_detect_col(struct sdram_cap_info *cap_info,
> break;
> }

[PATCH 2/2] cmd: sbi: add coreboot and oreboot implementation IDs

[Heinrich Schuchardt]

Let the sbi command detect the coreboot and oreboot SBI Implementation IDs
defined in SBI specification v2.0.

Signed-off-by: Heinrich Schuchardt 
---
 cmd/riscv/sbi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/cmd/riscv/sbi.c b/cmd/riscv/sbi.c
index 55507b0aa63..2d8ee7e5bbb 100644
--- a/cmd/riscv/sbi.c
+++ b/cmd/riscv/sbi.c
@@ -29,6 +29,8 @@ static struct sbi_imp implementations[] = {
{ 6, "Coffer" },
{ 7, "Xen Project" },
{ 8, "PolarFire Hart Software Services" },
+   { 9, "coreboot" },
+   { 10, "oreboot" },
 };
 
 static struct sbi_ext extensions[] = {
-- 
2.43.0

[PATCH 1/2] cmd: sbi: add Supervisor Software Events extension

[Heinrich Schuchardt]

OpenSBI has implemented the Supervisor Software Events Extension.
Allow detecting it in the sbi command.

Signed-off-by: Heinrich Schuchardt 
---
 arch/riscv/include/asm/sbi.h | 1 +
 cmd/riscv/sbi.c  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index d1113f3d703..ad32dedb589 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -34,6 +34,7 @@ enum sbi_ext_id {
SBI_EXT_NACL = 0x4E41434C,
SBI_EXT_STA = 0x535441,
SBI_EXT_DBTR = 0x44425452,
+   SBI_EXT_SSE = 0x535345,
 };
 
 enum sbi_ext_base_fid {
diff --git a/cmd/riscv/sbi.c b/cmd/riscv/sbi.c
index bd9d9c4765d..55507b0aa63 100644
--- a/cmd/riscv/sbi.c
+++ b/cmd/riscv/sbi.c
@@ -54,6 +54,7 @@ static struct sbi_ext extensions[] = {
{ SBI_EXT_NACL,   "Nested Acceleration Extension" },
{ SBI_EXT_STA,"Steal-time Accounting Extension" 
},
{ SBI_EXT_DBTR,   "Debug Trigger Extension" },
+   { SBI_EXT_SSE,"Supervisor Software Events" },
 };
 
 static int do_sbi(struct cmd_tbl *cmdtp, int flag, int argc,
-- 
2.43.0

Re: [PATCH] ram: rockchip: px30: Replace misleading log message

[Quentin Schulz]

BTW,

On 4/17/24 13:16, lukasz.czechow...@thaumatec.com wrote:

From: Lukasz Czechowski 



There is a typo in your mail address. I assume your git config 
user.email may be wrong, because your signed-off-by is correct.


Cheers,
Quentin

Re: [PATCH] ram: rockchip: px30: Replace misleading log message

[Quentin Schulz]

Hi Lukasz,

On 4/17/24 13:16, lukasz.czechow...@thaumatec.com wrote:

From: Lukasz Czechowski 

Remove the log message "out" from sdram_init function which
pollutes the console. It brings no meaningful information and
might be unwanted in case silencing the console is required.
Instead, add a debug log with a more meaningful message, printed
only if DEBUG is set. The same convention is used for other
boards, i.e. rk3399.

Signed-off-by: Lukasz Czechowski 
---
  drivers/ram/rockchip/sdram_px30.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ram/rockchip/sdram_px30.c 
b/drivers/ram/rockchip/sdram_px30.c
index 21498e8957..3b429973cd 100644
--- a/drivers/ram/rockchip/sdram_px30.c
+++ b/drivers/ram/rockchip/sdram_px30.c
@@ -712,7 +712,7 @@ int sdram_init(void)
  
  	sdram_print_ddr_info(_params->ch.cap_info, _params->base, 0);
  
-	printascii("out\n");

+   debug("Finish SDRAM initialization...\n");


Mmmm I don't think this is appropriate. debug() is essentially replaced 
with printf() whenever the selected log level permits. The issue is that 
printf() != printascii() and I have a feeling we use printascii 
explicitly because it is a much smaller way to print data than printf.


Additionally, we're extremely size constrained in TPL on PX30, so 
increasing the string size to print may also not be wise. If we are not 
enable to even build the TPL by defining DEBUG, this is basically dead 
code and it's as good to us as removing it entirely.


So... questions:
1) does it actually build if you set #define DEBUG 1 or whatever is 
needed for having the debug message printed? If yes, does it boot?
2) What's the size of the TPL with this change and with this 
change+DEBUG set?


What I can suggest instead is to guard all printascii (or at least the 
ones only useful for debugging) with the appropriate symbol, e.g. could 
be CONFIG_RAM_ROCKCHIP_DEBUG.


I have not tested it (only build tested on Ringneck PX30 without 
CONFIG_RAM_ROCKCHIP_DEBUG nor CONFIG_TPL_SERIAL), but here's a quick 
attempt:


"""
diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index b7a6f100d41..17656e99a2f 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -11,7 +11,7 @@ config ROCKCHIP_PX30
select TPL_NEEDS_SEPARATE_STACK if TPL
imply SPL_SEPARATE_BSS
select SPL_SERIAL
-   select TPL_SERIAL
+   imply TPL_SERIAL
select DEBUG_UART_BOARD_INIT
imply ROCKCHIP_COMMON_BOARD
imply SPL_ROCKCHIP_COMMON_BOARD
diff --git a/arch/arm/mach-rockchip/px30-board-tpl.c 
b/arch/arm/mach-rockchip/px30-board-tpl.c

index 637a5e1b18b..8f56147ca7a 100644
--- a/arch/arm/mach-rockchip/px30-board-tpl.c
+++ b/arch/arm/mach-rockchip/px30-board-tpl.c
@@ -36,7 +36,7 @@ void board_init_f(ulong dummy)
 {
int ret;

-#ifdef CONFIG_DEBUG_UART
+#if defined(CONFIG_DEBUG_UART) && defined(CONFIG_TPL_SERIAL)
debug_uart_init();
/*
 * Debug UART can be used from here if required:
@@ -51,8 +51,10 @@ void board_init_f(ulong dummy)

secure_timer_init();
ret = sdram_init();
+#if defined(CONFIG_DEBUG_UART) && defined(CONFIG_TPL_SERIAL)
if (ret)
printascii("sdram_init failed\n");
+#endif

/* return to maskrom */
back_to_bootrom(BROM_BOOT_NEXTSTAGE);
diff --git a/drivers/ram/rockchip/Kconfig b/drivers/ram/rockchip/Kconfig
index 67c63ecba04..cb59bcf0414 100644
--- a/drivers/ram/rockchip/Kconfig
+++ b/drivers/ram/rockchip/Kconfig
@@ -16,6 +16,8 @@ if RAM_ROCKCHIP
 config RAM_ROCKCHIP_DEBUG
bool "Rockchip ram drivers debugging"
default y
+   depends on TPL_SERIAL if TPL_RAM
+   depends on SPL_SERIAL if SPL_RAM
help
  This enables debugging ram driver API's for the platforms
  based on Rockchip SoCs.
diff --git a/drivers/ram/rockchip/sdram_common.c 
b/drivers/ram/rockchip/sdram_common.c

index 60fc90d0a5c..8076cfa9aad 100644
--- a/drivers/ram/rockchip/sdram_common.c
+++ b/drivers/ram/rockchip/sdram_common.c
@@ -231,7 +231,9 @@ int sdram_detect_col(struct sdram_cap_info *cap_info,
break;
}
if (col == 8) {
+#ifdef CONFIG_RAM_ROCKCHIP_DEBUG
printascii("col error\n");
+#endif
return -1;
}

@@ -348,7 +350,9 @@ int sdram_detect_row(struct sdram_cap_info *cap_info,
break;
}
if (row == 12) {
+#ifdef CONFIG_RAM_ROCKCHIP_DEBUG
printascii("row error");
+#endif
return -1;
}

diff --git a/drivers/ram/rockchip/sdram_px30.c 
b/drivers/ram/rockchip/sdram_px30.c

index 21498e89570..607d97c268e 100644
--- a/drivers/ram/rockchip/sdram_px30.c
+++ b/drivers/ram/rockchip/sdram_px30.c
@@ -231,8 +231,10 @@ static unsigned int calculate_ddrconfig(struct 
px30_sdram_params *sdram_params)

ddrconf = i;
break;
  

Re: [PATCH v2 3/4] efi_loader: add an EFI variable with the file contents

[Ilias Apalodimas]

Hi Heinrich,

[...]

> >   {
> >   int ret = 0;
> >
> > diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> > index c8f7a88ba8db..99ad1f35d8f1 100644
> > --- a/lib/efi_loader/efi_runtime.c
> > +++ b/lib/efi_loader/efi_runtime.c
> > @@ -130,6 +130,8 @@ efi_status_t efi_init_runtime_supported(void)
> >   EFI_RT_SUPPORTED_CONVERT_POINTER;
> >
> >   if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
> > + int s = 0;
>
> u8 s = 0;
> should be enough.

Sure,

> >   }
> > + ret = efi_set_variable_int(u"VarToFile",
> > +_guid_efi_rt_var_file,
> > +EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > +EFI_VARIABLE_RUNTIME_ACCESS,
>
> Why is the variable set here? A comment would be helpful.

We need to set it so it shows up in the efivarfs, fo linux, and the
variable list in general for other OSes. Otherwise, people won't be
able to read it, unless they specifically search for it. I'll add a
comment though

> If you set it for protection, shouldn't it be EFI_VARIABLE_READ_ONLY?
>
> An alternative would be to return EFI_INVALID_PARAMETER in
> efi_set_variable_int for any attempt to set a variable with GUID
> efi_guid_efi_rt_var_file.

It's not volatile, so it will be treated as RO at runtime. But I'll
add EFI_VARIABLE_READ_ONLY as well. I prefer using the existing EFI
APIs instead of treating GUIDs specially

[...]

> > +/**
> > + * efi_var_collect_mem() - Copy EFI variables matching attributes mask from
> > + * efi_var_buf
> > + *
> > + * @buf: buffer containing variable collection
> > + * @lenp:buffer length
> > + * @mask:mask of matched attributes
> > + *
> > + * Return:   Status code
> > + */
> > +efi_status_t __efi_runtime
> > +efi_var_collect_mem(struct efi_var_file *buf, efi_uintn_t *lenp, u32 mask)
> > +{
> > + static struct efi_var_file __efi_runtime_data hdr = {
> > + .magic = EFI_VAR_FILE_MAGIC,
> > + };
> > + struct efi_var_entry *last, *var, *var_to;
> > +
> > + hdr.length = sizeof(struct efi_var_file);
> > +
> > + var = efi_var_buf->var;
> > + last = (struct efi_var_entry *)
> > +((uintptr_t)efi_var_buf + efi_var_buf->length);
> > + if (buf)
> > + var_to = buf->var;
> > +
> > + while (var < last) {
> > + u32 len;
> > + struct efi_var_entry *var_next;
> > +
> > + efi_var_mem_compare(var, NULL, u"", _next);
>
> On IRC you suggested to make u16_strlen __efi_runtime_code to replace
> this efi_var_mem_compare() call and avoid the modifications of said
> function. That might be more readable.

Yes it will, I'll change that on v3. That function will also help us
clean various open-coded sequences of length calculations.

FWIW I just managed to run FWTS over this -- and I also fixed
QueryVariableInfo at runtime
We had 22 tests passing and 1 failing. I am still trying to figure out
what failed, but I'll fix that as well for v3

>
> Best regards
>
> Heinrich
>
> > + len = (uintptr_t)var_next - (uintptr_t)var;
> > +
> > + if ((var->attr & mask) != mask) {

[...]

Thanks
/Ilias

Re: [PATCH v6 0/5] imx93: Conver to OF_UPSTREAM

[Fabio Estevam]

On Mon, Apr 15, 2024 at 9:55 AM Peng Fan  wrote:

> ok.
> Do I need to switch back to only convert i.MX93 11x11 EVK to
> OF_UPSTREM?

Yes, please convert only the imx93 evk board for now.

Currently, on the dts/upstream repo in U-Boot master, only the imx93
evk board is present.

The Variscite and Phytec i.MX93 boards are not there yet.

They will show up after the dts/upstream is synced with kernel 6.9.

Re: [PATCH v2 3/4] efi_loader: add an EFI variable with the file contents

[Heinrich Schuchardt]

On 17.04.24 12:19, Ilias Apalodimas wrote:

Previous patches enabled SetVariableRT using a RAM backend.
Although EBBR [0] defines a variable format we can teach userspace tools
and write the altered variables, it's better if we skip the ABI
requirements completely.

So let's add a new variable, in its own namespace called "VarToFile"
which contains a binary dump of the updated RT, BS and, NV variables
and will be updated when GetVariable is called.

Some adjustments are needed to do that.
Currently we discard BS-only variables in EBS(). We need to preserve
those on the RAM backend that exposes the variables. Since BS-only
variables can't appear at runtime we need to move the memory masking
checks from efi_var_collect() to efi_get_next_variable_name_mem()/
efi_get_variable_mem() and do the filtering at runtime.

We also need an efi_var_collect() variant available at runtime, in order
to construct the "VarToFile" buffer on the fly.

All users and applications (for linux) have to do when updating a variable
is dd that variable in the file described by "RTStorageVolatile".

Linux efivarfs uses a first 4 bytes of the output to represent attributes
in little-endian format. So, storing variables works like this:

$~ efibootmgr -n 0001
$~ dd 
if=/sys/firmware/efi/efivars/VarToFile-b2ac5fc9-92b7-4acd-aeac-11e818c3130c 
of=/boot/efi/ubootefi.var skip=4 bs=1

[0] 
https://arm-software.github.io/ebbr/index.html#document-chapter5-variable-storage

Co-developed-by: Heinrich Schuchardt 
Signed-off-by: Heinrich Schuchardt 
Signed-off-by: Ilias Apalodimas 
---
  include/efi_variable.h|  14 ++-
  lib/charset.c |   2 +-
  lib/efi_loader/efi_runtime.c  |  19 
  lib/efi_loader/efi_var_common.c   |   6 +-
  lib/efi_loader/efi_var_mem.c  | 146 ++
  lib/efi_loader/efi_variable.c |   6 +-
  lib/efi_loader/efi_variable_tee.c |   5 -
  7 files changed, 130 insertions(+), 68 deletions(-)

diff --git a/include/efi_variable.h b/include/efi_variable.h
index 42a2b7c52bef..b545a36aac50 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -271,13 +271,16 @@ const efi_guid_t *efi_auth_var_get_guid(const u16 *name);
   *
   * @variable_name_size:   size of variable_name buffer in bytes
   * @variable_name:name of uefi variable's name in u16
+ * @mask:  bitmask with required attributes of variables to be 
collected.
+ *  variables are only collected if all of the required
+ *  attributes match. Use 0 to skip matching
   * @vendor:   vendor's guid
   *
   * Return: status code
   */
  efi_status_t __efi_runtime
  efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, u16 
*variable_name,
-  efi_guid_t *vendor);
+  efi_guid_t *vendor, u32 mask);
  /**
   * efi_get_variable_mem() - Runtime common code across efi variable
   *  implementations for GetVariable() from
@@ -289,12 +292,15 @@ efi_get_next_variable_name_mem(efi_uintn_t 
*variable_name_size, u16 *variable_na
   * @data_size:size of the buffer to which the variable value 
is copied
   * @data: buffer to which the variable value is copied
   * @timep:authentication time (seconds since start of epoch)
+ * @mask:  bitmask with required attributes of variables to be 
collected.
+ *  variables are only collected if all of the required
+ *  attributes match. Use 0 to skip matching
   * Return:status code
   */
  efi_status_t __efi_runtime
  efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor,
 u32 *attributes, efi_uintn_t *data_size, void *data,
-u64 *timep);
+u64 *timep, u32 mask);
  
  /**

   * efi_get_variable_runtime() - runtime implementation of GetVariable()
@@ -334,4 +340,8 @@ efi_get_next_variable_name_runtime(efi_uintn_t 
*variable_name_size,
   */
  void efi_var_buf_update(struct efi_var_file *var_buf);
  
+efi_status_t __efi_runtime efi_var_collect_mem(struct efi_var_file *buf,

+  efi_uintn_t *lenp,
+  u32 check_attr_mask);
+
  #endif
diff --git a/lib/charset.c b/lib/charset.c
index df4f04074852..182c92a50c48 100644
--- a/lib/charset.c
+++ b/lib/charset.c
@@ -387,7 +387,7 @@ int u16_strcasecmp(const u16 *s1, const u16 *s2)
   *> 0 if the first different u16 in s1 is greater than the
   *corresponding u16 in s2
   */
-int u16_strncmp(const u16 *s1, const u16 *s2, size_t n)
+int __efi_runtime u16_strncmp(const u16 *s1, const u16 *s2, size_t n)
  {
int ret = 0;
  
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c

index c8f7a88ba8db..99ad1f35d8f1 100644
--- a/lib/efi_loader/efi_runtime.c
+++ 

Re: [PATCH v3] rockchip: px30-board-tpl: Sync ifdef guards with full TPL

[Quentin Schulz]

Hi Lukasz,

On 4/17/24 13:21, lukasz.czechow...@thaumatec.com wrote:

From: Lukasz Czechowski 

Display TPL init information message only when TPL_BANNER_PRINT
configuration entry is set. This allows to disable information
message in case logs on UART are unwanted.
Update parent ifdef condition to check also CONFIG_TPL_SERIAL
to match logic of the non-PX30 TPL implementation.

Signed-off-by: Lukasz Czechowski 



When someone gives a tag (Acked-by, Reviewed-by, Tested-by, ...) on a 
version and you send a new one, it's best to include this in the commit 
log now (above your Signed-off-by if I remember correctly) if and only 
if the content only changed a bit (like no big logic change or rewrite). 
So here, my Reviewed-by would have been nice since I gave it in the v2 
:) (please do not send a v4 for this :) )


b4 does this with `b4 trailers -u` automatically by the way :)

Reviewed-by: Quentin Schulz 

Thanks,
Quentin

[PATCH v1] arm: dts: verdin-imx8mm/imx8mp: use gpio-hog for sleep moci

[Stefan Eichenberger]

From: Stefan Eichenberger 

In Linux, we allow sleep moci to be turned off when the carrier board
supports it and the system is in suspend. In U-Boot, however, we want
the sleep moci to be always on. So we use a gpio hog and disable the
regulator. This change is necessary because we switched to upstream
device tree files with commit 23fe2def1edf
("verdin-imx8mm/verdin-imx8mp: move imx verdins to OF_UPSTREAM"). A
recent upstream patch removes the gpio hog from the Linux device tree,
so we need to add it to the u-boot dtsi. The following patch will remove
the gpio hog from the Linux device tree:
https://lore.kernel.org/linux-devicetree/20240405160720.5977-1-eich...@gmail.com/
The U-Boot patch can be applied without it and will not break the build.

Signed-off-by: Stefan Eichenberger 
---
 arch/arm/dts/imx8mm-verdin-wifi-dev-u-boot.dtsi | 5 +
 arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi | 4 
 2 files changed, 9 insertions(+)

diff --git a/arch/arm/dts/imx8mm-verdin-wifi-dev-u-boot.dtsi 
b/arch/arm/dts/imx8mm-verdin-wifi-dev-u-boot.dtsi
index 38db56059d..8b397f535c 100644
--- a/arch/arm/dts/imx8mm-verdin-wifi-dev-u-boot.dtsi
+++ b/arch/arm/dts/imx8mm-verdin-wifi-dev-u-boot.dtsi
@@ -60,6 +60,11 @@
 
ctrl-sleep-moci-hog {
bootph-pre-ram;
+   gpio-hog;
+   output-high;
+   gpios = <1 GPIO_ACTIVE_HIGH>;
+   line-name = "CTRL_SLEEP_MOCI#";
+
};
 };
 
diff --git a/arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi 
b/arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi
index 03f211d5f7..7b45a87450 100644
--- a/arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi
+++ b/arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi
@@ -58,6 +58,10 @@
 
ctrl-sleep-moci-hog {
bootph-pre-ram;
+   gpio-hog;
+   output-high;
+   gpios = <29 GPIO_ACTIVE_HIGH>;
+   line-name = "CTRL_SLEEP_MOCI#";
};
 };
 
-- 
2.40.1

[PATCH v1] arm: dts: verdin-am62: use gpio-hog for sleep moci

[Stefan Eichenberger]

From: Stefan Eichenberger 

In Linux, we allow sleep moci to be turned off when the carrier board
supports it and the system is in suspend. In U-Boot, however, we want
the sleep moci to be always on. So we use a gpio hog and disable the
regulator. This change is necessary because we switched to upstream
device tree files with commit c07bba7a2c7e ("verdin-am62: move verdin
am62 to OF_UPSTREAM"). A recent upstream patch removes the gpio hog from
the Linux device tree, so we need to add it to the u-boot dtsi. The
following patch will remove the gpio hog from the Linux device tree:
https://lore.kernel.org/linux-devicetree/20240301084901.16656-1-eich...@gmail.com/
The U-Boot patch can be applied without it and will not break the build.

Signed-off-by: Stefan Eichenberger 
---
 arch/arm/dts/k3-am625-verdin-wifi-dev-u-boot.dtsi | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/arm/dts/k3-am625-verdin-wifi-dev-u-boot.dtsi 
b/arch/arm/dts/k3-am625-verdin-wifi-dev-u-boot.dtsi
index 7fe7ae4154..9ecb305274 100644
--- a/arch/arm/dts/k3-am625-verdin-wifi-dev-u-boot.dtsi
+++ b/arch/arm/dts/k3-am625-verdin-wifi-dev-u-boot.dtsi
@@ -91,6 +91,14 @@
 
 _gpio0 {
bootph-all;
+
+   ctrl-sleep-moci-hog {
+   bootph-all;
+   gpio-hog;
+   gpios = <31 GPIO_ACTIVE_HIGH>;
+   line-name = "CTRL_SLEEP_MOCI#";
+   output-high;
+   };
 };
 
 /* On-module I2C - PMIC_I2C */
@@ -165,10 +173,6 @@
status = "disabled";
 };
 
-_ctrl_sleep_moci {
-   bootph-all;
-};
-
 /* Verdin UART_2 */
 _uart0 {
bootph-all;
-- 
2.40.1

[PATCH v1 1/1] net: Add drivers for Sysnopsys Ethernet 10G device

[Boon Khai Ng]

This driver support the Synopsys Designware Ethernet 10G
IP block refer from the driver dwc_eth_qos.

The driver MAC register mapping is different between
Synopsys QoS IP and Synopsys 10G IP, and thus new file
is created meant for Sysnopsys 10G IP.

The dwc_eth_xgmac_socfpga.c is specific to a device family,
the driver support the specific configuration used in
Intel SoC FPGA Agilex5.

This driver is extensible for other device family to use.

Signed-off-by: Boon Khai Ng 
---
 drivers/net/Kconfig |   18 +
 drivers/net/Makefile|2 +
 drivers/net/dwc_eth_xgmac.c | 1165 +++
 drivers/net/dwc_eth_xgmac.h |  298 +++
 drivers/net/dwc_eth_xgmac_socfpga.c |  226 ++
 5 files changed, 1709 insertions(+)
 create mode 100644 drivers/net/dwc_eth_xgmac.c
 create mode 100644 drivers/net/dwc_eth_xgmac.h
 create mode 100644 drivers/net/dwc_eth_xgmac_socfpga.c

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index b2d7b49976..b4ff033afa 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -193,6 +193,24 @@ config CALXEDA_XGMAC
  This driver supports the XGMAC in Calxeda Highbank and Midway
  machines.
 
+config DWC_ETH_XGMAC
+bool "Synopsys DWC Ethernet XGMAC device support"
+   select PHYLIB
+help
+  This driver supports the Synopsys Designware Ethernet XGMAC (10G
+  Ethernet MAC) IP block. The IP supports many options for bus type,
+  clocking/reset structure, and feature list.
+
+config DWC_ETH_XGMAC_SOCFPGA
+   bool "Synopsys DWC Ethernet XGMAC device support for SOCFPGA"
+   select REGMAP
+   select SYSCON
+   depends on DWC_ETH_XGMAC
+   default y if TARGET_SOCFPGA_AGILEX5
+   help
+ The Synopsys Designware Ethernet XGMAC IP block with specific
+ configuration used in Intel SoC FPGA chip.
+
 config DRIVER_DM9000
bool "Davicom DM9000 controller driver"
help
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 6677366ebd..823e317fcb 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -22,6 +22,8 @@ obj-$(CONFIG_DWC_ETH_QOS) += dwc_eth_qos.o
 obj-$(CONFIG_DWC_ETH_QOS_IMX) += dwc_eth_qos_imx.o
 obj-$(CONFIG_DWC_ETH_QOS_ROCKCHIP) += dwc_eth_qos_rockchip.o
 obj-$(CONFIG_DWC_ETH_QOS_QCOM) += dwc_eth_qos_qcom.o
+obj-$(CONFIG_DWC_ETH_XGMAC) += dwc_eth_xgmac.o
+obj-$(CONFIG_DWC_ETH_XGMAC_SOCFPGA) += dwc_eth_xgmac_socfpga.o
 obj-$(CONFIG_DWC_ETH_QOS_STARFIVE) += dwc_eth_qos_starfive.o
 obj-$(CONFIG_E1000) += e1000.o
 obj-$(CONFIG_E1000_SPI) += e1000_spi.o
diff --git a/drivers/net/dwc_eth_xgmac.c b/drivers/net/dwc_eth_xgmac.c
new file mode 100644
index 00..d3e5f9255f
--- /dev/null
+++ b/drivers/net/dwc_eth_xgmac.c
@@ -0,0 +1,1165 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023, Intel Corporation.
+ *
+ * Portions based on U-Boot's dwc_eth_qos.c.
+ */
+
+/*
+ * This driver supports the Synopsys Designware Ethernet XGMAC (10G Ethernet
+ * MAC) IP block. The IP supports multiple options for bus type, clocking/
+ * reset structure, and feature list.
+ *
+ * The driver is written such that generic core logic is kept separate from
+ * configuration-specific logic. Code that interacts with configuration-
+ * specific resources is split out into separate functions to avoid polluting
+ * common code. If/when this driver is enhanced to support multiple
+ * configurations, the core code should be adapted to call all configuration-
+ * specific functions through function pointers, with the definition of those
+ * function pointers being supplied by struct udevice_id xgmac_ids[]'s .data
+ * field.
+ *
+ * This configuration uses an AXI master/DMA bus, an AHB slave/register bus,
+ * contains the DMA, MTL, and MAC sub-blocks, and supports a single RGMII PHY.
+ * This configuration also has SW control over all clock and reset signals to
+ * the HW block.
+ */
+
+#define LOG_CATEGORY UCLASS_ETH
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "dwc_eth_xgmac.h"
+
+static void *xgmac_alloc_descs(struct xgmac_priv *xgmac, unsigned int num)
+{
+   return memalign(ARCH_DMA_MINALIGN, num * xgmac->desc_size);
+}
+
+static void xgmac_free_descs(void *descs)
+{
+   free(descs);
+}
+
+static struct xgmac_desc *xgmac_get_desc(struct xgmac_priv *xgmac,
+unsigned int num, bool rx)
+{
+   return (rx ? xgmac->rx_descs : xgmac->tx_descs) +
+  (num * xgmac->desc_size);
+}
+
+void xgmac_inval_desc_generic(void *desc)
+{
+   unsigned long start;
+   unsigned long end;
+
+   if (!desc) {
+   pr_err("%s invalid input buffer\n", __func__);
+   return;
+   }
+
+   start = (unsigned long)desc & ~(ARCH_DMA_MINALIGN - 1);
+   end = ALIGN(start + 

[PATCH v1 0/1] add driver for Synopsys Ethernet 10G device

[Boon Khai Ng]

This driver support the Synopsys Designware Ethernet 10G
IP block refer from the driver dwc_eth_qos.

The driver MAC register mapping is different between
Synopsys QoS IP and Synopsys 10G IP, and thus new file
is created meant for Sysnopsys 10G IP.

The dwc_eth_xgmac_socfpga.c is specific to a device family,
the driver support the specific configuration used in
Intel SoC FPGA Agilex5.

This driver is extensible for other device family to use.

Boon Khai Ng (1):
  net: Add drivers for Sysnopsys Ethernet 10G device

 drivers/net/Kconfig |   18 +
 drivers/net/Makefile|2 +
 drivers/net/dwc_eth_xgmac.c | 1165 +++
 drivers/net/dwc_eth_xgmac.h |  298 +++
 drivers/net/dwc_eth_xgmac_socfpga.c |  226 ++
 5 files changed, 1709 insertions(+)
 create mode 100644 drivers/net/dwc_eth_xgmac.c
 create mode 100644 drivers/net/dwc_eth_xgmac.h
 create mode 100644 drivers/net/dwc_eth_xgmac_socfpga.c

-- 
2.25.1

Re: [PATCH v2 5/5] common: Convert *.c/h from UTF-8 to ASCII enconfing

[Michal Simek]

On 4/16/24 18:19, Heinrich Schuchardt wrote:

On 16.04.24 18:06, Tom Rini wrote:

On Tue, Apr 16, 2024 at 08:55:19AM +0200, Michal Simek wrote:


Convert UTF-8 chars to ASCII in cases where make sense. No Copyright or
names are converted.

Signed-off-by: Michal Simek 



Reviewed-by: Tom Rini 

Now, how did you test / find these? Given names a CI test is unlikely
to be doable but if it's otherwise scriptable I can put it in my loops
and just fixup as needed (like I do today for adding  for
example).



There seem no to be too many non-ASCI strings outside of comments.
Should we care about non-ASCII comments?

$ find . -name '*.h' -exec grep -P -Hn "[^\x00-\x7F]" {} \; | grep -v
':\s*[\/\*']
./include/configs/tec-ng.h:13:#define CFG_TEGRA_BOARD_STRING    "Avionic
Design Tamonten™ NG Evaluation Carrier"
./arch/mips/mach-octeon/include/mach/cvmx-pko3.h:369:   MEMALG_SUB = 9,
  /* mem = mem – PKO_SEND_MEM_S[OFFSET] */

$ find . -name '*.c' -exec grep -P -Hn "[^\x00-\x7F]" {} \; | grep -v
':\s*[\/\*']
./drivers/mtd/nand/raw/nand_ids.c:65:   {"H27QCG8T2E5R‐BCF 64G 3.3V 8-bit",
./drivers/video/dw_mipi_dsi.c:861:MODULE_AUTHOR("Yannick Fertré
");
./board/bosch/acc/acc.c:440:    .SRT = 0, // Set to 1 for temperatures
above 85°C
./cmd/2048.c:65:    printf("   ·   ");
./cmd/2048.c:79:    printf("    ←, ↑, →, ↓ or q    \n");


I actually use more force way and simply run uni2ascii -B < file to all files 
and then look at git diff and pick what it is valid.

Above find is not able to find all that strings.

For example this one could be also fixed which above filter it not able to find.

--- a/include/mtd/ubi-user.h
+++ b/include/mtd/ubi-user.h
@@ -1,8 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0+ */
 /*
- * Copyright © International Business Machines Corp., 2006
+ * Copyright (c) International Business Machines Corp., 2006

M

Re: [PATCH v2 2/4] efi_loader: Add OS notifications for SetVariable at runtime

[Heinrich Schuchardt]

On 17.04.24 12:19, Ilias Apalodimas wrote:

Previous patches enable SetVariable at runtime using a volatile storage
backend using EFI_RUNTIME_SERVICES_DATA allocared memory. Since there's
no recommendation from the spec on how to notify the OS, add a volatile
EFI variable that contains the filename relative to the ESP. OS'es
can use that file and update it at runtime

$~ efivar -p -n b2ac5fc9-92b7-4acd-aeac-11e818c3130c-RTStorageVolatile
GUID: b2ac5fc9-92b7-4acd-aeac-11e818c3130c
Name: "RTStorageVolatile"
Attributes:
Boot Service Access
Runtime Service Access
Value:
  75 62 6f 6f 74 65 66 69  2e 76 61 72 00   |ubootefi.var.   |

Signed-off-by: Ilias Apalodimas 


Reviewed-by: Heinrich Schuchardt 


---
  include/efi_loader.h |  4 
  lib/efi_loader/efi_runtime.c | 19 ---
  2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index bb51c0281774..69442f4e58de 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -159,6 +159,10 @@ static inline void efi_set_bootdev(const char *dev, const 
char *devnr,
  #define EFICONFIG_AUTO_GENERATED_ENTRY_GUID \
EFI_GUID(0x8108ac4e, 0x9f11, 0x4d59, \
 0x85, 0x0e, 0xe2, 0x1a, 0x52, 0x2c, 0x59, 0xb2)
+#define U_BOOT_EFI_RT_VAR_FILE_GUID \
+   EFI_GUID(0xb2ac5fc9, 0x92b7, 0x4acd, \
+0xae, 0xac, 0x11, 0xe8, 0x18, 0xc3, 0x13, 0x0c)
+
  
  /* Use internal device tree when starting UEFI application */

  #define EFI_FDT_USE_INTERNAL NULL
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index dde083b09665..c8f7a88ba8db 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -10,6 +10,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -110,6 +111,7 @@ static __efi_runtime_data efi_uintn_t efi_descriptor_size;
   */
  efi_status_t efi_init_runtime_supported(void)
  {
+   const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID;
efi_status_t ret;
struct efi_rt_properties_table *rt_table;
  
@@ -127,9 +129,20 @@ efi_status_t efi_init_runtime_supported(void)

EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
EFI_RT_SUPPORTED_CONVERT_POINTER;
  
-	if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE))

-   rt_table->runtime_services_supported |=
-   EFI_RT_SUPPORTED_SET_VARIABLE;
+   if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
+   ret = efi_set_variable_int(u"RTStorageVolatile",
+  _guid_efi_rt_var_file,
+  EFI_VARIABLE_BOOTSERVICE_ACCESS |
+  EFI_VARIABLE_RUNTIME_ACCESS |
+  EFI_VARIABLE_READ_ONLY,
+  sizeof(EFI_VAR_FILE_NAME),
+  EFI_VAR_FILE_NAME, false);
+   if (ret != EFI_SUCCESS) {
+   log_err("Failed to set RTStorageVolatile\n");
+   return ret;
+   }
+   rt_table->runtime_services_supported |= 
EFI_RT_SUPPORTED_SET_VARIABLE;
+   }
  
  	/*

 * This value must be synced with efi_runtime_detach_list

Re: [PATCH v2 1/4] efi_loader: conditionally enable SetvariableRT

[Ilias Apalodimas]

On Wed, 17 Apr 2024 at 15:28, Heinrich Schuchardt
 wrote:
>
> On 17.04.24 12:19, Ilias Apalodimas wrote:
> > When we store EFI variables on file we don't allow SetVariable at runtime,
> > since the OS doesn't know how to access or write that file.  At the same
> > time keeping the U-Boot drivers alive in runtime sections and performing
> > writes from the firmware is dangerous -- if at all possible.
> >
> > For GetVariable at runtime we copy runtime variables in RAM and expose them
> > to the OS. Add a Kconfig option and provide SetVariable at runtime using
> > the same memory backend. The OS will be responsible for syncing the RAM
> > contents to the file, otherwise any changes made during runtime won't
> > persist reboots.
> >
> > It's worth noting that the variable store format is defined in EBBR [0]
> > and authenticated variables are explicitly prohibited, since they have
> > to be stored on a medium that's tamper and rollback protected.
> >
> > - pre-patch
> > $~ mount | grep efiva
> > efivarfs on /sys/firmware/efi/efivars type efivarfs 
> > (ro,nosuid,nodev,noexec,relatime)
> >
> > $~ efibootmgr -n 0001
> > Could not set BootNext: Read-only file system
> >
> > - post-patch
> > $~ mount | grep efiva
> > efivarfs on /sys/firmware/efi/efivars type efivarfs 
> > (rw,nosuid,nodev,noexec,relatime)
> >
> > $~ efibootmgr -n 0001
> > BootNext: 0001
> > BootCurrent: 
> > BootOrder: ,0001
> > Boot* debian
> > HD(1,GPT,bdae5610-3331-4e4d-9466-acb5caf0b4a6,0x800,0x10)/File(EFI\debian\grubaa64.efi)
> > Boot0001* virtio 0  
> > VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,85001f00)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,16008500){auto_created_boot_option}
> >
> > $~ efivar -p -n 8be4df61-93ca-11d2-aa0d-00e098032b8c-BootNext
> > GUID: 8be4df61-93ca-11d2-aa0d-00e098032b8c
> > Name: "BootNext"
> > Attributes:
> >  Non-Volatile
> >  Boot Service Access
> >  Runtime Service Access
> > Value:
> >   01 00
> >
> > [0] 
> > https://arm-software.github.io/ebbr/index.html#document-chapter5-variable-storage
> >
> > Signed-off-by: Ilias Apalodimas 
> > ---
> >   lib/efi_loader/Kconfig|  16 +++
> >   lib/efi_loader/efi_runtime.c  |   4 +
> >   lib/efi_loader/efi_variable.c | 115 --
> >   .../efi_selftest_variables_runtime.c  |  13 +-
> >   4 files changed, 134 insertions(+), 14 deletions(-)
> >
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index e13a6f9f4c3a..cc8371a3bb4c 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -62,6 +62,22 @@ config EFI_VARIABLE_FILE_STORE
> > Select this option if you want non-volatile UEFI variables to be
> > stored as file /ubootefi.var on the EFI system partition.
> >
> > +config EFI_RT_VOLATILE_STORE
> > + bool "Allow variable runtime services in volatile storage (e.g RAM)"
> > + depends on EFI_VARIABLE_FILE_STORE
> > + help
> > +   When EFI variables are stored on file we don't allow SetVariableRT,
> > +   since the OS doesn't know how to write that file. At he same time
> > +   we copy runtime variables in DRAM and support GetVariableRT
> > +
> > +   Enable this option to allow SetVariableRT on the RAM backend of
> > +   the EFI variable storage. The OS will be responsible for syncing
> > +   the RAM contents to the file, otherwise any changes made during
> > +   runtime won't persist reboots.
> > +   Authenticated variables are not supported. Note that this will
> > +   violate the EFI spec since writing auth variables will return
> > +   EFI_INVALID_PARAMETER
> > +
> >   config EFI_MM_COMM_TEE
> >   bool "UEFI variables storage service via the trusted world"
> >   depends on OPTEE
> > diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> > index a61c9a77b13f..dde083b09665 100644
> > --- a/lib/efi_loader/efi_runtime.c
> > +++ b/lib/efi_loader/efi_runtime.c
> > @@ -127,6 +127,10 @@ efi_status_t efi_init_runtime_supported(void)
> >   EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
> >   EFI_RT_SUPPORTED_CONVERT_POINTER;
> >
> > + if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE))
> > + rt_table->runtime_services_supported |=
> > + EFI_RT_SUPPORTED_SET_VARIABLE;
> > +
> >   /*
> >* This value must be synced with efi_runtime_detach_list
> >* as well as efi_runtime_services.
> > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> > index e6c1219a11c8..abc2a3402f42 100644
> > --- a/lib/efi_loader/efi_variable.c
> > +++ b/lib/efi_loader/efi_variable.c
> > @@ -219,17 +219,20 @@ efi_get_next_variable_name_int(efi_uintn_t 
> > *variable_name_size,
> >   return efi_get_next_variable_name_mem(variable_name_size, 

Re: [PATCH v2 4/4] efi_selftest: add tests for setvariableRT

[Ilias Apalodimas]

Hi Heinrich,


[...]

> >
> > + memset(v2, 0x1, sizeof(v2));
> >   ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS,
> >  _storage, _storage,
> >  _size);
> > @@ -63,10 +69,107 @@ static int execute(void)
> >   EFI_VARIABLE_RUNTIME_ACCESS,
> >   3, v + 4);
> >   if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
> > + efi_uintn_t prev_len, delta;
> > +
> >   if (ret != EFI_INVALID_PARAMETER) {
> >   efi_st_error("SetVariable failed\n");
> >   return EFI_ST_FAILURE;
> >   }
> > +
> > + len = sizeof(data);
> > + ret = runtime->get_variable(u"RTStorageVolatile",
> > + _rt_var_guid,
> > + , , data);
> > + if (ret != EFI_SUCCESS) {
> > + efi_st_error("GetVariable failed\n");
> > + return EFI_ST_FAILURE;
> > + }
> > +
> > + len = sizeof(data2);
> > + ret = runtime->get_variable(u"VarToFile", _rt_var_guid,
> > + , , data2);
> > + if (ret != EFI_SUCCESS) {
> > + efi_st_error("GetVariable failed\n");
> > + return EFI_ST_FAILURE;
> > + }
> > + /*
> > +  * VarToFile will size must change once a variable is inserted
> > +  * Store it now, we'll use it later
> > +  */
> > + prev_len = len;
> > + ret = runtime->set_variable(u"efi_st_var0", _vendor0,
> > + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > + EFI_VARIABLE_RUNTIME_ACCESS |
> > + EFI_VARIABLE_NON_VOLATILE,
> > + sizeof(v2),
> > + v2);
> > + /*
> > +  * This will try to update VarToFile as well and must fail,
> > +  * without changing or deleting VarToFile
> > +  */
> > + if (ret != EFI_OUT_OF_RESOURCES) {
> > + efi_st_error("SetVariable failed\n");
> > + return EFI_ST_FAILURE;
> > + }
> > + len = sizeof(data2);
> > + ret = runtime->get_variable(u"VarToFile", _rt_var_guid,
> > + , , data2);
> > + if (ret != EFI_SUCCESS || prev_len != len) {
> > + efi_st_error("Get/SetVariable failed\n");
> > + return EFI_ST_FAILURE;
> > + }
> > +
> > + /* SetVariableRT updates VarToFile with efi_st_var0 */
> > + ret = runtime->set_variable(u"efi_st_var0", _vendor0,
> > + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > + EFI_VARIABLE_RUNTIME_ACCESS |
> > + EFI_VARIABLE_NON_VOLATILE,
> > + sizeof(v), v);
>
> It is fine to test with variable name size (24) and variable value size
> (16) both being multiples of 8. But this does not cover the generic case.

We already test SetVariable with a non-aligned variable at the start.
I'll keep this as is and add append 1byte at the last test as you suggested.

>
> > + if (ret != EFI_SUCCESS) {
> > + efi_st_error("SetVariable failed\n");
> > + return EFI_ST_FAILURE;
> > + }
> > + len = sizeof(data2);
> > + delta = 2 * (u16_strlen(u"efi_st_var0") + 1) + sizeof(v) +
> > + sizeof(struct efi_var_entry);
>
> In the file all variable are stored at 8 byte aligned positions.
> I am missing ALIGN(,8) here.

Ah yes

>
> > + ret = runtime->get_variable(u"VarToFile", _rt_var_guid,
> > + , , data2);
> > + if (ret != EFI_SUCCESS || prev_len + delta != len) {
> > + efi_st_error("Get/SetVariable failed\n");
> > + return EFI_ST_FAILURE;
> > + }
>
> We should check the header fields of VarToFile (reserved, magic, length,
> crc32), e.g.
>
> struct efi_var_file *hdr = (void *)data2;
> if (memcmp(hdr->magic, EFI_VAR_FILE_MAGIC, 8) ||
>  len != ALIGN(hdr->length + sizeof(hdr), 8) ||
>  ... )

Sure, good idea,

>
> > +
> > + /* append on an existing variable will updateVarToFile */
> > + ret = runtime->set_variable(u"efi_st_var0", _vendor0,
> > + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > + EFI_VARIABLE_RUNTIME_ACCESS |
> > +   

Re: [PATCH v2 1/4] efi_loader: conditionally enable SetvariableRT

[Heinrich Schuchardt]

On 17.04.24 12:19, Ilias Apalodimas wrote:

When we store EFI variables on file we don't allow SetVariable at runtime,
since the OS doesn't know how to access or write that file.  At the same
time keeping the U-Boot drivers alive in runtime sections and performing
writes from the firmware is dangerous -- if at all possible.

For GetVariable at runtime we copy runtime variables in RAM and expose them
to the OS. Add a Kconfig option and provide SetVariable at runtime using
the same memory backend. The OS will be responsible for syncing the RAM
contents to the file, otherwise any changes made during runtime won't
persist reboots.

It's worth noting that the variable store format is defined in EBBR [0]
and authenticated variables are explicitly prohibited, since they have
to be stored on a medium that's tamper and rollback protected.

- pre-patch
$~ mount | grep efiva
efivarfs on /sys/firmware/efi/efivars type efivarfs 
(ro,nosuid,nodev,noexec,relatime)

$~ efibootmgr -n 0001
Could not set BootNext: Read-only file system

- post-patch
$~ mount | grep efiva
efivarfs on /sys/firmware/efi/efivars type efivarfs 
(rw,nosuid,nodev,noexec,relatime)

$~ efibootmgr -n 0001
BootNext: 0001
BootCurrent: 
BootOrder: ,0001
Boot* debian
HD(1,GPT,bdae5610-3331-4e4d-9466-acb5caf0b4a6,0x800,0x10)/File(EFI\debian\grubaa64.efi)
Boot0001* virtio 0  
VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,85001f00)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,16008500){auto_created_boot_option}

$~ efivar -p -n 8be4df61-93ca-11d2-aa0d-00e098032b8c-BootNext
GUID: 8be4df61-93ca-11d2-aa0d-00e098032b8c
Name: "BootNext"
Attributes:
 Non-Volatile
 Boot Service Access
 Runtime Service Access
Value:
  01 00

[0] 
https://arm-software.github.io/ebbr/index.html#document-chapter5-variable-storage

Signed-off-by: Ilias Apalodimas 
---
  lib/efi_loader/Kconfig|  16 +++
  lib/efi_loader/efi_runtime.c  |   4 +
  lib/efi_loader/efi_variable.c | 115 --
  .../efi_selftest_variables_runtime.c  |  13 +-
  4 files changed, 134 insertions(+), 14 deletions(-)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index e13a6f9f4c3a..cc8371a3bb4c 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -62,6 +62,22 @@ config EFI_VARIABLE_FILE_STORE
  Select this option if you want non-volatile UEFI variables to be
  stored as file /ubootefi.var on the EFI system partition.
  
+config EFI_RT_VOLATILE_STORE

+   bool "Allow variable runtime services in volatile storage (e.g RAM)"
+   depends on EFI_VARIABLE_FILE_STORE
+   help
+ When EFI variables are stored on file we don't allow SetVariableRT,
+ since the OS doesn't know how to write that file. At he same time
+ we copy runtime variables in DRAM and support GetVariableRT
+
+ Enable this option to allow SetVariableRT on the RAM backend of
+ the EFI variable storage. The OS will be responsible for syncing
+ the RAM contents to the file, otherwise any changes made during
+ runtime won't persist reboots.
+ Authenticated variables are not supported. Note that this will
+ violate the EFI spec since writing auth variables will return
+ EFI_INVALID_PARAMETER
+
  config EFI_MM_COMM_TEE
bool "UEFI variables storage service via the trusted world"
depends on OPTEE
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index a61c9a77b13f..dde083b09665 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -127,6 +127,10 @@ efi_status_t efi_init_runtime_supported(void)
EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
EFI_RT_SUPPORTED_CONVERT_POINTER;
  
+	if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE))

+   rt_table->runtime_services_supported |=
+   EFI_RT_SUPPORTED_SET_VARIABLE;
+
/*
 * This value must be synced with efi_runtime_detach_list
 * as well as efi_runtime_services.
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index e6c1219a11c8..abc2a3402f42 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -219,17 +219,20 @@ efi_get_next_variable_name_int(efi_uintn_t 
*variable_name_size,
return efi_get_next_variable_name_mem(variable_name_size, 
variable_name, vendor);
  }
  
-efi_status_t efi_set_variable_int(const u16 *variable_name,

- const efi_guid_t *vendor,
- u32 attributes, efi_uintn_t data_size,
- const void *data, bool ro_check)
+/**
+ * setvariable_allowed() - checks defined by the UEFI spec for setvariable
+ *
+ * @variable_name: name of the variable

Re: [PATCH v2 4/4] efi_selftest: add tests for setvariableRT

[Heinrich Schuchardt]

On 17.04.24 12:19, Ilias Apalodimas wrote:

Since we support SetVariableRT now add the relevant tests

- Search for the RTStorageVolatile and VarToFile variables after EBS
- Try to update with invalid variales (BS, RT only)
- Try to write a variable bigger than our backend storage
- Write a variable that fits and check VarToFile has been updated
   correclty
- Append to the variable and check VarToFile changes
- Try to delete VarToFile which is write protected

Signed-off-by: Ilias Apalodimas 
---
  .../efi_selftest_variables_runtime.c  | 103 ++
  1 file changed, 103 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c 
b/lib/efi_selftest/efi_selftest_variables_runtime.c
index 4c9405c0a7c7..e492b50a43c2 100644
--- a/lib/efi_selftest/efi_selftest_variables_runtime.c
+++ b/lib/efi_selftest/efi_selftest_variables_runtime.c
@@ -10,6 +10,7 @@
   */

  #include 
+#include 

  #define EFI_ST_MAX_DATA_SIZE 16
  #define EFI_ST_MAX_VARNAME_SIZE 40
@@ -17,6 +18,8 @@
  static struct efi_boot_services *boottime;
  static struct efi_runtime_services *runtime;
  static const efi_guid_t guid_vendor0 = EFI_GLOBAL_VARIABLE_GUID;
+static const efi_guid_t __efi_runtime_data efi_rt_var_guid =
+   U_BOOT_EFI_RT_VAR_FILE_GUID;

  /*
   * Setup unit test.
@@ -45,11 +48,14 @@ static int execute(void)
u32 attr;
u8 v[16] = {0x5d, 0xd1, 0x5e, 0x51, 0x5a, 0x05, 0xc7, 0x0c,
0x35, 0x4a, 0xae, 0x87, 0xa5, 0xdf, 0x0f, 0x65,};
+   u8 v2[CONFIG_EFI_VAR_BUF_SIZE];
u8 data[EFI_ST_MAX_DATA_SIZE];
+   u8 data2[CONFIG_EFI_VAR_BUF_SIZE];
u16 varname[EFI_ST_MAX_VARNAME_SIZE];
efi_guid_t guid;
u64 max_storage, rem_storage, max_size;

+   memset(v2, 0x1, sizeof(v2));
ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS,
   _storage, _storage,
   _size);
@@ -63,10 +69,107 @@ static int execute(void)
EFI_VARIABLE_RUNTIME_ACCESS,
3, v + 4);
if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
+   efi_uintn_t prev_len, delta;
+
if (ret != EFI_INVALID_PARAMETER) {
efi_st_error("SetVariable failed\n");
return EFI_ST_FAILURE;
}
+
+   len = sizeof(data);
+   ret = runtime->get_variable(u"RTStorageVolatile",
+   _rt_var_guid,
+   , , data);
+   if (ret != EFI_SUCCESS) {
+   efi_st_error("GetVariable failed\n");
+   return EFI_ST_FAILURE;
+   }
+
+   len = sizeof(data2);
+   ret = runtime->get_variable(u"VarToFile", _rt_var_guid,
+   , , data2);
+   if (ret != EFI_SUCCESS) {
+   efi_st_error("GetVariable failed\n");
+   return EFI_ST_FAILURE;
+   }
+   /*
+* VarToFile will size must change once a variable is inserted
+* Store it now, we'll use it later
+*/
+   prev_len = len;
+   ret = runtime->set_variable(u"efi_st_var0", _vendor0,
+   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+   EFI_VARIABLE_RUNTIME_ACCESS |
+   EFI_VARIABLE_NON_VOLATILE,
+   sizeof(v2),
+   v2);
+   /*
+* This will try to update VarToFile as well and must fail,
+* without changing or deleting VarToFile
+*/
+   if (ret != EFI_OUT_OF_RESOURCES) {
+   efi_st_error("SetVariable failed\n");
+   return EFI_ST_FAILURE;
+   }
+   len = sizeof(data2);
+   ret = runtime->get_variable(u"VarToFile", _rt_var_guid,
+   , , data2);
+   if (ret != EFI_SUCCESS || prev_len != len) {
+   efi_st_error("Get/SetVariable failed\n");
+   return EFI_ST_FAILURE;
+   }
+
+   /* SetVariableRT updates VarToFile with efi_st_var0 */
+   ret = runtime->set_variable(u"efi_st_var0", _vendor0,
+   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+   EFI_VARIABLE_RUNTIME_ACCESS |
+   EFI_VARIABLE_NON_VOLATILE,
+   sizeof(v), v);


It is fine to test with variable name size (24) and variable value size
(16) both 

Re: [PATCH 01/10] board: ti: am62x: Init DRAM size in R5/A53 SPL

[Sughosh Ganu]

hi Chintan,

On Wed, 17 Apr 2024 at 13:21, Chintan Vankar  wrote:
>
>
>
> On 16/04/24 22:30, Tom Rini wrote:
> > On Tue, Apr 16, 2024 at 05:52:58PM +0530, Chintan Vankar wrote:
> >>
> >>
> >> On 12/04/24 03:37, Tom Rini wrote:
> >>> On Wed, Apr 03, 2024 at 06:18:01PM +0530, Chintan Vankar wrote:
> 
> 
>  On 22/01/24 10:11, Siddharth Vadapalli wrote:
> >
> >
> > On 20/01/24 22:11, Tom Rini wrote:
> >> On Mon, Jan 15, 2024 at 01:42:51PM +0530, Siddharth Vadapalli wrote:
> >>> Hello Tom,
> >>>
> >>> On 12/01/24 18:56, Tom Rini wrote:
> >
> > ...
> >
>  The list of conditionals in common/spl/spl.c::board_init_r() should 
>  be
>  updated and probably use SPL_NET as the option to check for.
> >>>
> >>> Thank you for reviewing the patch and pointing this out. I wasn't 
> >>> aware of it. I
> >>> assume that you are referring to the following change:
> >>>
> >>>if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || 
> >>> CONFIG_IS_ENABLED(HANDOFF) ||
> >>> -   IS_ENABLED(CONFIG_SPL_ATF))
> >>> +   IS_ENABLED(CONFIG_SPL_ATF) || IS_ENABLED(CONFIG_SPL_NET))
> >>>dram_init_banksize();
> >>>
> >>> I shall replace the current patch with the above change in the v2 
> >>> series. Since
> >>> this is in the common section, is there a generic reason I could 
> >>> provide in the
> >>> commit message rather than the existing commit message which seems to 
> >>> be board
> >>> specific? Also, I hope that the above change will not cause 
> >>> regressions for
> >>> other non-TI devices. Please let me know.
> >>
> >> Yes, that's the area, and just note that networking also requires the
> >> DDR to be initialized.
> >>
> >
> > Thank you for confirming and providing your suggestion for the contents 
> > of the
> > commit message.
> >
>  Following Tom's Suggestion of adding CONFIG_SPL_NET in common/spl/spl.c
>  "dram_init_banksize()", the issue of fetching a file at SPL stage seemed
>  to be fixed. However the commit "ba20b2443c29", which sets gd->ram_top
>  for the very first time in "spl_enable_cache()" results in
>  "arch_lmb_reserve()" function reserving memory region from Stack pointer
>  at "0x81FFB820" to gd->ram_top pointing to "0x1". Previously
>  when gd->ram_top was zero "arch_lmb_reserve()" was noop. Now using TFTP
>  to fetch U-Boot image at SPL stage results in "tftp_init_load_addr()"
>  function call that invokes "arch_lmb_reserve()" function, which reserves
>  entire memory starting from Stack Pointer to gd->ram_top leaving no
>  space to load U-Boot image via TFTP since TFTP loads files at pre
>  configured memory address at "0x8200".
> 
>  As a workaround for this issue, one solution we can propose is to
>  disable the checks "lmb_get_free_size()" at SPL and U-Boot stage. For
>  that we can define a new config option for LMB reserve checks as
>  "SPL_LMB". This config will be enable by default for the backword
>  compatibility and disable for our use case at SPL and U-Boot stage.
> >>>
> >>> The problem here is that we need LMB for booting an OS, which is
> >>> something we'll want in SPL in non-cortex-R cases too, which means this
> >>> platform, so that's a no-go. I think you need to dig harder and see if
> >>> you can correct the logic somewhere so that we don't over reserve?
> >>>
> >> Since this issue is due to function call "lmb_init_and_reserve()"
> >> function invoked from "tftp_init_load_addr()" function. This function
> >> is defined by Simon in commit "a156c47e39ad", which fixes
> >> "CVE-2018-18439" to prevent overwriting reserved memory. Simon, can you
> >> explain why do we need to call "lmb_init_and_reserve()" function here ?
> >
> > This is indeed a tricky area which is why Sughosh is looking in to
> > trying to re-work the LMB mechanic and we've had a few long threads
> > about it as well.
> >
> > I've honestly forgotten the use case you have here, can you please
> > remind us?
> >
> We are trying to boot AM62x using Ethernet for which we need to load
> binary files at SPL and U-Boot stage using TFTP. To store the file we
> need a free memory in RAM, specifically we are storing these files at
> 0x8200. But we are facing an issue while loading the file since
> the memory area having an address 0x8200 is reserved due to
> "lmb_init_and_reserve()" function call. This function is called in
> "tftp_init_load_addr()" function which is getting called exactly before
> we are trying to get the free memory area by calling
> "lmb_get_free_size()".

I have no idea about your platform but I was wondering if there is any
particular importance of the load address of 0x8200? It looks as
though the current location of the SP when arch_lmb_reserve() gets
called means that the load address is 

RE: [PATCH] mmc: cv1800b: Add transmit tap delay config to fix write error

[Jaehoon Chung]

> -Original Message-
> From: Kongyang Liu 
> Sent: Tuesday, April 16, 2024 4:31 PM
> To: u-boot@lists.denx.de
> Cc: Jaehoon Chung ; Leo Yu-Chi Liang 
> ; Peng Fan
> ; Tom Rini 
> Subject: [PATCH] mmc: cv1800b: Add transmit tap delay config to fix write 
> error
> 
> Currently, only the receive delay is configured while the transmit delay
> is not set, which may result in errors when writing to the file. This issue
> can be resolved by setting PHY_TX_SRC_INVERT to SDHCI_PHY_TX_RX_DLY.
> 
> Signed-off-by: Kongyang Liu 

Reviewed-by: Jaehoon Chung 

Best Regards,
Jaehoon Chung

> ---
> 
>  drivers/mmc/cv1800b_sdhci.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/cv1800b_sdhci.c b/drivers/mmc/cv1800b_sdhci.c
> index 2275c53777..a50f4cff0d 100644
> --- a/drivers/mmc/cv1800b_sdhci.c
> +++ b/drivers/mmc/cv1800b_sdhci.c
> @@ -12,6 +12,8 @@
>  #define MMC_MAX_CLOCK37500
>  #define TUNE_MAX_PHCODE  128
> 
> +#define PHY_TX_SRC_INVERT  BIT(8)
> +
>  struct cv1800b_sdhci_plat {
>   struct mmc_config cfg;
>   struct mmc mmc;
> @@ -19,7 +21,7 @@ struct cv1800b_sdhci_plat {
> 
>  static void cv1800b_set_tap_delay(struct sdhci_host *host, u16 tap)
>  {
> - sdhci_writel(host, tap << 16, SDHCI_PHY_TX_RX_DLY);
> + sdhci_writel(host, PHY_TX_SRC_INVERT | tap << 16, SDHCI_PHY_TX_RX_DLY);
>  }
> 
>  static void cv1800b_sdhci_reset(struct sdhci_host *host, u8 mask)
> --
> 2.41.0

RE: [PATCH 4/5] mmc: am654_sdhci: Set ENDLL=1 for DDR52 mode

[Jaehoon Chung]

> -Original Message-
> From: Judith Mendez 
> Sent: Tuesday, April 16, 2024 6:28 AM
> To: Peng Fan ; Jaehoon Chung ; Tom 
> Rini 
> Cc: Nitin Yadav ; Simon Glass ; 
> u-boot@lists.denx.de
> Subject: [PATCH 4/5] mmc: am654_sdhci: Set ENDLL=1 for DDR52 mode
> 
> According to the device datasheet [0], ENDLL=1 for
> DDR52 mode, so call am654_sdhci_setup_dll() and write
> itapdly after since we do not carry out tuning.
> 
> [0] https://www.ti.com/lit/ds/symlink/am62p.pdf
> Fixes: c964447ea3d6 ("mmc: am654_sdhci: Add support for input tap delay")
> Signed-off-by: Judith Mendez 

Reviewed-by: Jaehoon Chung 

Best Regards,
Jaehoon Chung

> ---
>  drivers/mmc/am654_sdhci.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c
> index 38f1ad28ec4..dee56dfdbaa 100644
> --- a/drivers/mmc/am654_sdhci.c
> +++ b/drivers/mmc/am654_sdhci.c
> @@ -287,12 +287,14 @@ static int am654_sdhci_set_ios_post(struct sdhci_host 
> *host)
> 
>   regmap_update_bits(plat->base, PHY_CTRL4, mask, val);
> 
> - if (mode > UHS_SDR25 && speed >= CLOCK_TOO_SLOW_HZ) {
> + if ((mode > UHS_SDR25 || mode == MMC_DDR_52) && speed >= 
> CLOCK_TOO_SLOW_HZ) {
>   ret = am654_sdhci_setup_dll(plat, speed);
>   if (ret)
>   return ret;
> 
>   plat->dll_enable = true;
> + am654_sdhci_write_itapdly(plat, plat->itap_del_sel[mode],
> +   plat->itap_del_ena[mode]);
>   } else {
>   am654_sdhci_setup_delay_chain(plat, mode);
>   plat->dll_enable = false;
> --
> 2.43.2

RE: [PATCH 3/5] mmc: am654_sdhci: Add itap_del_ena[] to store itapdlyena bit

[Jaehoon Chung]

Hi,

> -Original Message-
> From: Judith Mendez 
> Sent: Tuesday, April 16, 2024 6:28 AM
> To: Peng Fan ; Jaehoon Chung ; Tom 
> Rini 
> Cc: Nitin Yadav ; Simon Glass ; 
> u-boot@lists.denx.de
> Subject: [PATCH 3/5] mmc: am654_sdhci: Add itap_del_ena[] to store itapdlyena 
> bit
> 
> Set itap_del_ena if ITAPDLY is found in DT or if the tuning
> algorithm was executed and found the optimal ITAPDLY. Add the
> functionality to save ITAPDLYENA that can be referenced later
> by storing the bit in array itap_del_ena[].
> 
> Signed-off-by: Judith Mendez 
> ---
>  drivers/mmc/am654_sdhci.c | 30 --
>  1 file changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c
> index 1dd032e1e36..38f1ad28ec4 100644
> --- a/drivers/mmc/am654_sdhci.c
> +++ b/drivers/mmc/am654_sdhci.c
> @@ -92,6 +92,7 @@ struct am654_sdhci_plat {
>   bool non_removable;
>   u32 otap_del_sel[MMC_MODES_END];
>   u32 itap_del_sel[MMC_MODES_END];
> + u32 itap_del_ena[MMC_MODES_END];
>   u32 trm_icp;
>   u32 drv_strength;
>   u32 strb_sel;
> @@ -223,8 +224,10 @@ static int am654_sdhci_setup_dll(struct am654_sdhci_plat 
> *plat,
>  }
> 
>  static void am654_sdhci_write_itapdly(struct am654_sdhci_plat *plat,
> -   u32 itapdly)
> +   u32 itapdly, u32 enable)
>  {
> + regmap_update_bits(plat->base, PHY_CTRL4, ITAPDLYENA_MASK,
> +enable << ITAPDLYENA_SHIFT);
>   /* Set ITAPCHGWIN before writing to ITAPDLY */
>   regmap_update_bits(plat->base, PHY_CTRL4, ITAPCHGWIN_MASK,
>  1 << ITAPCHGWIN_SHIFT);
> @@ -242,7 +245,8 @@ static void am654_sdhci_setup_delay_chain(struct 
> am654_sdhci_plat *plat,
>   mask = SELDLYTXCLK_MASK | SELDLYRXCLK_MASK;
>   regmap_update_bits(plat->base, PHY_CTRL5, mask, val);
> 
> - am654_sdhci_write_itapdly(plat, plat->itap_del_sel[mode]);
> + am654_sdhci_write_itapdly(plat, plat->itap_del_sel[mode],
> +   plat->itap_del_ena[mode]);
>  }
> 
>  static int am654_sdhci_set_ios_post(struct sdhci_host *host)
> @@ -443,6 +447,7 @@ static int am654_sdhci_execute_tuning(struct mmc *mmc, u8 
> opcode)
>   struct udevice *dev = mmc->dev;
>   struct am654_sdhci_plat *plat = dev_get_plat(dev);
>   struct window fail_window[ITAPDLY_LENGTH];
> + int mode = mmc->selected_mode;
>   u8 curr_pass, itap;
>   u8 fail_index = 0;
>   u8 prev_pass = 1;
> @@ -450,11 +455,10 @@ static int am654_sdhci_execute_tuning(struct mmc *mmc, 
> u8 opcode)
>   memset(fail_window, 0, sizeof(fail_window));
> 
>   /* Enable ITAPDLY */
> - regmap_update_bits(plat->base, PHY_CTRL4, ITAPDLYENA_MASK,
> -1 << ITAPDLYENA_SHIFT);
> + plat->itap_del_ena[mode] = 0x1;

0x1 means "enable"? I want to use a macro with meaning.

> 
>   for (itap = 0; itap < ITAPDLY_LENGTH; itap++) {
> - am654_sdhci_write_itapdly(plat, itap);
> + am654_sdhci_write_itapdly(plat, itap, plat->itap_del_ena[mode]);
> 
>   curr_pass = !mmc_send_tuning(mmc, opcode, NULL);
> 
> @@ -478,7 +482,7 @@ static int am654_sdhci_execute_tuning(struct mmc *mmc, u8 
> opcode)
>   itap = am654_sdhci_calculate_itap(dev, fail_window, fail_index,
> plat->dll_enable);
> 
> - am654_sdhci_write_itapdly(plat, itap);
> + am654_sdhci_write_itapdly(plat, itap, plat->itap_del_ena[mode]);
> 
>   return 0;
>  }
> @@ -515,6 +519,7 @@ static int j721e_4bit_sdhci_set_ios_post(struct 
> sdhci_host *host)
>   struct am654_sdhci_plat *plat = dev_get_plat(dev);
>   int mode = host->mmc->selected_mode;
>   u32 otap_del_sel;
> + u32 itap_del_ena;
>   u32 itap_del_sel;
>   u32 mask, val;
> 
> @@ -524,10 +529,11 @@ static int j721e_4bit_sdhci_set_ios_post(struct 
> sdhci_host *host)
>   val = (1 << OTAPDLYENA_SHIFT) |
> (otap_del_sel << OTAPDLYSEL_SHIFT);
> 
> + itap_del_ena = plat->itap_del_ena[mode];
>   itap_del_sel = plat->itap_del_sel[mode];
> 
>   mask |= ITAPDLYENA_MASK | ITAPDLYSEL_MASK;
> - val = (1 << ITAPDLYENA_SHIFT) |
> + val = (itap_del_ena << ITAPDLYENA_SHIFT) |
> (itap_del_sel << ITAPDLYSEL_SHIFT);
> 
>   regmap_update_bits(plat->base, PHY_CTRL4, ITAPCHGWIN_MASK,
> @@ -599,9 +605,13 @@ static int sdhci_am654_get_otap_delay(struct udevice 
> *dev,
>   cfg->host_caps &= ~td[i].capability;
>   }
> 
> - if (td[i].itap_binding)
> - dev_read_u32(dev, td[i].itap_binding,
> -  >itap_del_sel[i]);
> + if (td[i].itap_binding) {
> + ret = dev_read_u32(dev, td[i].itap_binding,
> +>itap_del_sel[i]);
> +
> + if (!ret)
> + 

RE: [PATCH 2/5] mmc: am654_sdhci: Fix OTAP/ITAP delay values

[Jaehoon Chung]

Hi Judith,

> -Original Message-
> From: Judith Mendez 
> Sent: Tuesday, April 16, 2024 6:28 AM
> To: Peng Fan ; Jaehoon Chung ; Tom 
> Rini 
> Cc: Nitin Yadav ; Simon Glass ; 
> u-boot@lists.denx.de
> Subject: [PATCH 2/5] mmc: am654_sdhci: Fix OTAP/ITAP delay values
> 
> From: Nitin Yadav 
> 
> U-Boot is failing to boot class U1 UHS SD cards due to incorrect
> OTAP and ITAP delay select values. Update OTAP and ITAP delay select
> values from DT.
> 
> Fixes: c7d106b4eb3 ("mmc: am654_sdhci: Update output tap delay writes")
> Signed-off-by: Nitin Yadav 
> Signed-off-by: Judith Mendez 
> ---
>  drivers/mmc/am654_sdhci.c | 23 +++
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c
> index e5ad00e2531..1dd032e1e36 100644
> --- a/drivers/mmc/am654_sdhci.c
> +++ b/drivers/mmc/am654_sdhci.c
> @@ -513,12 +513,27 @@ static int j721e_4bit_sdhci_set_ios_post(struct 
> sdhci_host *host)
>  {
>   struct udevice *dev = host->mmc->dev;
>   struct am654_sdhci_plat *plat = dev_get_plat(dev);
> - u32 otap_del_sel, mask, val;
> + int mode = host->mmc->selected_mode;
> + u32 otap_del_sel;
> + u32 itap_del_sel;
> + u32 mask, val;
> +
> + otap_del_sel = plat->otap_del_sel[mode];
> 
> - otap_del_sel = plat->otap_del_sel[host->mmc->selected_mode];
>   mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
> - val = (1 << OTAPDLYENA_SHIFT) | (otap_del_sel << OTAPDLYSEL_SHIFT);
> + val = (1 << OTAPDLYENA_SHIFT) |
> +   (otap_del_sel << OTAPDLYSEL_SHIFT);

Is there any reason to touch this? And I can't understood this, this val 
doesn’t use anywhere.
Because val is resetting the below. It seems same value, right?

> +
> + itap_del_sel = plat->itap_del_sel[mode];
> +
> + mask |= ITAPDLYENA_MASK | ITAPDLYSEL_MASK;

Can it be set at above?

mask |= OTAPDLYENA_MASK | OTAPDLYSEL_MASK | 
ITAPDLYENA_MASK | ITAPDLYSEL_MASK;

Best Regards,
Jaehoon Chung



> + val = (1 << ITAPDLYENA_SHIFT) |
> +   (itap_del_sel << ITAPDLYSEL_SHIFT);
> +
> + regmap_update_bits(plat->base, PHY_CTRL4, ITAPCHGWIN_MASK,
> +1 << ITAPCHGWIN_SHIFT);
>   regmap_update_bits(plat->base, PHY_CTRL4, mask, val);
> + regmap_update_bits(plat->base, PHY_CTRL4, ITAPCHGWIN_MASK, 0);
> 
>   regmap_update_bits(plat->base, PHY_CTRL5, CLKBUFSEL_MASK,
>  plat->clkbuf_sel);
> @@ -572,7 +587,7 @@ static int sdhci_am654_get_otap_delay(struct udevice *dev,
>* Remove the corresponding capability if an otap-del-sel
>* value is not found
>*/
> - for (i = MMC_HS; i <= MMC_HS_400; i++) {
> + for (i = MMC_LEGACY; i <= MMC_HS_400; i++) {
>   ret = dev_read_u32(dev, td[i].otap_binding,
>  >otap_del_sel[i]);
>   if (ret) {
> --
> 2.43.2

RE: [PATCH 1/5] mmc: am654_sdhci: Add tuning algorithm for delay chain

[Jaehoon Chung]

> -Original Message-
> From: Judith Mendez 
> Sent: Tuesday, April 16, 2024 6:28 AM
> To: Peng Fan ; Jaehoon Chung ; Tom 
> Rini 
> Cc: Nitin Yadav ; Simon Glass ; 
> u-boot@lists.denx.de
> Subject: [PATCH 1/5] mmc: am654_sdhci: Add tuning algorithm for delay chain
> 
> Currently the sdhci_am654 driver only supports one tuning
> algorithm which should be used only when DLL is enabled. The
> ITAPDLY is selected from the largest passing window and the
> buffer is viewed as a circular buffer.
> 
> The new tuning algorithm should be used when the delay chain
> is enabled; the ITAPDLY is selected from the largest passing
> window and the buffer is not viewed as a circular buffer.
> 
> This implementation is based off of the following paper: [1].
> 
> Also add support for multiple failing windows.
> 
> [1] https://www.ti.com/lit/an/spract9/spract9.pdf
> 
> Fixes: a759abf569d4 ("mmc: am654_sdhci: Add support for software tuning")
> Signed-off-by: Judith Mendez 
> ---
>  drivers/mmc/am654_sdhci.c | 107 +++---
>  1 file changed, 89 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c
> index 05595bdac39..e5ad00e2531 100644
> --- a/drivers/mmc/am654_sdhci.c
> +++ b/drivers/mmc/am654_sdhci.c
> @@ -97,6 +97,7 @@ struct am654_sdhci_plat {
>   u32 strb_sel;
>   u32 clkbuf_sel;
>   u32 flags;
> + bool dll_enable;
>  #define DLL_PRESENT  BIT(0)
>  #define IOMUX_PRESENTBIT(1)
>  #define FREQSEL_2_BITBIT(2)
> @@ -110,6 +111,12 @@ struct timing_data {
>   u32 capability;
>  };
> 
> +struct window {
> + u8 start;
> + u8 end;
> + u8 length;
> +};
> +
>  static const struct timing_data td[] = {
>   [MMC_LEGACY]= {"ti,otap-del-sel-legacy",
>  "ti,itap-del-sel-legacy",
> @@ -280,8 +287,11 @@ static int am654_sdhci_set_ios_post(struct sdhci_host 
> *host)
>   ret = am654_sdhci_setup_dll(plat, speed);
>   if (ret)
>   return ret;
> +
> + plat->dll_enable = true;
>   } else {
>   am654_sdhci_setup_delay_chain(plat, mode);
> + plat->dll_enable = false;
>   }
> 
>   regmap_update_bits(plat->base, PHY_CTRL5, CLKBUFSEL_MASK,
> @@ -375,38 +385,99 @@ static void am654_sdhci_write_b(struct sdhci_host 
> *host, u8 val, int reg)
>   writeb(val, host->ioaddr + reg);
>  }
>  #ifdef MMC_SUPPORTS_TUNING
> -#define ITAP_MAX 32
> +#define ITAPDLY_LENGTH 32
> +#define ITAPDLY_LAST_INDEX (ITAPDLY_LENGTH - 1)
> +
> +static u32 am654_sdhci_calculate_itap(struct udevice *dev, struct window
> +   *fail_window, u8 num_fails, bool circular_buffer)
> +{
> + u8 itap = 0, start_fail = 0, end_fail = 0, pass_length = 0;
> + u8 first_fail_start = 0, last_fail_end = 0;
> + struct window pass_window = {0, 0, 0};
> + int prev_fail_end = -1;
> + u8 i;
> +
> + if (!num_fails)
> + return ITAPDLY_LAST_INDEX >> 1;
> +
> + if (fail_window->length == ITAPDLY_LENGTH) {
> + dev_err(dev, "No passing ITAPDLY, return 0\n");
> + return 0;
> + }
> +
> + first_fail_start = fail_window->start;
> + last_fail_end = fail_window[num_fails - 1].end;
> +
> + for (i = 0; i < num_fails; i++) {
> + start_fail = fail_window[i].start;
> + end_fail = fail_window[i].end;
> + pass_length = start_fail - (prev_fail_end + 1);
> +
> + if (pass_length > pass_window.length) {
> + pass_window.start = prev_fail_end + 1;
> + pass_window.length = pass_length;
> + }
> + prev_fail_end = end_fail;
> + }
> +
> + if (!circular_buffer)
> + pass_length = ITAPDLY_LAST_INDEX - last_fail_end;
> + else
> + pass_length = ITAPDLY_LAST_INDEX - last_fail_end + 
> first_fail_start;
> +
> + if (pass_length > pass_window.length) {
> + pass_window.start = last_fail_end + 1;
> + pass_window.length = pass_length;
> + }
> +
> + if (!circular_buffer)
> + itap = pass_window.start + (pass_window.length >> 1);
> + else
> + itap = (pass_window.start + (pass_window.length >> 1)) % 
> ITAPDLY_LENGTH;
> +
> + return (itap > ITAPDLY_LAST_INDEX) ? ITAPDLY_LAST_INDEX >> 1 : itap;
> +}
> +
>  static int am654_sdhci_execute_tuning(struct mmc *mmc, u8 opcode)
>  {
>   struct udevice *dev = mmc->dev;
>   struct am654_sdhci_plat *plat = dev_get_plat(dev);
> - int cur_val, prev_val = 1, fail_len = 0, pass_window = 0, pass_len;
> - u32 itap;
> + struct window fail_window[ITAPDLY_LENGTH];
> + u8 curr_pass, itap;
> + u8 fail_index = 0;
> + u8 prev_pass = 1;
> +
> + memset(fail_window, 0, sizeof(fail_window));
> 
>   /* Enable ITAPDLY */
>   regmap_update_bits(plat->base, PHY_CTRL4, ITAPDLYENA_MASK,
>

[PATCH v3] rockchip: px30-board-tpl: Sync ifdef guards with full TPL

[lukasz . czechowski]

From: Lukasz Czechowski 

Display TPL init information message only when TPL_BANNER_PRINT
configuration entry is set. This allows to disable information
message in case logs on UART are unwanted.
Update parent ifdef condition to check also CONFIG_TPL_SERIAL
to match logic of the non-PX30 TPL implementation.

Signed-off-by: Lukasz Czechowski 

---
Changes for v2:
- Updated parent ifdef condition

Changes for v3:
- Updated commit title
---
 arch/arm/mach-rockchip/px30-board-tpl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-rockchip/px30-board-tpl.c 
b/arch/arm/mach-rockchip/px30-board-tpl.c
index 637a5e1b18..db368a7b8c 100644
--- a/arch/arm/mach-rockchip/px30-board-tpl.c
+++ b/arch/arm/mach-rockchip/px30-board-tpl.c
@@ -36,7 +36,7 @@ void board_init_f(ulong dummy)
 {
int ret;
 
-#ifdef CONFIG_DEBUG_UART
+#if defined(CONFIG_DEBUG_UART) && defined(CONFIG_TPL_SERIAL)
debug_uart_init();
/*
 * Debug UART can be used from here if required:
@@ -46,7 +46,9 @@ void board_init_f(ulong dummy)
 * printhex8(0x1234);
 * printascii("string");
 */
+#if CONFIG_TPL_BANNER_PRINT
printascii("U-Boot TPL board init\n");
+#endif
 #endif
 
secure_timer_init();
-- 
2.34.1

[PATCH] ram: rockchip: px30: Replace misleading log message

[lukasz . czechowski]

From: Lukasz Czechowski 

Remove the log message "out" from sdram_init function which
pollutes the console. It brings no meaningful information and
might be unwanted in case silencing the console is required.
Instead, add a debug log with a more meaningful message, printed
only if DEBUG is set. The same convention is used for other
boards, i.e. rk3399.

Signed-off-by: Lukasz Czechowski 
---
 drivers/ram/rockchip/sdram_px30.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ram/rockchip/sdram_px30.c 
b/drivers/ram/rockchip/sdram_px30.c
index 21498e8957..3b429973cd 100644
--- a/drivers/ram/rockchip/sdram_px30.c
+++ b/drivers/ram/rockchip/sdram_px30.c
@@ -712,7 +712,7 @@ int sdram_init(void)
 
sdram_print_ddr_info(_params->ch.cap_info, _params->base, 
0);
 
-   printascii("out\n");
+   debug("Finish SDRAM initialization...\n");
return ret;
 error:
return (-1);
-- 
2.34.1

RE: [PATCH 2/2] mmc: stm32_sdmmc2: Fix AARCH64 compilation warnings

[Jaehoon Chung]

> -Original Message-
> From: Patrick DELAUNAY 
> Sent: Wednesday, April 17, 2024 6:02 PM
> To: Patrice Chotard ; u-boot@lists.denx.de
> Cc: U-Boot STM32 ; Jaehoon Chung 
> ;
> Peng Fan ; Sean Anderson ; Simon Glass 
> ; Tom
> Rini 
> Subject: Re: [PATCH 2/2] mmc: stm32_sdmmc2: Fix AARCH64 compilation warnings
> 
> Hi,
> 
> On 3/8/24 15:26, Patrice Chotard wrote:
> > When building with AARCH64 defconfig, we got warnings, fix them.
> >
> > Signed-off-by: Patrice Chotard 
> > ---
> >
> >   drivers/mmc/stm32_sdmmc2.c | 8 
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mmc/stm32_sdmmc2.c b/drivers/mmc/stm32_sdmmc2.c
> > index d4982a14281..39ae79ba129 100644
> > --- a/drivers/mmc/stm32_sdmmc2.c
> > +++ b/drivers/mmc/stm32_sdmmc2.c
> > @@ -220,9 +220,9 @@ static void stm32_sdmmc2_start_data(struct udevice *dev,
> >
> > if (data->flags & MMC_DATA_READ) {
> > data_ctrl |= SDMMC_DCTRL_DTDIR;
> > -   idmabase0 = (u32)data->dest;
> > +   idmabase0 = (u32)(long)data->dest;
> > } else {
> > -   idmabase0 = (u32)data->src;
> > +   idmabase0 = (u32)(long)data->src;
> > }
> >
> > /* Set the SDMMC DataLength value */
> > @@ -463,8 +463,8 @@ retry_cmd:
> >
> > stm32_sdmmc2_start_cmd(dev, cmd, cmdat, );
> >
> > -   dev_dbg(dev, "send cmd %d data: 0x%x @ 0x%x\n",
> > -   cmd->cmdidx, data ? ctx.data_length : 0, (unsigned int)data);
> > +   dev_dbg(dev, "send cmd %d data: 0x%x @ 0x%p\n",
> > +   cmd->cmdidx, data ? ctx.data_length : 0, data);
> >
> > ret = stm32_sdmmc2_end_cmd(dev, cmd, );
> >
> 
> 
> 
> Reviewed-by: Patrick Delaunay 

Reviewed-by: Jaehoon Chung 

Best Regards,
Jaehoon Chung

> 
> Thanks
> Patrick

RE: [PATCH 1/2] mmc: stm32_sdmmc2: Add "st,stm32mp25-sdmmc2" compatible

[Jaehoon Chung]

Hi

> -Original Message-
> From: Patrick DELAUNAY 
> Sent: Wednesday, April 17, 2024 6:02 PM
> To: Patrice Chotard ; u-boot@lists.denx.de
> Cc: U-Boot STM32 ; Jaehoon Chung 
> ;
> Peng Fan ; Sean Anderson ; Simon Glass 
> ; Tom
> Rini 
> Subject: Re: [PATCH 1/2] mmc: stm32_sdmmc2: Add "st,stm32mp25-sdmmc2" 
> compatible
> 
> Hi,
> 
> On 3/8/24 15:26, Patrice Chotard wrote:
> > From: Patrick Delaunay 
> >
> > Add compatible used for STM32MP25 family.
> >
> > Signed-off-by: Patrick Delaunay 
> > Signed-off-by: Patrice Chotard 
> > ---
> >
> >   drivers/mmc/stm32_sdmmc2.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/mmc/stm32_sdmmc2.c b/drivers/mmc/stm32_sdmmc2.c
> > index a2b111a8435..d4982a14281 100644
> > --- a/drivers/mmc/stm32_sdmmc2.c
> > +++ b/drivers/mmc/stm32_sdmmc2.c
> > @@ -789,6 +789,7 @@ static int stm32_sdmmc2_bind(struct udevice *dev)
> >
> >   static const struct udevice_id stm32_sdmmc2_ids[] = {
> > { .compatible = "st,stm32-sdmmc2" },
> > +   { .compatible = "st,stm32mp25-sdmmc2" },
> > { }
> >   };
> >
> 
> 
> Reviewed-by: Patrick Delaunay 

Reviewed-by: Jaehoon Chung 

Best Regards,
Jaehoon Chung

> 
> Thanks
> Patrick
> 

RE: [RFC PATCH v1 1/1] mmc: snps_sdhci: Add sdhci driver support for TH1520 SoC

[Jaehoon Chung]

Hi,

> -Original Message-
> From: Maxim Kiselev 
> Sent: Wednesday, April 3, 2024 3:57 AM
> To: Sean Anderson 
> Cc: Tom Rini ; Peng Fan ; Jaehoon Chung 
> ;
> Marek Vasut ; Paul Barker 
> ; Kever Yang
> ; Peter Geis ; Oleksandr 
> Suvorov
> ; Stefan Roese ; 
> u-boot@lists.denx.de
> Subject: Re: [RFC PATCH v1 1/1] mmc: snps_sdhci: Add sdhci driver support for 
> TH1520 SoC
> 
> Hi, Sean
> 
> вт, 2 апр. 2024 г. в 18:39, Sean Anderson :
> >
> > On 3/30/24 13:59, Maksim Kiselev wrote:
> > > Add support for DesignWare SDHCI host controller on Alibaba TH1520 SoC
> > >
> > > Signed-off-by: Maksim Kiselev 
> > > ---
> > > drivers/mmc/Kconfig | 12 +
> > > drivers/mmc/Makefile | 1 +
> > > drivers/mmc/snps_sdhci.c | 464 +++
> > > 3 files changed, 477 insertions(+)
> > > create mode 100644 drivers/mmc/snps_sdhci.c
> > >
> > > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> > > index cef05790dd..6936fa0928 100644
> > > --- a/drivers/mmc/Kconfig
> > > +++ b/drivers/mmc/Kconfig
> > > @@ -671,6 +671,18 @@ config MMC_SDHCI_S5P
> > >
> > > If unsure, say N.
> > >
> > > +config MMC_SDHCI_SNPS
> > > + bool "Synopsys DesignWare SDHCI controller"
> > > + depends on MMC_SDHCI
> > > + depends on DM_MMC
> > > + help
> > > + Support for DesignWare SDHCI host controller on Alibaba TH1520 SoC.
> > > + This is a highly configurable and programmable, high performance
> > > + Mobile Storage Host Controller (MSHC) with AXI as the bus interface
> > > + for data transfer.
> > > +
> > > + If unsure, say N.
> > > +
> >
> > Why not use MMC_DW?
> 
> That driver looks completely different. As I notice in the cover letter,
> the MMC_SDHCI_ROCKCHIP driver is closer to TH1520 driver.
> https://lore.kernel.org/u-boot/20240330175948.80931-1-biguncle...@gmail.com/
> 
> Their IP blocks look very similar.
> Moreover, on Linux, Rockchip SDHCI controllers (ex. rockchip,rk3588-dwcmshc)
> and the TH1520 controller live in the same driver - sdhci-of-dwcmshc.c

Right. I didn't have its TRM. AFAIK, It's a different IP with DW_MMC.

> 
> We could do the same for U-boot. It will require to get rid of Rockchip
> specific defines that come from asm/arch-rockchip/hardware.h.
> And maybe rename the rockchip_sdhci.c driver to sdhci-of-dwcmshc.c.

In u-boot, rochkip_sdhci.c is including the specific rockchip code. (rockchip 
header..)
So I think that it can be a huge refactoring to use both SoC.

But if it's possible, I think that it can be reusable.

> 
> That's why I mark this series as RFC. So I'm willing to discuss any option.
> 
> > --Sean
> >
> >
> > [Embedded World 2024, SECO SpA] ticket.de/Nuernberg/embeddedworld2024/Register/ew24517689>
> 
> Cheers,
> Maksim

Re: [PATCH v2] rockchip: px30-board-tpl: Use CONFIG_TPL_BANNER_PRINT flag

[Łukasz Czechowski]

Hi Quentin,

Thanks for your response.
As for the CONFIG_TPL_SERIAL flag, I agree that it should use 'imply'
to be allowed to disable it, to silence the serial in TPL. However,
currently, disabling this flag makes the code unbuildable (which could
be the reason 'select' is used). I'd keep this change out of scope for
this commit.
I'll prepare v3 then, which will contain only updated Cc and commit title.

Best regards,
Łukasz

śr., 17 kwi 2024 o 11:55 Quentin Schulz
 napisał(a):
>
> Hi Lukasz,
>
> I would have renamed the commit title to be something like
>
> rockchip: px30-board-tpl: sync ifdef guards with full TPL
>
> because CONFIG_TPL_BANNER_PRINT isn't really a flag but a Kconfig symbol
> and the effect of this patch is to not print the banner when the symbol
> is not selected.
>
> Another option could have been (only for the v1, since you do more now):
>
> rockchip: px30-board-tpl: print banner only if CONFIG_TPL_BANNER_PRINT
>
> On 4/17/24 11:35, lukasz.czechow...@thaumatec.com wrote:
> > From: Lukasz Czechowski 
> >
> > Display TPL init information message only when TPL_BANNER_PRINT
> > configuration entry is set. This allows to disable information
> > message in case logs on UART are unwanted.
> > Update parent ifdef condition to check also CONFIG_TPL_SERIAL
> > to match logic of the non-PX30 TPL implementation.
> >
> > Signed-off-by: Lukasz Czechowski 
> > Cc: Tom Rini 
> > Cc: Simon Glass 
> > Cc: Philipp Tomsich 
> > Cc: Kever Yang 
> >
>
> Small nitpick, the Cc would make it to the git history, which isn't
> really necessary. I believe you could have those below the --- and
> git-send-email still find them.
>
> Additionally, git send-email allows you to provide --cc and --to manually.
>
> > ---
> > Changes for v2:
> > - Updated parent ifdef condition
> > ---
> >   arch/arm/mach-rockchip/px30-board-tpl.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-rockchip/px30-board-tpl.c 
> > b/arch/arm/mach-rockchip/px30-board-tpl.c
> > index 637a5e1b18..db368a7b8c 100644
> > --- a/arch/arm/mach-rockchip/px30-board-tpl.c
> > +++ b/arch/arm/mach-rockchip/px30-board-tpl.c
> > @@ -36,7 +36,7 @@ void board_init_f(ulong dummy)
> >   {
> >   int ret;
> >
> > -#ifdef CONFIG_DEBUG_UART
> > +#if defined(CONFIG_DEBUG_UART) && defined(CONFIG_TPL_SERIAL)
>
> Quite interestingly, CONFIG_TPL_SERIAL cannot be disabled on PX30, maybe
> we should change arch/arm/mach-rockchip/Kconfig to use an `imply`
> instead of `select`.
>
> Anyway, no change required on my side. Only, if there's a v3 (for the Cc
> and commit title, the Kconfig change would be a separate patch anyway).
>
> Reviewed-by: Quentin Schulz 
>
> Thanks,
> Quentin

[PATCH v2 4/4] efi_selftest: add tests for setvariableRT

[Ilias Apalodimas]

Since we support SetVariableRT now add the relevant tests

- Search for the RTStorageVolatile and VarToFile variables after EBS
- Try to update with invalid variales (BS, RT only)
- Try to write a variable bigger than our backend storage
- Write a variable that fits and check VarToFile has been updated
  correclty
- Append to the variable and check VarToFile changes
- Try to delete VarToFile which is write protected

Signed-off-by: Ilias Apalodimas 
---
 .../efi_selftest_variables_runtime.c  | 103 ++
 1 file changed, 103 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c 
b/lib/efi_selftest/efi_selftest_variables_runtime.c
index 4c9405c0a7c7..e492b50a43c2 100644
--- a/lib/efi_selftest/efi_selftest_variables_runtime.c
+++ b/lib/efi_selftest/efi_selftest_variables_runtime.c
@@ -10,6 +10,7 @@
  */
 
 #include 
+#include 
 
 #define EFI_ST_MAX_DATA_SIZE 16
 #define EFI_ST_MAX_VARNAME_SIZE 40
@@ -17,6 +18,8 @@
 static struct efi_boot_services *boottime;
 static struct efi_runtime_services *runtime;
 static const efi_guid_t guid_vendor0 = EFI_GLOBAL_VARIABLE_GUID;
+static const efi_guid_t __efi_runtime_data efi_rt_var_guid =
+   U_BOOT_EFI_RT_VAR_FILE_GUID;
 
 /*
  * Setup unit test.
@@ -45,11 +48,14 @@ static int execute(void)
u32 attr;
u8 v[16] = {0x5d, 0xd1, 0x5e, 0x51, 0x5a, 0x05, 0xc7, 0x0c,
0x35, 0x4a, 0xae, 0x87, 0xa5, 0xdf, 0x0f, 0x65,};
+   u8 v2[CONFIG_EFI_VAR_BUF_SIZE];
u8 data[EFI_ST_MAX_DATA_SIZE];
+   u8 data2[CONFIG_EFI_VAR_BUF_SIZE];
u16 varname[EFI_ST_MAX_VARNAME_SIZE];
efi_guid_t guid;
u64 max_storage, rem_storage, max_size;
 
+   memset(v2, 0x1, sizeof(v2));
ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS,
   _storage, _storage,
   _size);
@@ -63,10 +69,107 @@ static int execute(void)
EFI_VARIABLE_RUNTIME_ACCESS,
3, v + 4);
if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
+   efi_uintn_t prev_len, delta;
+
if (ret != EFI_INVALID_PARAMETER) {
efi_st_error("SetVariable failed\n");
return EFI_ST_FAILURE;
}
+
+   len = sizeof(data);
+   ret = runtime->get_variable(u"RTStorageVolatile",
+   _rt_var_guid,
+   , , data);
+   if (ret != EFI_SUCCESS) {
+   efi_st_error("GetVariable failed\n");
+   return EFI_ST_FAILURE;
+   }
+
+   len = sizeof(data2);
+   ret = runtime->get_variable(u"VarToFile", _rt_var_guid,
+   , , data2);
+   if (ret != EFI_SUCCESS) {
+   efi_st_error("GetVariable failed\n");
+   return EFI_ST_FAILURE;
+   }
+   /*
+* VarToFile will size must change once a variable is inserted
+* Store it now, we'll use it later
+*/
+   prev_len = len;
+   ret = runtime->set_variable(u"efi_st_var0", _vendor0,
+   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+   EFI_VARIABLE_RUNTIME_ACCESS |
+   EFI_VARIABLE_NON_VOLATILE,
+   sizeof(v2),
+   v2);
+   /*
+* This will try to update VarToFile as well and must fail,
+* without changing or deleting VarToFile
+*/
+   if (ret != EFI_OUT_OF_RESOURCES) {
+   efi_st_error("SetVariable failed\n");
+   return EFI_ST_FAILURE;
+   }
+   len = sizeof(data2);
+   ret = runtime->get_variable(u"VarToFile", _rt_var_guid,
+   , , data2);
+   if (ret != EFI_SUCCESS || prev_len != len) {
+   efi_st_error("Get/SetVariable failed\n");
+   return EFI_ST_FAILURE;
+   }
+
+   /* SetVariableRT updates VarToFile with efi_st_var0 */
+   ret = runtime->set_variable(u"efi_st_var0", _vendor0,
+   EFI_VARIABLE_BOOTSERVICE_ACCESS |
+   EFI_VARIABLE_RUNTIME_ACCESS |
+   EFI_VARIABLE_NON_VOLATILE,
+   sizeof(v), v);
+   if (ret != EFI_SUCCESS) {
+   efi_st_error("SetVariable failed\n");
+   return 

[PATCH v2 3/4] efi_loader: add an EFI variable with the file contents

[Ilias Apalodimas]

Previous patches enabled SetVariableRT using a RAM backend.
Although EBBR [0] defines a variable format we can teach userspace tools
and write the altered variables, it's better if we skip the ABI
requirements completely.

So let's add a new variable, in its own namespace called "VarToFile"
which contains a binary dump of the updated RT, BS and, NV variables
and will be updated when GetVariable is called.

Some adjustments are needed to do that.
Currently we discard BS-only variables in EBS(). We need to preserve
those on the RAM backend that exposes the variables. Since BS-only
variables can't appear at runtime we need to move the memory masking
checks from efi_var_collect() to efi_get_next_variable_name_mem()/
efi_get_variable_mem() and do the filtering at runtime.

We also need an efi_var_collect() variant available at runtime, in order
to construct the "VarToFile" buffer on the fly.

All users and applications (for linux) have to do when updating a variable
is dd that variable in the file described by "RTStorageVolatile".

Linux efivarfs uses a first 4 bytes of the output to represent attributes
in little-endian format. So, storing variables works like this:

$~ efibootmgr -n 0001
$~ dd 
if=/sys/firmware/efi/efivars/VarToFile-b2ac5fc9-92b7-4acd-aeac-11e818c3130c 
of=/boot/efi/ubootefi.var skip=4 bs=1

[0] 
https://arm-software.github.io/ebbr/index.html#document-chapter5-variable-storage

Co-developed-by: Heinrich Schuchardt 
Signed-off-by: Heinrich Schuchardt 
Signed-off-by: Ilias Apalodimas 
---
 include/efi_variable.h|  14 ++-
 lib/charset.c |   2 +-
 lib/efi_loader/efi_runtime.c  |  19 
 lib/efi_loader/efi_var_common.c   |   6 +-
 lib/efi_loader/efi_var_mem.c  | 146 ++
 lib/efi_loader/efi_variable.c |   6 +-
 lib/efi_loader/efi_variable_tee.c |   5 -
 7 files changed, 130 insertions(+), 68 deletions(-)

diff --git a/include/efi_variable.h b/include/efi_variable.h
index 42a2b7c52bef..b545a36aac50 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -271,13 +271,16 @@ const efi_guid_t *efi_auth_var_get_guid(const u16 *name);
  *
  * @variable_name_size:size of variable_name buffer in bytes
  * @variable_name: name of uefi variable's name in u16
+ * @mask:  bitmask with required attributes of variables to be 
collected.
+ *  variables are only collected if all of the required
+ *  attributes match. Use 0 to skip matching
  * @vendor:vendor's guid
  *
  * Return: status code
  */
 efi_status_t __efi_runtime
 efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, u16 
*variable_name,
-  efi_guid_t *vendor);
+  efi_guid_t *vendor, u32 mask);
 /**
  * efi_get_variable_mem() - Runtime common code across efi variable
  *  implementations for GetVariable() from
@@ -289,12 +292,15 @@ efi_get_next_variable_name_mem(efi_uintn_t 
*variable_name_size, u16 *variable_na
  * @data_size: size of the buffer to which the variable value is copied
  * @data:  buffer to which the variable value is copied
  * @timep: authentication time (seconds since start of epoch)
+ * @mask:  bitmask with required attributes of variables to be 
collected.
+ *  variables are only collected if all of the required
+ *  attributes match. Use 0 to skip matching
  * Return: status code
  */
 efi_status_t __efi_runtime
 efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor,
 u32 *attributes, efi_uintn_t *data_size, void *data,
-u64 *timep);
+u64 *timep, u32 mask);
 
 /**
  * efi_get_variable_runtime() - runtime implementation of GetVariable()
@@ -334,4 +340,8 @@ efi_get_next_variable_name_runtime(efi_uintn_t 
*variable_name_size,
  */
 void efi_var_buf_update(struct efi_var_file *var_buf);
 
+efi_status_t __efi_runtime efi_var_collect_mem(struct efi_var_file *buf,
+  efi_uintn_t *lenp,
+  u32 check_attr_mask);
+
 #endif
diff --git a/lib/charset.c b/lib/charset.c
index df4f04074852..182c92a50c48 100644
--- a/lib/charset.c
+++ b/lib/charset.c
@@ -387,7 +387,7 @@ int u16_strcasecmp(const u16 *s1, const u16 *s2)
  * > 0 if the first different u16 in s1 is greater than the
  * corresponding u16 in s2
  */
-int u16_strncmp(const u16 *s1, const u16 *s2, size_t n)
+int __efi_runtime u16_strncmp(const u16 *s1, const u16 *s2, size_t n)
 {
int ret = 0;
 
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index c8f7a88ba8db..99ad1f35d8f1 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -130,6 +130,8 @@ efi_status_t efi_init_runtime_supported(void)
 

[PATCH v2 2/4] efi_loader: Add OS notifications for SetVariable at runtime

[Ilias Apalodimas]

Previous patches enable SetVariable at runtime using a volatile storage
backend using EFI_RUNTIME_SERVICES_DATA allocared memory. Since there's
no recommendation from the spec on how to notify the OS, add a volatile
EFI variable that contains the filename relative to the ESP. OS'es
can use that file and update it at runtime

$~ efivar -p -n b2ac5fc9-92b7-4acd-aeac-11e818c3130c-RTStorageVolatile
GUID: b2ac5fc9-92b7-4acd-aeac-11e818c3130c
Name: "RTStorageVolatile"
Attributes:
Boot Service Access
Runtime Service Access
Value:
  75 62 6f 6f 74 65 66 69  2e 76 61 72 00   |ubootefi.var.   |

Signed-off-by: Ilias Apalodimas 
---
 include/efi_loader.h |  4 
 lib/efi_loader/efi_runtime.c | 19 ---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index bb51c0281774..69442f4e58de 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -159,6 +159,10 @@ static inline void efi_set_bootdev(const char *dev, const 
char *devnr,
 #define EFICONFIG_AUTO_GENERATED_ENTRY_GUID \
EFI_GUID(0x8108ac4e, 0x9f11, 0x4d59, \
 0x85, 0x0e, 0xe2, 0x1a, 0x52, 0x2c, 0x59, 0xb2)
+#define U_BOOT_EFI_RT_VAR_FILE_GUID \
+   EFI_GUID(0xb2ac5fc9, 0x92b7, 0x4acd, \
+0xae, 0xac, 0x11, 0xe8, 0x18, 0xc3, 0x13, 0x0c)
+
 
 /* Use internal device tree when starting UEFI application */
 #define EFI_FDT_USE_INTERNAL NULL
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index dde083b09665..c8f7a88ba8db 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -110,6 +111,7 @@ static __efi_runtime_data efi_uintn_t efi_descriptor_size;
  */
 efi_status_t efi_init_runtime_supported(void)
 {
+   const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID;
efi_status_t ret;
struct efi_rt_properties_table *rt_table;
 
@@ -127,9 +129,20 @@ efi_status_t efi_init_runtime_supported(void)
EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
EFI_RT_SUPPORTED_CONVERT_POINTER;
 
-   if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE))
-   rt_table->runtime_services_supported |=
-   EFI_RT_SUPPORTED_SET_VARIABLE;
+   if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
+   ret = efi_set_variable_int(u"RTStorageVolatile",
+  _guid_efi_rt_var_file,
+  EFI_VARIABLE_BOOTSERVICE_ACCESS |
+  EFI_VARIABLE_RUNTIME_ACCESS |
+  EFI_VARIABLE_READ_ONLY,
+  sizeof(EFI_VAR_FILE_NAME),
+  EFI_VAR_FILE_NAME, false);
+   if (ret != EFI_SUCCESS) {
+   log_err("Failed to set RTStorageVolatile\n");
+   return ret;
+   }
+   rt_table->runtime_services_supported |= 
EFI_RT_SUPPORTED_SET_VARIABLE;
+   }
 
/*
 * This value must be synced with efi_runtime_detach_list
-- 
2.40.1

[PATCH v2 1/4] efi_loader: conditionally enable SetvariableRT

[Ilias Apalodimas]

When we store EFI variables on file we don't allow SetVariable at runtime,
since the OS doesn't know how to access or write that file.  At the same
time keeping the U-Boot drivers alive in runtime sections and performing
writes from the firmware is dangerous -- if at all possible.

For GetVariable at runtime we copy runtime variables in RAM and expose them
to the OS. Add a Kconfig option and provide SetVariable at runtime using
the same memory backend. The OS will be responsible for syncing the RAM
contents to the file, otherwise any changes made during runtime won't
persist reboots.

It's worth noting that the variable store format is defined in EBBR [0]
and authenticated variables are explicitly prohibited, since they have
to be stored on a medium that's tamper and rollback protected.

- pre-patch
$~ mount | grep efiva
efivarfs on /sys/firmware/efi/efivars type efivarfs 
(ro,nosuid,nodev,noexec,relatime)

$~ efibootmgr -n 0001
Could not set BootNext: Read-only file system

- post-patch
$~ mount | grep efiva
efivarfs on /sys/firmware/efi/efivars type efivarfs 
(rw,nosuid,nodev,noexec,relatime)

$~ efibootmgr -n 0001
BootNext: 0001
BootCurrent: 
BootOrder: ,0001
Boot* debian
HD(1,GPT,bdae5610-3331-4e4d-9466-acb5caf0b4a6,0x800,0x10)/File(EFI\debian\grubaa64.efi)
Boot0001* virtio 0  
VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,85001f00)/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b,16008500){auto_created_boot_option}

$~ efivar -p -n 8be4df61-93ca-11d2-aa0d-00e098032b8c-BootNext
GUID: 8be4df61-93ca-11d2-aa0d-00e098032b8c
Name: "BootNext"
Attributes:
Non-Volatile
Boot Service Access
Runtime Service Access
Value:
  01 00

[0] 
https://arm-software.github.io/ebbr/index.html#document-chapter5-variable-storage

Signed-off-by: Ilias Apalodimas 
---
 lib/efi_loader/Kconfig|  16 +++
 lib/efi_loader/efi_runtime.c  |   4 +
 lib/efi_loader/efi_variable.c | 115 --
 .../efi_selftest_variables_runtime.c  |  13 +-
 4 files changed, 134 insertions(+), 14 deletions(-)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index e13a6f9f4c3a..cc8371a3bb4c 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -62,6 +62,22 @@ config EFI_VARIABLE_FILE_STORE
  Select this option if you want non-volatile UEFI variables to be
  stored as file /ubootefi.var on the EFI system partition.
 
+config EFI_RT_VOLATILE_STORE
+   bool "Allow variable runtime services in volatile storage (e.g RAM)"
+   depends on EFI_VARIABLE_FILE_STORE
+   help
+ When EFI variables are stored on file we don't allow SetVariableRT,
+ since the OS doesn't know how to write that file. At he same time
+ we copy runtime variables in DRAM and support GetVariableRT
+
+ Enable this option to allow SetVariableRT on the RAM backend of
+ the EFI variable storage. The OS will be responsible for syncing
+ the RAM contents to the file, otherwise any changes made during
+ runtime won't persist reboots.
+ Authenticated variables are not supported. Note that this will
+ violate the EFI spec since writing auth variables will return
+ EFI_INVALID_PARAMETER
+
 config EFI_MM_COMM_TEE
bool "UEFI variables storage service via the trusted world"
depends on OPTEE
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index a61c9a77b13f..dde083b09665 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -127,6 +127,10 @@ efi_status_t efi_init_runtime_supported(void)
EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
EFI_RT_SUPPORTED_CONVERT_POINTER;
 
+   if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE))
+   rt_table->runtime_services_supported |=
+   EFI_RT_SUPPORTED_SET_VARIABLE;
+
/*
 * This value must be synced with efi_runtime_detach_list
 * as well as efi_runtime_services.
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index e6c1219a11c8..abc2a3402f42 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -219,17 +219,20 @@ efi_get_next_variable_name_int(efi_uintn_t 
*variable_name_size,
return efi_get_next_variable_name_mem(variable_name_size, 
variable_name, vendor);
 }
 
-efi_status_t efi_set_variable_int(const u16 *variable_name,
- const efi_guid_t *vendor,
- u32 attributes, efi_uintn_t data_size,
- const void *data, bool ro_check)
+/**
+ * setvariable_allowed() - checks defined by the UEFI spec for setvariable
+ *
+ * @variable_name: name of the variable
+ * @vendor:vendor GUID
+ * @attributes:

[PATCH v2 0/4] Enable SetVariable at runtime

[Ilias Apalodimas]

Hi all,
This is the new version of [0]

The main difference from v1 is that VarToFile is now filled on the fly,
during a GetVariable call for it, instead of creating it during
SetVariable.

The advantage of doing that is memory efficiency, since the buffer comes
from the OS now and we don't have to allocate and preserve runtime memory
to copy things around. We also don't consume space from the existsing
variable storage.

The disadvantage is that it complicates the code a bit more since we need
to check for the variable name in GetVariable. We also need a function to
collect variables at runtime that operates on the memory backend.

In the future we can clean up and try to unify efi_var_collectXXX() variants,
but I'd rather keep tha patchset simpler for now.
We will also need support for QueryVariableInfo, which I will send in
another series as part of a cleanup since it's mostly supported already.

Changes since v1:
- Instead of Creating VarToFile at SetVariable, create it on GetVariable.
  This allows us to get rid of the preallocated RT buffer, since the
  address is user provided
- convert Set/GetVariableRT -> Set/GetVariable at runtime
- return EFI_INVALID_PARAM is NV is not set at runtime
- Heinrich sent me the efi_var_collect_mem() variant

Changes since the RFC:
- Return EFI_INVALID_PARAM if attributes are not volatile
- Add EFI_WRITE_PROTECTED checks for BS, RT *only* variables
- Add 2 EFI variables and allow userspace to write the file
- Add selftests

[0] 
https://lore.kernel.org/u-boot/20240406140203.248211-1-ilias.apalodi...@linaro.org/



Ilias Apalodimas (4):
  efi_loader: conditionally enable SetvariableRT
  efi_loader: Add OS notifications for SetVariable at runtime
  efi_loader: add an EFI variable with the file contents
  efi_selftest: add tests for setvariableRT

 include/efi_loader.h  |   4 +
 include/efi_variable.h|  14 +-
 lib/charset.c |   2 +-
 lib/efi_loader/Kconfig|  16 ++
 lib/efi_loader/efi_runtime.c  |  36 +
 lib/efi_loader/efi_var_common.c   |   6 +-
 lib/efi_loader/efi_var_mem.c  | 146 +++---
 lib/efi_loader/efi_variable.c | 121 +--
 lib/efi_loader/efi_variable_tee.c |   5 -
 .../efi_selftest_variables_runtime.c  | 116 +-
 10 files changed, 384 insertions(+), 82 deletions(-)

--
2.40.1

Re: [PATCH v2 01/16] board: Define GUIDs for firmware images

[Caleb Connolly]

On 12/04/2024 22:58, Jon Humphreys wrote:
> Ilias Apalodimas  writes:
> 
>> Hi both
>>
>> On Wed, 10 Apr 2024 at 10:36, Heinrich Schuchardt  wrote:
>>>
>>> On 09.04.24 23:05, Jon Humphreys wrote:
 Heinrich Schuchardt  writes:

> On 4/9/24 00:31, Jonathan Humphreys wrote:
>> Define GUIDs for the different firmware images (tiboot3.bin, tispl.bin,
>> u-boot.img, sysfw). >
>> Signed-off-by: Jonathan Humphreys 
>> ---
>>include/configs/ti_armv7_common.h | 17 +
>>1 file changed, 17 insertions(+)
>>
>> diff --git a/include/configs/ti_armv7_common.h 
>> b/include/configs/ti_armv7_common.h
>> index 3def7b1027e..4ce14a9b84c 100644
>> --- a/include/configs/ti_armv7_common.h
>> +++ b/include/configs/ti_armv7_common.h
>> @@ -16,6 +16,23 @@
>>#ifndef __CONFIG_TI_ARMV7_COMMON_H__
>>#define __CONFIG_TI_ARMV7_COMMON_H__
>>
>> +/* GUIDs for capsule updatable firmware images */
>
> Please, provide code comments for the GUIDs, e.g.
>
> /**
>* define K3_TIBOOT3_IMAGE_GUID - firmware GUID for K3 tiboot3.bin
>*
>* This GUID is used in capsules updates to identify the tiboot3.bin
>* binary.
>*/
>>
>> I'd go one step further tbh.  Those GUIDs can be used by OEMs/ODMs
>> producing capsules for their future products.
>> Currently, we don't have a documented way to allow users to redefine
>> this (but Linaro is working on it).  If both TI and an OEM use the
>> same GUID and try to upload the firmware to LVFS, we will have a GUID
>> clash.
>> I think we should have that on the comment as well, until we can
>> properly sort it out
> 
> Ilias, I realized I may have defined the GUID's a bit incorrectly.  I
> was thinking that the GUID's identify the firmware type (eg, SPL vs
> Uboot for TI devices).  However, the SPL binary for say am62 will be
> different than the SPL binary for say am64, so I should have unique
> GUID's per firmware type *per board*.  Correct?
> 
> If so, I'll need to move these definitions to the board .h files, and my
> generic capsule binman nodes will need to have each board override the
> image GUID used.

Yes, consider for example if you wanted to maintain community firmware
images on LVFS, for a given image (like U-Boot itself) you would
presumably update it for each board, which requires each board to have
it's own GUID so you can tell them apart.

If one of the payloads is identical for a bunch of boards then it might
make sense to use one GUID for it.
> 
> thanks
> Jon
> 
>>
>> Thanks
>> /Ilias
>
> Cf.
> https://docs.kernel.org/doc-guide/kernel-doc.html#object-like-macro-documentation
>
> Best regards
>
> Heinrich
>

 Heinrich, thanks for reviewing!

 I modelled the GUID macros after how other boards and even core code
 defined there's.  (eg, include/configs/kontron-sl-mx8mm.h or
 include/efi_api.h).

 However, if this is the new direction, I will format as you suggest.
 Please confirm.
>>>
>>> Hello Jon,
>>>
>>> Without properly documenting macros we make the live of developers more
>>> difficult. Yes, we still have a lot of missing code documentation. But
>>> we should not follow poor example.
>>>
>>> Best regards
>>>
>>> Heinrich
>>>

 thanks
 Jon

>> +#define K3_TIBOOT3_IMAGE_GUID \
>> +   EFI_GUID(0xe672b518, 0x7cd7, 0x4014, 0xbd, 0x8d, \
>> +0x40, 0x72, 0x4d, 0x0a, 0xd4, 0xdc)
>> +
>> +#define K3_SPL_IMAGE_GUID \
>> +   EFI_GUID(0x86f710ad, 0x10cf, 0x46ea, 0xac, 0x67, \
>> +0x85, 0x6a, 0xe0, 0x6e, 0xfa, 0xd2)
>> +
>> +#define K3_UBOOT_IMAGE_GUID \
>> +   EFI_GUID(0x81b58fb0, 0x3b00, 0x4add, 0xa2, 0x0a, \
>> +0xc1, 0x85, 0xbb, 0xac, 0xa1, 0xed)
>> +
>> +#define K3_SYSFW_IMAGE_GUID \
>> +   EFI_GUID(0x6fd10680, 0x361b, 0x431f, 0x80, 0xaa, \
>> +0x89, 0x94, 0x55, 0x81, 0x9e, 0x11)
>> +
>>/*
>> * We setup defaults based on constraints from the Linux kernel, 
>> which should
>> * also be safe elsewhere.  We have the default load at 32MB into DDR 
>> (for
>>>

-- 
// Caleb (they/them)

Re: [PATCH v2] rockchip: px30-board-tpl: Use CONFIG_TPL_BANNER_PRINT flag

[Quentin Schulz]

Hi Lukasz,

I would have renamed the commit title to be something like

rockchip: px30-board-tpl: sync ifdef guards with full TPL

because CONFIG_TPL_BANNER_PRINT isn't really a flag but a Kconfig symbol 
and the effect of this patch is to not print the banner when the symbol 
is not selected.


Another option could have been (only for the v1, since you do more now):

rockchip: px30-board-tpl: print banner only if CONFIG_TPL_BANNER_PRINT

On 4/17/24 11:35, lukasz.czechow...@thaumatec.com wrote:

From: Lukasz Czechowski 

Display TPL init information message only when TPL_BANNER_PRINT
configuration entry is set. This allows to disable information
message in case logs on UART are unwanted.
Update parent ifdef condition to check also CONFIG_TPL_SERIAL
to match logic of the non-PX30 TPL implementation.

Signed-off-by: Lukasz Czechowski 
Cc: Tom Rini 
Cc: Simon Glass 
Cc: Philipp Tomsich 
Cc: Kever Yang 



Small nitpick, the Cc would make it to the git history, which isn't 
really necessary. I believe you could have those below the --- and 
git-send-email still find them.


Additionally, git send-email allows you to provide --cc and --to manually.


---
Changes for v2:
- Updated parent ifdef condition
---
  arch/arm/mach-rockchip/px30-board-tpl.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-rockchip/px30-board-tpl.c 
b/arch/arm/mach-rockchip/px30-board-tpl.c
index 637a5e1b18..db368a7b8c 100644
--- a/arch/arm/mach-rockchip/px30-board-tpl.c
+++ b/arch/arm/mach-rockchip/px30-board-tpl.c
@@ -36,7 +36,7 @@ void board_init_f(ulong dummy)
  {
int ret;
  
-#ifdef CONFIG_DEBUG_UART

+#if defined(CONFIG_DEBUG_UART) && defined(CONFIG_TPL_SERIAL)


Quite interestingly, CONFIG_TPL_SERIAL cannot be disabled on PX30, maybe 
we should change arch/arm/mach-rockchip/Kconfig to use an `imply` 
instead of `select`.


Anyway, no change required on my side. Only, if there's a v3 (for the Cc 
and commit title, the Kconfig change would be a separate patch anyway).


Reviewed-by: Quentin Schulz 

Thanks,
Quentin

[PATCH v2] rockchip: px30-board-tpl: Use CONFIG_TPL_BANNER_PRINT flag

[lukasz . czechowski]

From: Lukasz Czechowski 

Display TPL init information message only when TPL_BANNER_PRINT
configuration entry is set. This allows to disable information
message in case logs on UART are unwanted.
Update parent ifdef condition to check also CONFIG_TPL_SERIAL
to match logic of the non-PX30 TPL implementation.

Signed-off-by: Lukasz Czechowski 
Cc: Tom Rini 
Cc: Simon Glass 
Cc: Philipp Tomsich 
Cc: Kever Yang 

---
Changes for v2:
- Updated parent ifdef condition
---
 arch/arm/mach-rockchip/px30-board-tpl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-rockchip/px30-board-tpl.c 
b/arch/arm/mach-rockchip/px30-board-tpl.c
index 637a5e1b18..db368a7b8c 100644
--- a/arch/arm/mach-rockchip/px30-board-tpl.c
+++ b/arch/arm/mach-rockchip/px30-board-tpl.c
@@ -36,7 +36,7 @@ void board_init_f(ulong dummy)
 {
int ret;
 
-#ifdef CONFIG_DEBUG_UART
+#if defined(CONFIG_DEBUG_UART) && defined(CONFIG_TPL_SERIAL)
debug_uart_init();
/*
 * Debug UART can be used from here if required:
@@ -46,7 +46,9 @@ void board_init_f(ulong dummy)
 * printhex8(0x1234);
 * printascii("string");
 */
+#if CONFIG_TPL_BANNER_PRINT
printascii("U-Boot TPL board init\n");
+#endif
 #endif
 
secure_timer_init();
-- 
2.34.1

Re: [PATCH v1 25/25] ARM: dts: stm32: Add led-blue for stm32mp157c-ed1-scmi-u-boot

[Patrick DELAUNAY]

Hi,

On 4/9/24 17:02, Patrice Chotard wrote:

The blue led is used to indicate U-Boot entering / exit indication
then Linux heartbeat.

Signed-off-by: Patrice Chotard 
---

  arch/arm/dts/stm32mp157c-ed1-scmi-u-boot.dtsi | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)



Reviewed-by: Patrick Delaunay 

Thanks
Patrick

Re: [PATCH] rockchip: px30-board-tpl: Use CONFIG_TPL_BANNER_PRINT flag

[Łukasz Czechowski]

Hi Quentin,

Thanks for the hints. Matching the parent ifdef with non-PX30 TPL
implementation is indeed a good idea. I'll send the updated version of
the patch.

Best regards,
Lukasz

wt., 16 kwi 2024 o 15:22 Quentin Schulz
 napisał(a):
>
> Hi Lukasz,
>
> Please use scripts/get_maintainer.pl to set the Cc and To recipients of
> your mail to make sure it reaches the appropriate people explicitly.
>
> $ scripts/get_maintainer.pl arch/arm/mach-rockchip/px30-board-tpl.c
> Tom Rini  (maintainer:ARM)
> Simon Glass  (maintainer:ARM ROCKCHIP)
> Philipp Tomsich  (maintainer:ARM ROCKCHIP)
> Kever Yang  (maintainer:ARM ROCKCHIP)
> u-boot@lists.denx.de (open list)
>
> (one can use scripts/get_maintainer.pl on patches instead of files, and
> it'll return whatever is needed).
>
> Plugging `b4` here as well, because it's a pretty nice tool to use:
> https://git.kernel.org/pub/scm/utils/b4/b4.git/ One can install it with
> pip. I use it for Linux kernel and U-Boot contributions for a couple of
> years now and I'm not going back to manual workflow :)
>
> b4 prep --auto-to-cc would set everything up properly for you.
>
> On 4/16/24 14:47, Lukasz Czechowski wrote:
> > Display TPL init information message only when TPL_BANNER_PRINT
> > configuration entry is set. This allows to disable information
> > message in case logs on UART are unwanted.
> >
> > Signed-off-by: Lukasz Czechowski 
>
> This matches Rockchip's non-PX30 TPL, so:
>
> Reviewed-by: Quentin Schulz 
>
> > ---
> >   arch/arm/mach-rockchip/px30-board-tpl.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/arm/mach-rockchip/px30-board-tpl.c 
> > b/arch/arm/mach-rockchip/px30-board-tpl.c
> > index 637a5e1b18..a660816db0 100644
> > --- a/arch/arm/mach-rockchip/px30-board-tpl.c
> > +++ b/arch/arm/mach-rockchip/px30-board-tpl.c
> > @@ -46,7 +46,9 @@ void board_init_f(ulong dummy)
> >* printhex8(0x1234);
> >* printascii("string");
> >*/
> > +#if CONFIG_TPL_BANNER_PRINT
> >   printascii("U-Boot TPL board init\n");
> > +#endif
>
> I'm wondering if we shouldn't have the parent ifdef also match the logic
> of the non-PX30 TPL?
>
> #if defined(CONFIG_DEBUG_UART) && defined(CONFIG_TPL_SERIAL)
>
> instead of
>
> #if defined(CONFIG_DEBUG_UART)
>
> ?
>
> Thanks,
> Quentin

Re: [PATCH] board: ti: j721e: Add support for both esm probe

[J, KEERTHY]

On 4/17/2024 10:06 AM, Udit Kumar wrote:

At present only MCU domain ESM is probed, this means errors
occurred in mcu domain will be propagate to MCU_SAFETY_ERRORn.
MCU ESM accepts SOC_SAFETY_ERRORn signal as Error
event and propagate to MCU_SAFETY_ERRORn.[0]

Therefore adding support to probe both main domain and mcu
domain ESM.


Reviewed-by: Keerthy 



[0]: https://www.ti.com/lit/zip/spruil1
spruil1c.pdf from zip
Figure 12-1244. ESM Modules Overview

Signed-off-by: Udit Kumar 
---
  board/ti/j721e/evm.c | 9 ++---
  1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/board/ti/j721e/evm.c b/board/ti/j721e/evm.c
index 9dc3ed6dff..539eaf4718 100644
--- a/board/ti/j721e/evm.c
+++ b/board/ti/j721e/evm.c
@@ -465,10 +465,13 @@ void spl_board_init(void)
}
  
  	if (IS_ENABLED(CONFIG_ESM_K3)) {

-   ret = uclass_get_device_by_driver(UCLASS_MISC,
- DM_DRIVER_GET(k3_esm), );
+   ret = uclass_get_device_by_name(UCLASS_MISC, "esm@70", 
);
+   if (ret)
+   printf("MISC init for esm@70 failed: %d\n", ret);
+
+   ret = uclass_get_device_by_name(UCLASS_MISC, "esm@4080", 
);
if (ret)
-   printf("ESM init failed: %d\n", ret);
+   printf("MISC init for esm@4080 failed: %d\n", ret);
}
  
  	if (IS_ENABLED(CONFIG_ESM_PMIC)) {

Re: [PATCH v1 24/25] ARM: dts: stm32: Update red led node for stm32mp157c-ed1-scmi-u-boot

[Patrick DELAUNAY]

Hi,

On 4/9/24 17:02, Patrice Chotard wrote:

As indicated in kernel led dt-bindings, label is a deprecated
property, so remove it and use led node's name instead for
u-boot,error-led property.
Rename red led node's name to led-red.
Remove status property which is useless.
Add compatible = "gpio-leds"; which is not present in kernel DT.

Signed-off-by: Patrice Chotard 
---

  arch/arm/dts/stm32mp157c-ed1-scmi-u-boot.dtsi | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)




Reviewed-by: Patrick Delaunay 

Thanks
Patrick

Re: [PATCH v1 23/25] ARM: dts: stm32: Don't probe red led at boot for stm32mp157c-ed1-scmi-u-boot

[Patrick DELAUNAY]

Hi,

On 4/9/24 17:02, Patrice Chotard wrote:

red led and button dedicated to fastboot share the same gpio GPIOA13.
Led driver is probed early so the corresponding gpio is taken and
configured in output which forbid fastboot and stm32prog button usage.

To avoid this, remove the "default-state" property from red led node.

This will avoid to trigger the led driver probe() to configure the led
default state during startup.

Signed-off-by: Patrice Chotard 
---

  arch/arm/dts/stm32mp157c-ed1-scmi-u-boot.dtsi | 1 -
  1 file changed, 1 deletion(-)




Reviewed-by: Patrick Delaunay 

Thanks
Patrick

Re: [PATCH v1 22/25] ARM: dts: stm32: Add gpio-keys for stm32mp157c-ed1-scmi-u-boot

[Patrick DELAUNAY]

Hi,

On 4/9/24 17:02, Patrice Chotard wrote:

Add 2 gpio-keys :
   _ button-user-1 for stm32prog mode activation.
   _ button-user-2 for fastboot mode activation.

Remove proprietary st,fastboot-gpios and st,stm32prog-gpios.

Signed-off-by: Patrice Chotard 
---

  arch/arm/dts/stm32mp157c-ed1-scmi-u-boot.dtsi | 19 +--
  1 file changed, 17 insertions(+), 2 deletions(-)



Reviewed-by: Patrick Delaunay 

Thanks
Patrick

Re: [PATCH v1 21/25] ARM: dts: stm32: Add led-blue for stm32mp157c-ed1-u-boot

[Patrick DELAUNAY]

Hi,

On 4/9/24 17:02, Patrice Chotard wrote:

The blue led is used to indicate U-Boot entering / exit indication
then Linux heartbeat.

Signed-off-by: Patrice Chotard 
---

  arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)




Reviewed-by: Patrick Delaunay 

Thanks
Patrick

Re: [PATCH v1 20/25] ARM: dts: stm32: Update red led node for stm32mp157c-ed1-u-boot

[Patrick DELAUNAY]

Hi,

On 4/9/24 17:02, Patrice Chotard wrote:

As indicated in kernel led dt-bindings, label is a deprecated
property, so remove it and use led node's name instead for
u-boot,error-led property.
Rename red led node's name to led-red.
Remove status property which is useless.
Add compatible = "gpio-leds" which is not present in kernel DT.

Signed-off-by: Patrice Chotard 
---

  arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)




Reviewed-by: Patrick Delaunay 

Thanks
Patrick

Re: [PATCH v1 19/25] ARM: dts: stm32: Don't probe red led at boot for stm32mp157c-ed1-u-boot

[Patrick DELAUNAY]

Hi,

On 4/9/24 17:02, Patrice Chotard wrote:

red led and button dedicated to fastboot share the same gpio GPIOA13.
Led driver is probed early so the corresponding gpio is taken and
configured in output which forbid fastboot and stm32prog button usage.

To avoid this, remove the "default-state" property from red led node.

This will avoid to trigger the led driver probe() to configure the led
default state during startup.

Signed-off-by: Patrice Chotard 
---

  arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi | 1 -
  1 file changed, 1 deletion(-)




Reviewed-by: Patrick Delaunay 

Thanks
Patrick

Re: [PATCH v1 18/25] ARM: dts: stm32: Add gpio-keys for stm32mp157c-ed1-u-boot

[Patrick DELAUNAY]

Hi,

On 4/9/24 17:02, Patrice Chotard wrote:

Add 2 gpio-keys :
   _ button-user-1 for stm32prog mode activation.
   _ button-user-2 for fastboot mode activation.

Remove proprietary st,fastboot-gpios and st,stm32prog-gpios.

Signed-off-by: Patrice Chotard 
---

  arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi | 19 +--
  1 file changed, 17 insertions(+), 2 deletions(-)



Reviewed-by: Patrick Delaunay 

Thanks
Patrick

Re: [PATCH v1 17/25] ARM: dts: stm32: Update u-boot,boot-led for stm32mp157a-dk1-u-boot

[Patrick DELAUNAY]

Hi,

On 4/9/24 17:02, Patrice Chotard wrote:

As indicated in kernel led dt-bindings, label is a deprecated
property, so remove it and use blue led node's name instead
for u-boot,boot-led property.

Signed-off-by: Patrice Chotard 
---

  arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)




Reviewed-by: Patrick Delaunay 

Thanks
Patrick

Re: [PATCH v1 16/25] ARM: dts: stm32: Update red led node for stm32mp157a-dk1-u-boot

[Patrick DELAUNAY]

Hi,

On 4/9/24 17:02, Patrice Chotard wrote:

As indicated in kernel led dt-bindings, label is a deprecated
property, so remove it and use red led node's name instead
for u-boot,error-led property.
Rename red led node's name to led-red.
Remove status property which is useless.

Signed-off-by: Patrice Chotard 
---

  arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)




Reviewed-by: Patrick Delaunay 

Thanks
Patrick

Re: [PATCH v1 15/25] ARM: dts: stm32: Don't probe red led at boot for stm32mp157a-dk1-u-boot

[Patrick DELAUNAY]

Hi,

On 4/9/24 17:02, Patrice Chotard wrote:

red led and button dedicated to fastboot share the same gpio GPIOA13.
Led driver is probed early so the corresponding gpio is taken and
configured in output which forbid fastboot and stm32prog button usage.

To avoid this, remove the "default-state" property from red led node.

This will avoid to trigger the led driver probe() to configure the led
default state during startup.

Signed-off-by: Patrice Chotard 
---

  arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi | 1 -
  1 file changed, 1 deletion(-)

diff --git a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi 
b/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
index 6bf6136c5fd..ee9b51d42b7 100644
--- a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
+++ b/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
@@ -67,7 +67,6 @@
red {
label = "error";
gpios = < 13 GPIO_ACTIVE_LOW>;
-   default-state = "off";
status = "okay";
};
};




Reviewed-by: Patrick Delaunay 

Thanks
Patrick

Re: [PATCH v1 14/25] ARM: dts: stm32: Add gpio-keys for stm32mp157a-dk1-u-boot

[Patrick DELAUNAY]

Hi,

On 4/9/24 17:02, Patrice Chotard wrote:

Instead of using "st,fastboot-gpios" and "st,stm32prog-gpios", declare
2 gpio-keys.

Signed-off-by: Patrice Chotard 
---

  arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi | 19 +--
  1 file changed, 17 insertions(+), 2 deletions(-)




Reviewed-by: Patrick Delaunay 

Thanks
Patrick

Re: [PATCH v1 13/25] ARM: dts: stm32: Add led-blue for stm32mp157a-dk1-scmi-u-boot

[Patrick DELAUNAY]

Hi,

On 4/9/24 17:02, Patrice Chotard wrote:

As indicated in kernel led dt-bindings, label is a deprecated
property, so remove it and use blue led node's name instead
for u-boot,boot-led property.

Signed-off-by: Patrice Chotard 
---

  arch/arm/dts/stm32mp157a-dk1-scmi-u-boot.dtsi | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/dts/stm32mp157a-dk1-scmi-u-boot.dtsi 
b/arch/arm/dts/stm32mp157a-dk1-scmi-u-boot.dtsi
index e61814fd66e..a5158fec7ef 100644
--- a/arch/arm/dts/stm32mp157a-dk1-scmi-u-boot.dtsi
+++ b/arch/arm/dts/stm32mp157a-dk1-scmi-u-boot.dtsi
@@ -13,7 +13,7 @@
};
  
  	config {

-   u-boot,boot-led = "heartbeat";
+   u-boot,boot-led = "led-blue";
u-boot,error-led = "led-red";
u-boot,mmc-env-partition = "u-boot-env";
st,adc_usb_pd = < 18>, < 19>;
@@ -36,6 +36,10 @@
};
  
  	led {

+   led-blue {
+   /delete-property/label;
+   };
+
led-red {
gpios = < 13 GPIO_ACTIVE_LOW>;
};




Reviewed-by: Patrick Delaunay 

Thanks
Patrick

RE: [PATCH v1] arm: dts: verdin-imx8mm/imx8mp: use gpio-hog for sleep moci

[Peng Fan]

> Subject: [PATCH v1] arm: dts: verdin-imx8mm/imx8mp: use gpio-hog for
> sleep moci
>
> From: Stefan Eichenberger 
>
> In Linux, we allow sleep moci to be turned off when the carrier board
> supports it and the system is in suspend. In U-Boot, however, we want the
> sleep moci to be always on. So we use a gpio hog and disable the regulator.
> This change is necessary because we switched to upstream device tree files
> with commit 23fe2def1edf
> ("verdin-imx8mm/verdin-imx8mp: move imx verdins to OF_UPSTREAM"). A
> recent upstream patch removes the gpio hog from the Linux device tree, so
> we need to add it to the u-boot dtsi. The following patch will remove the gpio
> hog from the Linux device tree:
> https://lore.ke/
> rnel.org%2Flinux-devicetree%2F20240405160720.5977-1-
> eichest%40gmail.com%2F=05%7C02%7Cpeng.fan%40nxp.com%7C0ec7
> 6e3870ef48ab53fd08dc5ebb433c%7C686ea1d3bc2b4c6fa92cd99c5c301635
> %7C0%7C0%7C638489405607145323%7CUnknown%7CTWFpbGZsb3d8eyJ
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> 7C0%7C%7C%7C=jSxtB3eu0uvBO0UPV8TY16yrEgCdTqgj6fwrwmG%2F
> v7s%3D=0
> The U-Boot patch can be applied without it and will not break the build.
>
> Signed-off-by: Stefan Eichenberger 

Reviewed-by: Peng Fan 
> ---
>  arch/arm/dts/imx8mm-verdin-wifi-dev-u-boot.dtsi | 5 +
> arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi | 4 
>  2 files changed, 9 insertions(+)
>
> diff --git a/arch/arm/dts/imx8mm-verdin-wifi-dev-u-boot.dtsi
> b/arch/arm/dts/imx8mm-verdin-wifi-dev-u-boot.dtsi
> index 38db56059d..8b397f535c 100644
> --- a/arch/arm/dts/imx8mm-verdin-wifi-dev-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mm-verdin-wifi-dev-u-boot.dtsi
> @@ -60,6 +60,11 @@
>
>   ctrl-sleep-moci-hog {
>   bootph-pre-ram;
> + gpio-hog;
> + output-high;
> + gpios = <1 GPIO_ACTIVE_HIGH>;
> + line-name = "CTRL_SLEEP_MOCI#";
> +
>   };
>  };
>
> diff --git a/arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi
> b/arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi
> index 03f211d5f7..7b45a87450 100644
> --- a/arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi
> @@ -58,6 +58,10 @@
>
>   ctrl-sleep-moci-hog {
>   bootph-pre-ram;
> + gpio-hog;
> + output-high;
> + gpios = <29 GPIO_ACTIVE_HIGH>;
> + line-name = "CTRL_SLEEP_MOCI#";
>   };
>  };
>
> --
> 2.40.1

Re: [PATCH v1 12/25] ARM: dts: stm32: Update red led node for stm32mp157a-dk1-scmi-u-boot

[Patrick DELAUNAY]

Hi,

On 4/9/24 17:02, Patrice Chotard wrote:

As indicated in kernel led dt-bindings, label is a deprecated
property, so remove it and use red led node's name instead
for u-boot,error-led property.
Rename "red" led node's name to "led-red".
Remove status property which is useless.

Signed-off-by: Patrice Chotard 
---

  arch/arm/dts/stm32mp157a-dk1-scmi-u-boot.dtsi | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/arm/dts/stm32mp157a-dk1-scmi-u-boot.dtsi 
b/arch/arm/dts/stm32mp157a-dk1-scmi-u-boot.dtsi
index 8760d6c7d93..e61814fd66e 100644
--- a/arch/arm/dts/stm32mp157a-dk1-scmi-u-boot.dtsi
+++ b/arch/arm/dts/stm32mp157a-dk1-scmi-u-boot.dtsi
@@ -14,7 +14,7 @@
  
  	config {

u-boot,boot-led = "heartbeat";
-   u-boot,error-led = "error";
+   u-boot,error-led = "led-red";
u-boot,mmc-env-partition = "u-boot-env";
st,adc_usb_pd = < 18>, < 19>;
};
@@ -36,10 +36,8 @@
};
  
  	led {

-   red {
-   label = "error";
+   led-red {
gpios = < 13 GPIO_ACTIVE_LOW>;
-   status = "okay";
};
};
  };




Reviewed-by: Patrick Delaunay 

Thanks
Patrick

Re: [PATCH v1 10/25] ARM: dts: stm32: Add gpio-keys for stm32mp157a-dk1-scmi-u-boot

[Patrick DELAUNAY]

Hi,

On 4/9/24 17:02, Patrice Chotard wrote:

Instead of using "st,fastboot-gpios" and "st,stm32prog-gpios", declare
2 gpio-keys.

Signed-off-by: Patrice Chotard 
---

  arch/arm/dts/stm32mp157a-dk1-scmi-u-boot.dtsi | 19 +--
  1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/arch/arm/dts/stm32mp157a-dk1-scmi-u-boot.dtsi 
b/arch/arm/dts/stm32mp157a-dk1-scmi-u-boot.dtsi
index 20728f27ee1..5d49b09c35d 100644
--- a/arch/arm/dts/stm32mp157a-dk1-scmi-u-boot.dtsi
+++ b/arch/arm/dts/stm32mp157a-dk1-scmi-u-boot.dtsi
@@ -3,6 +3,7 @@
   * Copyright : STMicroelectronics 2022
   */
  
+#include 

  #include "stm32mp15-scmi-u-boot.dtsi"
  
  / {

@@ -16,8 +17,22 @@
u-boot,error-led = "error";
u-boot,mmc-env-partition = "u-boot-env";
st,adc_usb_pd = < 18>, < 19>;
-   st,fastboot-gpios = < 13 (GPIO_ACTIVE_LOW | 
GPIO_PULL_UP)>;
-   st,stm32prog-gpios = < 14 (GPIO_ACTIVE_LOW | 
GPIO_PULL_UP)>;
+   };
+
+   gpio-keys {
+   compatible = "gpio-keys";
+
+   button-user-1 {
+   label = "User-1";
+   linux,code = ;
+   gpios = < 14 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
+   };
+
+   button-user-2 {
+   label = "User-2";
+   linux,code = ;
+   gpios = < 13 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
+   };
};
  
  	led {




Reviewed-by: Patrick Delaunay 

Thanks
Patrick