May 2020 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call to save ...
by Angel Pons (Code Review) May 19, 2020

May 19, 2020

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 ) Change subject: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call to save programming time ...................................................................... Patch Set 8: Code-Review+1 (3 comments) https://review.coreboot.org/c/flashrom/+/40477/8//COMMIT_MSG Commit Message: https://review.coreboot.org/c/flashrom/+/40477/8//COMMIT_MSG@7 PS8, Line 7: to save programming time Omit this part from the commit summary https://review.coreboot.org/c/flashrom/+/40477/8//COMMIT_MSG@9 PS8, Line 9: Every ftdi_write_data() call is quite time consuming as the ftdi-chips seems to take always 2-3ms to respond. Please split these lines so that they aren't longer than 72 characters https://review.coreboot.org/c/flashrom/+/40477/8//COMMIT_MSG@17 PS8, Line 17: https://ibb.co/0c1J25d These links will stop working at some point. -- To view, visit https://review.coreboot.org/c/flashrom/+/40477 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a Gerrit-Change-Number: 40477 Gerrit-PatchSet: 8 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 19 May 2020 18:31:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call to save ...
by Angel Pons (Code Review) May 19, 2020

May 19, 2020

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 ) Change subject: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call to save programming time ...................................................................... Patch Set 8: (6 comments) https://review.coreboot.org/c/flashrom/+/40477/8/ft2232_spi.c File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/40477/8/ft2232_spi.c@402 PS8, Line 402: /* 280 Byte = (9 Byte CMD + 1 Byte WREN) + (9 Byte CMD + 1 Byte op) + 4 Byte Addr This comment would need an extra tab and could be tidied up. Maybe use: /* * 270 bytes = * + 9 B (CMD) * + 1 B (WREN) * + 9 B (CMD) * + 1 B (op) * + 4 B (addr) * + 256 B (page data) * * With op: PageProgram or Erase; CMD: FTDI-Chip commands */ https://review.coreboot.org/c/flashrom/+/40477/8/ft2232_spi.c@471 PS8, Line 471: static unsigned char *buf = NULL; Just make this an array: static unsigned char buf[FTDI_HW_BUFFER_SIZE]; https://review.coreboot.org/c/flashrom/+/40477/8/ft2232_spi.c@473 PS8, Line 473: static Why is `i` static now? https://review.coreboot.org/c/flashrom/+/40477/8/ft2232_spi.c@480 PS8, Line 480: ftdi FTDI (and missing a space before the comment closes) https://review.coreboot.org/c/flashrom/+/40477/8/ft2232_spi.c@481 PS8, Line 481: 4096 Please #define this magic number somewhere https://review.coreboot.org/c/flashrom/+/40477/8/ft2232_spi.c@542 PS8, Line 542: /* Return to get second op (Program or Erase) without resetting buf nor i*/ Why? -- To view, visit https://review.coreboot.org/c/flashrom/+/40477 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a Gerrit-Change-Number: 40477 Gerrit-PatchSet: 8 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 19 May 2020 18:29:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call to save ...
by Angel Pons (Code Review) May 19, 2020

May 19, 2020

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 ) Change subject: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call to save programming time ...................................................................... Patch Set 8: > Patch Set 8: > > > Patch Set 8: > > > > Is that good practice to add reviewers by myself? > > Sure, if you think somebody is a suitable reviewer, add them (but not just "anybody"). Just respect if they remove themselves from a commit and you're all set. Sorry, this flew under my radar. -- To view, visit https://review.coreboot.org/c/flashrom/+/40477 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a Gerrit-Change-Number: 40477 Gerrit-PatchSet: 8 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 19 May 2020 18:20:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add board enable for ASUS P3B-F
by Keith Hui (Code Review) May 19, 2020

May 19, 2020

