mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/3] imx-bbu-nand-fcb: Show additional information from decoded FCB
@ 2021-10-12  1:53 Trent Piepho
  2021-10-12  1:53 ` [PATCH 2/3] imx-bbu-nand-fcb: Save bootloader location into device parameters Trent Piepho
  2021-10-12  1:53 ` [PATCH 3/3] imx-bbu-nand-fcb: Add command to help debug FCB issues Trent Piepho
  0 siblings, 2 replies; 7+ messages in thread
From: Trent Piepho @ 2021-10-12  1:53 UTC (permalink / raw)
  To: Barebox List; +Cc: Yunus Bas, Trent Piepho

In additional to the raw FCB fields, show some useful values calculated
from them.  E.g., display the size of the bootloader image in kB based
on the configured NAND page size and number of pages in the bootloader
image.  Example (all values in "()" are added):

Checksum:                   0xfffffa78
FingerPrint:                0x20424346
Version:                    0x01000000
DataSetup:                  0x50
DataHold:                   0x3c
AddressSetup:               0x19
DSAMPLE_TIME:               0x06
NandTimingState:            0x00
REA:                        0x00
RLOH:                       0x00
RHOH:                       0x00
PageDataSize:               0x00000800 (2 kB)
TotalPageSize:              0x00000840
SectorsPerBlock:            0x00000040 (128 kB eraseblocks)
NumberOfNANDs:              0x00000000
TotalInternalDie:           0x00000000
CellType:                   0x00000000
EccBlockNEccType:           0x00000004 (8 bits ECC)
EccBlock0Size:              0x00000200
EccBlockNSize:              0x00000200
EccBlock0EccType:           0x00000004 (8 bits ECC)
MetadataBytes:              0x0000000a
NumEccBlocksPerPage:        0x00000003 (total 4 blocks/page)
EccBlockNEccLevelSDK:       0x00000000
EccBlock0SizeSDK:           0x00000000
EccBlockNSizeSDK:           0x00000000
EccBlock0EccLevelSDK:       0x00000000
NumEccBlocksPerPageSDK:     0x00000000
MetadataBytesSDK:           0x00000000
EraseThreshold:             0x00000000
BootPatch:                  0x00000000
PatchSectors:               0x00000000
Firmware1_startingPage:     0x00000100 (address 0x080000)
Firmware2_startingPage:     0x00000380 (address 0x1c0000)
PagesInFirmware1:           0x000000f2 (484 kB)
PagesInFirmware2:           0x000000f2 (484 kB)
DBBTSearchAreaStartAddress: 0x00000001 (address 0x000800)
BadBlockMarkerByte:         0x000007cf
BadBlockMarkerStartBit:     0x00000000
BBMarkerPhysicalOffset:     0x00000800
BCHType:                    0x00000000
TMTiming2_ReadLatency:      0x00000000
TMTiming2_PreambleDelay:    0x00000000
TMTiming2_CEDelay:          0x00000000
TMTiming2_PostambleDelay:   0x00000000
TMTiming2_CmdAddPause:      0x00000000
TMTiming2_DataPause:        0x00000000
TMSpeed:                    0x00000000
TMTiming1_BusyTimeout:      0x00000000
DISBBM:                     0x00000000 (BBM swap enabled)
BBMarkerPhysOfsInSpareData: 0x00000000

Signed-off-by: Trent Piepho <trent.piepho@igorinstitute.com>
---
 common/imx-bbu-nand-fcb.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/common/imx-bbu-nand-fcb.c b/common/imx-bbu-nand-fcb.c
index 3b07d539e..7108a327a 100644
--- a/common/imx-bbu-nand-fcb.c
+++ b/common/imx-bbu-nand-fcb.c
@@ -293,18 +293,23 @@ static __maybe_unused void dump_fcb(void *buf)
 	pr_debug("REA:                        0x%02x\n", fcb->REA);
 	pr_debug("RLOH:                       0x%02x\n", fcb->RLOH);
 	pr_debug("RHOH:                       0x%02x\n", fcb->RHOH);
-	pr_debug("PageDataSize:               0x%08x\n", fcb->PageDataSize);
+	pr_debug("PageDataSize:               0x%08x (%d kB)\n",
+		 fcb->PageDataSize, fcb->PageDataSize / 1024);
 	pr_debug("TotalPageSize:              0x%08x\n", fcb->TotalPageSize);
-	pr_debug("SectorsPerBlock:            0x%08x\n", fcb->SectorsPerBlock);
+	pr_debug("SectorsPerBlock:            0x%08x (%u kB eraseblocks)\n",
+		 fcb->SectorsPerBlock, fcb->PageDataSize * fcb->SectorsPerBlock / 1024);
 	pr_debug("NumberOfNANDs:              0x%08x\n", fcb->NumberOfNANDs);
 	pr_debug("TotalInternalDie:           0x%08x\n", fcb->TotalInternalDie);
 	pr_debug("CellType:                   0x%08x\n", fcb->CellType);
