Enable use of DBBT during firmware loading#245
Conversation
Fix issues with DBBT creation. DBBT now includes only bad blocks from kernel partitions. The size is hardcoded to 1 page (5 total), which is sufficient to hold up to 32 bad block markers for our kernel partitions. Enable DBBT usage and disable bad block marker scanning in FCB. JIRA: DTR-507
JIRA: DTR-507
35d52c8 to
521549a
Compare
Unit Test Results9 277 tests +1 076 8 688 ✅ +1 011 57m 9s ⏱️ + 15m 20s Results for commit 521549a. ± Comparison against base commit 728bc94. This pull request removes 207 and adds 1283 tests. Note that renamed tests count towards both. |
nalajcie
left a comment
There was a problem hiding this comment.
Some nitpicks + request to make the nandtool implementation more resilient to power cutoff
| #define DBBT_START 0x100 | ||
| #define BCB_CNT 4 | ||
| #define BCB_FCB_START 0x0 | ||
| #define BCB_DBBT_START 0x100 |
There was a problem hiding this comment.
this is BCB_CNT * info->erasesz? (worth to note it down if we would use NAND with different geometry in the future). Or compute it dynamically from erasesz always
| #define BCB_BB_MAX (BCB_DBBT_SIZE * 1024 - 2) | ||
| #define BCB_FW1_PART 2 | ||
| #define BCB_FW2_PART 3 | ||
| #define BCB_FW1_DEV "/dev/mtd0p2" |
There was a problem hiding this comment.
uhh, this should not be hard-coded into generic tool (depends on ptable)
|
|
||
| printf("Total blocks checked: %u\n", size / flashmng_common.info.erasesz); | ||
| printf("Number of bad blocks: %u\n", bbtn); | ||
| printf("Total blocks checked: %u\n", addr / flashmng_common.info.erasesz); |
There was a problem hiding this comment.
size, now it's invalid if start > 0?
|
|
||
|
|
||
| int flashmng_checkRange(oid_t oid, unsigned int start, unsigned int size, dbbt_t **dbbt) | ||
| int flashmng_checkRange(oid_t oid, unsigned int start, unsigned int size, unsigned int offs, dbbt_t *dbbt) |
There was a problem hiding this comment.
what's the difference between start and offs? (write a comment, maybe rename to partOffs or something along the lines). If start is not used (always 0 passed) than maybe reduce the parameters.
| } | ||
|
|
||
| /* DBBT includes only bad blocks from firmware partitions */ | ||
| printf("PSD: Check FW1 bad blocks - start: %u end: %llu.\n", 0, psd_common.partsz / psd_common.flash.erasesz); |
There was a problem hiding this comment.
should not be hard-coded, maybe add special command to SDP language to "check current partition and add to the DBBT"?
| printf("WARN: %u out of %u DBBT are broken\n", dbbt_failed, BCB_CNT); | ||
| } | ||
|
|
||
| return (dbbt_failed == BCB_CNT); |
| /* DBBT includes only bad blocks from firmware partitions */ | ||
| err = nandtool_scan(BCB_FW1_DEV, dbbt); | ||
| if (err < 0) { | ||
| perror("nandtool: failed to flash DBBT"); |
| return err; | ||
| } | ||
|
|
||
| err = dbbt_flash(nandtool_common.oid, nandtool_common.fd, dbbt, nandtool_common.info); |
There was a problem hiding this comment.
we should update (erase + write) DBBT ONLY if it differs
| } | ||
|
|
||
| /* DBBT includes only bad blocks from firmware partitions */ | ||
| err = nandtool_scan(BCB_FW1_DEV, dbbt); |
There was a problem hiding this comment.
would be great to not have the partitions hard-coded, maybe make device files a parameter to -t?
| printf("\t-r - flash raw data\n"); | ||
| printf("\t-s <block> - start flashing from given block (requires -i)\n"); | ||
| #ifdef HAS_BCB | ||
| printf("\t-f - write FCB\n"); |
Description
Motivation and Context
Types of changes
How Has This Been Tested?
Checklist:
Special treatment