[PATCH u-boot-marvell 0/3] Turris Omnia DDR speed change with reset button (for 2024.10)

[PATCH u-boot-marvell 0/3] Turris Omnia DDR speed change with reset button (for 2024.10)

[Marek Behún]

Hello Stefan,

this series adds the ability to select 1333H DDR speed mode for DDR
training (instead of the default 1600K) with the factory reset button.

If possible, I would like if this series went for U-Boot 2024.10
release. The heavy stuff to support DDR speed change is already in,
this just adds the ability to do it with reset button instead of
console command (which is inconvenient for most users).

On Omnia, the user can select factory reset type by pressing the reset
button on the rear panel and holding it. The front LEDs light on in
sequence, representing a counter. Every ~2.5 second another LED lights
on. When only the first LED is on, the board will do a standard reboot.
Then different kinds of factory reboot will happen for reset types 1 to
7 (when 1, 2, ... or 7 LEDs are on) [1].

This series adds another reset type, number 10, which will make the
board boot in 1333H DDR speed mode, and also write this setting to
EEPROM, so that subsequent boots will also train in 1333H mode.

The user can set back the default mode with the `eeprom` U-Boot command,
or with the `omnia-eeprom` Linux command.

Marek

[1] https://docs.turris.cz/hw/omnia/rescue-modes/

Marek Behún (3):
  arm: mvebu: turris_omnia: Rename variable holding EEPROM udevice
  arm: mvebu: turris_omnia: Use the i2c_eeprom misc driver for EEPROM
    reading in U-Boot proper
  arm: mvebu: turris_omnia: Switch DDR speed to 1333H when reset 9 is
    selected

 arch/arm/mach-mvebu/Kconfig              |   1 +
 board/CZ.NIC/turris_omnia/turris_omnia.c | 101 +++++++++++++++++++----
 configs/turris_omnia_defconfig           |   1 -
 3 files changed, 86 insertions(+), 17 deletions(-)

-- 
2.44.2

[PATCH u-boot-marvell 1/3] arm: mvebu: turris_omnia: Rename variable holding EEPROM udevice

[Marek Behún]