-	pr_debug("EccBlockNEccType:           0x%08x\n", fcb->EccBlockNEccType);
+	pr_debug("EccBlockNEccType:           0x%08x (%d bits ECC)\n",
+		 fcb->EccBlockNEccType, fcb->EccBlockNEccType * 2);
 	pr_debug("EccBlock0Size:              0x%08x\n", fcb->EccBlock0Size);
 	pr_debug("EccBlockNSize:              0x%08x\n", fcb->EccBlockNSize);
-	pr_debug("EccBlock0EccType:           0x%08x\n", fcb->EccBlock0EccType);
+	pr_debug("EccBlock0EccType:           0x%08x (%d bits ECC)\n",
+		 fcb->EccBlock0EccType, fcb->EccBlock0EccType * 2);
 	pr_debug("MetadataBytes:              0x%08x\n", fcb->MetadataBytes);
-	pr_debug("NumEccBlocksPerPage:        0x%08x\n", fcb->NumEccBlocksPerPage);
+	pr_debug("NumEccBlocksPerPage:        0x%08x (total %d blocks/page)\n",
+		 fcb->NumEccBlocksPerPage, fcb->NumEccBlocksPerPage + 1);
 	pr_debug("EccBlockNEccLevelSDK:       0x%08x\n", fcb->EccBlockNEccLevelSDK);
 	pr_debug("EccBlock0SizeSDK:           0x%08x\n", fcb->EccBlock0SizeSDK);
 	pr_debug("EccBlockNSizeSDK:           0x%08x\n", fcb->EccBlockNSizeSDK);
@@ -314,11 +319,16 @@ static __maybe_unused void dump_fcb(void *buf)
 	pr_debug("EraseThreshold:             0x%08x\n", fcb->EraseThreshold);
 	pr_debug("BootPatch:                  0x%08x\n", fcb->BootPatch);
 	pr_debug("PatchSectors:               0x%08x\n", fcb->PatchSectors);
-	pr_debug("Firmware1_startingPage:     0x%08x\n", fcb->Firmware1_startingPage);
-	pr_debug("Firmware2_startingPage:     0x%08x\n", fcb->Firmware2_startingPage);
-	pr_debug("PagesInFirmware1:           0x%08x\n", fcb->PagesInFirmware1);
-	pr_debug("PagesInFirmware2:           0x%08x\n", fcb->PagesInFirmware2);
-	pr_debug("DBBTSearchAreaStartAddress: 0x%08x\n", fcb->DBBTSearchAreaStartAddress);
+	pr_debug("Firmware1_startingPage:     0x%08x (address 0x%06x)\n",
+		 fcb->Firmware1_startingPage, fcb->Firmware1_startingPage * fcb->PageDataSize);
+	pr_debug("Firmware2_startingPage:     0x%08x (address 0x%06x)\n",
+		 fcb->Firmware2_startingPage, fcb->Firmware2_startingPage * fcb->PageDataSize);
+	pr_debug("PagesInFirmware1:           0x%08x (%d kB)\n",
+		 fcb->PagesInFirmware1, fcb->PagesInFirmware1 * fcb->PageDataSize / 1024);
+	pr_debug("PagesInFirmware2:           0x%08x (%d kB)\n",
+		 fcb->PagesInFirmware2, fcb->PagesInFirmware2 * fcb->PageDataSize / 1024);
+	pr_debug("DBBTSearchAreaStartAddress: 0x%08x (address 0x%06x)\n",
+		 fcb->DBBTSearchAreaStartAddress, fcb->DBBTSearchAreaStartAddress * fcb->PageDataSize);
 	pr_debug("BadBlockMarkerByte:         0x%08x\n", fcb->BadBlockMarkerByte);
 	pr_debug("BadBlockMarkerStartBit:     0x%08x\n", fcb->BadBlockMarkerStartBit);
 	pr_debug("BBMarkerPhysicalOffset:     0x%08x\n", fcb->BBMarkerPhysicalOffset);
@@ -331,7 +341,8 @@ static __maybe_unused void dump_fcb(void *buf)
 	pr_debug("TMTiming2_DataPause:        0x%08x\n", fcb->TMTiming2_DataPause);
 	pr_debug("TMSpeed:                    0x%08x\n", fcb->TMSpeed);
 	pr_debug("TMTiming1_BusyTimeout:      0x%08x\n", fcb->TMTiming1_BusyTimeout);
-	pr_debug("DISBBM:                     0x%08x\n", fcb->DISBBM);
+	pr_debug("DISBBM:                     0x%08x (BBM swap %sabled)\n",
+		 fcb->DISBBM, fcb->DISBBM ? "dis" : "en");
 	pr_debug("BBMarkerPhysOfsInSpareData: 0x%08x\n", fcb->BBMarkerPhysicalOffsetInSpareData);
 }
 
