March 2022 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: ich_descriptors.c: Initialize structures
by Edward O'Callaghan (Code Review) 28 Mar '22

28 Mar '22

Attention is currently required from: Felix Singer, Nico Huber, Angel Pons, Light, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62763 ) Change subject: ich_descriptors.c: Initialize structures ...................................................................... Patch Set 10: Code-Review+2 (1 comment) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62763/comment/20011bf3_0843bc37 PS7, Line 1364: int ret = read_ich_descriptors_from_dump(dump, len, &cs, &desc); > It could be a false-positive (in a project where tools like scan-build find little […] Not sure if it actually covers up a bug Nico, more that it would fail predictably in the same manner if there was a bug rather than - happens to have zero in memory this time. I get what your thinking here but types having default values at instantiation I wouldn't say shadows bugs. However, one could argue read_ich_descriptors_from_dump() should memset to zero before filling in fields so that the domain of the function is always well defined in desc. Depending on how one wishes to define their conventions of responsibilities. -- To view, visit https://review.coreboot.org/c/flashrom/+/62763 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6e5bc84c6199a0731db0a9c8ef56f1215686dab2 Gerrit-Change-Number: 62763 Gerrit-PatchSet: 10 Gerrit-Owner: Light <aarya.chaumal(a)gmail.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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 28 Mar 2022 09:34:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Light <aarya.chaumal(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add Jasper Lake support
by Edward O'Callaghan (Code Review) 28 Mar '22

28 Mar '22

Attention is currently required from: Nico Huber, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62282 ) Change subject: ichspi: Add Jasper Lake support ...................................................................... Patch Set 6: (1 comment) Patchset: PS4: > No that was my fault, my dyslexic brain pattern matched the letters EHL to something else. […] Not sure what is actionable here Nico? The guess_ich_chipset_from_content() has now been implemented, there is subtle difference between them but not really much. -- 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: 6 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 28 Mar 2022 09:26:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: linux_mtd: Allow to open parition by name
by Arthur Heymans (Code Review) 28 Mar '22

28 Mar '22

Attention is currently required from: Patrick Rudolph. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63135 ) Change subject: linux_mtd: Allow to open parition by name ...................................................................... Patch Set 1: (1 comment) File linux_mtd.c: https://review.coreboot.org/c/flashrom/+/63135/comment/9ba955ec_05b8e860 PS1, Line 464: extract_programmer_param("name"); Would it make sense to use this path when strtol path is not taken so that no new parameter is needed? Otherwise you might want some logic to make "dev" and "name" mutually exclusive? -- To view, visit https://review.coreboot.org/c/flashrom/+/63135 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I566b9809950cbf47fbb01ae89305641c4e259f68 Gerrit-Change-Number: 63135 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Mon, 28 Mar 2022 08:48:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: linux_mtd: Allow to open parition by name
by Patrick Rudolph (Code Review) 28 Mar '22

28 Mar '22

Attention is currently required from: Patrick Rudolph. Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/63135 to review the following change. Change subject: linux_mtd: Allow to open parition by name ...................................................................... linux_mtd: Allow to open parition by name MTD device numbers might change, while the name keeps the same. Add new programmer parameter 'name', which is used when 'dev' isn't specified and try to locate a single MTD partition matching the name. If more than one partition is found it's assumed to be an error. Change-Id: I566b9809950cbf47fbb01ae89305641c4e259f68 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M flashrom.8.tmpl M linux_mtd.c 2 files changed, 86 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/35/63135/1 diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index fe2d9d6..6302c36 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -1182,6 +1182,14 @@ is the Linux device node for your MTD device. If left unspecified the first MTD device found (e.g. /dev/mtd0) will be used by default. .sp +You may specify the name of the MTD device to use with the +.sp +.B " flashrom \-p linux_mtd:name=Y" +.sp +syntax where +.B Y +is the unique name identifying your MTD device. +.sp Please note that the linux_mtd driver only works on Linux. .SS .BR "linux_spi " programmer diff --git a/linux_mtd.c b/linux_mtd.c index 9d80a51..d9c7c6b 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -15,6 +15,7 @@ */ #include <ctype.h> +#include <dirent.h> #include <errno.h> #include <fcntl.h> #include <stdio.h> @@ -370,6 +371,77 @@ return ret; } +static int linux_mtd_by_name(char *name, int *dev_num) +{ + char sysfs_path[32], device_name[32]; + struct dirent *dir; + char *n, *endptr; + int matches, dev; + DIR *d; + + *dev_num = -1; + matches = 0; + + d = opendir(LINUX_MTD_SYSFS_ROOT); + if (!d) { + msg_perr("Cannot access %s.\n", LINUX_MTD_SYSFS_ROOT); + return -1; + } + + while ((dir = readdir(d)) != NULL) { + n = dir->d_name; + + /* Must have mtdX format */ + if (strlen(n) < 4) + continue; + + /* Skip non mtd devices */ + if (memcmp(n, "mtd", 3) != 0) + continue; + + n += 3; + + /* Skip read only partitions */ + if (strlen(n) > 2 && strcmp(&n[strlen(n)- 2], "ro") == 0) + continue; + + /* Get the device number */ + dev = strtol(n, &endptr, 0); + if ((*endptr != '\0') || (dev < 0)) + continue; + + /* so far so good, get more info from other files in this dir */ + if (snprintf(sysfs_path, sizeof(sysfs_path), "%s/mtd%d/", LINUX_MTD_SYSFS_ROOT, dev) < 0) + continue; + + /* Device name */ + if (read_sysfs_string(sysfs_path, "name", device_name, sizeof(device_name))) + return 1; + + if (strcmp(device_name, name) == 0) { + msg_pdbg("Found %s on device MTD%d.\n", name, dev); + matches += 1; + if (matches > 1) + break; + *dev_num = dev; + } + } + closedir(d); + + if (matches == 0) { + msg_perr("Invalid device name %s. Use flashrom -p " + "linux_mtd:name=N where N is a valid MTD\n" + "device name.\n", name); + return -1; + } else if (matches > 1) { + msg_perr("Duplicated device name %s. Consider using " + "device numbers instead.\n", name); + return -1; + } + + return 0; +} + static int linux_mtd_init(void) { char *param; @@ -388,6 +460,12 @@ "device number.\n", param); goto linux_mtd_init_exit; } + } else { + param = extract_programmer_param("name"); + if (param) { + if (linux_mtd_by_name(param, &dev_num)) + goto linux_mtd_init_exit; + } } /* -- To view, visit https://review.coreboot.org/c/flashrom/+/63135 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I566b9809950cbf47fbb01ae89305641c4e259f68 Gerrit-Change-Number: 63135 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ichspi: Add Jasper Lake support
by Edward O'Callaghan (Code Review) 28 Mar '22

28 Mar '22

Attention is currently required from: Nico Huber, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62282 ) Change subject: ichspi: Add Jasper Lake support ...................................................................... Patch Set 6: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62282/comment/cc0a8317_3c9aec7b PS4, Line 10: TEST=dedede with `flashrom -p internal --flash-size`. > Thanks Angel, a rw was tested to validate it doesn't regress the chromeos fw update mechanism. […] Done File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62282/comment/6fe33345_11e7adc9 PS4, Line 1064: case CHIPSET_JASPER_LAKE: > > Just FYI Angel, I cannot control how public the data sheets are however I can have Intel collabora […] 😄 well the documentation pretty much wasn't great for this gen but I finally got some time to figure out the value of CSSO to branch off to get the detection logic working so guess would work. -- 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: 6 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 28 Mar 2022 07:03:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add Jasper Lake support
by Edward O'Callaghan (Code Review) 28 Mar '22

28 Mar '22

