Re: [U-Boot] [PATCH v3] socfpga: Add a signer that is integrated into mkimage

Dear Charles,

would you please start following the established rules? Especially after having been asked before to to so?

In http://article.gmane.org/gmane.comp.boot-loaders.u-boot/180904 I asked you:

| Would you please read [1] and especially [2], the section about | posting modified versions of patches ? | | You are posting multiple patches with the same subject but different | content. Do you think we have time to figure out what might be the | difference? Please make sure to include a version tagin the Subject: | line, and to add a history oof changes in the comment section. | | And - do you think that "This one passes checkpatch too :-)" is a | helpful and descriptive commit message? [No, it is not.] | | [1] http://www.denx.de/wiki/U-Boot/Patches | [2] http://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions

In message 1393114104-7301-1-git-send-email-cdhmanning@gmail.com you wrote:

This adds a signer for the socfpga preloader built from SPL.

Instead of using the arcane Altera signing tool, this automatically creates a signed version of the SPL in the u-boot root directory.

Changes since previous submissions:

• This version is integrated into mkimage, with image type socfpgaimage.
• This version passes checkpatch too :-)

This belongs into the comment section, i. e. below the "---" line.

Signed-off-by: Charles Manning cdhmanning@gmail.com

This is the place where comments go.

--- a/common/image.c +++ b/common/image.c @@ -144,6 +144,7 @@ static const table_entry_t uimage_type[] = { { IH_TYPE_INVALID, NULL, "Invalid Image", }, { IH_TYPE_MULTI, "multi", "Multi-File Image", }, { IH_TYPE_OMAPIMAGE, "omapimage", "TI OMAP SPL With GP CH",},

• { IH_TYPE_SOCFPGAIMAGE, "socfpgaimage", "Altera SOCFPGA preloader",}, { IH_TYPE_PBLIMAGE, "pblimage", "Freescale PBL Boot Image",}, { IH_TYPE_RAMDISK, "ramdisk", "RAMDisk Image", }, { IH_TYPE_SCRIPT, "script", "Script", },

I asked before:

| Please always keep such lists sorted.

Why do you ignore review comments?

+++ b/spl/Makefile @@ -144,8 +144,12 @@ $(OBJTREE)/MLO: $(obj)u-boot-spl.bin

$(OBJTREE)/MLO.byteswap: $(obj)u-boot-spl.bin $(OBJTREE)/tools/mkimage -T omapimage -n byteswap \

• -a $(CONFIG_SPL_TEXT_BASE) -d $< $@

Me and Gerhard have asked you about this. Is this eve working? I doubt that. But you ignore all such comments.

+ifdef CONFIG_SOCFPGA +ALL-y += $(OBJTREE)/socfpga-signed-preloader.bin +endif

I commented on this - you ignored it.

@@ -95,6 +95,7 @@ NOPED_OBJ_FILES-y += kwbimage.o NOPED_OBJ_FILES-y += pblimage.o NOPED_OBJ_FILES-y += imximage.o NOPED_OBJ_FILES-y += omapimage.o +NOPED_OBJ_FILES-y += socfpgaimage.o NOPED_OBJ_FILES-y += mkenvimage.o NOPED_OBJ_FILES-y += mkimage.o OBJ_FILES-$(CONFIG_SMDK5250) += mkexynosspl.o

I commented on this - you ignored it.

@@ -214,6 +215,7 @@ $(obj)mkimage$(SFX): $(obj)aisimage.o \ $(obj)mkimage.o \ $(obj)os_support.o \ $(obj)omapimage.o \

• 	$(obj)socfpgaimage.o \
  $(obj)sha1.o \
  $(obj)ublimage.o \
  $(LIBFDT_OBJS)
  

I commented on this - you ignored it.

...

• • Reference doc http://www.altera.com.cn/literature/hb/cyclone-v/cv_5400A.pdf
• • Note this doc is not entirely accurate.

I asked for more information which parts of the doc are not correct. You ignored this.

• • This uses the CRC32 calc out of the well known Apple
• • crc32.c code. Copyright for the CRC code:
• • COPYRIGHT (C) 1986 Gary S. Brown. You may use this program, or
• • code or tables extracted from it, as desired without restriction.

Many more ignored review comments folow.

Sorry, but this is not the way how code reviews are supposed to work.

NAK!!!

Wolfgang Denk