-- 
2.31.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/3] imx-bbu-nand-fcb: Save bootloader location into device parameters
  2021-10-12  1:53 [PATCH 1/3] imx-bbu-nand-fcb: Show additional information from decoded FCB Trent Piepho
@ 2021-10-12  1:53 ` Trent Piepho
  2021-10-12  8:11   ` Sascha Hauer
  2021-10-12  1:53 ` [PATCH 3/3] imx-bbu-nand-fcb: Add command to help debug FCB issues Trent Piepho
  1 sibling, 1 reply; 7+ messages in thread
From: Trent Piepho @ 2021-10-12  1:53 UTC (permalink / raw)
  To: Barebox List; +Cc: Yunus Bas, Trent Piepho

When updating the FCB, save the location and size of the Barebox images
into parameters on the NAND device.  Since the location and padding is
calculated based on net NAND partition size, with an unknown number of
bad blocks, these values aren't known beforehard.  Saving them will
allow extracting or checksumming the Barebox image in NAND.

For example:
sha256sum /dev/nand0.barebox ${nand0.barebox.firmware1_addr}+${nand0.barebox.firmware1_size}
memcpy -s /dev/nand0.barebox -d /mnt/mmc0.0/imagedump ${nand0.barebox.firmware2_addr} 0 ${nand0.barebox.firmware2_size}

Find sum of Barebox image file with NUL padding to NAND page size (4kB
here) in Linux:
dd if=barebox-myboard.img ibs=4k conv=sync | sha256sum

Signed-off-by: Trent Piepho <trent.piepho@igorinstitute.com>
---
 common/imx-bbu-nand-fcb.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/common/imx-bbu-nand-fcb.c b/common/imx-bbu-nand-fcb.c
index 7108a327a..76ac1d4f2 100644
--- a/common/imx-bbu-nand-fcb.c
+++ b/common/imx-bbu-nand-fcb.c
@@ -227,6 +227,15 @@ static uint32_t calc_chksum(void *buf, size_t size)
 	return ~chksum;
 }
 
+/* Set parameters on the device with the firmware location and size */
+static void set_dev_params(struct cdev *cdev, const struct fcb_block *fcb)
+{
+	dev_add_param_uint32_fixed(cdev->dev, "firmware1_addr", fcb->Firmware1_startingPage * fcb->PageDataSize, "0x%08x");
+	dev_add_param_uint32_fixed(cdev->dev, "firmware2_addr", fcb->Firmware2_startingPage * fcb->PageDataSize, "0x%08x");
+	dev_add_param_uint32_fixed(cdev->dev, "firmware1_size", fcb->PagesInFirmware1 * fcb->PageDataSize, "0x%08x");
+	dev_add_param_uint32_fixed(cdev->dev, "firmware2_size", fcb->PagesInFirmware2 * fcb->PageDataSize, "0x%08x");
+}
+
 static struct fcb_block *read_fcb_hamming_13_8(void *rawpage)
 {
 	int i;
@@ -1363,6 +1372,8 @@ static int imx_bbu_nand_update(struct bbu_handler *handler, struct bbu_data *dat
 			goto out;
 	}
 
+	set_dev_params(bcb_cdev, fcb);
+
 out:
 	free(fw);
 	free(fcb);
-- 
2.31.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/3] imx-bbu-nand-fcb: Add command to help debug FCB issues
  2021-10-12  1:53 [PATCH 1/3] imx-bbu-nand-fcb: Show additional information from decoded FCB Trent Piepho
  2021-10-12  1:53 ` [PATCH 2/3] imx-bbu-nand-fcb: Save bootloader location into device parameters Trent Piepho
@ 2021-10-12  1:53 ` Trent Piepho
  2021-10-12  8:21   ` Sascha Hauer
  1 sibling, 1 reply; 7+ messages in thread
From: Trent Piepho @ 2021-10-12  1:53 UTC (permalink / raw)
  To: Barebox List; +Cc: Yunus Bas, Trent Piepho

Add new "fcb" command.  It can save a decoded copy of the FCB to a file,
do a hexdump of the decoded FCB, or display the FCB fields.  Or simply
read and validate the FCB.

The FCB uses a different ECC system that the rest of flash and there is
no easy way to decode it in Barebox or Linux.  The code already here
does it.

This will also set the nand0.barebox device parameters with the location
of the bootloader images as read from the FCB.

Signed-off-by: Trent Piepho <trent.piepho@igorinstitute.com>
---
 commands/Kconfig          |  19 ++++++
 common/imx-bbu-nand-fcb.c | 126 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 139 insertions(+), 6 deletions(-)

diff --git a/commands/Kconfig b/commands/Kconfig
index 5ae3cb3dd..d3b5cd7fa 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -277,6 +277,25 @@ config CMD_SLICE
 	  command can be used to print informations about slices and also to manipulate
 	  them on the command line for debugging purposes.
 