Keith Hui has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41354 ) Change subject: Add board enable for ASUS P3B-F ...................................................................... Add board enable for ASUS P3B-F With this change flashrom can detect, enable and flash on this board both under vendor BIOS and coreboot. Change-Id: I395ff50fbcda8ecdaa26033f0d99b2b0eb42f7ff Signed-off-by: Keith Hui <buurin(a)gmail.com> --- M board_enable.c M print.c 2 files changed, 48 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/54/41354/1 diff --git a/board_enable.c b/board_enable.c index a5980eb..9cf0103 100644 --- a/board_enable.c +++ b/board_enable.c @@ -875,6 +875,52 @@ /* * Suited for: + * - ASUS P3B-F + * + * We are talking to a proprietary device on SMBus: the AS99127F which does + * much more than the Winbond W83781D it tries to be compatible with. + */ +static int board_asus_p3b_f(void) +{ + /* + * Find where the SMBus host is. ASUS sets it to 0xE800; coreboot sets it to 0x0F00. + */ + struct pci_dev *dev; + uint16_t smbba; + uint8_t b; + + dev = pci_dev_find(0x8086, 0x7113); /* Intel PIIX4, PM/SMBus function. */ + if (!dev) { + msg_perr("\nERROR: Intel PIIX4 PM not found.\n"); + return -1; + } + + smbba = pci_read_word(dev, 0x90) & 0xfff0; + + OUTB(0xFF, smbba); /* Clear previous SMBus status. */ + OUTB(0x48 << 1, smbba + 4); + OUTB(0x80, smbba + 3); + OUTB(0x80, smbba + 5); + OUTB(0x48, smbba + 2); + + /* Wait until SMBus transaction is complete. */ + b = 0x1; + while (b & 0x01) { + b = INB(0x80); + b = INB(smbba); + } + + /* Write failed if any status is set. */ + if (b & 0x1e) { + msg_perr("Failed to write to device.\n"); + return -1; + } + + return 0; +} + +/* + * Suited for: * - ASUS P5A * * This is rather nasty code, but there's no way to do this cleanly. @@ -2354,6 +2400,7 @@ {0x8086, 0x24cc, 0, 0, 0x8086, 0x24c3, 0x1043, 0x1869, "^M6Ne$", NULL, NULL, P3, "ASUS", "M6Ne", 0, NT, intel_ich_gpio43_raise}, {0x8086, 0x7180, 0, 0, 0x8086, 0x7110, 0, 0, "^OPLX-M$", NULL, NULL, P3, "ASUS", "OPLX-M", 0, NT, intel_piix4_gpo18_lower}, {0x8086, 0x7190, 0, 0, 0x8086, 0x7110, 0, 0, "^P2B-N$", NULL, NULL, P3, "ASUS", "P2B-N", 0, OK, intel_piix4_gpo18_lower}, + {0x8086, 0x7190, 0x1043, 0x8024, 0x8086, 0x7110, 0, 0, "P3B-F", "asus", "p3b-f", P3, "ASUS", "P3B-F", 0, OK, board_asus_p3b_f}, {0x8086, 0x1A30, 0x1043, 0x8025, 0x8086, 0x244B, 0x104D, 0x80F0, NULL, NULL, NULL, P3, "ASUS", "P4B266-LM", 0, OK, intel_ich_gpio21_raise}, {0x8086, 0x1a30, 0x1043, 0x8070, 0x8086, 0x244b, 0x1043, 0x8028, NULL, NULL, NULL, P3, "ASUS", "P4B266", 0, OK, intel_ich_gpio22_raise}, {0x8086, 0x1A30, 0x1043, 0x8088, 0x8086, 0x24C3, 0x1043, 0x8089, NULL, NULL, NULL, P3, "ASUS", "P4B533-E", 0, NT, intel_ich_gpio22_raise}, diff --git a/print.c b/print.c index 6a7ff5d..feca45e 100644 --- a/print.c +++ b/print.c @@ -667,7 +667,7 @@ B("ASUS", "P2B-N", OK, NULL, NULL), B("ASUS", "P2E-M", OK, NULL, NULL), B("ASUS", "P2L97-S", OK, NULL, NULL), - B("ASUS", "P3B-F", BAD, NULL, "No public report found. Owned by Uwe Hermann <uwe(a)hermann-uwe.de>. May work now."), + B("ASUS", "P3B-F", OK, NULL, "Owned by Uwe Hermann <uwe(a)hermann-uwe.de>."), B("ASUS", "P4B266", OK, NULL, NULL), B("ASUS", "P4B266-LM", OK, "http://esupport.sony.com/US/perl/swu-list.pl?mdl=PCVRX650", NULL), B("ASUS", "P4B533-E", OK, NULL, NULL), -- To view, visit https://review.coreboot.org/c/flashrom/+/41354 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I395ff50fbcda8ecdaa26033f0d99b2b0eb42f7ff Gerrit-Change-Number: 41354 Gerrit-PatchSet: 1 Gerrit-Owner: Keith Hui <buurin(a)gmail.com> Gerrit-MessageType: newchange

2 2

Change in flashrom[master]: 82802ab.c: Remove duplicated check
by Angel Pons (Code Review) May 13, 2020

May 13, 2020

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/38661 ) Change subject: 82802ab.c: Remove duplicated check ...................................................................... 82802ab.c: Remove duplicated check Change-Id: I5d511d7ec254bdbd9926e6d8efc308fb2339cb81 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M 82802ab.c 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/61/38661/1 diff --git a/82802ab.c b/82802ab.c index a44a66e..a13a4a0 100644 --- a/82802ab.c +++ b/82802ab.c @@ -89,9 +89,9 @@ chipaddr bios = flash->virtual_memory; chip_writeb(flash, 0x70, bios); - if ((chip_readb(flash, bios) & 0x80) == 0) { /* it's busy */ - while ((chip_readb(flash, bios) & 0x80) == 0) ; - } + + while ((chip_readb(flash, bios) & 0x80) == 0) /* it's busy */ + ; status = chip_readb(flash, bios); -- To view, visit https://review.coreboot.org/c/flashrom/+/38661 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5d511d7ec254bdbd9926e6d8efc308fb2339cb81 Gerrit-Change-Number: 38661 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