Attention is currently required from: Nico Huber. Hello build bot (Jenkins), Subrata Banik, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62282 to look at the new patch set (#6). Change subject: ichspi: Add Jasper Lake support ...................................................................... ichspi: Add Jasper Lake support Additionally, utilize CSSO (CPU Soft Strap Offset) to uniquely detect the chipset when the CSSL (CPU Soft Strap Length) field default value (0x03) on Jasper Lake is the same as Elkhart Lake. BUG=b:221175960 TEST=dedede with `flashrom -p internal --flash-size`. ``` $ flashrom -VVV -p internal --ifd -i fd -i bios -r /tmp/filename.rom <snip> Enabling hardware sequencing by default for 100+ series PCH. OK. No board enable found matching coreboot IDs vendor="Google", model="Magolor". The following protocols are supported: Programmer-specific. Probing for Programmer Opaque flash chip, 0 kB: Chip identified: GD25Q127C/GD25Q128C Hardware sequencing reports 1 attached SPI flash chip with a density of 16384 kB. There is only one partition containing the whole address space (0x000000 - 0xffffff). There are 4096 erase blocks with 4096 B each. Added layout entry 00000000 - 00ffffff named complete flash Found GigaDevice flash chip "GD25Q127C/GD25Q128C" (16384 kB, Programmer-specific) mapped at physical address 0x0000000000000000. Found GigaDevice flash chip "GD25Q127C/GD25Q128C" (16384 kB, Programmer-specific). This chip may contain one-time programmable memory. flashrom cannot read and may never be able to write it, hence it may not be able to completely clone the contents of this chip (see man page for details). Reading Status register Block protection is disabled. Reading ich descriptor... Reading 4096 bytes starting at 0x000000. done. Assuming chipset 'Jasper Lake'. Added layout entry 00000000 - 00000fff named fd Added layout entry 00381000 - 00ffffff named bios Added layout entry 00001000 - 00380fff named me restore_power_management: Re-enabling power management. Using regions: "bios", "fd". Reading Status register Block protection is disabled. Reading flash... 0x381000-0xffffff:R Reading 13103104 bytes starting at 0x381000. 000000-0x0fff:R Reading 4096 bytes starting at 0x000000. done. restore_power_management: Re-enabling power management. SUCCESS Restoring PCI config space for 00:1f:5 reg 0xdc restore_power_management: Re-enabling power management. ``` Change-Id: Ib942d0b8942fe0a991b2af0b187414818485153d Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M chipset_enable.c M ich_descriptors.c M ichspi.c M programmer.h M util/ich_descriptors_tool/ich_descriptors_tool.c 5 files changed, 37 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/82/62282/6 -- 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: 6 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: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ichspi: Add support for Meteor Lake
by Edward O'Callaghan (Code Review) 28 Mar '22

28 Mar '22

Attention is currently required from: Wonkyu Kim, Subrata Banik, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62783 ) Change subject: ichspi: Add support for Meteor Lake ...................................................................... Patch Set 8: (1 comment) Patchset: PS8: > Can someone help to merge this CL? I'll merge in a few days unless there are any further objections. -- To view, visit https://review.coreboot.org/c/flashrom/+/62783 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a2ffe2ba8d96c90d89b77e0d8583d179ff02a75 Gerrit-Change-Number: 62783 Gerrit-PatchSet: 8 Gerrit-Owner: Subrata Banik <subratabanik(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: Wonkyu Kim Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Wonkyu Kim Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 28 Mar 2022 04:40:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.8.tmpl: document lspcon_i2c_spi
by Nikolai Artemiev (Code Review) 28 Mar '22

28 Mar '22

