Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42115 ) Change subject: spi/winbond: Pull out winbond_region_to_bpbits function ...................................................................... Patch Set 2: (3 comments) https://review.coreboot.org/c/coreboot/+/42115/2/src/drivers/spi/winbond.c File src/drivers/spi/winbond.c: https://review.coreboot.org/c/coreboot/+/42115/2/src/drivers/spi/winbond.c@2... PS2, Line 214: static int winbond_region_to_bpbits( Same here. https://review.coreboot.org/c/coreboot/+/42115/2/src/drivers/spi/winbond.c@4... PS2, Line 467: ret = winbond_get_bpbits(flash, &bpbits); Why the get_bpbits() here? Aren't you going to overwrite the whole thing anyway? https://review.coreboot.org/c/coreboot/+/42115/2/src/drivers/spi/winbond.c@4... PS2, Line 497: val.reg1.bp3.srp0 = bpbits.winbond.srp0; Why is this down here and not with the others above? -- To view, visit https://review.coreboot.org/c/coreboot/+/42115 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2c7b08cb56772aa620e690077bbbb1fde3d7aae7 Gerrit-Change-Number: 42115 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Gröber (dxld) Gerrit-Reviewer: Daniel Gröber <dxld@darkboxed.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Julius Werner <jwerner@chromium.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 18 Jun 2020 03:03:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment