Attention is currently required from: Angel Pons, Nikolai Artemiev, Sergii Dmytruk.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58475 )
Change subject: spi25_statusreg: make register read/write functions generic
......................................................................
Patch Set 24: Code-Review+2
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/58475/comment/eb113063_a5e2babf
PS24, Line 24: TEST=flashrom -{r,w,E}
Note, this doesn't necessarily include calls to spi_write_register()
which depend on the chip and its current state of (software) write-
protection.
I did my best to review these paths one more time, it looks all good.
Just keep in mind that such simple tests usually don't cover all paths.
--
To view, visit https://review.coreboot.org/c/flashrom/+/58475
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0a3951bbf993f2d8d830143b29d3ce16cc6901d7
Gerrit-Change-Number: 58475
Gerrit-PatchSet: 24
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Thu, 24 Feb 2022 16:10:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62282 )
Change subject: ichspi: Add Jasper Lake support
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62282/comment/f9a61e3a_94d738ef
PS4, Line 10: TEST=dedede with `flashrom -p internal --flash-size`.
Any other tests, e.g. reading? If the ME region is locked, you can still read the other regions. A good test would be:
flashrom -p internal --ifd -i fd -i bios -r filename.rom
This makes use of flashrom's IFD parsing code to decide which parts of the flash chip should be read.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62282
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib942d0b8942fe0a991b2af0b187414818485153d
Gerrit-Change-Number: 62282
Gerrit-PatchSet: 4
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Thu, 24 Feb 2022 16:08:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Sam McNally, Nico Huber, Subrata Banik, Rizwan Qureshi, Edward O'Callaghan.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62251 )
Change subject: ichspi: Add Alder Lake support
......................................................................
Patch Set 11:
(1 comment)
File chipset_enable.c:
https://review.coreboot.org/c/flashrom/+/62251/comment/97115816_5cdad4a8
PS10, Line 2154: {0x8086, 0x7aa4, B_S, DEP, "Intel", "Alder Lake-S", enable_flash_pch600},
> > > > > No, he said it can be removed. […]
Please don't remove the entry for ADL-S, just leave it there as NT (Not Tested). It most likely works, someone will eventually test it and report on the mailing list.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62251
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie66cf519df13f3391c41f5016b16a81ef3dfd4bf
Gerrit-Change-Number: 62251
Gerrit-PatchSet: 11
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Thu, 24 Feb 2022 14:53:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Sam McNally, Subrata Banik, Rizwan Qureshi, Edward O'Callaghan, Angel Pons.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62251 )
Change subject: ichspi: Add Alder Lake support
......................................................................
Patch Set 11:
(1 comment)
File chipset_enable.c:
https://review.coreboot.org/c/flashrom/+/62251/comment/f6d6e266_259b6c5e
PS10, Line 2154: {0x8086, 0x7aa4, B_S, DEP, "Intel", "Alder Lake-S", enable_flash_pch600},
> > > > No, he said it can be removed.
> > >
> > > what is the point of adding unused PCI id when we don't have ADL-S support in general in open-source platform.
> >
> > What do you mean? That your chromeos firmware efforts don't include ADL-S?
> > If yes, what does it matter to flashrom?
>
> May be you have misread my original post.
>
> > I will pick the task of removing ADL-S from common code and SoC code if any.
>
> I've mentioned to clean up coreboot code. If you like to keep ADL-S without testing in flashrom, I'm not here to stop you.
Well, I know about your coreboot efforts and it seems sane what you are
doing there. But flashrom is another project.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62251
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie66cf519df13f3391c41f5016b16a81ef3dfd4bf
Gerrit-Change-Number: 62251
Gerrit-PatchSet: 11
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Thu, 24 Feb 2022 14:35:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Sam McNally, Nico Huber, Rizwan Qureshi, Edward O'Callaghan, Angel Pons.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62251 )
Change subject: ichspi: Add Alder Lake support
......................................................................
Patch Set 11:
(1 comment)
File chipset_enable.c:
https://review.coreboot.org/c/flashrom/+/62251/comment/c5015b56_ec3fa447
PS10, Line 2154: {0x8086, 0x7aa4, B_S, DEP, "Intel", "Alder Lake-S", enable_flash_pch600},
> > > No, he said it can be removed.
> >
> > what is the point of adding unused PCI id when we don't have ADL-S support in general in open-source platform.
>
> What do you mean? That your chromeos firmware efforts don't include ADL-S?
> If yes, what does it matter to flashrom?
May be you have misread my original post.
> I will pick the task of removing ADL-S from common code and SoC code if any.
I've mentioned to clean up coreboot code. If you like to keep ADL-S without testing in flashrom, I'm not here to stop you.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62251
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie66cf519df13f3391c41f5016b16a81ef3dfd4bf
Gerrit-Change-Number: 62251
Gerrit-PatchSet: 11
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Thu, 24 Feb 2022 14:25:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Sam McNally, Subrata Banik, Rizwan Qureshi, Edward O'Callaghan, Angel Pons.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62251 )
Change subject: ichspi: Add Alder Lake support
......................................................................
Patch Set 11:
(1 comment)
File chipset_enable.c:
https://review.coreboot.org/c/flashrom/+/62251/comment/daa68955_09aacdb5
PS10, Line 2154: {0x8086, 0x7aa4, B_S, DEP, "Intel", "Alder Lake-S", enable_flash_pch600},
> > No, he said it can be removed.
>
> what is the point of adding unused PCI id when we don't have ADL-S support in general in open-source platform.
What do you mean? That your chromeos firmware efforts don't include ADL-S?
If yes, what does it matter to flashrom?
--
To view, visit https://review.coreboot.org/c/flashrom/+/62251
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie66cf519df13f3391c41f5016b16a81ef3dfd4bf
Gerrit-Change-Number: 62251
Gerrit-PatchSet: 11
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Thu, 24 Feb 2022 14:19:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Sam McNally, Nico Huber, Rizwan Qureshi, Edward O'Callaghan, Angel Pons.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62251 )
Change subject: ichspi: Add Alder Lake support
......................................................................
Patch Set 11:
(1 comment)
File chipset_enable.c:
https://review.coreboot.org/c/flashrom/+/62251/comment/1dd95ab1_b7814d20
PS10, Line 2154: {0x8086, 0x7aa4, B_S, DEP, "Intel", "Alder Lake-S", enable_flash_pch600},
> No, he said it can be removed.
what is the point of adding unused PCI id when we don't have ADL-S support in general in open-source platform. Don't know why we are exhort on everything small thing.
>
> That a PCH of the same generation behaves differently is very unlikely, hence
> we usually add all IDs at once and hence we have the tags to reflect the test
> status. Please don't ignore this projects infrastructure and processes.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62251
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie66cf519df13f3391c41f5016b16a81ef3dfd4bf
Gerrit-Change-Number: 62251
Gerrit-PatchSet: 11
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Thu, 24 Feb 2022 14:09:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Sam McNally, Subrata Banik, Rizwan Qureshi, Edward O'Callaghan, Angel Pons.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62251 )
Change subject: ichspi: Add Alder Lake support
......................................................................
Patch Set 11:
(1 comment)
File chipset_enable.c:
https://review.coreboot.org/c/flashrom/+/62251/comment/0b7e130e_d818f3dc
PS10, Line 2154: {0x8086, 0x7aa4, B_S, DEP, "Intel", "Alder Lake-S", enable_flash_pch600},
> Subrata isn't nobody, he said to remove it and I agree. […]
No, he said it can be removed.
That a PCH of the same generation behaves differently is very unlikely, hence
we usually add all IDs at once and hence we have the tags to reflect the test
status. Please don't ignore this projects infrastructure and processes.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62251
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie66cf519df13f3391c41f5016b16a81ef3dfd4bf
Gerrit-Change-Number: 62251
Gerrit-PatchSet: 11
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Thu, 24 Feb 2022 13:48:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment