June 2024 - flashrom-gerrit - mail.coreboot.org

[XS] Change in flashrom[main]: util/ich_descriptors_tool: Add Panther Lake SoC to supported chipsets
by Subrata Banik (Code Review) 21 Jun '24

21 Jun '24

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/83149?usp=email ) Change subject: util/ich_descriptors_tool: Add Panther Lake SoC to supported chipsets ...................................................................... util/ich_descriptors_tool: Add Panther Lake SoC to supported chipsets This change extends the ich_descriptors_tool to recognize and process descriptors for Intel's upcoming Panther Lake SoC. BUG=b:347669091 TEST=ich_descriptors_tool is able to detect "panther" chipset and show below information: > ./util/ich_descriptors_tool/ich_descriptors_tool Need the file name of a descriptor image to read from. usage: './util/ich_descriptors_tool/ich_descriptors_tool -f <image file name> [-c <chipset name>] [-d]' ... ... To also print the data stored in the descriptor straps you have to indicate the chipset series with the '-c' parameter and one of the possible arguments: - "ich8", - "ich9", - "ich10", - "apollo" for Intel's Apollo Lake SoC. - "gemini" for Intel's Gemini Lake SoC. - "jasper" for Intel's Jasper Lake SoC. - "meteor" for Intel's Meteor Lake SoC. - "panther" for Intel's Panther Lake SoC. ... ... - "300" or "cannon" for Intel's 300 series chipsets. - "400" or "comet" for Intel's 400 series chipsets. - "500" or "tiger" for Intel's 500 series chipsets. - "600" or "alder" for Intel's 600 series chipsets. Change-Id: I17d616d346daca15b43eb294401ac0c672b64c4a Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M util/ich_descriptors_tool/ich_descriptors_tool.c 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/49/83149/1 diff --git a/util/ich_descriptors_tool/ich_descriptors_tool.c b/util/ich_descriptors_tool/ich_descriptors_tool.c index 844d8e9..09587f7 100644 --- a/util/ich_descriptors_tool/ich_descriptors_tool.c +++ b/util/ich_descriptors_tool/ich_descriptors_tool.c @@ -129,6 +129,7 @@ "\t- \"gemini\" for Intel's Gemini Lake SoC.\n" "\t- \"jasper\" for Intel's Jasper Lake SoC.\n" "\t- \"meteor\" for Intel's Meteor Lake SoC.\n" +"\t- \"panther\" for Intel's Panther Lake SoC.\n" "\t- \"5\" or \"ibex\" for Intel's 5 series chipsets,\n" "\t- \"6\" or \"cougar\" for Intel's 6 series chipsets,\n" "\t- \"7\" or \"panther\" for Intel's 7 series chipsets.\n" @@ -248,6 +249,8 @@ cs = CHIPSET_ELKHART_LAKE; else if (strcmp(csn, "meteor") == 0) cs = CHIPSET_METEOR_LAKE; + else if (strcmp(csn, "panther") == 0) + cs = CHIPSET_PANTHER_LAKE; } ret = read_ich_descriptors_from_dump(buf, len, &cs, &desc); -- To view, visit https://review.coreboot.org/c/flashrom/+/83149?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I17d616d346daca15b43eb294401ac0c672b64c4a Gerrit-Change-Number: 83149 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>

1 0

