[PATCH u-boot-marvell v2 0/9] More verifications for kwbimage in SPL

[PATCH u-boot-marvell v2 0/9] More verifications for kwbimage in SPL

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

Hello Stefan,

this is v2 of series that adds more checks for kwbimage validity and
consistency to SPL, mainly checking image data checksum.

Changes since v1:
- updated error messages as requested by Stefan
- fixed checkpatch warnings for uintN_t types (converted to preferred
  uN)
- added more checkpatch fixes

Marek

Marek Behún (4):
  arm: mvebu: spl: Print srcaddr in error message
  arm: mvebu: spl: Use preferred types u8/u16/u32 instead of uintN_t
  arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible
  arm: mvebu: spl: Fix 100 column exceeds

Pali Rohár (5):
  arm: mvebu: Check that kwbimage offset and blocksize are valid
  SPL: Add struct spl_boot_device parameter into
    spl_parse_board_header()
  arm: mvebu: Check that kwbimage blockid matches boot mode
  SPL: Add support for checking board / BootROM specific image types
  arm: mvebu: Check for kwbimage data checksum

 arch/arm/mach-mvebu/spl.c           | 153 +++++++++++++++++++---------
 arch/arm/mach-sunxi/spl_spi_sunxi.c |   2 +-
 common/spl/spl.c                    |  13 ++-
 common/spl/spl_ext.c                |   9 +-
 common/spl/spl_fat.c                |  11 +-
 common/spl/spl_legacy.c             |   3 +-
 common/spl/spl_mmc.c                |  43 +++++---
 common/spl/spl_nand.c               |   5 +-
 common/spl/spl_net.c                |   2 +-
 common/spl/spl_nor.c                |   4 +-
 common/spl/spl_onenand.c            |   2 +-
 common/spl/spl_ram.c                |   2 +-
 common/spl/spl_sata.c               |   9 +-
 common/spl/spl_sdp.c                |   2 +-
 common/spl/spl_spi.c                |   9 +-
 common/spl/spl_ubi.c                |   4 +-
 common/spl/spl_usb.c                |   4 +-
 common/spl/spl_xip.c                |   4 +-
 common/spl/spl_ymodem.c             |   4 +-
 drivers/usb/gadget/f_sdp.c          |  12 ++-
 include/sdp.h                       |   3 +-
 include/spl.h                       |   7 ++
 22 files changed, 201 insertions(+), 106 deletions(-)

-- 
2.32.0

[PATCH u-boot-marvell v2 1/9] arm: mvebu: Check that kwbimage offset and blocksize are valid

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

