July 2018 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: usbdev: Extract libusb1 device discovery into a seperate file
by Paul Menzel (Code Review) 12 Jul '18

12 Jul '18

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/27443 ) Change subject: usbdev: Extract libusb1 device discovery into a seperate file ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/27443 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idfcc79371241c2c1dea97faf5e532aa971546a79 Gerrit-Change-Number: 27443 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 12 Jul 2018 15:46:55 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: programmer: Add Developerbox/CP2104 bit bang driver
by Daniel Thompson (Code Review) 12 Jul '18

12 Jul '18

Daniel Thompson has posted comments on this change. ( https://review.coreboot.org/26948 ) Change subject: programmer: Add Developerbox/CP2104 bit bang driver ...................................................................... Patch Set 3: Sorry for the extra patch here. I didn't commit between acting on review comments and factoring out the device discovery code... meaning it ended up looking like I was ignoring review comments. -- To view, visit https://review.coreboot.org/26948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2547a96c1a2259ad0d52cd4b6ef42261b37cccf3 Gerrit-Change-Number: 26948 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Thu, 12 Jul 2018 11:27:03 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in flashrom[master]: usbdev: Refactor device discovery code
by Daniel Thompson (Code Review) 12 Jul '18

12 Jul '18

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/27444 to look at the new patch set (#2). Change subject: usbdev: Refactor device discovery code ...................................................................... usbdev: Refactor device discovery code Currently there is a lot of code shared between usb_dev_get_by_vid_pid_serial() and usb_dev_get_by_vid_pid_number(). Fix this by pulling out the conditional filtering at the heart of each loop and calling it via a function pointer. I haven't got (two) dediprog programmers to test with but I have tested both by...serial() and by...number() calls using a pair of Developerboxen and a hacked driver. Change-Id: I31ed572501e4314b9455e1b70a5e934ec96408b1 Signed-off-by: Daniel Thompson <daniel.thompson(a)linaro.org> --- M usbdev.c 1 file changed, 57 insertions(+), 58 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/27444/2 -- To view, visit https://review.coreboot.org/27444 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I31ed572501e4314b9455e1b70a5e934ec96408b1 Gerrit-Change-Number: 27444 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: usbdev: Extract libusb1 device discovery into a seperate file
by Daniel Thompson (Code Review) 12 Jul '18

12 Jul '18

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/27443 to look at the new patch set (#2). Change subject: usbdev: Extract libusb1 device discovery into a seperate file ...................................................................... usbdev: Extract libusb1 device discovery into a seperate file Currently there is a TODO-like comment in the dediprog driver: "Might be useful for other USB devices as well". Act on this comment by collecting all the device discovery code for libusb1 devices into a seperate file. Change-Id: Idfcc79371241c2c1dea97faf5e532aa971546a79 Signed-off-by: Daniel Thompson <daniel.thompson(a)linaro.org> --- M Makefile M dediprog.c M developerbox_spi.c M programmer.h A usbdev.c 5 files changed, 143 insertions(+), 108 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/27443/2 -- To view, visit https://review.coreboot.org/27443 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idfcc79371241c2c1dea97faf5e532aa971546a79 Gerrit-Change-Number: 27443 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: programmer: Add Developerbox/CP2104 bit bang driver
by Daniel Thompson (Code Review) 12 Jul '18

12 Jul '18

Hello Paul Menzel, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/26948 to look at the new patch set (#5). Change subject: programmer: Add Developerbox/CP2104 bit bang driver ...................................................................... programmer: Add Developerbox/CP2104 bit bang driver The 96Boards Developerbox (a.k.a. Synquacer E-series) provides a CP2102 debug UART with its GPIO pins hooked up to the SPI NOR FLASH. The circuit is intended to provide emergency recovery functions without requiring any additional tools (such as a JTAG or SPI programmer). This was expected to be very slow (and it is) but CP2102 is much cheaper than a full dual channel USB comms chip. Read performance is roughly on par with a 2400 baud modem (between 60 and 70 minutes per megabyte if you prefer) and write performance is 50% slower still. The full recovery process, with backup and verification of 4MB data written takes between 14 and 15 hours. Thus it is only really practical as an emergency recovery tool, firmware developers will need to use an alternative programmer. Change-Id: I2547a96c1a2259ad0d52cd4b6ef42261b37cccf3 Signed-off-by: Daniel Thompson <daniel.thompson(a)linaro.org> --- M Makefile A developerbox_spi.c M flashrom.c M programmer.h 4 files changed, 278 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/48/26948/5 -- To view, visit https://review.coreboot.org/26948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2547a96c1a2259ad0d52cd4b6ef42261b37cccf3 Gerrit-Change-Number: 26948 Gerrit-PatchSet: 5 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[master]: usbdev: Refactor device discovery code
by Daniel Thompson (Code Review) 12 Jul '18

12 Jul '18

Daniel Thompson has uploaded this change for review. ( https://review.coreboot.org/27444 Change subject: usbdev: Refactor device discovery code ...................................................................... usbdev: Refactor device discovery code Currently there is a lot of code shared between usb_dev_get_by_vid_pid_serial() and usb_dev_get_by_vid_pid_number(). Fix this by pulling out the conditional filtering at the heart of each loop and calling it via a function pointer. I haven't got (two) dediprog programmers to test with but I have tested both by...serial() and by...number() calls using a pair of Developerboxen and a hacked driver. Change-Id: I31ed572501e4314b9455e1b70a5e934ec96408b1 Signed-off-by: Daniel Thompson <daniel.thompson(a)linaro.org> --- M usbdev.c 1 file changed, 57 insertions(+), 58 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/27444/1 diff --git a/usbdev.c b/usbdev.c index fb26cbb..3282b93 100644 --- a/usbdev.c +++ b/usbdev.c @@ -17,12 +17,15 @@ #include "platform.h" +#include <stdbool.h> #include <string.h> #include <libusb.h> #include "programmer.h" -struct libusb_device_handle *usb_dev_get_by_vid_pid_serial( - struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, const char *serialno) +typedef bool (*filter_t)(struct libusb_device_descriptor *desc, struct libusb_device_handle *handle, void*ctx); + +static struct libusb_device_handle *get_by_vid_pid_filter(struct libusb_context *usb_ctx, + uint16_t vid, uint16_t pid, filter_t filter_fn, void *filter_ctx) { struct libusb_device **list; ssize_t count = libusb_get_device_list(usb_ctx, &list); @@ -50,28 +53,22 @@ desc.idVendor, desc.idProduct, libusb_get_bus_number(dev), libusb_get_device_address(dev)); + /* allow filters to trigger before the device is opened */ + if (filter_fn(&desc, NULL, filter_ctx)) + continue; + res = libusb_open(dev, &handle); if (res != 0) { - msg_perr("Opening the USB device failed (%s)!\n", libusb_error_name(res)); - continue; + msg_perr("Opening the USB device at address %d-%d failed (%s)!\n", + libusb_get_bus_number(dev), libusb_get_device_address(dev), + libusb_error_name(res)); + break; } - if (serialno) { - unsigned char myserial[64]; - res = libusb_get_string_descriptor_ascii(handle, desc.iSerialNumber, myserial, - sizeof(myserial)); - if (res < 0) { - msg_perr("Reading the USB serialno failed (%s)!\n", libusb_error_name(res)); - libusb_close(handle); - continue; - } - msg_pdbg("Serial number is %s\n", myserial); - - /* Reject any serial number that does not commence with serialno */ - if (0 != strncmp(serialno, (char *)myserial, strlen(serialno))) { - libusb_close(handle); - continue; - } + /* filter can also trigger after a device is opened */ + if (filter_fn(&desc, handle, filter_ctx)) { + libusb_close(handle); + continue; } libusb_free_device_list(list, 1); @@ -80,6 +77,45 @@ libusb_free_device_list(list, 1); return NULL; + +} + +static bool filter_by_serial(struct libusb_device_descriptor *desc, struct libusb_device_handle *handle, + void *serialno) +{ + /* never filter if device is not yet open or when user did not provide a serial number */ + if (!handle || !serialno) + return false; + + unsigned char myserial[64]; + int res = libusb_get_string_descriptor_ascii(handle, desc->iSerialNumber, myserial, sizeof(myserial)); + if (res < 0) { + msg_perr("Reading the USB serialno failed (%s)!\n", libusb_error_name(res)); + return true; + } + msg_pdbg("Serial number is %s\n", myserial); + + /* Filter out any serial number that does not commence with serialno */ + return 0 != strncmp(serialno, (char *)myserial, strlen(serialno)); +} + +static bool filter_by_number(struct libusb_device_descriptor *desc, struct libusb_device_handle *handle, + void *ctx) +{ + /* no need to NULL check handle; the conditional decrement makes it unnecessary */ + unsigned int *nump = ctx; + if (*nump) { + (*nump)--; + return true; + } + + return false; +} + +struct libusb_device_handle *usb_dev_get_by_vid_pid_serial( + struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, const char *serialno) +{ + return get_by_vid_pid_filter(usb_ctx, vid, pid, filter_by_serial, (void *) serialno); } /* @@ -91,42 +127,5 @@ struct libusb_device_handle *usb_dev_get_by_vid_pid_number( struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, unsigned int num) { - struct libusb_device **list; - ssize_t count = libusb_get_device_list(usb_ctx, &list); - if (count < 0) { - msg_perr("Getting the USB device list failed (%s)!\n", libusb_error_name(count)); - return NULL; - } - - struct libusb_device_handle *handle = NULL; - ssize_t i = 0; - for (i = 0; i < count; i++) { - struct libusb_device *dev = list[i]; - struct libusb_device_descriptor desc; - int err = libusb_get_device_descriptor(dev, &desc); - if (err != 0) { - msg_perr("Reading the USB device descriptor failed (%s)!\n", libusb_error_name(err)); - libusb_free_device_list(list, 1); - return NULL; - } - if ((desc.idVendor == vid) && (desc.idProduct == pid)) { - msg_pdbg("Found USB device %04"PRIx16":%04"PRIx16" at address %d-%d.\n", - desc.idVendor, desc.idProduct, - libusb_get_bus_number(dev), libusb_get_device_address(dev)); - if (num == 0) { - err = libusb_open(dev, &handle); - if (err != 0) { - msg_perr("Opening the USB device failed (%s)!\n", - libusb_error_name(err)); - libusb_free_device_list(list, 1); - return NULL; - } - break; - } - num--; - } - } - libusb_free_device_list(list, 1); - - return handle; + return get_by_vid_pid_filter(usb_ctx, vid, pid, filter_by_number, &num); } -- To view, visit https://review.coreboot.org/27444 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I31ed572501e4314b9455e1b70a5e934ec96408b1 Gerrit-Change-Number: 27444 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org>

1 0

Change in flashrom[master]: usbdev: Extract libusb1 device discovery into a seperate file
by Daniel Thompson (Code Review) 12 Jul '18

12 Jul '18

Daniel Thompson has uploaded this change for review. ( https://review.coreboot.org/27443 Change subject: usbdev: Extract libusb1 device discovery into a seperate file ...................................................................... usbdev: Extract libusb1 device discovery into a seperate file Currently there is a TODO-like comment in the dediprog driver: "Might be useful for other USB devices as well". Act on this comment by collecting all the device discovery code for libusb1 devices into a seperate file. Change-Id: Idfcc79371241c2c1dea97faf5e532aa971546a79 Signed-off-by: Daniel Thompson <daniel.thompson(a)linaro.org> --- M Makefile M dediprog.c M developerbox_spi.c M programmer.h A usbdev.c 5 files changed, 143 insertions(+), 105 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/27443/1 diff --git a/Makefile b/Makefile index ddfd711..b13cf7e 100644 --- a/Makefile +++ b/Makefile @@ -1023,6 +1023,7 @@ ifneq ($(NEED_LIBUSB1), ) CHECK_LIBUSB1 = yes FEATURE_CFLAGS += -D'NEED_LIBUSB1=1' +PROGRAMMER_OBJS += usbdev.o # FreeBSD and DragonflyBSD use a reimplementation of libusb-1.0 that is simply called libusb ifeq ($(TARGET_OS),$(filter $(TARGET_OS),FreeBSD DragonFlyBSD)) USB1LIBS += -lusb diff --git a/dediprog.c b/dediprog.c index 7fcadfb..1a469a7 100644 --- a/dediprog.c +++ b/dediprog.c @@ -242,50 +242,6 @@ } -/* Might be useful for other USB devices as well. static for now. - * num parameter allows user to specify one device of multiple installed */ -static struct libusb_device_handle *get_device_by_vid_pid_number(uint16_t vid, uint16_t pid, unsigned int num) -{ - struct libusb_device **list; - ssize_t count = libusb_get_device_list(usb_ctx, &list); - if (count < 0) { - msg_perr("Getting the USB device list failed (%s)!\n", libusb_error_name(count)); - return NULL; - } - - struct libusb_device_handle *handle = NULL; - ssize_t i = 0; - for (i = 0; i < count; i++) { - struct libusb_device *dev = list[i]; - struct libusb_device_descriptor desc; - int err = libusb_get_device_descriptor(dev, &desc); - if (err != 0) { - msg_perr("Reading the USB device descriptor failed (%s)!\n", libusb_error_name(err)); - libusb_free_device_list(list, 1); - return NULL; - } - if ((desc.idVendor == vid) && (desc.idProduct == pid)) { - msg_pdbg("Found USB device %04"PRIx16":%04"PRIx16" at address %d-%d.\n", - desc.idVendor, desc.idProduct, - libusb_get_bus_number(dev), libusb_get_device_address(dev)); - if (num == 0) { - err = libusb_open(dev, &handle); - if (err != 0) { - msg_perr("Opening the USB device failed (%s)!\n", - libusb_error_name(err)); - libusb_free_device_list(list, 1); - return NULL; - } - break; - } - num--; - } - } - libusb_free_device_list(list, 1); - - return handle; -} - /* This function sets the GPIOs connected to the LEDs as well as IO1-IO4. */ static int dediprog_set_leds(int leds) { @@ -1102,7 +1058,7 @@ const uint16_t vid = devs_dediprog[0].vendor_id; const uint16_t pid = devs_dediprog[0].device_id; - dediprog_handle = get_device_by_vid_pid_number(vid, pid, (unsigned int) usedevice); + dediprog_handle = usb_dev_get_by_vid_pid_number(usb_ctx, vid, pid, (unsigned int) usedevice); if (!dediprog_handle) { msg_perr("Could not find a Dediprog programmer on USB.\n"); libusb_exit(usb_ctx); diff --git a/developerbox_spi.c b/developerbox_spi.c index 4f3ba1f..4ad966e 100644 --- a/developerbox_spi.c +++ b/developerbox_spi.c @@ -34,7 +34,6 @@ #include "platform.h" #include <stdlib.h> -#include <string.h> #include <libusb.h> #include "programmer.h" #include "spi.h" @@ -130,64 +129,6 @@ .set_sck_set_mosi = cp210x_bitbang_set_sck_set_mosi, }; -static struct libusb_device_handle *get_device_by_vid_pid_serial(uint16_t vid, uint16_t pid, char *serialno) -{ - struct libusb_device **list; - ssize_t count = libusb_get_device_list(usb_ctx, &list); - if (count < 0) { - msg_perr("Getting the USB device list failed (%s)!\n", libusb_error_name(count)); - return NULL; - } - - ssize_t i = 0; - for (i = 0; i < count; i++) { - struct libusb_device *dev = list[i]; - struct libusb_device_descriptor desc; - struct libusb_device_handle *handle; - - int res = libusb_get_device_descriptor(dev, &desc); - if (res != 0) { - msg_perr("Reading the USB device descriptor failed (%s)!\n", libusb_error_name(res)); - continue; - } - - if ((desc.idVendor != vid) && (desc.idProduct != pid)) - continue; - - msg_pdbg("Found USB device %04"PRIx16":%04"PRIx16" at address %d-%d.\n", - desc.idVendor, desc.idProduct, - libusb_get_bus_number(dev), libusb_get_device_address(dev)); - - res = libusb_open(dev, &handle); - if (res != 0) { - msg_perr("Opening the USB device failed (%s)!\n", libusb_error_name(res)); - continue; - } - - if (serialno) { - unsigned char myserial[64]; - res = libusb_get_string_descriptor_ascii(handle, desc.iSerialNumber, myserial, 64); - if (res < 0) { - msg_perr("Reading the USB serialno failed (%s)!\n", libusb_error_name(res)); - libusb_close(handle); - continue; - } - msg_pdbg("Serial number is %s\n", myserial); - - if (0 != strncmp(serialno, (char *) myserial, strlen(serialno))) { - libusb_close(handle); - continue; - } - } - - libusb_free_device_list(list, 1); - return handle; - } - - libusb_free_device_list(list, 1); - return NULL; -} - static int developerbox_spi_shutdown(void *data) { libusb_close(cp210x_handle); @@ -207,7 +148,7 @@ char *serialno = extract_programmer_param("serial"); if (serialno) msg_pdbg("Looking for serial number commencing %s\n", serialno); - cp210x_handle = get_device_by_vid_pid_serial( + cp210x_handle = usb_dev_get_by_vid_pid_serial(usb_ctx, devs_developerbox_spi[0].vendor_id, devs_developerbox_spi[0].device_id, serialno); free(serialno); if (!cp210x_handle) { diff --git a/programmer.h b/programmer.h index 300cf5f..311992a 100644 --- a/programmer.h +++ b/programmer.h @@ -841,4 +841,12 @@ flash->mst->spi.features & SPI_MASTER_4BA; } +/* usbdev.c */ +struct libusb_device_handle; +struct libusb_context; +struct libusb_device_handle *usb_dev_get_by_vid_pid_serial( + struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, const char *serialno); +struct libusb_device_handle *usb_dev_get_by_vid_pid_number( + struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, unsigned int num); + #endif /* !__PROGRAMMER_H__ */ diff --git a/usbdev.c b/usbdev.c new file mode 100644 index 0000000..fb26cbb --- /dev/null +++ b/usbdev.c @@ -0,0 +1,132 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2016 secunet Security Networks AG + * Copyright (C) 2018 Linaro Limited + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include "platform.h" + +#include <string.h> +#include <libusb.h> +#include "programmer.h" + +struct libusb_device_handle *usb_dev_get_by_vid_pid_serial( + struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, const char *serialno) +{ + struct libusb_device **list; + ssize_t count = libusb_get_device_list(usb_ctx, &list); + if (count < 0) { + msg_perr("Getting the USB device list failed (%s)!\n", libusb_error_name(count)); + return NULL; + } + + ssize_t i = 0; + for (i = 0; i < count; i++) { + struct libusb_device *dev = list[i]; + struct libusb_device_descriptor desc; + struct libusb_device_handle *handle; + + int res = libusb_get_device_descriptor(dev, &desc); + if (res != 0) { + msg_perr("Reading the USB device descriptor failed (%s)!\n", libusb_error_name(res)); + continue; + } + + if ((desc.idVendor != vid) && (desc.idProduct != pid)) + continue; + + msg_pdbg("Found USB device %04"PRIx16":%04"PRIx16" at address %d-%d.\n", + desc.idVendor, desc.idProduct, + libusb_get_bus_number(dev), libusb_get_device_address(dev)); + + res = libusb_open(dev, &handle); + if (res != 0) { + msg_perr("Opening the USB device failed (%s)!\n", libusb_error_name(res)); + continue; + } + + if (serialno) { + unsigned char myserial[64]; + res = libusb_get_string_descriptor_ascii(handle, desc.iSerialNumber, myserial, + sizeof(myserial)); + if (res < 0) { + msg_perr("Reading the USB serialno failed (%s)!\n", libusb_error_name(res)); + libusb_close(handle); + continue; + } + msg_pdbg("Serial number is %s\n", myserial); + + /* Reject any serial number that does not commence with serialno */ + if (0 != strncmp(serialno, (char *)myserial, strlen(serialno))) { + libusb_close(handle); + continue; + } + } + + libusb_free_device_list(list, 1); + return handle; + } + + libusb_free_device_list(list, 1); + return NULL; +} + +/* + * This function allows different devices to be targeted based on enumeration order. Different + * hotplug sequencing (or simply a reboot) may change the enumeration order. This function should + * only be used if a programmers does not provide an alternative way to identify itself uniquely + * (such as a unique serial number). + */ +struct libusb_device_handle *usb_dev_get_by_vid_pid_number( + struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, unsigned int num) +{ + struct libusb_device **list; + ssize_t count = libusb_get_device_list(usb_ctx, &list); + if (count < 0) { + msg_perr("Getting the USB device list failed (%s)!\n", libusb_error_name(count)); + return NULL; + } + + struct libusb_device_handle *handle = NULL; + ssize_t i = 0; + for (i = 0; i < count; i++) { + struct libusb_device *dev = list[i]; + struct libusb_device_descriptor desc; + int err = libusb_get_device_descriptor(dev, &desc); + if (err != 0) { + msg_perr("Reading the USB device descriptor failed (%s)!\n", libusb_error_name(err)); + libusb_free_device_list(list, 1); + return NULL; + } + if ((desc.idVendor == vid) && (desc.idProduct == pid)) { + msg_pdbg("Found USB device %04"PRIx16":%04"PRIx16" at address %d-%d.\n", + desc.idVendor, desc.idProduct, + libusb_get_bus_number(dev), libusb_get_device_address(dev)); + if (num == 0) { + err = libusb_open(dev, &handle); + if (err != 0) { + msg_perr("Opening the USB device failed (%s)!\n", + libusb_error_name(err)); + libusb_free_device_list(list, 1); + return NULL; + } + break; + } + num--; + } + } + libusb_free_device_list(list, 1); + + return handle; +} -- To view, visit https://review.coreboot.org/27443 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Idfcc79371241c2c1dea97faf5e532aa971546a79 Gerrit-Change-Number: 27443 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org>

1 0

Change in flashrom[master]: programmer: Add Developerbox/CP2104 bit bang driver
by Daniel Thompson (Code Review) 12 Jul '18

12 Jul '18

Hello Paul Menzel, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/26948 to look at the new patch set (#4). Change subject: programmer: Add Developerbox/CP2104 bit bang driver ...................................................................... programmer: Add Developerbox/CP2104 bit bang driver The 96Boards Developerbox (a.k.a. Synquacer E-series) provides a CP2102 debug UART with its GPIO pins hooked up to the SPI NOR FLASH. The circuit is intended to provide emergency recovery functions without requiring any additional tools (such as a JTAG or SPI programmer). This was expected to be very slow (and it is) but CP2102 is much cheaper than a full dual channel USB comms chip. Read performance is roughly on par with a 2400 baud modem (between 60 and 70 minutes per megabyte if you prefer) and write performance is 50% slower still. The full recovery process, with backup and verification of 4MB data written takes between 14 and 15 hours. Thus it is only really practical as an emergency recovery tool, firmware developers will need to use an alternative programmer. Change-Id: I2547a96c1a2259ad0d52cd4b6ef42261b37cccf3 Signed-off-by: Daniel Thompson <daniel.thompson(a)linaro.org> --- M Makefile A developerbox_spi.c M flashrom.c M programmer.h 4 files changed, 275 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/48/26948/4 -- To view, visit https://review.coreboot.org/26948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2547a96c1a2259ad0d52cd4b6ef42261b37cccf3 Gerrit-Change-Number: 26948 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[master]: programmer: Add Developerbox/CP2104 bit bang driver
by Daniel Thompson (Code Review) 11 Jul '18

11 Jul '18

Daniel Thompson has posted comments on this change. ( https://review.coreboot.org/26948 ) Change subject: programmer: Add Developerbox/CP2104 bit bang driver ...................................................................... Patch Set 3: (4 comments) You can assume for now that not replying to a specific bit of feedback means "will fix"... https://review.coreboot.org/#/c/26948/3/developerbox_spi.c File developerbox_spi.c: https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@8 PS3, Line 8: version 2 of the License > is it intentional to restrict the license to v2? This would have been copied from the existing code. I just got "unlucky". Across the code base it looks like there is currently an approximate 60/40 split in favour or "or later". I'm happy to change this. https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@28 PS3, Line 28: DSW4-1 > DSW4-1 is the most significant bit? The presentation here is more geometric than numeric! DW4-1 is left most switch in the DIP panel (with "up" defined by the text on the silkscreen). I think this phrasing is unambiguous for someone who can see the board. https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@181 PS3, Line 181: strlen(serialno) > strncmp() with `n == strlen()` of one of its arguments looks very odd. […] I'll add a comment. The intent is to permit a partial prefix (developers influences how git handles partial SHA-1 hashes grow to expect this). https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@207 PS3, Line 207: commencing > heh, should have read this first... :-) I'll add a comment anyway. -- To view, visit https://review.coreboot.org/26948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2547a96c1a2259ad0d52cd4b6ef42261b37cccf3 Gerrit-Change-Number: 26948 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Wed, 11 Jul 2018 16:32:39 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: programmer: Add Developerbox/CP2104 bit bang driver
by Nico Huber (Code Review) 11 Jul '18

11 Jul '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/26948 ) Change subject: programmer: Add Developerbox/CP2104 bit bang driver ...................................................................... Patch Set 3: (16 comments) https://review.coreboot.org/#/c/26948/3/developerbox_spi.c File developerbox_spi.c: PS3: Hex literals are sometimes written in upper case, sometimes in lower case, please unify (personally, I would prefer all lower case). https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@8 PS3, Line 8: version 2 of the License is it intentional to restrict the license to v2? https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@16 PS3, Line 16: /* Bit bang driver for the 96Boards Developerbox (a.k.a. Synquacer E-series) nit, please start comment with /* on an otherwise empty line https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@18 PS3, Line 18: will which? https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@27 PS3, Line 27: UART DSW4 nit, maybe place a comma between `UART, DSW4` https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@28 PS3, Line 28: DSW4-1 DSW4-1 is the most significant bit? https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@29 PS3, Line 29: * turned on) nit, missing full-stop https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@34 PS3, Line 34: #include <sys/types.h> : #include <stdlib.h> : #include <stdio.h> : #include <string.h> : #include <limits.h> : #include <errno.h> : #include <libusb.h> : #include "flash.h" : #include "chipdrivers.h" : #include "programmer.h" : #include "spi.h" The list seems excessive (for the little code below), are these all needed? https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@137 PS3, Line 137: static struct libusb_device_handle *get_device_by_vid_pid_serial(uint16_t vid, uint16_t pid, char *serialno) We should make this more generic and generally available instead of copying the code around. I would prefer that to happen before this gets in, but it doesn't have to. Btw. if this function is the reason for the GPL v2 only: I wrote it, and would sign-off any patch that makes it GPL v2+. https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@137 PS3, Line 137: char * `const char *` please https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@173 PS3, Line 173: 64 sizeof(myserial) https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@181 PS3, Line 181: strlen(serialno) strncmp() with `n == strlen()` of one of its arguments looks very odd. If this is to compensate for a missing `\0` termination in `myserial`, please use memcmp() instead. Hmm, but it seems not, the msg_pdbg() would already fail. If you are just comparing two C strings, just use strcmp(). What it effectively does here is allowing `serialno` to be a prefix of `myserial`. If that is intentional, please add a comment (I've seen this pattern very often and it was almost always not intentional). Now I've also looked into libusb, they always add the terminating `\0`. Wonder, why they don't mention that in the documentation... https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@181 PS3, Line 181: (char *) Nit, no space after the closing parenthesis, please. https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@207 PS3, Line 207: commencing heh, should have read this first... https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@227 PS3, Line 227: return 1; libusb_exit(...) https://review.coreboot.org/#/c/26948/3/developerbox_spi.c@232 PS3, Line 232: return 1; same here -- To view, visit https://review.coreboot.org/26948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2547a96c1a2259ad0d52cd4b6ef42261b37cccf3 Gerrit-Change-Number: 26948 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Wed, 11 Jul 2018 16:04:43 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit July 2018