[S] Change in flashrom[main]: ichspi: Add support for Panther Lake
by Subrata Banik (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Anastasia Klimchuk, Hsuan Ting Chen, Hsuan-ting Chen, Sam McNally. Hello Anastasia Klimchuk, Hsuan Ting Chen, Hsuan-ting Chen, Sam McNally, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/83144?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: ichspi: Add support for Panther Lake ...................................................................... ichspi: Add support for Panther Lake This patch adds Panther Lake support into flashrom as per Intel Panther Lake SPI programming doc, number: 815466. BUG=b:347669091 TEST=Flashrom is able to detect PTL SPI DID and show chipset name as below: > flashrom --flash-name .... Found chipset "Intel Panther Lake-U/H 12Xe". .... > flashrom -p internal --ifd -i fd -i bios -r /tmp/bios.rom .... Reading ich_descriptor... done. Assuming chipset 'Panther Lake'. Using regions: "bios", "fd". Reading flash... done. SUCCESS Change-Id: I99cd8eb7cbb11381f8e8455b06cf90b9db77d8f0 Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M chipset_enable.c M ich_descriptors.c M ichspi.c M include/programmer.h 4 files changed, 33 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/83144/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/83144?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I99cd8eb7cbb11381f8e8455b06cf90b9db77d8f0 Gerrit-Change-Number: 83144 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: ichspi: Add support for Panther Lake
by Subrata Banik (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Anastasia Klimchuk, Hsuan Ting Chen, Hsuan-ting Chen, Sam McNally. Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/flashrom/+/83144?usp=email ) Change subject: ichspi: Add support for Panther Lake ...................................................................... Patch Set 2: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/83144/comment/ec682ae6_d4389ddc?us… : PS2, Line 9: Panther Lake > I am wondering, maybe there is a datasheet available? Asking for myself. yes, Intel doc number: 815466 File include/programmer.h: https://review.coreboot.org/c/flashrom/+/83144/comment/ce83b134_79ff0e4b?us… : PS1, Line 364: /* All chipsets after METEOR_LAKE should support checking BIOS_BM to get read/write access to of FREG0~15 */ > I think what Hsuan-ting meant to say (but correct me if I am wrong), is that we have 100 places in all ich* code that switch through the cases of ich_chipset enum, and the order is not the same between these 100 places. > For example in `programmer.h` and `ichspi.c` Meteor Lake goes after Elkhart, but in `ich_descriptors.c` Meteor is before Elkhart. > > I think it would be more readable to have the same order where it's possible (it is not always be possible, like pprint_freq is different). But in a separate commit, because this one adds support for Panther Lake. > > what do you think? why would be the order is important inside the switch case ? for example: ``` switch (chipset): case 0: case 5: case 2: fool() break; ``` in above example, the execution of "foo" in important for all cases like 0/2/5 but order in which they are being added in switch case not important at all. As mentioned earlier, the only case where the order was meaningful earlier was fixed with CB:83143. Apart from there, there are only one newly added logic that cares about the index value in which the Chipset numbers being added post MTL ``` static unsigned int ich_get_defined_region_count(void) { return (ich_generation >= CHIPSET_METEOR_LAKE) ? 16 : 8; } ``` other than that I don't think the order make sense elsewhere. -- To view, visit https://review.coreboot.org/c/flashrom/+/83144?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I99cd8eb7cbb11381f8e8455b06cf90b9db77d8f0 Gerrit-Change-Number: 83144 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 21 Jun 2024 13:22:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: ichspi: Add support for Panther Lake
by Anastasia Klimchuk (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen, Sam McNally, Subrata Banik. Anastasia Klimchuk has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/flashrom/+/83144?usp=email ) Change subject: ichspi: Add support for Panther Lake ...................................................................... Patch Set 2: (5 comments) Patchset: PS2: Thank you for the patch! Commit Message: https://review.coreboot.org/c/flashrom/+/83144/comment/4e2558fe_c9044367?us… : PS2, Line 9: Panther Lake I am wondering, maybe there is a datasheet available? Asking for myself. File include/programmer.h: https://review.coreboot.org/c/flashrom/+/83144/comment/b2b591c5_c4565e9b?us… : PS1, Line 364: /* All chipsets after METEOR_LAKE should support checking BIOS_BM to get read/write access to of FREG0~15 */ > > > We attempted to move METEOR_LAKE to the bottom of the ich_chipset enum, but this patch reveals t […] I think what Hsuan-ting meant to say (but correct me if I am wrong), is that we have 100 places in all ich* code that switch through the cases of ich_chipset enum, and the order is not the same between these 100 places. For example in `programmer.h` and `ichspi.c` Meteor Lake goes after Elkhart, but in `ich_descriptors.c` Meteor is before Elkhart. I think it would be more readable to have the same order where it's possible (it is not always be possible, like pprint_freq is different). But in a separate commit, because this one adds support for Panther Lake. what do you think? File util/ich_descriptors_tool/ich_descriptors_tool.c: https://review.coreboot.org/c/flashrom/+/83144/comment/62776c40_463373d8?us… : PS2, Line 123: the chipset series with the '-c' parameter and one of the possible arguments I think you need to add panther here too https://review.coreboot.org/c/flashrom/+/83144/comment/51840fd7_b3299d94?us… : PS2, Line 251: else if (strcmp(csn, "panther") == 0) : cs = CHIPSET_PANTHER_LAKE; I think the ich_descriptor_tool is run separately from flashrom. Can you run it and add as one more test scenario? (and in a commit message too) Thanks! -- To view, visit https://review.coreboot.org/c/flashrom/+/83144?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I99cd8eb7cbb11381f8e8455b06cf90b9db77d8f0 Gerrit-Change-Number: 83144 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Comment-Date: Fri, 21 Jun 2024 12:58:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com>

1 0

[S] Change in flashrom[main]: flashchips: Set default wp test status for chips with custom config
by Sergii Dmytruk (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer. Sergii Dmytruk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/83132?usp=email ) Change subject: flashchips: Set default wp test status for chips with custom config ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/83132?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I6ed8e04cd215865dc6a7d9415634dedbe3014ab5 Gerrit-Change-Number: 83132 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 21 Jun 2024 12:20:28 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in flashrom[main]: ich_descriptor: Fix chipset_names index for Intel Meteor Lake
by Subrata Banik (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Hsuan Ting Chen. Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/flashrom/+/83143?usp=email ) Change subject: ich_descriptor: Fix chipset_names index for Intel Meteor Lake ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Thank you for catching this! can you please help to submit in next 24 hr as per submission rule, I don't have submit access right for this project. (based on my contribution to Flashrom, i would love to get that) -- To view, visit https://review.coreboot.org/c/flashrom/+/83143?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ic09cf0474c980369bcbf90924d45f697bc1b0a0d Gerrit-Change-Number: 83143 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Comment-Date: Fri, 21 Jun 2024 09:20:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[XS] Change in flashrom[main]: ich_descriptor: Fix chipset_names index for Intel Meteor Lake
by Anastasia Klimchuk (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Hsuan Ting Chen, Subrata Banik. Anastasia Klimchuk has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/flashrom/+/83143?usp=email ) Change subject: ich_descriptor: Fix chipset_names index for Intel Meteor Lake ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Patchset: PS1: Thank you for catching this! -- To view, visit https://review.coreboot.org/c/flashrom/+/83143?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ic09cf0474c980369bcbf90924d45f697bc1b0a0d Gerrit-Change-Number: 83143 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Comment-Date: Fri, 21 Jun 2024 08:59:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[XS] Change in flashrom[main]: flashchips: add GD25LB256E chip model
by Anastasia Klimchuk (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Nikolai Artemiev, Stefan Reinauer, Victor Lim. Anastasia Klimchuk has posted comments on this change by Victor Lim. ( https://review.coreboot.org/c/flashrom/+/83140?usp=email ) Change subject: flashchips: add GD25LB256E chip model ...................................................................... Patch Set 1: (2 comments) Patchset: PS1: This looks good, thank you! I have just one comment about commit message. Commit Message: https://review.coreboot.org/c/flashrom/+/83140/comment/86ed56f1_7cbf7c9e?us… : PS1, Line 12: Could you please add here, in commit message, the operations that you tested? We usually add this information in commit message when adding a chip model. Just list what you tested, for example: Tested read, write, erase, --wp-enable, --wp-disable, --wp-list and --wp-range (but list what you were testing). -- To view, visit https://review.coreboot.org/c/flashrom/+/83140?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I0aa520b068a86098f6b4a1b68401c425b33e501f Gerrit-Change-Number: 83140 Gerrit-PatchSet: 1 Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Victor Lim <vlim(a)gigadevice.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 21 Jun 2024 08:06:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in flashrom[main]: hwaccess_x86_io: Fix Android compilation with bionic libc
by Jakub Czapiga (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Anastasia Klimchuk, Hsuan-ting Chen, Peter Marheine. Jakub Czapiga has posted comments on this change by Jakub Czapiga. ( https://review.coreboot.org/c/flashrom/+/83133?usp=email ) Change subject: hwaccess_x86_io: Fix Android compilation with bionic libc ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/83133/comment/50b2ccd7_82c60bc1?us… : PS1, Line 9: Recently Android Bionic LibC got sys/io.h. > This does imply there will be minimum required version for Android support? As you are saying "recen […] With the current code flashrom on Android will work only on super-old builds because glibc-syscalls.h was moved to bits/ in [this patch](https://android-review.git.corp.google.com/c/platform/bionic/+/22180…. Yet this file still only defines syscall numbers/mappings and does not provide implementation of iopl/ioperm nor anything in sys/io.h. iopl and ioperm were introduced recently in [this patch](https://android-review.git.corp.google.com/c/platform/bionic/+/31352…. File hwaccess_x86_io.c: https://review.coreboot.org/c/flashrom/+/83133/comment/4662e030_44540076?us… : PS1, Line 60: #if defined(__ANDROID__) > How did you build flashrom, with make or meson? […] For Android you can only build with Android.mk or Android.bp (Soong BluePrint). Building with anything else than these two methods is not really viable on Android, and hacking meson into this build system is not allowed in AOSP. I can provide Android.bp if you want, but it requires pulling some dependencies which are in progress of being accepted, so I wouldn't recommend it TBH. Ad. 1) I ported pciutils (libpci) to Android. Now it only has to get through 3rd-party review in Android. Ad. 2) I do not think so. As I mentioned, building with anything other than Kati or Soong is not allowed in Android. Everything has to have Android.{mk,bp} now and it's maintained on the Android side. Examples: [cpuinfo](https://android.googlesource.com/platform/external/cpuinfo/+/refs/…, [curl](https://android.googlesource.com/platform/external/curl/+/refs/heads/…, [elfutils](https://android.googlesource.com/platform/external/elfutils/+/ref… -- To view, visit https://review.coreboot.org/c/flashrom/+/83133?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Id80b83c2718679c925ed6ddfe33cbe837eea0429 Gerrit-Change-Number: 83133 Gerrit-PatchSet: 1 Gerrit-Owner: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 21 Jun 2024 07:48:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[XS] Change in flashrom[main]: hwaccess_x86_io: Fix Android compilation with bionic libc
by Anastasia Klimchuk (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Hsuan-ting Chen, Jakub Czapiga, Peter Marheine. Anastasia Klimchuk has posted comments on this change by Jakub Czapiga. ( https://review.coreboot.org/c/flashrom/+/83133?usp=email ) Change subject: hwaccess_x86_io: Fix Android compilation with bionic libc ...................................................................... Patch Set 1: (4 comments) Patchset: PS1: Jakub, thank you so your patch! Not often people use flashrom on Android, this is very useful that you do. I have few comments. Commit Message: https://review.coreboot.org/c/flashrom/+/83133/comment/68a18c59_25ada184?us… : PS1, Line 9: Recently Android Bionic LibC got sys/io.h. This does imply there will be minimum required version for Android support? As you are saying "recently" , so older versions before recently won't be supported anymore, is this correct? Which is the minimum Android version that has `sys/io.h` ? There isn't much documented about using flashrom on Android, but if we are restricting minimum version then we need to document it. https://review.coreboot.org/c/flashrom/+/83133/comment/ba5f962a_cf37c80f?us… : PS1, Line 12: Could you please add testing info to commit message? In this case, commands you ran with make and meson, and the environment (OS version specifically). Thank you! File hwaccess_x86_io.c: https://review.coreboot.org/c/flashrom/+/83133/comment/93716079_959efe4c?us… : PS1, Line 60: #if defined(__ANDROID__) How did you build flashrom, with make or meson? With this patch, you need to build with both (we will remove make later this year, but for now there is still both...) I looked through those build files. I think you need to change them too: 1) Makefile has `ifeq ($(TARGET_OS), Android)` (currently on line 324) which says `Android on x86 (currently) does not provide raw PCI port I/O operations.` This is not true anymore? 2) meson has definition for `systems_hwaccess` (currently on line 149) which has no Android on the list. Should it be there now? -- To view, visit https://review.coreboot.org/c/flashrom/+/83133?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Id80b83c2718679c925ed6ddfe33cbe837eea0429 Gerrit-Change-Number: 83133 Gerrit-PatchSet: 1 Gerrit-Owner: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 21 Jun 2024 07:27:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit June 2024