[flashrom] Libflashrom draft patch - initial review
Антон Кочков
anton.kochkov at gmail.com
Fri Jun 19 13:50:17 CEST 2015
Uh, forgot to add, that loading layout will be quite useful too, in the API and
adding romentry_t* argument for all fl_image_* functions.
Best regards,
Anton Kochkov.
On Fri, Jun 19, 2015 at 2:43 PM, Антон Кочков <anton.kochkov at gmail.com> wrote:
> Hi!
> I've done also some rebasing of the libflashrom patches before (on top
> of the layout branch from the stefanct)
> https://github.com/XVilka/flashrom/commits/libflashrom
>
> May be you'll find something useful here.
> Best regards,
> Anton Kochkov.
>
>
> On Thu, Jun 18, 2015 at 7:45 AM, Urja Rannikko <urjaman at gmail.com> wrote:
>> Hi,
>>
>>
>> On Fri, Jun 12, 2015 at 12:26 AM, Łukasz Dmitrowski
>> <lukasz.dmitrowski at gmail.com> wrote:
>>> Hello,
>>>
>>> My name is Łukasz Dmitrowski and I am one of coreboot/flashrom GSoC students
>>> this year. You can read about my project here:
>>> http://blogs.coreboot.org/blog/author/dmitro/.
>>>
>>> One of my first steps is updating and extending Nico Hubers libflashrom
>>> patch. Work is still in progress, but some changes are already made, so I
>>> thought that it will be good idea to send a patch just for review to know if
>>> I am going in a good direction. I want to improve my skills and provide best
>>> code quality as I can, so I am open for criticism - I am here to learn :)
>>>
>> I'm here essentially trying out reviewing stuff, so expect more from
>> somebody a bit more experienced, but I heard my comments would be
>> useful, so moving on :)
>>
>>> Patch summary:
>>>
>> First thing i have to say about the patch is that it has been
>> malformed, most likely line wrapped by your email client. If you cant
>> properly include the patch inline, attaching a .patch would be okay.
>> (Also turn off that HTML output if you can... although it wasnt used
>> for anything, but still.). So I didnt do any compile testing on this
>> stuff because of this.
>>
>>> Makefile: I added cli_common.o and print.o to LIB_OBJS - it solves undefined
>>> references in current libflashrom version, but spoils standard flashrom
>>> build with 'make', I did not investigated it yet as my focus is to extend
>>> library with new functions and update already existing, need to discuss it
>>> with my mentor
>>>
>>> cli_classic.c, cli_output.c, flash.h: Only Nico Hubers changes here, didn't
>>> touch it
>> I might comment on any part of the patch because I havent reviewed the
>> original and it's only about the code.
>>
>>>
>>> libflashrom.c, libflashrom.h: I added 8 functions and made a few changes to
>>> existing ones, as some of used functions are static, or does not exist.
>>>
>>> New functions:
>>>
>>> int fl_supported_programmers(const char **supported_programmers);
>>> int fl_supported_flash_chips(fl_flashchip_info_t *fchips);
>>> int fl_supported_boards(fl_board_info_t *boards);
>>> int fl_supported_chipsets(fl_chipset_info_t *chipsets);
>>> int fl_supported_programmers_number();
>>> int fl_supported_flash_chips_number();
>>> int fl_supported_boards_number();
>>> int fl_supported_chipsets_number();
>>>
>>> New types:
>>> fl_flashchip_info_t - smaller version of struct flashchip
>>> fl_board_info_t - smaller version of struct board_info
>>> fl_chipset_info_t - smaller version of struct penable
>>> fl_test_state - copy of enum test_state
>>>
>>> So we have 4 functions that return a list of supported programmers, chips,
>>> boards and chipsets. Next 4 functions return number of supported hardware of
>>> certain type. For example, to obtain a list of supported boards, you can
>>> create an array of structures fl_board_info of suitable size (the size is
>>> known - fl_supported_boards_number()), then pass it to
>>> fl_supported_boards(fl_board_info_t *boards) and you will have it filled
>>> with data.
>> My opinion is that the _number functions make the API unnecessarily
>> complex to use.
>> I'm suggesting like this:
>> fl_flashchip_info_t * fl_supported_flash_chips(void);
>> Have the call allocate the table using a not specified allocator
>> (really just malloc for now, but we're not telling), and freed with a
>> function like
>> int fl_support_info_free(void*p);
>> This would allow implementation using const tables or allocation,
>> whatever we like.
>> (if (p == const_table) return 0; in our _free() if mixed.)
>>
>>>
>>> Regarding changes in Nico Hubers functions: I made temporary comments for
>>> every change and marked it with LD_CHANGE_NOTE. Every comment contains
>>> previous code and reason why it has been commented out.
>>>
>>> Thanks in advance for your insights!
>>>
>>> Regards,
>>> Lukasz Dmitrowski
>>>
>>> ---
>>>
>>> Signed-off-by: Łukasz Dmitrowski <lukasz.dmitrowski at gmail.com>
>>>
>>> Index: Makefile
>>> ===================================================================
>>> --- Makefile (revision 1891)
>>> +++ Makefile (working copy)
>>> @@ -374,7 +374,7 @@
>>>
>>> ###############################################################################
>>> # Library code.
>>>
>>> -LIB_OBJS = layout.o flashrom.o udelay.o programmer.o helpers.o
>>> +LIB_OBJS = libflashrom.o layout.o flashrom.o udelay.o programmer.o
>>> helpers.o cli_common.o print.o
>>>
>>>
>>> ###############################################################################
>>> # Frontend related stuff.
>>> Index: cli_classic.c
>>> ===================================================================
>>> --- cli_classic.c (revision 1891)
>>> +++ cli_classic.c (working copy)
>>> @@ -30,6 +30,7 @@
>>> #include "flash.h"
>>> #include "flashchips.h"
>>> #include "programmer.h"
>>> +#include "libflashrom.h"
>>>
>>> static void cli_classic_usage(const char *name)
>>> {
>>> @@ -135,6 +136,8 @@
>>> char *tempstr = NULL;
>>> char *pparam = NULL;
>>>
>>> + fl_set_log_callback((fl_log_callback_t *)&fl_print_cb);
>>> +
>>> print_version();
>>> print_banner();
>>>
>>> Index: cli_output.c
>>> ===================================================================
>>> --- cli_output.c (revision 1891)
>>> +++ cli_output.c (working copy)
>>> @@ -71,9 +71,8 @@
>>> #endif /* !STANDALONE */
>>>
>>> /* Please note that level is the verbosity, not the importance of the
>>> message. */
>>> -int print(enum msglevel level, const char *fmt, ...)
>>> +int fl_print_cb(enum msglevel level, const char *fmt, va_list ap)
>>> {
>>> - va_list ap;
>>> int ret = 0;
>>> FILE *output_type = stdout;
>>>
>>> @@ -81,9 +80,7 @@
>>> output_type = stderr;
>>>
>>> if (level <= verbose_screen) {
>>> - va_start(ap, fmt);
>>> ret = vfprintf(output_type, fmt, ap);
>>> - va_end(ap);
>>> /* msg_*spew often happens inside chip accessors in possibly
>>> * time-critical operations. Don't slow them down by flushing. */
>>> if (level != MSG_SPEW)
>>> @@ -91,9 +88,7 @@
>>> }
>>> #ifndef STANDALONE
>>> if ((level <= verbose_logfile) && logfile) {
>>> - va_start(ap, fmt);
>>> ret = vfprintf(logfile, fmt, ap);
>>> - va_end(ap);
>>> if (level != MSG_SPEW)
>>> fflush(logfile);
>>> }
>>> Index: flash.h
>>> ===================================================================
>>> --- flash.h (revision 1891)
>>> +++ flash.h (working copy)
>>> @@ -31,6 +31,7 @@
>>> #include <stdint.h>
>>> #include <stddef.h>
>>> #include <stdbool.h>
>>> +#include <stdarg.h>
>>> #if IS_WINDOWS
>>> #include <windows.h>
>>> #undef min
>>> @@ -317,6 +318,7 @@
>>> MSG_DEBUG2 = 4,
>>> MSG_SPEW = 5,
>>> };
>>> +int fl_print_cb(enum msglevel level, const char *fmt, va_list ap);
>>> /* Let gcc and clang check for correct printf-style format strings. */
>>> int print(enum msglevel level, const char *fmt, ...)
>>> #ifdef __MINGW32__
>>> Index: libflashrom.c
>>> ===================================================================
>>> --- libflashrom.c (revision 0)
>>> +++ libflashrom.c (working copy)
>>> @@ -0,0 +1,603 @@
>>> +/*
>>> + * This file is part of the flashrom project.
>>> + *
>>> + * Copyright (C) 2012 secunet Security Networks AG
>>> + * (Written by Nico Huber <nico.huber at secunet.com> for secunet)
>>> + *
>>> + * 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.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program; if not, write to the Free Software
>>> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301
>>> USA
>>> + */
>>> +/**
>>> + * @mainpage
>>> + *
>>> + * Have a look at the Modules section for a function reference.
>>> + */
>>> +
>>> +#include <stdlib.h>
>>> +#include <string.h>
>>> +#include <stdarg.h>
>>> +
>>> +#include "flash.h"
>>> +#include "programmer.h"
>>> +#include "libflashrom.h"
>>> +
>>> +/**
>>> + * @defgroup fl-general General
>>> + * @{
>>> + */
>>> +
>>> +/** Pointer to log callback function. */
>>> +static fl_log_callback_t *fl_log_callback = NULL;
>>> +
>>> +/**
>>> + * @brief Initialize libflashrom.
>>> + *
>>> + * @param perform_selfcheck If not zero, perform a self check.
>>> + * @return 0 on success
>>> + */
>>> +int fl_init(const int perform_selfcheck)
>>> +{
>>> + if (perform_selfcheck && selfcheck())
>>> + return 1;
>>> + myusec_calibrate_delay();
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * @brief Shut down libflashrom.
>>> + * @return 0 on success
>>> + */
>>> +int fl_shutdown(void)
>>> +{
>>> + return 0; /* TODO: nothing to do? */
>> I'm pretty sure there is, but need to investigate more.
>>
>>> +}
>>> +
>>> +/* TODO: fl_set_loglevel()? do we need it?
>>> + For now, let the user decide in his callback. */
>>> +
>>> +/**
>>> + * @brief Set the log callback function.
>>> + *
>>> + * Set a callback function which will be invoked whenever libflashrom wants
>>> + * to output messages. This allows frontends to do whatever they see fit
>>> with
>>> + * such messages, e.g. write them to syslog, or to file, or print them in a
>>> + * GUI window, etc.
>>> + *
>>> + * @param log_callback Pointer to the new log callback function.
>>> + */
>>> +void fl_set_log_callback(fl_log_callback_t *const log_callback)
>>> +{
>>> + fl_log_callback = log_callback;
>>> +}
>>> +/** @private */
>>> +int print(const enum msglevel level, const char *const fmt, ...)
>>> +{
>>> + if (fl_log_callback) {
>>> + int ret;
>>> + va_list args;
>>> + va_start(args, fmt);
>>> + ret = fl_log_callback(level, fmt, args);
>>> + va_end(args);
>>> + return ret;
>>> + }
>>> + return 0;
>>> +}
>>> +
>>> +/** @} */ /* end fl-general */
>>> +
>>> +
>>> +
>>> +/**
>>> + * @defgroup fl-query Querying
>>> + * @{
>>> + */
>>> +
>>> +int fl_supported_programmers(const char **supported_programmers)
>>> +{
>>> + int ret = 0;
>>> + enum programmer p = 0;
>>> +
>>> + if (supported_programmers != NULL) {
>>> + for (; p < PROGRAMMER_INVALID; ++p)
>>> + supported_programmers[p] =
>>> programmer_table[p].name;
>>> + } else {
>>> + ret = 1;
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +int fl_supported_programmers_number()
>>> +{
>>> + return PROGRAMMER_INVALID;
>>> +}
>>> +
>>> +int fl_supported_flash_chips(fl_flashchip_info_t *fchips)
>>> +{
>>> + int ret = 0;
>>> + int i = 0;
>>> +
>>> + if (fchips != NULL) {
>>> + for (; i < flashchips_size; ++i) {
>>> + fchips[i].vendor = flashchips[i].vendor;
>>> + fchips[i].name = flashchips[i].name;
>>> + fchips[i].tested.erase =
>>> flashchips[i].tested.erase;
>>> + fchips[i].tested.probe =
>>> flashchips[i].tested.probe;
>>> + fchips[i].tested.read = flashchips[i].tested.read;
>>> + fchips[i].tested.write =
>>> flashchips[i].tested.write;
>>> + fchips[i].total_size = flashchips[i].total_size;
>>> + }
>>> + } else {
>>> + ret = 1;
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +int fl_supported_flash_chips_number()
>>> +{
>>> + return flashchips_size;
>>> +}
>>> +
>>> +int fl_supported_boards(fl_board_info_t *boards)
>>> +{
>>> + int ret = 0;
>>> + const struct board_info *binfo = boards_known;
>>> +
>>> + if (boards != NULL) {
>>> + while (binfo->vendor != NULL) {
>>> + boards->vendor = binfo->vendor;
>>> + boards->name = binfo->name;
>>> + boards->working = binfo->working;
>>> + ++binfo;
>>> + ++boards;
>>> + }
>>> + } else {
>>> + ret = 1;
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +int fl_supported_boards_number()
>>> +{
>>> + int boards_number = 0;
>>> + const struct board_info *binfo = boards_known;
>>> +
>>> + while (binfo->vendor != NULL) {
>>> + ++boards_number;
>>> + ++binfo;
>>> + }
>>> +
>>> + return boards_number;
>>> +}
>>> +
>>> +int fl_supported_chipsets(fl_chipset_info_t *chipsets)
>>> +{
>>> + int ret = 0;
>>> + const struct penable *chipset = chipset_enables;
>>> +
>>> + if (chipsets != NULL) {
>>> + while (chipset->vendor_name != NULL) {
>>> + chipsets->vendor = chipset->vendor_name;
>>> + chipsets->chipset = chipset->device_name;
>>> + chipsets->status = chipset->status;
>>> + ++chipset;
>>> + ++chipsets;
>> These names are confusing (just FYI), i first thought we're counting
>> chipsets here..
>>> + }
>>> + } else {
>>> + return ret;
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +int fl_supported_chipsets_number()
>>> +{
>>> + int chipsets_number = 0;
>>> + const struct penable *chipset = chipset_enables;
>>> +
>>> + while (chipset->vendor_name != NULL) {
>>> + ++chipsets_number;
>>> + ++chipset;
>>> + }
>>> +
>>> + return chipsets_number;
>>> +}
>>> +
>>> +/** @} */ /* end fl-query */
>>> +
>>> +
>>> +
>>> +/**
>>> + * @defgroup fl-prog Programmers
>>> + * @{
>>> + */
>>> +
>>> +/**
>>> + * @brief Initialize the specified programmer.
>>> + *
>>> + * @param prog_name Name of the programmer to initialize.
>>> + * @param prog_param Pointer to programmer specific parameters.
>>> + * @return 0 on success
>>> + */
>>> +int fl_programmer_init(const char *const prog_name, const char *const
>>> prog_param)
>>> +{
>>> + unsigned prog;
>>> +
>>> + for (prog = 0; prog < PROGRAMMER_INVALID; prog++) {
>>> + if (strcmp(prog_name, programmer_table[prog].name) == 0)
>>> + break;
>>> + }
>>> + if (prog >= PROGRAMMER_INVALID) {
>>> + msg_ginfo("Error: Unknown programmer \"%s\". Valid choices
>>> are:\n", prog_name);
>>> + list_programmers_linebreak(0, 80, 0);
>>> + return 1;
>>> + }
>>> + return programmer_init(prog, prog_param);
>>> +}
>>> +
>>> +/**
>>> + * @brief Shut down the initialized programmer.
>>> + *
>>> + * @return 0 on success
>>> + */
>>> +int fl_programmer_shutdown(void)
>>> +{
>>> + return programmer_shutdown();
>>> +}
>>> +
>>> +/* TODO: fl_programmer_capabilities()? */
>>> +
>>> +/** @} */ /* end fl-prog */
>>> +
>>> +
>>> +
>>> +/**
>>> + * @defgroup fl-flash Flash chips
>>> + * @{
>>> + */
>>> +
>>> +/**
>>> + * @brief Probe for a flash chip.
>>> + *
>>> + * Probes for a flash chip and returns a flash context, that can be used
>>> + * later with flash chip and @ref fl-ops "image operations", if exactly one
>>> + * matching chip is found.
>>> + *
>>> + * @param[out] flashctx Points to a pointer of type fl_flashctx_t that will
>>> + * be set if exactly one chip is found. *flashctx has
>>> + * to be freed by the caller with @ref
>>> fl_flash_release.
>>> + * @param[in] chip_name Name of a chip to probe for, or NULL to probe for
>>> + * all known chips.
>>> + * @return 0 on success,
>>> + * 3 if multiple chips were found,
>>> + * 2 if no chip was found,
>>> + * or 1 on any other error.
>>> + */
>> Need a way to output multiple flash chips from here, it shouldnt be an
>> error, just a
>> "pick from these" dialog and continue with "OK", or something like that.
>> Maybe add int *chip_count as a parameter and the set pointer will
>> point to an array of flashctx with the int set to the count of found
>> chips.
>> (Hopefully I'm not saying totally obvious stuff here...)
>>
>>> +int fl_flash_probe(fl_flashctx_t **const flashctx, const char *const
>>> chip_name)
>>> +{
>>> + int i, ret = 2;
>>> + fl_flashctx_t second_flashctx = { 0, };
>>> +
>>> + chip_to_probe = chip_name; /* chip_to_probe is global in flashrom.c
>>> */
>>> +
>>> + *flashctx = malloc(sizeof(**flashctx));
>>> + if (!*flashctx)
>>> + return 1;
>>> + memset(*flashctx, 0, sizeof(**flashctx));
>>> +
>>> + /* LD_CHANGE_NOTE for (i = 0; i < registered_programmer_count; ++i)
>>> {
>>> + *
>>> + * Reason of change: registered_programmer_count does not exist, I
>>> assume that a proper one is
>>> + * now registered_master_count - I will check and confirm
>>> + */
>>> + for (i = 0; i < registered_master_count; ++i) {
>>> + int flash_idx = -1;
>>> + /* LD_CHANGE_NOTE if (!ret || (flash_idx =
>>> probe_flash(®istered_programmers[i], 0, *flashctx, 0)) != -1) {
>>> + *
>>> + * Reason of change: registered_programmers does not exist,
>>> I assume that a proper one is
>>> + * now registered_masters - I will check and confirm
>>> + */
>>> + if (!ret || (flash_idx =
>>> probe_flash(®istered_masters[i], 0, *flashctx, 0)) != -1) {
>>> + ret = 0;
>>> + /* We found one chip, now check that there is no
>>> second match. */
>>> + /* LD_CHANGE_NOTE if
>>> (probe_flash(®istered_programmers[i], flash_idx + 1, &second_flashctx, 0)
>>> != -1) {
>>> + *
>>> + * Reason of change: registered_programmers does
>>> not exist, I assume that a proper one is
>>> + * now registered_masters - I will check and
>>> confirm
>>> + *
>>> + */
>>> + if (probe_flash(®istered_masters[i], flash_idx +
>>> 1, &second_flashctx, 0) != -1) {
>>> + ret = 3;
>>> + break;
>>> + }
>>> + }
>>> + }
>>> + if (ret) {
>>> + free(*flashctx);
>>> + *flashctx = NULL;
>>> + }
>>> + return ret;
>>> +}
>>> +
>>> +/**
>>> + * @brief Returns the size of the specified flash chip in bytes.
>>> + *
>>> + * @param flashctx The queried flash context.
>>> + * @return Size of flash chip in bytes.
>>> + */
>>> +size_t fl_flash_getsize(const fl_flashctx_t *const flashctx)
>>> +{
>>> + return flashctx->chip->total_size << 10;
>>> +}
>>> +
>>> +/** @private */
>>> +int erase_and_write_flash(struct flashctx *flash, uint8_t *oldcontents,
>>> uint8_t *newcontents);
>>> +/** @private */
>>> +/* LD_CHANGE_NOTE void emergency_help_message(void);
>>> + *
>>> + * Reason of change: This has been commented out as emergency_help_message
>>> is static
>> Maybe have some other method to indicate that the UI should display an
>> emergency help message, maybe ret = 2. Same comment for the _write
>> version, but that already has ret values for these.
>>
>>> + */
>>> +/**
>>> + * @brief Erase the specified ROM chip.
>>> + *
>>> + * @param flashctx The context of the flash chip to erase.
>>> + * @return 0 on success.
>>> + */
>>> +int fl_flash_erase(fl_flashctx_t *const flashctx)
>>> +{
>>> + const size_t flash_size = flashctx->chip->total_size * 1024;
>>> +
>>> + int ret = 0;
>>> +
>>> + uint8_t *const newcontents = malloc(flash_size);
>>> + if (!newcontents) {
>>> + msg_gerr("Out of memory!\n");
>>> + return 1;
>>> + }
>>> + uint8_t *const oldcontents = malloc(flash_size);
>>> + if (!oldcontents) {
>>> + msg_gerr("Out of memory!\n");
>>> + free(newcontents);
>>> + return 1;
>>> + }
>>> +
>>> + if (flashctx->chip->unlock)
>>> + flashctx->chip->unlock(flashctx);
>>> +
>>> + /* Assume worst case for old contents: All bits are 0. */
>>> + memset(oldcontents, 0x00, flash_size);
>>> + /* Assume best case for new contents: All bits should be 1. */
>>> + memset(newcontents, 0xff, flash_size);
>>> + /* Side effect of the assumptions above: Default write action is
>>> erase
>>> + * because newcontents looks like a completely erased chip, and
>>> + * oldcontents being completely 0x00 means we have to erase
>>> everything
>>> + * before we can write.
>>> + */
>>> +
>>> + if (erase_and_write_flash(flashctx, oldcontents, newcontents)) {
>>> + /* FIXME: Do we really want the scary warning if erase
>>> failed?
>>> + * After all, after erase the chip is either blank or
>>> partially
>>> + * blank or it has the old contents. A blank chip won't
>>> boot,
>>> + * so if the user wanted erase and reboots afterwards, the
>>> user
>>> + * knows very well that booting won't work.
>>> + */
>>> + /* LD_CHANGE_NOTE emergency_help_message();
>>> + *
>>> + * Reason of change: This function is static
>>> + */
>>> + ret = 1;
>>> + }
>>> +
>>> + free(oldcontents);
>>> + free(newcontents);
>>> + return ret;
>>> +}
>>> +
>>> +/**
>>> + * @brief Free a flash context.
>>> + *
>>> + * @param flashctx Flash context to free.
>>> + */
>>> +void fl_flash_release(fl_flashctx_t *const flashctx)
>>> +{
>>> + free(flashctx);
>>> +}
>>> +
>>> +/** @} */ /* end fl-flash */
>>> +
>>> +
>>> +
>>> +/**
>>> + * @defgroup fl-ops Operations
>>> + * @{
>>> + */
>>> +
>>> +/**
>>> + * @brief Read the current image from the specified ROM chip.
>>> + *
>>> + * @param flashctx The context of the flash chip.
>>> + * @param buffer Target buffer to write image to.
>>> + * @param buffer_len Size of target buffer in bytes.
>>> + * @return 0 on success,
>>> + * 2 if buffer_len is to short for the flash chip's contents,
>>> + * or 1 on any other failure.
>>> + */
>>> +int fl_image_read(fl_flashctx_t *const flashctx, void *const buffer, const
>>> size_t buffer_len)
>>> +{
>>> + const size_t flash_size = flashctx->chip->total_size * 1024;
>>> +
>>> + int ret = 0;
>>> +
>>> + if (flashctx->chip->unlock)
>>> + flashctx->chip->unlock(flashctx);
>>> +
>>> + msg_cinfo("Reading flash... ");
>>> + if (flash_size > buffer_len) {
>>> + msg_cerr("Buffer to short for this flash chip (%u <
>>> %u).\n",
>>> + (unsigned int)buffer_len, (unsigned
>>> int)flash_size);
>>> + ret = 2;
>>> + goto _out;
>>> + }
>>> + if (!flashctx->chip->read) {
>>> + msg_cerr("No read function available for this flash
>>> chip.\n");
>>> + ret = 1;
>>> + goto _out;
>>> + }
>>> + if (flashctx->chip->read(flashctx, buffer, 0, flash_size)) {
>>> + msg_cerr("Read operation failed!\n");
>>> + ret = 1;
>>> + goto _out;
>>> + }
>>> +_out:
>>> + msg_cinfo("%s.\n", ret ? "FAILED" : "done");
>>> + return ret;
>>> +}
>>> +
>>> +/** @private */
>>> +/* LD_CHANGE_NOTE void nonfatal_help_message(void);
>>> + *
>>> + * Reason of change: This function is static
>>> + */
>>> +/**
>>> + * @brief Write the specified image to the ROM chip.
>>> + *
>>> + * @param flashctx The context of the flash chip.
>>> + * @param buffer Source buffer to read image from.
>>> + * @param buffer_len Size of source buffer in bytes.
>>> + * @return 0 on success,
>>> + * 4 if buffer_len doesn't match the size of the flash chip,
>>> + * 3 if write was tried, but nothing has changed,
>>> + * 2 if write was tried, but flash contents changed,
>>> + * or 1 on any other failure.
>>> + */
>>> +int fl_image_write(fl_flashctx_t *const flashctx, void *const buffer, const
>>> size_t buffer_len)
>>> +{
>>> + const size_t flash_size = flashctx->chip->total_size * 1024;
>>> +
>>> + int ret = 0;
>>> +
>>> + if (buffer_len != flash_size) {
>>> + msg_cerr("Buffer size doesn't match size of flash chip (%u
>>> != %u)\n.",
>>> + (unsigned int)buffer_len, (unsigned
>>> int)flash_size);
>>> + return 4;
>>> + }
>>> +
>>> + uint8_t *const newcontents = buffer;
>>> + uint8_t *const oldcontents = malloc(flash_size);
>>> + if (!oldcontents) {
>>> + msg_gerr("Out of memory!\n");
>>> + return 1;
>>> + }
>>> + if (fl_image_read(flashctx, oldcontents, flash_size)) {
>>> + ret = 1;
>>> + goto _free_out;
>>> + }
>>> +
>>> + /* LD_CHANGE_NOTE handle_romentries(flashctx, oldcontents,
>>> newcontents);
>>> + *
>>> + * Reason of change: This function does not exist. There is a need
>>> to investigate what was
>>> + * its purpose and how it can be replaced.
>>> + */
>>> +
>>> + if (erase_and_write_flash(flashctx, oldcontents, newcontents)) {
>>> + msg_cerr("Uh oh. Erase/write failed. Checking if anything
>>> changed.\n");
>>> + if (!flashctx->chip->read(flashctx, newcontents, 0,
>>> flash_size)) {
>>> + if (!memcmp(oldcontents, newcontents, flash_size))
>>> {
>>> + msg_cinfo("Good. It seems nothing was
>>> changed.\n");
>>> + /* LD_CHANGE_NOTE nonfatal_help_message();
>>> + *
>>> + * Reason of change: This function is
>>> static
>>> + */
>>> + ret = 3;
>>> + goto _free_out;
>>> + }
>>> + }
>>> + /* LD_CHANGE_NOTE emergency_help_message();
>>> + *
>>> + * Reason of change: This function is static
>>> + */
>>> + ret = 2;
>>> + goto _free_out;
>>> + }
>>> +
>>> +_free_out:
>>> + free(oldcontents);
>>> + return ret;
>>> +}
>>> +
>>> +/** @private */
>>> +/* LD_CHANGE_NOTE int compare_range(const uint8_t *wantbuf, const uint8_t
>>> *havebuf, unsigned int start, unsigned int len);
>>> + *
>>> + * Reason of change: This function is static
>> For any of these "this is static"... then make it not static or figure
>> out a cleaner way of doing it, but yes i get this is an early patch.
>>
>>> + */
>>> +/**
>>> + * @brief Verify the ROM chip's contents with the specified image.
>>> + *
>>> + * @param flashctx The context of the flash chip.
>>> + * @param buffer Source buffer to verify with.
>>> + * @param buffer_len Size of source buffer in bytes.
>>> + * @return 0 on success,
>>> + * 2 if buffer_len doesn't match the size of the flash chip,
>>> + * or 1 on any other failure.
>>> + */
>>> +int fl_image_verify(fl_flashctx_t *const flashctx, void *const buffer,
>>> const size_t buffer_len)
>>> +{
>>> + const size_t flash_size = flashctx->chip->total_size * 1024;
>>> +
>>> + int ret = 0;
>>> +
>>> + if (buffer_len != flash_size) {
>>> + msg_cerr("Buffer size doesn't match size of flash chip (%u
>>> != %u)\n.",
>>> + (unsigned int)buffer_len, (unsigned
>>> int)flash_size);
>>> + return 2;
>>> + }
>>> +
>>> + /* LD_CHANGE_NOTE uint8_t *const newcontents = buffer; - used only
>>> in handle_romentries() function
>>> + *
>>> + * Reason of change: This pointer is used only in handle_romentries
>>> function, which has been
>>> + * commented out
>>> + */
>>> + uint8_t *const oldcontents = malloc(flash_size);
>>> + if (!oldcontents) {
>>> + msg_gerr("Out of memory!\n");
>>> + return 1;
>>> + }
>>> + if (fl_image_read(flashctx, oldcontents, flash_size)) {
>>> + ret = 1;
>>> + goto _free_out;
>>> + }
>>> +
>>> + /* LD_CHANGE_NOTE handle_romentries(flashctx, oldcontents,
>>> newcontents);
>>> + *
>>> + * Reason of change: This function does not exist
>>> + */
>>> +
>>> + msg_cinfo("Verifying flash... ");
>>> +
>>> + /* LD_CHANGE_NOTE ret = compare_range(newcontents, oldcontents, 0,
>>> flash_size);
>>> + *
>>> + * Reason of change: This function is static. For now replaced with
>>> ret = 1 as ret must be used
>>> + */
>>> + ret = 1;
>>> + if (!ret)
>>> + msg_cinfo("VERIFIED.\n");
>>> +
>>> +_free_out:
>>> + free(oldcontents);
>>> + return ret;
>>> +}
>>> +
>>> +/** @} */ /* end fl-ops */
>>> Index: libflashrom.h
>>> ===================================================================
>>> --- libflashrom.h (revision 0)
>>> +++ libflashrom.h (working copy)
>>> @@ -0,0 +1,97 @@
>>> +/*
>>> + * This file is part of the flashrom project.
>>> + *
>>> + * Copyright (C) 2012 secunet Security Networks AG
>>> + * (Written by Nico Huber <nico.huber at secunet.com> for secunet)
>>> + *
>>> + * 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.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program; if not, write to the Free Software
>>> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301
>>> USA
>>> + */
>>> +
>>> +#ifndef __LIBFLASHROM_H__
>>> +#define __LIBFLASHROM_H__ 1
>>> +
>>> +#include <stdarg.h>
>>> +
>>> +int fl_init(int perform_selfcheck);
>>> +int fl_shutdown(void);
>>> +/** @ingroup fl-general */
>>> +typedef enum { /* This has to match enum msglevel. */
>> Maybe you could just bring enum msglevel here from flash.h...
>> or have some other header for this printing stuff thats
>> used by both libflashrom.h and other parts of flashrom.
>>
>>> + FL_MSG_ERROR = 0,
>>> + FL_MSG_INFO = 1,
>>> + FL_MSG_DEBUG = 2,
>>> + FL_MSG_DEBUG2 = 3,
>>> + FL_MSG_SPEW = 4,
>>> +} fl_log_level_t;
>>> +/** @ingroup fl-general */
>>> +typedef int(fl_log_callback_t)(fl_log_level_t, const char *format,
>>> va_list);
>>> +void fl_set_log_callback(fl_log_callback_t *);
>>> +
>>> +typedef enum {
>>> + FL_TESTED_OK = 0,
>>> + FL_TESTED_NT = 1,
>>> + FL_TESTED_BAD = 2,
>>> + FL_TESTED_DEP = 3,
>>> + FL_TESTED_NA = 4,
>>> +} fl_test_state;
>>> +
>>> +typedef struct {
>>> + const char *vendor;
>>> + const char *name;
>>> + unsigned int total_size;
>>> +
>>> + struct fl_tested {
>>> + fl_test_state probe;
>>> + fl_test_state read;
>>> + fl_test_state erase;
>>> + fl_test_state write;
>>> + } tested;
>>> +} fl_flashchip_info_t;
>>> +
>>> +typedef struct {
>>> + const char *vendor;
>>> + const char *name;
>>> + fl_test_state working;
>>> +} fl_board_info_t;
>>> +
>>> +typedef struct {
>>> + const char *vendor;
>>> + const char *chipset;
>>> + fl_test_state status;
>>> +} fl_chipset_info_t;
>>> +
>>> +int fl_supported_programmers(const char **supported_programmers);
>>> +int fl_supported_flash_chips(fl_flashchip_info_t *fchips);
>>> +int fl_supported_boards(fl_board_info_t *boards);
>>> +int fl_supported_chipsets(fl_chipset_info_t *chipsets);
>>> +int fl_supported_programmers_number();
>>> +int fl_supported_flash_chips_number();
>>> +int fl_supported_boards_number();
>>> +int fl_supported_chipsets_number();
>>> +
>>> +int fl_programmer_init(const char *prog_name, const char *prog_params);
>>> +int fl_programmer_shutdown(void);
>>> +
>>> +struct flashctx;
>>> +typedef struct flashctx fl_flashctx_t;
>>> +int fl_flash_probe(fl_flashctx_t **, const char *chip_name);
>>> +size_t fl_flash_getsize(const fl_flashctx_t *);
>>> +int fl_flash_erase(fl_flashctx_t *);
>>> +void fl_flash_release(fl_flashctx_t *);
>>> +
>>> +int fl_image_read(fl_flashctx_t *, void *buffer, size_t buffer_len);
>>> +int fl_image_write(fl_flashctx_t *, void *buffer, size_t buffer_len);
>>> +int fl_image_verify(fl_flashctx_t *, void *buffer, size_t buffer_len);
>>> +
>>> +#endif /* !__LIBFLASHROM_H__ */
>>>
>>> _______________________________________________
>>> flashrom mailing list
>>> flashrom at flashrom.org
>>> http://www.flashrom.org/mailman/listinfo/flashrom
>>
>> --
>> Urja Rannikko
>>
>> _______________________________________________
>> flashrom mailing list
>> flashrom at flashrom.org
>> http://www.flashrom.org/mailman/listinfo/flashrom
More information about the flashrom
mailing list