coreboot-gerrit September 2020

coreboot-gerrit@coreboot.org

1 participants
2629 discussions

Change in coreboot[master]: spi/winbond: Pull out winbond_set_bpbits function
by Daniel Gröber (dxld) (Code Review) 28 Feb '23

28 Feb '23

Hello Daniel Gröber, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/42121 to review the following change. Change subject: spi/winbond: Pull out winbond_set_bpbits function ...................................................................... spi/winbond: Pull out winbond_set_bpbits function Split bpbits write code from winbond_set_write_protection into a new function: winbond_set_bpbits. Change-Id: If3ed24cd5bf31272bad5c3a404503cc0b3889e98 Signed-off-by: Daniel Gröber <dxld(a)darkboxed.org> --- M src/drivers/spi/winbond.c 1 file changed, 68 insertions(+), 54 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/42121/1 diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c index afffd4d..b834585 100644 --- a/src/drivers/spi/winbond.c +++ b/src/drivers/spi/winbond.c @@ -210,6 +210,9 @@ static int winbond_get_bpbits(const struct spi_flash *flash, struct spi_flash_bpbits *bpbits); +static int winbond_set_bpbits(const struct spi_flash *flash, + const struct spi_flash_part_id *params, + const struct spi_flash_bpbits *bpbits); /* * Convert BPx, TB and CMP to a region. @@ -432,84 +435,48 @@ return ret; } -/* - * Available on all devices. - * Protect a region starting from start of flash or end of flash. - * The caller must provide a supported protected region size. - * SEC isn't supported and set to zero. - * Write block protect bits to Status/Status2 Reg. - * Optionally lock the status register if lock_sreg is set with the provided - * mode. - * - * @param flash: The flash to operate on - * @param region: The region to write protect - * @param mode: Optional status register lock-down mode - * - * @return 0 on success - */ -static int -winbond_set_write_protection(const struct spi_flash *flash, - const struct region *region, - const enum spi_flash_status_reg_lockdown mode) +static int winbond_set_bpbits(const struct spi_flash *flash, + const struct spi_flash_part_id *params, + const struct spi_flash_bpbits *bpbits) { - const struct spi_flash_part_id *params; - struct status_regs mask, val; - struct spi_flash_bpbits bpbits; + struct status_regs msk, val; int ret; - params = flash->part; - - if (!params) - return -1; - - ret = winbond_get_bpbits(flash, &bpbits); - if (!ret) - return ret; - - ret = winbond_region_to_bpbits(flash, params, region, mode, &bpbits); - if (!ret) - return ret; - - /* Write block protection bits */ - if (params->bp_bits == 3) { val.reg1 = (union status_reg1) { - .bp3 = { .bp = bpbits.bp, .tb = bpbits.tb, .sec = 0 } + .bp3 = { .bp = bpbits->bp, .tb = bpbits->tb, .sec = 0 } }; - mask.reg1 = (union status_reg1) { + msk.reg1 = (union status_reg1) { .bp3 = { .bp = ~0, .tb = 1, .sec = 1 } }; } else { val.reg1 = (union status_reg1) { - .bp4 = { .bp = bpbits.bp, .tb = bpbits.tb } + .bp4 = { .bp = bpbits->bp, .tb = bpbits->tb } }; - mask.reg1 = (union status_reg1) { + msk.reg1 = (union status_reg1) { .bp4 = { .bp = ~0, .tb = 1 } }; } - val.reg2 = (union status_reg2) { .cmp = bpbits.cmp }; - mask.reg2 = (union status_reg2) { .cmp = 1 }; + val.reg2 = (union status_reg2) { .cmp = bpbits->cmp }; + msk.reg2 = (union status_reg2) { .cmp = 1 }; if (params->bp_bits == 3) { - val.reg1.bp3.srp0 = bpbits.winbond.srp0; - mask.reg1.bp3.srp0 = 1; + val.reg1.bp3.srp0 = bpbits->winbond.srp0; + msk.reg1.bp3.srp0 = 1; } else { - val.reg1.bp4.srp = bpbits.winbond.srp0; - mask.reg1.bp4.srp = 1; + val.reg1.bp4.srp = bpbits->winbond.srp; + msk.reg1.bp4.srp = 1; } - val.reg2.srp1 = bpbits.winbond.srp1; - mask.reg2.srp1 = 1; + val.reg2.srp1 = bpbits->winbond.srp1; + msk.reg2.srp1 = 1; - ret = winbond_flash_cmd_status(flash, mask.u, val.u, true); + ret = winbond_flash_cmd_status(flash, msk.u, val.u, true); if (ret) return ret; - printk(BIOS_DEBUG, "WINBOND: write-protection set to range " - "0x%08zx-0x%08zx\n", region_offset(region), region_end(region)); - - return ret; + return 0; } int winbond_region_to_bpbits( @@ -590,6 +557,53 @@ return 0; } +/* + * Available on all devices. + * Protect a region starting from start of flash or end of flash. + * The caller must provide a supported protected region size. + * SEC isn't supported and set to zero. + * Write block protect bits to Status/Status2 Reg. + * Optionally lock the status register if lock_sreg is set with the provided + * mode. + * + * @param flash: The flash to operate on + * @param region: The region to write protect + * @param mode: Optional status register lock-down mode + * + * @return 0 on success + */ +static int +winbond_set_write_protection(const struct spi_flash *flash, + const struct region *region, + const enum spi_flash_status_reg_lockdown mode) +{ + const struct spi_flash_part_id *params; + struct spi_flash_bpbits bpbits; + int ret; + + params = flash->part; + if (!params) + return -1; + + ret = winbond_get_bpbits(flash, &bpbits); + if (!ret) + return ret; + + ret = winbond_region_to_bpbits(flash, params, region, mode, &bpbits); + if (!ret) + return ret; + + + ret = winbond_set_bpbits(flash, params, &bpbits); + if (!ret) + return ret; + + printk(BIOS_DEBUG, "WINBOND: write-protection set to range " + "0x%08zx-0x%08zx\n", region_offset(region), region_end(region)); + + return ret; +} + static const struct spi_flash_protection_ops spi_flash_protection_ops = { .get_write = winbond_get_write_protection, .set_write = winbond_set_write_protection, -- To view, visit https://review.coreboot.org/c/coreboot/+/42121 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If3ed24cd5bf31272bad5c3a404503cc0b3889e98 Gerrit-Change-Number: 42121 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Gröber (dxld) Gerrit-Reviewer: Daniel Gröber <dxld(a)darkboxed.org> Gerrit-MessageType: newchange

3 3

Change in coreboot[master]: spi/winbond: Add 'params' as get_bpbits arg
by Daniel Gröber (dxld) (Code Review) 28 Feb '23

28 Feb '23

Hello Daniel Gröber, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/42122 to review the following change. Change subject: spi/winbond: Add 'params' as get_bpbits arg ...................................................................... spi/winbond: Add 'params' as get_bpbits arg This lets us avoid repeted checks for flash->part being NULL. Change-Id: I385fd0a73733ce23b3cc22557037f4a9aa3b331d Signed-off-by: Daniel Gröber <dxld(a)darkboxed.org> --- M src/drivers/spi/winbond.c 1 file changed, 4 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/42122/1 diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c index b834585..d127287 100644 --- a/src/drivers/spi/winbond.c +++ b/src/drivers/spi/winbond.c @@ -209,6 +209,7 @@ }; static int winbond_get_bpbits(const struct spi_flash *flash, + const struct spi_flash_part_id *params, struct spi_flash_bpbits *bpbits); static int winbond_set_bpbits(const struct spi_flash *flash, const struct spi_flash_part_id *params, @@ -267,7 +268,7 @@ if (!params) return -1; - ret = winbond_get_bpbits(flash, &bpbits); + ret = winbond_get_bpbits(flash, params, &bpbits); if (ret) return ret; @@ -286,16 +287,13 @@ } static int winbond_get_bpbits(const struct spi_flash *flash, + const struct spi_flash_part_id *params, struct spi_flash_bpbits *bpbits) { union status_reg1 reg1 = { .u = 0 }; union status_reg2 reg2 = { .u = 0 }; int ret; - const struct spi_flash_part_id *params = flash->part; - if (!params) - return -1; - ret = spi_flash_cmd(&flash->spi, flash->status_cmd, &reg1.u, sizeof(reg1.u)); if (ret) @@ -585,7 +583,7 @@ if (!params) return -1; - ret = winbond_get_bpbits(flash, &bpbits); + ret = winbond_get_bpbits(flash, params, &bpbits); if (!ret) return ret; -- To view, visit https://review.coreboot.org/c/coreboot/+/42122 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I385fd0a73733ce23b3cc22557037f4a9aa3b331d Gerrit-Change-Number: 42122 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Gröber (dxld) Gerrit-Reviewer: Daniel Gröber <dxld(a)darkboxed.org> Gerrit-MessageType: newchange

3 3

Change in coreboot[master]: spi: Use common code for top-level flash block protection functions
by Daniel Gröber (dxld) (Code Review) 28 Feb '23

28 Feb '23

Hello Daniel Gröber, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/42123 to review the following change. Change subject: spi: Use common code for top-level flash block protection functions ...................................................................... spi: Use common code for top-level flash block protection functions This turns *_get_write_protection and *_set_write_protection into common code by having the vendor code implement getter/setter functions for bpbits and convertion functions between bpbits and a region. Change-Id: I509a1cedff5de4bee34b8856ce651d100a32fd13 Signed-off-by: Daniel Gröber <dxld(a)darkboxed.org> --- M src/drivers/spi/macronix.c M src/drivers/spi/spi_flash.c M src/drivers/spi/spi_winbond.h M src/drivers/spi/winbond.c M src/include/spi_flash.h 5 files changed, 156 insertions(+), 222 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/42123/1 diff --git a/src/drivers/spi/macronix.c b/src/drivers/spi/macronix.c index 40f4a47..e5a7b98 100644 --- a/src/drivers/spi/macronix.c +++ b/src/drivers/spi/macronix.c @@ -316,64 +316,6 @@ return 0; } - -/* - * Read block protect bits from Status Reg. - * Converts block protection bits to a region. - * - * Returns: - * -1 on error - * 1 if region is covered by write protection - * 0 if a part of region isn't covered by write protection - */ -static int macronix_get_write_protection(const struct spi_flash *flash, - const struct region *region) -{ - const struct spi_flash_part_id *params; - struct region wp_region; - int ret; - - params = flash->part; - - if (!params) - return -1; - - if (!params->protection_granularity_shift) - return -1; - - struct spi_flash_bpbits bpbits = {}; - ret = macronix_get_bpbits(flash, params, &bpbits); - if (!ret) - return ret; - - ret = macronix_bpbits_to_region(flash, params, &bpbits, &wp_region); - if (!ret) - return ret; - - if (!region_sz(&wp_region)) { - printk(BIOS_DEBUG, "MACRONIX: flash isn't protected\n"); - return 0; - } - - printk(BIOS_DEBUG, "MACRONIX: flash protected range 0x%08zx-0x%08zx\n", - region_offset(&wp_region), region_end(&wp_region)); - - return region_is_subregion(&wp_region, region); -} - -/* - * Protect a region starting from start of flash or end of flash. - * The caller must provide a supported protected region size. - * Writes block protect bits to Status Reg. - * Optionally lock the status register if lock_sreg is set with the provided - * mode. - * - * @param flash: The flash to operate on - * @param region: The region to write protect - * @param mode: Optional status register lock-down mode - * - * @return 0 on success - */ static int macronix_set_bpbits(const struct spi_flash *flash, const struct spi_flash_part_id *params, const struct spi_flash_bpbits *bits) @@ -400,52 +342,12 @@ return 0; } - -static int -macronix_set_write_protection(const struct spi_flash *flash, - const struct region *region, - const enum spi_flash_status_reg_lockdown mode) -{ - const struct spi_flash_part_id *params; - int ret; - - printk(BIOS_DEBUG, "MACRONIX: want to set protection for" - " 0x%08zx-0x%08zx\n", region_offset(region), region_end(region)); - - params = flash->part; - - if (!params) - return -1; - - if (!params->protection_granularity_shift) { - printk(BIOS_ERR, "MACRONIX: ERROR: spi_flash part doesn't support block protection\n"); - return -1; - } - - struct spi_flash_bpbits bpbits = {}; - ret = macronix_get_bpbits(flash, params, &bpbits); - if (!ret) - return ret; - - ret = macronix_region_to_bpbits(flash, params, region, mode, &bpbits); - if (!ret) { - printk(BIOS_ERR, "MACRONIX: ERROR: unsupported block protection" - " region requested: 0x%08zx-0x%08zx\n", - region_offset(region), region_end(region)); - return ret; - } - - ret = macronix_set_bpbits(flash, params, &bpbits); - if (!ret) - return ret; - - return ret; -} - - static const struct spi_flash_protection_ops spi_flash_protection_ops = { - .get_write = macronix_get_write_protection, - .set_write = macronix_set_write_protection, + .get_bpbits = macronix_get_bpbits, + .set_bpbits = macronix_set_bpbits, + + .bpbits_to_region = macronix_bpbits_to_region, + .region_to_bpbits = macronix_region_to_bpbits, }; const struct spi_flash_vendor_info spi_flash_macronix_vi = { diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c index 33416b5..e57076b 100644 --- a/src/drivers/spi/spi_flash.c +++ b/src/drivers/spi/spi_flash.c @@ -532,6 +532,50 @@ return -1; } +/* + * Read block protect bits from Status Reg and convert to a region. + * + * Returns: + * -1 on error + * 1 if region is covered by write protection + * 0 if a part of region isn't covered by write protection + */ +static int spi_flash_get_write_protection(const struct spi_flash *flash, + const struct region *region) +{ + const struct spi_flash_part_id *params; + struct spi_flash_bpbits bpbits = {}; + struct region wp_region; + int ret; + + params = flash->part; + if (!params) + return -1; + + if (!params->protection_granularity_shift) + return -1; + + ret = flash->prot_ops->get_bpbits(flash, params, &bpbits); + if (ret) + return ret; + + ret = flash->prot_ops->bpbits_to_region(flash, params, &bpbits, + &wp_region); + if (ret) + return ret; + + if (!region_sz(&wp_region)) { + printk(BIOS_DEBUG, "SPI: flash isn't write-protected\n"); + return 0; + } + + printk(BIOS_DEBUG, "SPI: flash write protected range:" + " 0x%08zx-0x%08zx\n", region_offset(&wp_region), + region_end(&wp_region)); + + return region_is_subregion(&wp_region, region); +} + int spi_flash_is_write_protected(const struct spi_flash *flash, const struct region *region) { @@ -551,7 +595,64 @@ return -1; } - return flash->prot_ops->get_write(flash, region); + return spi_flash_get_write_protection(flash, region); +} + +/* + * Protect a region starting from start of flash or end of flash. + * The caller must provide a supported protected region size. + * Writes block protect bits to Status Reg. + * Optionally lock the status register if appropriate mode is used. + * + * @param flash: The flash to operate on + * @param region: The region to write protect + * @param mode: Optional status register lock-down mode + * + * @return 0 on success + */ +static int spi_flash_set_write_protection( + const struct spi_flash *flash, + const struct region *region, + const enum spi_flash_status_reg_lockdown mode) +{ + const struct spi_flash_part_id *params; + int ret; + + printk(BIOS_DEBUG, "SPI: want to set protection for" + " 0x%08zx-0x%08zx\n", region_offset(region), region_end(region)); + + params = flash->part; + if (!params) + return -1; + + if (!params->protection_granularity_shift) { + printk(BIOS_ERR, "SPI: ERROR: flash part doesn't support block" + " protection\n"); + return -1; + } + + struct spi_flash_bpbits bpbits = {}; + ret = flash->prot_ops->get_bpbits(flash, params, &bpbits); + if (ret) + return ret; + + ret = flash->prot_ops->region_to_bpbits(flash, params, region, mode, + &bpbits); + if (ret) { + printk(BIOS_ERR, "SPI: ERROR: unsupported block protection" + " region requested: 0x%08zx-0x%08zx\n", + region_offset(region), region_end(region)); + return ret; + } + + ret = flash->prot_ops->set_bpbits(flash, params, &bpbits); + if (ret) + return ret; + + printk(BIOS_DEBUG, "SPI: write-protection range set to " + "0x%08zx-0x%08zx\n", region_offset(region), region_end(region)); + + return 0; } int spi_flash_set_write_protected(const struct spi_flash *flash, @@ -575,7 +676,7 @@ return -1; } - ret = flash->prot_ops->set_write(flash, region, mode); + ret = spi_flash_set_write_protection(flash, region, mode); if (ret == 0 && mode != SPI_WRITE_PROTECTION_PRESERVE) { printk(BIOS_INFO, "SPI: SREG lock-down was set to "); diff --git a/src/drivers/spi/spi_winbond.h b/src/drivers/spi/spi_winbond.h index 2a3a4ae..b4f455a8 100644 --- a/src/drivers/spi/spi_winbond.h +++ b/src/drivers/spi/spi_winbond.h @@ -29,7 +29,7 @@ const enum spi_flash_status_reg_lockdown mode, struct spi_flash_bpbits *bits); -void winbond_bpbits_to_region( +int winbond_bpbits_to_region( const struct spi_flash *flash, const struct spi_flash_part_id *params, const struct spi_flash_bpbits *bits, diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c index d127287..9605279 100644 --- a/src/drivers/spi/winbond.c +++ b/src/drivers/spi/winbond.c @@ -219,10 +219,10 @@ * Convert BPx, TB and CMP to a region. * SEC (if available) must be zero. */ -void winbond_bpbits_to_region(const struct spi_flash *flash, - const struct spi_flash_part_id *params, - const struct spi_flash_bpbits *bits, - struct region *out) +int winbond_bpbits_to_region(const struct spi_flash *flash, + const struct spi_flash_part_id *params, + const struct spi_flash_bpbits *bits, + struct region *out) { const size_t granularity = 1 << params->protection_granularity_shift; size_t protected_size = @@ -244,46 +244,8 @@ out->offset = tb ? 0 : flash->size - protected_size; out->size = protected_size; } -} -/* - * Available on all devices. - * Read block protect bits from Status/Status2 Reg. - * Converts block protection bits to a region. - * - * Returns: - * -1 on error - * 1 if region is covered by write protection - * 0 if a part of region isn't covered by write protection - */ -static int winbond_get_write_protection(const struct spi_flash *flash, - const struct region *region) -{ - const struct spi_flash_part_id *params; - struct region wp_region; - struct spi_flash_bpbits bpbits; - int ret; - - params = flash->part; - if (!params) - return -1; - - ret = winbond_get_bpbits(flash, params, &bpbits); - if (ret) - return ret; - - winbond_bpbits_to_region(flash, params, &bpbits, &wp_region); - - if (!region_sz(&wp_region)) { - printk(BIOS_DEBUG, "WINBOND: flash isn't protected\n"); - - return 0; - } - - printk(BIOS_DEBUG, "WINBOND: flash protected range 0x%08zx-0x%08zx\n", - region_offset(&wp_region), region_end(&wp_region)); - - return region_is_subregion(&wp_region, region); + return 0; } static int winbond_get_bpbits(const struct spi_flash *flash, @@ -555,56 +517,12 @@ return 0; } -/* - * Available on all devices. - * Protect a region starting from start of flash or end of flash. - * The caller must provide a supported protected region size. - * SEC isn't supported and set to zero. - * Write block protect bits to Status/Status2 Reg. - * Optionally lock the status register if lock_sreg is set with the provided - * mode. - * - * @param flash: The flash to operate on - * @param region: The region to write protect - * @param mode: Optional status register lock-down mode - * - * @return 0 on success - */ -static int -winbond_set_write_protection(const struct spi_flash *flash, - const struct region *region, - const enum spi_flash_status_reg_lockdown mode) -{ - const struct spi_flash_part_id *params; - struct spi_flash_bpbits bpbits; - int ret; - - params = flash->part; - if (!params) - return -1; - - ret = winbond_get_bpbits(flash, params, &bpbits); - if (!ret) - return ret; - - ret = winbond_region_to_bpbits(flash, params, region, mode, &bpbits); - if (!ret) - return ret; - - - ret = winbond_set_bpbits(flash, params, &bpbits); - if (!ret) - return ret; - - printk(BIOS_DEBUG, "WINBOND: write-protection set to range " - "0x%08zx-0x%08zx\n", region_offset(region), region_end(region)); - - return ret; -} - static const struct spi_flash_protection_ops spi_flash_protection_ops = { - .get_write = winbond_get_write_protection, - .set_write = winbond_set_write_protection, + .get_bpbits = winbond_get_bpbits, + .set_bpbits = winbond_set_bpbits, + + .bpbits_to_region = winbond_bpbits_to_region, + .region_to_bpbits = winbond_region_to_bpbits, }; const struct spi_flash_vendor_info spi_flash_winbond_vi = { diff --git a/src/include/spi_flash.h b/src/include/spi_flash.h index fe62069..bf84ce0 100644 --- a/src/include/spi_flash.h +++ b/src/include/spi_flash.h @@ -85,37 +85,50 @@ }; }; +struct spi_flash_part_id; + /* Current code assumes all callbacks are supplied in this object. */ struct spi_flash_protection_ops { - /* - * Returns 1 if the whole region is software write protected. - * Hardware write protection mechanism aren't accounted. - * If the write protection could be changed, due to unlocked status - * register for example, 0 should be returned. - * Returns 0 on success. - */ - int (*get_write)(const struct spi_flash *flash, - const struct region *region); - /* - * Enable the status register write protection, if supported on the - * requested region, and optionally enable status register lock-down. - * Returns 0 if the whole region was software write protected. - * Hardware write protection mechanism aren't accounted. - * If the status register is locked and the requested configuration - * doesn't match the selected one, return an error. - * Only a single region is supported ! + + /** + * Read current block protection bits from device status registers. * - * @return 0 on success + * Returns non-zero on error, zero on success. */ - int - (*set_write)(const struct spi_flash *flash, + int (*get_bpbits)(const struct spi_flash *flash, + const struct spi_flash_part_id *part, + struct spi_flash_bpbits *bpbits); + /** + * Write block protection bits to non-volatile device status registers. + * + * Returns non-zero on error, zero on success. + */ + int (*set_bpbits)(const struct spi_flash *flash, + const struct spi_flash_part_id *part, + const struct spi_flash_bpbits *bpbits); + + /** + * Convert block protection bits to write-protected address region. + * + * Returns non-zero on error, zero on success. + */ + int (*bpbits_to_region)(const struct spi_flash *flash, + const struct spi_flash_part_id *part, + const struct spi_flash_bpbits *bpbits, + struct region *region); + /** + * Convert address region to be write-protected into block + * protection bits. + * + * Returns non-zero on error, zero on success. + */ + int (*region_to_bpbits)(const struct spi_flash *flash, + const struct spi_flash_part_id *part, const struct region *region, - const enum spi_flash_status_reg_lockdown mode); - + const enum spi_flash_status_reg_lockdown mode, + struct spi_flash_bpbits *bpbits); }; -struct spi_flash_part_id; - struct spi_flash { struct spi_slave spi; u8 vendor; -- To view, visit https://review.coreboot.org/c/coreboot/+/42123 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I509a1cedff5de4bee34b8856ce651d100a32fd13 Gerrit-Change-Number: 42123 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Gröber (dxld) Gerrit-Reviewer: Daniel Gröber <dxld(a)darkboxed.org> Gerrit-MessageType: newchange

2 2

Change in coreboot[master]: spi/winbond: Pass flash and part info to bpbits_to_region
by Daniel Gröber (dxld) (Code Review) 28 Feb '23

28 Feb '23

Hello Daniel Gröber, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/42119 to review the following change. Change subject: spi/winbond: Pass flash and part info to bpbits_to_region ...................................................................... spi/winbond: Pass flash and part info to bpbits_to_region This makes bpbits_to_region mirror region_to_bpbits more closely. Change-Id: Ie60e5feaab29f4db80db6ea688453b042397257e Signed-off-by: Daniel Gröber <dxld(a)darkboxed.org> --- M src/drivers/spi/spi_winbond.h M src/drivers/spi/winbond.c M tests/drivers/winbond-spi-test.c 3 files changed, 11 insertions(+), 16 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/42119/1 diff --git a/src/drivers/spi/spi_winbond.h b/src/drivers/spi/spi_winbond.h index ff799ff..2a3a4ae 100644 --- a/src/drivers/spi/spi_winbond.h +++ b/src/drivers/spi/spi_winbond.h @@ -30,7 +30,7 @@ struct spi_flash_bpbits *bits); void winbond_bpbits_to_region( - const size_t granularity, + const struct spi_flash *flash, + const struct spi_flash_part_id *params, const struct spi_flash_bpbits *bits, - const size_t flash_size, struct region *out); diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c index 49d3c5b..7d44bfe 100644 --- a/src/drivers/spi/winbond.c +++ b/src/drivers/spi/winbond.c @@ -215,21 +215,22 @@ * Convert BPx, TB and CMP to a region. * SEC (if available) must be zero. */ -void winbond_bpbits_to_region(const size_t granularity, +void winbond_bpbits_to_region(const struct spi_flash *flash, + const struct spi_flash_part_id *params, const struct spi_flash_bpbits *bits, - const size_t flash_size, struct region *out) { + const size_t granularity = 1 << params->protection_granularity_shift; size_t protected_size = - MIN(bits->bp ? granularity << (bits->bp - 1) : 0, flash_size); + MIN(bits->bp ? granularity << (bits->bp - 1) : 0, flash->size); int tb = bits->tb; if (bits->cmp) { - protected_size = flash_size - protected_size; + protected_size = flash->size - protected_size; tb = !tb; } - out->offset = tb ? 0 : flash_size - protected_size; + out->offset = tb ? 0 : flash->size - protected_size; out->size = protected_size; } @@ -252,18 +253,14 @@ int ret; params = flash->part; - if (!params) return -1; - const size_t granularity = (1 << params->protection_granularity_shift); - ret = winbond_get_bpbits(flash, &bpbits); if (ret) return ret; - winbond_bpbits_to_region(granularity, &bpbits, flash->size, - &wp_region); + winbond_bpbits_to_region(flash, params, &bpbits, &wp_region); if (!region_sz(&wp_region)) { printk(BIOS_DEBUG, "WINBOND: flash isn't protected\n"); diff --git a/tests/drivers/winbond-spi-test.c b/tests/drivers/winbond-spi-test.c index da7177b..98c715d 100644 --- a/tests/drivers/winbond-spi-test.c +++ b/tests/drivers/winbond-spi-test.c @@ -161,8 +161,7 @@ struct region expected, out = {-1, -1}; struct region roundtripped = {-1, -1}; - winbond_bpbits_to_region(granularity, bpbits, - flash.size, &out); + winbond_bpbits_to_region(&flash, part, bpbits, &out); enum spi_flash_status_reg_lockdown mode = SPI_WRITE_PROTECTION_PIN; @@ -170,8 +169,7 @@ winbond_region_to_bpbits(&flash, part, &out, mode, &bpbits1); - winbond_bpbits_to_region(granularity, &bpbits1, - flash.size, &roundtripped); + winbond_bpbits_to_region(&flash, part, &bpbits1, &roundtripped); const struct block_region *wp_blocks = &table->wp_blocks[bpbits->cmp][bpbits->tb][bpbits->bp]; -- To view, visit https://review.coreboot.org/c/coreboot/+/42119 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie60e5feaab29f4db80db6ea688453b042397257e Gerrit-Change-Number: 42119 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Gröber (dxld) Gerrit-Reviewer: Daniel Gröber <dxld(a)darkboxed.org> Gerrit-MessageType: newchange

3 3

Change in coreboot[master]: spi/winbond: Fix bpbits_to_region edge cases
by Daniel Gröber (dxld) (Code Review) 28 Feb '23

28 Feb '23

Hello Daniel Gröber, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/42120 to review the following change. Change subject: spi/winbond: Fix bpbits_to_region edge cases ...................................................................... spi/winbond: Fix bpbits_to_region edge cases Currently bpbits_to_region returns the wrong protection range for min/max bp values. See winbond-spi-test for the list. Here we bring everything in line with the datasheets. Change-Id: Ia50dce81937402f1022cf2269e33461d3f605dbf Signed-off-by: Daniel Gröber <dxld(a)darkboxed.org> --- M src/drivers/spi/winbond.c M tests/drivers/winbond-spi-test.c 2 files changed, 26 insertions(+), 24 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/42120/1 diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c index 7d44bfe..afffd4d 100644 --- a/src/drivers/spi/winbond.c +++ b/src/drivers/spi/winbond.c @@ -230,8 +230,16 @@ tb = !tb; } - out->offset = tb ? 0 : flash->size - protected_size; - out->size = protected_size; + if (protected_size == 0) { + out->offset = 0; + out->size = 0; + } else if (protected_size == flash->size) { + out->offset = 0; + out->size = flash->size; + } else { + out->offset = tb ? 0 : flash->size - protected_size; + out->size = protected_size; + } } /* diff --git a/tests/drivers/winbond-spi-test.c b/tests/drivers/winbond-spi-test.c index 98c715d..2ed0e90 100644 --- a/tests/drivers/winbond-spi-test.c +++ b/tests/drivers/winbond-spi-test.c @@ -41,12 +41,12 @@ [0][0][0] = { 0, 0 }, [0][1][0] = { 0, 0 }, - /* cmp=1, tb=x, bp=0b0000: FIXME: ALL */ - [1][0][0] = { 0, 0 }, - [1][1][0] = { 0, 0 }, + /* cmp=1, tb=x, bp=0b0000: ALL */ + [1][0][0] = { 0, 512 }, + [1][1][0] = { 0, 512 }, - /* cmp=0, tb=x, bp=0b110x|0b1x1x: FIXME: ALL */ - [0][0 ... 1][10 ... 15] = { 0, 0 }, + /* cmp=0, tb=x, bp=0b110x|0b1x1x: ALL */ + [0][0 ... 1][10 ... 15] = { 0, 512 }, /* cmp=1, tb=x, bp=0b110x|0b1x1x: NONE */ [1][0 ... 1][10 ... 15] = { 0, 0 }, @@ -105,9 +105,9 @@ [0][0][0] = { 0, 0 }, [0][1][0] = { 0, 0 }, - /* cmp=0, tb=x, bp=0b111: FIXME: ALL */ - [0][0][7] = { 0, 0 }, - [0][1][7] = { 0, 0 }, + /* cmp=0, tb=x, bp=0b111: ALL */ + [0][0][7] = { 0, 256 }, + [0][1][7] = { 0, 256 }, [0][0][1] = { 252, 4 }, [0][0][2] = { 248, 8 }, @@ -123,9 +123,9 @@ [0][1][5] = { 0, 64 }, [0][1][6] = { 0, 128 }, - /* cmp=1, tb=x, bp=0b000: FIXME: ALL */ - [1][0][0] = { 0, 0 }, - [1][1][0] = { 0, 0 }, + /* cmp=1, tb=x, bp=0b000: ALL */ + [1][0][0] = { 0, 256 }, + [1][1][0] = { 0, 256 }, /* cmp=1, tb=x, bp=0b111: NONE */ [1][0][7] = { 0, 0 }, @@ -234,23 +234,16 @@ assert_non_null(table); - /* FIXME: Max values below are clamped to 6 and 9 - * respectively because the implementation currently thinks - * everything above those is fully protected when the - * datasheets say the protection range is empty in those - * cases. */ u8 bp_max; if (part->bp_bits == 3) - bp_max = 6; + bp_max = 7; else if (part->bp_bits == 4) - bp_max = 9; + bp_max = 0xf; else fail_msg("Unrecognized bp_bits"); for (u8 tbcmp = 0; tbcmp < 4; tbcmp++) { - /* FIXME: bp=0 isn't tested because the - * implementation is currently wrong */ - for (u8 bp = 1; bp <= bp_max; bp++) { + for (u8 bp = 0; bp <= bp_max; bp++) { struct spi_flash_bpbits bpbits = { .cmp = tbcmp & (1<<1), .tb = tbcmp & (1<<0), @@ -265,7 +258,8 @@ } } - assert_true(n_cfgs_tested == 60); + /* print_message("n_cfgs_tested: %d\n", n_cfgs_tested); */ + assert_true(n_cfgs_tested == 96); } /* Mocks */ -- To view, visit https://review.coreboot.org/c/coreboot/+/42120 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia50dce81937402f1022cf2269e33461d3f605dbf Gerrit-Change-Number: 42120 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Gröber (dxld) Gerrit-Reviewer: Daniel Gröber <dxld(a)darkboxed.org> Gerrit-MessageType: newchange

3 3

Change in coreboot[master]: spi: Share common logic of vendor specific bpbits<>region functions
by Daniel Gröber (dxld) (Code Review) 28 Feb '23

28 Feb '23

Hello Daniel Gröber, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/42124 to review the following change. Change subject: spi: Share common logic of vendor specific bpbits<>region functions ...................................................................... spi: Share common logic of vendor specific bpbits<>region functions Change-Id: Iadedf160575b20ee7fc671642412e6b3d437a20e Signed-off-by: Daniel Gröber <dxld(a)darkboxed.org> --- M src/drivers/spi/Makefile.inc M src/drivers/spi/macronix.c A src/drivers/spi/spi_common.c A src/drivers/spi/spi_common.h M src/drivers/spi/spi_flash_internal.h M src/drivers/spi/winbond.c M tests/drivers/Makefile.inc D tests/drivers/macronix-spi-test.c A tests/drivers/spi-test.c D tests/drivers/winbond-spi-test.c 10 files changed, 608 insertions(+), 635 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/42124/1 diff --git a/src/drivers/spi/Makefile.inc b/src/drivers/spi/Makefile.inc index 6dbc43a..e94d14f 100644 --- a/src/drivers/spi/Makefile.inc +++ b/src/drivers/spi/Makefile.inc @@ -14,7 +14,7 @@ $(1)-y += spi-generic.c $(1)-y += bitbang.c $(1)-$(CONFIG_COMMON_CBFS_SPI_WRAPPER) += cbfs_spi.c -$(1)-$(CONFIG_SPI_FLASH) += spi_flash.c +$(1)-$(CONFIG_SPI_FLASH) += spi_flash.c spi_common.c $(1)-$(CONFIG_SPI_SDCARD) += spi_sdcard.c $(1)-$(CONFIG_BOOT_DEVICE_SPI_FLASH_RW_NOMMAP$(2)) += boot_device_rw_nommap.c $(1)-$(CONFIG_CONSOLE_SPI_FLASH) += flashconsole.c diff --git a/src/drivers/spi/macronix.c b/src/drivers/spi/macronix.c index e5a7b98..b19f085 100644 --- a/src/drivers/spi/macronix.c +++ b/src/drivers/spi/macronix.c @@ -7,6 +7,7 @@ #include <lib.h> #include "spi_flash_internal.h" +#include "spi_common.h" #include "spi_macronix.h" /* MX25xx-specific commands */ @@ -47,6 +48,7 @@ .id[0] = 0x2015, .nr_sectors_shift = 9, .protection_granularity_shift = 16, + .bp_bits = 4, .bp_lower_th = 10, }, { @@ -54,6 +56,7 @@ .id[0] = 0x2016, .nr_sectors_shift = 10, .protection_granularity_shift = 16, + .bp_bits = 4, .bp_lower_th = 9, }, { @@ -61,6 +64,7 @@ .id[0] = 0x2017, .nr_sectors_shift = 11, .protection_granularity_shift = 17, + .bp_bits = 4, .bp_lower_th = 9, }, { @@ -167,36 +171,7 @@ const struct spi_flash_bpbits *bits, struct region *out) { - if (bits->bp == 0xf) { - out->offset = 0; - out->size = flash->size; - } else if (bits->bp == 0) { - out->offset = 0; - out->size = 0; - } else { - const u16 granularity_shift = - part->protection_granularity_shift; - - if (bits->bp >= part->bp_lower_th) { - /* Lower half is protected */ - const size_t unprotected_size = - MIN(1<<granularity_shift<<(14 - bits->bp), - flash->size); - - out->offset = 0; - out->size = flash->size - unprotected_size; - } else { - /* Upper half is protected */ - const size_t protected_size = - MIN(1<<(granularity_shift + bits->bp - 1), - flash->size); - - out->offset = flash->size - protected_size; - out->size = protected_size; - } - } - - return 0; + return spi_flash_common_bpbits_to_region(flash, part, bits, out); } /** @@ -212,41 +187,11 @@ const enum spi_flash_status_reg_lockdown mode, struct spi_flash_bpbits *bits) { - if (region->offset == 0 && region->size >= flash->size) { - bits->bp = 0xf; - } else if (region->offset == 0 && region->size == 0) { - bits->bp = 0; - } else if (region->offset >= flash->size) { - return -1; - } else if (region->size >= flash->size) { - return -1; - } else { - const size_t gran_shift = params->protection_granularity_shift; - const u16 granularity = 1<<gran_shift; + int ret; - if (region->offset == 0) { /* Lower half should be protected */ - const size_t unprotected_size = - flash->size - region->size; - - const int is_valid = - IS_POWER_OF_2(unprotected_size) && - unprotected_size >= granularity; - if (!is_valid) - return -1; - - bits->bp = 14 - log2(unprotected_size>>gran_shift); - } else { - const size_t protected_size = region->size; - - const int is_valid = - IS_POWER_OF_2(protected_size) && - protected_size >= granularity; - if (!is_valid) - return -1; - - bits->bp = log2(protected_size>>gran_shift) + 1; - } - } + ret = spi_flash_common_region_to_bpbits(flash, params, region, bits); + if (ret) + return ret; switch (mode) { case SPI_WRITE_PROTECTION_PRESERVE: diff --git a/src/drivers/spi/spi_common.c b/src/drivers/spi/spi_common.c new file mode 100644 index 0000000..88cc23a --- /dev/null +++ b/src/drivers/spi/spi_common.c @@ -0,0 +1,133 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <assert.h> +#include <spi_flash.h> +#include <lib.h> + +#include "spi_flash_internal.h" +#include "spi_common.h" + +int spi_flash_common_region_to_bpbits( + const struct spi_flash *flash, + const struct spi_flash_part_id *part, + const struct region *region, + struct spi_flash_bpbits *bits) +{ + /* Need to touch TOP or BOTTOM */ + if (region_offset(region) != 0 && region_end(region) != flash->size) + return -1; + + if (region_offset(region) == 0 && part->has_tb) + bits->tb = 1; + else + bits->tb = 0; + + struct region wp_region = *region; + if (region_sz(region) > flash->size / 2 && part->has_cmp) { + assert(part->has_tb); + /* When we want to protect more than half the flash, but + * not the whole thing we can use the cmp bit to make the + * normally protected region specified by bp below to be + * what is unprotected instead */ + bits->cmp = 1; + wp_region.offset = bits->tb ? 0 : region_sz(region); + wp_region.size = flash->size - region_sz(region); + bits->tb = !bits->tb; + } else { + bits->cmp = 0; + } + + if (region_sz(&wp_region) == 0) { + bits->bp = 0; + return 0; + } else if (region_offset(&wp_region) == 0 && + region_sz(&wp_region) == flash->size) { + assert(part->bp_bits > 0); + bits->bp = (1<<part->bp_bits) - 1; + return 0; + } + + const size_t gran_shift = part->protection_granularity_shift; + const size_t granularity = 1 << gran_shift; + + if (region_sz(&wp_region) < granularity) + return -1; + + if (part->bp_lower_th && region_offset(region) == 0) { + const size_t complement_size = flash->size - region->size; + + const int is_valid = IS_POWER_OF_2(complement_size) && + complement_size >= granularity; + if (!is_valid) + return -1; + + bits->bp = 14 - log2(complement_size>>gran_shift); + } else { + const size_t size = region_sz(&wp_region); + + const int is_valid = IS_POWER_OF_2(size) && + size >= granularity; + if (!is_valid) + return -1; + + bits->bp = log2(size) - gran_shift + 1; + } + + return 0; +} + +int spi_flash_common_bpbits_to_region( + const struct spi_flash *flash, + const struct spi_flash_part_id *part, + const struct spi_flash_bpbits *bits, + struct region *out) +{ + int tb = 0; + if (part->has_tb) { + assert(!part->bp_lower_th); + tb = bits->tb; + } else if (part->bp_lower_th && bits->bp >= part->bp_lower_th) { + tb = 1; + } + + const size_t granularity = + 1 << part->protection_granularity_shift; + + size_t protected_size; + if (part->bp_lower_th && bits->bp >= part->bp_lower_th) { + size_t unprotected_size = + MIN(granularity<<(14 - bits->bp), flash->size); + protected_size = flash->size - unprotected_size; + } else{ + protected_size = + MIN(bits->bp ? granularity << (bits->bp - 1) : 0, + flash->size); + } + + if (part->has_cmp && bits->cmp) { + protected_size = flash->size - protected_size; + tb = !tb; + } + + if (protected_size == 0) { + *out = (struct region){ + .offset = 0, + .size = 0, + }; + } else if (protected_size == flash->size) { + *out = (struct region){ + .offset = 0, + .size = flash->size, + }; + } else { + if (tb) { /* tb=1: BOTTOM */ + out->offset = 0; + } else { /* tb=0: TOP */ + out->offset = flash->size - protected_size; + } + + out->size = protected_size; + } + + return 0; +} diff --git a/src/drivers/spi/spi_common.h b/src/drivers/spi/spi_common.h new file mode 100644 index 0000000..a6aab46 --- /dev/null +++ b/src/drivers/spi/spi_common.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +int spi_flash_common_region_to_bpbits( + const struct spi_flash *flash, + const struct spi_flash_part_id *part, + const struct region *region, + struct spi_flash_bpbits *bits); + +int spi_flash_common_bpbits_to_region( + const struct spi_flash *flash, + const struct spi_flash_part_id *part, + const struct spi_flash_bpbits *bits, + struct region *out); diff --git a/src/drivers/spi/spi_flash_internal.h b/src/drivers/spi/spi_flash_internal.h index 98837c2..8c8cff9 100644 --- a/src/drivers/spi/spi_flash_internal.h +++ b/src/drivers/spi/spi_flash_internal.h @@ -80,9 +80,12 @@ uint16_t _reserved_for_flags: 3; /* Block protection. */ uint16_t protection_granularity_shift : 5; - uint16_t bp_bits : 3; /* Used by Winbond */ + uint16_t bp_bits : 3; /* bp values starting here protect the lower half */ uint16_t bp_lower_th : 5; /* Used by Macronix */ + uint16_t has_cmp : 1; /* region complement available, currently the + * code assumes has_cmp implies has_tb */ + uint16_t has_tb : 1; /* top/bottom select available */ }; struct spi_flash_ops_descriptor { diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c index 9605279..8b5b29e 100644 --- a/src/drivers/spi/winbond.c +++ b/src/drivers/spi/winbond.c @@ -8,6 +8,7 @@ #include <lib.h> #include "spi_flash_internal.h" +#include "spi_common.h" #include "spi_winbond.h" union status_reg1 { @@ -101,6 +102,7 @@ .id[0] = 0x4014, .nr_sectors_shift = 8, .fast_read_dual_output_support = 1, + .has_tb = 1, }, { /* W25Q16_V */ @@ -109,6 +111,7 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 16, .bp_bits = 3, + .has_tb = 1, }, { /* W25Q16DW */ @@ -117,6 +120,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 16, .bp_bits = 3, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q32_V */ @@ -125,6 +130,7 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 16, .bp_bits = 3, + .has_tb = 1, }, { /* W25Q32DW */ @@ -133,6 +139,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 16, .bp_bits = 3, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q64_V */ @@ -141,6 +149,7 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 17, .bp_bits = 3, + .has_tb = 1, }, { /* W25Q64DW */ @@ -149,6 +158,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 17, .bp_bits = 3, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q64JW */ @@ -157,6 +168,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 17, .bp_bits = 3, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q128_V */ @@ -165,6 +178,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 18, .bp_bits = 3, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q128FW */ @@ -173,6 +188,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 18, .bp_bits = 3, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q128J */ @@ -181,6 +198,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 18, .bp_bits = 3, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q128JW */ @@ -189,6 +208,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 18, .bp_bits = 3, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q256_V */ @@ -197,6 +218,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 16, .bp_bits = 4, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q256J */ @@ -205,6 +228,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 16, .bp_bits = 4, + .has_cmp = 1, + .has_tb = 1, }, }; @@ -224,28 +249,7 @@ const struct spi_flash_bpbits *bits, struct region *out) { - const size_t granularity = 1 << params->protection_granularity_shift; - size_t protected_size = - MIN(bits->bp ? granularity << (bits->bp - 1) : 0, flash->size); - - int tb = bits->tb; - if (bits->cmp) { - protected_size = flash->size - protected_size; - tb = !tb; - } - - if (protected_size == 0) { - out->offset = 0; - out->size = 0; - } else if (protected_size == flash->size) { - out->offset = 0; - out->size = flash->size; - } else { - out->offset = tb ? 0 : flash->size - protected_size; - out->size = protected_size; - } - - return 0; + return spi_flash_common_bpbits_to_region(flash, params, bits, out); } static int winbond_get_bpbits(const struct spi_flash *flash, @@ -446,43 +450,17 @@ const enum spi_flash_status_reg_lockdown mode, struct spi_flash_bpbits *bits) { - struct region wp_region; - - /* Need to touch TOP or BOTTOM */ - if (region_offset(region) != 0 && region_end(region) != flash->size) - return -1; + int ret; if (params->bp_bits != 3 && params->bp_bits != 4) { /* FIXME: not implemented */ return -1; } - wp_region = *region; - - if (region_offset(&wp_region) == 0) - bits->tb = 1; - else - bits->tb = 0; - - if (region_sz(&wp_region) > flash->size / 2) { - bits->cmp = 1; - wp_region.offset = bits->tb ? 0 : region_sz(&wp_region); - wp_region.size = flash->size - region_sz(&wp_region); - bits->tb = !bits->tb; - } else { - bits->cmp = 0; - } - - if (region_sz(&wp_region) == 0) { - bits->bp = 0; - } else if (IS_POWER_OF_2(region_sz(&wp_region)) && - (region_sz(&wp_region) >= - (1 << params->protection_granularity_shift))) { - bits->bp = log2(region_sz(&wp_region)) - - params->protection_granularity_shift + 1; - } else { + ret = spi_flash_common_region_to_bpbits(flash, params, region, bits); + if (ret) { printk(BIOS_ERR, "WINBOND: ERROR: unsupported region size\n"); - return -1; + return ret; } switch (mode) { diff --git a/tests/drivers/Makefile.inc b/tests/drivers/Makefile.inc index d29502f..142dbaa 100644 --- a/tests/drivers/Makefile.inc +++ b/tests/drivers/Makefile.inc @@ -1,17 +1,11 @@ # SPDX-License-Identifier: GPL-2.0-only -tests-y += winbond-spi-test +tests-y += spi-test -winbond-spi-test-srcs += tests/drivers/winbond-spi-test.c -winbond-spi-test-srcs += src/drivers/spi/winbond.c -winbond-spi-test-srcs += src/drivers/spi/spi_flash.c -winbond-spi-test-srcs += src/commonlib/region.c -winbond-spi-test-srcs += src/commonlib/mem_pool.c - -tests-y += macronix-spi-test - -macronix-spi-test-srcs += tests/drivers/macronix-spi-test.c -macronix-spi-test-srcs += src/drivers/spi/macronix.c -macronix-spi-test-srcs += src/drivers/spi/spi_flash.c -macronix-spi-test-srcs += src/commonlib/region.c -macronix-spi-test-srcs += src/commonlib/mem_pool.c +spi-test-srcs += tests/drivers/spi-test.c +spi-test-srcs += src/drivers/spi/winbond.c +spi-test-srcs += src/drivers/spi/macronix.c +spi-test-srcs += src/drivers/spi/spi_flash.c +spi-test-srcs += src/drivers/spi/spi_common.c +spi-test-srcs += src/commonlib/region.c +spi-test-srcs += src/commonlib/mem_pool.c diff --git a/tests/drivers/macronix-spi-test.c b/tests/drivers/macronix-spi-test.c deleted file mode 100644 index bacb9d5..0000000 --- a/tests/drivers/macronix-spi-test.c +++ /dev/null @@ -1,202 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -#include <stdarg.h> -#include <stddef.h> -#include <setjmp.h> -#include <cmocka.h> - -#include <string.h> - -#include <stdio.h> /* debug */ - -#include <commonlib/helpers.h> -#include <spi_flash.h> -#include <spi-generic.h> -#include "../drivers/spi/spi_flash_internal.h" -#include "../drivers/spi/spi_macronix.h" - -struct bp_table { - uint16_t id; - size_t size; - struct block_region { - /* these are in units of 64k-byte blocks */ - size_t offset; - size_t size; - } wp_blocks[1<<4]; -} bp_table[] = { - { - /* MX25L1605D */ - .id = 0x2015, - .size = 0x200000, - .wp_blocks = { - /* off size */ - [0] = { 0, 0 }, - [1] = { 31, 1 }, - [2] = { 30, 2 }, - [3] = { 28, 4 }, - [4] = { 24, 8 }, - [5] = { 16, 16 }, - [6] = { 0, 32 }, - [7] = { 0, 32 }, - [8] = { 0, 32 }, - [9] = { 0, 32 }, - - [10] = { 0, 16 }, - [11] = { 0, 24 }, - [12] = { 0, 28 }, - [13] = { 0, 30 }, - [14] = { 0, 31 }, - [15] = { 0, 32 }, - }, - }, - { - /* MX25L3205D */ - .id = 0x2016, - .size = 0x400000, - .wp_blocks = { - /* off size */ - [0] = { 0, 0 }, - [1] = { 63, 1 }, - [2] = { 62, 2 }, - [3] = { 60, 4 }, - [4] = { 56, 8 }, - [5] = { 48, 16 }, - [6] = { 32, 32 }, - [7] = { 0, 64 }, - [8] = { 0, 64 }, - - [9] = { 0, 32 }, - [10] = { 0, 48 }, - [11] = { 0, 56 }, - [12] = { 0, 60 }, - [13] = { 0, 62 }, - [14] = { 0, 63 }, - [15] = { 0, 64 }, - }, - }, - { - /* MX25L6405D */ - .id = 0x2017, - .size = 0x800000, - .wp_blocks = { - /* off size */ - [0] = { 0, 0 }, - [1] = { 126, 2 }, - [2] = { 124, 4 }, - [3] = { 120, 8 }, - [4] = { 112, 16 }, - [5] = { 96, 32 }, - [6] = { 64, 64 }, - [7] = { 0, 128 }, - [8] = { 0, 128 }, - - [9] = { 0, 64 }, - [10] = { 0, 96 }, - [11] = { 0, 112 }, - [12] = { 0, 120 }, - [13] = { 0, 124 }, - [14] = { 0, 126 }, - [15] = { 0, 128 }, - }, - }, -}; - - -static void macronix_region_bpbits_roundtrip_test(void **state) -{ - int n_cfgs_tested = 0; - - const struct spi_flash_vendor_info *vi = &spi_flash_macronix_vi; - - for (size_t i = 0; i < vi->nr_part_ids; i++) { - const struct spi_flash_part_id *part = &vi->ids[i]; - - if (!part->protection_granularity_shift) - continue; - - struct spi_slave spi; - struct spi_flash flash; - int ret = fill_spi_flash(&spi, &flash, vi, part); - assert_int_equal(ret, 0); - - const struct bp_table *table = NULL; - for (uint16_t j = 0; j < ARRAY_SIZE(bp_table); j++) { - if (bp_table[j].id == part->id[0]) { - table = &bp_table[j]; - break; - } - } - - assert_non_null(table); - - for (u8 bp = 0; bp <= 0xf; bp++) { - struct region expected, out = {-1, -1}; - struct region roundtripped = {-1, -1}; - - const struct block_region *wp_blocks = - &table->wp_blocks[bp]; - const size_t block_size = 1<<16; - - expected.offset = wp_blocks->offset * block_size; - expected.size = wp_blocks->size * block_size; - - struct spi_flash_bpbits bpbits = { - .bp = bp, - }; - - macronix_bpbits_to_region(&flash, part, &bpbits, &out); - - enum spi_flash_status_reg_lockdown mode = - SPI_WRITE_PROTECTION_PIN; - struct spi_flash_bpbits bpbits1 = {}; - int ret = macronix_region_to_bpbits(&flash, part, &out, - mode, &bpbits1); - assert_int_equal(ret, 0); - - /* we have to go through bpbits_to_region again as - * the bp<>range mapping isn't unique. For example - * bp=0b1111 always means everything is protected - * but bp values just after the largest upper-half - * protection also mean "all blocks protected" */ - macronix_bpbits_to_region(&flash, part, &bpbits1, - &roundtripped); - -#if 0 - /* debugging */ - print_message("\nfor part 0x%04x bp=%d:\n", - part->id[0], (int)bp); - print_message("should: 0x%08zx-0x%08zx\n", - region_offset(&expected), - region_end(&expected)); - print_message("is: 0x%08zx-0x%08zx\n", - region_offset(&out), - region_end(&out)); - print_message("rt'ed: 0x%08zx-0x%08zx\n", - region_offset(&roundtripped), - region_end(&roundtripped)); - print_message("bp1: %d\n", bp1); -#endif - - assert_true(expected.size == out.size); - assert_true(expected.offset == out.offset); - - assert_true(expected.size == roundtripped.size); - assert_true(expected.offset == roundtripped.offset); - - n_cfgs_tested++; - } - } - - assert_true(n_cfgs_tested == 48); -} - -#include "spi-mocks.c" - -int main(void) -{ - const struct CMUnitTest tests[] = { - cmocka_unit_test(macronix_region_bpbits_roundtrip_test), - }; - - return cmocka_run_group_tests(tests, NULL, NULL); -} diff --git a/tests/drivers/spi-test.c b/tests/drivers/spi-test.c new file mode 100644 index 0000000..97b0af1 --- /dev/null +++ b/tests/drivers/spi-test.c @@ -0,0 +1,410 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <stdarg.h> +#include <stddef.h> +#include <setjmp.h> +#include <cmocka.h> + +#include <string.h> + +#include <stdio.h> /* debug */ + +#include <commonlib/helpers.h> +#include <spi_flash.h> +#include <spi-generic.h> +#include "../drivers/spi/spi_flash_internal.h" +#include "../drivers/spi/spi_winbond.h" + +int fill_spi_flash(const struct spi_slave *spi, struct spi_flash *flash, + const struct spi_flash_vendor_info *vi, + const struct spi_flash_part_id *part); + +struct block_region { + /* these are in units of 64k-byte blocks */ + size_t offset; + size_t size; +}; + +struct bp_table { + const struct spi_flash_vendor_info *vi; + uint16_t id; + size_t size; + struct block_region wp_blocks[2][2][1<<4]; +} bp_table[] = { + { + /* W25Q16_V */ + .vi = &spi_flash_winbond_vi, + .id = 0x4015, + .wp_blocks = { + /* [cmp][tb][bp] */ + /* off size */ + [0][0][0] = { 0, 0 }, + + /* tb=x, bp=0b000: NONE */ + [0][0][0] = { 0, 0 }, + [0][1][0] = { 0, 0 }, + + /* tb=x, bp=0b11x: ALL */ + [0][0][6] = { 0, 32 }, + [0][1][6] = { 0, 32 }, + [0][0][7] = { 0, 32 }, + [0][1][7] = { 0, 32 }, + + [0][0][1] = { 31, 1 }, + [0][0][2] = { 30, 2 }, + [0][0][3] = { 28, 4 }, + [0][0][4] = { 24, 8 }, + [0][0][5] = { 16, 16 }, + + [0][1][1] = { 0, 1 }, + [0][1][2] = { 0, 2 }, + [0][1][3] = { 0, 4 }, + [0][1][4] = { 0, 8 }, + [0][1][5] = { 0, 16 }, + } + }, + { + /* W25Q256J */ + .vi = &spi_flash_winbond_vi, + .id = 0x7019, + .wp_blocks = { + /* [cmp][tb][bp] */ + /* off size */ + + /* cmp=0, tb=x, bp=0b0000: NONE */ + [0][0][0] = { 0, 0 }, + [0][1][0] = { 0, 0 }, + + /* cmp=1, tb=x, bp=0b0000: ALL */ + [1][0][0] = { 0, 512 }, + [1][1][0] = { 0, 512 }, + + /* cmp=0, tb=x, bp=0b110x|0b1x1x: ALL */ + [0][0 ... 1][10 ... 15] = { 0, 512 }, + + /* cmp=1, tb=x, bp=0b110x|0b1x1x: NONE */ + [1][0 ... 1][10 ... 15] = { 0, 0 }, + + [0][0][1] = { 511, 1 }, + [0][0][2] = { 510, 2 }, + [0][0][3] = { 508, 4 }, + [0][0][4] = { 504, 8 }, + [0][0][5] = { 496, 16 }, + [0][0][6] = { 480, 32 }, + [0][0][7] = { 448, 64 }, + [0][0][8] = { 384, 128 }, + [0][0][9] = { 256, 256 }, + + [0][1][1] = { 0, 1 }, + [0][1][2] = { 0, 2 }, + [0][1][3] = { 0, 4 }, + [0][1][4] = { 0, 8 }, + [0][1][5] = { 0, 16 }, + [0][1][6] = { 0, 32 }, + [0][1][7] = { 0, 64 }, + [0][1][8] = { 0, 128 }, + [0][1][9] = { 0, 256 }, + + [1][0][1] = { 0, 512-1 }, + [1][0][2] = { 0, 512-2 }, + [1][0][3] = { 0, 512-4 }, + [1][0][4] = { 0, 512-8 }, + [1][0][5] = { 0, 512-16 }, + [1][0][6] = { 0, 512-32 }, + [1][0][7] = { 0, 512-64 }, + [1][0][8] = { 0, 512-128 }, + [1][0][9] = { 0, 512-256 }, + + [1][1][1] = { 1, 512-1 }, + [1][1][2] = { 2, 512-2 }, + [1][1][3] = { 4, 512-4 }, + [1][1][4] = { 8, 512-8 }, + [1][1][5] = { 16, 512-16 }, + [1][1][6] = { 32, 512-32 }, + [1][1][7] = { 64, 512-64 }, + [1][1][8] = { 128, 512-128 }, + [1][1][9] = { 256, 512-256 }, + } + }, + + { + /* W25Q128FW */ + .vi = &spi_flash_winbond_vi, + .id = 0x6018, + .wp_blocks = { + /* [cmp][tb][bp] */ + /* off size */ + [0][0][0] = { 0, 0 }, + + /* cmp=0, tb=x, bp=0b000: NONE */ + [0][0][0] = { 0, 0 }, + [0][1][0] = { 0, 0 }, + + /* cmp=0, tb=x, bp=0b111: ALL */ + [0][0][7] = { 0, 256 }, + [0][1][7] = { 0, 256 }, + + [0][0][1] = { 252, 4 }, + [0][0][2] = { 248, 8 }, + [0][0][3] = { 240, 16 }, + [0][0][4] = { 224, 32 }, + [0][0][5] = { 192, 64 }, + [0][0][6] = { 128, 128 }, + + [0][1][1] = { 0, 4 }, + [0][1][2] = { 0, 8 }, + [0][1][3] = { 0, 16 }, + [0][1][4] = { 0, 32 }, + [0][1][5] = { 0, 64 }, + [0][1][6] = { 0, 128 }, + + /* cmp=1, tb=x, bp=0b000: ALL */ + [1][0][0] = { 0, 256 }, + [1][1][0] = { 0, 256 }, + + /* cmp=1, tb=x, bp=0b111: NONE */ + [1][0][7] = { 0, 0 }, + [1][1][7] = { 0, 0 }, + + [1][0][1] = { 0, 256-4 }, + [1][0][2] = { 0, 256-8 }, + [1][0][3] = { 0, 256-16 }, + [1][0][4] = { 0, 256-32 }, + [1][0][5] = { 0, 256-64 }, + [1][0][6] = { 0, 256-128 }, + + [1][1][1] = { 4, 256-4 }, + [1][1][2] = { 8, 256-8 }, + [1][1][3] = { 16, 256-16 }, + [1][1][4] = { 32, 256-32 }, + [1][1][5] = { 64, 256-64 }, + [1][1][6] = { 128, 256-128 }, + } + }, + { + /* MX25L1605D */ + .vi = &spi_flash_macronix_vi, + .id = 0x2015, + .wp_blocks = { + /* off size */ + [0][0][0] = { 0, 0 }, + [0][0][1] = { 31, 1 }, + [0][0][2] = { 30, 2 }, + [0][0][3] = { 28, 4 }, + [0][0][4] = { 24, 8 }, + [0][0][5] = { 16, 16 }, + [0][0][6] = { 0, 32 }, + [0][0][7] = { 0, 32 }, + [0][0][8] = { 0, 32 }, + [0][0][9] = { 0, 32 }, + + [0][0][10] = { 0, 16 }, + [0][0][11] = { 0, 24 }, + [0][0][12] = { 0, 28 }, + [0][0][13] = { 0, 30 }, + [0][0][14] = { 0, 31 }, + [0][0][15] = { 0, 32 }, + }, + }, + { + /* MX25L3205D */ + .vi = &spi_flash_macronix_vi, + .id = 0x2016, + .wp_blocks = { + /* off size */ + [0][0][0] = { 0, 0 }, + [0][0][1] = { 63, 1 }, + [0][0][2] = { 62, 2 }, + [0][0][3] = { 60, 4 }, + [0][0][4] = { 56, 8 }, + [0][0][5] = { 48, 16 }, + [0][0][6] = { 32, 32 }, + [0][0][7] = { 0, 64 }, + [0][0][8] = { 0, 64 }, + + [0][0][9] = { 0, 32 }, + [0][0][10] = { 0, 48 }, + [0][0][11] = { 0, 56 }, + [0][0][12] = { 0, 60 }, + [0][0][13] = { 0, 62 }, + [0][0][14] = { 0, 63 }, + [0][0][15] = { 0, 64 }, + }, + }, + { + /* MX25L6405D */ + .vi = &spi_flash_macronix_vi, + .id = 0x2017, + .wp_blocks = { + /* off size */ + [0][0][0] = { 0, 0 }, + [0][0][1] = { 126, 2 }, + [0][0][2] = { 124, 4 }, + [0][0][3] = { 120, 8 }, + [0][0][4] = { 112, 16 }, + [0][0][5] = { 96, 32 }, + [0][0][6] = { 64, 64 }, + [0][0][7] = { 0, 128 }, + [0][0][8] = { 0, 128 }, + + [0][0][9] = { 0, 64 }, + [0][0][10] = { 0, 96 }, + [0][0][11] = { 0, 112 }, + [0][0][12] = { 0, 120 }, + [0][0][13] = { 0, 124 }, + [0][0][14] = { 0, 126 }, + [0][0][15] = { 0, 128 }, + }, + }, + +}; + +static void test_one_config( + void **state, + const struct spi_flash *flash, + const struct spi_flash_part_id *part, + const struct bp_table *table, + const struct spi_flash_bpbits *bpbits) +{ + const size_t granularity = + 1<<part->protection_granularity_shift; + + struct region expected, out = {-1, -1}; + struct region roundtripped = {-1, -1}; + + flash->prot_ops->bpbits_to_region(flash, part, bpbits, &out); + + enum spi_flash_status_reg_lockdown mode = + SPI_WRITE_PROTECTION_PIN; + struct spi_flash_bpbits bpbits1; + flash->prot_ops->region_to_bpbits(flash, part, &out, mode, &bpbits1); + + flash->prot_ops->bpbits_to_region(flash, part, &bpbits1, &roundtripped); + + const struct block_region *wp_blocks = + &table->wp_blocks[bpbits->cmp][bpbits->tb][bpbits->bp]; + const size_t block_size = 1<<16; + + expected.offset = wp_blocks->offset * block_size; + expected.size = wp_blocks->size * block_size; + +#if 0 + /* debugging */ + print_message("\nfor part 0x%04x:\n", part->id[0]); + print_message("in: cmp=%d tb=%d bp=%d\n", + (int)bpbits->cmp, (int)bpbits->tb, (int)bpbits->bp); + print_message("should: 0x%08zx-0x%08zx\n", + region_offset(&expected), + region_end(&expected)); + print_message("is: 0x%08zx-0x%08zx\n", + region_offset(&out), + region_end(&out)); + print_message("rt: cmp=%d, tb=%d, bp=%d\n", + bpbits1.cmp, bpbits1.tb, bpbits1.bp); + print_message("rt'ed: 0x%08zx-0x%08zx\n", + region_offset(&roundtripped), + region_end(&roundtripped)); +#endif + + assert_true(expected.size == out.size); + assert_true(expected.offset == out.offset); + + assert_true(expected.size == roundtripped.size); + assert_true(expected.offset == roundtripped.offset); +} + +static void spi_flash_region_bpbits_roundtrip_test(void **state) +{ + int n_cfgs_tested = 0; + + for (size_t i = 0; i < ARRAY_SIZE(bp_table); i++) { + const struct bp_table *table = &bp_table[i]; + const struct spi_flash_vendor_info *vi = table->vi; + const struct spi_flash_part_id *part = NULL; + + for (size_t j = 0; j < vi->nr_part_ids; j++) { + if (vi->ids[j].id[0] == table->id) { + part = &vi->ids[j]; + break; + } + } + + assert_true(part->protection_granularity_shift); + + struct spi_slave spi; + struct spi_flash flash; + int ret = fill_spi_flash(&spi, &flash, vi, part); + assert_int_equal(ret, 0); + + u8 tb_max = part->has_tb + 1; + u8 cmp_max = part->has_cmp + 1; + for (u8 tb = 0; tb < tb_max; tb++) { + for (u8 cmp = 0; cmp < cmp_max; cmp++) { + u8 bp_max + = part->bp_bits + ? 1<<part->bp_bits + : 0x10; + for (u8 bp = 0; bp < bp_max; bp++) { + struct spi_flash_bpbits bpbits = { + .cmp = cmp, + .tb = tb, + .bp = bp, + }; + + test_one_config( + state, + &flash, + part, + table, + &bpbits); + + n_cfgs_tested++; + } + } + } + } + + size_t n_cfgs_tested_expected = 160; + if (n_cfgs_tested != n_cfgs_tested_expected) { + fail_msg("n_cfgs_tested is=%d, expected=%d!\n", + n_cfgs_tested, n_cfgs_tested_expected); + } +} + + /* Mocks */ +int do_printk(int msg_level, const char *fmt, ...) +{ + return -1; +} +int spi_claim_bus(const struct spi_slave *slave) +{ + return -1; +} +int spi_xfer_vector(const struct spi_slave *slave, + struct spi_op vectors[], size_t count) +{ + return -1; +} +void spi_release_bus(const struct spi_slave *slave) +{ +} +unsigned int spi_crop_chunk(const struct spi_slave *slave, unsigned int cmd_len, + unsigned int buf_len) +{ + return 0; +} +struct mono_time; +void timer_monotonic_get(struct mono_time *mt) +{ +} + +const struct spi_flash_ops_descriptor spi_flash_pp_0x20_sector_desc; + +int main(void) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test(spi_flash_region_bpbits_roundtrip_test), + }; + + return cmocka_run_group_tests(tests, NULL, NULL); +} diff --git a/tests/drivers/winbond-spi-test.c b/tests/drivers/winbond-spi-test.c deleted file mode 100644 index 2ed0e90..0000000 --- a/tests/drivers/winbond-spi-test.c +++ /dev/null @@ -1,301 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -#include <stdarg.h> -#include <stddef.h> -#include <setjmp.h> -#include <cmocka.h> - -#include <string.h> - -#include <stdio.h> /* debug */ - -#include <commonlib/helpers.h> -#include <spi_flash.h> -#include <spi-generic.h> -#include "../drivers/spi/spi_flash_internal.h" -#include "../drivers/spi/spi_winbond.h" - -int fill_spi_flash(const struct spi_slave *spi, struct spi_flash *flash, - const struct spi_flash_vendor_info *vi, - const struct spi_flash_part_id *part); - -struct block_region { - /* these are in units of 64k-byte blocks */ - size_t offset; - size_t size; -}; - -struct bp_table { - uint16_t id; - size_t size; - struct block_region wp_blocks[2][2][1<<4]; -} bp_table[] = { - { - /* W25Q256J */ - .id = 0x7019, - .wp_blocks = { - /* [cmp][tb][bp] */ - /* off size */ - - /* cmp=0, tb=x, bp=0b0000: NONE */ - [0][0][0] = { 0, 0 }, - [0][1][0] = { 0, 0 }, - - /* cmp=1, tb=x, bp=0b0000: ALL */ - [1][0][0] = { 0, 512 }, - [1][1][0] = { 0, 512 }, - - /* cmp=0, tb=x, bp=0b110x|0b1x1x: ALL */ - [0][0 ... 1][10 ... 15] = { 0, 512 }, - - /* cmp=1, tb=x, bp=0b110x|0b1x1x: NONE */ - [1][0 ... 1][10 ... 15] = { 0, 0 }, - - [0][0][1] = { 511, 1 }, - [0][0][2] = { 510, 2 }, - [0][0][3] = { 508, 4 }, - [0][0][4] = { 504, 8 }, - [0][0][5] = { 496, 16 }, - [0][0][6] = { 480, 32 }, - [0][0][7] = { 448, 64 }, - [0][0][8] = { 384, 128 }, - [0][0][9] = { 256, 256 }, - - [0][1][1] = { 0, 1 }, - [0][1][2] = { 0, 2 }, - [0][1][3] = { 0, 4 }, - [0][1][4] = { 0, 8 }, - [0][1][5] = { 0, 16 }, - [0][1][6] = { 0, 32 }, - [0][1][7] = { 0, 64 }, - [0][1][8] = { 0, 128 }, - [0][1][9] = { 0, 256 }, - - [1][0][1] = { 0, 512-1 }, - [1][0][2] = { 0, 512-2 }, - [1][0][3] = { 0, 512-4 }, - [1][0][4] = { 0, 512-8 }, - [1][0][5] = { 0, 512-16 }, - [1][0][6] = { 0, 512-32 }, - [1][0][7] = { 0, 512-64 }, - [1][0][8] = { 0, 512-128 }, - [1][0][9] = { 0, 512-256 }, - - [1][1][1] = { 1, 512-1 }, - [1][1][2] = { 2, 512-2 }, - [1][1][3] = { 4, 512-4 }, - [1][1][4] = { 8, 512-8 }, - [1][1][5] = { 16, 512-16 }, - [1][1][6] = { 32, 512-32 }, - [1][1][7] = { 64, 512-64 }, - [1][1][8] = { 128, 512-128 }, - [1][1][9] = { 256, 512-256 }, - } - }, - - { - /* W25Q128FW */ - .id = 0x6018, - .wp_blocks = { - /* [cmp][tb][bp] */ - /* off size */ - [0][0][0] = { 0, 0 }, - - /* cmp=0, tb=x, bp=0b000: NONE */ - [0][0][0] = { 0, 0 }, - [0][1][0] = { 0, 0 }, - - /* cmp=0, tb=x, bp=0b111: ALL */ - [0][0][7] = { 0, 256 }, - [0][1][7] = { 0, 256 }, - - [0][0][1] = { 252, 4 }, - [0][0][2] = { 248, 8 }, - [0][0][3] = { 240, 16 }, - [0][0][4] = { 224, 32 }, - [0][0][5] = { 192, 64 }, - [0][0][6] = { 128, 128 }, - - [0][1][1] = { 0, 4 }, - [0][1][2] = { 0, 8 }, - [0][1][3] = { 0, 16 }, - [0][1][4] = { 0, 32 }, - [0][1][5] = { 0, 64 }, - [0][1][6] = { 0, 128 }, - - /* cmp=1, tb=x, bp=0b000: ALL */ - [1][0][0] = { 0, 256 }, - [1][1][0] = { 0, 256 }, - - /* cmp=1, tb=x, bp=0b111: NONE */ - [1][0][7] = { 0, 0 }, - [1][1][7] = { 0, 0 }, - - [1][0][1] = { 0, 256-4 }, - [1][0][2] = { 0, 256-8 }, - [1][0][3] = { 0, 256-16 }, - [1][0][4] = { 0, 256-32 }, - [1][0][5] = { 0, 256-64 }, - [1][0][6] = { 0, 256-128 }, - - [1][1][1] = { 4, 256-4 }, - [1][1][2] = { 8, 256-8 }, - [1][1][3] = { 16, 256-16 }, - [1][1][4] = { 32, 256-32 }, - [1][1][5] = { 64, 256-64 }, - [1][1][6] = { 128, 256-128 }, - } - } -}; - -static void test_one_config( - void **state, - struct spi_flash flash, - const struct spi_flash_part_id *part, - const struct bp_table *table, - const struct spi_flash_bpbits *bpbits) -{ - const size_t granularity = - 1<<part->protection_granularity_shift; - - struct region expected, out = {-1, -1}; - struct region roundtripped = {-1, -1}; - - winbond_bpbits_to_region(&flash, part, bpbits, &out); - - enum spi_flash_status_reg_lockdown mode = - SPI_WRITE_PROTECTION_PIN; - struct spi_flash_bpbits bpbits1; - winbond_region_to_bpbits(&flash, part, &out, mode, - &bpbits1); - - winbond_bpbits_to_region(&flash, part, &bpbits1, &roundtripped); - - const struct block_region *wp_blocks = - &table->wp_blocks[bpbits->cmp][bpbits->tb][bpbits->bp]; - const size_t block_size = 1<<16; - - expected.offset = wp_blocks->offset * block_size; - expected.size = wp_blocks->size * block_size; - -#if 0 - /* debugging */ - print_message("\nfor part 0x%04x cmp=%d tb=%d bp=%d:\n", - part->id[0], (int)bpbits->cmp, (int)bpbits->tb, - (int)bpbits->bp); - print_message("should: 0x%08zx-0x%08zx\n", - region_offset(&expected), - region_end(&expected)); - print_message("is: 0x%08zx-0x%08zx\n", - region_offset(&out), - region_end(&out)); - print_message("rt'ed: 0x%08zx-0x%08zx\n", - region_offset(&roundtripped), - region_end(&roundtripped)); - print_message("bp1: %d\n", bpbits1.bp); -#endif - - assert_true(expected.size == out.size); - assert_true(expected.offset == out.offset); - - assert_true(expected.size == roundtripped.size); - assert_true(expected.offset == roundtripped.offset); -} - -static void winbond_region_bpbits_roundtrip_test(void **state) -{ - int n_cfgs_tested = 0; - - const struct spi_flash_vendor_info *vi = &spi_flash_winbond_vi; - - for (size_t i = 0; i < vi->nr_part_ids; i++) { - const struct spi_flash_part_id *part = &vi->ids[i]; - - if (!part->protection_granularity_shift) - continue; - - /* FIXME: Add tables for the other parts supporting block - * protection */ - if (part->id[0] != 0x7019 && part->id[0] != 0x6018) - continue; - - struct spi_slave spi; - struct spi_flash flash; - int ret = fill_spi_flash(&spi, &flash, vi, part); - assert_int_equal(ret, 0); - - const struct bp_table *table = NULL; - for (uint16_t j = 0; j < ARRAY_SIZE(bp_table); j++) { - if (bp_table[j].id == part->id[0]) { - table = &bp_table[j]; - break; - } - } - - assert_non_null(table); - - u8 bp_max; - if (part->bp_bits == 3) - bp_max = 7; - else if (part->bp_bits == 4) - bp_max = 0xf; - else - fail_msg("Unrecognized bp_bits"); - - for (u8 tbcmp = 0; tbcmp < 4; tbcmp++) { - for (u8 bp = 0; bp <= bp_max; bp++) { - struct spi_flash_bpbits bpbits = { - .cmp = tbcmp & (1<<1), - .tb = tbcmp & (1<<0), - .bp = bp, - }; - - test_one_config( - state, flash, part, table, &bpbits); - - n_cfgs_tested++; - } - } - } - - /* print_message("n_cfgs_tested: %d\n", n_cfgs_tested); */ - assert_true(n_cfgs_tested == 96); -} - - /* Mocks */ -int do_printk(int msg_level, const char *fmt, ...) -{ - return -1; -} -int spi_claim_bus(const struct spi_slave *slave) -{ - return -1; -} -int spi_xfer_vector(const struct spi_slave *slave, - struct spi_op vectors[], size_t count) -{ - return -1; -} -void spi_release_bus(const struct spi_slave *slave) -{ -} -unsigned int spi_crop_chunk(const struct spi_slave *slave, unsigned int cmd_len, - unsigned int buf_len) -{ - return 0; -} -struct mono_time; -void timer_monotonic_get(struct mono_time *mt) -{ -} - -const struct spi_flash_ops_descriptor spi_flash_pp_0x20_sector_desc; - -int main(void) -{ - const struct CMUnitTest tests[] = { - cmocka_unit_test(winbond_region_bpbits_roundtrip_test), - }; - - return cmocka_run_group_tests(tests, NULL, NULL); -} -- To view, visit https://review.coreboot.org/c/coreboot/+/42124 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iadedf160575b20ee7fc671642412e6b3d437a20e Gerrit-Change-Number: 42124 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Gröber (dxld) Gerrit-Reviewer: Daniel Gröber <dxld(a)darkboxed.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

4 8

Change in coreboot[master]: spi/macronix: Add support for boot media protection
by Daniel Gröber (dxld) (Code Review) 28 Feb '23

28 Feb '23

Hello Daniel Gröber, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/41750 to review the following change. Change subject: spi/macronix: Add support for boot media protection ...................................................................... spi/macronix: Add support for boot media protection Tested on a Lenovo T500 with MX25L6405D flash. Change-Id: Icb92ba9f92746f8c7886220b31d86c130f906c6d Signed-off-by: Daniel Gröber <dxld(a)darkboxed.org> --- M src/drivers/spi/macronix.c M src/drivers/spi/spi_flash_internal.h A src/drivers/spi/spi_macronix.h A tests/drivers/Makefile.inc A tests/drivers/macronix-spi-test.c 5 files changed, 519 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/41750/1 diff --git a/src/drivers/spi/macronix.c b/src/drivers/spi/macronix.c index f3f7e2d..333276d 100644 --- a/src/drivers/spi/macronix.c +++ b/src/drivers/spi/macronix.c @@ -4,8 +4,10 @@ #include <commonlib/helpers.h> #include <spi_flash.h> #include <spi-generic.h> +#include <lib.h> #include "spi_flash_internal.h" +#include "spi_macronix.h" /* MX25xx-specific commands */ #define CMD_MX25XX_WREN 0x06 /* Write Enable */ @@ -23,6 +25,17 @@ #define MACRONIX_SR_WIP (1 << 0) /* Write-in-Progress */ +union status_reg { + u8 u; + struct { + u8 wip : 1; + u8 wel : 1; + u8 bp : 4; + u8 _res : 1; + u8 srwd : 1; + }; +}; + static const struct spi_flash_part_id flash_table[] = { { /* MX25L8005 */ @@ -33,16 +46,22 @@ /* MX25L1605D */ .id[0] = 0x2015, .nr_sectors_shift = 9, + .protection_granularity_shift = 16, + .bp_lower_th = 10, }, { /* MX25L3205D */ .id[0] = 0x2016, .nr_sectors_shift = 10, + .protection_granularity_shift = 16, + .bp_lower_th = 9, }, { /* MX25L6405D */ .id[0] = 0x2017, .nr_sectors_shift = 11, + .protection_granularity_shift = 17, + .bp_lower_th = 9, }, { /* MX25L12805D */ @@ -121,6 +140,270 @@ }, }; +/** + * Convert block protection bits, to a region. + */ +void macronix_bpbits_to_region(const struct spi_flash_part_id *params, + const u8 bp, + const size_t flash_size, + struct region *out) +{ + if (bp == 0xf) { + out->offset = 0; + out->size = flash_size; + } else if (bp == 0) { + out->offset = 0; + out->size = 0; + } else { + const u16 granularity_shift = + params->protection_granularity_shift; + + if (bp >= params->bp_lower_th) { + /* Lower half is protected */ + const size_t unprotected_size = + MIN(1<<granularity_shift<<(14 - bp), + flash_size); + + out->offset = 0; + out->size = flash_size - unprotected_size; + } else { + /* Upper half is protected */ + const size_t protected_size = + MIN(1<<(granularity_shift + bp - 1), + flash_size); + + out->offset = flash_size - protected_size; + out->size = protected_size; + } + } +} + +/** + * Convert a region to block protection bits. + * + * rv >= 0: converted bp value + * rv < 0: error + */ +int macronix_region_to_bpbits(const struct spi_flash_part_id *params, + const struct region *region, + const size_t flash_size) +{ + if (region->offset == 0 && region->size >= flash_size) { + return 0xf; + } else if (region->offset == 0 && region->size == 0) { + return 0; + } else if (region->offset >= flash_size || region->size >= flash_size) { + return -1; + } else { + const size_t gran_shift = params->protection_granularity_shift; + const u16 granularity = 1<<gran_shift; + + if (region->offset == 0) { /* Lower half should be protected */ + const size_t unprotected_size = + flash_size - region->size; + + const int is_valid = + IS_POWER_OF_2(unprotected_size) && + unprotected_size >= granularity; + if (!is_valid) + return -1; + + return 14 - log2(unprotected_size>>gran_shift); + } else { + const size_t protected_size = region->size; + + const int is_valid = + IS_POWER_OF_2(protected_size) && + protected_size >= granularity; + if (!is_valid) + return -1; + + return log2(protected_size>>gran_shift) + 1; + } + } +} + +static int macronix_flash_write_status(const struct spi_flash *flash, + const union status_reg val, + const union status_reg mask) +{ + int ret; + union status_reg status; + union status_reg new_status; + + if (val.u & ~mask.u) { + printk(BIOS_WARNING, "MACRONIX: given status reg value %02x" + " doesn't conform to mask %02x!", val.u, mask.u); + } + + ret = spi_flash_cmd_status(flash, &status.u); + if (ret) + return ret; + + new_status.u = (status.u & ~mask.u) | (val.u & mask.u); + + /* already configured right */ + if (new_status.u == status.u) + return 0; + + if (status.srwd) { + printk(BIOS_WARNING, "MACRONIX: status reg write is disabled!" + " Trying anyway."); + } + + ret = spi_flash_cmd(&flash->spi, flash->wren_cmd, NULL, 0); + if (ret) + return ret; + + ret = spi_flash_cmd(&flash->spi, CMD_MX25XX_WRSR, + &status, sizeof(status)); + if (ret) + return ret; + + ret = spi_flash_cmd_status(flash, &status.u); + if (ret) + return ret; + + if ((status.u & mask.u) != (val.u & mask.u)) { + printk(BIOS_WARNING, "MACRONIX: status reg write didn't take," + " is WP pin at GND?\n"); + return -1; + } + + return 0; +} + + +/* + * Read block protect bits from Status Reg. + * Converts block protection bits to a region. + * + * Returns: + * -1 on error + * 1 if region is covered by write protection + * 0 if a part of region isn't covered by write protection + */ +static int macronix_get_write_protection(const struct spi_flash *flash, + const struct region *region) +{ + const struct spi_flash_part_id *params; + struct region wp_region; + int ret; + + params = flash->part; + + if (!params) + return -1; + + if (!params->protection_granularity_shift) + return -1; + + union status_reg reg = { .u = 0 }; + + ret = spi_flash_cmd(&flash->spi, flash->status_cmd, &reg.u, + sizeof(reg.u)); + if (ret) + return ret; + + macronix_bpbits_to_region(params, reg.bp, flash->size, &wp_region); + + if (!region_sz(&wp_region)) { + printk(BIOS_DEBUG, "MACRONIX: flash isn't protected\n"); + return 0; + } + + printk(BIOS_DEBUG, "MACRONIX: flash protected range 0x%08zx-0x%08zx\n", + region_offset(&wp_region), region_end(&wp_region)); + + return region_is_subregion(&wp_region, region); +} + +/* + * Protect a region starting from start of flash or end of flash. + * The caller must provide a supported protected region size. + * Writes block protect bits to Status Reg. + * Optionally lock the status register if lock_sreg is set with the provided + * mode. + * + * @param flash: The flash to operate on + * @param region: The region to write protect + * @param mode: Optional status register lock-down mode + * + * @return 0 on success + */ +static int +macronix_set_write_protection(const struct spi_flash *flash, + const struct region *region, + const enum spi_flash_status_reg_lockdown mode) +{ + const struct spi_flash_part_id *params; + union status_reg val, mask; + int bp; + int ret; + + printk(BIOS_DEBUG, "MACRONIX: want to set protection for" + " 0x%08zx-0x%08zx\n", region_offset(region), region_end(region)); + + params = flash->part; + + if (!params) + return -1; + + if (!params->protection_granularity_shift) { + printk(BIOS_ERR, "MACRONIX: ERROR: spi_flash part doesn't support block protection\n"); + return -1; // TODO: Warning + } + + bp = macronix_region_to_bpbits(params, region, flash->size); + if (bp < 0) { + printk(BIOS_ERR, "MACRONIX: ERROR: unsupported region size\n"); + return -1; + } + + /* Write block protection bits */ + val = (union status_reg) { .bp = bp, }; + mask = (union status_reg) { .bp = ~0, .srwd = ~0, }; + + if (mode != SPI_WRITE_PROTECTION_PRESERVE) { + switch (mode) { + case SPI_WRITE_PROTECTION_NONE: + val.srwd = 0; + break; + case SPI_WRITE_PROTECTION_PIN: + val.srwd = 1; + break; + default: + printk(BIOS_ERR, "MACRONIX: ERROR: requested write" + " protection duration (%d) is not supported by" + " this chip\n", mode); + return -1; + } + } + + ret = macronix_flash_write_status(flash, val, mask); + if (ret) { + printk(BIOS_DEBUG, "MACRONIX: macronix_flash_write_status" + " failed: ret=%d\n", ret); + return ret; + } + + printk(BIOS_DEBUG, "MACRONIX: write-protection range is" + " 0x%08zx-0x%08zx\n", region_offset(region), region_end(region)); + + if (val.srwd) { + printk(BIOS_DEBUG, "MACRONIX: conditional WP# pin" + " hadrware protection is enabled.\n"); + } + + return ret; +} + + +static const struct spi_flash_protection_ops spi_flash_protection_ops = { + .get_write = macronix_get_write_protection, + .set_write = macronix_set_write_protection, +}; + const struct spi_flash_vendor_info spi_flash_macronix_vi = { .id = VENDOR_ID_MACRONIX, .page_size_shift = 8, @@ -129,4 +412,5 @@ .ids = flash_table, .nr_part_ids = ARRAY_SIZE(flash_table), .desc = &spi_flash_pp_0x20_sector_desc, + .prot_ops = &spi_flash_protection_ops, }; diff --git a/src/drivers/spi/spi_flash_internal.h b/src/drivers/spi/spi_flash_internal.h index b4d39b3..f23e440 100644 --- a/src/drivers/spi/spi_flash_internal.h +++ b/src/drivers/spi/spi_flash_internal.h @@ -71,9 +71,11 @@ uint16_t nr_sectors_shift: 4; uint16_t fast_read_dual_output_support : 1; uint16_t _reserved_for_flags: 3; - /* Block protection. Currently used by Winbond. */ + /* Block protection. */ uint16_t protection_granularity_shift : 5; - uint16_t bp_bits : 3; + uint16_t bp_bits : 3; /* Used by Winbond */ + /* bp values starting here protect the lower half */ + uint16_t bp_lower_th : 5; /* Used by Macronix */ }; struct spi_flash_ops_descriptor { diff --git a/src/drivers/spi/spi_macronix.h b/src/drivers/spi/spi_macronix.h new file mode 100644 index 0000000..da32f81 --- /dev/null +++ b/src/drivers/spi/spi_macronix.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef SPI_MACRONIX_H +#define SPI_MACRONIX_H + +void macronix_bpbits_to_region(const struct spi_flash_part_id *params, + const u8 bp, + const size_t flash_size, + struct region *out); + +int macronix_region_to_bpbits(const struct spi_flash_part_id *params, + const struct region *region, + const size_t flash_size); + +#endif /* SPI_MACRONIX_H */ diff --git a/tests/drivers/Makefile.inc b/tests/drivers/Makefile.inc new file mode 100644 index 0000000..691ddc5 --- /dev/null +++ b/tests/drivers/Makefile.inc @@ -0,0 +1,9 @@ +# SPDX-License-Identifier: GPL-2.0-only + +tests-y += macronix-spi-test + +macronix-spi-test-srcs += tests/drivers/macronix-spi-test.c +macronix-spi-test-srcs += src/drivers/spi/macronix.c +macronix-spi-test-srcs += src/commonlib/region.c +macronix-spi-test-srcs += src/commonlib/mem_pool.c +macronix-spi-test-srcs-mocks += do_printk region_is_subregion spi_flash_cmd diff --git a/tests/drivers/macronix-spi-test.c b/tests/drivers/macronix-spi-test.c new file mode 100644 index 0000000..232f069 --- /dev/null +++ b/tests/drivers/macronix-spi-test.c @@ -0,0 +1,207 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <stdarg.h> +#include <stddef.h> +#include <setjmp.h> +#include <cmocka.h> + +#include <string.h> + +#include <stdio.h> /* debug */ + +#include <commonlib/helpers.h> +#include <spi_flash.h> +#include <spi-generic.h> +#include "../drivers/spi/spi_flash_internal.h" +#include "../drivers/spi/spi_macronix.h" + +struct bp_table { + uint16_t id; + size_t size; + struct block_region { + /* these are in units of 64k-byte blocks */ + size_t offset; + size_t size; + } wp_blocks[1<<4]; +} bp_table[] = { + { + /* MX25L1605D */ + .id = 0x2015, + .size = 0x200000, + .wp_blocks = { + /* off size */ + [0] = { 0, 0 }, + [1] = { 31, 1 }, + [2] = { 30, 2 }, + [3] = { 28, 4 }, + [4] = { 24, 8 }, + [5] = { 16, 16 }, + [6] = { 0, 32 }, + [7] = { 0, 32 }, + [8] = { 0, 32 }, + [9] = { 0, 32 }, + + [10] = { 0, 16 }, + [11] = { 0, 24 }, + [12] = { 0, 28 }, + [13] = { 0, 30 }, + [14] = { 0, 31 }, + [15] = { 0, 32 }, + }, + }, + { + /* MX25L3205D */ + .id = 0x2016, + .size = 0x400000, + .wp_blocks = { + /* off size */ + [0] = { 0, 0 }, + [1] = { 63, 1 }, + [2] = { 62, 2 }, + [3] = { 60, 4 }, + [4] = { 56, 8 }, + [5] = { 48, 16 }, + [6] = { 32, 32 }, + [7] = { 0, 64 }, + [8] = { 0, 64 }, + + [9] = { 0, 32 }, + [10] = { 0, 48 }, + [11] = { 0, 56 }, + [12] = { 0, 60 }, + [13] = { 0, 62 }, + [14] = { 0, 63 }, + [15] = { 0, 64 }, + }, + }, + { + /* MX25L6405D */ + .id = 0x2017, + .size = 0x800000, + .wp_blocks = { + /* off size */ + [0] = { 0, 0 }, + [1] = { 126, 2 }, + [2] = { 124, 4 }, + [3] = { 120, 8 }, + [4] = { 112, 16 }, + [5] = { 96, 32 }, + [6] = { 64, 64 }, + [7] = { 0, 128 }, + [8] = { 0, 128 }, + + [9] = { 0, 64 }, + [10] = { 0, 96 }, + [11] = { 0, 112 }, + [12] = { 0, 120 }, + [13] = { 0, 124 }, + [14] = { 0, 126 }, + [15] = { 0, 128 }, + }, + }, +}; + + +static void region_bpbits_roundtrip_test(void **state) +{ + int n_parts_tested; + + const struct spi_flash_vendor_info *vi = &spi_flash_macronix_vi; + + for (size_t i = 0; i < vi->nr_part_ids; i++) { + const struct spi_flash_part_id *id = &vi->ids[i]; + + if (!id->protection_granularity_shift) + continue; + + const struct bp_table *table = NULL; + for (uint16_t j = 0; j < ARRAY_SIZE(bp_table); j++) { + if (bp_table[j].id == id->id[0]) { + table = &bp_table[j]; + break; + } + } + + n_parts_tested++; + + assert_non_null(table); + + for (u8 bp = 0; bp <= 0xf; bp++) { + struct region expected, out = {-1, -1}; + struct region roundtripped = {-1, -1}; + + const struct block_region *wp_blocks = + &table->wp_blocks[bp]; + const size_t block_size = 1<<16; + + expected.offset = wp_blocks->offset * block_size; + expected.size = wp_blocks->size * block_size; + + macronix_bpbits_to_region(id, bp, table->size, &out); + + int bp1 = macronix_region_to_bpbits(id, &out, + table->size); + + /* we have to go through bpbits_to_region again as + * the bp<>range mapping isn't unique. For example + * bp=0b1111 always means everything is protected + * but bp values just after the largest upper-half + * protection also mean "all blocks protected" */ + macronix_bpbits_to_region(id, bp1, table->size, + &roundtripped); + +#if 0 + /* debugging */ + print_message("\nfor part 0x%04x bp=%d:\n", + id->id[0], (int)bp); + print_message("should: 0x%08zx-0x%08zx\n", + region_offset(&expected), + region_end(&expected)); + print_message("is: 0x%08zx-0x%08zx\n", + region_offset(&out), + region_end(&out)); + print_message("rt'ed: 0x%08zx-0x%08zx\n", + region_offset(&roundtripped), + region_end(&roundtripped)); + print_message("bp1: %d\n", bp1); +#endif + + assert_true(expected.size == out.size); + assert_true(expected.offset == out.offset); + + assert_true(expected.size == roundtripped.size); + assert_true(expected.offset == roundtripped.offset); + } + } + + /* Just making sure we actually tested something */ + assert_true(n_parts_tested >= 3); +} + + /* Mocks */ +int spi_flash_cmd(const struct spi_slave *spi, u8 cmd, void *response, + size_t len) +{ + return -1; +} + +int do_printk(int msg_level, const char *fmt, ...) +{ + return -1; +} + +int spi_flash_cmd_status(const struct spi_flash *flash, u8 *reg) +{ + return -1; +} + +const struct spi_flash_ops_descriptor spi_flash_pp_0x20_sector_desc; + +int main(void) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test(region_bpbits_roundtrip_test), + }; + + return cmocka_run_group_tests(tests, NULL, NULL); +} -- To view, visit https://review.coreboot.org/c/coreboot/+/41750 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icb92ba9f92746f8c7886220b31d86c130f906c6d Gerrit-Change-Number: 41750 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Gröber (dxld) Gerrit-Reviewer: Daniel Gröber <dxld(a)darkboxed.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

4 17

Change in coreboot[master]: [WIP]mb/gigabyte/ga-g41m-combo: Add variant
by Arthur Heymans (Code Review) 22 Feb '23

22 Feb '23

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41868 ) Change subject: [WIP]mb/gigabyte/ga-g41m-combo: Add variant ...................................................................... [WIP]mb/gigabyte/ga-g41m-combo: Add variant Untested. Change-Id: I2752b259ff4a0612a7de1c8a3a9a79a3a070cdfc Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/mainboard/gigabyte/ga-g41m-es2l/Kconfig M src/mainboard/gigabyte/ga-g41m-es2l/Kconfig.name A src/mainboard/gigabyte/ga-g41m-es2l/variants/ga-g41m-combo/gpio.c A src/mainboard/gigabyte/ga-g41m-es2l/variants/ga-g41m-combo/hda_verb.c A src/mainboard/gigabyte/ga-g41m-es2l/variants/ga-g41m-combo/overridetree.cb A src/mainboard/gigabyte/ga-g41m-es2l/variants/ga-g41m-combo/superio.c 6 files changed, 247 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/41868/1 diff --git a/src/mainboard/gigabyte/ga-g41m-es2l/Kconfig b/src/mainboard/gigabyte/ga-g41m-es2l/Kconfig index 97f155a..5137c96 100644 --- a/src/mainboard/gigabyte/ga-g41m-es2l/Kconfig +++ b/src/mainboard/gigabyte/ga-g41m-es2l/Kconfig @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-only -if BOARD_GIGABYTE_GA_G41M_ES2L +if BOARD_GIGABYTE_GA_G41M_ES2L || BOARD_GIGABYTE_GA_G41M_COMBO config BOARD_SPECIFIC_OPTIONS def_bool y @@ -8,12 +8,13 @@ select NORTHBRIDGE_INTEL_X4X select SOUTHBRIDGE_INTEL_I82801GX select SUPERIO_ITE_IT8718F if BOARD_GIGABYTE_GA_G41M_ES2L + select SUPERIO_ITE_IT8720F if BOARD_GIGABYTE_GA_G41M_COMBO select HAVE_ACPI_TABLES select BOARD_ROMSIZE_KB_1024 select PCIEXP_ASPM select PCIEXP_CLK_PM select PCIEXP_L1_SUB_STATE - select REALTEK_8168_RESET + select REALTEK_8168_RESET if BOARD_GIGABYTE_GA_G41M_ES2L select HAVE_OPTION_TABLE select HAVE_CMOS_DEFAULT select HAVE_ACPI_RESUME @@ -27,6 +28,7 @@ config VARIANT_DIR string default "ga-g41m-es2l" if BOARD_GIGABYTE_GA_G41M_ES2L + default "ga-g41m-combo" if BOARD_GIGABYTE_GA_G41M_COMBO config OVERRIDE_DEVICETREE string @@ -35,9 +37,14 @@ config MAINBOARD_PART_NUMBER string default "GA-G41M-ES2L" if BOARD_GIGABYTE_GA_G41M_ES2L + default "G41M-Combo" if BOARD_GIGABYTE_GA_G41M_COMBO config MAX_CPUS int default 4 +# Override the default variant behavior, since the data.vbt is the same +config INTEL_GMA_VBT_FILE + default "src/mainboard/$(MAINBOARDDIR)/data.vbt" + endif # BOARD_GIGABYTE_GA_G41M_ES2L diff --git a/src/mainboard/gigabyte/ga-g41m-es2l/Kconfig.name b/src/mainboard/gigabyte/ga-g41m-es2l/Kconfig.name index e685ce1..efa4fa7 100644 --- a/src/mainboard/gigabyte/ga-g41m-es2l/Kconfig.name +++ b/src/mainboard/gigabyte/ga-g41m-es2l/Kconfig.name @@ -1,2 +1,5 @@ config BOARD_GIGABYTE_GA_G41M_ES2L bool "GA-G41M-ES2L" + +config BOARD_GIGABYTE_GA_G41M_COMBO + bool "GA-G41M-Combo" diff --git a/src/mainboard/gigabyte/ga-g41m-es2l/variants/ga-g41m-combo/gpio.c b/src/mainboard/gigabyte/ga-g41m-es2l/variants/ga-g41m-combo/gpio.c new file mode 100644 index 0000000..1f5dfa0 --- /dev/null +++ b/src/mainboard/gigabyte/ga-g41m-es2l/variants/ga-g41m-combo/gpio.c @@ -0,0 +1,100 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <southbridge/intel/common/gpio.h> + +static const struct pch_gpio_set1 pch_gpio_set1_mode = { + .gpio6 = GPIO_MODE_GPIO, + .gpio7 = GPIO_MODE_GPIO, + .gpio8 = GPIO_MODE_GPIO, + .gpio9 = GPIO_MODE_GPIO, + .gpio10 = GPIO_MODE_GPIO, + .gpio12 = GPIO_MODE_GPIO, + .gpio13 = GPIO_MODE_GPIO, + .gpio14 = GPIO_MODE_GPIO, + .gpio15 = GPIO_MODE_GPIO, + .gpio16 = GPIO_MODE_GPIO, + .gpio18 = GPIO_MODE_GPIO, + .gpio20 = GPIO_MODE_GPIO, + .gpio24 = GPIO_MODE_GPIO, + .gpio25 = GPIO_MODE_GPIO, + .gpio26 = GPIO_MODE_GPIO, + .gpio27 = GPIO_MODE_GPIO, + .gpio28 = GPIO_MODE_GPIO, +}; + +static const struct pch_gpio_set1 pch_gpio_set1_direction = { + .gpio6 = GPIO_DIR_INPUT, + .gpio7 = GPIO_DIR_INPUT, + .gpio8 = GPIO_DIR_INPUT, + .gpio9 = GPIO_DIR_INPUT, + .gpio10 = GPIO_DIR_INPUT, + .gpio12 = GPIO_DIR_INPUT, + .gpio13 = GPIO_DIR_INPUT, + .gpio14 = GPIO_DIR_INPUT, + .gpio15 = GPIO_DIR_INPUT, + .gpio16 = GPIO_DIR_INPUT, + .gpio18 = GPIO_DIR_OUTPUT, + .gpio20 = GPIO_DIR_OUTPUT, + .gpio24 = GPIO_DIR_OUTPUT, + .gpio25 = GPIO_DIR_INPUT, + .gpio26 = GPIO_DIR_OUTPUT, + .gpio27 = GPIO_DIR_OUTPUT, + .gpio28 = GPIO_DIR_OUTPUT, +}; + +static const struct pch_gpio_set1 pch_gpio_set1_level = { + .gpio18 = GPIO_LEVEL_HIGH, + .gpio20 = GPIO_LEVEL_HIGH, + .gpio24 = GPIO_LEVEL_LOW, + .gpio26 = GPIO_LEVEL_LOW, + .gpio27 = GPIO_LEVEL_LOW, + .gpio28 = GPIO_LEVEL_LOW, +}; + +static const struct pch_gpio_set1 pch_gpio_set1_invert = { + .gpio6 = GPIO_INVERT, + .gpio7 = GPIO_INVERT, + .gpio8 = GPIO_INVERT, + .gpio12 = GPIO_INVERT, + .gpio13 = GPIO_INVERT, +}; + +static const struct pch_gpio_set1 pch_gpio_set1_blink = { +}; + +static const struct pch_gpio_set2 pch_gpio_set2_mode = { + .gpio32 = GPIO_MODE_GPIO, + .gpio33 = GPIO_MODE_GPIO, + .gpio34 = GPIO_MODE_GPIO, + .gpio38 = GPIO_MODE_GPIO, + .gpio39 = GPIO_MODE_GPIO, +}; + +static const struct pch_gpio_set2 pch_gpio_set2_direction = { + .gpio32 = GPIO_DIR_OUTPUT, + .gpio33 = GPIO_DIR_OUTPUT, + .gpio34 = GPIO_DIR_OUTPUT, + .gpio38 = GPIO_DIR_INPUT, + .gpio39 = GPIO_DIR_INPUT, +}; + +static const struct pch_gpio_set2 pch_gpio_set2_level = { + .gpio32 = GPIO_LEVEL_HIGH, + .gpio33 = GPIO_LEVEL_HIGH, + .gpio34 = GPIO_LEVEL_LOW, +}; + +const struct pch_gpio_map mainboard_gpio_map = { + .set1 = { + .mode = &pch_gpio_set1_mode, + .direction = &pch_gpio_set1_direction, + .level = &pch_gpio_set1_level, + .blink = &pch_gpio_set1_blink, + .invert = &pch_gpio_set1_invert, + }, + .set2 = { + .mode = &pch_gpio_set2_mode, + .direction = &pch_gpio_set2_direction, + .level = &pch_gpio_set2_level, + }, +}; diff --git a/src/mainboard/gigabyte/ga-g41m-es2l/variants/ga-g41m-combo/hda_verb.c b/src/mainboard/gigabyte/ga-g41m-es2l/variants/ga-g41m-combo/hda_verb.c new file mode 100644 index 0000000..f4816ee --- /dev/null +++ b/src/mainboard/gigabyte/ga-g41m-es2l/variants/ga-g41m-combo/hda_verb.c @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <device/azalia_device.h> + +const u32 cim_verb_data[] = { + /* coreboot specific header */ + 0x10ec0887, + 0x1458a002, // Subsystem ID + 0x0000000e, // Number of entries + + /* Pin Widget Verb Table */ + + AZALIA_PIN_CFG(2, 0x11, 0x411110f0), + AZALIA_PIN_CFG(2, 0x12, 0x411111f0), + AZALIA_PIN_CFG(2, 0x14, 0x01014410), + AZALIA_PIN_CFG(2, 0x15, 0x411111f0), + AZALIA_PIN_CFG(2, 0x16, 0x411111f0), + AZALIA_PIN_CFG(2, 0x17, 0x411111f0), + AZALIA_PIN_CFG(2, 0x18, 0x01a19c20), + AZALIA_PIN_CFG(2, 0x19, 0x02a19c30), + AZALIA_PIN_CFG(2, 0x1a, 0x0181342f), + AZALIA_PIN_CFG(2, 0x1b, 0x02214c1f), + AZALIA_PIN_CFG(2, 0x1c, 0x411111f0), + AZALIA_PIN_CFG(2, 0x1d, 0x4004c601), + AZALIA_PIN_CFG(2, 0x1e, 0x411111f0), + AZALIA_PIN_CFG(2, 0x1f, 0x411111f0), +}; + +const u32 pc_beep_verbs[0] = {}; + +const u32 pc_beep_verbs_size = ARRAY_SIZE(pc_beep_verbs); +const u32 cim_verb_data_size = ARRAY_SIZE(cim_verb_data); diff --git a/src/mainboard/gigabyte/ga-g41m-es2l/variants/ga-g41m-combo/overridetree.cb b/src/mainboard/gigabyte/ga-g41m-es2l/variants/ga-g41m-combo/overridetree.cb new file mode 100644 index 0000000..d7cd61f --- /dev/null +++ b/src/mainboard/gigabyte/ga-g41m-es2l/variants/ga-g41m-combo/overridetree.cb @@ -0,0 +1,55 @@ +# SPDX-License-Identifier: GPL-2.0-or-later + +chip northbridge/intel/x4x # Northbridge + device domain 0 on # PCI domain + chip southbridge/intel/i82801gx # Southbridge + device pci 1f.0 on # ISA bridge + subsystemid 0x1458 0x5001 + chip superio/ite/it8720f # Super I/O + device pnp 2e.0 on # Floppy + io 0x60 = 0x3f0 + irq 0x70 = 6 + drq 0x74 = 2 + irq 0xf0 = 0x00 + irq 0xf1 = 0x80 + end + device pnp 2e.1 on # COM1 + io 0x60 = 0x3f8 + irq 0x70 = 4 + end + device pnp 2e.2 off end # COM2 + device pnp 2e.3 on # Parallel port + io 0x60 = 0x378 + io 0x62 = 0x000 + irq 0x70 = 7 + drq 0x74 = 4 + irq 0xf0 = 0x08 + end + device pnp 2e.4 on # Environment controller + io 0x60 = 0x290 + io 0x62 = 0x000 + irq 0x70 = 0x00 + irq 0xf0 = 0x80 + irq 0xf1 = 0x40 + irq 0xf2 = 0x0a + irq 0xf3 = 0x80 + irq 0xf4 = 0x80 + end + device pnp 2e.5 on # Keyboard + io 0x60 = 0x60 + io 0x62 = 0x64 + irq 0x70 = 1 + irq 0xf0 = 0x48 + end + device pnp 2e.6 on # Mouse + irq 0x70 = 12 + irq 0x71 = 2 + irq 0xf0 = 0 + end + device pnp 2e.7 on end # GPIO (done in bootblock) + device pnp 2e.a off end # Consumer IR + end + end + end + end +end diff --git a/src/mainboard/gigabyte/ga-g41m-es2l/variants/ga-g41m-combo/superio.c b/src/mainboard/gigabyte/ga-g41m-es2l/variants/ga-g41m-combo/superio.c new file mode 100644 index 0000000..7aa4c2c --- /dev/null +++ b/src/mainboard/gigabyte/ga-g41m-es2l/variants/ga-g41m-combo/superio.c @@ -0,0 +1,48 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <mainboard/superio.h> +#include <superio/ite/it8720f/it8720f.h> +#include <superio/ite/common/ite.h> + +#define SERIAL_DEV PNP_DEV(0x2e, IT8720F_SP1) +#define GPIO_DEV PNP_DEV(0x2e, IT8720F_GPIO) +#define EC_DEV PNP_DEV(0x2e, IT8720F_EC) + +void mainboard_early_superio(void) +{ + /* TODO: check this with datasheet */ + /* Set default GPIOs on superio */ + ite_reg_write(GPIO_DEV, 0x25, 0x50); + ite_reg_write(GPIO_DEV, 0x26, 0x0c); + ite_reg_write(GPIO_DEV, 0x27, 0x80); + ite_reg_write(GPIO_DEV, 0x28, 0x43); + ite_reg_write(GPIO_DEV, 0x29, 0x0a); + ite_reg_write(GPIO_DEV, 0x2c, 0x01); + ite_reg_write(GPIO_DEV, 0x62, 0x08); + ite_reg_write(GPIO_DEV, 0x73, 0x00); + ite_reg_write(GPIO_DEV, 0xb8, 0x40); + ite_reg_write(GPIO_DEV, 0xc0, 0x10); + ite_reg_write(GPIO_DEV, 0xc1, 0x0c); + ite_reg_write(GPIO_DEV, 0xc2, 0x80); + ite_reg_write(GPIO_DEV, 0xc3, 0x43); + ite_reg_write(GPIO_DEV, 0xc4, 0x0a); + ite_reg_write(GPIO_DEV, 0xc8, 0x00); + ite_reg_write(GPIO_DEV, 0xc9, 0x00); + ite_reg_write(GPIO_DEV, 0xcb, 0x42); + ite_reg_write(GPIO_DEV, 0xcc, 0x02); + ite_reg_write(GPIO_DEV, 0xf0, 0x10); + ite_reg_write(GPIO_DEV, 0xf1, 0x40); + ite_reg_write(GPIO_DEV, 0xf6, 0x0e); + + ite_reg_write(EC_DEV, 0xf0, 0x80); + ite_reg_write(EC_DEV, 0xf1, 0x40); + ite_reg_write(EC_DEV, 0xf2, 0x0a); + ite_reg_write(EC_DEV, 0xf3, 0x80); + ite_reg_write(EC_DEV, 0x70, 0x00); // Don't use IRQ9 + ite_reg_write(EC_DEV, 0x30, 0x01); // Enable + + ite_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE); + + /* Disable SIO reboot */ + ite_reg_write(GPIO_DEV, 0xEF, 0x7E); +} -- To view, visit https://review.coreboot.org/c/coreboot/+/41868 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2752b259ff4a0612a7de1c8a3a9a79a3a070cdfc Gerrit-Change-Number: 41868 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