There are certain restrictions for kwbimage offset and blocksize.
Validate them.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Stefan Roese <sr@denx.de>
---
Changes since v1:
- updated error messages as requested by Stefan
---
 arch/arm/mach-mvebu/spl.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index 73c4b9af3e..a6ed4367dd 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -163,6 +163,18 @@ int spl_parse_board_header(struct spl_image_info *spl_image,
 		spl_image->offset *= 512;
 #endif
 
+	if (spl_image->offset % 4 != 0) {
+		printf("ERROR: Wrong srcaddr (%u) in kwbimage\n",
+		       spl_image->offset);
+		return -EINVAL;
+	}
+
+	if (mhdr->blocksize <= 4 || mhdr->blocksize % 4 != 0) {
+		printf("ERROR: Wrong blocksize (%u) in kwbimage\n",
+		       mhdr->blocksize);
+		return -EINVAL;
+	}
+
 	spl_image->size = mhdr->blocksize;
 	spl_image->entry_point = mhdr->execaddr;
 	spl_image->load_addr = mhdr->destaddr;
-- 
2.32.0

[PATCH u-boot-marvell v2 2/9] SPL: Add struct spl_boot_device parameter into spl_parse_board_header()

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

Add parameter spl_boot_device to spl_parse_board_header(), which allows
the implementations to see from which device we are booting and do
boot-device-specific checks of the image header.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Stefan Roese <sr@denx.de>
---
 arch/arm/mach-mvebu/spl.c           |  1 +
 arch/arm/mach-sunxi/spl_spi_sunxi.c |  2 +-
 common/spl/spl.c                    |  4 ++-
 common/spl/spl_ext.c                |  9 ++++--
 common/spl/spl_fat.c                | 11 +++++---
 common/spl/spl_legacy.c             |  3 +-
 common/spl/spl_mmc.c                | 43 ++++++++++++++++++-----------
 common/spl/spl_nand.c               |  5 ++--
 common/spl/spl_net.c                |  2 +-
 common/spl/spl_nor.c                |  4 +--
 common/spl/spl_onenand.c            |  2 +-
 common/spl/spl_ram.c                |  2 +-
 common/spl/spl_sata.c               |  9 +++---
 common/spl/spl_sdp.c                |  2 +-
 common/spl/spl_spi.c                |  9 +++---
 common/spl/spl_ubi.c                |  4 +--
 common/spl/spl_usb.c                |  4 +--
 common/spl/spl_xip.c                |  4 +--
 common/spl/spl_ymodem.c             |  4 +--
 drivers/usb/gadget/f_sdp.c          | 12 ++++----
 include/sdp.h                       |  3 +-
 include/spl.h                       |  7 +++++
 22 files changed, 90 insertions(+), 56 deletions(-)

diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index a6ed4367dd..716217083b 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -101,6 +101,7 @@ u32 spl_mmc_boot_mode(const u32 boot_device)
 #endif
 
 int spl_parse_board_header(struct spl_image_info *spl_image,
+			   const struct spl_boot_device *bootdev,
 			   const void *image_header, size_t size)
 {
 	const struct kwbimage_main_hdr_v1 *mhdr = image_header;
diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c b/arch/arm/mach-sunxi/spl_spi_sunxi.c
index 3499c4cc5f..910e805016 100644
--- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
+++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
@@ -348,7 +348,7 @@ static int spl_spi_load_image(struct spl_image_info *spl_image,
 		ret = spl_load_simple_fit(spl_image, &load,
 					  load_offset, header);
 	} else {
-		ret = spl_parse_image_header(spl_image, header);
+		ret = spl_parse_image_header(spl_image, bootdev, header);
 		if (ret)
 			return ret;
 
diff --git a/common/spl/spl.c b/common/spl/spl.c
index 4c101ec5d3..bf2139a058 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -312,6 +312,7 @@ static int spl_load_fit_image(struct spl_image_info *spl_image,
 #endif
 
 __weak int spl_parse_board_header(struct spl_image_info *spl_image,
+				  const struct spl_boot_device *bootdev,
 				  const void *image_header, size_t size)
 {
 	return -EINVAL;
@@ -326,6 +327,7 @@ __weak int spl_parse_legacy_header(struct spl_image_info *spl_image,
 }
 
 int spl_parse_image_header(struct spl_image_info *spl_image,
+			   const struct spl_boot_device *bootdev,
 			   const struct image_header *header)
 {
 #if CONFIG_IS_ENABLED(LOAD_FIT_FULL)
@@ -369,7 +371,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
 		}
 #endif
 
-		if (!spl_parse_board_header(spl_image, (const void *)header, sizeof(*header)))
+		if (!spl_parse_board_header(spl_image, bootdev, (const void *)header, sizeof(*header)))
 			return 0;
 
 #ifdef CONFIG_SPL_RAW_IMAGE_SUPPORT
diff --git a/common/spl/spl_ext.c b/common/spl/spl_ext.c
index 6a28fe9bdb..ebd914c492 100644
--- a/common/spl/spl_ext.c
+++ b/common/spl/spl_ext.c
@@ -10,6 +10,7 @@
 #include <image.h>
 
 int spl_load_image_ext(struct spl_image_info *spl_image,
+		       struct spl_boot_device *bootdev,
 		       struct blk_desc *block_dev, int partition,
 		       const char *filename)
 {
@@ -46,7 +47,7 @@ int spl_load_image_ext(struct spl_image_info *spl_image,
 		goto end;
 	}
 
-	err = spl_parse_image_header(spl_image, header);
+	err = spl_parse_image_header(spl_image, bootdev, header);
 	if (err < 0) {
 		puts("spl: ext: failed to parse image header\n");
 		goto end;
@@ -66,6 +67,7 @@ end:
 
 #if CONFIG_IS_ENABLED(OS_BOOT)
 int spl_load_image_ext_os(struct spl_image_info *spl_image,
+			  struct spl_boot_device *bootdev,
 			  struct blk_desc *block_dev, int partition)
 {
 	int err;
@@ -103,7 +105,7 @@ int spl_load_image_ext_os(struct spl_image_info *spl_image,
 		}
 		file = env_get("falcon_image_file");
 		if (file) {
-			err = spl_load_image_ext(spl_image, block_dev,
+			err = spl_load_image_ext(spl_image, bootdev, block_dev,
 						 partition, file);
 			if (err != 0) {
 				puts("spl: falling back to default\n");
@@ -134,11 +136,12 @@ defaults:
 		return -1;
 	}
 
-	return spl_load_image_ext(spl_image, block_dev, partition,
+	return spl_load_image_ext(spl_image, bootdev, block_dev, partition,
 			CONFIG_SPL_FS_LOAD_KERNEL_NAME);
 }
 #else
 int spl_load_image_ext_os(struct spl_image_info *spl_image,
+			  struct spl_boot_device *bootdev,
 			  struct blk_desc *block_dev, int partition)
 {
 	return -ENOSYS;
diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
index 576c2e876a..5b270541fc 100644
--- a/common/spl/spl_fat.c
+++ b/common/spl/spl_fat.c
@@ -55,6 +55,7 @@ static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
 }
 
 int spl_load_image_fat(struct spl_image_info *spl_image,
+		       struct spl_boot_device *bootdev,
 		       struct blk_desc *block_dev, int partition,
 		       const char *filename)
 {
@@ -76,7 +77,7 @@ int spl_load_image_fat(struct spl_image_info *spl_image,
 		err = file_fat_read(filename, (void *)CONFIG_SYS_LOAD_ADDR, 0);
 		if (err <= 0)
 			goto end;
-		err = spl_parse_image_header(spl_image,
+		err = spl_parse_image_header(spl_image, bootdev,
 				(struct image_header *)CONFIG_SYS_LOAD_ADDR);
 		if (err == -EAGAIN)
 			return err;
@@ -94,7 +95,7 @@ int spl_load_image_fat(struct spl_image_info *spl_image,
 
 		return spl_load_simple_fit(spl_image, &load, 0, header);
 	} else {
-		err = spl_parse_image_header(spl_image, header);
+		err = spl_parse_image_header(spl_image, bootdev, header);
 		if (err)
 			goto end;
 
@@ -114,6 +115,7 @@ end:
 
 #if CONFIG_IS_ENABLED(OS_BOOT)
 int spl_load_image_fat_os(struct spl_image_info *spl_image,
+			  struct spl_boot_device *bootdev,
 			  struct blk_desc *block_dev, int partition)
 {
 	int err;
@@ -134,7 +136,7 @@ int spl_load_image_fat_os(struct spl_image_info *spl_image,
 		}
 		file = env_get("falcon_image_file");
 		if (file) {
-			err = spl_load_image_fat(spl_image, block_dev,
+			err = spl_load_image_fat(spl_image, bootdev, block_dev,
 						 partition, file);
 			if (err != 0) {
 				puts("spl: falling back to default\n");
@@ -160,11 +162,12 @@ defaults:
 		return -1;
 	}
 
-	return spl_load_image_fat(spl_image, block_dev, partition,
+	return spl_load_image_fat(spl_image, bootdev, block_dev, partition,
 			CONFIG_SPL_FS_LOAD_KERNEL_NAME);
 }
 #else
 int spl_load_image_fat_os(struct spl_image_info *spl_image,
+			  struct spl_boot_device *bootdev,
 			  struct blk_desc *block_dev, int partition)
 {
 	return -ENOSYS;
diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c
index 82d0326806..2ec7154423 100644
--- a/common/spl/spl_legacy.c
+++ b/common/spl/spl_legacy.c
@@ -76,6 +76,7 @@ static inline int spl_image_get_comp(const struct image_header *hdr)
 }
 
 int spl_load_legacy_img(struct spl_image_info *spl_image,
+			struct spl_boot_device *bootdev,
 			struct spl_load_info *load, ulong header)
 {
 	__maybe_unused SizeT lzma_len;
@@ -87,7 +88,7 @@ int spl_load_legacy_img(struct spl_image_info *spl_image,
 	/* Read header into local struct */
 	load->read(load, header, sizeof(hdr), &hdr);
 
-	ret = spl_parse_image_header(spl_image, &hdr);
+	ret = spl_parse_image_header(spl_image, bootdev, &hdr);
 	if (ret)
 		return ret;
 
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index e1a7d25bd0..d550da2d97 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -17,7 +17,9 @@
 #include <mmc.h>
 #include <image.h>
 
-static int mmc_load_legacy(struct spl_image_info *spl_image, struct mmc *mmc,
+static int mmc_load_legacy(struct spl_image_info *spl_image,
+			   struct spl_boot_device *bootdev,
+			   struct mmc *mmc,
 			   ulong sector, struct image_header *header)
 {
 	u32 image_offset_sectors;
@@ -26,7 +28,7 @@ static int mmc_load_legacy(struct spl_image_info *spl_image, struct mmc *mmc,
 	u32 image_offset;
 	int ret;
 
-	ret = spl_parse_image_header(spl_image, header);
+	ret = spl_parse_image_header(spl_image, bootdev, header);
 	if (ret)
 		return ret;
 
@@ -77,6 +79,7 @@ static __maybe_unused unsigned long spl_mmc_raw_uboot_offset(int part)
 
 static __maybe_unused
 int mmc_load_image_raw_sector(struct spl_image_info *spl_image,
+			      struct spl_boot_device *bootdev,
 			      struct mmc *mmc, unsigned long sector)
 {
 	unsigned long count;
@@ -116,7 +119,7 @@ int mmc_load_image_raw_sector(struct spl_image_info *spl_image,
 
 		ret = spl_load_imx_container(spl_image, &load, sector);
 	} else {
-		ret = mmc_load_legacy(spl_image, mmc, sector, header);
+		ret = mmc_load_legacy(spl_image, bootdev, mmc, sector, header);
 	}
 
 end:
@@ -181,6 +184,7 @@ static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device)
 
 #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
 static int mmc_load_image_raw_partition(struct spl_image_info *spl_image,
+					struct spl_boot_device *bootdev,
 					struct mmc *mmc, int partition,
 					unsigned long sector)
 {
@@ -211,15 +215,16 @@ static int mmc_load_image_raw_partition(struct spl_image_info *spl_image,
 	}
 
 #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR
-	return mmc_load_image_raw_sector(spl_image, mmc, info.start + sector);
+	return mmc_load_image_raw_sector(spl_image, bootdev, mmc, info.start + sector);
 #else
-	return mmc_load_image_raw_sector(spl_image, mmc, info.start);
+	return mmc_load_image_raw_sector(spl_image, bootdev, mmc, info.start);
 #endif
 }
 #endif
 
 #if CONFIG_IS_ENABLED(OS_BOOT)
 static int mmc_load_image_raw_os(struct spl_image_info *spl_image,
+				 struct spl_boot_device *bootdev,
 				 struct mmc *mmc)
 {
 	int ret;
@@ -239,7 +244,7 @@ static int mmc_load_image_raw_os(struct spl_image_info *spl_image,
 	}
 #endif	/* CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTOR */
 
-	ret = mmc_load_image_raw_sector(spl_image, mmc,
+	ret = mmc_load_image_raw_sector(spl_image, bootdev, mmc,
 		CONFIG_SYS_MMCSD_RAW_MODE_KERNEL_SECTOR);
 	if (ret)
 		return ret;
@@ -257,6 +262,7 @@ int spl_start_uboot(void)
 	return 1;
 }
 static int mmc_load_image_raw_os(struct spl_image_info *spl_image,
+				 struct spl_boot_device *bootdev,
 				 struct mmc *mmc)
 {
 	return -ENOSYS;
@@ -264,20 +270,22 @@ static int mmc_load_image_raw_os(struct spl_image_info *spl_image,
 #endif
 
 #ifdef CONFIG_SYS_MMCSD_FS_BOOT_PARTITION
-static int spl_mmc_do_fs_boot(struct spl_image_info *spl_image, struct mmc *mmc,
+static int spl_mmc_do_fs_boot(struct spl_image_info *spl_image,
+			      struct spl_boot_device *bootdev,
+			      struct mmc *mmc,
 			      const char *filename)
 {
 	int err = -ENOSYS;
 
 #ifdef CONFIG_SPL_FS_FAT
 	if (!spl_start_uboot()) {
-		err = spl_load_image_fat_os(spl_image, mmc_get_blk_desc(mmc),
+		err = spl_load_image_fat_os(spl_image, bootdev, mmc_get_blk_desc(mmc),
 			CONFIG_SYS_MMCSD_FS_BOOT_PARTITION);
 		if (!err)
 			return err;
 	}
 #ifdef CONFIG_SPL_FS_LOAD_PAYLOAD_NAME
-	err = spl_load_image_fat(spl_image, mmc_get_blk_desc(mmc),
+	err = spl_load_image_fat(spl_image, bootdev, mmc_get_blk_desc(mmc),
 				 CONFIG_SYS_MMCSD_FS_BOOT_PARTITION,
 				 filename);
 	if (!err)
@@ -286,13 +294,13 @@ static int spl_mmc_do_fs_boot(struct spl_image_info *spl_image, struct mmc *mmc,
 #endif
 #ifdef CONFIG_SPL_FS_EXT4
 	if (!spl_start_uboot()) {
-		err = spl_load_image_ext_os(spl_image, mmc_get_blk_desc(mmc),
+		err = spl_load_image_ext_os(spl_image, bootdev, mmc_get_blk_desc(mmc),
 			CONFIG_SYS_MMCSD_FS_BOOT_PARTITION);
 		if (!err)
 			return err;
 	}
 #ifdef CONFIG_SPL_FS_LOAD_PAYLOAD_NAME
-	err = spl_load_image_ext(spl_image, mmc_get_blk_desc(mmc),
+	err = spl_load_image_ext(spl_image, bootdev, mmc_get_blk_desc(mmc),
 				 CONFIG_SYS_MMCSD_FS_BOOT_PARTITION,
 				 filename);
 	if (!err)
@@ -307,7 +315,9 @@ static int spl_mmc_do_fs_boot(struct spl_image_info *spl_image, struct mmc *mmc,
 	return err;
 }
 #else
-static int spl_mmc_do_fs_boot(struct spl_image_info *spl_image, struct mmc *mmc,
+static int spl_mmc_do_fs_boot(struct spl_image_info *spl_image,
+			      struct spl_boot_device *bootdev,
+			      struct mmc *mmc,
 			      const char *filename)
 {
 	return -ENOSYS;
@@ -410,7 +420,7 @@ int spl_mmc_load(struct spl_image_info *spl_image,
 		debug("spl: mmc boot mode: raw\n");
 
 		if (!spl_start_uboot()) {
-			err = mmc_load_image_raw_os(spl_image, mmc);
+			err = mmc_load_image_raw_os(spl_image, bootdev, mmc);
 			if (!err)
 				return err;
 		}
@@ -418,13 +428,14 @@ int spl_mmc_load(struct spl_image_info *spl_image,
 		raw_sect = spl_mmc_get_uboot_raw_sector(mmc, raw_sect);
 
 #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
-		err = mmc_load_image_raw_partition(spl_image, mmc, raw_part,
+		err = mmc_load_image_raw_partition(spl_image, bootdev,
+						   mmc, raw_part,
 						   raw_sect);
 		if (!err)
 			return err;
 #endif
 #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR
-		err = mmc_load_image_raw_sector(spl_image, mmc,
+		err = mmc_load_image_raw_sector(spl_image, bootdev, mmc,
 				raw_sect + spl_mmc_raw_uboot_offset(part));
 		if (!err)
 			return err;
@@ -433,7 +444,7 @@ int spl_mmc_load(struct spl_image_info *spl_image,
 	case MMCSD_MODE_FS:
 		debug("spl: mmc boot mode: fs\n");
 
-		err = spl_mmc_do_fs_boot(spl_image, mmc, filename);
+		err = spl_mmc_do_fs_boot(spl_image, bootdev, mmc, filename);
 		if (!err)
 			return err;
 
diff --git a/common/spl/spl_nand.c b/common/spl/spl_nand.c
index 8ae7d04fa6..8fe592a154 100644
--- a/common/spl/spl_nand.c
+++ b/common/spl/spl_nand.c
@@ -65,6 +65,7 @@ struct mtd_info * __weak nand_get_mtd(void)
 }
 
 static int spl_nand_load_element(struct spl_image_info *spl_image,
+				 struct spl_boot_device *bootdev,
 				 int offset, struct image_header *header)
 {
 	struct mtd_info *mtd = nand_get_mtd();
@@ -96,7 +97,7 @@ static int spl_nand_load_element(struct spl_image_info *spl_image,
 		load.read = spl_nand_fit_read;
 		return spl_load_imx_container(spl_image, &load, offset / bl_len);
 	} else {
-		err = spl_parse_image_header(spl_image, header);
+		err = spl_parse_image_header(spl_image, bootdev, header);
 		if (err)
 			return err;
 		return nand_spl_load_image(offset, spl_image->size,
@@ -145,7 +146,7 @@ static int spl_nand_load_image(struct spl_image_info *spl_image,
 		/* load linux */
 		nand_spl_load_image(CONFIG_SYS_NAND_SPL_KERNEL_OFFS,
 			sizeof(*header), (void *)header);
-		err = spl_parse_image_header(spl_image, header);
+		err = spl_parse_image_header(spl_image, bootdev, header);
 		if (err)
 			return err;
 		if (header->ih_os == IH_OS_LINUX) {
diff --git a/common/spl/spl_net.c b/common/spl/spl_net.c
index d23b395ab9..a853e6aead 100644
--- a/common/spl/spl_net.c
+++ b/common/spl/spl_net.c
@@ -58,7 +58,7 @@ static int spl_net_load_image(struct spl_image_info *spl_image,
 	} else {
 		debug("Legacy image\n");
 
-		rv = spl_parse_image_header(spl_image, header);
+		rv = spl_parse_image_header(spl_image, bootdev, header);
 		if (rv)
 			return rv;
 
diff --git a/common/spl/spl_nor.c b/common/spl/spl_nor.c
index 68c12413fa..0f4fff8493 100644
--- a/common/spl/spl_nor.c
+++ b/common/spl/spl_nor.c
@@ -66,7 +66,7 @@ static int spl_nor_load_image(struct spl_image_info *spl_image,
 			/* happy - was a Linux */
 			int ret;
 
-			ret = spl_parse_image_header(spl_image, header);
+			ret = spl_parse_image_header(spl_image, bootdev, header);
 			if (ret)
 				return ret;
 
@@ -113,7 +113,7 @@ static int spl_nor_load_image(struct spl_image_info *spl_image,
 	if (IS_ENABLED(CONFIG_SPL_LEGACY_IMAGE_SUPPORT)) {
 		load.bl_len = 1;
 		load.read = spl_nor_load_read;
-		return spl_load_legacy_img(spl_image, &load,
+		return spl_load_legacy_img(spl_image, bootdev, &load,
 					   spl_nor_get_uboot_base());
 	}
 
diff --git a/common/spl/spl_onenand.c b/common/spl/spl_onenand.c
index 93cbf47e82..f80769a027 100644
--- a/common/spl/spl_onenand.c
+++ b/common/spl/spl_onenand.c
@@ -27,7 +27,7 @@ static int spl_onenand_load_image(struct spl_image_info *spl_image,
 	/* Load u-boot */
 	onenand_spl_load_image(CONFIG_SYS_ONENAND_U_BOOT_OFFS,
 		CONFIG_SYS_ONENAND_PAGE_SIZE, (void *)header);
-	ret = spl_parse_image_header(spl_image, header);
+	ret = spl_parse_image_header(spl_image, bootdev, header);
 	if (ret)
 		return ret;
 	onenand_spl_load_image(CONFIG_SYS_ONENAND_U_BOOT_OFFS,
diff --git a/common/spl/spl_ram.c b/common/spl/spl_ram.c
index df9f3a4d00..3f7f7accc1 100644
--- a/common/spl/spl_ram.c
+++ b/common/spl/spl_ram.c
@@ -70,7 +70,7 @@ static int spl_ram_load_image(struct spl_image_info *spl_image,
 		}
 		header = (struct image_header *)map_sysmem(u_boot_pos, 0);
 
-		spl_parse_image_header(spl_image, header);
+		spl_parse_image_header(spl_image, bootdev, header);
 	}
 
 	return 0;
diff --git a/common/spl/spl_sata.c b/common/spl/spl_sata.c
index e9f6c5f050..1f3a144cdf 100644
--- a/common/spl/spl_sata.c
+++ b/common/spl/spl_sata.c
@@ -31,6 +31,7 @@
 #endif
 
 static int spl_sata_load_image_raw(struct spl_image_info *spl_image,
+		struct spl_boot_device *bootdev,
 		struct blk_desc *stor_dev, unsigned long sector)
 {
 	struct image_header *header;
@@ -45,7 +46,7 @@ static int spl_sata_load_image_raw(struct spl_image_info *spl_image,
 	if (count == 0)
 		return -EIO;
 
-	ret = spl_parse_image_header(spl_image, header);
+	ret = spl_parse_image_header(spl_image, bootdev, header);
 	if (ret)
 		return ret;
 
@@ -90,18 +91,18 @@ static int spl_sata_load_image(struct spl_image_info *spl_image,
 
 #if CONFIG_IS_ENABLED(OS_BOOT)
 	if (spl_start_uboot() ||
-	    spl_load_image_fat_os(spl_image, stor_dev,
+	    spl_load_image_fat_os(spl_image, bootdev, stor_dev,
 				  CONFIG_SYS_SATA_FAT_BOOT_PARTITION))
 #endif
 	{
 		err = -ENOSYS;
 
 		if (IS_ENABLED(CONFIG_SPL_FS_FAT)) {
-			err = spl_load_image_fat(spl_image, stor_dev,
+			err = spl_load_image_fat(spl_image, bootdev, stor_dev,
 					CONFIG_SYS_SATA_FAT_BOOT_PARTITION,
 					CONFIG_SPL_FS_LOAD_PAYLOAD_NAME);
 		} else if (IS_ENABLED(CONFIG_SPL_SATA_RAW_U_BOOT_USE_SECTOR)) {
-			err = spl_sata_load_image_raw(spl_image, stor_dev,
+			err = spl_sata_load_image_raw(spl_image, bootdev, stor_dev,
 				CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR);
 		}
 	}
diff --git a/common/spl/spl_sdp.c b/common/spl/spl_sdp.c
index ae9c09883a..36c31aff09 100644
--- a/common/spl/spl_sdp.c
+++ b/common/spl/spl_sdp.c
@@ -39,7 +39,7 @@ static int spl_sdp_load_image(struct spl_image_info *spl_image,
 	 * or it loads a FIT image and returns it to be handled by the SPL
 	 * code.
 	 */
-	ret = spl_sdp_handle(controller_index, spl_image);
+	ret = spl_sdp_handle(controller_index, spl_image, bootdev);
 	debug("SDP ended\n");
 
 	usb_gadget_release(controller_index);
diff --git a/common/spl/spl_spi.c b/common/spl/spl_spi.c
index 4e20a23dea..cf3f7ef4c0 100644
--- a/common/spl/spl_spi.c
+++ b/common/spl/spl_spi.c
@@ -24,6 +24,7 @@
  * the kernel and then device tree.
  */
 static int spi_load_image_os(struct spl_image_info *spl_image,
+			     struct spl_boot_device *bootdev,
 			     struct spi_flash *flash,
 			     struct image_header *header)
 {
@@ -36,7 +37,7 @@ static int spi_load_image_os(struct spl_image_info *spl_image,
 	if (image_get_magic(header) != IH_MAGIC)
 		return -1;
 
-	err = spl_parse_image_header(spl_image, header);
+	err = spl_parse_image_header(spl_image, bootdev, header);
 	if (err)
 		return err;
 
@@ -108,7 +109,7 @@ static int spl_spi_load_image(struct spl_image_info *spl_image,
 	}
 
 #if CONFIG_IS_ENABLED(OS_BOOT)
-	if (spl_start_uboot() || spi_load_image_os(spl_image, flash, header))
+	if (spl_start_uboot() || spi_load_image_os(spl_image, bootdev, flash, header))
 #endif
 	{
 		/* Load u-boot, mkimage header is 64 bytes. */
@@ -127,7 +128,7 @@ static int spl_spi_load_image(struct spl_image_info *spl_image,
 					     (void *)CONFIG_SYS_LOAD_ADDR);
 			if (err)
 				return err;
-			err = spl_parse_image_header(spl_image,
+			err = spl_parse_image_header(spl_image, bootdev,
 					(struct image_header *)CONFIG_SYS_LOAD_ADDR);
 		} else if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
 			   image_get_magic(header) == FDT_MAGIC) {
@@ -154,7 +155,7 @@ static int spl_spi_load_image(struct spl_image_info *spl_image,
 			err = spl_load_imx_container(spl_image, &load,
 						     payload_offs);
 		} else {
-			err = spl_parse_image_header(spl_image, header);
+			err = spl_parse_image_header(spl_image, bootdev, header);
 			if (err)
 				return err;
 			err = spi_flash_read(flash, payload_offs + spl_image->offset,
diff --git a/common/spl/spl_ubi.c b/common/spl/spl_ubi.c
index 2f2d74a02d..bdf5cc4c38 100644
--- a/common/spl/spl_ubi.c
+++ b/common/spl/spl_ubi.c
@@ -55,7 +55,7 @@ int spl_ubi_load_image(struct spl_image_info *spl_image,
 		ret = ubispl_load_volumes(&info, volumes, 2);
 		if (!ret) {
 			header = (struct image_header *)volumes[0].load_addr;
-			spl_parse_image_header(spl_image, header);
+			spl_parse_image_header(spl_image, bootdev, header);
 			puts("Linux loaded.\n");
 			goto out;
 		}
@@ -75,7 +75,7 @@ int spl_ubi_load_image(struct spl_image_info *spl_image,
 
 	ret = ubispl_load_volumes(&info, volumes, 1);
 	if (!ret)
-		spl_parse_image_header(spl_image, header);
+		spl_parse_image_header(spl_image, bootdev, header);
 out:
 #ifdef CONFIG_SPL_NAND_SUPPORT
 	if (bootdev->boot_device == BOOT_DEVICE_NAND)
diff --git a/common/spl/spl_usb.c b/common/spl/spl_usb.c
index 67d503026c..ccf01c8276 100644
--- a/common/spl/spl_usb.c
+++ b/common/spl/spl_usb.c
@@ -49,10 +49,10 @@ int spl_usb_load(struct spl_image_info *spl_image,
 
 #if CONFIG_IS_ENABLED(OS_BOOT)
 	if (spl_start_uboot() ||
-	    spl_load_image_fat_os(spl_image, stor_dev, partition))
+	    spl_load_image_fat_os(spl_image, bootdev, stor_dev, partition))
 #endif
 	{
-		err = spl_load_image_fat(spl_image, stor_dev, partition, filename);
+		err = spl_load_image_fat(spl_image, bootdev, stor_dev, partition, filename);
 	}
 
 	if (err) {
diff --git a/common/spl/spl_xip.c b/common/spl/spl_xip.c
index ba4af38a3e..33863fe7d4 100644
--- a/common/spl/spl_xip.c
+++ b/common/spl/spl_xip.c
@@ -24,7 +24,7 @@ static int spl_xip(struct spl_image_info *spl_image,
 		return 0;
 	}
 #endif
-	return(spl_parse_image_header(spl_image, (const struct image_header *)
-	       CONFIG_SYS_UBOOT_BASE));
+	return(spl_parse_image_header(spl_image, bootdev,
+	       (const struct image_header *)CONFIG_SYS_UBOOT_BASE));
 }
 SPL_LOAD_IMAGE_METHOD("XIP", 0, BOOT_DEVICE_XIP, spl_xip);
diff --git a/common/spl/spl_ymodem.c b/common/spl/spl_ymodem.c
index e979f780ad..047df74856 100644
--- a/common/spl/spl_ymodem.c
+++ b/common/spl/spl_ymodem.c
@@ -112,7 +112,7 @@ int spl_ymodem_load_image(struct spl_image_info *spl_image,
 			addr += res;
 		}
 
-		ret = spl_parse_image_header(spl_image, ih);
+		ret = spl_parse_image_header(spl_image, bootdev, ih);
 		if (ret)
 			return ret;
 	} else if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
@@ -135,7 +135,7 @@ int spl_ymodem_load_image(struct spl_image_info *spl_image,
 			size += res;
 	} else {
 		ih = (struct image_header *)buf;
-		ret = spl_parse_image_header(spl_image, ih);
+		ret = spl_parse_image_header(spl_image, bootdev, ih);
 		if (ret)
 			goto end_stream;
 #ifdef CONFIG_SPL_GZIP
diff --git a/drivers/usb/gadget/f_sdp.c b/drivers/usb/gadget/f_sdp.c
index e48aa2f90d..79936ed05b 100644
--- a/drivers/usb/gadget/f_sdp.c
+++ b/drivers/usb/gadget/f_sdp.c
@@ -773,7 +773,8 @@ static ulong search_container_header(ulong p, int size)
 }
 #endif
 
-static int sdp_handle_in_ep(struct spl_image_info *spl_image)
+static int sdp_handle_in_ep(struct spl_image_info *spl_image,
+			    struct spl_boot_device *bootdev)
 {
 	u8 *data = sdp_func->in_req->buf;
 	u32 status;
@@ -862,7 +863,7 @@ static int sdp_handle_in_ep(struct spl_image_info *spl_image)
 
 			/* In SPL, allow jumps to U-Boot images */
 			struct spl_image_info spl_image = {};
-			spl_parse_image_header(&spl_image, header);
+			spl_parse_image_header(&spl_image, bootdev, header);
 			jump_to_image_no_args(&spl_image);
 #else
 			/* In U-Boot, allow jumps to scripts */
@@ -910,7 +911,8 @@ static void sdp_handle_out_ep(void)
 #ifndef CONFIG_SPL_BUILD
 int sdp_handle(int controller_index)
 #else
-int spl_sdp_handle(int controller_index, struct spl_image_info *spl_image)
+int spl_sdp_handle(int controller_index, struct spl_image_info *spl_image,
+		   struct spl_boot_device *bootdev)
 #endif
 {
 	int flag = 0;
@@ -928,9 +930,9 @@ int spl_sdp_handle(int controller_index, struct spl_image_info *spl_image)
 		usb_gadget_handle_interrupts(controller_index);
 
 #ifdef CONFIG_SPL_BUILD
-		flag = sdp_handle_in_ep(spl_image);
+		flag = sdp_handle_in_ep(spl_image, bootdev);
 #else
-		flag = sdp_handle_in_ep(NULL);
+		flag = sdp_handle_in_ep(NULL, bootdev);
 #endif
 		if (sdp_func->ep_int_enable)
 			sdp_handle_out_ep();
diff --git a/include/sdp.h b/include/sdp.h
index 6ac64fb1f3..6d89baa04e 100644
--- a/include/sdp.h
+++ b/include/sdp.h
@@ -14,7 +14,8 @@ int sdp_init(int controller_index);
 #ifdef CONFIG_SPL_BUILD
 #include <spl.h>
 
-int spl_sdp_handle(int controller_index, struct spl_image_info *spl_image);
+int spl_sdp_handle(int controller_index, struct spl_image_info *spl_image,
+		   struct spl_boot_device *bootdev);
 #else
 int sdp_handle(int controller_index);
 #endif
diff --git a/include/spl.h b/include/spl.h
index 0af0ee3003..469e7fe1cc 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -29,6 +29,7 @@ struct image_header;
 
 struct blk_desc;
 struct image_header;
+struct spl_boot_device;
 
 /*
  * u_boot_first_phase() - check if this is the first U-Boot phase
@@ -340,6 +341,7 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
  * Returns 0 on success.
  */
 int spl_load_legacy_img(struct spl_image_info *spl_image,
+			struct spl_boot_device *bootdev,
 			struct spl_load_info *load, ulong header);
 
 /**
@@ -438,6 +440,7 @@ void spl_set_header_raw_uboot(struct spl_image_info *spl_image);
  * @return 0 if a header was correctly parsed, -ve on error
  */
 int spl_parse_image_header(struct spl_image_info *spl_image,
+			   const struct spl_boot_device *bootdev,
 			   const struct image_header *header);
 
 void spl_board_prepare_for_linux(void);
@@ -574,18 +577,22 @@ static inline const char *spl_loader_name(const struct spl_image_loader *loader)
 
 /* SPL FAT image functions */
 int spl_load_image_fat(struct spl_image_info *spl_image,
+		       struct spl_boot_device *bootdev,
 		       struct blk_desc *block_dev, int partition,
 		       const char *filename);
 int spl_load_image_fat_os(struct spl_image_info *spl_image,
+			  struct spl_boot_device *bootdev,
 			  struct blk_desc *block_dev, int partition);
 
 void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image);
 
 /* SPL EXT image functions */
 int spl_load_image_ext(struct spl_image_info *spl_image,
+		       struct spl_boot_device *bootdev,
 		       struct blk_desc *block_dev, int partition,
 		       const char *filename);
 int spl_load_image_ext_os(struct spl_image_info *spl_image,
+			  struct spl_boot_device *bootdev,
 			  struct blk_desc *block_dev, int partition);
 
 /**
-- 
2.32.0

[PATCH u-boot-marvell v2 3/9] arm: mvebu: Check that kwbimage blockid matches boot mode

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

Each boot mode has its own kwbimage specified by blockid. So check that
kwbimage is valid by blockid.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Stefan Roese <sr@denx.de>
---
Changes since v1:
- updated error messages as requested by Stefan
---
 arch/arm/mach-mvebu/spl.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index 716217083b..4af52c626c 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -118,22 +118,39 @@ int spl_parse_board_header(struct spl_image_info *spl_image,
 	 * (including SPL content) which is not included in U-Boot image_header.
 	 */
 	if (mhdr->version != 1 ||
-	    ((mhdr->headersz_msb << 16) | mhdr->headersz_lsb) < sizeof(*mhdr) ||
-	    (
+	    ((mhdr->headersz_msb << 16) | mhdr->headersz_lsb) < sizeof(*mhdr)) {
+		printf("ERROR: Invalid kwbimage v1\n");
+		return -EINVAL;
+	}
+
 #ifdef CONFIG_SPL_SPI_FLASH_SUPPORT
-	     mhdr->blockid != IBR_HDR_SPI_ID &&
+	if (bootdev->boot_device == BOOT_DEVICE_SPI &&
+	    mhdr->blockid != IBR_HDR_SPI_ID) {
+		printf("ERROR: Wrong blockid (%u) in SPI kwbimage\n",
+		       mhdr->blockid);
+		return -EINVAL;
+	}
 #endif
+
 #ifdef CONFIG_SPL_SATA
-	     mhdr->blockid != IBR_HDR_SATA_ID &&
+	if (bootdev->boot_device == BOOT_DEVICE_SATA &&
+	    mhdr->blockid != IBR_HDR_SATA_ID) {
+		printf("ERROR: Wrong blockid (%u) in SATA kwbimage\n",
+		       mhdr->blockid);
+		return -EINVAL;
+	}
 #endif
+
 #ifdef CONFIG_SPL_MMC
-	     mhdr->blockid != IBR_HDR_SDIO_ID &&
-#endif
-	     1
-	    )) {
-		printf("ERROR: Not valid SPI/NAND/SATA/SDIO kwbimage v1\n");
+	if ((bootdev->boot_device == BOOT_DEVICE_MMC1 ||
+	     bootdev->boot_device == BOOT_DEVICE_MMC2 ||
+	     bootdev->boot_device == BOOT_DEVICE_MMC2_2) &&
+	    mhdr->blockid != IBR_HDR_SDIO_ID) {
+		printf("ERROR: Wrong blockid (%u) in SDIO kwbimage\n",
+		       mhdr->blockid);
 		return -EINVAL;
 	}
+#endif
 
 	spl_image->offset = mhdr->srcaddr;
 
-- 
2.32.0

[PATCH u-boot-marvell v2 4/9] SPL: Add support for checking board / BootROM specific image types

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

Commit 9baab60b8054 ("SPL: Add support for parsing board / BootROM specific
image types") added support for loading board specific image types.

This commit adds support for a new weak function spl_parse_board_header()
which is called after loading boot image. Board may implement this function
for checking if loaded board specific image is valid.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Stefan Roese <sr@denx.de>
---
 common/spl/spl.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index bf2139a058..cc3b3b3438 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -589,6 +589,12 @@ static struct spl_image_loader *spl_ll_find_loader(uint boot_device)
 	return NULL;
 }
 
+__weak int spl_check_board_image(struct spl_image_info *spl_image,
+				 const struct spl_boot_device *bootdev)
+{
+	return 0;
+}
+
 static int spl_load_image(struct spl_image_info *spl_image,
 			  struct spl_image_loader *loader)
 {
@@ -610,6 +616,9 @@ static int spl_load_image(struct spl_image_info *spl_image,
 		}
 	}
 #endif
+	if (!ret)
+		ret = spl_check_board_image(spl_image, &bootdev);
+
 	return ret;
 }
 
-- 
2.32.0

[PATCH u-boot-marvell v2 5/9] arm: mvebu: Check for kwbimage data checksum

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

Last 4 bytes of kwbimage boot image is checksum. Verify it via the new
spl_check_board_image() function which is called by U-Boot SPL after
loading kwbimage.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Stefan Roese <sr@denx.de>
---
Changes since v1:
- changed uint32_t to u32
---
 arch/arm/mach-mvebu/spl.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index 4af52c626c..af9e45ac7a 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -100,6 +100,33 @@ u32 spl_mmc_boot_mode(const u32 boot_device)
 }
 #endif
 
+static u32 checksum32(void *start, u32 len)
+{
+	u32 csum = 0;
+	u32 *p = start;
+
+	while (len > 0) {
+		csum += *p++;
+		len -= sizeof(u32);
+	};
+
+	return csum;
+}
+
+int spl_check_board_image(struct spl_image_info *spl_image,
+			  const struct spl_boot_device *bootdev)
+{
+	u32 csum = *(u32 *)(spl_image->load_addr + spl_image->size - 4);
+
+	if (checksum32((void *)spl_image->load_addr,
+		       spl_image->size - 4) != csum) {
+		printf("ERROR: Invalid data checksum in kwbimage\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 int spl_parse_board_header(struct spl_image_info *spl_image,
 			   const struct spl_boot_device *bootdev,
 			   const void *image_header, size_t size)
-- 
2.32.0

[PATCH u-boot-marvell v2 6/9] arm: mvebu: spl: Print srcaddr in error message

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

Print the wrong srcaddr (spl_image->offset) in error message also for
SATA case.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 arch/arm/mach-mvebu/spl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index af9e45ac7a..8c8cbc833f 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -190,7 +190,8 @@ int spl_parse_board_header(struct spl_image_info *spl_image,
 	 */
 	if (mhdr->blockid == IBR_HDR_SATA_ID) {
 		if (spl_image->offset < 1) {
-			printf("ERROR: Wrong SATA srcaddr in kwbimage\n");
+			printf("ERROR: Wrong SATA srcaddr (%u) in kwbimage\n",
+			       spl_image->offset);
 			return -EINVAL;
 		}
 		spl_image->offset -= 1;
-- 
2.32.0

Re: [PATCH u-boot-marvell v2 6/9] arm: mvebu: spl: Print srcaddr in error message

[Stefan Roese]

On 11/26/21 15:37, Marek Behún wrote:
> From: Marek Behún <marek.behun@nic.cz>
> 
> Print the wrong srcaddr (spl_image->offset) in error message also for
> SATA case.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

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

Thanks,
Stefan

> ---
>   arch/arm/mach-mvebu/spl.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
> index af9e45ac7a..8c8cbc833f 100644
> --- a/arch/arm/mach-mvebu/spl.c
> +++ b/arch/arm/mach-mvebu/spl.c
> @@ -190,7 +190,8 @@ int spl_parse_board_header(struct spl_image_info *spl_image,
>   	 */
>   	if (mhdr->blockid == IBR_HDR_SATA_ID) {
>   		if (spl_image->offset < 1) {
> -			printf("ERROR: Wrong SATA srcaddr in kwbimage\n");
> +			printf("ERROR: Wrong SATA srcaddr (%u) in kwbimage\n",
> +			       spl_image->offset);
>   			return -EINVAL;
>   		}
>   		spl_image->offset -= 1;
> 

Viele Grüße,
Stefan Roese

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

Re: [PATCH u-boot-marvell v2 6/9] arm: mvebu: spl: Print srcaddr in error message

[Pali Rohár]

On Friday 26 November 2021 15:37:35 Marek Behún wrote:
> From: Marek Behún <marek.behun@nic.cz>
> 
> Print the wrong srcaddr (spl_image->offset) in error message also for
> SATA case.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> ---
>  arch/arm/mach-mvebu/spl.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
> index af9e45ac7a..8c8cbc833f 100644
> --- a/arch/arm/mach-mvebu/spl.c
> +++ b/arch/arm/mach-mvebu/spl.c
> @@ -190,7 +190,8 @@ int spl_parse_board_header(struct spl_image_info *spl_image,
>  	 */
>  	if (mhdr->blockid == IBR_HDR_SATA_ID) {
>  		if (spl_image->offset < 1) {
> -			printf("ERROR: Wrong SATA srcaddr in kwbimage\n");
> +			printf("ERROR: Wrong SATA srcaddr (%u) in kwbimage\n",

Maybe change message to "ERROR: Wrong srcaddr (%u) in SATA kwbimage" for
consistency with other newly added error messages in this patch series?

> +			       spl_image->offset);
>  			return -EINVAL;
>  		}
>  		spl_image->offset -= 1;
> -- 
> 2.32.0
> 

Re: [PATCH u-boot-marvell v2 6/9] arm: mvebu: spl: Print srcaddr in error message

[Marek Behún]

On Tue, 14 Dec 2021 12:10:30 +0100
Pali Rohár <pali@kernel.org> wrote:

> On Friday 26 November 2021 15:37:35 Marek Behún wrote:
> > From: Marek Behún <marek.behun@nic.cz>
> > 
> > Print the wrong srcaddr (spl_image->offset) in error message also for
> > SATA case.
> > 
> > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > ---
> >  arch/arm/mach-mvebu/spl.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
> > index af9e45ac7a..8c8cbc833f 100644
> > --- a/arch/arm/mach-mvebu/spl.c
> > +++ b/arch/arm/mach-mvebu/spl.c
> > @@ -190,7 +190,8 @@ int spl_parse_board_header(struct spl_image_info *spl_image,
> >  	 */
> >  	if (mhdr->blockid == IBR_HDR_SATA_ID) {
> >  		if (spl_image->offset < 1) {
> > -			printf("ERROR: Wrong SATA srcaddr in kwbimage\n");
> > +			printf("ERROR: Wrong SATA srcaddr (%u) in kwbimage\n",  
> 
> Maybe change message to "ERROR: Wrong srcaddr (%u) in SATA kwbimage" for
> consistency with other newly added error messages in this patch series?

OK I'll send v3

[PATCH u-boot-marvell v2 7/9] arm: mvebu: spl: Use preferred types u8/u16/u32 instead of uintN_t

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

Checkpatch warns about using uint32/16/8_t instead of u32/16/8.

Use the preferred types.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 arch/arm/mach-mvebu/spl.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index 8c8cbc833f..97d7aea179 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -74,23 +74,23 @@
 
 /* Structure of the main header, version 1 (Armada 370/XP/375/38x/39x) */
 struct kwbimage_main_hdr_v1 {
-	uint8_t  blockid;               /* 0x0       */
-	uint8_t  flags;                 /* 0x1       */
-	uint16_t nandpagesize;          /* 0x2-0x3   */
-	uint32_t blocksize;             /* 0x4-0x7   */
-	uint8_t  version;               /* 0x8       */
-	uint8_t  headersz_msb;          /* 0x9       */
-	uint16_t headersz_lsb;          /* 0xA-0xB   */
-	uint32_t srcaddr;               /* 0xC-0xF   */
-	uint32_t destaddr;              /* 0x10-0x13 */
-	uint32_t execaddr;              /* 0x14-0x17 */
-	uint8_t  options;               /* 0x18      */
-	uint8_t  nandblocksize;         /* 0x19      */
-	uint8_t  nandbadblklocation;    /* 0x1A      */
-	uint8_t  reserved4;             /* 0x1B      */
-	uint16_t reserved5;             /* 0x1C-0x1D */
-	uint8_t  ext;                   /* 0x1E      */
-	uint8_t  checksum;              /* 0x1F      */
+	u8  blockid;               /* 0x0       */
+	u8  flags;                 /* 0x1       */
+	u16 nandpagesize;          /* 0x2-0x3   */
+	u32 blocksize;             /* 0x4-0x7   */
+	u8  version;               /* 0x8       */
+	u8  headersz_msb;          /* 0x9       */
+	u16 headersz_lsb;          /* 0xA-0xB   */
+	u32 srcaddr;               /* 0xC-0xF   */
+	u32 destaddr;              /* 0x10-0x13 */
+	u32 execaddr;              /* 0x14-0x17 */
+	u8  options;               /* 0x18      */
+	u8  nandblocksize;         /* 0x19      */
+	u8  nandbadblklocation;    /* 0x1A      */
+	u8  reserved4;             /* 0x1B      */
+	u16 reserved5;             /* 0x1C-0x1D */
+	u8  ext;                   /* 0x1E      */
+	u8  checksum;              /* 0x1F      */
 } __packed;
 
 #ifdef CONFIG_SPL_MMC
-- 
2.32.0

Re: [PATCH u-boot-marvell v2 7/9] arm: mvebu: spl: Use preferred types u8/u16/u32 instead of uintN_t

[Stefan Roese]

On 11/26/21 15:37, Marek Behún wrote:
> From: Marek Behún <marek.behun@nic.cz>
> 
> Checkpatch warns about using uint32/16/8_t instead of u32/16/8.
> 
> Use the preferred types.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

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

Thanks,
Stefan

> ---
>   arch/arm/mach-mvebu/spl.c | 34 +++++++++++++++++-----------------
>   1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
> index 8c8cbc833f..97d7aea179 100644
> --- a/arch/arm/mach-mvebu/spl.c
> +++ b/arch/arm/mach-mvebu/spl.c
> @@ -74,23 +74,23 @@
>   
>   /* Structure of the main header, version 1 (Armada 370/XP/375/38x/39x) */
>   struct kwbimage_main_hdr_v1 {
> -	uint8_t  blockid;               /* 0x0       */
> -	uint8_t  flags;                 /* 0x1       */
> -	uint16_t nandpagesize;          /* 0x2-0x3   */
> -	uint32_t blocksize;             /* 0x4-0x7   */
> -	uint8_t  version;               /* 0x8       */
> -	uint8_t  headersz_msb;          /* 0x9       */
> -	uint16_t headersz_lsb;          /* 0xA-0xB   */
> -	uint32_t srcaddr;               /* 0xC-0xF   */
> -	uint32_t destaddr;              /* 0x10-0x13 */
> -	uint32_t execaddr;              /* 0x14-0x17 */
> -	uint8_t  options;               /* 0x18      */
> -	uint8_t  nandblocksize;         /* 0x19      */
> -	uint8_t  nandbadblklocation;    /* 0x1A      */
> -	uint8_t  reserved4;             /* 0x1B      */
> -	uint16_t reserved5;             /* 0x1C-0x1D */
> -	uint8_t  ext;                   /* 0x1E      */
> -	uint8_t  checksum;              /* 0x1F      */
> +	u8  blockid;               /* 0x0       */
> +	u8  flags;                 /* 0x1       */
> +	u16 nandpagesize;          /* 0x2-0x3   */
> +	u32 blocksize;             /* 0x4-0x7   */
> +	u8  version;               /* 0x8       */
> +	u8  headersz_msb;          /* 0x9       */
> +	u16 headersz_lsb;          /* 0xA-0xB   */
> +	u32 srcaddr;               /* 0xC-0xF   */
> +	u32 destaddr;              /* 0x10-0x13 */
> +	u32 execaddr;              /* 0x14-0x17 */
> +	u8  options;               /* 0x18      */
> +	u8  nandblocksize;         /* 0x19      */
> +	u8  nandbadblklocation;    /* 0x1A      */
> +	u8  reserved4;             /* 0x1B      */
> +	u16 reserved5;             /* 0x1C-0x1D */
> +	u8  ext;                   /* 0x1E      */
> +	u8  checksum;              /* 0x1F      */
>   } __packed;
>   
>   #ifdef CONFIG_SPL_MMC
> 

Viele Grüße,
Stefan Roese

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

[PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

Use the preferred
  if (IS_ENABLED(X))
instead of
  #ifdef X
where possible.

There are still places where this is not possible or is more complicated
to convert in this file. Leave those be for now.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 arch/arm/mach-mvebu/spl.c | 43 ++++++++++++++++-----------------------
 1 file changed, 18 insertions(+), 25 deletions(-)

diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index 97d7aea179..7dbe8eeba3 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -150,26 +150,24 @@ int spl_parse_board_header(struct spl_image_info *spl_image,
 		return -EINVAL;
 	}
 
-#ifdef CONFIG_SPL_SPI_FLASH_SUPPORT
-	if (bootdev->boot_device == BOOT_DEVICE_SPI &&
+	if (IS_ENABLED(CONFIG_SPL_SPI_FLASH_SUPPORT) &&
+	    bootdev->boot_device == BOOT_DEVICE_SPI &&
 	    mhdr->blockid != IBR_HDR_SPI_ID) {
 		printf("ERROR: Wrong blockid (%u) in SPI kwbimage\n",
 		       mhdr->blockid);
 		return -EINVAL;
 	}
-#endif
 
-#ifdef CONFIG_SPL_SATA
-	if (bootdev->boot_device == BOOT_DEVICE_SATA &&
+	if (IS_ENABLED(CONFIG_SPL_SATA) &&
+	    bootdev->boot_device == BOOT_DEVICE_SATA &&
 	    mhdr->blockid != IBR_HDR_SATA_ID) {
 		printf("ERROR: Wrong blockid (%u) in SATA kwbimage\n",
 		       mhdr->blockid);
 		return -EINVAL;
 	}
-#endif
 
-#ifdef CONFIG_SPL_MMC
-	if ((bootdev->boot_device == BOOT_DEVICE_MMC1 ||
+	if (IS_ENABLED(CONFIG_SPL_MMC) &&
+	    (bootdev->boot_device == BOOT_DEVICE_MMC1 ||
 	     bootdev->boot_device == BOOT_DEVICE_MMC2 ||
 	     bootdev->boot_device == BOOT_DEVICE_MMC2_2) &&
 	    mhdr->blockid != IBR_HDR_SDIO_ID) {
@@ -177,18 +175,16 @@ int spl_parse_board_header(struct spl_image_info *spl_image,
 		       mhdr->blockid);
 		return -EINVAL;
 	}
-#endif
 
 	spl_image->offset = mhdr->srcaddr;
 
-#ifdef CONFIG_SPL_SATA
 	/*
 	 * For SATA srcaddr is specified in number of sectors.
 	 * The main header is must be stored at sector number 1.
 	 * This expects that sector size is 512 bytes and recalculates
 	 * data offset to bytes relative to the main header.
 	 */
-	if (mhdr->blockid == IBR_HDR_SATA_ID) {
+	if (IS_ENABLED(CONFIG_SPL_SATA) && mhdr->blockid == IBR_HDR_SATA_ID) {
 		if (spl_image->offset < 1) {
 			printf("ERROR: Wrong SATA srcaddr (%u) in kwbimage\n",
 			       spl_image->offset);
@@ -197,17 +193,14 @@ int spl_parse_board_header(struct spl_image_info *spl_image,
 		spl_image->offset -= 1;
 		spl_image->offset *= 512;
 	}
-#endif
 
-#ifdef CONFIG_SPL_MMC
 	/*
 	 * For SDIO (eMMC) srcaddr is specified in number of sectors.
 	 * This expects that sector size is 512 bytes and recalculates
 	 * data offset to bytes.
 	 */
-	if (mhdr->blockid == IBR_HDR_SDIO_ID)
+	if (IS_ENABLED(CONFIG_SPL_MMC) && mhdr->blockid == IBR_HDR_SDIO_ID)
 		spl_image->offset *= 512;
-#endif
 
 	if (spl_image->offset % 4 != 0) {
 		printf("ERROR: Wrong srcaddr (%u) in kwbimage\n",
@@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
 	timer_init();
 
 	/* Armada 375 does not support SerDes and DDR3 init yet */
-#if !defined(CONFIG_ARMADA_375)
-	/* First init the serdes PHY's */
-	serdes_phy_config();
-
-	/* Setup DDR */
-	ret = ddr3_init();
-	if (ret) {
-		debug("ddr3_init() failed: %d\n", ret);
-		hang();
+	if (!IS_ENABLED(CONFIG_ARMADA_375)) {
+		/* First init the serdes PHY's */
+		serdes_phy_config();
+
+		/* Setup DDR */
+		ret = ddr3_init();
+		if (ret) {
+			debug("ddr3_init() failed: %d\n", ret);
+			hang();
+		}
 	}
-#endif
 
 	/* Initialize Auto Voltage Scaling */
 	mv_avs_init();
-- 
2.32.0

Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible

[Stefan Roese]

On 11/26/21 15:37, Marek Behún wrote:
> From: Marek Behún <marek.behun@nic.cz>
> 
> Use the preferred
>    if (IS_ENABLED(X))
> instead of
>    #ifdef X
> where possible.
> 
> There are still places where this is not possible or is more complicated
> to convert in this file. Leave those be for now.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

Nice, thanks.

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

Thanks,
Stefan

> ---
>   arch/arm/mach-mvebu/spl.c | 43 ++++++++++++++++-----------------------
>   1 file changed, 18 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
> index 97d7aea179..7dbe8eeba3 100644
> --- a/arch/arm/mach-mvebu/spl.c
> +++ b/arch/arm/mach-mvebu/spl.c
> @@ -150,26 +150,24 @@ int spl_parse_board_header(struct spl_image_info *spl_image,
>   		return -EINVAL;
>   	}
>   
> -#ifdef CONFIG_SPL_SPI_FLASH_SUPPORT
> -	if (bootdev->boot_device == BOOT_DEVICE_SPI &&
> +	if (IS_ENABLED(CONFIG_SPL_SPI_FLASH_SUPPORT) &&
> +	    bootdev->boot_device == BOOT_DEVICE_SPI &&
>   	    mhdr->blockid != IBR_HDR_SPI_ID) {
>   		printf("ERROR: Wrong blockid (%u) in SPI kwbimage\n",
>   		       mhdr->blockid);
>   		return -EINVAL;
>   	}
> -#endif
>   
> -#ifdef CONFIG_SPL_SATA
> -	if (bootdev->boot_device == BOOT_DEVICE_SATA &&
> +	if (IS_ENABLED(CONFIG_SPL_SATA) &&
> +	    bootdev->boot_device == BOOT_DEVICE_SATA &&
>   	    mhdr->blockid != IBR_HDR_SATA_ID) {
>   		printf("ERROR: Wrong blockid (%u) in SATA kwbimage\n",
>   		       mhdr->blockid);
>   		return -EINVAL;
>   	}
> -#endif
>   
> -#ifdef CONFIG_SPL_MMC
> -	if ((bootdev->boot_device == BOOT_DEVICE_MMC1 ||
> +	if (IS_ENABLED(CONFIG_SPL_MMC) &&
> +	    (bootdev->boot_device == BOOT_DEVICE_MMC1 ||
>   	     bootdev->boot_device == BOOT_DEVICE_MMC2 ||
>   	     bootdev->boot_device == BOOT_DEVICE_MMC2_2) &&
>   	    mhdr->blockid != IBR_HDR_SDIO_ID) {
> @@ -177,18 +175,16 @@ int spl_parse_board_header(struct spl_image_info *spl_image,
>   		       mhdr->blockid);
>   		return -EINVAL;
>   	}
> -#endif
>   
>   	spl_image->offset = mhdr->srcaddr;
>   
> -#ifdef CONFIG_SPL_SATA
>   	/*
>   	 * For SATA srcaddr is specified in number of sectors.
>   	 * The main header is must be stored at sector number 1.
>   	 * This expects that sector size is 512 bytes and recalculates
>   	 * data offset to bytes relative to the main header.
>   	 */
> -	if (mhdr->blockid == IBR_HDR_SATA_ID) {
> +	if (IS_ENABLED(CONFIG_SPL_SATA) && mhdr->blockid == IBR_HDR_SATA_ID) {
>   		if (spl_image->offset < 1) {
>   			printf("ERROR: Wrong SATA srcaddr (%u) in kwbimage\n",
>   			       spl_image->offset);
> @@ -197,17 +193,14 @@ int spl_parse_board_header(struct spl_image_info *spl_image,
>   		spl_image->offset -= 1;
>   		spl_image->offset *= 512;
>   	}
> -#endif
>   
> -#ifdef CONFIG_SPL_MMC
>   	/*
>   	 * For SDIO (eMMC) srcaddr is specified in number of sectors.
>   	 * This expects that sector size is 512 bytes and recalculates
>   	 * data offset to bytes.
>   	 */
> -	if (mhdr->blockid == IBR_HDR_SDIO_ID)
> +	if (IS_ENABLED(CONFIG_SPL_MMC) && mhdr->blockid == IBR_HDR_SDIO_ID)
>   		spl_image->offset *= 512;
> -#endif
>   
>   	if (spl_image->offset % 4 != 0) {
>   		printf("ERROR: Wrong srcaddr (%u) in kwbimage\n",
> @@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
>   	timer_init();
>   
>   	/* Armada 375 does not support SerDes and DDR3 init yet */
> -#if !defined(CONFIG_ARMADA_375)
> -	/* First init the serdes PHY's */
> -	serdes_phy_config();
> -
> -	/* Setup DDR */
> -	ret = ddr3_init();
> -	if (ret) {
> -		debug("ddr3_init() failed: %d\n", ret);
> -		hang();
> +	if (!IS_ENABLED(CONFIG_ARMADA_375)) {
> +		/* First init the serdes PHY's */
> +		serdes_phy_config();
> +
> +		/* Setup DDR */
> +		ret = ddr3_init();
> +		if (ret) {
> +			debug("ddr3_init() failed: %d\n", ret);
> +			hang();
> +		}
>   	}
> -#endif
>   
>   	/* Initialize Auto Voltage Scaling */
>   	mv_avs_init();
> 

Viele Grüße,
Stefan Roese

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

Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible

[Pali Rohár]

On Friday 26 November 2021 15:37:37 Marek Behún wrote:
> @@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
>  	timer_init();
>  
>  	/* Armada 375 does not support SerDes and DDR3 init yet */
> -#if !defined(CONFIG_ARMADA_375)
> -	/* First init the serdes PHY's */
> -	serdes_phy_config();
> -
> -	/* Setup DDR */
> -	ret = ddr3_init();
> -	if (ret) {
> -		debug("ddr3_init() failed: %d\n", ret);
> -		hang();
> +	if (!IS_ENABLED(CONFIG_ARMADA_375)) {
> +		/* First init the serdes PHY's */
> +		serdes_phy_config();
> +
> +		/* Setup DDR */
> +		ret = ddr3_init();
> +		if (ret) {
> +			debug("ddr3_init() failed: %d\n", ret);
> +			hang();
> +		}
>  	}
> -#endif

As written in comment above there is no SerDes and DDR3 support for
Armada 375 and therefore there is no serdes_phy_config() or ddr3_init()
function. So this code needs not to be compiled at all and usage of
#ifdef is correct here.

Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible

[Marek Behún]

On Tue, 14 Dec 2021 10:36:00 +0100
Pali Rohár <pali@kernel.org> wrote:

> On Friday 26 November 2021 15:37:37 Marek Behún wrote:
> > @@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
> >  	timer_init();
> >  
> >  	/* Armada 375 does not support SerDes and DDR3 init yet */
> > -#if !defined(CONFIG_ARMADA_375)
> > -	/* First init the serdes PHY's */
> > -	serdes_phy_config();
> > -
> > -	/* Setup DDR */
> > -	ret = ddr3_init();
> > -	if (ret) {
> > -		debug("ddr3_init() failed: %d\n", ret);
> > -		hang();
> > +	if (!IS_ENABLED(CONFIG_ARMADA_375)) {
> > +		/* First init the serdes PHY's */
> > +		serdes_phy_config();
> > +
> > +		/* Setup DDR */
> > +		ret = ddr3_init();
> > +		if (ret) {
> > +			debug("ddr3_init() failed: %d\n", ret);
> > +			hang();
> > +		}
> >  	}
> > -#endif  
> 
> As written in comment above there is no SerDes and DDR3 support for
> Armada 375 and therefore there is no serdes_phy_config() or ddr3_init()
> function. So this code needs not to be compiled at all and usage of
> #ifdef is correct here.

#ifdefs are almost never correct in C-files, for the parts of the code
they guard isn't put through syntactic analysis, and can therefore
contain bugs which we are not warned about.

Using if (IS_ENABLED()) almost never producess a different binary,
because the code is optimized away.

Marek

Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible

[Pali Rohár]

On Tuesday 14 December 2021 10:45:15 Marek Behún wrote:
> On Tue, 14 Dec 2021 10:36:00 +0100
> Pali Rohár <pali@kernel.org> wrote:
> 
> > On Friday 26 November 2021 15:37:37 Marek Behún wrote:
> > > @@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
> > >  	timer_init();
> > >  
> > >  	/* Armada 375 does not support SerDes and DDR3 init yet */
> > > -#if !defined(CONFIG_ARMADA_375)
> > > -	/* First init the serdes PHY's */
> > > -	serdes_phy_config();
> > > -
> > > -	/* Setup DDR */
> > > -	ret = ddr3_init();
> > > -	if (ret) {
> > > -		debug("ddr3_init() failed: %d\n", ret);
> > > -		hang();
> > > +	if (!IS_ENABLED(CONFIG_ARMADA_375)) {
> > > +		/* First init the serdes PHY's */
> > > +		serdes_phy_config();
> > > +
> > > +		/* Setup DDR */
> > > +		ret = ddr3_init();
> > > +		if (ret) {
> > > +			debug("ddr3_init() failed: %d\n", ret);
> > > +			hang();
> > > +		}
> > >  	}
> > > -#endif  
> > 
> > As written in comment above there is no SerDes and DDR3 support for
> > Armada 375 and therefore there is no serdes_phy_config() or ddr3_init()
> > function. So this code needs not to be compiled at all and usage of
> > #ifdef is correct here.
> 
> #ifdefs are almost never correct in C-files, for the parts of the code
> they guard isn't put through syntactic analysis, and can therefore
> contain bugs which we are not warned about.
> 
> Using if (IS_ENABLED()) almost never producess a different binary,
> because the code is optimized away.
> 
> Marek

There is no function serdes_phy_config() for Armada 375, so if you put
it outside of #ifdef you will get compile error.

Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible

[Marek Behún]

On Tue, 14 Dec 2021 12:12:34 +0100
Pali Rohár <pali@kernel.org> wrote:

> On Tuesday 14 December 2021 10:45:15 Marek Behún wrote:
> > On Tue, 14 Dec 2021 10:36:00 +0100
> > Pali Rohár <pali@kernel.org> wrote:
> >   
> > > On Friday 26 November 2021 15:37:37 Marek Behún wrote:  
> > > > @@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
> > > >  	timer_init();
> > > >  
> > > >  	/* Armada 375 does not support SerDes and DDR3 init yet */
> > > > -#if !defined(CONFIG_ARMADA_375)
> > > > -	/* First init the serdes PHY's */
> > > > -	serdes_phy_config();
> > > > -
> > > > -	/* Setup DDR */
> > > > -	ret = ddr3_init();
> > > > -	if (ret) {
> > > > -		debug("ddr3_init() failed: %d\n", ret);
> > > > -		hang();
> > > > +	if (!IS_ENABLED(CONFIG_ARMADA_375)) {
> > > > +		/* First init the serdes PHY's */
> > > > +		serdes_phy_config();
> > > > +
> > > > +		/* Setup DDR */
> > > > +		ret = ddr3_init();
> > > > +		if (ret) {
> > > > +			debug("ddr3_init() failed: %d\n", ret);
> > > > +			hang();
> > > > +		}
> > > >  	}
> > > > -#endif    
> > > 
> > > As written in comment above there is no SerDes and DDR3 support for
> > > Armada 375 and therefore there is no serdes_phy_config() or ddr3_init()
> > > function. So this code needs not to be compiled at all and usage of
> > > #ifdef is correct here.  
> > 
> > #ifdefs are almost never correct in C-files, for the parts of the code
> > they guard isn't put through syntactic analysis, and can therefore
> > contain bugs which we are not warned about.
> > 
> > Using if (IS_ENABLED()) almost never producess a different binary,
> > because the code is optimized away.
> > 
> > Marek  
> 
> There is no function serdes_phy_config() for Armada 375, so if you put
> it outside of #ifdef you will get compile error.

The function is always declared in
  arch/arm/mach-mvebu/include/mach/cpu.h
regardless of architecture.

Thus an error will be raised only when linking, and the compliation was
done with -O0, which I don't think anyone does.

Anyway, if we want to support -O0, this can and should be solved via
defining serdes_phy_config() as empty static inline function in the
cpu.h header, guarded by #ifdef. In header files #ifdefs are allowed,
in this manner:
  #if X
  declare function
  #else
  define that function as empty static inline
  #endif

So if you think we should support -O0, I can do this.

But the #ifdefs should really go away from real C code, that is the way
both Linux and U-Boot are trying to go for the last couple of years, if
I understand it correctly.

Marek

Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible

[Stefan Roese]

On 12/14/21 13:45, Marek Behún wrote:
> On Tue, 14 Dec 2021 12:12:34 +0100
> Pali Rohár <pali@kernel.org> wrote:
> 
>> On Tuesday 14 December 2021 10:45:15 Marek Behún wrote:
>>> On Tue, 14 Dec 2021 10:36:00 +0100
>>> Pali Rohár <pali@kernel.org> wrote:
>>>    
>>>> On Friday 26 November 2021 15:37:37 Marek Behún wrote:
>>>>> @@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
>>>>>   	timer_init();
>>>>>   
>>>>>   	/* Armada 375 does not support SerDes and DDR3 init yet */
>>>>> -#if !defined(CONFIG_ARMADA_375)
>>>>> -	/* First init the serdes PHY's */
>>>>> -	serdes_phy_config();
>>>>> -
>>>>> -	/* Setup DDR */
>>>>> -	ret = ddr3_init();
>>>>> -	if (ret) {
>>>>> -		debug("ddr3_init() failed: %d\n", ret);
>>>>> -		hang();
>>>>> +	if (!IS_ENABLED(CONFIG_ARMADA_375)) {
>>>>> +		/* First init the serdes PHY's */
>>>>> +		serdes_phy_config();
>>>>> +
>>>>> +		/* Setup DDR */
>>>>> +		ret = ddr3_init();
>>>>> +		if (ret) {
>>>>> +			debug("ddr3_init() failed: %d\n", ret);
>>>>> +			hang();
>>>>> +		}
>>>>>   	}
>>>>> -#endif
>>>>
>>>> As written in comment above there is no SerDes and DDR3 support for
>>>> Armada 375 and therefore there is no serdes_phy_config() or ddr3_init()
>>>> function. So this code needs not to be compiled at all and usage of
>>>> #ifdef is correct here.
>>>
>>> #ifdefs are almost never correct in C-files, for the parts of the code
>>> they guard isn't put through syntactic analysis, and can therefore
>>> contain bugs which we are not warned about.
>>>
>>> Using if (IS_ENABLED()) almost never producess a different binary,
>>> because the code is optimized away.
>>>
>>> Marek
>>
>> There is no function serdes_phy_config() for Armada 375, so if you put
>> it outside of #ifdef you will get compile error.
> 
> The function is always declared in
>    arch/arm/mach-mvebu/include/mach/cpu.h
> regardless of architecture.
> 
> Thus an error will be raised only when linking, and the compliation was
> done with -O0, which I don't think anyone does.
> 
> Anyway, if we want to support -O0, this can and should be solved via
> defining serdes_phy_config() as empty static inline function in the
> cpu.h header, guarded by #ifdef. In header files #ifdefs are allowed,
> in this manner:
>    #if X
>    declare function
>    #else
>    define that function as empty static inline
>    #endif
> 
> So if you think we should support -O0, I can do this.
> 
> But the #ifdefs should really go away from real C code, that is the way
> both Linux and U-Boot are trying to go for the last couple of years, if
> I understand it correctly.

Yes, the #ifdef's really should be avoided if possible. So *if* your
patch above does not generate any build issues, then I don't see any
problems to include it. I personally don't think that we need to support
-O0 builds.

Thanks,
Stefan

Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible

[Marek Behún]

On Tue, 14 Dec 2021 13:48:31 +0100
Stefan Roese <sr@denx.de> wrote:

> On 12/14/21 13:45, Marek Behún wrote:
> > On Tue, 14 Dec 2021 12:12:34 +0100
> > Pali Rohár <pali@kernel.org> wrote:
> >   
> >> On Tuesday 14 December 2021 10:45:15 Marek Behún wrote:  
> >>> On Tue, 14 Dec 2021 10:36:00 +0100
> >>> Pali Rohár <pali@kernel.org> wrote:
> >>>      
> >>>> On Friday 26 November 2021 15:37:37 Marek Behún wrote:  
> >>>>> @@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
> >>>>>   	timer_init();
> >>>>>   
> >>>>>   	/* Armada 375 does not support SerDes and DDR3 init yet */
> >>>>> -#if !defined(CONFIG_ARMADA_375)
> >>>>> -	/* First init the serdes PHY's */
> >>>>> -	serdes_phy_config();
> >>>>> -
> >>>>> -	/* Setup DDR */
> >>>>> -	ret = ddr3_init();
> >>>>> -	if (ret) {
> >>>>> -		debug("ddr3_init() failed: %d\n", ret);
> >>>>> -		hang();
> >>>>> +	if (!IS_ENABLED(CONFIG_ARMADA_375)) {
> >>>>> +		/* First init the serdes PHY's */
> >>>>> +		serdes_phy_config();
> >>>>> +
> >>>>> +		/* Setup DDR */
> >>>>> +		ret = ddr3_init();
> >>>>> +		if (ret) {
> >>>>> +			debug("ddr3_init() failed: %d\n", ret);
> >>>>> +			hang();
> >>>>> +		}
> >>>>>   	}
> >>>>> -#endif  
> >>>>
> >>>> As written in comment above there is no SerDes and DDR3 support for
> >>>> Armada 375 and therefore there is no serdes_phy_config() or ddr3_init()
> >>>> function. So this code needs not to be compiled at all and usage of
> >>>> #ifdef is correct here.  
> >>>
> >>> #ifdefs are almost never correct in C-files, for the parts of the code
> >>> they guard isn't put through syntactic analysis, and can therefore
> >>> contain bugs which we are not warned about.
> >>>
> >>> Using if (IS_ENABLED()) almost never producess a different binary,
> >>> because the code is optimized away.
> >>>
> >>> Marek  
> >>
> >> There is no function serdes_phy_config() for Armada 375, so if you put
> >> it outside of #ifdef you will get compile error.  
> > 
> > The function is always declared in
> >    arch/arm/mach-mvebu/include/mach/cpu.h
> > regardless of architecture.
> > 
> > Thus an error will be raised only when linking, and the compliation was
> > done with -O0, which I don't think anyone does.
> > 
> > Anyway, if we want to support -O0, this can and should be solved via
> > defining serdes_phy_config() as empty static inline function in the
> > cpu.h header, guarded by #ifdef. In header files #ifdefs are allowed,
> > in this manner:
> >    #if X
> >    declare function
> >    #else
> >    define that function as empty static inline
> >    #endif
> > 
> > So if you think we should support -O0, I can do this.
> > 
> > But the #ifdefs should really go away from real C code, that is the way
> > both Linux and U-Boot are trying to go for the last couple of years, if
> > I understand it correctly.  
> 
> Yes, the #ifdef's really should be avoided if possible. So *if* your
> patch above does not generate any build issues, then I don't see any
> problems to include it. I personally don't think that we need to support
> -O0 builds.

db-88f6720_defconfig builds without issue (armada 375). And it builds the
relevant file, spl/arch/arm/mach-mvebu/spl.o.

Marek

Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible

[Pali Rohár]

On Tuesday 14 December 2021 14:01:04 Marek Behún wrote:
> On Tue, 14 Dec 2021 13:48:31 +0100
> Stefan Roese <sr@denx.de> wrote:
> 
> > On 12/14/21 13:45, Marek Behún wrote:
> > > On Tue, 14 Dec 2021 12:12:34 +0100
> > > Pali Rohár <pali@kernel.org> wrote:
> > >   
> > >> On Tuesday 14 December 2021 10:45:15 Marek Behún wrote:  
> > >>> On Tue, 14 Dec 2021 10:36:00 +0100
> > >>> Pali Rohár <pali@kernel.org> wrote:
> > >>>      
> > >>>> On Friday 26 November 2021 15:37:37 Marek Behún wrote:  
> > >>>>> @@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
> > >>>>>   	timer_init();
> > >>>>>   
> > >>>>>   	/* Armada 375 does not support SerDes and DDR3 init yet */
> > >>>>> -#if !defined(CONFIG_ARMADA_375)
> > >>>>> -	/* First init the serdes PHY's */
> > >>>>> -	serdes_phy_config();
> > >>>>> -
> > >>>>> -	/* Setup DDR */
> > >>>>> -	ret = ddr3_init();
> > >>>>> -	if (ret) {
> > >>>>> -		debug("ddr3_init() failed: %d\n", ret);
> > >>>>> -		hang();
> > >>>>> +	if (!IS_ENABLED(CONFIG_ARMADA_375)) {
> > >>>>> +		/* First init the serdes PHY's */
> > >>>>> +		serdes_phy_config();
> > >>>>> +
> > >>>>> +		/* Setup DDR */
> > >>>>> +		ret = ddr3_init();
> > >>>>> +		if (ret) {
> > >>>>> +			debug("ddr3_init() failed: %d\n", ret);
> > >>>>> +			hang();
> > >>>>> +		}
> > >>>>>   	}
> > >>>>> -#endif  
> > >>>>
> > >>>> As written in comment above there is no SerDes and DDR3 support for
> > >>>> Armada 375 and therefore there is no serdes_phy_config() or ddr3_init()
> > >>>> function. So this code needs not to be compiled at all and usage of
> > >>>> #ifdef is correct here.  
> > >>>
> > >>> #ifdefs are almost never correct in C-files, for the parts of the code
> > >>> they guard isn't put through syntactic analysis, and can therefore
> > >>> contain bugs which we are not warned about.
> > >>>
> > >>> Using if (IS_ENABLED()) almost never producess a different binary,
> > >>> because the code is optimized away.
> > >>>
> > >>> Marek  
> > >>
> > >> There is no function serdes_phy_config() for Armada 375, so if you put
> > >> it outside of #ifdef you will get compile error.  
> > > 
> > > The function is always declared in
> > >    arch/arm/mach-mvebu/include/mach/cpu.h
> > > regardless of architecture.
> > > 
> > > Thus an error will be raised only when linking, and the compliation was
> > > done with -O0, which I don't think anyone does.
> > > 
> > > Anyway, if we want to support -O0, this can and should be solved via
> > > defining serdes_phy_config() as empty static inline function in the
> > > cpu.h header, guarded by #ifdef. In header files #ifdefs are allowed,
> > > in this manner:
> > >    #if X
> > >    declare function
> > >    #else
> > >    define that function as empty static inline
> > >    #endif
> > > 
> > > So if you think we should support -O0, I can do this.
> > > 
> > > But the #ifdefs should really go away from real C code, that is the way
> > > both Linux and U-Boot are trying to go for the last couple of years, if
> > > I understand it correctly.  
> > 
> > Yes, the #ifdef's really should be avoided if possible. So *if* your
> > patch above does not generate any build issues, then I don't see any
> > problems to include it. I personally don't think that we need to support
> > -O0 builds.
> 
> db-88f6720_defconfig builds without issue (armada 375). And it builds the
> relevant file, spl/arch/arm/mach-mvebu/spl.o.
> 
> Marek

-O0 is useful for debugging purposes, it generates more readable
assembler code.

Anyway, the issue here is that those two functions are not defined and
implemented for armada 375 soc. #ifdef is here to selectively do not
compile code which is not implemented on armada 375. And this cannot be
done by normal if(). The reason that it currently works is just because
gcc compiler does not do all checks before doing optimizations and so it
currently does generate any errors or warnings. But this is just
undefined behavior and like any other undefined behavior it may change
in some future version of gcc (or changing compiler to some other).

This approach with converting correct #ifdef to if() with undefined
behavior just hides the real issue that those two functions are not
defined and implemented for all mvebu platforms.

Why not rather to define these two functions are empty static inline
stubs with big comment that they are missing? I think this is proper
solution as it does not depends on undefined behavior of compiler and
linker, supports also -O0 and removes that #ifdef in spl.c file.

Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible

[Marek Behún]

On Thu, 16 Dec 2021 19:16:40 +0100
Pali Rohár <pali@kernel.org> wrote:

> The reason that it currently works is just because
> gcc compiler does not do all checks before doing optimizations and so it
> currently does generate any errors or warnings.

Compiler cannot currently check this, only linker, because the function
is always declared in mvebu's cpu.h.

See https://lore.kernel.org/u-boot/20211214134536.2baeb2a0@thinkpad/
where I also proposed empty static inline implementations for non-A375
platforms, but Stefan thinks it's not an issue currently, because it
does not cause any regressions, I guess. U-Boot's build system
currently does not allow for -O0, you can choose only -O2 or -Os.

We can always add empty static inline implementations into mvebu's
cpu.h when it becomes an issue, or you can send a patch now, if you
want a completely perfect code ASAP.

But note that for that you'll need to check other functions there, as
well. (If you look at
  https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/mach-mvebu/include/mach/cpu.h
there are functions declared, without guarding #ifs, for all mvebu
platforms: A3k, A7k, A37x and A38x.)

Marek

Re: [PATCH u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible

[Marek Behún]

On Thu, 16 Dec 2021 23:09:03 +0100
Marek Behún <kabel@kernel.org> wrote:

> On Thu, 16 Dec 2021 19:16:40 +0100
> Pali Rohár <pali@kernel.org> wrote:
> 
> > The reason that it currently works is just because
> > gcc compiler does not do all checks before doing optimizations and so it
> > currently does generate any errors or warnings.  
> 
> Compiler cannot currently check this, only linker, because the function
> is always declared in mvebu's cpu.h.
> 
> See https://lore.kernel.org/u-boot/20211214134536.2baeb2a0@thinkpad/
> where I also proposed empty static inline implementations for non-A375
> platforms, but Stefan thinks it's not an issue currently, because it
> does not cause any regressions, I guess. U-Boot's build system
> currently does not allow for -O0, you can choose only -O2 or -Os.
> 
> We can always add empty static inline implementations into mvebu's
> cpu.h when it becomes an issue, or you can send a patch now, if you
> want a completely perfect code ASAP.
> 
> But note that for that you'll need to check other functions there, as
> well. (If you look at
>   https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/mach-mvebu/include/mach/cpu.h
> there are functions declared, without guarding #ifs, for all mvebu
> platforms: A3k, A7k, A37x and A38x.)

And btw, I just tried forcing -O0, and didn't even get to SPL compiling
stage. U-Boot proper didn't failed to link with:

   undefined reference to `of_read_u32_index'
   undefined reference to `of_read_u64'
   undefined reference to `of_find_property'
   undefined reference to `of_read_u32_array'
   undefined reference to `of_device_is_available'
   undefined reference to `of_get_parent'
   undefined reference to `of_get_address'
   undefined reference to `of_n_size_cells'
   undefined reference to `of_translate_address'
   undefined reference to `of_n_addr_cells'
   undefined reference to `of_property_match_string'
   undefined reference to `of_parse_phandle_with_args'
   undefined reference to `of_count_phandle_with_args'
   undefined reference to `of_find_node_opts_by_path'
   undefined reference to `of_get_property'
   undefined reference to `of_device_is_available'
   undefined reference to `of_get_property'
   undefined reference to `of_simple_addr_cells'
   undefined reference to `of_simple_size_cells'
   undefined reference to `of_device_is_compatible'
   undefined reference to `of_get_stdout'

Since no-one noticed this till now, I would bet the reality is that -O0
really isn't done, and if someone really needs it, they will have to
fix other things as well.

Also with -O0 I think SPL would be too big so you won't be able to test
it anyway. Although you could study generated and linked assembler
code, but why would you do that? You can just disassemble the object
file.

Marek

[PATCH u-boot-marvell v2 9/9] arm: mvebu: spl: Fix 100 column exceeds

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

Fix 100 column exceeds in arch/arm/mach-mvebu/spl.c.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 arch/arm/mach-mvebu/spl.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index 7dbe8eeba3..7450e1e3da 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -47,7 +47,8 @@
 #if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR) && CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR != 0
 #error CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR must be set to 0
 #endif
-#if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET) && CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET != 0
+#if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET) && \
+    CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET != 0
 #error CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET must be set to 0
 #endif
 #endif
@@ -58,7 +59,8 @@
  * set to 1. Otherwise U-Boot SPL would not be able to load U-Boot proper.
  */
 #ifdef CONFIG_SPL_SATA
-#if !defined(CONFIG_SPL_SATA_RAW_U_BOOT_USE_SECTOR) || !defined(CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR) || CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR != 1
+#if !defined(CONFIG_SPL_SATA_RAW_U_BOOT_USE_SECTOR) || \
+    !defined(CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR) || CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR != 1
 #error CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR must be set to 1
 #endif
 #endif
-- 
2.32.0

Re: [PATCH u-boot-marvell v2 9/9] arm: mvebu: spl: Fix 100 column exceeds

[Stefan Roese]

On 11/26/21 15:37, Marek Behún wrote:
> From: Marek Behún <marek.behun@nic.cz>
> 
> Fix 100 column exceeds in arch/arm/mach-mvebu/spl.c.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

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

Thanks,
Stefan

> ---
>   arch/arm/mach-mvebu/spl.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
> index 7dbe8eeba3..7450e1e3da 100644
> --- a/arch/arm/mach-mvebu/spl.c
> +++ b/arch/arm/mach-mvebu/spl.c
> @@ -47,7 +47,8 @@
>   #if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR) && CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR != 0
>   #error CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR must be set to 0
>   #endif
> -#if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET) && CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET != 0
> +#if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET) && \
> +    CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET != 0
>   #error CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET must be set to 0
>   #endif
>   #endif
> @@ -58,7 +59,8 @@
>    * set to 1. Otherwise U-Boot SPL would not be able to load U-Boot proper.
>    */
>   #ifdef CONFIG_SPL_SATA
> -#if !defined(CONFIG_SPL_SATA_RAW_U_BOOT_USE_SECTOR) || !defined(CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR) || CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR != 1
> +#if !defined(CONFIG_SPL_SATA_RAW_U_BOOT_USE_SECTOR) || \
> +    !defined(CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR) || CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR != 1
>   #error CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR must be set to 1
>   #endif
>   #endif
> 

Viele Grüße,
Stefan Roese

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