Attention is currently required from: Felix Singer, Anastasia Klimchuk, Peter Marheine. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63104 ) Change subject: flashrom.8.tmpl: document lspcon_i2c_spi ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/63104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9ccb9694fdea29e68f062cc049efc0204917a139 Gerrit-Change-Number: 63104 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(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-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Mon, 28 Mar 2022 03:03:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.8.tmpl: document realtek_mst_i2c_spi
by Nikolai Artemiev (Code Review) 28 Mar '22

28 Mar '22

Attention is currently required from: Felix Singer, Anastasia Klimchuk, Peter Marheine. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63103 ) Change subject: flashrom.8.tmpl: document realtek_mst_i2c_spi ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/63103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idde5a8de014fe84c4a472f8fbfd3562350997d39 Gerrit-Change-Number: 63103 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(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-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Mon, 28 Mar 2022 03:00:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: pony_spi.c: Extract out get_params to simplify init
by Edward O'Callaghan (Code Review) 28 Mar '22

28 Mar '22

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/63130 ) Change subject: pony_spi.c: Extract out get_params to simplify init ...................................................................... pony_spi.c: Extract out get_params to simplify init In light of `commit caa0335114a81`, extract out the get_param logic to its own function to simplify the number of cleanup paths. BUG=none TEST=builds Change-Id: I364febc05c870683cbad114583762b0c006f4bac Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M pony_spi.c 1 file changed, 48 insertions(+), 40 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/30/63130/1 diff --git a/pony_spi.c b/pony_spi.c index a3ac274..2285c64 100644 --- a/pony_spi.c +++ b/pony_spi.c @@ -120,13 +120,51 @@ return ret; } +static int get_params(enum pony_type *type, bool *have_device) +{ + char *arg = NULL; + int ret = 0; + + /* defaults */ + *type = TYPE_SI_PROG; + *have_device = false; + + /* The parameter is in format "dev=/dev/device,type=serbang" */ + arg = extract_programmer_param("dev"); + if (arg && strlen(arg)) { + sp_fd = sp_openserport(arg, 9600); + if (sp_fd == SER_INV_FD) + ret = 1; + else + *have_device = true; + } + free(arg); + + arg = extract_programmer_param("type"); + if (arg && !strcasecmp(arg, "serbang")) { + *type = TYPE_SERBANG; + } else if (arg && !strcasecmp(arg, "si_prog")) { + *type = TYPE_SI_PROG; + } else if (arg && !strcasecmp( arg, "ajawe")) { + *type = TYPE_AJAWE; + } else if (arg && !strlen(arg)) { + msg_perr("Error: Missing argument for programmer type.\n"); + ret = 1; + } else if (arg){ + msg_perr("Error: Invalid programmer type specified.\n"); + ret = 1; + } + free(arg); + + return ret; +} + static int pony_spi_init(void) { int i, data_out; - char *arg = NULL; - enum pony_type type = TYPE_SI_PROG; + enum pony_type type; const char *name; - int have_device = 0; + bool have_device; int have_prog = 0; struct pony_spi_data *data = calloc(1, sizeof(*data)); @@ -139,49 +177,19 @@ data->negate_mosi = 0; data->negate_miso = 0; - /* The parameter is in format "dev=/dev/device,type=serbang" */ - arg = extract_programmer_param("dev"); - if (arg && strlen(arg)) { - sp_fd = sp_openserport(arg, 9600); - if (sp_fd == SER_INV_FD) { - free(arg); - free(data); - return 1; - } - if (register_shutdown(pony_spi_shutdown, data) != 0) { - free(arg); - free(data); - serialport_shutdown(NULL); - return 1; - } - have_device++; - } - free(arg); - + get_params(&type, &have_device); if (!have_device) { msg_perr("Error: No valid device specified.\n" "Use flashrom -p pony_spi:dev=/dev/device[,type=name]\n"); - free(data); return 1; } - arg = extract_programmer_param("type"); - if (arg && !strcasecmp(arg, "serbang")) { - type = TYPE_SERBANG; - } else if (arg && !strcasecmp(arg, "si_prog")) { - type = TYPE_SI_PROG; - } else if (arg && !strcasecmp( arg, "ajawe")) { - type = TYPE_AJAWE; - } else if (arg && !strlen(arg)) { - msg_perr("Error: Missing argument for programmer type.\n"); - free(arg); - return 1; - } else if (arg){ - msg_perr("Error: Invalid programmer type specified.\n"); - free(arg); + + if (register_shutdown(pony_spi_shutdown, data) != 0) { + free(data); + serialport_shutdown(NULL); return 1; } - free(arg); /* * Configure the serial port pins, depending on the used programmer. @@ -221,12 +229,12 @@ switch (type) { case TYPE_AJAWE: - have_prog = 1; + have_prog = true; break; case TYPE_SI_PROG: case TYPE_SERBANG: default: - have_prog = 1; + have_prog = true; /* We toggle RTS/SCK a few times and see if DSR changes too. */ for (i = 1; i <= 10; i++) { data_out = i & 1; -- To view, visit https://review.coreboot.org/c/flashrom/+/63130 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I364febc05c870683cbad114583762b0c006f4bac Gerrit-Change-Number: 63130 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2022