3 3

Change in ...coreboot[master]: Mistral: Enable USB in romstage
by Name of user not set (Code Review) 17 Jan '23

17 Jan '23

nsekar(a)codeaurora.org has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32063 Change subject: Mistral: Enable USB in romstage ...................................................................... Mistral: Enable USB in romstage Enable USB support for mistral in romstage. TEST=build & run Change-Id: I5c2bbe16aa3601e014a2b77d192565402ed23794 Signed-off-by: Nitheesh Sekar <nsekar(a)codeaurora.org> --- M src/mainboard/google/mistral/Makefile.inc M src/mainboard/google/mistral/mainboard.c A src/mainboard/google/mistral/romstage.c 3 files changed, 48 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/32063/1 diff --git a/src/mainboard/google/mistral/Makefile.inc b/src/mainboard/google/mistral/Makefile.inc index dfb0bbc..2cb9631 100644 --- a/src/mainboard/google/mistral/Makefile.inc +++ b/src/mainboard/google/mistral/Makefile.inc @@ -11,6 +11,7 @@ romstage-y += memlayout.ld romstage-y += chromeos.c romstage-y += reset.c +romstage-y += romstage.c ramstage-y += memlayout.ld ramstage-y += chromeos.c diff --git a/src/mainboard/google/mistral/mainboard.c b/src/mainboard/google/mistral/mainboard.c index b45657f..1d62adb 100644 --- a/src/mainboard/google/mistral/mainboard.c +++ b/src/mainboard/google/mistral/mainboard.c @@ -17,6 +17,20 @@ #include <bootblock_common.h> #include <timestamp.h> #include <vendorcode/google/chromeos/chromeos.h> +#include <soc/usb.h> + +static struct usb_board_data usb1_board_data = { + .parameter_override_x0 = 0x63, + .parameter_override_x1 = 0x03, + .parameter_override_x0 = 0x1d, + .parameter_override_x1 = 0x03, +}; + +static void setup_usb(void) +{ + /* Setting Secondary usb controller */ + setup_usb_host(HSUSB_HS_PORT_1, &usb1_board_data); +} static void mainboard_init(struct device *dev) { @@ -24,6 +38,8 @@ /* Copy WIFI calibration data into CBMEM. */ cbmem_add_vpd_calibration_data(); } + + setup_usb(); } static void mainboard_enable(struct device *dev) diff --git a/src/mainboard/google/mistral/romstage.c b/src/mainboard/google/mistral/romstage.c new file mode 100644 index 0000000..41ee4ed --- /dev/null +++ b/src/mainboard/google/mistral/romstage.c @@ -0,0 +1,31 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2018, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <arch/stages.h> +#include <soc/usb.h> + +static void prepare_usb(void) +{ + /* + * Do DWC3 core and phy reset. Kick these resets off early + * so they get atleast 1msec to settle. + */ + reset_usb(HSUSB_HS_PORT_1); +} + +void platform_romstage_main(void) +{ + prepare_usb(); +} -- To view, visit https://review.coreboot.org/c/coreboot/+/32063 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5c2bbe16aa3601e014a2b77d192565402ed23794 Gerrit-Change-Number: 32063 Gerrit-PatchSet: 1 Gerrit-Owner: nsekar(a)codeaurora.org Gerrit-MessageType: newchange

