Simon Buhrow 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
......................................................................
Patch Set 11:
(4 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@471
PS8, Line 471: static unsigned char *buf = NULL;
> Just make this an array: […]
This is related to Line 481.
FTDI_HW_BUFFER_SIZE = 4096
Done
https://review.coreboot.org/c/flashrom/+/40477/8/ft2232_spi.c@473
PS8, Line 473: static
> Why is `i` static now?
This is related to Line 541 and following lines. It is explained (shortly) in Line 542.
If there is a WREN command this will not be send immediately to the FTDI chip but is saved in buf. Then WREN command will be send together with the next WR ore ERASE command (the command which made WREN necessary).
The FTDI goes through it´s buffer and will first send WREN and then the next cmd. So the data send to the flash are the same but this saves one time consuming FTDI call every PROGRAM or ERASE command.
To not overwrite the WREN in buf 'i' must be static and only set to '0' after send_buf/FTDI call.
https://review.coreboot.org/c/flashrom/+/40477/8/ft2232_spi.c@481
PS8, Line 481: 4096
> Please #define this magic number somewhere
s. Line 471
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?
s Line 473
--
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: 11
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, 26 May 2020 10:27:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Hello build bot (Jenkins), Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/40477
to look at the new patch set (#11).
Change subject: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call
......................................................................
ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call
Every ftdi_write_data() call is quite time consuming as the ftdi-chips
seems to take always 2-3ms to respond.
This leads to what the comment already says: Minimize USB transfers
by packing as many commands as possible together.
So I packed the WREN command together with the following operation
which can be program or erase operation.
This saves about 1 minute when programming a 128MBit Flash with my
config!
See timings WREN in extra ftdi_write_data() call:
https://ibb.co/0c1J25d
WREN packed with op:
https://ibb.co/7G1hLkj
I´m using ftdi-2232H chip. That´s why I put it at this place.
If anyone has a good overview about all programmers:
This could be implemented in spi_write_cmd() in case that it is
compatible to all programmers
or this principle could be transfered to other programmers which act
in a similar way.
Signed-off-by: Simon Buhrow <simon.buhrow(a)posteo.de>
Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a
---
M ft2232_spi.c
1 file changed, 26 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/40477/11
--
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: 11
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-MessageType: newpatchset
Simon Buhrow 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
......................................................................
Patch Set 10:
(2 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: […]
Done
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)
Done
--
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: 10
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, 26 May 2020 09:49:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Hello build bot (Jenkins), Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/40477
to look at the new patch set (#10).
Change subject: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call
......................................................................
ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call
Every ftdi_write_data() call is quite time consuming as the ftdi-chips
seems to take always 2-3ms to respond.
This leads to what the comment already says: Minimize USB transfers
by packing as many commands as possible together.
So I packed the WREN command together with the following operation
which can be program or erase operation.
This saves about 1 minute when programming a 128MBit Flash with my
config!
See timings WREN in extra ftdi_write_data() call:
https://ibb.co/0c1J25d
WREN packed with op:
https://ibb.co/7G1hLkj
I´m using ftdi-2232H chip. That´s why I put it at this place.
If anyone has a good overview about all programmers:
This could be implemented in spi_write_cmd() in case that it is
compatible to all programmers
or this principle could be transfered to other programmers which act
in a similar way.
Signed-off-by: Simon Buhrow <simon.buhrow(a)posteo.de>
Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a
---
M ft2232_spi.c
1 file changed, 26 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/40477/10
--
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: 10
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-MessageType: newpatchset
Simon Buhrow 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
......................................................................
Patch Set 9:
(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
Done
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
Done
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.
How do I better? Is there a better way to provide images? Just let me know.
--
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: 9
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, 26 May 2020 09:18:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Hello build bot (Jenkins), Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/40477
to look at the new patch set (#9).
Change subject: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call
......................................................................
ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call
Every ftdi_write_data() call is quite time consuming as the ftdi-chips
seems to take always 2-3ms to respond.
This leads to what the comment already says: Minimize USB transfers
by packing as many commands as possible together.
So I packed the WREN command together with the following operation
which can be program or erase operation.
This saves about 1 minute when programming a 128MBit Flash with my
config!
See timings WREN in extra ftdi_write_data() call:
https://ibb.co/0c1J25d
WREN packed with op:
https://ibb.co/7G1hLkj
I´m using ftdi-2232H chip. That´s why I put it at this place.
If anyone has a good overview about all programmers:
This could be implemented in spi_write_cmd() in case that it is
compatible to all programmers
or this principle could be transfered to other programmers which act
in a similar way.
Signed-off-by: Simon Buhrow <simon.buhrow(a)posteo.de>
Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a
---
M ft2232_spi.c
1 file changed, 18 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/40477/9
--
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: 9
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-MessageType: newpatchset
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