| Message ID | 20210518213749.144355-2-thomas.petazzoni@bootlin.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | My take on the at91bootstrap 4.x story | expand |
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
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
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 >
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
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 >
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.
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 >> >
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
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]
On 6/14/21 10:25 PM, Arnout Vandecappelle wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > 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... Hi, Thanks for the tip ! I managed to use REUSE tool with a large extent. Eugen > > 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 --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)