Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/38209 )
Change subject: raiden_debug: Upstream ChromiumOS servo debug board prog
......................................................................
raiden_debug: Upstream ChromiumOS servo debug board prog
Initial check-in of the Raiden debugger programmer.
Squash in,
usb_device: needed for raiden_debug prog
BUG=b:143389556
BRANCH=none
TEST=builds
Change-Id: Ifad273a708acea4de797a0808be58960635a8864
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M Makefile
M flashrom.c
M meson.build
M meson_options.txt
M programmer.h
A raiden_debug_spi.c
A usb_device.c
A usb_device.h
8 files changed, 977 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/09/38209/1
diff --git a/Makefile b/Makefile
index 7242b09..e8ca7a8 100644
--- a/Makefile
+++ b/Makefile
@@ -226,6 +226,11 @@
else
override CONFIG_RAYER_SPI = no
endif
+ifeq ($(CONFIG_RAIDEN), yes)
+UNSUPPORTED_FEATURES += CONFIG_RAIDEN=yes
+else
+override CONFIG_RAIDEN = no
+endif
ifeq ($(CONFIG_NIC3COM), yes)
UNSUPPORTED_FEATURES += CONFIG_NIC3COM=yes
else
@@ -607,6 +612,9 @@
# RayeR SPIPGM hardware support
CONFIG_RAYER_SPI ?= yes
+# ChromiumOS servo DUT debug board hardware support
+CONFIG_RAIDEN ?= yes
+
# PonyProg2000 SPI hardware support
CONFIG_PONY_SPI ?= yes
@@ -817,6 +825,11 @@
NEED_RAW_ACCESS += CONFIG_RAYER_SPI
endif
+ifeq ($(CONFIG_RAIDEN), yes)
+FEATURE_CFLAGS += -D'CONFIG_RAIDEN=1'
+PROGRAMMER_OBJS += raiden_debug_spi.o usb_device.o
+endif
+
ifeq ($(CONFIG_PONY_SPI), yes)
FEATURE_CFLAGS += -D'CONFIG_PONY_SPI=1'
PROGRAMMER_OBJS += pony_spi.o
diff --git a/flashrom.c b/flashrom.c
index e540027..e541b68 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -133,6 +133,18 @@
},
#endif
+#if CONFIG_RAIDEN == 1
+ {
+ .name = "raiden_debug",
+ .type = USB,
+ .devs.note = "All programmer devices speaking the raiden protocol\n",
+ .init = raiden_debug_spi_init,
+ .map_flash_region = fallback_map,
+ .unmap_flash_region = fallback_unmap,
+ .delay = internal_delay,
+ },
+#endif
+
#if CONFIG_DRKAISER == 1
{
.name = "drkaiser",
diff --git a/meson.build b/meson.build
index 375089c..88b14cf 100644
--- a/meson.build
+++ b/meson.build
@@ -42,6 +42,7 @@
config_dummy = get_option('config_dummy')
config_ft2232_spi = get_option('config_ft2232_spi')
config_gfxnvidia = get_option('config_gfxnvidia')
+config_raiden = get_option('config_raiden')
config_internal = get_option('config_internal')
config_it8212 = get_option('config_it8212')
config_linux_mtd = get_option('config_linux_mtd')
@@ -104,6 +105,7 @@
config_atavia = false
config_drkaiser = false
config_gfxnvidia = false
+ config_raiden = false
config_internal = false
config_it8212 = false
config_nic3com = false
@@ -170,6 +172,11 @@
srcs += 'gfxnvidia.c'
cargs += '-DCONFIG_GFXNVIDIA=1'
endif
+if config_raiden
+ srcs += 'raiden_debug_spi.c'
+ srcs += 'usb_device.c'
+ cargs += '-DCONFIG_RAIDEN=1'
+endif
if config_internal
srcs += 'board_enable.c'
srcs += 'cbtable.c'
diff --git a/meson_options.txt b/meson_options.txt
index b7633d0..4334814 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -13,6 +13,7 @@
option('config_dummy', type : 'boolean', value : true, description : 'dummy tracing')
option('config_ft2232_spi', type : 'boolean', value : true, description : 'FT2232 SPI dongles')
option('config_gfxnvidia', type : 'boolean', value : true, description : 'NVIDIA graphics cards')
+option('config_raiden', type : 'boolean', value : true, description : 'ChromiumOS Servo DUT debug board')
option('config_internal', type : 'boolean', value : true, description : 'internal/onboard')
option('config_internal_dmi', type : 'boolean', value : true, description : 'Use internal DMI parser')
option('config_it8212', type : 'boolean', value : true, description : 'ITE IT8212F PATA')
diff --git a/programmer.h b/programmer.h
index 3cf53b9..08500c6 100644
--- a/programmer.h
+++ b/programmer.h
@@ -43,6 +43,9 @@
#if CONFIG_GFXNVIDIA == 1
PROGRAMMER_GFXNVIDIA,
#endif
+#if CONFIG_RAIDEN == 1
+ PROGRAMMER_RAIDEN,
+#endif
#if CONFIG_DRKAISER == 1
PROGRAMMER_DRKAISER,
#endif
@@ -401,6 +404,11 @@
extern const struct dev_entry gfx_nvidia[];
#endif
+/* raiden_debug_spi.c */
+#if CONFIG_RAIDEN == 1
+int raiden_debug_spi_init(void);
+#endif
+
/* drkaiser.c */
#if CONFIG_DRKAISER == 1
int drkaiser_init(void);
diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c
new file mode 100644
index 0000000..9af0f7d
--- /dev/null
+++ b/raiden_debug_spi.c
@@ -0,0 +1,369 @@
+/*
+ * This file is part of the flashrom project.
+ *
+ * Copyright 2014, Google Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") version 2 as published by the Free
+ * Software Foundation.
+ */
+
+/*
+ * This SPI flash programming interface is designed to talk to a Chromium OS
+ * device over a Raiden USB connection. The USB connection is routed to a
+ * microcontroller running an image compiled from:
+ *
+ * https://chromium.googlesource.com/chromiumos/platform/ec
+ *
+ * The protocol for the USB-SPI bridge is documented in the following file in
+ * that respository:
+ *
+ * chip/stm32/usb_spi.c
+ */
+
+#include "programmer.h"
+#include "spi.h"
+#include "usb_device.h"
+
+#include <libusb.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#define GOOGLE_VID 0x18D1
+#define GOOGLE_RAIDEN_SPI_SUBCLASS 0x51
+#define GOOGLE_RAIDEN_SPI_PROTOCOL 0x01
+
+enum raiden_debug_spi_request {
+ RAIDEN_DEBUG_SPI_REQ_ENABLE = 0x0000,
+ RAIDEN_DEBUG_SPI_REQ_DISABLE = 0x0001,
+ RAIDEN_DEBUG_SPI_REQ_ENABLE_AP = 0x0002,
+ RAIDEN_DEBUG_SPI_REQ_ENABLE_EC = 0x0003,
+};
+
+#define PACKET_HEADER_SIZE 2
+#define MAX_PACKET_SIZE 64
+
+/*
+ * This timeout is so large because the Raiden SPI timeout is 800ms.
+ */
+#define TRANSFER_TIMEOUT_MS 1000
+
+struct usb_device *device = NULL;
+uint8_t in_endpoint = 0;
+uint8_t out_endpoint = 0;
+
+static int send_command(struct flashctx *flash,
+ unsigned int write_count,
+ unsigned int read_count,
+ const unsigned char *write_buffer,
+ unsigned char *read_buffer)
+{
+ uint8_t buffer[MAX_PACKET_SIZE];
+ int transferred;
+
+ if (write_count > MAX_PACKET_SIZE - PACKET_HEADER_SIZE) {
+ msg_perr("Raiden: invalid write_count of %d\n", write_count);
+ return SPI_INVALID_LENGTH;
+ }
+
+ if (read_count > MAX_PACKET_SIZE - PACKET_HEADER_SIZE) {
+ msg_perr("Raiden: invalid read_count of %d\n", read_count);
+ return SPI_INVALID_LENGTH;
+ }
+
+ buffer[0] = write_count;
+ buffer[1] = read_count;
+
+ memcpy(buffer + PACKET_HEADER_SIZE, write_buffer, write_count);
+
+ CHECK(LIBUSB(libusb_bulk_transfer(device->handle,
+ out_endpoint,
+ buffer,
+ write_count + PACKET_HEADER_SIZE,
+ &transferred,
+ TRANSFER_TIMEOUT_MS)),
+ "Raiden: OUT transfer failed\n"
+ " write_count = %d\n"
+ " read_count = %d\n",
+ write_count,
+ read_count);
+
+ if ((unsigned) transferred != write_count + PACKET_HEADER_SIZE) {
+ msg_perr("Raiden: Write failure (wrote %d, expected %d)\n",
+ transferred, write_count + PACKET_HEADER_SIZE);
+ return 0x10001;
+ }
+
+ CHECK(LIBUSB(libusb_bulk_transfer(device->handle,
+ in_endpoint,
+ buffer,
+ read_count + PACKET_HEADER_SIZE,
+ &transferred,
+ TRANSFER_TIMEOUT_MS)),
+ "Raiden: IN transfer failed\n"
+ " write_count = %d\n"
+ " read_count = %d\n",
+ write_count,
+ read_count);
+
+ if ((unsigned) transferred != read_count + PACKET_HEADER_SIZE) {
+ msg_perr("Raiden: Read failure (read %d, expected %d)\n",
+ transferred, read_count + PACKET_HEADER_SIZE);
+ return 0x10002;
+ }
+
+ memcpy(read_buffer, buffer + PACKET_HEADER_SIZE, read_count);
+
+ return buffer[0] | (buffer[1] << 8);
+}
+
+/*
+ * Unfortunately there doesn't seem to be a way to specify the maximum number
+ * of bytes that your SPI device can read/write, these values are the maximum
+ * data chunk size that flashrom will package up with an additional five bytes
+ * of command for the flash device, resulting in a 62 byte packet, that we then
+ * add two bytes to in either direction, making our way up to the 64 byte
+ * maximum USB packet size for the device.
+ *
+ * The largest command that flashrom generates is the byte program command, so
+ * we use that command header maximum size here.
+ */
+#define MAX_DATA_SIZE (MAX_PACKET_SIZE - \
+ PACKET_HEADER_SIZE - \
+ JEDEC_BYTE_PROGRAM_OUTSIZE)
+
+static const struct spi_master spi_master_raiden_debug = {
+ .features = SPI_MASTER_4BA,
+ .max_data_read = MAX_DATA_SIZE,
+ .max_data_write = MAX_DATA_SIZE,
+ .command = send_command,
+ .multicommand = default_spi_send_multicommand,
+ .read = default_spi_read,
+ .write_256 = default_spi_write_256,
+};
+
+static int match_endpoint(struct libusb_endpoint_descriptor const *descriptor,
+ enum libusb_endpoint_direction direction)
+{
+ return (((descriptor->bEndpointAddress & LIBUSB_ENDPOINT_DIR_MASK) ==
+ direction) &&
+ ((descriptor->bmAttributes & LIBUSB_TRANSFER_TYPE_MASK) ==
+ LIBUSB_TRANSFER_TYPE_BULK));
+}
+
+static int find_endpoints(struct usb_device *dev, uint8_t *in_ep, uint8_t *out_ep)
+{
+ int i;
+ int in_count = 0;
+ int out_count = 0;
+
+ for (i = 0; i < dev->interface_descriptor->bNumEndpoints; i++) {
+ struct libusb_endpoint_descriptor const *endpoint =
+ &dev->interface_descriptor->endpoint[i];
+
+ if (match_endpoint(endpoint, LIBUSB_ENDPOINT_IN)) {
+ in_count++;
+ *in_ep = endpoint->bEndpointAddress;
+ } else if (match_endpoint(endpoint, LIBUSB_ENDPOINT_OUT)) {
+ out_count++;
+ *out_ep = endpoint->bEndpointAddress;
+ }
+ }
+
+ if (in_count != 1 || out_count != 1) {
+ msg_perr("Raiden: Failed to find one IN and one OUT endpoint\n"
+ " found %d IN and %d OUT endpoints\n",
+ in_count,
+ out_count);
+ return 1;
+ }
+
+ msg_pdbg("Raiden: Found IN endpoint = 0x%02x\n", *in_ep);
+ msg_pdbg("Raiden: Found OUT endpoint = 0x%02x\n", *out_ep);
+
+ return 0;
+}
+
+static int shutdown(void * data)
+{
+ CHECK(LIBUSB(libusb_control_transfer(
+ device->handle,
+ LIBUSB_ENDPOINT_OUT |
+ LIBUSB_REQUEST_TYPE_VENDOR |
+ LIBUSB_RECIPIENT_INTERFACE,
+ RAIDEN_DEBUG_SPI_REQ_DISABLE,
+ 0,
+ device->interface_descriptor->bInterfaceNumber,
+ NULL,
+ 0,
+ TRANSFER_TIMEOUT_MS)),
+ "Raiden: Failed to disable SPI bridge\n");
+
+ usb_device_free(device);
+
+ device = NULL;
+ libusb_exit(NULL);
+
+ return 0;
+}
+
+static int get_target()
+{
+ int request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE;
+
+ char *target_str = extract_programmer_param("target");
+ if (target_str) {
+ if (!strcasecmp(target_str, "ap"))
+ request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE_AP;
+ else if (!strcasecmp(target_str, "ec"))
+ request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE_EC;
+ else {
+ msg_perr("Invalid target: %s\n", target_str);
+ request_enable = -1;
+ }
+ }
+ free(target_str);
+
+ return request_enable;
+}
+
+static void free_dev_list(struct usb_device **dev_lst)
+{
+ struct usb_device *dev = *dev_lst;
+ /* free devices we don't care about */
+ dev = dev->next;
+ while (dev)
+ dev = usb_device_free(dev);
+}
+
+int raiden_debug_spi_init(void)
+{
+ struct usb_match match;
+ char *serial = extract_programmer_param("serial");
+ struct usb_device *current;
+ int found = 0;
+
+ int request_enable = get_target();
+ if (request_enable < 0)
+ return 1;
+
+ usb_match_init(&match);
+
+ usb_match_value_default(&match.vid, GOOGLE_VID);
+ usb_match_value_default(&match.class, LIBUSB_CLASS_VENDOR_SPEC);
+ usb_match_value_default(&match.subclass, GOOGLE_RAIDEN_SPI_SUBCLASS);
+ usb_match_value_default(&match.protocol, GOOGLE_RAIDEN_SPI_PROTOCOL);
+
+ CHECK(LIBUSB(libusb_init(NULL)), "Raiden: libusb_init failed\n");
+
+ CHECK(usb_device_find(&match, ¤t),
+ "Raiden: Failed to find devices\n");
+
+ while (current) {
+ device = current;
+
+ if (find_endpoints(device, &in_endpoint, &out_endpoint)) {
+ msg_pdbg("Raiden: Failed to find valid endpoints on device");
+ usb_device_show(" ", current);
+ goto loop_end;
+ }
+
+ if (usb_device_claim(device)) {
+ msg_pdbg("Raiden: Failed to claim USB device");
+ usb_device_show(" ", current);
+ goto loop_end;
+ }
+
+ if (!serial) {
+ found = 1;
+ goto loop_end;
+ } else {
+ unsigned char dev_serial[32];
+ struct libusb_device_descriptor descriptor;
+ int rc;
+
+ memset(dev_serial, 0, sizeof(dev_serial));
+
+ if (libusb_get_device_descriptor(device->device, &descriptor)) {
+ msg_pdbg("USB: Failed to get device descriptor.\n");
+ goto loop_end;
+ }
+
+ rc = libusb_get_string_descriptor_ascii(device->handle,
+ descriptor.iSerialNumber,
+ dev_serial,
+ sizeof(dev_serial));
+ if (rc < 0) {
+ LIBUSB(rc);
+ } else {
+ if (strcmp(serial, (char *)dev_serial)) {
+ msg_pdbg("Raiden: Serial number %s did not match device", serial);
+ usb_device_show(" ", current);
+ } else {
+ msg_pinfo("Raiden: Serial number %s matched device", serial);
+ usb_device_show(" ", current);
+ found = 1;
+ }
+ }
+ }
+
+loop_end:
+ if (found)
+ break;
+ else
+ current = usb_device_free(current);
+ }
+
+ if (!device || !found) {
+ msg_perr("Raiden: No usable device found.\n");
+ return 1;
+ }
+
+ free_dev_list(¤t);
+
+ CHECK(LIBUSB(libusb_control_transfer(
+ device->handle,
+ LIBUSB_ENDPOINT_OUT |
+ LIBUSB_REQUEST_TYPE_VENDOR |
+ LIBUSB_RECIPIENT_INTERFACE,
+ request_enable,
+ 0,
+ device->interface_descriptor->bInterfaceNumber,
+ NULL,
+ 0,
+ TRANSFER_TIMEOUT_MS)),
+ "Raiden: Failed to enable SPI bridge\n");
+
+ register_spi_master(&spi_master_raiden_debug);
+ register_shutdown(shutdown, NULL);
+
+ return 0;
+}
diff --git a/usb_device.c b/usb_device.c
new file mode 100644
index 0000000..0909694
--- /dev/null
+++ b/usb_device.c
@@ -0,0 +1,366 @@
+/*
+ * This file is part of the flashrom project.
+ *
+ * Copyright 2015, Google Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") version 2 as published by the Free
+ * Software Foundation.
+ */
+
+#include "programmer.h"
+#include "spi.h"
+#include "usb_device.h"
+
+#include <assert.h>
+#include <libusb.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+/*
+ * Possibly extract a programmer parameter and use it to initialize the given
+ * match value structure.
+ */
+static void usb_match_value_init(struct usb_match_value *match,
+ char const *parameter)
+{
+ char *string = extract_programmer_param(parameter);
+
+ match->name = parameter;
+
+ if (string) {
+ match->set = 1;
+ match->value = strtol(string, NULL, 0);
+ } else {
+ match->set = 0;
+ }
+
+ free(string);
+}
+
+#define USB_MATCH_VALUE_INIT(NAME) \
+ usb_match_value_init(&match->NAME, #NAME)
+
+void usb_match_init(struct usb_match *match)
+{
+ USB_MATCH_VALUE_INIT(vid);
+ USB_MATCH_VALUE_INIT(pid);
+ USB_MATCH_VALUE_INIT(bus);
+ USB_MATCH_VALUE_INIT(address);
+ USB_MATCH_VALUE_INIT(config);
+ USB_MATCH_VALUE_INIT(interface);
+ USB_MATCH_VALUE_INIT(altsetting);
+ USB_MATCH_VALUE_INIT(class);
+ USB_MATCH_VALUE_INIT(subclass);
+ USB_MATCH_VALUE_INIT(protocol);
+}
+
+void usb_match_value_default(struct usb_match_value *value,
+ long int default_value)
+{
+ if (value->set)
+ return;
+
+ value->set = 1;
+ value->value = default_value;
+}
+
+/*
+ * Match the value against a possible user supplied parameter.
+ *
+ * Return:
+ * 0: The user supplied the given parameter and it did not match the value.
+ * 1: Either the user didn't supply the parameter, or they did and it
+ * matches the given value.
+ */
+static int check_match(struct usb_match_value const *match_value, int value)
+{
+ int reject = match_value->set && (match_value->value != value);
+
+ if (reject)
+ msg_pdbg("USB: Rejecting device because %s = %d != %d\n",
+ match_value->name,
+ value,
+ match_value->value);
+
+ return !reject;
+}
+
+/*
+ * Allocate a copy of device and add it to the head of the devices list.
+ */
+static void add_device(struct usb_device *device,
+ struct usb_device **devices)
+{
+ struct usb_device *copy = malloc(sizeof(struct usb_device));
+
+ assert(copy != NULL);
+
+ *copy = *device;
+
+ copy->next = *devices;
+ *devices = copy;
+
+ libusb_ref_device(copy->device);
+}
+
+/*
+ * Look through the interfaces of the current device config for a match. Stop
+ * looking after the first valid match is found.
+ *
+ * Return:
+ * 0: No matching interface was found.
+ * 1: A complete match was found and added to the devices list.
+ */
+static int find_interface(struct usb_match const *match,
+ struct usb_device *current,
+ struct usb_device **devices)
+{
+ int i, j;
+
+ for (i = 0; i < current->config_descriptor->bNumInterfaces; ++i) {
+ struct libusb_interface const *interface;
+
+ interface = ¤t->config_descriptor->interface[i];
+
+ for (j = 0; j < interface->num_altsetting; ++j) {
+ struct libusb_interface_descriptor const *descriptor;
+
+ descriptor = &interface->altsetting[j];
+
+ if (check_match(&match->interface,
+ descriptor->bInterfaceNumber) &&
+ check_match(&match->altsetting,
+ descriptor->bAlternateSetting) &&
+ check_match(&match->class,
+ descriptor->bInterfaceClass) &&
+ check_match(&match->subclass,
+ descriptor->bInterfaceSubClass) &&
+ check_match(&match->protocol,
+ descriptor->bInterfaceProtocol)) {
+ current->interface_descriptor = descriptor;
+ add_device(current, devices);
+ msg_pdbg("USB: Found matching device\n");
+ return 1;
+ }
+ }
+ }
+
+ return 0;
+}
+
+/*
+ * Look through the configs of the current device for a match. Stop looking
+ * after the first valid match is found.
+ *
+ * Return:
+ * 0: All configurations successfully checked, one may have been added to
+ * the list.
+ * non-zero: There was a failure while checking for a match.
+ */
+static int find_config(struct usb_match const *match,
+ struct usb_device *current,
+ struct libusb_device_descriptor const *device_descriptor,
+ struct usb_device **devices)
+{
+ int i;
+
+ for (i = 0; i < device_descriptor->bNumConfigurations; ++i) {
+ CHECK(LIBUSB(libusb_get_config_descriptor(
+ current->device,
+ i,
+ ¤t->config_descriptor)),
+ "USB: Failed to get config descriptor");
+
+ if (check_match(&match->config,
+ current->config_descriptor->
+ bConfigurationValue) &&
+ find_interface(match, current, devices))
+ break;
+
+ libusb_free_config_descriptor(current->config_descriptor);
+ }
+
+ return 0;
+}
+
+int usb_device_find(struct usb_match const *match, struct usb_device **devices)
+{
+ libusb_device **list;
+ size_t count;
+ size_t i;
+
+ *devices = NULL;
+
+ CHECK(LIBUSB(count = libusb_get_device_list(NULL, &list)),
+ "USB: Failed to get device list");
+
+ for (i = 0; i < count; ++i) {
+ struct libusb_device_descriptor descriptor;
+ struct usb_device current = {
+ .device = list[i],
+ .handle = NULL,
+ .next = NULL,
+ };
+
+ uint8_t bus = libusb_get_bus_number(list[i]);
+ uint8_t address = libusb_get_device_address(list[i]);
+
+ msg_pdbg("USB: Inspecting device (Bus %d, Address %d)\n",
+ bus,
+ address);
+
+ CHECK(LIBUSB(libusb_get_device_descriptor(list[i],
+ &descriptor)),
+ "USB: Failed to get device descriptor");
+
+ if (check_match(&match->vid, descriptor.idVendor) &&
+ check_match(&match->pid, descriptor.idProduct) &&
+ check_match(&match->bus, bus) &&
+ check_match(&match->address, address))
+ CHECK(find_config(match,
+ ¤t,
+ &descriptor,
+ devices),
+ "USB: Failed to find config");
+ }
+
+ libusb_free_device_list(list, 1);
+
+ return (*devices == NULL);
+}
+
+/*
+ * If the underlying libusb device is not open, open it.
+ *
+ * Return:
+ * 0: The device was already open or was successfully opened.
+ * non-zero: There was a failure while opening the device.
+ */
+static int usb_device_open(struct usb_device *device)
+{
+ if (device->handle == NULL)
+ CHECK(LIBUSB(libusb_open(device->device, &device->handle)),
+ "USB: Failed to open device\n");
+
+ return 0;
+}
+
+int usb_device_show(char const *prefix, struct usb_device *device)
+{
+ struct libusb_device_descriptor descriptor;
+ unsigned char product[256];
+
+ CHECK(usb_device_open(device), "USB: Failed to open device\n");
+
+ CHECK(LIBUSB(libusb_get_device_descriptor(device->device, &descriptor)),
+ "USB: Failed to get device descriptor\n");
+
+ CHECK(LIBUSB(libusb_get_string_descriptor_ascii(
+ device->handle,
+ descriptor.iProduct,
+ product,
+ sizeof(product))),
+ "USB: Failed to get device product string\n");
+
+ product[255] = '\0';
+
+ msg_perr("%sbus=0x%02x,address=0x%02x | %s\n",
+ prefix,
+ libusb_get_bus_number(device->device),
+ libusb_get_device_address(device->device),
+ product);
+
+ return 0;
+}
+
+int usb_device_claim(struct usb_device *device)
+{
+ int current_config;
+
+ CHECK(usb_device_open(device), "USB: Failed to open device\n");
+
+ CHECK(LIBUSB(libusb_get_configuration(device->handle,
+ ¤t_config)),
+ "USB: Failed to get current device configuration\n");
+
+ if (current_config != device->config_descriptor->bConfigurationValue)
+ CHECK(LIBUSB(libusb_set_configuration(
+ device->handle,
+ device->
+ config_descriptor->
+ bConfigurationValue)),
+ "USB: Failed to set new configuration from %d to %d\n",
+ current_config,
+ device->config_descriptor->bConfigurationValue);
+
+ CHECK(LIBUSB(libusb_set_auto_detach_kernel_driver(device->handle, 1)),
+ "USB: Failed to enable auto kernel driver detach\n");
+
+ CHECK(LIBUSB(libusb_claim_interface(device->handle,
+ device->
+ interface_descriptor->
+ bInterfaceNumber)),
+ "USB: Could not claim device interface %d\n",
+ device->interface_descriptor->bInterfaceNumber);
+
+ if (device->interface_descriptor->bAlternateSetting != 0)
+ CHECK(LIBUSB(libusb_set_interface_alt_setting(
+ device->handle,
+ device->
+ interface_descriptor->
+ bInterfaceNumber,
+ device->
+ interface_descriptor->
+ bAlternateSetting)),
+ "USB: Failed to set alternate setting %d\n",
+ device->interface_descriptor->bAlternateSetting);
+
+ return 0;
+}
+
+struct usb_device *usb_device_free(struct usb_device *device)
+{
+ struct usb_device *next = device->next;
+
+ if (device->handle != NULL)
+ libusb_close(device->handle);
+
+ /*
+ * This unref balances the ref added in the add_device function.
+ */
+ libusb_unref_device(device->device);
+ libusb_free_config_descriptor(device->config_descriptor);
+
+ free(device);
+
+ return next;
+}
diff --git a/usb_device.h b/usb_device.h
new file mode 100644
index 0000000..46d2458
--- /dev/null
+++ b/usb_device.h
@@ -0,0 +1,201 @@
+/*
+ * This file is part of the flashrom project.
+ *
+ * Copyright 2015, Google Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") version 2 as published by the Free
+ * Software Foundation.
+ */
+
+/*
+ * USB device matching framework
+ *
+ * This can be used to match a USB device by a number of different parameters.
+ * The parameters can be passed on the command line and defaults can be set
+ * by the programmer.
+ */
+
+#include <libusb.h>
+#include <stdint.h>
+
+/*
+ * Check Macro
+ *
+ * The check macro simplifies common return error code checking logic. If the
+ * expression does not evaluate to zero then the string error is printed and
+ * the expressions result is imediately returned, else the result is available
+ * as the value of the CHECK expression statement.
+ */
+
+#define CHECK(expression, string...) \
+ ({ \
+ int error__ = (expression); \
+ \
+ if (error__ != 0) { \
+ msg_perr(string); \
+ return error__; \
+ } \
+ \
+ error__; \
+ })
+
+/*
+ * The LIBUSB macro converts a libusb failure code into an error code that
+ * flashrom recognizes. It also displays additional libusb specific
+ * information about the failure.
+ */
+#define LIBUSB(expression) \
+ ({ \
+ int libusb_error__ = (expression); \
+ \
+ if (libusb_error__ < 0) { \
+ msg_perr("libusb error: %s:%d %s\n", \
+ __FILE__, \
+ __LINE__, \
+ libusb_error_name(libusb_error__)); \
+ libusb_error__ = 0x20000 | -libusb_error__; \
+ } else { \
+ libusb_error__ = 0; \
+ } \
+ \
+ libusb_error__; \
+ })
+
+/*
+ * A USB match and associated value struct are used to encode the information
+ * about a device against which we wish to match. If the value of a
+ * usb_match_value has been set then a device must match that value. The name
+ * of the usb_match_value is used to fetch the programmer parameter from the
+ * flashrom command line and is the same as the name of the corresponding
+ * field in usb_match.
+ */
+struct usb_match_value {
+ char const *name;
+ int value;
+ int set;
+};
+
+struct usb_match {
+ struct usb_match_value bus;
+ struct usb_match_value address;
+ struct usb_match_value vid;
+ struct usb_match_value pid;
+ struct usb_match_value serial;
+ struct usb_match_value config;
+ struct usb_match_value interface;
+ struct usb_match_value altsetting;
+ struct usb_match_value class;
+ struct usb_match_value subclass;
+ struct usb_match_value protocol;
+};
+
+/*
+ * Initialize a usb_match structure so that each value's name matches the
+ * values name in the usb_match structure (so bus.name == "bus"...), and
+ * look for each value in the flashrom command line via
+ * extract_programmer_param. If the value is found convert it to an integer
+ * using strtol, accepting hex, decimal and octal encoding.
+ */
+void usb_match_init(struct usb_match *match);
+
+/*
+ * Add a default value to a usb_match_value. This must be done after calling
+ * usb_match_init. If usb_match_init already set the value of a usb_match_value
+ * we do nothing, otherwise set the value to default_value. This ensures that
+ * parameters passed on the command line override defaults.
+ */
+void usb_match_value_default(struct usb_match_value *match,
+ long int default_value);
+
+/*
+ * The usb_device structure is an entry in a linked list of devices that were
+ * matched by usb_device_find.
+ */
+struct usb_device {
+ struct libusb_device *device;
+ struct libusb_config_descriptor *config_descriptor;
+ struct libusb_interface_descriptor const *interface_descriptor;
+
+ /*
+ * Initially NULL, the libusb_device_handle is only valid once the
+ * usb_device has been successfully passed to usb_device_show or
+ * usb_device_claim.
+ */
+ struct libusb_device_handle *handle;
+
+ /*
+ * Link to next device, or NULL
+ */
+ struct usb_device *next;
+};
+
+/*
+ * Find and return a list of all compatible devices. Each device is added to
+ * the list with its first valid configuration and interface. If an alternate
+ * configuration (config, interface, altsetting...) is desired the specifics
+ * can be supplied as programmer parameters.
+ *
+ * Return:
+ * 0: At least one matching device was found.
+ * 1: No matching devices were found.
+ */
+int usb_device_find(struct usb_match const *match, struct usb_device **devices);
+
+/*
+ * Display the devices bus and address as well as its product string. The
+ * underlying libusb device is opened if it is not already open.
+ *
+ * Return:
+ * 0: The device information was displayed.
+ * non-zero: There was a failure while displaying the device information.
+ */
+int usb_device_show(char const *prefix, struct usb_device *device);
+
+/*
+ * Open the underlying libusb device, set its config, claim the interface and
+ * select the correct alternate interface.
+ *
+ * Return:
+ * 0: The device was successfully claimed.
+ * non-zero: There was a failure while trying to claim the device.
+ */
+int usb_device_claim(struct usb_device *device);
+
+/*
+ * Free a usb_device structure.
+ *
+ * This ensures that the libusb device is closed and that all allocated
+ * handles and descriptors are freed.
+ *
+ * Return:
+ * The next device in the device list.
+ */
+struct usb_device *usb_device_free(struct usb_device *device);
--
To view, visit https://review.coreboot.org/c/flashrom/+/38209
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ifad273a708acea4de797a0808be58960635a8864
Gerrit-Change-Number: 38209
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Nico Huber, Daniel Campello.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51929 )
Change subject: meson: remove rayer_spi dependency on libpci
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/51929
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If7206a69d031c9bba9475a9e6769f6ef35701379
Gerrit-Change-Number: 51929
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Comment-Date: Mon, 29 Mar 2021 23:27:50 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, David Hendricks, Edward O'Callaghan.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51243 )
Change subject: Enable dynamic memory allocation checks for cmocka unit tests
......................................................................
Patch Set 6:
(1 comment)
Patchset:
PS6:
Even better news: now after adding includes in meson.build (my previous patchset) I discovered that unittest.h is not needed anymore in test files! So I am removing it from here. And now ninja test warnings are fixed - because there are no re-definitions anymore.
This looks like a happy ending: only one header, included in build file, no need to touch the code, all files are covered by memory checks when they are built for test environment.
I did all the testing for this last patchset, as before: nm files built for test vs for real, nm test files, running tests. Also introducing memory leak and verifying that test catches it (examples described in commit message).
--
To view, visit https://review.coreboot.org/c/flashrom/+/51243
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f
Gerrit-Change-Number: 51243
Gerrit-PatchSet: 6
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)google.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Daniel Kurtz <djkurtz(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-CC: Victor Ding <victording(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Mon, 29 Mar 2021 23:25:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, David Hendricks, Edward O'Callaghan.
Hello build bot (Jenkins), Nico Huber, Daniel Kurtz, David Hendricks, Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/51243
to look at the new patch set (#6).
Change subject: Enable dynamic memory allocation checks for cmocka unit tests
......................................................................
Enable dynamic memory allocation checks for cmocka unit tests
This commit enables the feature and makes changes to existing
files and tests. I am writing more new tests with this.
Commit includes tests/flashrom.c because after enabling memory
checks the test started to fail (it used to leak memory indeed).
If you are wondering how to verify it works (because at the moment
all tests [still] pass so it’s not obvious that anything has
changed), then for example:
1) Remove free’s in flashbuses_to_text_test_success test, and it
will fail with message similar to this (line numbers from your local
source)
[ ERROR ] --- Blocks allocated...
../flashrom.c:1239: note: block 0x55f42304b640 allocated here
../flashrom.c:1239: note: block 0x55f42304b5c0 allocated here
../flashrom.c:1239: note: block 0x55f42304b3d0 allocated here
../flashrom.c:1239: note: block 0x55f42304b700 allocated here
../flashrom.c:1239: note: block 0x55f42304b780 allocated here
../flashrom.c:1239: note: block 0x55f42304bb00 allocated here
../flashrom.c:1239: note: block 0x55f42304b810 allocated here
ERROR: flashbuses_to_text_test_success leaked 7 block(s)
2) Add char *temp = malloc just before return from strcat_realloc
[ ERROR ] --- Blocks allocated...
../helpers.c:88: note: block 0x55a51307b6c0 allocated here
../helpers.c:88: note: block 0x55a51307b9e0 allocated here
ERROR: strcat_realloc_test_success leaked 2 block(s)
BUG=b:181803212
TEST=builds and ninja test
nm builddir/tests/flashrom_unit_tests.p/.._flashrom.c.o
nm builddir/tests/flashrom_unit_tests.p/flashrom.c.o
nm builddir/flashrom.p/flashrom.c.o
Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M meson.build
M tests/flashrom.c
M tests/helpers.c
M tests/meson.build
A unittest_env.h
5 files changed, 86 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/51243/6
--
To view, visit https://review.coreboot.org/c/flashrom/+/51243
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f
Gerrit-Change-Number: 51243
Gerrit-PatchSet: 6
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)google.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Daniel Kurtz <djkurtz(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-CC: Victor Ding <victording(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, David Hendricks, Edward O'Callaghan.
Hello build bot (Jenkins), Nico Huber, Daniel Kurtz, David Hendricks, Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/51243
to look at the new patch set (#5).
Change subject: Enable dynamic memory allocation checks for cmocka unit tests
......................................................................
Enable dynamic memory allocation checks for cmocka unit tests
This commit enables the feature and makes changes to existing
files and tests. I am writing more new tests with this.
Commit includes tests/flashrom.c because after enabling memory
checks the test started to fail (it used to leak memory indeed).
If you are wondering how to verify it works (because at the moment
all tests [still] pass so it’s not obvious that anything has
changed), then for example:
1) Remove free’s in flashbuses_to_text_test_success test, and it
will fail with message similar to this (line numbers from your local
source)
[ ERROR ] --- Blocks allocated...
../flashrom.c:1239: note: block 0x55f42304b640 allocated here
../flashrom.c:1239: note: block 0x55f42304b5c0 allocated here
../flashrom.c:1239: note: block 0x55f42304b3d0 allocated here
../flashrom.c:1239: note: block 0x55f42304b700 allocated here
../flashrom.c:1239: note: block 0x55f42304b780 allocated here
../flashrom.c:1239: note: block 0x55f42304bb00 allocated here
../flashrom.c:1239: note: block 0x55f42304b810 allocated here
ERROR: flashbuses_to_text_test_success leaked 7 block(s)
2) Add char *temp = malloc just before return from strcat_realloc
[ ERROR ] --- Blocks allocated...
../helpers.c:88: note: block 0x55a51307b6c0 allocated here
../helpers.c:88: note: block 0x55a51307b9e0 allocated here
ERROR: strcat_realloc_test_success leaked 2 block(s)
BUG=b:181803212
TEST=builds and ninja test
nm builddir/tests/flashrom_unit_tests.p/.._flashrom.c.o
nm builddir/tests/flashrom_unit_tests.p/flashrom.c.o
nm builddir/flashrom.p/flashrom.c.o
Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M meson.build
M tests/flashrom.c
M tests/helpers.c
M tests/meson.build
A unittest_env.h
5 files changed, 85 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/51243/5
--
To view, visit https://review.coreboot.org/c/flashrom/+/51243
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f
Gerrit-Change-Number: 51243
Gerrit-PatchSet: 5
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)google.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Daniel Kurtz <djkurtz(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-CC: Victor Ding <victording(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: newpatchset
Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51577 )
Change subject: meson: fix dependency on raw access
......................................................................
Patch Set 3:
(1 comment)
File meson.build:
https://review.coreboot.org/c/flashrom/+/51577/comment/7e84b8d7_6a6bff1e
PS3, Line 280: need_raw_access = true
> I've just read through `rayer_spi.c`. It's a very simple driver using an LPT […]
Sounds good. Sent review.coreboot.org/c/flashrom/+/51929
--
To view, visit https://review.coreboot.org/c/flashrom/+/51577
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id3fa4ec7a735b81e989ba9fe2b53b18d0956627a
Gerrit-Change-Number: 51577
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Mario Limonciello <superm1(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Richard Hughes <hughsient(a)gmail.com>
Gerrit-Reviewer: Richard Hughes <richard(a)hughsie.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)users.sourceforge.net>
Gerrit-Comment-Date: Mon, 29 Mar 2021 22:58:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51577 )
Change subject: meson: fix dependency on raw access
......................................................................
Patch Set 3:
(1 comment)
File meson.build:
https://review.coreboot.org/c/flashrom/+/51577/comment/f6de76e5_91b3b3f6
PS3, Line 280: need_raw_access = true
> You are right! I did not notice that the Makefile does not request libpci. […]
I've just read through `rayer_spi.c`. It's a very simple driver using an LPT
(parallel) port. No sign of PCI. A follow up would be nice. Feel free to also
remove the `config_rayer_spi = false` for libpci above (line ~124).
--
To view, visit https://review.coreboot.org/c/flashrom/+/51577
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id3fa4ec7a735b81e989ba9fe2b53b18d0956627a
Gerrit-Change-Number: 51577
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Mario Limonciello <superm1(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Richard Hughes <hughsient(a)gmail.com>
Gerrit-Reviewer: Richard Hughes <richard(a)hughsie.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)users.sourceforge.net>
Gerrit-Comment-Date: Mon, 29 Mar 2021 22:32:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, David Hendricks, Edward O'Callaghan.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51243 )
Change subject: Enable dynamic memory allocation checks for cmocka unit tests
......................................................................
Patch Set 4:
(2 comments)
Patchset:
PS2:
> Nico, sorry for a delay, I just needed some time to think about it. […]
Good news: I made it work! No need to include unittest_env.h in every file under test, they are now all covered. I inspected object files with nm, everything built for tests has malloc, free etc redefined, at the same time files built for real have normal memory functions.
Specifically:
nm builddir/tests/flashrom_unit_tests.p/.._flashrom.c.o
Has _test_calloc _test_free _test_malloc _test_realloc
nm builddir/flashrom.p/flashrom.c.o
Has calloc free malloc realloc
Side effect is that now running ninja test produces warnings (but seems fine, especially given this is exactly what’s happening). Example of warning (there are 4 of them)
In file included from ../tests/flashrom.c:19:
../tests/unittest.h:27: warning: "malloc" redefined
27 | #define malloc test_malloc
|
In file included from <command-line>:
../unittest_env.h:46: note: this is the location of the previous definition
46 | #define malloc(size) _test_malloc(size, __FILE__, __LINE__)
However the same approach did not work for test files :( so I am still including unittest.h into a test file, not from tests/meson.build. Which I hope is less of a problem, since this is now contained in tests/ directory.
Patchset:
PS4:
I have the first attempt to include unittest_env.h from command line for all files under test. This solution is working, but comments are very welcome!
--
To view, visit https://review.coreboot.org/c/flashrom/+/51243
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f
Gerrit-Change-Number: 51243
Gerrit-PatchSet: 4
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)google.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Daniel Kurtz <djkurtz(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-CC: Victor Ding <victording(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Mon, 29 Mar 2021 21:28:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment