Change in flashrom[master]: flashchips.c: Take GD25LQ128C/D from downstream

[Alan Green (Code Review)]

Alan Green has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/35478 )


Change subject: flashchips.c: Take GD25LQ128C/D from downstream
......................................................................

flashchips.c: Take GD25LQ128C/D from downstream

Take the diffs for GD25LQ128C/D from the ChromiumOS flashrom repo.

This chips was added in Chromium OS `commit 62cd8106` by
[e-mail] on 2016-07-18, and has been marked tested.

Signed-off-by: Alan Green <[e-mail]>
Change-Id: I4358392520507ddbfc654feb49fa982d49db2f28
---
M flashchips.c
1 file changed, 2 insertions(+), 3 deletions(-)



  git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/35478/1

diff --git a/flashchips.c b/flashchips.c
index 3994c93..5c797fd 100644
--- a/flashchips.c
+++ b/flashchips.c
@@ -5894,7 +5894,7 @@
 		.page_size	= 256,
 		/* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44 */
 		.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP,
-		.tested		= TEST_UNTESTED,
+		.tested		= TEST_OK_PREW,
 		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
@@ -5916,8 +5916,7 @@
 				.block_erase = spi_block_erase_c7,
 			}
 		},
-		.printlock	= spi_prettyprint_status_register_bp4_srwd,
-		.unlock		= spi_disable_blockprotect_bp4_srwd, /* TODO: 2nd status reg (read with 0x35) */
+		.unlock		= spi_disable_blockprotect,
 		.write		= spi_chip_write_256,
 		.read		= spi_chip_read, /* Fast read (0x0B) and multi I/O supported */
 		.voltage	= {1695, 1950},

-- 
To view, visit https://review.coreboot.org/c/flashrom/+/35478
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4358392520507ddbfc654feb49fa982d49db2f28
Gerrit-Change-Number: 35478
Gerrit-PatchSet: 1
Gerrit-Owner: Alan Green <[e-mail]>
Gerrit-MessageType: newchange

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/35478 )

Change subject: flashchips.c: Take GD25LQ128C/D from downstream
......................................................................


Patch Set 1:

(1 comment)

https://review.coreboot.org/c/flashrom/+/35478/1/flashchips.c 
File flashchips.c:

https://review.coreboot.org/c/flashrom/+/35478/1/flashchips.c@a5919 
PS1, Line 5919: printlock
I think you can't remove this as it will create a NULL ptr deref in the dispatch site? Also why remove it?



-- 
To view, visit https://review.coreboot.org/c/flashrom/+/35478
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4358392520507ddbfc654feb49fa982d49db2f28
Gerrit-Change-Number: 35478
Gerrit-PatchSet: 1
Gerrit-Owner: Alan Green <[e-mail]>
Gerrit-Reviewer: Alan Green <[e-mail]>
Gerrit-Reviewer: Edward O'Callaghan <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Comment-Date: Fri, 20 Sep 2019 05:35:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Alan Green (Code Review)]

Hello Edward O'Callaghan, build bot (Jenkins), 

I'd like you to reexamine a change. Please visit

    https://review.coreboot.org/c/flashrom/+/35478

to look at the new patch set (#2).

Change subject: flashchips.c: Take GD25LQ128C/D from downstream
......................................................................

flashchips.c: Take GD25LQ128C/D from downstream

Take the diffs for GD25LQ128C/D from the ChromiumOS flashrom repo.

This chips was added in Chromium OS `commit 62cd8106` by
[e-mail] on 2016-07-18, and has been marked tested.

Signed-off-by: Alan Green <[e-mail]>
Change-Id: I4358392520507ddbfc654feb49fa982d49db2f28
---
M flashchips.c
1 file changed, 2 insertions(+), 2 deletions(-)


  git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/35478/2
-- 
To view, visit https://review.coreboot.org/c/flashrom/+/35478
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4358392520507ddbfc654feb49fa982d49db2f28
Gerrit-Change-Number: 35478
Gerrit-PatchSet: 2
Gerrit-Owner: Alan Green <[e-mail]>
Gerrit-Reviewer: Alan Green <[e-mail]>
Gerrit-Reviewer: Edward O'Callaghan <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-MessageType: newpatchset

[Alan Green (Code Review)]

Alan Green has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/35478 )

Change subject: flashchips.c: Take GD25LQ128C/D from downstream
......................................................................


Patch Set 2:

(1 comment)

https://review.coreboot.org/c/flashrom/+/35478/1/flashchips.c 
File flashchips.c:

https://review.coreboot.org/c/flashrom/+/35478/1/flashchips.c@a5919 
PS1, Line 5919: printlock
> I think you can't remove this as it will create a NULL ptr deref in the dispatch site? Also why remo […]
> You're right. In ChromiumOS, both uses of printlock are guarded, but in coreboot, only one of them is guarded.

To your other question: As per the patch description, I was copying the definition from ChromiumOS, which doesn't have a printlock, but is marked TESTED_PREW.



-- 
To view, visit https://review.coreboot.org/c/flashrom/+/35478
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4358392520507ddbfc654feb49fa982d49db2f28
Gerrit-Change-Number: 35478
Gerrit-PatchSet: 2
Gerrit-Owner: Alan Green <[e-mail]>
Gerrit-Reviewer: Alan Green <[e-mail]>
Gerrit-Reviewer: Edward O'Callaghan <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Comment-Date: Sun, 22 Sep 2019 23:03:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Edward O'Callaghan <[e-mail]>
Gerrit-MessageType: comment

[Alan Green (Code Review)]

Hello Edward O'Callaghan, build bot (Jenkins), 

I'd like you to reexamine a change. Please visit

    https://review.coreboot.org/c/flashrom/+/35478

to look at the new patch set (#4).

Change subject: flashchips.c: Take GD25LQ128C/D from downstream
......................................................................

flashchips.c: Take GD25LQ128C/D from downstream

Take the diffs for GD25LQ128C/D from the ChromiumOS flashrom repo.

This chips was added in Chromium OS `commit 62cd8106` by
[e-mail] on 2016-07-18, and has been marked tested.

Signed-off-by: Alan Green <[e-mail]>
Change-Id: I4358392520507ddbfc654feb49fa982d49db2f28
---
M flashchips.c
1 file changed, 2 insertions(+), 2 deletions(-)


  git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/35478/4
-- 
To view, visit https://review.coreboot.org/c/flashrom/+/35478
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4358392520507ddbfc654feb49fa982d49db2f28
Gerrit-Change-Number: 35478
Gerrit-PatchSet: 4
Gerrit-Owner: Alan Green <[e-mail]>
Gerrit-Reviewer: Alan Green <[e-mail]>
Gerrit-Reviewer: Edward O'Callaghan <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-MessageType: newpatchset

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/35478 )

Change subject: flashchips.c: Take GD25LQ128C/D from downstream
......................................................................


Patch Set 4: Code-Review+2

(1 comment)

Looks good I think

https://review.coreboot.org/c/flashrom/+/35478/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/flashrom/+/35478/4//COMMIT_MSG@7 
PS4, Line 7: Take
Nit: "take" is rather ambiguous. Maybe use "update" instead?



-- 
To view, visit https://review.coreboot.org/c/flashrom/+/35478
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4358392520507ddbfc654feb49fa982d49db2f28
Gerrit-Change-Number: 35478
Gerrit-PatchSet: 4
Gerrit-Owner: Alan Green <[e-mail]>
Gerrit-Reviewer: Alan Green <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Edward O'Callaghan <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Sun, 29 Sep 2019 14:27:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in flashrom[master]: flashchips.c: Update GD25LQ128C/D from downstream

[Alan Green (Code Review)]

Hello Edward O'Callaghan, Angel Pons, build bot (Jenkins), 

I'd like you to reexamine a change. Please visit

    https://review.coreboot.org/c/flashrom/+/35478

to look at the new patch set (#5).

Change subject: flashchips.c: Update GD25LQ128C/D from downstream
......................................................................

flashchips.c: Update GD25LQ128C/D from downstream

Update the diffs for GD25LQ128C/D from the ChromiumOS flashrom repo.

This chips was added in Chromium OS `commit 62cd8106` by
[e-mail] on 2016-07-18, and has been marked tested.

Signed-off-by: Alan Green <[e-mail]>
Change-Id: I4358392520507ddbfc654feb49fa982d49db2f28
---
M flashchips.c
1 file changed, 2 insertions(+), 2 deletions(-)


  git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/35478/5
-- 
To view, visit https://review.coreboot.org/c/flashrom/+/35478
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4358392520507ddbfc654feb49fa982d49db2f28
Gerrit-Change-Number: 35478
Gerrit-PatchSet: 5
Gerrit-Owner: Alan Green <[e-mail]>
Gerrit-Reviewer: Alan Green <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Edward O'Callaghan <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-MessageType: newpatchset

Change in flashrom[master]: flashchips.c: Update GD25LQ128C/D from downstream

[Alan Green (Code Review)]

Alan Green has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/35478 )

Change subject: flashchips.c: Update GD25LQ128C/D from downstream
......................................................................


Patch Set 5:

(1 comment)
> Patch Set 1:
>
> (1 comment)

https://review.coreboot.org/c/flashrom/+/35478/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/flashrom/+/35478/4//COMMIT_MSG@7 
PS4, Line 7: Take
> Nit: "take" is rather ambiguous. […]
> Done

Change in flashrom[master]: flashchips.c: Update GD25LQ128C/D from downstream

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/35478 )

Change subject: flashchips.c: Update GD25LQ128C/D from downstream
......................................................................


Patch Set 5: Code-Review+2


-- 
To view, visit https://review.coreboot.org/c/flashrom/+/35478
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4358392520507ddbfc654feb49fa982d49db2f28
Gerrit-Change-Number: 35478
Gerrit-PatchSet: 5
Gerrit-Owner: Alan Green <[e-mail]>
Gerrit-Reviewer: Alan Green <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Edward O'Callaghan <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Fri, 04 Oct 2019 07:24:00 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in flashrom[master]: flashchips.c: Update GD25LQ128C/D from downstream

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/35478 )

Change subject: flashchips.c: Update GD25LQ128C/D from downstream
......................................................................


Patch Set 5: Code-Review-1

(1 comment)

-1 just to draw attention...

https://review.coreboot.org/c/flashrom/+/35478/5/flashchips.c 
File flashchips.c:

https://review.coreboot.org/c/flashrom/+/35478/5/flashchips.c@a5920 
PS5, Line 5920: 		.unlock		= spi_disable_blockprotect_bp4_srwd, /* TODO: 2nd status reg (read with 0x35) */
Something wrong with this? Why remove it?



-- 
To view, visit https://review.coreboot.org/c/flashrom/+/35478
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4358392520507ddbfc654feb49fa982d49db2f28
Gerrit-Change-Number: 35478
Gerrit-PatchSet: 5
Gerrit-Owner: Alan Green <[e-mail]>
Gerrit-Reviewer: Alan Green <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Edward O'Callaghan <[e-mail]>
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Fri, 04 Oct 2019 18:27:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in flashrom[master]: flashchips.c: Update GD25LQ128C/D from downstream

[Alan Green (Code Review)]

Alan Green has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/35478 )

Change subject: flashchips.c: Update GD25LQ128C/D from downstream
......................................................................


Abandoned

old patch
-- 
To view, visit https://review.coreboot.org/c/flashrom/+/35478
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4358392520507ddbfc654feb49fa982d49db2f28
Gerrit-Change-Number: 35478
Gerrit-PatchSet: 5
Gerrit-Owner: Alan Green <[e-mail]>
Gerrit-Reviewer: Alan Green <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Edward O'Callaghan <[e-mail]>
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-MessageType: abandon