+config CMD_IMX_NAND_FCB
+	tristate
+	prompt "iMX FCB decoding and display"
+	depends on BAREBOX_UPDATE_IMX_NAND_FCB
+	help
+	  Decode and display or save the contents of the iMX FCB.
+
+	  This will add a command named "fcb" that will decode the FCB and can
+	  save the decode data to a file or display the contents.
+
+	  The FCB is a block of data at the start of NAND flash that instructs
+	  the iMX ROM bootloader on how to find Barebox.  It uses a different
+	  ECC config than the rest of NAND flash and can't be read correctly
+	  with normal "md" commands.
+
+	  The command also saves the locations of the Barebox image in NAND
+	  from the FCB into parameters on the NAND deivce, which are available
+	  in scripts as environment variables.
+
 # end Information commands
 endmenu
 
diff --git a/common/imx-bbu-nand-fcb.c b/common/imx-bbu-nand-fcb.c
index 76ac1d4f2..e61494930 100644
--- a/common/imx-bbu-nand-fcb.c
+++ b/common/imx-bbu-nand-fcb.c
@@ -7,6 +7,8 @@
 
 #include <filetype.h>
 #include <common.h>
+#include <command.h>
+#include <getopt.h>
 #include <malloc.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -14,6 +16,7 @@
 #include <linux/sizes.h>
 #include <bbu.h>
 #include <fs.h>
+#include <libfile.h>
 #include <linux/mtd/mtd-abi.h>
 #include <linux/mtd/nand_mxs.h>
 #include <linux/mtd/mtd.h>
@@ -27,6 +30,9 @@
 #include <mtd/mtd-peb.h>
 #include <soc/imx/imx-nand-bcb.h>
 
+/* Name of NAND device that contains FCB */
+#define FCB_NAND_PART "nand0.barebox"
+
 #ifdef CONFIG_ARCH_IMX6
 #include <mach/imx6.h>
 static inline int fcb_is_bch_encoded(void)
@@ -387,6 +393,7 @@ static ssize_t raw_write_page(struct mtd_info *mtd, void *buf, loff_t offset)
         return ret;
 }
 
+/* Returns size of FCB on success, negative on error */
 static int read_fcb(struct mtd_info *mtd, int num, struct fcb_block **retfcb)
 {
 	int ret;
@@ -403,10 +410,13 @@ static int read_fcb(struct mtd_info *mtd, int num, struct fcb_block **retfcb)
 		goto err;
 	}
 
-	if (fcb_is_bch_encoded())
+	if (fcb_is_bch_encoded()) {
 		fcb = read_fcb_bch(rawpage, 40);
-	else
+		ret = 128 * 8;
+	} else {
 		fcb = read_fcb_hamming_13_8(rawpage);
+		ret = 512;
+	}
 
 	if (IS_ERR(fcb)) {
 		pr_err("Cannot read fcb on block %d\n", num);
@@ -415,7 +425,6 @@ static int read_fcb(struct mtd_info *mtd, int num, struct fcb_block **retfcb)
 	}
 
 	*retfcb = fcb;
-	ret = 0;
 err:
 	free(rawpage);
 
@@ -870,7 +879,7 @@ static int fcb_dbbt_check(struct mtd_info *mtd, int num, struct fcb_block *fcb)
 	int pages_per_block = mtd->erasesize / mtd->writesize;
 
 	ret = read_fcb(mtd, num, &f);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	if (memcmp(fcb, f, sizeof(*fcb))) {
@@ -1403,7 +1412,7 @@ int imx6_bbu_nand_register_handler(const char *name, unsigned long flags)
 	imx_handler->filetype = filetype_arm_barebox;
 
 	handler = &imx_handler->handler;
-	handler->devicefile = "nand0.barebox";
+	handler->devicefile = FCB_NAND_PART;
 	handler->name = name;
 	handler->flags = flags | BBU_HANDLER_CAN_REFRESH;
 	handler->handler = imx_bbu_nand_update;
@@ -1480,7 +1489,7 @@ int imx28_bbu_nand_register_handler(const char *name, unsigned long flags)
 	imx_handler->filetype = filetype_mxs_bootstream;
 
 	handler = &imx_handler->handler;
-	handler->devicefile = "nand0.barebox";
+	handler->devicefile = FCB_NAND_PART;
 	handler->name = name;
 	handler->flags = flags | BBU_HANDLER_CAN_REFRESH;
 	handler->handler = imx_bbu_nand_update;
@@ -1492,3 +1501,108 @@ int imx28_bbu_nand_register_handler(const char *name, unsigned long flags)
 	return ret;
 }
 #endif
