Sorry again, that was old tree, I will rearrange patches and will push updated version in a hour or so. Best regards, Anton Kochkov.
On Fri, Jun 19, 2015 at 2:50 PM, Антон Кочков <[email protected]> wrote: > 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, Антон Кочков <[email protected]> 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 <[email protected]> wrote: >>> Hi, >>> >>> >>> On Fri, Jun 12, 2015 at 12:26 AM, Łukasz Dmitrowski >>> <[email protected]> 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 <[email protected]> >>>> >>>> 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 <[email protected]> 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 <[email protected]> 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 >>>> [email protected] >>>> http://www.flashrom.org/mailman/listinfo/flashrom >>> >>> -- >>> Urja Rannikko >>> >>> _______________________________________________ >>> flashrom mailing list >>> [email protected] >>> http://www.flashrom.org/mailman/listinfo/flashrom _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
