[PATCH v2 1/3] mx6sabresd: Fix U-Boot corruption after saving the environment
From: Fabio Estevam festevam@denx.de
U-Boot binary has grown in such a way that it goes beyond the reserved area for the environment variables.
Running "saveenv" and rebooting the board causes U-Boot to hang because of this overlap.
Fix this problem by selecting CONFIG_LTO so that the U-Boot proper size can be reduced.
Also, to prevent this same problem to happen in the future, use CONFIG_BOARD_SIZE_LIMIT, which can detect the overlap in build-time.
CONFIG_BOARD_SIZE_LIMIT is calculated as follows:
CONFIG_BOARD_SIZE_LIMIT = CONFIG_ENV_OFFSET - u-boot-img.dtb offset CONFIG_BOARD_SIZE_LIMIT = 0xc000 - 69 * 1024 CONFIG_BOARD_SIZE_LIMIT = 715766
Signed-off-by: Fabio Estevam festevam@denx.de --- Changes since v1: - Select LTO. (Tom)
configs/mx6sabresd_defconfig | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/configs/mx6sabresd_defconfig b/configs/mx6sabresd_defconfig index a90efe4a7786..d4fb55622cf1 100644 --- a/configs/mx6sabresd_defconfig +++ b/configs/mx6sabresd_defconfig @@ -21,6 +21,9 @@ CONFIG_SPL_SERIAL=y CONFIG_SPL=y CONFIG_SPL_LIBDISK_SUPPORT=y CONFIG_PCI=y +CONFIG_LTO=y +CONFIG_HAS_BOARD_SIZE_LIMIT=y +CONFIG_BOARD_SIZE_LIMIT=715766 CONFIG_FIT=y CONFIG_SPL_FIT_PRINT=y CONFIG_SPL_LOAD_FIT=y
From: Fabio Estevam festevam@denx.de
Commit 68dcbdd594d4 ("ARM: imx: Add weak default reset_cpu()") caused the 'reset' command in U-Boot to not cause a board reset.
Fix it by switching to the watchdog driver model via sysreset, which is the preferred method for implementing the watchdog reset.
Signed-off-by: Fabio Estevam festevam@denx.de --- Changes since v1: - None.
arch/arm/dts/imx6qdl-sabresd-u-boot.dtsi | 9 +++++++++ configs/mx6sabresd_defconfig | 3 +++ 2 files changed, 12 insertions(+)
diff --git a/arch/arm/dts/imx6qdl-sabresd-u-boot.dtsi b/arch/arm/dts/imx6qdl-sabresd-u-boot.dtsi index 5c4101b76da2..9e9c4422f00e 100644 --- a/arch/arm/dts/imx6qdl-sabresd-u-boot.dtsi +++ b/arch/arm/dts/imx6qdl-sabresd-u-boot.dtsi @@ -9,6 +9,11 @@ aliases { mmc1 = &usdhc3; }; + wdt-reboot { + compatible = "wdt-reboot"; + wdt = <&wdog2>; + bootph-pre-ram; + }; };
&usdhc3 { @@ -18,3 +23,7 @@ &pinctrl_usdhc3 { bootph-pre-ram; }; + +&wdog2 { + bootph-pre-ram; +}; diff --git a/configs/mx6sabresd_defconfig b/configs/mx6sabresd_defconfig index f19df607e7ae..69685f12eb10 100644 --- a/configs/mx6sabresd_defconfig +++ b/configs/mx6sabresd_defconfig @@ -103,6 +103,8 @@ CONFIG_DM_REGULATOR_FIXED=y CONFIG_DM_REGULATOR_GPIO=y CONFIG_DM_SERIAL=y CONFIG_MXC_UART=y +CONFIG_SYSRESET=y +CONFIG_SYSRESET_WATCHDOG=y CONFIG_SPI=y CONFIG_DM_SPI=y CONFIG_MXC_SPI=y @@ -129,3 +131,4 @@ CONFIG_IMX_HDMI=y CONFIG_SPLASH_SCREEN=y CONFIG_SPLASH_SCREEN_ALIGN=y CONFIG_BMP_16BPP=y +CONFIG_IMX_WATCHDOG=y
On Fri, Feb 02, 2024 at 01:04:04PM -0300, Fabio Estevam wrote:
From: Fabio Estevam festevam@denx.de
Commit 68dcbdd594d4 ("ARM: imx: Add weak default reset_cpu()") caused the 'reset' command in U-Boot to not cause a board reset.
Fix it by switching to the watchdog driver model via sysreset, which is the preferred method for implementing the watchdog reset.
Signed-off-by: Fabio Estevam festevam@denx.de
Changes since v1:
- None.
arch/arm/dts/imx6qdl-sabresd-u-boot.dtsi | 9 +++++++++ configs/mx6sabresd_defconfig | 3 +++ 2 files changed, 12 insertions(+)
diff --git a/arch/arm/dts/imx6qdl-sabresd-u-boot.dtsi b/arch/arm/dts/imx6qdl-sabresd-u-boot.dtsi index 5c4101b76da2..9e9c4422f00e 100644 --- a/arch/arm/dts/imx6qdl-sabresd-u-boot.dtsi +++ b/arch/arm/dts/imx6qdl-sabresd-u-boot.dtsi @@ -9,6 +9,11 @@ aliases { mmc1 = &usdhc3; };
- wdt-reboot {
compatible = "wdt-reboot";wdt = <&wdog2>;bootph-pre-ram;- };
};
&usdhc3 { @@ -18,3 +23,7 @@ &pinctrl_usdhc3 { bootph-pre-ram; };
+&wdog2 {
- bootph-pre-ram;
+};
Can all of this be upstreamed as well?
On Fri, Feb 2, 2024 at 1:07 PM Tom Rini trini@konsulko.com wrote:
Can all of this be upstreamed as well?
I can try it, but that would go into kernel 6.9 in the best case.
For now, let's keep it in u-boot.dtsi.
On Fri, Feb 02, 2024 at 01:17:06PM -0300, Fabio Estevam wrote:
On Fri, Feb 2, 2024 at 1:07 PM Tom Rini trini@konsulko.com wrote:
Can all of this be upstreamed as well?
I can try it, but that would go into kernel 6.9 in the best case.
For now, let's keep it in u-boot.dtsi.
Yes, sorry, I'm not saying don't do this here, but can you please submit it so that long term this can be converted to OF_UPSTREAM and also not have a -u-boot.dtsi potentially.
From: Fabio Estevam festevam@denx.de
With Ethernet DM in place, there is no longer the need for having the board_phy_config() anymore.
Remove it.
Confirmed that TFTP transfer still works fine without board_phy_config().
Signed-off-by: Fabio Estevam festevam@denx.de --- Changes since v1: - Newly introduced.
board/freescale/mx6sabresd/mx6sabresd.c | 34 ------------------------- 1 file changed, 34 deletions(-)
diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c index b558a596dff8..bb066a5d36c3 100644 --- a/board/freescale/mx6sabresd/mx6sabresd.c +++ b/board/freescale/mx6sabresd/mx6sabresd.c @@ -7,7 +7,6 @@
#include <image.h> #include <init.h> -#include <net.h> #include <asm/arch/clock.h> #include <asm/arch/imx-regs.h> #include <asm/arch/iomux.h> @@ -254,39 +253,6 @@ int board_mmc_init(struct bd_info *bis) } #endif
-static int ar8031_phy_fixup(struct phy_device *phydev) -{ - unsigned short val; - - /* To enable AR8031 ouput a 125MHz clk from CLK_25M */ - phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x7); - phy_write(phydev, MDIO_DEVAD_NONE, 0xe, 0x8016); - phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x4007); - - val = phy_read(phydev, MDIO_DEVAD_NONE, 0xe); - val &= 0xffe3; - val |= 0x18; - phy_write(phydev, MDIO_DEVAD_NONE, 0xe, val); - - /* introduce tx clock delay */ - phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x5); - val = phy_read(phydev, MDIO_DEVAD_NONE, 0x1e); - val |= 0x0100; - phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, val); - - return 0; -} - -int board_phy_config(struct phy_device *phydev) -{ - ar8031_phy_fixup(phydev); - - if (phydev->drv->config) - phydev->drv->config(phydev); - - return 0; -} - #if defined(CONFIG_VIDEO_IPUV3) static void disable_lvds(struct display_info_t const *dev) {
Hello Fabio,
On Fri, Feb 2, 2024 at 5:04 PM Fabio Estevam festevam@gmail.com wrote:
From: Fabio Estevam festevam@denx.de
With Ethernet DM in place, there is no longer the need for having the board_phy_config() anymore.
Remove it.
Confirmed that TFTP transfer still works fine without board_phy_config().
Signed-off-by: Fabio Estevam festevam@denx.de
Changes since v1:
- Newly introduced.
board/freescale/mx6sabresd/mx6sabresd.c | 34 ------------------------- 1 file changed, 34 deletions(-)
diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c index b558a596dff8..bb066a5d36c3 100644 --- a/board/freescale/mx6sabresd/mx6sabresd.c +++ b/board/freescale/mx6sabresd/mx6sabresd.c @@ -7,7 +7,6 @@
#include <image.h> #include <init.h> -#include <net.h> #include <asm/arch/clock.h> #include <asm/arch/imx-regs.h> #include <asm/arch/iomux.h> @@ -254,39 +253,6 @@ int board_mmc_init(struct bd_info *bis) } #endif
-static int ar8031_phy_fixup(struct phy_device *phydev) -{
unsigned short val;/* To enable AR8031 ouput a 125MHz clk from CLK_25M */phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x7);phy_write(phydev, MDIO_DEVAD_NONE, 0xe, 0x8016);phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x4007);val = phy_read(phydev, MDIO_DEVAD_NONE, 0xe);val &= 0xffe3;val |= 0x18;phy_write(phydev, MDIO_DEVAD_NONE, 0xe, val);/* introduce tx clock delay */phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x5);val = phy_read(phydev, MDIO_DEVAD_NONE, 0x1e);val |= 0x0100;phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, val);return 0;-}
-int board_phy_config(struct phy_device *phydev) -{
ar8031_phy_fixup(phydev);if (phydev->drv->config)phydev->drv->config(phydev);return 0;-}
#if defined(CONFIG_VIDEO_IPUV3) static void disable_lvds(struct display_info_t const *dev) { -- 2.34.1
Reviewed-by: Igor Opaniuk igor.opaniuk@foundries.io
On Fri, Feb 02, 2024 at 01:04:03PM -0300, Fabio Estevam wrote:
From: Fabio Estevam festevam@denx.de
U-Boot binary has grown in such a way that it goes beyond the reserved area for the environment variables.
Running "saveenv" and rebooting the board causes U-Boot to hang because of this overlap.
Fix this problem by selecting CONFIG_LTO so that the U-Boot proper size can be reduced.
Also, to prevent this same problem to happen in the future, use CONFIG_BOARD_SIZE_LIMIT, which can detect the overlap in build-time.
CONFIG_BOARD_SIZE_LIMIT is calculated as follows:
CONFIG_BOARD_SIZE_LIMIT = CONFIG_ENV_OFFSET - u-boot-img.dtb offset CONFIG_BOARD_SIZE_LIMIT = 0xc000 - 69 * 1024 CONFIG_BOARD_SIZE_LIMIT = 715766
Signed-off-by: Fabio Estevam festevam@denx.de
Thanks!
Reviewed-by: Tom Rini trini@konsulko.com
On Fri, Feb 2, 2024 at 5:04 PM Fabio Estevam festevam@gmail.com wrote:
From: Fabio Estevam festevam@denx.de
U-Boot binary has grown in such a way that it goes beyond the reserved area for the environment variables.
Running "saveenv" and rebooting the board causes U-Boot to hang because of this overlap.
Fix this problem by selecting CONFIG_LTO so that the U-Boot proper size can be reduced.
Also, to prevent this same problem to happen in the future, use CONFIG_BOARD_SIZE_LIMIT, which can detect the overlap in build-time.
CONFIG_BOARD_SIZE_LIMIT is calculated as follows:
CONFIG_BOARD_SIZE_LIMIT = CONFIG_ENV_OFFSET - u-boot-img.dtb offset CONFIG_BOARD_SIZE_LIMIT = 0xc000 - 69 * 1024 CONFIG_BOARD_SIZE_LIMIT = 715766
Signed-off-by: Fabio Estevam festevam@denx.de
Changes since v1:
- Select LTO. (Tom)
configs/mx6sabresd_defconfig | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/configs/mx6sabresd_defconfig b/configs/mx6sabresd_defconfig index a90efe4a7786..d4fb55622cf1 100644 --- a/configs/mx6sabresd_defconfig +++ b/configs/mx6sabresd_defconfig @@ -21,6 +21,9 @@ CONFIG_SPL_SERIAL=y CONFIG_SPL=y CONFIG_SPL_LIBDISK_SUPPORT=y CONFIG_PCI=y +CONFIG_LTO=y +CONFIG_HAS_BOARD_SIZE_LIMIT=y +CONFIG_BOARD_SIZE_LIMIT=715766 CONFIG_FIT=y CONFIG_SPL_FIT_PRINT=y CONFIG_SPL_LOAD_FIT=y -- 2.34.1
Reviewed-by: Igor Opaniuk igor.opaniuk@foundries.io
btw, from my experience, the same issue will probably occur again in future, especially when the current U-Boot size is already close to the defined limit (yes, with this patch it will be reported during compile time, but still).
Have you considered adjusting the boot image layout instead (moving CONFIG_ENV_OFFSET, so unlikely U-Boot image will overlap again) to tackle feature problems?
On Fri, Feb 2, 2024 at 9:34 PM Igor Opaniuk igor.opaniuk@foundries.io wrote:
On Fri, Feb 2, 2024 at 5:04 PM Fabio Estevam festevam@gmail.com wrote:
From: Fabio Estevam festevam@denx.de
U-Boot binary has grown in such a way that it goes beyond the reserved area for the environment variables.
Running "saveenv" and rebooting the board causes U-Boot to hang because of this overlap.
Fix this problem by selecting CONFIG_LTO so that the U-Boot proper size can be reduced.
Also, to prevent this same problem to happen in the future, use CONFIG_BOARD_SIZE_LIMIT, which can detect the overlap in build-time.
CONFIG_BOARD_SIZE_LIMIT is calculated as follows:
CONFIG_BOARD_SIZE_LIMIT = CONFIG_ENV_OFFSET - u-boot-img.dtb offset CONFIG_BOARD_SIZE_LIMIT = 0xc000 - 69 * 1024 CONFIG_BOARD_SIZE_LIMIT = 715766
Signed-off-by: Fabio Estevam festevam@denx.de
Changes since v1:
- Select LTO. (Tom)
configs/mx6sabresd_defconfig | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/configs/mx6sabresd_defconfig b/configs/mx6sabresd_defconfig index a90efe4a7786..d4fb55622cf1 100644 --- a/configs/mx6sabresd_defconfig +++ b/configs/mx6sabresd_defconfig @@ -21,6 +21,9 @@ CONFIG_SPL_SERIAL=y CONFIG_SPL=y CONFIG_SPL_LIBDISK_SUPPORT=y CONFIG_PCI=y +CONFIG_LTO=y +CONFIG_HAS_BOARD_SIZE_LIMIT=y +CONFIG_BOARD_SIZE_LIMIT=715766 CONFIG_FIT=y CONFIG_SPL_FIT_PRINT=y CONFIG_SPL_LOAD_FIT=y -- 2.34.1
Reviewed-by: Igor Opaniuk igor.opaniuk@foundries.io
btw, from my experience, the same issue will probably occur again in future, especially when the current U-Boot size is already close to the defined limit (yes, with this patch it will be reported during compile time, but still).
Have you considered adjusting the boot image layout instead (moving CONFIG_ENV_OFFSET, so unlikely U-Boot image will overlap again) to tackle feature problems?
I've just checked the previous patch version, where you addressed that exactly with CONFIG_ENV_OFFSET change and following suggestions/objections from Tom, so sorry for the noise :)
-- Best regards - Freundliche Grüsse - Meilleures salutations
Igor Opaniuk Senior Software Engineer, Embedded & Security E: igor.opaniuk@foundries.io W: www.foundries.io
On Fri, Feb 2, 2024 at 1:04 PM Fabio Estevam festevam@gmail.com wrote:
From: Fabio Estevam festevam@denx.de
U-Boot binary has grown in such a way that it goes beyond the reserved area for the environment variables.
Running "saveenv" and rebooting the board causes U-Boot to hang because of this overlap.
Fix this problem by selecting CONFIG_LTO so that the U-Boot proper size can be reduced.
Also, to prevent this same problem to happen in the future, use CONFIG_BOARD_SIZE_LIMIT, which can detect the overlap in build-time.
CONFIG_BOARD_SIZE_LIMIT is calculated as follows:
CONFIG_BOARD_SIZE_LIMIT = CONFIG_ENV_OFFSET - u-boot-img.dtb offset CONFIG_BOARD_SIZE_LIMIT = 0xc000 - 69 * 1024 CONFIG_BOARD_SIZE_LIMIT = 715766
Signed-off-by: Fabio Estevam festevam@denx.de
Applied all, thanks.
participants (3)
-
Fabio Estevam -
Igor Opaniuk -
Tom Rini