+
+#if IS_ENABLED(CONFIG_CMD_IMX_NAND_FCB)
+
+static int do_fcb(int argc, char *argv[])
+{
+	int opt;
+	int fd;
+	int ret;
+	int fcbsize;
+	struct cdev *cdev;
+	struct fcb_block *fcb;
+	bool hex = false, info = false;
+	const char *outfile = NULL;
+	unsigned int block = 0;
+
+	while ((opt = getopt(argc, argv, "xin:o:")) > 0) {
+		switch (opt) {
+		case 'x':
+			hex = true;
+			break;
+		case 'i':
+			info = true;
+			break;
+		case 'o':
+			outfile = optarg;
+			break;
+		case 'n':
+			block = strtoull_suffix(optarg, NULL, 0);
+			break;
+		default:
+			return COMMAND_ERROR_USAGE;
+		}
+	}
+
+	if (optind != argc)
+		return COMMAND_ERROR_USAGE;
+
+	cdev = cdev_by_name(FCB_NAND_PART);
+	if (!cdev) {
+		pr_err("Couldn't find FCB flash device '%s'\n", FCB_NAND_PART);
+		return -ENODEV;
+	}
+
+	ret = read_fcb(cdev->mtd, block, &fcb);
+	if (ret < 0) {
+		perror("read_fcb");
+		return ret;
+	}
+	fcbsize = ret;
+	ret = 0;
+	if (!info && !hex && !outfile)
+		printf("FCB OK (%d bytes)\n", fcbsize);
+
+	set_dev_params(cdev, fcb);
+
+	if (info) {
+		const int oldlevel = barebox_loglevel;
+
+		barebox_loglevel = MSG_DEBUG;
+		pr_debug("Decoded FCB size:           %d bytes\n", fcbsize);
+		dump_fcb(fcb);
+		barebox_loglevel = oldlevel;
+	}
+
+	if (hex)
+		memory_display(fcb, 0, fcbsize, 1, 0);
+
+	if (outfile) {
+		fd = open(outfile, O_WRONLY | O_CREAT, 0644);
+		if (fd < 0) {
+			perror("open");
+			ret = fd;
+			goto out;
+		}
+		ret = write_full(fd, fcb, fcbsize);
+		close(fd);
+		if (ret < 0) {
+			perror("write");
+			goto out;
+		}
+	}
+
+out:
+	free(fcb);
+	return ret;
+}
+
+
+BAREBOX_CMD_HELP_START(fcb)
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT("-i", "Print information from FCB")
+BAREBOX_CMD_HELP_OPT("-x", "Display FCB data bytes")
+BAREBOX_CMD_HELP_OPT("-n BLK", "Eraseblock number to read FCB from")
+BAREBOX_CMD_HELP_OPT("-o FILE", "Write decoded FCB to file")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(fcb)
+	.cmd		= do_fcb,
+	BAREBOX_CMD_DESC("i.MX FCB decoder")
+	BAREBOX_CMD_OPTS("[-ixno]")
+	BAREBOX_CMD_GROUP(CMD_GRP_INFO)
+	BAREBOX_CMD_HELP(cmd_fcb_help)
+BAREBOX_CMD_END
+
+#endif /*CONFIG_CMD_IMX_NAND_FCB*/
-- 
2.31.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] imx-bbu-nand-fcb: Save bootloader location into device parameters
  2021-10-12  1:53 ` [PATCH 2/3] imx-bbu-nand-fcb: Save bootloader location into device parameters Trent Piepho
@ 2021-10-12  8:11   ` Sascha Hauer
  0 siblings, 0 replies; 7+ messages in thread
From: Sascha Hauer @ 2021-10-12  8:11 UTC (permalink / raw)
  To: Trent Piepho; +Cc: Barebox List, Yunus Bas

On Mon, Oct 11, 2021 at 06:53:58PM -0700, Trent Piepho wrote:
> When updating the FCB, save the location and size of the Barebox images
> into parameters on the NAND device.  Since the location and padding is
> calculated based on net NAND partition size, with an unknown number of
> bad blocks, these values aren't known beforehard.  Saving them will
> allow extracting or checksumming the Barebox image in NAND.
> 
> For example:
> sha256sum /dev/nand0.barebox ${nand0.barebox.firmware1_addr}+${nand0.barebox.firmware1_size}
> memcpy -s /dev/nand0.barebox -d /mnt/mmc0.0/imagedump ${nand0.barebox.firmware2_addr} 0 ${nand0.barebox.firmware2_size}

