diff mbox series

[1/3] boot/at91bootstrap3: add support for at91bootstrap 4.x series

Message ID 20210518213749.144355-2-thomas.petazzoni@bootlin.com
State Accepted
Headers show
Series My take on the at91bootstrap 4.x story | expand

Commit Message

Thomas Petazzoni May 18, 2021, 9:37 p.m. UTC
From: Eugen Hristev <eugen.hristev@microchip.com>

The project at https://github.com/linux4sam/at91bootstrap was until
now releasing 3.x versions, which were packaged using
boot/at91bootstrap3/ in Buildroot. Microchip has now started a new
branch of at91bootstrap, called 4.x, which will only support the
following devices: sam9x60, sama5d2, sama5d3, sama5d4, sama7g5. A
number of older devices from Microchip will only be supported by the
existing 3.x series.

Therefore, we cannot simply remove support for the 3.x series, and
allow using only the 4.x series.

So what this commit does is extend the boot/at91bootstrap3 package to
support building both 3.x and 4.x versions. In detail, this implies:

 * Having the BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION symbol point to
   the latest 4.x version. Indeed, we want
   BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION to really point to the
   latest upstream version, even if that means potential breakage for
   users. Users who want to use a fixed version of at91bootstrap
   should anyway not be using
   BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION.

 * Introduce BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION_3X for users who
   would like to use the latest 3.x series.

 * Adjust the installation logic, as images to install are now in
   build/binaries/*.bin instead of binaries/*.bin. In order to not
   have to differentiate 3.x and 4.x, we simply use $(wildcard ...) to
   expand the list of files to install.

 * To make it clear that boot/at91bootstrap3 supports both 3.x and
   4.x, we also update the prompt of the package.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
[Thomas: while this patch is based on previous work by Eugen, it was
reworked quite significantly.]
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 boot/at91bootstrap3/Config.in           | 10 +++++++---
 boot/at91bootstrap3/at91bootstrap3.hash |  1 +
 boot/at91bootstrap3/at91bootstrap3.mk   |  2 +-
 3 files changed, 9 insertions(+), 4 deletions(-)

Comments

Yann E. MORIN May 19, 2021, 9:33 a.m. UTC | #1
Thomas, All,

On 2021-05-18 23:37 +0200, Thomas Petazzoni spake thusly:
> From: Eugen Hristev <eugen.hristev@microchip.com>
> 
> The project at https://github.com/linux4sam/at91bootstrap was until
> now releasing 3.x versions, which were packaged using
> boot/at91bootstrap3/ in Buildroot. Microchip has now started a new
> branch of at91bootstrap, called 4.x, which will only support the
> following devices: sam9x60, sama5d2, sama5d3, sama5d4, sama7g5. A
> number of older devices from Microchip will only be supported by the
> existing 3.x series.
> 
> Therefore, we cannot simply remove support for the 3.x series, and
> allow using only the 4.x series.
> 
> So what this commit does is extend the boot/at91bootstrap3 package to
> support building both 3.x and 4.x versions. In detail, this implies:
> 
>  * Having the BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION symbol point to
>    the latest 4.x version. Indeed, we want
>    BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION to really point to the
>    latest upstream version, even if that means potential breakage for
>    users. Users who want to use a fixed version of at91bootstrap
>    should anyway not be using
>    BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION.
> 
>  * Introduce BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION_3X for users who
>    would like to use the latest 3.x series.

This introduces two issues.

First, and most important: this means that we now have two "known
versions". However, you forgot to account for the licensing stuff. We
use main.c as a substitute for a license file, but of course main.c
differs between 4.x and 3.x...

We could carry a per-version hash file, but using main.c as a license
file is really just hidding the issue. IOnstead, I've dropped it
altogether.

Eugen, Nicolas (and Simon?): would you care to add an actual license
file to your repository, please?

The second issue is less of an issue: there were two defconfigs that use
the default 3.x version. Without a tweak, they would have switched over
to using 4.x. So I pinned them with BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION_3X.

>  * Adjust the installation logic, as images to install are now in
>    build/binaries/*.bin instead of binaries/*.bin. In order to not
>    have to differentiate 3.x and 4.x, we simply use $(wildcard ...) to
>    expand the list of files to install.
> 
>  * To make it clear that boot/at91bootstrap3 supports both 3.x and
>    4.x, we also update the prompt of the package.
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> [Thomas: while this patch is based on previous work by Eugen, it was
> reworked quite significantly.]
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

With the aboce two issues adressed: applied to next, thanks.

Regards,
Yann E. MORIN.

> ---
>  boot/at91bootstrap3/Config.in           | 10 +++++++---
>  boot/at91bootstrap3/at91bootstrap3.hash |  1 +
>  boot/at91bootstrap3/at91bootstrap3.mk   |  2 +-
>  3 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/boot/at91bootstrap3/Config.in b/boot/at91bootstrap3/Config.in
> index 25ab30489f..ebc912e46c 100644
> --- a/boot/at91bootstrap3/Config.in
> +++ b/boot/at91bootstrap3/Config.in
> @@ -1,5 +1,5 @@
>  config BR2_TARGET_AT91BOOTSTRAP3
> -	bool "AT91 Bootstrap 3"
> +	bool "AT91 Bootstrap 3+"
>  	depends on BR2_arm926t || BR2_cortex_a5 || BR2_cortex_a7
>  	help
>  	  AT91Bootstrap is a first level bootloader for the Atmel AT91
> @@ -16,9 +16,12 @@ if BR2_TARGET_AT91BOOTSTRAP3
>  
>  choice
>  
> -	prompt "AT91 Bootstrap 3 version"
> +	prompt "AT91 Bootstrap 3+ version"
>  
>  config BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION
> +	bool "4.0.0-rc2"
> +
> +config BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION_3X
>  	bool "3.9.3"
>  
>  config BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_GIT
> @@ -51,7 +54,8 @@ endif
>  
>  config BR2_TARGET_AT91BOOTSTRAP3_VERSION
>  	string
> -	default "v3.9.3" if BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION
> +	default "v4.0.0-rc2" if BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION
> +	default "v3.9.3" if BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION_3X
>  	default BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_REPO_VERSION \
>  		if BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_GIT
>  	default "custom"	if BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_TARBALL
> diff --git a/boot/at91bootstrap3/at91bootstrap3.hash b/boot/at91bootstrap3/at91bootstrap3.hash
> index 6b6257b0ab..a63eb1d623 100644
> --- a/boot/at91bootstrap3/at91bootstrap3.hash
> +++ b/boot/at91bootstrap3/at91bootstrap3.hash
> @@ -1,3 +1,4 @@
>  # Locally calculated
>  sha256  dd6a3c57c1c84fc3b18187bee3d139146a0e032dd1d8edea7b242730e0bc4fe1  at91bootstrap3-v3.9.3.tar.gz
> +sha256  b5d5f042297cad0d091f7d8734e61eb9ec7b6020898e086503fb5f8bc71fb9fc  at91bootstrap3-v4.0.0-rc2.tar.gz
>  sha256  fd7a1ce5719bb7abf5e289da2e0ea8c933af3ba0f6ad03dbdbd2b7f54a77498a  main.c
> diff --git a/boot/at91bootstrap3/at91bootstrap3.mk b/boot/at91bootstrap3/at91bootstrap3.mk
> index a942afcdc9..fdd87591bb 100644
> --- a/boot/at91bootstrap3/at91bootstrap3.mk
> +++ b/boot/at91bootstrap3/at91bootstrap3.mk
> @@ -48,7 +48,7 @@ define AT91BOOTSTRAP3_BUILD_CMDS
>  endef
>  
>  define AT91BOOTSTRAP3_INSTALL_IMAGES_CMDS
> -	cp $(@D)/binaries/*.bin $(BINARIES_DIR)
> +	cp $(wildcard $(@D)/build/binaries/*.bin $(@D)/binaries/*.bin) $(BINARIES_DIR)
>  endef
>  
>  ifeq ($(BR2_TARGET_AT91BOOTSTRAP3_USE_DEFCONFIG),y)
> -- 
> 2.31.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Thomas Petazzoni May 19, 2021, 10:02 a.m. UTC | #2
Hello Yann,

On Wed, 19 May 2021 11:33:23 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> This introduces two issues.
> 
> First, and most important: this means that we now have two "known
> versions". However, you forgot to account for the licensing stuff. We
> use main.c as a substitute for a license file, but of course main.c
> differs between 4.x and 3.x...
> 
> We could carry a per-version hash file, but using main.c as a license
> file is really just hidding the issue. IOnstead, I've dropped it
> altogether.

Aaah, yes, indeed. Thanks for spotting/detecting that.

> Eugen, Nicolas (and Simon?): would you care to add an actual license
> file to your repository, please?

Agreed, Eugen and Nicolas: it would really be better to have a proper
LICENSE file in your repo. Please add it to both the 3.x branch and the
4.x branch :-)

> The second issue is less of an issue: there were two defconfigs that use
> the default 3.x version. Without a tweak, they would have switched over
> to using 4.x. So I pinned them with BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION_3X.

Aah, yes, well spotted. I think we should ask the contributor of these
defconfigs to use a fixed version of AT91Bootstrap, because that's
normally what we do for bootloader/kernel versions in defconfigs.

> With the aboce two issues adressed: applied to next, thanks.

Many thanks!

Thomas
Weber, Matthew L Collins via buildroot May 19, 2021, 11:36 a.m. UTC | #3
On 5/19/21 1:02 PM, Thomas Petazzoni wrote:
> Hello Yann,
> 
> On Wed, 19 May 2021 11:33:23 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> 
>> This introduces two issues.
>>
>> First, and most important: this means that we now have two "known
>> versions". However, you forgot to account for the licensing stuff. We
>> use main.c as a substitute for a license file, but of course main.c
>> differs between 4.x and 3.x...
>>
>> We could carry a per-version hash file, but using main.c as a license
>> file is really just hidding the issue. IOnstead, I've dropped it
>> altogether.
> 
> Aaah, yes, indeed. Thanks for spotting/detecting that.
> 
>> Eugen, Nicolas (and Simon?): would you care to add an actual license
>> file to your repository, please?
> 
> Agreed, Eugen and Nicolas: it would really be better to have a proper
> LICENSE file in your repo. Please add it to both the 3.x branch and the
> 4.x branch :-)

Hi,

Thank you for taking care of this. You could have given me more feedback 
on my previous patch set so I would have changed it, if you felt like 
it, but now it's done, we can move forward, which is good.
I will add a LICENSE file in the repo soon.

Thanks,
Eugen

> 
>> The second issue is less of an issue: there were two defconfigs that use
>> the default 3.x version. Without a tweak, they would have switched over
>> to using 4.x. So I pinned them with BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION_3X.
> 
> Aah, yes, well spotted. I think we should ask the contributor of these
> defconfigs to use a fixed version of AT91Bootstrap, because that's
> normally what we do for bootloader/kernel versions in defconfigs.
> 
>> With the aboce two issues adressed: applied to next, thanks.
> 
> Many thanks!
> 
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
Thomas Petazzoni May 19, 2021, 12:20 p.m. UTC | #4
Hello Eugen,

On Wed, 19 May 2021 11:36:56 +0000
<Eugen.Hristev@microchip.com> wrote:

> Thank you for taking care of this. You could have given me more feedback 
> on my previous patch set so I would have changed it, if you felt like 
> it, but now it's done, we can move forward, which is good.

Agreed, but sometimes the comments/ideas only come when really you
apply the patch, and you mess up with the code to figure out the most
appropriate way. Also, it was a way of avoiding another back-and-forth
with another iteration. I discussed the patch series yesterday with the
other BR maintainers, respin a series, and it got applied today.
Problem solved :-)

(To the exception of the mistake you spotted in PATCH 3/3, of course!).

> I will add a LICENSE file in the repo soon.

That would be awesome. Could you add it to both the 3.x and the 4.x
branches ? It would be nice to have another release on the 3.x branch
that includes the LICENSE file.

Thanks!

Thomas
Weber, Matthew L Collins via buildroot May 19, 2021, 12:29 p.m. UTC | #5
On 5/19/21 3:20 PM, Thomas Petazzoni wrote:
> Hello Eugen,
> 
> On Wed, 19 May 2021 11:36:56 +0000
> <Eugen.Hristev@microchip.com> wrote:
> 
>> Thank you for taking care of this. You could have given me more feedback
>> on my previous patch set so I would have changed it, if you felt like
>> it, but now it's done, we can move forward, which is good.
> 
> Agreed, but sometimes the comments/ideas only come when really you
> apply the patch, and you mess up with the code to figure out the most
> appropriate way. Also, it was a way of avoiding another back-and-forth
> with another iteration. I discussed the patch series yesterday with the
> other BR maintainers, respin a series, and it got applied today.
> Problem solved :-)
> 
> (To the exception of the mistake you spotted in PATCH 3/3, of course!).
> 
>> I will add a LICENSE file in the repo soon.
> 
> That would be awesome. Could you add it to both the 3.x and the 4.x
> branches ? It would be nice to have another release on the 3.x branch
> that includes the LICENSE file.

Yes, I would do it for both branches. We have to agree on the format of 
the LICENSE first.

There will be probably a 3.10.3 at some point. There are many patches 
yet on top of 3.10.2

I am sending now a v4 for the ICP board configs.

Eugen

> 
> Thanks!
> 
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
Edgar Bonet May 19, 2021, 7:44 p.m. UTC | #6
Hi!

Thomas Petazzoni wrote:
> I think we should ask the contributor of these defconfigs to use a
> fixed version of AT91Bootstrap, because that's normally what we do for
> bootloader/kernel versions in defconfigs.

Oh, right, I hadn't noticed. Sorry for the mess. I just submitted a
patch to pin these defconfifs to at91bootstrap 3.10.2, which I tested.

Best regards,

Edgar Bonet.
Weber, Matthew L Collins via buildroot June 14, 2021, 9:01 a.m. UTC | #7
On 5/19/21 3:29 PM, Eugen Hristev - M18282 wrote:
> On 5/19/21 3:20 PM, Thomas Petazzoni wrote:
>> Hello Eugen,
>>
>> On Wed, 19 May 2021 11:36:56 +0000
>> <Eugen.Hristev@microchip.com> wrote:
>>
>>> Thank you for taking care of this. You could have given me more feedback
>>> on my previous patch set so I would have changed it, if you felt like
>>> it, but now it's done, we can move forward, which is good.
>>
>> Agreed, but sometimes the comments/ideas only come when really you
>> apply the patch, and you mess up with the code to figure out the most
>> appropriate way. Also, it was a way of avoiding another back-and-forth
>> with another iteration. I discussed the patch series yesterday with the
>> other BR maintainers, respin a series, and it got applied today.
>> Problem solved :-)
>>
>> (To the exception of the mistake you spotted in PATCH 3/3, of course!).
>>
>>> I will add a LICENSE file in the repo soon.
>>
>> That would be awesome. Could you add it to both the 3.x and the 4.x
>> branches ? It would be nice to have another release on the 3.x branch
>> that includes the LICENSE file.
> 
> Yes, I would do it for both branches. We have to agree on the format of
> the LICENSE first.

Hello Thomas,

Do you know of some easy way (a script) to parse the source files and 
remove current header and add SPDX headers ?
We are considering selecting the SPDX way of licensing the files.

Thanks,
Eugen

> 
> There will be probably a 3.10.3 at some point. There are many patches
> yet on top of 3.10.2
> 
> I am sending now a v4 for the ICP board configs.
> 
> Eugen
> 
>>
>> Thanks!
>>
>> Thomas
>> --
>> Thomas Petazzoni, co-owner and CEO, Bootlin
>> Embedded Linux and Kernel engineering
>> https://bootlin.com
>>
>
Thomas Petazzoni June 14, 2021, 12:28 p.m. UTC | #8
On Mon, 14 Jun 2021 09:01:06 +0000
<Eugen.Hristev@microchip.com> wrote:

> Do you know of some easy way (a script) to parse the source files and 
> remove current header and add SPDX headers ?
> We are considering selecting the SPDX way of licensing the files.

I don't know of any tool doing that, but I also never had the need or
researched something like that, so perhaps there is some tooling
available, but I'm not sure.

Best regards,

Thomas
Arnout Vandecappelle June 14, 2021, 7:25 p.m. UTC | #9
On 14/06/2021 11:01, Eugen.Hristev--- via buildroot wrote:
> On 5/19/21 3:29 PM, Eugen Hristev - M18282 wrote:
>> On 5/19/21 3:20 PM, Thomas Petazzoni wrote:
>>> Hello Eugen,
>>>
>>> On Wed, 19 May 2021 11:36:56 +0000
>>> <Eugen.Hristev@microchip.com> wrote:
>>>
[snip]
>>>> I will add a LICENSE file in the repo soon.
>>>
>>> That would be awesome. Could you add it to both the 3.x and the 4.x
>>> branches ? It would be nice to have another release on the 3.x branch
>>> that includes the LICENSE file.
>>
>> Yes, I would do it for both branches. We have to agree on the format of
>> the LICENSE first.
> 
> Hello Thomas,
> 
> Do you know of some easy way (a script) to parse the source files and 
> remove current header and add SPDX headers ?
> We are considering selecting the SPDX way of licensing the files.

 I believe the REUSE tool [1] is able to add license headers to files. However,
I don't think it will remove the existing license blurb...

 You could check with the kernel developers how they did the mass replacement. E.g.:

commit d2912cb15bdda8ba4a5dd73396ad62641af2f520
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Tue Jun 4 10:11:33 2019

    treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 500

    Based on 2 normalized pattern(s):

      this program is free software you can redistribute it and or modify
      it under the terms of the gnu general public license version 2 as
      published by the free software foundation

      this program is free software you can redistribute it and or modify
      it under the terms of the gnu general public license version 2 as
      published by the free software foundation #

    extracted by the scancode license scanner the SPDX license identifier

      GPL-2.0-only

    has been chosen to replace the boilerplate/reference in 4122 file(s).

    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Reviewed-by: Enrico Weigelt <info@metux.net>
    Reviewed-by: Kate Stewart <kstewart@linuxfoundation.org>
    Reviewed-by: Allison Randal <allison@lohutok.net>
    Cc: linux-spdx@vger.kernel.org
    Link: https://lkml.kernel.org/r/20190604081206.933168790@linutronix.de
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>



 Regards,
 Arnout

[snip]
diff mbox series

Patch

diff --git a/boot/at91bootstrap3/Config.in b/boot/at91bootstrap3/Config.in
index 25ab30489f..ebc912e46c 100644
--- a/boot/at91bootstrap3/Config.in
+++ b/boot/at91bootstrap3/Config.in
@@ -1,5 +1,5 @@ 
 config BR2_TARGET_AT91BOOTSTRAP3
-	bool "AT91 Bootstrap 3"
+	bool "AT91 Bootstrap 3+"
 	depends on BR2_arm926t || BR2_cortex_a5 || BR2_cortex_a7
 	help
 	  AT91Bootstrap is a first level bootloader for the Atmel AT91
@@ -16,9 +16,12 @@  if BR2_TARGET_AT91BOOTSTRAP3
 
 choice
 
-	prompt "AT91 Bootstrap 3 version"
+	prompt "AT91 Bootstrap 3+ version"
 
 config BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION
+	bool "4.0.0-rc2"
+
+config BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION_3X
 	bool "3.9.3"
 
 config BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_GIT
@@ -51,7 +54,8 @@  endif
 
 config BR2_TARGET_AT91BOOTSTRAP3_VERSION
 	string
-	default "v3.9.3" if BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION
+	default "v4.0.0-rc2" if BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION
+	default "v3.9.3" if BR2_TARGET_AT91BOOTSTRAP3_LATEST_VERSION_3X
 	default BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_REPO_VERSION \
 		if BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_GIT
 	default "custom"	if BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_TARBALL
diff --git a/boot/at91bootstrap3/at91bootstrap3.hash b/boot/at91bootstrap3/at91bootstrap3.hash
index 6b6257b0ab..a63eb1d623 100644
--- a/boot/at91bootstrap3/at91bootstrap3.hash
+++ b/boot/at91bootstrap3/at91bootstrap3.hash
@@ -1,3 +1,4 @@ 
 # Locally calculated
 sha256  dd6a3c57c1c84fc3b18187bee3d139146a0e032dd1d8edea7b242730e0bc4fe1  at91bootstrap3-v3.9.3.tar.gz
+sha256  b5d5f042297cad0d091f7d8734e61eb9ec7b6020898e086503fb5f8bc71fb9fc  at91bootstrap3-v4.0.0-rc2.tar.gz
 sha256  fd7a1ce5719bb7abf5e289da2e0ea8c933af3ba0f6ad03dbdbd2b7f54a77498a  main.c
diff --git a/boot/at91bootstrap3/at91bootstrap3.mk b/boot/at91bootstrap3/at91bootstrap3.mk
index a942afcdc9..fdd87591bb 100644
--- a/boot/at91bootstrap3/at91bootstrap3.mk
+++ b/boot/at91bootstrap3/at91bootstrap3.mk
@@ -48,7 +48,7 @@  define AT91BOOTSTRAP3_BUILD_CMDS
 endef
 
 define AT91BOOTSTRAP3_INSTALL_IMAGES_CMDS
-	cp $(@D)/binaries/*.bin $(BINARIES_DIR)
+	cp $(wildcard $(@D)/build/binaries/*.bin $(@D)/binaries/*.bin) $(BINARIES_DIR)
 endef
 
 ifeq ($(BR2_TARGET_AT91BOOTSTRAP3_USE_DEFCONFIG),y)