Rename the variable holding the EEPROM udevice from `chip` to `eeprom`.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 board/CZ.NIC/turris_omnia/turris_omnia.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
index 2f29d26edf8..392df53a6d8 100644
--- a/board/CZ.NIC/turris_omnia/turris_omnia.c
+++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
@@ -464,16 +464,16 @@ static bool check_eeprom_crc(const void *buf, size_t size, u32 expected,
 
 static bool omnia_read_eeprom(struct omnia_eeprom *oep)
 {
-	struct udevice *chip;
+	struct udevice *eeprom;
 	int ret;
 
-	chip = omnia_get_i2c_chip("EEPROM", OMNIA_I2C_EEPROM_CHIP_ADDR,
-				  OMNIA_I2C_EEPROM_CHIP_LEN);
+	eeprom = omnia_get_i2c_chip("EEPROM", OMNIA_I2C_EEPROM_CHIP_ADDR,
+				    OMNIA_I2C_EEPROM_CHIP_LEN);
 
-	if (!chip)
+	if (!eeprom)
 		return false;
 
-	ret = dm_i2c_read(chip, 0, (void *)oep, sizeof(*oep));
+	ret = dm_i2c_read(eeprom, 0, (void *)oep, sizeof(*oep));
 	if (ret) {
 		printf("dm_i2c_read failed: %i, cannot read EEPROM\n", ret);
 		return false;
-- 
2.44.2

[PATCH u-boot-marvell 2/3] arm: mvebu: turris_omnia: Use the i2c_eeprom misc driver for EEPROM reading in U-Boot proper

[Marek Behún]

Use the i2c_eeprom miscellaneous driver for reading Turris Omnia EEPROM
in U-Boot proper. Keep using dm_i2c_read() in SPL build, since adding
the i2c_eeprom driver to SPL build increases the image by 1.5 KiB.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 arch/arm/mach-mvebu/Kconfig              | 1 +
 board/CZ.NIC/turris_omnia/turris_omnia.c | 9 +++++++--
 configs/turris_omnia_defconfig           | 1 -
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
index 4a8328760eb..c1a1a333e6c 100644
--- a/arch/arm/mach-mvebu/Kconfig
+++ b/arch/arm/mach-mvebu/Kconfig
@@ -149,6 +149,7 @@ config TARGET_TURRIS_OMNIA
 	select SPL_SYS_MALLOC_SIMPLE
 	select SYS_I2C_MVTWSI
 	select ATSHA204A
+	select I2C_EEPROM
 	select ARMADA_38X_SUPPORT_OLD_DDR3_TRAINING
 
 config TARGET_TURRIS_MOX
diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
index 392df53a6d8..46f20f05c05 100644
--- a/board/CZ.NIC/turris_omnia/turris_omnia.c
+++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
@@ -23,6 +23,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <fdt_support.h>
 #include <hexdump.h>
+#include <i2c_eeprom.h>
 #include <time.h>
 #include <turris-omnia-mcu-interface.h>
 #include <linux/bitops.h>
@@ -473,9 +474,13 @@ static bool omnia_read_eeprom(struct omnia_eeprom *oep)
 	if (!eeprom)
 		return false;
 
-	ret = dm_i2c_read(eeprom, 0, (void *)oep, sizeof(*oep));
+	if (IS_ENABLED(CONFIG_SPL_BUILD))
+		ret = dm_i2c_read(eeprom, 0, (void *)oep, sizeof(*oep));
+	else
+		ret = i2c_eeprom_read(eeprom, 0, (void *)oep, sizeof(*oep));
+
 	if (ret) {
-		printf("dm_i2c_read failed: %i, cannot read EEPROM\n", ret);
+		printf("cannot read EEPROM: %d\n", ret);
 		return false;
 	}
 
diff --git a/configs/turris_omnia_defconfig b/configs/turris_omnia_defconfig
index c8756a3a788..93f0bc53f9e 100644
--- a/configs/turris_omnia_defconfig
+++ b/configs/turris_omnia_defconfig
@@ -92,7 +92,6 @@ CONFIG_SPL_OF_TRANSLATE=y
 CONFIG_AHCI_PCI=y
 CONFIG_AHCI_MVEBU=y
 CONFIG_DM_PCA953X=y
-CONFIG_I2C_EEPROM=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_MV=y
 CONFIG_DM_MTD=y
-- 
2.44.2

[PATCH u-boot-marvell 3/3] arm: mvebu: turris_omnia: Switch DDR speed to 1333H when reset 9 is selected

[Marek Behún]

Users experiencing random kernel crashes due to new versions of
Marvell's DDR training algorithm can solve the issue by setting DDR
speed to 1333H.

But if kernel crashes, it has to be done in U-Boot, which is impossible
without UART connection.

In order to make it easier for users, use the rescue button mechanism:
when rescue mode 9 is selected (that is when 10 LEDs are ON), U-Boot
will train DDR in 1333H mode and also update EEPROM so that subsequent
boot will use this mode.

User has to use the `eeprom` command in U-Boot or `omnia-eeprom` command
in OS to switch back to 1600K mode.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 board/CZ.NIC/turris_omnia/turris_omnia.c | 90 ++++++++++++++++++++----
 1 file changed, 77 insertions(+), 13 deletions(-)

diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
index 46f20f05c05..eb88ee7f1ed 100644
--- a/board/CZ.NIC/turris_omnia/turris_omnia.c
+++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
@@ -47,6 +47,9 @@ DECLARE_GLOBAL_DATA_PTR;
 #define OMNIA_I2C_EEPROM_CHIP_LEN	2
 #define OMNIA_I2C_EEPROM_MAGIC		0x0341a034
 
+#define OMNIA_RESET_TO_LOWER_DDR_SPEED	9
+#define OMNIA_LOWER_DDR_SPEED		"1333H"
+
 #define A385_SYS_RSTOUT_MASK		MVEBU_REGISTER(0x18260)
 #define   A385_SYS_RSTOUT_MASK_WD	BIT(10)
 
@@ -206,6 +209,20 @@ static u32 omnia_mcu_crc32(const void *p, size_t len)
 	return ~bitrev32(crc);
 }
 
+static int omnia_mcu_get_reset(void)
+{
+	u8 reset_status;
+	int ret;
+
+	ret = omnia_mcu_read(CMD_GET_RESET, &reset_status, 1);
+	if (ret) {
+		printf("omnia_mcu_read failed: %i, reset status unknown!\n", ret);
+		return ret;
+	}
+
+	return reset_status;
+}
+
 /* Can only be called after relocation, since it needs cleared BSS */
 static int omnia_mcu_board_info(char *serial, u8 *mac, char *version)
 {
@@ -463,14 +480,17 @@ static bool check_eeprom_crc(const void *buf, size_t size, u32 expected,
 	return true;
 }
 
+static struct udevice *omnia_get_eeprom(void)
+{
+	return omnia_get_i2c_chip("EEPROM", OMNIA_I2C_EEPROM_CHIP_ADDR,
+				  OMNIA_I2C_EEPROM_CHIP_LEN);
+}
+
 static bool omnia_read_eeprom(struct omnia_eeprom *oep)
 {
-	struct udevice *eeprom;
+	struct udevice *eeprom = omnia_get_eeprom();
 	int ret;
 
-	eeprom = omnia_get_i2c_chip("EEPROM", OMNIA_I2C_EEPROM_CHIP_ADDR,
-				    OMNIA_I2C_EEPROM_CHIP_LEN);
-
 	if (!eeprom)
 		return false;
 
@@ -502,6 +522,35 @@ static bool omnia_read_eeprom(struct omnia_eeprom *oep)
 	return true;
 }
 
+static void omnia_eeprom_set_lower_ddr_speed(void)
+{
+	struct udevice *eeprom = omnia_get_eeprom();
+	struct omnia_eeprom oep;
+	int ret;
+
+	if (!eeprom || !omnia_read_eeprom(&oep))
+		return;
+
+	puts("Setting DDR speed to " OMNIA_LOWER_DDR_SPEED " in EEPROM as requested by reset button... ");
+
+	/* check if already set */
+	if (!strncmp(oep.ddr_speed, OMNIA_LOWER_DDR_SPEED, sizeof(oep.ddr_speed)) &&
+	    (oep.old_ddr_training == 0 || oep.old_ddr_training == 0xff)) {
+		puts("was already set\n");
+		return;
+	}
+
+	strncpy(oep.ddr_speed, OMNIA_LOWER_DDR_SPEED, sizeof(oep.ddr_speed));
+	oep.old_ddr_training = 0xff;
+	oep.crc2 = crc32(0, (const void *)&oep, offsetof(struct omnia_eeprom, crc2));
+
+	ret = i2c_eeprom_write(eeprom, 0, (const void *)&oep, sizeof(oep));
+	if (ret)
+		printf("cannot write EEPROM: %d\n", ret);
+	else
+		puts("done\n");
+}
+
 int omnia_get_ram_size_gb(void)
 {
 	static int ram_size;
@@ -531,6 +580,13 @@ bool board_use_old_ddr3_training(void)
 {
 	struct omnia_eeprom oep;
 
+	/*
+	 * If lower DDR speed is requested by reset button, we can't use old DDR
+	 * training algorithm.
+	 */
+	if (omnia_mcu_get_reset() == OMNIA_RESET_TO_LOWER_DDR_SPEED)
+		return false;
+
 	if (!omnia_read_eeprom(&oep))
 		return false;
 
@@ -711,13 +767,19 @@ static void fixup_speed_in_ddr_topology(struct mv_ddr_topology_map *topology)
 	const struct omnia_ddr_speed *setting;
 	const char *speed;
 	static bool done;
+	int reset_status;
 
 	if (done)
 		return;
 
 	done = true;
 
-	speed = omnia_get_ddr_speed();
+	reset_status = omnia_mcu_get_reset();
+	if (reset_status == OMNIA_RESET_TO_LOWER_DDR_SPEED)
+		speed = OMNIA_LOWER_DDR_SPEED;
+	else
+		speed = omnia_get_ddr_speed();
+
 	if (!speed)
 		return;
 
@@ -734,7 +796,10 @@ static void fixup_speed_in_ddr_topology(struct mv_ddr_topology_map *topology)
 	if (params->speed_bin_index == setting->speed_bin)
 		return;
 
-	printf("Fixing up DDR3 speed (EEPROM defines %s)\n", speed);
+	if (reset_status == OMNIA_RESET_TO_LOWER_DDR_SPEED)
+		printf("Fixing up DDR3 speed to %s as requested by reset button\n", speed);
+	else
+		printf("Fixing up DDR3 speed (EEPROM defines %s)\n", speed);
 
 	params->speed_bin_index = setting->speed_bin;
 	params->memory_freq = setting->freq;
@@ -771,8 +836,7 @@ static int set_regdomain(void)
 static void handle_reset_button(void)
 {
 	const char * const vars[1] = { "bootcmd_rescue", };
-	int ret;
-	u8 reset_status;
+	int reset_status;
 
 	/*
 	 * Ensure that bootcmd_rescue has always stock value, so that running
@@ -781,12 +845,12 @@ static void handle_reset_button(void)
 	 */
 	env_set_default_vars(1, (char * const *)vars, 0);
 
-	ret = omnia_mcu_read(CMD_GET_RESET, &reset_status, 1);
-	if (ret) {
-		printf("omnia_mcu_read failed: %i, reset status unknown!\n",
-		       ret);
+	reset_status = omnia_mcu_get_reset();
+	if (reset_status < 0)
 		return;
-	}
+
+	if (reset_status == OMNIA_RESET_TO_LOWER_DDR_SPEED)
+		return omnia_eeprom_set_lower_ddr_speed();
 
 	env_set_ulong("omnia_reset", reset_status);
 
-- 
2.44.2

Re: [PATCH u-boot-marvell 1/3] arm: mvebu: turris_omnia: Rename variable holding EEPROM udevice

[Stefan Roese]

On 8/29/24 10:08, Marek Behún wrote:
> Rename the variable holding the EEPROM udevice from `chip` to `eeprom`.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>   board/CZ.NIC/turris_omnia/turris_omnia.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
> index 2f29d26edf8..392df53a6d8 100644
> --- a/board/CZ.NIC/turris_omnia/turris_omnia.c
> +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
> @@ -464,16 +464,16 @@ static bool check_eeprom_crc(const void *buf, size_t size, u32 expected,
>   
>   static bool omnia_read_eeprom(struct omnia_eeprom *oep)
>   {
> -	struct udevice *chip;
> +	struct udevice *eeprom;
>   	int ret;
>   
> -	chip = omnia_get_i2c_chip("EEPROM", OMNIA_I2C_EEPROM_CHIP_ADDR,
> -				  OMNIA_I2C_EEPROM_CHIP_LEN);
> +	eeprom = omnia_get_i2c_chip("EEPROM", OMNIA_I2C_EEPROM_CHIP_ADDR,
> +				    OMNIA_I2C_EEPROM_CHIP_LEN);
>   
> -	if (!chip)
> +	if (!eeprom)
>   		return false;
>   
> -	ret = dm_i2c_read(chip, 0, (void *)oep, sizeof(*oep));
> +	ret = dm_i2c_read(eeprom, 0, (void *)oep, sizeof(*oep));
>   	if (ret) {
>   		printf("dm_i2c_read failed: %i, cannot read EEPROM\n", ret);
>   		return false;

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

Re: [PATCH u-boot-marvell 2/3] arm: mvebu: turris_omnia: Use the i2c_eeprom misc driver for EEPROM reading in U-Boot proper

[Stefan Roese]

On 8/29/24 10:08, Marek Behún wrote:
> Use the i2c_eeprom miscellaneous driver for reading Turris Omnia EEPROM
> in U-Boot proper. Keep using dm_i2c_read() in SPL build, since adding
> the i2c_eeprom driver to SPL build increases the image by 1.5 KiB.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>   arch/arm/mach-mvebu/Kconfig              | 1 +
>   board/CZ.NIC/turris_omnia/turris_omnia.c | 9 +++++++--
>   configs/turris_omnia_defconfig           | 1 -
>   3 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> index 4a8328760eb..c1a1a333e6c 100644
> --- a/arch/arm/mach-mvebu/Kconfig
> +++ b/arch/arm/mach-mvebu/Kconfig
> @@ -149,6 +149,7 @@ config TARGET_TURRIS_OMNIA
>   	select SPL_SYS_MALLOC_SIMPLE
>   	select SYS_I2C_MVTWSI
>   	select ATSHA204A
> +	select I2C_EEPROM
>   	select ARMADA_38X_SUPPORT_OLD_DDR3_TRAINING
>   
>   config TARGET_TURRIS_MOX
> diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
> index 392df53a6d8..46f20f05c05 100644
> --- a/board/CZ.NIC/turris_omnia/turris_omnia.c
> +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
> @@ -23,6 +23,7 @@
>   #include <dt-bindings/gpio/gpio.h>
>   #include <fdt_support.h>
>   #include <hexdump.h>
> +#include <i2c_eeprom.h>
>   #include <time.h>
>   #include <turris-omnia-mcu-interface.h>
>   #include <linux/bitops.h>
> @@ -473,9 +474,13 @@ static bool omnia_read_eeprom(struct omnia_eeprom *oep)
>   	if (!eeprom)
>   		return false;
>   
> -	ret = dm_i2c_read(eeprom, 0, (void *)oep, sizeof(*oep));
> +	if (IS_ENABLED(CONFIG_SPL_BUILD))
> +		ret = dm_i2c_read(eeprom, 0, (void *)oep, sizeof(*oep));
> +	else
> +		ret = i2c_eeprom_read(eeprom, 0, (void *)oep, sizeof(*oep));

So whats the advantage using i2c_eeprom_read() here in U-Boot proper?
The only thing that stands out to me is that the code get more complex
because of the SPL/non-SPL distinction.

Thanks,
Stefan

> +
>   	if (ret) {
> -		printf("dm_i2c_read failed: %i, cannot read EEPROM\n", ret);
> +		printf("cannot read EEPROM: %d\n", ret);
>   		return false;
>   	}
>   
> diff --git a/configs/turris_omnia_defconfig b/configs/turris_omnia_defconfig
> index c8756a3a788..93f0bc53f9e 100644
> --- a/configs/turris_omnia_defconfig
> +++ b/configs/turris_omnia_defconfig
> @@ -92,7 +92,6 @@ CONFIG_SPL_OF_TRANSLATE=y
>   CONFIG_AHCI_PCI=y
>   CONFIG_AHCI_MVEBU=y
>   CONFIG_DM_PCA953X=y
> -CONFIG_I2C_EEPROM=y
>   CONFIG_MMC_SDHCI=y
>   CONFIG_MMC_SDHCI_MV=y
>   CONFIG_DM_MTD=y

Re: [PATCH u-boot-marvell 3/3] arm: mvebu: turris_omnia: Switch DDR speed to 1333H when reset 9 is selected

[Stefan Roese]

On 8/29/24 10:08, Marek Behún wrote:
> Users experiencing random kernel crashes due to new versions of
> Marvell's DDR training algorithm can solve the issue by setting DDR
> speed to 1333H.
> 
> But if kernel crashes, it has to be done in U-Boot, which is impossible
> without UART connection.
> 
> In order to make it easier for users, use the rescue button mechanism:
> when rescue mode 9 is selected (that is when 10 LEDs are ON), U-Boot
> will train DDR in 1333H mode and also update EEPROM so that subsequent
> boot will use this mode.
> 
> User has to use the `eeprom` command in U-Boot or `omnia-eeprom` command
> in OS to switch back to 1600K mode.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>   board/CZ.NIC/turris_omnia/turris_omnia.c | 90 ++++++++++++++++++++----
>   1 file changed, 77 insertions(+), 13 deletions(-)
> 
> diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
> index 46f20f05c05..eb88ee7f1ed 100644
> --- a/board/CZ.NIC/turris_omnia/turris_omnia.c
> +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
> @@ -47,6 +47,9 @@ DECLARE_GLOBAL_DATA_PTR;
>   #define OMNIA_I2C_EEPROM_CHIP_LEN	2
>   #define OMNIA_I2C_EEPROM_MAGIC		0x0341a034
>   
> +#define OMNIA_RESET_TO_LOWER_DDR_SPEED	9
> +#define OMNIA_LOWER_DDR_SPEED		"1333H"
> +
>   #define A385_SYS_RSTOUT_MASK		MVEBU_REGISTER(0x18260)
>   #define   A385_SYS_RSTOUT_MASK_WD	BIT(10)
>   
> @@ -206,6 +209,20 @@ static u32 omnia_mcu_crc32(const void *p, size_t len)
>   	return ~bitrev32(crc);
>   }
>   
> +static int omnia_mcu_get_reset(void)
> +{
> +	u8 reset_status;
> +	int ret;
> +
> +	ret = omnia_mcu_read(CMD_GET_RESET, &reset_status, 1);
> +	if (ret) {
> +		printf("omnia_mcu_read failed: %i, reset status unknown!\n", ret);
> +		return ret;
> +	}
> +
> +	return reset_status;
> +}
> +
>   /* Can only be called after relocation, since it needs cleared BSS */
>   static int omnia_mcu_board_info(char *serial, u8 *mac, char *version)
>   {
> @@ -463,14 +480,17 @@ static bool check_eeprom_crc(const void *buf, size_t size, u32 expected,
>   	return true;
>   }
>   
> +static struct udevice *omnia_get_eeprom(void)
> +{
> +	return omnia_get_i2c_chip("EEPROM", OMNIA_I2C_EEPROM_CHIP_ADDR,
> +				  OMNIA_I2C_EEPROM_CHIP_LEN);
> +}
> +
>   static bool omnia_read_eeprom(struct omnia_eeprom *oep)
>   {
> -	struct udevice *eeprom;
> +	struct udevice *eeprom = omnia_get_eeprom();
>   	int ret;
>   
> -	eeprom = omnia_get_i2c_chip("EEPROM", OMNIA_I2C_EEPROM_CHIP_ADDR,
> -				    OMNIA_I2C_EEPROM_CHIP_LEN);
> -
>   	if (!eeprom)
>   		return false;
>   
> @@ -502,6 +522,35 @@ static bool omnia_read_eeprom(struct omnia_eeprom *oep)
>   	return true;
>   }
>   
> +static void omnia_eeprom_set_lower_ddr_speed(void)
> +{
> +	struct udevice *eeprom = omnia_get_eeprom();
> +	struct omnia_eeprom oep;
> +	int ret;
> +
> +	if (!eeprom || !omnia_read_eeprom(&oep))
> +		return;
> +
> +	puts("Setting DDR speed to " OMNIA_LOWER_DDR_SPEED " in EEPROM as requested by reset button... ");
> +
> +	/* check if already set */
> +	if (!strncmp(oep.ddr_speed, OMNIA_LOWER_DDR_SPEED, sizeof(oep.ddr_speed)) &&
> +	    (oep.old_ddr_training == 0 || oep.old_ddr_training == 0xff)) {
> +		puts("was already set\n");
> +		return;
> +	}
> +
> +	strncpy(oep.ddr_speed, OMNIA_LOWER_DDR_SPEED, sizeof(oep.ddr_speed));
> +	oep.old_ddr_training = 0xff;
> +	oep.crc2 = crc32(0, (const void *)&oep, offsetof(struct omnia_eeprom, crc2));
> +
> +	ret = i2c_eeprom_write(eeprom, 0, (const void *)&oep, sizeof(oep));
> +	if (ret)
> +		printf("cannot write EEPROM: %d\n", ret);
> +	else
> +		puts("done\n");
> +}
> +
>   int omnia_get_ram_size_gb(void)
>   {
>   	static int ram_size;
> @@ -531,6 +580,13 @@ bool board_use_old_ddr3_training(void)
>   {
>   	struct omnia_eeprom oep;
>   
> +	/*
> +	 * If lower DDR speed is requested by reset button, we can't use old DDR
> +	 * training algorithm.
> +	 */
> +	if (omnia_mcu_get_reset() == OMNIA_RESET_TO_LOWER_DDR_SPEED)
> +		return false;
> +
>   	if (!omnia_read_eeprom(&oep))
>   		return false;
>   
> @@ -711,13 +767,19 @@ static void fixup_speed_in_ddr_topology(struct mv_ddr_topology_map *topology)
>   	const struct omnia_ddr_speed *setting;
>   	const char *speed;
>   	static bool done;
> +	int reset_status;
>   
>   	if (done)
>   		return;
>   
>   	done = true;
>   
> -	speed = omnia_get_ddr_speed();
> +	reset_status = omnia_mcu_get_reset();
> +	if (reset_status == OMNIA_RESET_TO_LOWER_DDR_SPEED)
> +		speed = OMNIA_LOWER_DDR_SPEED;
> +	else
> +		speed = omnia_get_ddr_speed();
> +
>   	if (!speed)
>   		return;
>   
> @@ -734,7 +796,10 @@ static void fixup_speed_in_ddr_topology(struct mv_ddr_topology_map *topology)
>   	if (params->speed_bin_index == setting->speed_bin)
>   		return;
>   
> -	printf("Fixing up DDR3 speed (EEPROM defines %s)\n", speed);
> +	if (reset_status == OMNIA_RESET_TO_LOWER_DDR_SPEED)
> +		printf("Fixing up DDR3 speed to %s as requested by reset button\n", speed);
> +	else
> +		printf("Fixing up DDR3 speed (EEPROM defines %s)\n", speed);
>   
>   	params->speed_bin_index = setting->speed_bin;
>   	params->memory_freq = setting->freq;
> @@ -771,8 +836,7 @@ static int set_regdomain(void)
>   static void handle_reset_button(void)
>   {
>   	const char * const vars[1] = { "bootcmd_rescue", };
> -	int ret;
> -	u8 reset_status;
> +	int reset_status;
>   
>   	/*
>   	 * Ensure that bootcmd_rescue has always stock value, so that running
> @@ -781,12 +845,12 @@ static void handle_reset_button(void)
>   	 */
>   	env_set_default_vars(1, (char * const *)vars, 0);
>   
> -	ret = omnia_mcu_read(CMD_GET_RESET, &reset_status, 1);
> -	if (ret) {
> -		printf("omnia_mcu_read failed: %i, reset status unknown!\n",
> -		       ret);
> +	reset_status = omnia_mcu_get_reset();
> +	if (reset_status < 0)
>   		return;
> -	}
> +
> +	if (reset_status == OMNIA_RESET_TO_LOWER_DDR_SPEED)
> +		return omnia_eeprom_set_lower_ddr_speed();
>   
>   	env_set_ulong("omnia_reset", reset_status);
>   

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

Re: [PATCH u-boot-marvell 2/3] arm: mvebu: turris_omnia: Use the i2c_eeprom misc driver for EEPROM reading in U-Boot proper

[Marek Behún]

On Tue, Sep 03, 2024 at 08:57:24AM +0200, Stefan Roese wrote:
> On 8/29/24 10:08, Marek Behún wrote:
> > Use the i2c_eeprom miscellaneous driver for reading Turris Omnia EEPROM
> > in U-Boot proper. Keep using dm_i2c_read() in SPL build, since adding
> > the i2c_eeprom driver to SPL build increases the image by 1.5 KiB.
> > 
> > Signed-off-by: Marek Behún <kabel@kernel.org>
> > ---
> >   arch/arm/mach-mvebu/Kconfig              | 1 +
> >   board/CZ.NIC/turris_omnia/turris_omnia.c | 9 +++++++--
> >   configs/turris_omnia_defconfig           | 1 -
> >   3 files changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
> > index 4a8328760eb..c1a1a333e6c 100644
> > --- a/arch/arm/mach-mvebu/Kconfig
> > +++ b/arch/arm/mach-mvebu/Kconfig
> > @@ -149,6 +149,7 @@ config TARGET_TURRIS_OMNIA
> >   	select SPL_SYS_MALLOC_SIMPLE
> >   	select SYS_I2C_MVTWSI
> >   	select ATSHA204A
> > +	select I2C_EEPROM
> >   	select ARMADA_38X_SUPPORT_OLD_DDR3_TRAINING
> >   config TARGET_TURRIS_MOX
> > diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
> > index 392df53a6d8..46f20f05c05 100644
> > --- a/board/CZ.NIC/turris_omnia/turris_omnia.c
> > +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
> > @@ -23,6 +23,7 @@
> >   #include <dt-bindings/gpio/gpio.h>
> >   #include <fdt_support.h>
> >   #include <hexdump.h>
> > +#include <i2c_eeprom.h>
> >   #include <time.h>
> >   #include <turris-omnia-mcu-interface.h>
> >   #include <linux/bitops.h>
> > @@ -473,9 +474,13 @@ static bool omnia_read_eeprom(struct omnia_eeprom *oep)
> >   	if (!eeprom)
> >   		return false;
> > -	ret = dm_i2c_read(eeprom, 0, (void *)oep, sizeof(*oep));
> > +	if (IS_ENABLED(CONFIG_SPL_BUILD))
> > +		ret = dm_i2c_read(eeprom, 0, (void *)oep, sizeof(*oep));
> > +	else
> > +		ret = i2c_eeprom_read(eeprom, 0, (void *)oep, sizeof(*oep));
> 
> So whats the advantage using i2c_eeprom_read() here in U-Boot proper?
> The only thing that stands out to me is that the code get more complex
> because of the SPL/non-SPL distinction.

The reasons are as follows:
- it's "more correct" semantically

- the i2c_eeprom driver was already enabled for U-Boot proper in
  defconfig (with this commit selected by the board), because of the
  `eeprom` commmand, so I thought "why not use it?"
  (looking at the code now, it goes
     i2c_eeprom_read()
       dereference of i2c_eeprom_ops->read method
       i2c_eeprom_std_read()
         dm_i2c_read()
   so the complication is trivial, IMO)

- the following commit adds code that writes the eeprom, and this cannot
  be done with simple dm_i2c_write(), because each eeprom page has to be
  written by another transaction, which is what i2c_eeprom_write()
  does.

  So I knew that I was going to add i2c_eeprom_write(), and I thought
  that it would seem weird writing with i2c_eeprom_write() but reading
  with dm_i2c_read().

Marek

Re: [PATCH u-boot-marvell 2/3] arm: mvebu: turris_omnia: Use the i2c_eeprom misc driver for EEPROM reading in U-Boot proper

[Stefan Roese]

On 9/3/24 10:04, Marek Behún wrote:
> On Tue, Sep 03, 2024 at 08:57:24AM +0200, Stefan Roese wrote:
>> On 8/29/24 10:08, Marek Behún wrote:
>>> Use the i2c_eeprom miscellaneous driver for reading Turris Omnia EEPROM
>>> in U-Boot proper. Keep using dm_i2c_read() in SPL build, since adding
>>> the i2c_eeprom driver to SPL build increases the image by 1.5 KiB.
>>>
>>> Signed-off-by: Marek Behún <kabel@kernel.org>
>>> ---
>>>    arch/arm/mach-mvebu/Kconfig              | 1 +
>>>    board/CZ.NIC/turris_omnia/turris_omnia.c | 9 +++++++--
>>>    configs/turris_omnia_defconfig           | 1 -
>>>    3 files changed, 8 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
>>> index 4a8328760eb..c1a1a333e6c 100644
>>> --- a/arch/arm/mach-mvebu/Kconfig
>>> +++ b/arch/arm/mach-mvebu/Kconfig
>>> @@ -149,6 +149,7 @@ config TARGET_TURRIS_OMNIA
>>>    	select SPL_SYS_MALLOC_SIMPLE
>>>    	select SYS_I2C_MVTWSI
>>>    	select ATSHA204A
>>> +	select I2C_EEPROM
>>>    	select ARMADA_38X_SUPPORT_OLD_DDR3_TRAINING
>>>    config TARGET_TURRIS_MOX
>>> diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
>>> index 392df53a6d8..46f20f05c05 100644
>>> --- a/board/CZ.NIC/turris_omnia/turris_omnia.c
>>> +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
>>> @@ -23,6 +23,7 @@
>>>    #include <dt-bindings/gpio/gpio.h>
>>>    #include <fdt_support.h>
>>>    #include <hexdump.h>
>>> +#include <i2c_eeprom.h>
>>>    #include <time.h>
>>>    #include <turris-omnia-mcu-interface.h>
>>>    #include <linux/bitops.h>
>>> @@ -473,9 +474,13 @@ static bool omnia_read_eeprom(struct omnia_eeprom *oep)
>>>    	if (!eeprom)
>>>    		return false;
>>> -	ret = dm_i2c_read(eeprom, 0, (void *)oep, sizeof(*oep));
>>> +	if (IS_ENABLED(CONFIG_SPL_BUILD))
>>> +		ret = dm_i2c_read(eeprom, 0, (void *)oep, sizeof(*oep));
>>> +	else
>>> +		ret = i2c_eeprom_read(eeprom, 0, (void *)oep, sizeof(*oep));
>>
>> So whats the advantage using i2c_eeprom_read() here in U-Boot proper?
>> The only thing that stands out to me is that the code get more complex
>> because of the SPL/non-SPL distinction.
> 
> The reasons are as follows:
> - it's "more correct" semantically
> 
> - the i2c_eeprom driver was already enabled for U-Boot proper in
>    defconfig (with this commit selected by the board), because of the
>    `eeprom` commmand, so I thought "why not use it?"
>    (looking at the code now, it goes
>       i2c_eeprom_read()
>         dereference of i2c_eeprom_ops->read method
>         i2c_eeprom_std_read()
>           dm_i2c_read()
>     so the complication is trivial, IMO)
> 
> - the following commit adds code that writes the eeprom, and this cannot
>    be done with simple dm_i2c_write(), because each eeprom page has to be
>    written by another transaction, which is what i2c_eeprom_write()
>    does.
> 
>    So I knew that I was going to add i2c_eeprom_write(), and I thought
>    that it would seem weird writing with i2c_eeprom_write() but reading
>    with dm_i2c_read().

That's what I thought after reviewing patch 3/3. Okay, makes sense.
Let's see, if we can pull this into the upcoming release.

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

Re: [PATCH u-boot-marvell 0/3] Turris Omnia DDR speed change with reset button (for 2024.10)

[Stefan Roese]

On 8/29/24 10:08, Marek Behún wrote:
> Hello Stefan,
> 
> this series adds the ability to select 1333H DDR speed mode for DDR
> training (instead of the default 1600K) with the factory reset button.
> 
> If possible, I would like if this series went for U-Boot 2024.10
> release. The heavy stuff to support DDR speed change is already in,
> this just adds the ability to do it with reset button instead of
> console command (which is inconvenient for most users).
> 
> On Omnia, the user can select factory reset type by pressing the reset
> button on the rear panel and holding it. The front LEDs light on in
> sequence, representing a counter. Every ~2.5 second another LED lights
> on. When only the first LED is on, the board will do a standard reboot.
> Then different kinds of factory reboot will happen for reset types 1 to
> 7 (when 1, 2, ... or 7 LEDs are on) [1].
> 
> This series adds another reset type, number 10, which will make the
> board boot in 1333H DDR speed mode, and also write this setting to
> EEPROM, so that subsequent boots will also train in 1333H mode.
> 
> The user can set back the default mode with the `eeprom` U-Boot command,
> or with the `omnia-eeprom` Linux command.

Applied to u-boot-marvell/master

Thanks,
Stefan

> Marek
> 
> [1] https://docs.turris.cz/hw/omnia/rescue-modes/
> 
> Marek Behún (3):
>    arm: mvebu: turris_omnia: Rename variable holding EEPROM udevice
>    arm: mvebu: turris_omnia: Use the i2c_eeprom misc driver for EEPROM
>      reading in U-Boot proper
>    arm: mvebu: turris_omnia: Switch DDR speed to 1333H when reset 9 is
>      selected
> 
>   arch/arm/mach-mvebu/Kconfig              |   1 +
>   board/CZ.NIC/turris_omnia/turris_omnia.c | 101 +++++++++++++++++++----
>   configs/turris_omnia_defconfig           |   1 -
>   3 files changed, 86 insertions(+), 17 deletions(-)
> 

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de