3 4

Change in coreboot[master]: soc/intel/apl: Add ACPI for SBMBUS control
by Maxim Polyakov (Code Review) 10 Jan '23

10 Jan '23

Maxim Polyakov has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SBMBUS control ...................................................................... soc/intel/apl: Add ACPI for SBMBUS control Based on acpidump from AMI firmware on the Kontron mAL10 COMe module and SMBus Control Methods Interface Specification, Ver 1.0, 1999. Signed-off-by: Maxim Polyakov <max.senia.poliak(a)gmail.com> Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 --- A src/soc/intel/apollolake/acpi/smbus.asl M src/soc/intel/apollolake/acpi/southbridge.asl 2 files changed, 795 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/44507/1 diff --git a/src/soc/intel/apollolake/acpi/smbus.asl b/src/soc/intel/apollolake/acpi/smbus.asl new file mode 100644 index 0000000..b41c0f2 --- /dev/null +++ b/src/soc/intel/apollolake/acpi/smbus.asl @@ -0,0 +1,792 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Device (SBUS) +{ + /* Intel SMBus Controller 0:1f.1 */ + Name (_ADR, 0x001F0001) + + OperationRegion (SMBP, PCI_Config, 0x40, 0xC0) + Field (SMBP, DWordAcc, NoLock, Preserve) + { + , 2, + I2CE, 1 + } + + OperationRegion (SMPB, PCI_Config, 0x20, 0x04) + Field (SMPB, DWordAcc, NoLock, Preserve) + { + , 5, + SBAR, 11 + } + + OperationRegion (SMBI, SystemIO, (SBAR << 0x05), 0x10) + Field (SMBI, ByteAcc, NoLock, Preserve) + { + HSTS, 8, // Host Status + Offset (0x02), + HCON, 8, // Host Control + HCOM, 8, // Host Command + TXSA, 8, // Transmit Slave Address + DAT0, 8, // Host Data 0 + DAT1, 8, // Host Data 1 + HBDR, 8, // Host Block Data Byte + PECR, 8, // Packet Error Check + RXSA, 8, // Receive Slave Data + SDAT, 16 + } + + // SMBus Send Byte + // Arg0: Address + // Arg1: Data + // Return: 1 = Success, 0=Failure + Method (SSXB, 2, Serialized) + { + If (STRT ()) + { // Is the SMBus Controller Ready? + Return (Zero) + } + + // Send Byte + // SMBus Enable + I2CE = Zero + HSTS = 0xBF + TXSA = Arg0 // Write Address + HCOM = Arg1 // Write Data + HCON = 0x48 // Start + Byte Data Protocol + If (COMP ()) + { + HSTS |= 0xFF // Clean up + Return (One) // Success + } + + Return (Zero) + } + + // SMBus Receive Byte + // Arg0: Address + // Return: 0xffff = Failure, Data (8bit) = Success + Method (SRXB, 1, Serialized) + { + // Is the SMBus Controller Ready? + If (STRT ()) + { + Return (0xFFFF) + } + + // Receive Byte + I2CE = Zero // SMBus Enable + HSTS = 0xBF + TXSA = (Arg0 | One) // Write Address + HCON = 0x44 + If (COMP ()) + { + HSTS |= 0xFF // Clean up + Return (DAT0) // Success + } + + Return (0xFFFF) + } + + // SMBus Write Byte + // Arg0: Address + // Arg1: Command + // Arg2: Data + // Return: 1 = Success, 0=Failure + Method (SWRB, 3, Serialized) + { + If (STRT ()) + { // Is the SMBus Controller Ready? + Return (Zero) + } + + I2CE = Zero // SMBus Enable + HSTS = 0xBF + TXSA = Arg0 // Write Address + HCOM = Arg1 // Write Command + DAT0 = Arg2 // Write Data + HCON = 0x48 // Start + Byte Protocol + If (COMP ()) + { + HSTS |= 0xFF // Clean up + Return (One) // Success + } + + Return (Zero) + } + + // SMBus Read Byte + // Arg0: Address + // Arg1: Command + // Return: 0xffff = Failure, Data (8bit) = Success + Method (SRDB, 2, Serialized) + { + If (STRT ()) + { // Is the SMBus Controller Ready? + Return (0xFFFF) + } + + I2CE = Zero // SMBus Enable + HSTS = 0xBF + // Write Address + TXSA = (Arg0 | One) + HCOM = Arg1 // Command + HCON = 0x48 // Start + If (COMP ()) + { + HSTS |= 0xFF // Clean up + Return (DAT0) // Success + } + + Return (0xFFFF) + } + + // SWRW - write word + // Arg0: Address + // Arg1: Command + // Arg2: Data + // Return: 0 on failure, 1, on success + Method (SWRW, 3, Serialized) + { + If (STRT ()) + { + Return (Zero) + } + + I2CE = Zero + HSTS = 0xBF + TXSA = Arg0 + HCOM = Arg1 + DAT1 = (Arg2 & 0xFF) + DAT0 = ((Arg2 >> 0x08) & 0xFF) + HCON = 0x4C + If (COMP ()) + { + HSTS |= 0xFF + Return (One) + } + + Return (Zero) + } + + // SRDW - read word + // Arg0: Address + // Arg1: Command + // Return: 0xffffffff on failure, the read word otherwise + Method (SRDW, 2, Serialized) + { + If (STRT ()) + { + Return (0xFFFF) + } + + I2CE = Zero + HSTS = 0xBF + TXSA = (Arg0 | One) + HCOM = Arg1 + HCON = 0x4C + If (COMP ()) + { + HSTS |= 0xFF + Return (((DAT0 << 0x08) | DAT1)) + } + + Return (0xFFFFFFFF) + } + + // SBLW - write block data + // Arg0: Address + // Arg1: Command + // Arg2: Data - data[0] is the number of bytes that follow + // Return: 0 on failure, 1, on success + Method (SBLW, 4, Serialized) + { + If (STRT ()) + { + Return (Zero) + } + + I2CE = Arg3 + HSTS = 0xBF + TXSA = Arg0 + HCOM = Arg1 + DAT0 = SizeOf (Arg2) + Local1 = Zero + HBDR = DerefOf (Arg2 [Zero]) + HCON = 0x54 + While ((SizeOf (Arg2) > Local1)) + { + Local0 = 0x0FA0 + While ((!(HSTS & 0x80) && Local0)) + { + Local0-- + Stall (0x32) + } + + If (!Local0) + { + KILL () + Return (Zero) + } + + HSTS = 0x80 + Local1++ + If ((SizeOf (Arg2) > Local1)) + { + HBDR = DerefOf (Arg2 [Local1]) + } + } + + If (COMP ()) + { + HSTS |= 0xFF + Return (One) + } + + Return (Zero) + } + + // SBLR - read block data + // Arg0: Address + // Arg1: Command + // Return: 0 on failure, the read data otherwise + Method (SBLR, 3, Serialized) + { + Name (TBUF, Buffer (0x0100){}) + If (STRT ()) + { + Return (Zero) + } + + I2CE = Arg2 + HSTS = 0xBF + TXSA = (Arg0 | One) + HCOM = Arg1 + HCON = 0x54 + Local0 = 0x0FA0 + While ((!(HSTS & 0x80) && Local0)) + { + Local0-- + Stall (0x32) + } + + If (!Local0) + { + KILL () + Return (Zero) + } + + TBUF [Zero] = DAT0 /* \_SB_.PCI0.SBUS.DAT0 */ + HSTS = 0x80 + Local1 = One + While ((Local1 < DerefOf (TBUF [Zero]))) + { + Local0 = 0x0FA0 + While ((!(HSTS & 0x80) && Local0)) + { + Local0-- + Stall (0x32) + } + + If (!Local0) + { + KILL () + Return (Zero) + } + + TBUF [Local1] = HBDR /* \_SB_.PCI0.SBUS.HBDR */ + HSTS = 0x80 + Local1++ + } + + If (COMP ()) + { + HSTS |= 0xFF + Return (TBUF) /* \_SB_.PCI0.SBUS.SBLR.TBUF */ + } + + Return (Zero) + } + + // Wait for SMBus to become ready + Method (STRT, 0, Serialized) + { + // Timeout 200ms + Local0 = 0xC8 + + While (Local0) + { + If ((HSTS & 0x40)) + { // Is used? + // timeout-- + Local0-- + + // Wait 1ms + Sleep (One) + If ((Local0 == Zero)) + { + Return (One) + } + } + Else + { + // We're ready + Local0 = Zero + } + } + + // Timeout 200ms (50us * 4000) + Local0 = 0x0FA0 + While (Local0) + { + If ((HSTS & One)) + { // Is the host busy? + // timeout-- + Local0-- + + // Wait 50us + Stall (0x32) + + If ((Local0 == Zero)) + { + KILL () + } + } + Else + { // Success + Return (Zero) + } + } + // Failure + Return (One) + } + + // Check if last operation completed + // return Failure = 0, Success = 1 + Method (COMP, 0, Serialized) + { + Local0 = 0x0FA0 // Timeout 200ms in 50us steps + While (Local0) + { + If ((HSTS & 0x02)) + { // Completion Status? + // Operation Completed + Return (One) + } + Else + { + Local0-- + Stall (0x32) + If ((Local0 == Zero)) + { + KILL () + } + } + } + Return (Zero) // Failure + } + + // Kill all SMBus communication + Method (KILL, 0, Serialized) + { + HCON |= 0x02 // Send Kill + HSTS |= 0xFF // Clean Status + } + + // SMBus transactional synchronization + Mutex (SBX0, 0x00) + + OperationRegion (SBU0, SystemIO, (SBAR << 0x05), 0x10) + Field (SBU0, ByteAcc, NoLock, Preserve) + { + HSTA, 8, + Offset (0x02), + HCNT, 8, + HCMD, 8, + HADD, 8, + HDT0, 8, + HDT1, 8, + BLKD, 8, + Offset (0x0D), + AUCT, 8 + } + + // SWTC - Wait for Transaction Complete + // Wait until the previous SMBus transaction has completed + // Arg0 : Timeout Value + // return + // 0x00 : OK + // 0x07 : Unknown Failure + // 0x10 : Address Not Acknowledged + // 0x11 : Device Error + // 0x12 : Command Access Denied + // 0x13 : Unknown Error + // 0x17 : Device Access Denied + // 0x18 : Timeout + // 0x19 : Unsupported Protocol + // 0x1A : Bus Busy + // 0x1F : PEC (CRC-8) Error + Method (SWTC, 1, NotSerialized) + { + Local0 = Arg0 + Local2 = 0x07 + // The previous command has completed when the protocol + // register is equal to 0 (zero). Wait <timeout> ms + // for this to occur. + Local1 = One + While ((Local1 == One)) + { + Local3 = (HSTA & 0x1E) + If ((Local3 != Zero)) + { + If ((Local3 == 0x02)) + { + Local2 = Zero + } + Else + { + Local2 = 0x07 + } + + Local1 = Zero + } + // Transaction isn't complete. Check for timeout, and if not, + // sleep 10ms and loop again. + ElseIf ((Local0 < 0x0A)) + { + Local2 = 0x18 // ERROR: Timeout occurred. + Local1 = Zero // Terminate loop. + } + Else + { + Sleep (0x0A) + Local0 -= 0x0A + } + } + Return (Local2) + } + + // SBR - SMBus Read + // Arg0 : Protocol (Integer) + // Arg1 : Slave Address (Integer) + // Arg2 : Command (Integer) + // return + // (0) : Status (Integer) + // (1) : Data Length (Integer) + // (2) : Data (Integer | Buffer) + Method (SBR, 3, Serialized) + { + I2CE = Zero + Local0 = Package (0x12) + { + 0x07, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero + } + + // Make sure the protocol is valid, if not return the + // 'invalid protocol' status code. Note that this segment + // does not support packet error checking. + If ((Arg0 != 0x03)) + { // Read Quick + If ((Arg0 != 0x05)) + { // Receive Byte + If ((Arg0 != 0x07)) + { // Read Byte + If ((Arg0 != 0x09)) + { // Read Word + If ((Arg0 != 0x0B)) + { // Read Block + Local0 [Zero] = 0x19 + Return (Local0) + } + } + } + } + } + + // Acquire the SMBus mutex to ensure transactional synchronization. + If ((Acquire (SBX0, 0xFFFF) == Zero)) + { + // Initiate the transaction by writing the slave address, + // command, and protocol registers. Note that the command + // code and data length are always written, even if the + // protocol doesn't require it (it + // will be ignored). + HSTA = (HSTA | 0xFF) + HADD = (Arg1 | One) + HCMD = Arg2 + If ((Arg0 == 0x03)) + { // Read Quick + HCNT = ((HCNT & 0xA0) | 0x40) + } + + If ((Arg0 == 0x05)) + { // Receive Byte + HCNT = ((HCNT & 0xA0) | 0x44) + } + + If ((Arg0 == 0x07)) + { // Read Byte + HCNT = ((HCNT & 0xA0) | 0x48) + } + + If ((Arg0 == 0x09)) + { // Read Word + HCNT = ((HCNT & 0xA0) | 0x4C) + } + + If ((Arg0 == 0x0B)) + { // Read Block + AUCT &= 0xFD + HCNT = ((HCNT & 0xA0) | 0x54) + Local1 = Zero + } + Else + { + // Wait for completion + Local1 = SWTC (0x03E8) + } + + // Save the status code, data size, and data into the return + // package (if required by the protocol). + Local0 [Zero] = Local1 + If ((Local1 == Zero)) + { + If ((Arg0 == 0x05)) + { // Receive Byte + Local0 [One] = One + Local0 [0x02] = HDT0 + } + + If ((Arg0 == 0x07)) + { // Read Byte + Local0 [One] = One + Local0 [0x02] = HDT0 + } + + If ((Arg0 == 0x09)) + { // Read Word + Local0 [One] = 0x02 + Local2 = HDT1 + Local2 <<= 0x08 + Local2 += HDT0 + Local0 [0x02] = Local2 + } + + If ((Arg0 == 0x0B)) + { // Read Block + Local0 [Zero] = 0x07 + Local4 = 0x9C40 + While ((!(HSTA & 0x80) && Local4)) + { + Local4-- + Stall (0x32) + } + + If (!Local4) + { + HCNT |= 0x02 + HSTA |= 0xFF + Local0 [Zero] = 0x17 + Release (SBX0) + Return (Local0) + } + + Local0 [Zero] = Zero + Local2 = HDT0 + If ((Local2 > 0x10)) + { + Local2 = 0x10 + } + + Local0 [One] = Local2 + Local0 [0x02] = BLKD + Local3 = 0x03 + HSTA = 0x80 + While ((Local3 < (Local2 + 0x02))) + { + Local4 = 0x9C40 + While ((!(HSTA & 0x80) && Local4)) + { + Local4-- + Stall (0x32) + } + + If (!Local4) + { + HCNT |= 0x02 + HSTA |= 0xFF + Local0 [Zero] = 0x18 + Release (SBX0) + Return (Local0) + } + + Local0 [Local3] = BLKD + HSTA = 0x80 + Local3++ + } + } + } + + Release (SBX0) + } + + Return (Local0) + } + + // _SBW - SMBus Write + // + // Arg0 : Protocol Value (Integer) + // Arg1 : Slave Address (Integer) + // Arg2 : Command Code (Integer) + // Arg3 : Data Length (Integer) + // Arg4 : Data (Integer | Buffer) + // + // return Value (Package): + // (0) : Status (Integer) + Method (SBW, 5, Serialized) + { + I2CE = Zero + // Local0 is the return package. The status code is defaulted + // to 'unknown failure' (0x07). + Local0 = Package (0x01) + { + 0x07 + } + + // Make sure the protocol is valid, if not return the + // 'invalid protocol' status code. Note that this segment + // does not support packet error checking or the 'process call' + // protocol. + If ((Arg0 != 0x02)) + { // Write Quick + If ((Arg0 != 0x04)) + { // Send Byte + If ((Arg0 != 0x06)) + { // Write Byte + If ((Arg0 != 0x08)) + { // Write Word + If ((Arg0 != 0x0A)) + { // Write Block + Local0 [Zero] = 0x19 + Return (Local0) + } + } + } + } + } + + // Acquire the SMBus mutex to ensure transactional synchronization. + If ((Acquire (SBX0, 0xFFFF) == Zero)) + { + HSTA = (HSTA | 0xFF) + HADD = (Arg1 & 0xFE) + HCMD = Arg2 + If ((Arg0 == 0x02)) + { // Write Quick + HCNT = ((HCNT & 0xA0) | 0x40) + } + + If ((Arg0 == 0x04)) + { // Send Byte + HCNT = ((HCNT & 0xA0) | 0x44) + } + + If ((Arg0 == 0x06)) + { // Write Byte + HDT0 = Arg4 + HCNT = ((HCNT & 0xA0) | 0x48) + } + + If ((Arg0 == 0x08)) + { // Write Word + HDT0 = (Arg4 & 0xFF) + HDT1 = (Arg4 >> 0x08) + HCNT = ((HCNT & 0xA0) | 0x4C) + } + + If ((Arg0 == 0x0A)) + { // Write Block + If ((Arg3 > 0x10)) + { + Local0 [Zero] = 0x12 + Release (SBX0) + Return (Local0) + } + + If ((Arg3 == Zero)) + { + Local0 [Zero] = 0x12 + Release (SBX0) + Return (Local0) + } + + AUCT &= 0xFD + HDT0 = Arg3 + Local3 = One + BLKD = DerefOf (Arg4 [Zero]) + HCNT = ((HCNT & 0x80) | 0x54) + While ((Arg3 > Local3)) + { + Local4 = 0x9C40 + While ((!(HSTA & 0x80) && Local4)) + { + Local4-- + Stall (0x32) + } + + If (!Local4) + { + HCNT |= 0x02 + HSTA |= 0xFF + Local0 [Zero] = 0x18 + Release (SBX0) + Return (Local0) + } + + BLKD = DerefOf (Arg4 [Local3]) + Local3++ + HSTA = 0x80 + } + + Local4 = 0x9C40 + While ((!(HSTA & 0x80) && Local4)) + { + Local4-- + Stall (0x32) + } + + If (!Local4) + { + HCNT |= 0x02 + HSTA |= 0xFF + Local0 [Zero] = 0x18 + Release (SBX0) + Return (Local0) + } + + HSTA = 0x80 + } + + Local0 [Zero] = SWTC (0x03E8) + Release (SBX0) + } + + Return (Local0) + } +} diff --git a/src/soc/intel/apollolake/acpi/southbridge.asl b/src/soc/intel/apollolake/acpi/southbridge.asl index f4d1497..9a5a5ec 100644 --- a/src/soc/intel/apollolake/acpi/southbridge.asl +++ b/src/soc/intel/apollolake/acpi/southbridge.asl @@ -20,6 +20,9 @@ /* LPC */ #include <soc/intel/common/block/acpi/acpi/lpc.asl> +/* SMBus 0:1f.1 */ +#include "smbus.asl" + /* eMMC */ #include "scs.asl" -- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 1 Gerrit-Owner: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

7 19

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit September 2020