Attention is currently required from: Angel Pons, Nicholas Chin.
qianfan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70529 )
Change subject: Add initial CH347T SPI programmer
......................................................................
Patch Set 5:
(4 comments)
File ch347t_spi.c:
https://review.coreboot.org/c/flashrom/+/70529/comment/4c87c78f_8af12ac2
PS5, Line 94: typedef uint32_t __le32;
> Why the typedefs?
all the data structure …
[View More]transfed to ch347 should be little endian. create this typedef can remind me doing a cpu_to_le32 before any transfer.
https://review.coreboot.org/c/flashrom/+/70529/comment/cbe771db_1f389490
PS5, Line 110: } __attribute__((packed));
> This doesn't seem to be portable.
It's OK for gcc based compiler. Or we can create a portable marco such as '__packed' to replace all the "__attribute__((packed))" in the source code.
https://review.coreboot.org/c/flashrom/+/70529/comment/04beb50a_e4d2a86b
PS5, Line 169: if (n > CH347_MAX_DATA_WRITE)
: n = CH347_MAX_DATA_WRITE;
> This doesn't seem to be a good idea...
I can't find a better idea, could you please give me some advices?
https://review.coreboot.org/c/flashrom/+/70529/comment/42db8761_ad22492a
PS5, Line 279: memset(&csctrl, 0, sizeof(csctrl));
> Why not use an initialiser? […]
OK
--
To view, visit https://review.coreboot.org/c/flashrom/+/70529
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6da5e308af76693e60308c8165349128e517e09a
Gerrit-Change-Number: 70529
Gerrit-PatchSet: 5
Gerrit-Owner: qianfan <qianfanguijin(a)163.com>
Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Angel Pons <angel.pons(a)9elements.com>
Gerrit-Comment-Date: Wed, 21 Dec 2022 11:28:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
[View Less]
Attention is currently required from: qianfan, Nicholas Chin, Angel Pons.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70529 )
Change subject: Add initial CH347T SPI programmer
......................................................................
Patch Set 5:
(4 comments)
File ch347t_spi.c:
https://review.coreboot.org/c/flashrom/+/70529/comment/ca704c2c_a8d4666c
PS5, Line 94: typedef uint32_t __le32;
Why the typedefs?
https://review.…
[View More]coreboot.org/c/flashrom/+/70529/comment/699d3264_064dd323
PS5, Line 110: } __attribute__((packed));
This doesn't seem to be portable.
https://review.coreboot.org/c/flashrom/+/70529/comment/f4213933_f6424ca9
PS5, Line 169: if (n > CH347_MAX_DATA_WRITE)
: n = CH347_MAX_DATA_WRITE;
This doesn't seem to be a good idea...
https://review.coreboot.org/c/flashrom/+/70529/comment/402d8dca_bf63b31c
PS5, Line 279: memset(&csctrl, 0, sizeof(csctrl));
Why not use an initialiser?
struct ch347_spi_csctrl_param csctrl = { 0 };
--
To view, visit https://review.coreboot.org/c/flashrom/+/70529
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6da5e308af76693e60308c8165349128e517e09a
Gerrit-Change-Number: 70529
Gerrit-PatchSet: 5
Gerrit-Owner: qianfan <qianfanguijin(a)163.com>
Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: qianfan <qianfanguijin(a)163.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Angel Pons <angel.pons(a)9elements.com>
Gerrit-Comment-Date: Wed, 21 Dec 2022 10:41:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
[View Less]
Attention is currently required from: Edward O'Callaghan, Aarya, Nikolai Artemiev.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70517 )
Change subject: flashrom: Check for flash access restrictions in erase path
......................................................................
Patch Set 10: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/70517
To unsubscribe, or for help writing mail filters, visit https://…
[View More]review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If027a96a024782c7707c6d38680709a1a117f3ef
Gerrit-Change-Number: 70517
Gerrit-PatchSet: 10
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Wed, 21 Dec 2022 09:33:45 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
[View Less]
Attention is currently required from: Edward O'Callaghan, Aarya, Nikolai Artemiev.
Edward O'Callaghan has uploaded a new patch set (#10) to the change originally created by Nikolai Artemiev. ( https://review.coreboot.org/c/flashrom/+/70517 )
Change subject: flashrom: Check for flash access restrictions in erase path
......................................................................
flashrom: Check for flash access restrictions in erase path
Skip unwritable regions if …
[View More]FLASHROM_FLAG_SKIP_UNWRITABLE_REGIONS is
true. If the flag is false, erase operations that include an unwritable
region will not erase anything and return an error.
BUG=b:260440773
BRANCH=none
TEST=flashrom -E on dedede (JSL)
Change-Id: If027a96a024782c7707c6d38680709a1a117f3ef
CoAuthored-by: Edward O'Callaghan <quasisec(a)google.com>
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com>
---
M flashrom.c
1 file changed, 51 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/17/70517/10
--
To view, visit https://review.coreboot.org/c/flashrom/+/70517
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If027a96a024782c7707c6d38680709a1a117f3ef
Gerrit-Change-Number: 70517
Gerrit-PatchSet: 10
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-MessageType: newpatchset
[View Less]