As you are adding a fcb utility command anyway in the next patch, maybe
it would be more convenient for the user to add a firmware extraction
option there instead of adding these device parameters.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] imx-bbu-nand-fcb: Add command to help debug FCB issues
  2021-10-12  1:53 ` [PATCH 3/3] imx-bbu-nand-fcb: Add command to help debug FCB issues Trent Piepho
@ 2021-10-12  8:21   ` Sascha Hauer
  2021-10-13 10:14     ` Trent Piepho
  0 siblings, 1 reply; 7+ messages in thread
From: Sascha Hauer @ 2021-10-12  8:21 UTC (permalink / raw)
  To: Trent Piepho; +Cc: Barebox List, Yunus Bas

On Mon, Oct 11, 2021 at 06:53:59PM -0700, Trent Piepho wrote:
> Add new "fcb" command.  It can save a decoded copy of the FCB to a file,
> do a hexdump of the decoded FCB, or display the FCB fields.  Or simply
> read and validate the FCB.
> 
> The FCB uses a different ECC system that the rest of flash and there is
> no easy way to decode it in Barebox or Linux.  The code already here
> does it.
> 
> This will also set the nand0.barebox device parameters with the location
> of the bootloader images as read from the FCB.
> 
> Signed-off-by: Trent Piepho <trent.piepho@igorinstitute.com>
> ---
>  commands/Kconfig          |  19 ++++++
>  common/imx-bbu-nand-fcb.c | 126 ++++++++++++++++++++++++++++++++++++--
>  2 files changed, 139 insertions(+), 6 deletions(-)
> 
> diff --git a/commands/Kconfig b/commands/Kconfig
> index 5ae3cb3dd..d3b5cd7fa 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -277,6 +277,25 @@ config CMD_SLICE
>  	  command can be used to print informations about slices and also to manipulate
>  	  them on the command line for debugging purposes.
>  
> +config CMD_IMX_NAND_FCB
> +	tristate
> +	prompt "iMX FCB decoding and display"
> +	depends on BAREBOX_UPDATE_IMX_NAND_FCB
> +	help
> +	  Decode and display or save the contents of the iMX FCB.
> +
> +	  This will add a command named "fcb" that will decode the FCB and can
> +	  save the decode data to a file or display the contents.
> +
> +	  The FCB is a block of data at the start of NAND flash that instructs
> +	  the iMX ROM bootloader on how to find Barebox.  It uses a different
> +	  ECC config than the rest of NAND flash and can't be read correctly
> +	  with normal "md" commands.
> +
> +	  The command also saves the locations of the Barebox image in NAND
> +	  from the FCB into parameters on the NAND deivce, which are available
> +	  in scripts as environment variables.
> +
>  # end Information commands
>  endmenu
>  
> diff --git a/common/imx-bbu-nand-fcb.c b/common/imx-bbu-nand-fcb.c
> index 76ac1d4f2..e61494930 100644
> --- a/common/imx-bbu-nand-fcb.c
> +++ b/common/imx-bbu-nand-fcb.c
> @@ -7,6 +7,8 @@
>  
>  #include <filetype.h>
>  #include <common.h>
> +#include <command.h>
> +#include <getopt.h>
>  #include <malloc.h>
>  #include <errno.h>
>  #include <fcntl.h>
> @@ -14,6 +16,7 @@
>  #include <linux/sizes.h>
>  #include <bbu.h>
>  #include <fs.h>
> +#include <libfile.h>
>  #include <linux/mtd/mtd-abi.h>
>  #include <linux/mtd/nand_mxs.h>
>  #include <linux/mtd/mtd.h>
> @@ -27,6 +30,9 @@
>  #include <mtd/mtd-peb.h>
>  #include <soc/imx/imx-nand-bcb.h>
>  
> +/* Name of NAND device that contains FCB */
> +#define FCB_NAND_PART "nand0.barebox"
> +
>  #ifdef CONFIG_ARCH_IMX6
>  #include <mach/imx6.h>
>  static inline int fcb_is_bch_encoded(void)
> @@ -387,6 +393,7 @@ static ssize_t raw_write_page(struct mtd_info *mtd, void *buf, loff_t offset)
>          return ret;
>  }
>  
> +/* Returns size of FCB on success, negative on error */
>  static int read_fcb(struct mtd_info *mtd, int num, struct fcb_block **retfcb)
>  {
>  	int ret;
> @@ -403,10 +410,13 @@ static int read_fcb(struct mtd_info *mtd, int num, struct fcb_block **retfcb)
>  		goto err;
>  	}
>  
> -	if (fcb_is_bch_encoded())
> +	if (fcb_is_bch_encoded()) {
>  		fcb = read_fcb_bch(rawpage, 40);
> -	else
> +		ret = 128 * 8;
> +	} else {
>  		fcb = read_fcb_hamming_13_8(rawpage);
> +		ret = 512;
> +	}
>  
>  	if (IS_ERR(fcb)) {
>  		pr_err("Cannot read fcb on block %d\n", num);
> @@ -415,7 +425,6 @@ static int read_fcb(struct mtd_info *mtd, int num, struct fcb_block **retfcb)
>  	}
>  
>  	*retfcb = fcb;
> -	ret = 0;
>  err:
>  	free(rawpage);
>  
> @@ -870,7 +879,7 @@ static int fcb_dbbt_check(struct mtd_info *mtd, int num, struct fcb_block *fcb)
>  	int pages_per_block = mtd->erasesize / mtd->writesize;
>  
>  	ret = read_fcb(mtd, num, &f);
> -	if (ret)
> +	if (ret < 0)
>  		return ret;
>  
>  	if (memcmp(fcb, f, sizeof(*fcb))) {
> @@ -1403,7 +1412,7 @@ int imx6_bbu_nand_register_handler(const char *name, unsigned long flags)
>  	imx_handler->filetype = filetype_arm_barebox;
>  
>  	handler = &imx_handler->handler;
> -	handler->devicefile = "nand0.barebox";
> +	handler->devicefile = FCB_NAND_PART;
>  	handler->name = name;
>  	handler->flags = flags | BBU_HANDLER_CAN_REFRESH;
>  	handler->handler = imx_bbu_nand_update;
> @@ -1480,7 +1489,7 @@ int imx28_bbu_nand_register_handler(const char *name, unsigned long flags)
>  	imx_handler->filetype = filetype_mxs_bootstream;
>  
>  	handler = &imx_handler->handler;
> -	handler->devicefile = "nand0.barebox";
> +	handler->devicefile = FCB_NAND_PART;
>  	handler->name = name;
>  	handler->flags = flags | BBU_HANDLER_CAN_REFRESH;
>  	handler->handler = imx_bbu_nand_update;
> @@ -1492,3 +1501,108 @@ int imx28_bbu_nand_register_handler(const char *name, unsigned long flags)
>  	return ret;
>  }
>  #endif
> +
> +#if IS_ENABLED(CONFIG_CMD_IMX_NAND_FCB)
> +
> +static int do_fcb(int argc, char *argv[])
> +{
> +	int opt;
> +	int fd;
> +	int ret;
> +	int fcbsize;
> +	struct cdev *cdev;
> +	struct fcb_block *fcb;
> +	bool hex = false, info = false;
> +	const char *outfile = NULL;
> +	unsigned int block = 0;
> +
> +	while ((opt = getopt(argc, argv, "xin:o:")) > 0) {
> +		switch (opt) {
> +		case 'x':
> +			hex = true;
> +			break;
> +		case 'i':
> +			info = true;
> +			break;
> +		case 'o':
> +			outfile = optarg;
> +			break;
> +		case 'n':
> +			block = strtoull_suffix(optarg, NULL, 0);
> +			break;
> +		default:
> +			return COMMAND_ERROR_USAGE;
> +		}
> +	}

Not sure if we need to control this command in such a fine grained way.
For me just extracting all possible FCBs including the firmware images,
maybe printing consistency information would be enough. That's just a
personal opinion though, feel free to override it.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] imx-bbu-nand-fcb: Add command to help debug FCB issues
  2021-10-12  8:21   ` Sascha Hauer
