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@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,