[U-Boot] [PATCH 2/4] tools: moved code common to all image tools to a separated module.

[Simon Glass]

Hi,
\
On Tue, Sep 17, 2013 at 6:41 PM, <guilherme.maciel.ferreira at gmail.com>wrote:

> From: Guilherme Maciel Ferreira <guilherme.maciel.ferreira at gmail.com>
>
> In order to avoid duplicating code and keep only one point of modification,
> the functions, structs and defines useful for "dumpimage" were moved from
> "mkimage" to a common module called "imagetool".
>
> This modification also weakens the coupling between image types (FIT, IMX,
> MXS,
> and so on) and image tools (mkimage and dumpimage). Any tool may
> initialize the
> "imagetool" through register_image_tool() function, while the image types
> register themselves within an image tool using the register_image_type()
> function:
>
>                                                       +---------------+
>                                                +------|   fit_image   |
>  +--------------+          +-----------+       |      +---------------+
>  |    mkimage   |--------> |           | <-----+
>  +--------------+          |           |              +---------------+
>                            | imagetool | <------------|    imximage   |
>  +--------------+          |           |              +---------------+
>  |  dumpimage   |--------> |           | <-----+
>  +--------------+          +-----------+       |      +---------------+
>                                                +------| default_image |
>                                                       +---------------+
>
>           register_image_tool()           register_image_type()
>
> Also, the struct "mkimage_params" was renamed to "image_tool_params" to
> make
> clear its general purpose.
>
> Signed-off-by: Guilherme Maciel Ferreira <
> guilherme.maciel.ferreira at gmail.com>
>

This seems like a reasonable plan to me. But please check your code style
with patman or checkpatch. You have space before (.

Also a suggestion - instead of register_image_tool() you might be able to
use the linker_list feature? It could automatically find the image types to
register a bit like we do with U_BOOT_CMD().




> ---
>  tools/Makefile        |    2 +
>  tools/aisimage.c      |   16 +++---
>  tools/default_image.c |   10 ++--
>  tools/fit_image.c     |   11 ++--
>  tools/imagetool.c     |   58 ++++++++++++++++++
>  tools/imagetool.h     |  159
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  tools/imximage.c      |   12 ++--
>  tools/kwbimage.c      |   10 ++--
>  tools/mkimage.c       |   24 ++------
>  tools/mkimage.h       |  123 +-------------------------------------
>  tools/mxsimage.c      |   12 ++--
>  tools/omapimage.c     |   10 ++--
>  tools/pblimage.c      |   10 ++--
>  tools/ublimage.c      |   10 ++--
>  14 files changed, 276 insertions(+), 191 deletions(-)
>  create mode 100644 tools/imagetool.c
>  create mode 100644 tools/imagetool.h
>
>
Regards,
Simon