@ 2021-10-13 10:14     ` Trent Piepho
  2021-10-14 12:09       ` Sascha Hauer
  0 siblings, 1 reply; 7+ messages in thread
From: Trent Piepho @ 2021-10-13 10:14 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List, Yunus Bas

On Tue, Oct 12, 2021 at 1:21 AM Sascha Hauer <sha@pengutronix.de> wrote:
> On Mon, Oct 11, 2021 at 06:53:59PM -0700, Trent Piepho wrote:
> > Add new "fcb" command.  It can save a decoded copy of the FCB to a file,
> > do a hexdump of the decoded FCB, or display the FCB fields.  Or simply
> > read and validate the FCB.

> Not sure if we need to control this command in such a fine grained way.
> For me just extracting all possible FCBs including the firmware images,
> maybe printing consistency information would be enough. That's just a
> personal opinion though, feel free to override it.

Originally I was having issues creating correct FCBs, mostly due to
kobs-ng (I wonder how hard it would be to port barebox_update to
Linux?) and wanted to extract factory FCB and kobs-ng generated FCB.
But copying data from Barebox to Linux and looking at hexdumps was
very tedious.  Really, one wants to see fields of FCB decoded and
Barebox already had code that did this.

So the -i flag that prints out FCB fields and -o to save a copy were
what I wrote originally as debug aid, with -i the most useful to me.
I don't actually want to extract firmware images.  It was the FCB that
was the issue.  I didn't think it would be useful enough to other
people to bother sending it to the list.

But then later there was a thread from Yunas at Phytec about the
difficulty of doing a flash crc check on NAND when one does not know
where data will be due to possible bad blocks.  Extracting this
information from the FCB seemed like the correct way to do it and I
realized it would be easy to add into the command I had written.  So
that is why this feature is there.  And this was evidence that this
would be useful to someone besides myself.

I added hexdump because it seemed like someone might like it and it
was one line.  I could drop this part.

Extracting all possible FCBs has issues.  Number and location of FCBs
varies.  Pin strapping and possibly OTP memory fuses control what the
boot ROM does.  However, the boot ROM's search, what is actually in
flash, and what kobs-ng wants to write, can all be different.  I did
write this debug aid for a real problem!

If it did just write dump everything, then how would it work?  Some
details are not clear to me.

Where does the data go?  Assume /tmp?  Or argument to supply directory name?
It will need multiple files.  How to name?  Arguments for each
filename?  Seems too many arguments.  Or have a fixed filename
pattern?  FCB1, FCB2, firmware1.img, firmware2.img, etc.  Not really a
huge fan of hardcoded filenames.
What happens if the FCBs are not where Barebox thinks they are?  This
really does happen.
What if all the FCBs do not agree on the location/size of the firmware images?
Is it possible extra space used to dump firmware copies into /tmp,
then crc the copies matters vs doing it in place from flash?
What if someone wants to script something with firmware images other
than a checksum?  E.g., they want to erase firmware1 to test that
fallback to firmware2 works.  Or want to know how large the firmware
is.  I do not know of a way to get the size of a file in hush.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] imx-bbu-nand-fcb: Add command to help debug FCB issues
  2021-10-13 10:14     ` Trent Piepho
@ 2021-10-14 12:09       ` Sascha Hauer
  0 siblings, 0 replies; 7+ messages in thread
From: Sascha Hauer @ 2021-10-14 12:09 UTC (permalink / raw)
  To: Trent Piepho; +Cc: Barebox List, Yunus Bas

On Wed, Oct 13, 2021 at 03:14:01AM -0700, Trent Piepho wrote:
> On Tue, Oct 12, 2021 at 1:21 AM Sascha Hauer <sha@pengutronix.de> wrote:
> > On Mon, Oct 11, 2021 at 06:53:59PM -0700, Trent Piepho wrote:
> > > Add new "fcb" command.  It can save a decoded copy of the FCB to a file,
> > > do a hexdump of the decoded FCB, or display the FCB fields.  Or simply
> > > read and validate the FCB.
> 
> > Not sure if we need to control this command in such a fine grained way.
> > For me just extracting all possible FCBs including the firmware images,
> > maybe printing consistency information would be enough. That's just a
> > personal opinion though, feel free to override it.
> 
> Originally I was having issues creating correct FCBs, mostly due to
> kobs-ng (I wonder how hard it would be to port barebox_update to
> Linux?) and wanted to extract factory FCB and kobs-ng generated FCB.
> But copying data from Barebox to Linux and looking at hexdumps was
> very tedious.  Really, one wants to see fields of FCB decoded and
> Barebox already had code that did this.
> 
> So the -i flag that prints out FCB fields and -o to save a copy were
> what I wrote originally as debug aid, with -i the most useful to me.
> I don't actually want to extract firmware images.  It was the FCB that
> was the issue.  I didn't think it would be useful enough to other
> people to bother sending it to the list.
> 
> But then later there was a thread from Yunas at Phytec about the
> difficulty of doing a flash crc check on NAND when one does not know
> where data will be due to possible bad blocks.  Extracting this
> information from the FCB seemed like the correct way to do it and I
> realized it would be easy to add into the command I had written.  So
> that is why this feature is there.  And this was evidence that this
> would be useful to someone besides myself.
> 
> I added hexdump because it seemed like someone might like it and it
> was one line.  I could drop this part.
> 
> Extracting all possible FCBs has issues.  Number and location of FCBs
> varies.  Pin strapping and possibly OTP memory fuses control what the
> boot ROM does.  However, the boot ROM's search, what is actually in
> flash, and what kobs-ng wants to write, can all be different.  I did
> write this debug aid for a real problem!
> 
> If it did just write dump everything, then how would it work?  Some
> details are not clear to me.
> 
> Where does the data go?  Assume /tmp?  Or argument to supply directory name?
> It will need multiple files.  How to name?  Arguments for each
> filename?  Seems too many arguments.  Or have a fixed filename
> pattern?  FCB1, FCB2, firmware1.img, firmware2.img, etc.  Not really a
> huge fan of hardcoded filenames.

Either way would be fine with me. I would use the current directory for
writing files with hardcoded filenames.

> What happens if the FCBs are not where Barebox thinks they are?  This
> really does happen.
> What if all the FCBs do not agree on the location/size of the firmware images?

I would write out all found FCBs by default. Additionally an option to
pass the block number of which FCB should be used like you already did
it.

Generally I think when by default all possible information is extracted
including the FCB/Firmware images, maybe sha256sums of the firmware
imags. That way we could easily add more information if we get new
ideas which infos might be useful additionally. With dedicated options
for each and every piece of information it will become difficult to
parse and hard to track which option is compatible/useful with which
other option. In the end this mostly useful for debugging anyway. I
don't think we should extend this command to do anything useful in a
production case.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-10-14 12:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-12  1:53 [PATCH 1/3] imx-bbu-nand-fcb: Show additional information from decoded FCB Trent Piepho
2021-10-12  1:53 ` [PATCH 2/3] imx-bbu-nand-fcb: Save bootloader location into device parameters Trent Piepho
2021-10-12  8:11   ` Sascha Hauer
2021-10-12  1:53 ` [PATCH 3/3] imx-bbu-nand-fcb: Add command to help debug FCB issues Trent Piepho
2021-10-12  8:21   ` Sascha Hauer
2021-10-13 10:14     ` Trent Piepho
2021-10-14 12:09       ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox