All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Yann E. MORIN" <yann.morin.1998@free.fr>
To: Brandon Maier <brandon.maier@collins.com>
Cc: Steve Kenton <skenton@ou.edu>,
	Abhishek Anand <abhishek.anand@collins.com>,
	Kalpesh Panchal <kalpesh.panchal2@collins.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 1/2] package/memtest86: bump to version 6.20
Date: Sat, 18 Nov 2023 19:30:37 +0100	[thread overview]
Message-ID: <20231118183037.GJ996011@scaer> (raw)
In-Reply-To: <20231110210001.58405-1-brandon.maier@collins.com>

Kalpesh, Brandon, All,

On 2023-11-10 20:59 +0000, Brandon Maier via buildroot spake thusly:
> From: Kalpesh Panchal <kalpesh.panchal2@collins.com>
> 
> - bumped package version to 6.20
> - as 6.20 prebuilt version is not available on same download site but
>   source code is released at https://github.com/memtest86plus/memtest86plus/releases
>   so updated mk file to download source from git release
> - prebuilt binary not found in 6.20 package and also README.md
>   does not mention any toolchain limitation for compilation
>   so added source build command depending on 32-bit or 64-bit x86 target.
> - corrected License file name
> - installed memtest binaries to image folder like other bootable
>   images(grub , kernel) are copied.
> - updated config help as per package README.md
> 
> Signed-off-by: Kalpesh Panchal <kalpesh.panchal2@collins.com>
> Signed-off-by: Abhishek Anand <abhishek.anand@collins.com>
> Signed-off-by: Brandon Maier <brandon.maier@collins.com>

Thanks for this patch, here are a few comments:

> ---
>  package/memtest86/Config.in      | 30 ++++++++++++++++++++++--------
>  package/memtest86/memtest86.hash |  4 ++--
>  package/memtest86/memtest86.mk   | 26 +++++++++++++++++++-------
>  3 files changed, 43 insertions(+), 17 deletions(-)
> 
> diff --git a/package/memtest86/Config.in b/package/memtest86/Config.in
> index 79c0161020..81c69e0222 100644
> --- a/package/memtest86/Config.in
> +++ b/package/memtest86/Config.in
> @@ -14,14 +14,28 @@ config BR2_PACKAGE_MEMTEST86
>  	  back again and verifies whether the result of the read is the
>  	  same as what was written to memory.
>  
> -	  Memtest86+ will only work on 32-bit or 64-bit x86 targets.
> -	  It boots as an i486 program and autodetects hardware. It can
> -	  be added to the grub2 boot menu by adding the following
> -	  lines to the bottom of /boot/grub/grub.cfg - note the use of
> -	  linux16.
> -
> -	  menuentry "Memtest86+" {
> -	    linux16 /boot/memtest86+.bin
> +	  Memtest86+ will only work on 32-bit or 64-bit x86 targets. It
> +	  boots and autodetects hardware. Refer README.md for boot
> +	  options.
> +
> +	  It can be loaded and run either directly by a PC BIOS (legacy
> +	  or UEFI) or via an intermediate bootloader that supports the
> +	  Linux 16-bit, 32-bit, 64-bit, or EFI handover boot protocol.
> +
> +	  It can be added to the grub2 boot menu by adding the following
> +	  lines to the bottom of grub.cfg
> +
> +	  Example for legacy BIOS using the Linux 16-bit boot protocol
> +
> +	  menuentry "Start Memtest86+, use built-in support for USB keyboards" {
> +	      linux16 /boot/memtest.bin console=ttyS0,115200 keyboard=both
> +	  }
> +
> +	  Or for UEFI BIOS using the Linux 32-bit or 64-bit boot
> +	  protocols.
> +
> +	  menuentry "Start Memtest86+, use built-in support for USB keyboards" {
> +	      linux /EFI/BOOT/memtest.efi console=ttyS0,115200 keyboard=both
>  	  }

    $ ./utils/docker-run make check-package
    package/memtest86/Config.in:30: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)
    package/memtest86/Config.in:31: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)
    package/memtest86/Config.in:37: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)
    package/memtest86/Config.in:38: help text: <tab><2 spaces><62 chars> (http://nightly.buildroot.org/#writing-rules-config-in)

I've rewrittten the help text to avoid this limitation.

>  	  Other boot loaders will have similar requirements.
> diff --git a/package/memtest86/memtest86.hash b/package/memtest86/memtest86.hash
> index 559ae0ac6c..df4699c63b 100644
> --- a/package/memtest86/memtest86.hash
> +++ b/package/memtest86/memtest86.hash
> @@ -1,3 +1,3 @@
>  # locally computed using sha256sum
> -sha256  142127b7953fbd829b1057fb64a78d3340c2b771484230a7347e94530a0d9039  memtest86+-5.01.tar.gz
> -sha256  2e15e2174b86640d7fbfcb62b51d9182062d9db71d66a46e5b01d736c68150ea  README
> +sha256  463b2db66128e45c96be76136272e2ee940f1319804b1b88cc2ef04edccd01d8  memtest86+-6.20.tar.gz
--------------------------------------------------------------------------------------^

    $ make memtest86-source
    >>> memtest86 6.20 Downloading
    [...]
    ERROR: No hash found for memtest86-6.20.tar.gz
    make[1]: *** [package/pkg-generic.mk:179: /home/ymorin/dev/buildroot/O/next/build/memtest86-6.20/.stamp_downloaded] Error 1
    make: *** [Makefile:23: _all] Error 2

I've fixed that as well.

> +sha256  8177f97513213526df2cf6184d8ff986c675afb514d4e68a404010521b880643  LICENSE
> diff --git a/package/memtest86/memtest86.mk b/package/memtest86/memtest86.mk
> index eb071cc29d..332157ab26 100644
> --- a/package/memtest86/memtest86.mk
> +++ b/package/memtest86/memtest86.mk
> @@ -4,15 +4,27 @@
>  #
>  ################################################################################
>  
> -MEMTEST86_VERSION = 5.01
> -MEMTEST86_SOURCE = memtest86+-$(MEMTEST86_VERSION).tar.gz
> -MEMTEST86_SITE = http://www.memtest.org/download/$(MEMTEST86_VERSION)
> +MEMTEST86_VERSION = 6.20
> +MEMTEST86_SITE = $(call github,memtest86plus,memtest86plus,v$(MEMTEST86_VERSION))
>  MEMTEST86_LICENSE = GPL-2.0
> -MEMTEST86_LICENSE_FILES = README
> +MEMTEST86_LICENSE_FILES = LICENSE
> +MEMTEST86_INSTALL_IMAGES = YES
> +MEMTEST86_INSTALL_TARGET = NO
>  
> -# memtest86+ is sensitive to toolchain changes, use the shipped binary version
> -define MEMTEST86_INSTALL_TARGET_CMDS
> -	$(INSTALL) -m 0755 -D $(@D)/precomp.bin $(TARGET_DIR)/boot/memtest86+.bin
> +ifeq ($(BR2_i386),y)
> +MEMTEST86_BUILD_DIR = build32
> +else
> +ifeq ($(BR2_x86_64),y)
> +MEMTEST86_BUILD_DIR = build64
> +endif
> +endif
> +
> +define MEMTEST86_BUILD_CMDS
> +	$(TARGET_CONFIGURE_OPTS) $(MAKE) -C $(@D)/$(MEMTEST86_BUILD_DIR)
> +endef
> +
> +define MEMTEST86_INSTALL_IMAGES_CMDS
> +	$(INSTALL) -m 0755 -D $(@D)/$(MEMTEST86_BUILD_DIR)/memtest.* $(BINARIES_DIR)/

When the destination of installation is a directory, one must ensure the
directory exists beforehand, with an explicit mkdir first.

But here, I've changed that to install the two files explicitly.

Applied to next with the above changes, thanks.

Regards,
Yann E. MORIN.

>  endef
>  
>  $(eval $(generic-package))
> -- 
> 2.41.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

      parent reply	other threads:[~2023-11-18 18:30 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-10 20:59 [Buildroot] [PATCH 1/2] package/memtest86: bump to version 6.20 Brandon Maier via buildroot
2023-11-10 21:00 ` [Buildroot] [PATCH 2/2] package/memtest86: rename package to memtest86plus Brandon Maier via buildroot
2023-11-18 18:34   ` Yann E. MORIN
2023-11-18 18:30 ` Yann E. MORIN [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231118183037.GJ996011@scaer \
    --to=yann.morin.1998@free.fr \
    --cc=abhishek.anand@collins.com \
    --cc=brandon.maier@collins.com \
    --cc=buildroot@buildroot.org \
    --cc=kalpesh.panchal2@collins.com \
    --cc=skenton@ou.edu \
    --cc=thomas.petazzoni@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.