5 9

Change in flashrom[master]: ati: add rv710/rv770/rv790 ids
by Idwer Vollering (Code Review) May 12, 2020

May 12, 2020

Idwer Vollering has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/29092 ) Change subject: ati: add rv710/rv770/rv790 ids ...................................................................... Patch Set 5: I'm tempted to rubberstamp +2 on this and other PCI ID additions, in the ati branch. -- To view, visit https://review.coreboot.org/c/flashrom/+/29092 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia3bad57e00a29741b67fb1e8141c1c9cce887331 Gerrit-Change-Number: 29092 Gerrit-PatchSet: 5 Gerrit-Owner: Luc Verhaegen <libv(a)skynet.be> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Comment-Date: Tue, 12 May 2020 14:51:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ati: re-enable generating wiki code
by Idwer Vollering (Code Review) May 11, 2020

May 11, 2020

Idwer Vollering has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41258 ) Change subject: ati: re-enable generating wiki code ...................................................................... ati: re-enable generating wiki code This unbreaks building with CONFIG_PRINT_WIKI=yes. Tested with gcc 7 and clang 6 on ubuntu, with https://review.coreboot.org/c/flashrom/+/36946 applied on top. Change-Id: Ifde0285fc1e9df0f6d159a5972c5d3a43c2f6a1a Signed-off-by: Idwer Vollering <vidwer(a)gmail.com> --- M print_wiki.c 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/58/41258/1 diff --git a/print_wiki.c b/print_wiki.c index 812e01d..ca5a71e 100644 --- a/print_wiki.c +++ b/print_wiki.c @@ -360,7 +360,7 @@ return count; } -static char *vendor_colour_get(void) +static const char *vendor_colour_get(void) { static bool vendor_colour = false; @@ -378,7 +378,7 @@ int i = 0; const struct dev_entry *devs = prog.devs.dev; const unsigned int count = count_supported_devs_wiki(devs); - char *colour = vendor_colour_get(); + const char *colour = vendor_colour_get(); for (i = 0; devs[i].vendor_id != 0; i++) { printf("|- bgcolor=\"#%s\"\n", colour); @@ -407,7 +407,7 @@ static void print_supported_pci_matches_wiki_helper(const struct programmer_entry prog) { const struct flashrom_pci_match *matches = prog.devs.pci_match; - char *colour = vendor_colour_get(); + const char *colour = vendor_colour_get(); int i, count = count_supported_pci_matches_wiki(matches); int vendor_length = 0; uint16_t vendor_id = 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/41258 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ifde0285fc1e9df0f6d159a5972c5d3a43c2f6a1a Gerrit-Change-Number: 41258 Gerrit-PatchSet: 1 Gerrit-Owner: Idwer Vollering <vidwer(a)gmail.com> Gerrit-MessageType: newchange

1 1

Change in flashrom[master]: pci2: add wiki printing support
by Angel Pons (Code Review) May 11, 2020

May 11, 2020

Angel Pons has uploaded a new patch set (#5) to the change originally created by Luc Verhaegen. ( https://review.coreboot.org/c/flashrom/+/29084 ) Change subject: pci2: add wiki printing support ...................................................................... pci2: add wiki printing support Change-Id: I8c839feda2ef3ec97183f1e06712881786622ebf Signed-off-by: Luc Verhaegen <libv(a)skynet.be> --- M print_wiki.c 1 file changed, 99 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/84/29084/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/29084 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8c839feda2ef3ec97183f1e06712881786622ebf Gerrit-Change-Number: 29084 Gerrit-PatchSet: 5 Gerrit-Owner: Luc Verhaegen <libv(a)skynet.be> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: add newer pci infrastructure
by Angel Pons (Code Review) May 11, 2020

May 11, 2020

Angel Pons has uploaded a new patch set (#3) to the change originally created by Luc Verhaegen. ( https://review.coreboot.org/c/flashrom/+/29078 ) Change subject: add newer pci infrastructure ...................................................................... add newer pci infrastructure This code does not negatively affect existing pci support and existing pci devices, it introduces parallel infrastructure which: - allows users to define a private which could be device specific. This avoids doing multiple lookups, which massively helps the ati spi driver which will support hundreds of devices. - uses linux sysfs pci infrastructure for enable/disable and mapping resources. This circumvents the security restrictions surrounding /dev/mem, but means that support for other operating systems still needs to be cobbled together. - looks up device names through libpci, keeping us from maintaining a separate names list. Change-Id: I59af37cba5cb78014e0714c654db2501151f605e Signed-off-by: Luc Verhaegen <libv(a)skynet.be> --- M pcidev.c M programmer.h 2 files changed, 165 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/29078/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/29078 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I59af37cba5cb78014e0714c654db2501151f605e Gerrit-Change-Number: 29078 Gerrit-PatchSet: 3 Gerrit-Owner: Luc Verhaegen <libv(a)skynet.be> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Luc Verhaegen <libv(a)skynet.be> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: pci2: add read/write/mask byte/long mmio accessors
by Angel Pons (Code Review) May 11, 2020

May 11, 2020

Angel Pons has uploaded a new patch set (#3) to the change originally created by Luc Verhaegen. ( https://review.coreboot.org/c/flashrom/+/29080 ) Change subject: pci2: add read/write/mask byte/long mmio accessors ...................................................................... pci2: add read/write/mask byte/long mmio accessors As for the mask command: For those "embedded developers" who grew up with the awkward x86 in/out assembly instructions with the illogical ordering it will seem alien. As a graphics driver developer who has had to deal with thousands of registers at a time, mostly setting a few bits at a time, i found the increased readability and debuggability of a mask command invaluable. Once the ati_spi code is added, it will be clear just how effective this command is. As for the lack of automatic rollback: - it makes little sense to roll back every single register written. One could imagine a scenario where this would perfectly undo the flash being written. - it makes little sense to roll back every mask command: Multiple mask commands are likely to touch the same register. - engines, even simple spi engines, are triggered by a single bit or a single register being written. Automatic rollback would trigger engines before the fitting registers are written. - reading might also trigger an engine. How does one roll back those? How, if the register is automatically read before it is written, are we dealing with that? Instead, by providing programmer internal save/restore functions, the programmer gets full control over what is saved and restored, and the order in which those happen. The fact that we automatically retain the device match private, should make it easier to have device specific save/restore callbacks. Change-Id: I5c3f57e9c510337bb81be30aa6c143840a349a60 Signed-off-by: Luc Verhaegen <libv(a)skynet.be> --- M pcidev.c M programmer.h 2 files changed, 101 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/80/29080/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/29080 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5c3f57e9c510337bb81be30aa6c143840a349a60 Gerrit-Change-Number: 29080 Gerrit-PatchSet: 3 Gerrit-Owner: Luc Verhaegen <libv(a)skynet.be> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0