| Message ID | 20161109065948.1851-2-ludovic.desroches@atmel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Ludovic, On Wed, Nov 09, 2016 at 07:59:45AM +0100, Ludovic Desroches wrote: > Bump to a recent version of AT91bootstrap and use mainline version of > U-Boot and Linux. > > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com> > --- > configs/at91sam9260eknf_defconfig | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > diff --git a/configs/at91sam9260eknf_defconfig b/configs/at91sam9260eknf_defconfig > index 92bb071..4c17cb6 100644 > --- a/configs/at91sam9260eknf_defconfig > +++ b/configs/at91sam9260eknf_defconfig > @@ -2,9 +2,6 @@ > BR2_arm=y > BR2_arm926t=y > > -# Linux headers same as kernel, a 3.9 series > -BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_3_9=y > - > # Packages > BR2_PACKAGE_HOST_SAM_BA=y > > @@ -13,15 +10,20 @@ BR2_TARGET_ROOTFS_UBIFS=y > BR2_TARGET_ROOTFS_UBI=y > > # Bootloaders > -BR2_TARGET_AT91BOOTSTRAP=y > -BR2_TARGET_AT91BOOTSTRAP_BOARD="at91sam9260ek" > -BR2_TARGET_AT91BOOTSTRAP_NANDFLASH=y > +BR2_TARGET_AT91BOOTSTRAP3=y > +BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_GIT=y > +BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_REPO_URL="https://github.com/linux4sam/at91bootstrap.git" > +BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_REPO_VERSION="v3.8.7" > +BR2_TARGET_AT91BOOTSTRAP3_DEFCONFIG="at91sam9260eknf_uboot" > BR2_TARGET_BAREBOX=y > BR2_TARGET_BAREBOX_BOARD_DEFCONFIG="at91sam9260ek" > +BR2_TARGET_UBOOT=y > +BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG=y > +BR2_TARGET_UBOOT_BOARD_DEFCONFIG="at91sam9260ek_nandflash" Why enable both U-Boot and Barebox? > # Kernel > BR2_LINUX_KERNEL=y > -BR2_LINUX_KERNEL_CUSTOM_VERSION=y > -BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="3.9.1" For the sake of reproducibility you should set this to the version you tested, say, 4.8.6. The same goes for other patches in this series. > -BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG=y > -BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE="board/atmel/at91sam9260ek/linux-3.9.config" You can now drop this file. > +BR2_LINUX_KERNEL_DEFCONFIG="at91_dt" > +BR2_LINUX_KERNEL_DTS_SUPPORT=y > +BR2_LINUX_KERNEL_INTREE_DTS_NAME="at91sam9260ek" baruch
On 09/11/2016 at 09:36:38 +0200, Baruch Siach wrote : > Hi Ludovic, > > On Wed, Nov 09, 2016 at 07:59:45AM +0100, Ludovic Desroches wrote: > > # Kernel > > BR2_LINUX_KERNEL=y > > -BR2_LINUX_KERNEL_CUSTOM_VERSION=y > > -BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="3.9.1" > > For the sake of reproducibility you should set this to the version you tested, > say, 4.8.6. The same goes for other patches in this series. > I'm not sure this is actually useful because the amount of testing is minimal and it just increases the maintenance burden.
Hi Alexandre, On Wed, Nov 09, 2016 at 10:52:49AM +0100, Alexandre Belloni wrote: > On 09/11/2016 at 09:36:38 +0200, Baruch Siach wrote : > > On Wed, Nov 09, 2016 at 07:59:45AM +0100, Ludovic Desroches wrote: > > > # Kernel > > > BR2_LINUX_KERNEL=y > > > -BR2_LINUX_KERNEL_CUSTOM_VERSION=y > > > -BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="3.9.1" > > > > For the sake of reproducibility you should set this to the version you tested, > > say, 4.8.6. The same goes for other patches in this series. > > I'm not sure this is actually useful because the amount of testing is > minimal and it just increases the maintenance burden. I don't understand. How would locking a specific kernel version in the defconfig increase maintenance burden? Any tested kernel version would do. We just want to ensure consistent user experience. Otherwise the built kernel will change with the default value of BR2_LINUX_KERNEL_VERSION. baruch
On Wed, Nov 09, 2016 at 09:36:38AM +0200, Baruch Siach wrote: > Hi Ludovic, > > On Wed, Nov 09, 2016 at 07:59:45AM +0100, Ludovic Desroches wrote: > > Bump to a recent version of AT91bootstrap and use mainline version of > > U-Boot and Linux. > > > > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com> > > --- > > configs/at91sam9260eknf_defconfig | 22 ++++++++++++---------- > > 1 file changed, 12 insertions(+), 10 deletions(-) > > > > diff --git a/configs/at91sam9260eknf_defconfig b/configs/at91sam9260eknf_defconfig > > index 92bb071..4c17cb6 100644 > > --- a/configs/at91sam9260eknf_defconfig > > +++ b/configs/at91sam9260eknf_defconfig > > @@ -2,9 +2,6 @@ > > BR2_arm=y > > BR2_arm926t=y > > > > -# Linux headers same as kernel, a 3.9 series > > -BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_3_9=y > > - > > # Packages > > BR2_PACKAGE_HOST_SAM_BA=y > > > > @@ -13,15 +10,20 @@ BR2_TARGET_ROOTFS_UBIFS=y > > BR2_TARGET_ROOTFS_UBI=y > > > > # Bootloaders > > -BR2_TARGET_AT91BOOTSTRAP=y > > -BR2_TARGET_AT91BOOTSTRAP_BOARD="at91sam9260ek" > > -BR2_TARGET_AT91BOOTSTRAP_NANDFLASH=y > > +BR2_TARGET_AT91BOOTSTRAP3=y > > +BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_GIT=y > > +BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_REPO_URL="https://github.com/linux4sam/at91bootstrap.git" > > +BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_REPO_VERSION="v3.8.7" > > +BR2_TARGET_AT91BOOTSTRAP3_DEFCONFIG="at91sam9260eknf_uboot" > > BR2_TARGET_BAREBOX=y > > BR2_TARGET_BAREBOX_BOARD_DEFCONFIG="at91sam9260ek" > > +BR2_TARGET_UBOOT=y > > +BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG=y > > +BR2_TARGET_UBOOT_BOARD_DEFCONFIG="at91sam9260ek_nandflash" > > Why enable both U-Boot and Barebox? > I don't know why Barebox was selected but the 'official' bootloader is U-Boot for all our products that's why I add it. I kept Barebox because it was already selected and I don't know if someone is still using it or not. > > # Kernel > > BR2_LINUX_KERNEL=y > > -BR2_LINUX_KERNEL_CUSTOM_VERSION=y > > -BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="3.9.1" > > For the sake of reproducibility you should set this to the version you tested, > say, 4.8.6. The same goes for other patches in this series. > As Alexandre said, we don't want to spend time for the maintainance of these old boards so sticking to the mainline seems to be the way to go. Giving a version is a kind of commitment but we no longer perform tests on these boards excepting kernel boot with kernelci. > > -BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG=y > > -BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE="board/atmel/at91sam9260ek/linux-3.9.config" > > You can now drop this file. Ok. > > > +BR2_LINUX_KERNEL_DEFCONFIG="at91_dt" > > +BR2_LINUX_KERNEL_DTS_SUPPORT=y > > +BR2_LINUX_KERNEL_INTREE_DTS_NAME="at91sam9260ek" > > baruch > > -- > http://baruch.siach.name/blog/ ~. .~ Tk Open Systems > =}------------------------------------------------ooO--U--Ooo------------{= > - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
On 09/11/2016 at 12:00:27 +0200, Baruch Siach wrote : > Hi Alexandre, > > On Wed, Nov 09, 2016 at 10:52:49AM +0100, Alexandre Belloni wrote: > > On 09/11/2016 at 09:36:38 +0200, Baruch Siach wrote : > > > On Wed, Nov 09, 2016 at 07:59:45AM +0100, Ludovic Desroches wrote: > > > > # Kernel > > > > BR2_LINUX_KERNEL=y > > > > -BR2_LINUX_KERNEL_CUSTOM_VERSION=y > > > > -BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="3.9.1" > > > > > > For the sake of reproducibility you should set this to the version you tested, > > > say, 4.8.6. The same goes for other patches in this series. > > > > I'm not sure this is actually useful because the amount of testing is > > minimal and it just increases the maintenance burden. > > I don't understand. How would locking a specific kernel version in the > defconfig increase maintenance burden? Any tested kernel version would do. We > just want to ensure consistent user experience. Otherwise the built kernel > will change with the default value of BR2_LINUX_KERNEL_VERSION. > Yeah, that's my point. Any kernel version will do and that avoids having to update it in the defconfig all the time, hence less maintenance. The previous defconfig has not been building for a while (basically, since the switch to gcc5).
On Wed, Nov 09, 2016 at 12:00:27PM +0200, Baruch Siach wrote: > Hi Alexandre, > > On Wed, Nov 09, 2016 at 10:52:49AM +0100, Alexandre Belloni wrote: > > On 09/11/2016 at 09:36:38 +0200, Baruch Siach wrote : > > > On Wed, Nov 09, 2016 at 07:59:45AM +0100, Ludovic Desroches wrote: > > > > # Kernel > > > > BR2_LINUX_KERNEL=y > > > > -BR2_LINUX_KERNEL_CUSTOM_VERSION=y > > > > -BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="3.9.1" > > > > > > For the sake of reproducibility you should set this to the version you tested, > > > say, 4.8.6. The same goes for other patches in this series. > > > > I'm not sure this is actually useful because the amount of testing is > > minimal and it just increases the maintenance burden. > > I don't understand. How would locking a specific kernel version in the > defconfig increase maintenance burden? Any tested kernel version would do. We > just want to ensure consistent user experience. Otherwise the built kernel > will change with the default value of BR2_LINUX_KERNEL_VERSION. > That's the goal. Kernel was no longer compiling because it was too old to be used with gcc5. We may have the same situation in the future and we'll simply choose the latest kernel release for the update as I do today. That's why Alexandre says that it will increase the maintenance. Ludovic
Hello, On Wed, 9 Nov 2016 11:10:09 +0100, Ludovic Desroches wrote: > > Why enable both U-Boot and Barebox? > > > > I don't know why Barebox was selected but the 'official' bootloader is > U-Boot for all our products that's why I add it. I kept Barebox because > it was already selected and I don't know if someone is still using it or > not. But the end result is a defconfig that doesn't make any sense. Please provide a defconfig that makes sense, and with an updated readme.txt if you decide to change the bootloader (and therefore the flashing instructions). > > > # Kernel > > > BR2_LINUX_KERNEL=y > > > -BR2_LINUX_KERNEL_CUSTOM_VERSION=y > > > -BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="3.9.1" > > > > For the sake of reproducibility you should set this to the version you tested, > > say, 4.8.6. The same goes for other patches in this series. > > As Alexandre said, we don't want to spend time for the maintainance of these > old boards so sticking to the mainline seems to be the way to go. Giving a > version is a kind of commitment but we no longer perform tests on these > boards excepting kernel boot with kernelci. That's not Buildroot policy. We want defconfigs with fixed kernel and bootloader versions, so that we know they have been tested. If you are not able/willing to test those defconfigs, then we could just as well remove them. But I'm not going to merge a defconfig that doesn't comply with our policy of having a fixed kernel and a fixed bootloader version. Best regards, Thomas
On Wed, Nov 09, 2016 at 11:25:50AM +0100, Thomas Petazzoni wrote: > Hello, > > On Wed, 9 Nov 2016 11:10:09 +0100, Ludovic Desroches wrote: > > > > Why enable both U-Boot and Barebox? > > > > > > > I don't know why Barebox was selected but the 'official' bootloader is > > U-Boot for all our products that's why I add it. I kept Barebox because > > it was already selected and I don't know if someone is still using it or > > not. > > But the end result is a defconfig that doesn't make any sense. > > Please provide a defconfig that makes sense, and with an updated > readme.txt if you decide to change the bootloader (and therefore the > flashing instructions). > > > > > # Kernel > > > > BR2_LINUX_KERNEL=y > > > > -BR2_LINUX_KERNEL_CUSTOM_VERSION=y > > > > -BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="3.9.1" > > > > > > For the sake of reproducibility you should set this to the version you tested, > > > say, 4.8.6. The same goes for other patches in this series. > > > > As Alexandre said, we don't want to spend time for the maintainance of these > > old boards so sticking to the mainline seems to be the way to go. Giving a > > version is a kind of commitment but we no longer perform tests on these > > boards excepting kernel boot with kernelci. > > That's not Buildroot policy. We want defconfigs with fixed kernel and > bootloader versions, so that we know they have been tested. > > If you are not able/willing to test those defconfigs, then we could > just as well remove them. But I'm not going to merge a defconfig that > doesn't comply with our policy of having a fixed kernel and a fixed > bootloader version. > Can I use a fixed version of the compiler too? It seems it makes sense because this defconfig was tested but is no more compiling. Regards Ludovic > Best regards, > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com
Hello, On Wed, 9 Nov 2016 11:39:55 +0100, Ludovic Desroches wrote: > > If you are not able/willing to test those defconfigs, then we could > > just as well remove them. But I'm not going to merge a defconfig that > > doesn't comply with our policy of having a fixed kernel and a fixed > > bootloader version. > > Can I use a fixed version of the compiler too? It seems it makes sense > because this defconfig was tested but is no more compiling. This could be an option indeed (reverting to gcc 4.x instead of gcc 5.x). However, the fact that no-one is willing to test/maintain this defconfig is a sign that nobody is interested in it. So I'd rather remove it, than keep a defconfig that gets never updated, even when there is a build issue. So, options are clear: 1. Remove the defconfig entirely. 2. Update the defconfig in the proper way, minimally tested on HW. Best regards, Thomas
diff --git a/configs/at91sam9260eknf_defconfig b/configs/at91sam9260eknf_defconfig index 92bb071..4c17cb6 100644 --- a/configs/at91sam9260eknf_defconfig +++ b/configs/at91sam9260eknf_defconfig @@ -2,9 +2,6 @@ BR2_arm=y BR2_arm926t=y -# Linux headers same as kernel, a 3.9 series -BR2_PACKAGE_HOST_LINUX_HEADERS_CUSTOM_3_9=y - # Packages BR2_PACKAGE_HOST_SAM_BA=y @@ -13,15 +10,20 @@ BR2_TARGET_ROOTFS_UBIFS=y BR2_TARGET_ROOTFS_UBI=y # Bootloaders -BR2_TARGET_AT91BOOTSTRAP=y -BR2_TARGET_AT91BOOTSTRAP_BOARD="at91sam9260ek" -BR2_TARGET_AT91BOOTSTRAP_NANDFLASH=y +BR2_TARGET_AT91BOOTSTRAP3=y +BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_GIT=y +BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_REPO_URL="https://github.com/linux4sam/at91bootstrap.git" +BR2_TARGET_AT91BOOTSTRAP3_CUSTOM_REPO_VERSION="v3.8.7" +BR2_TARGET_AT91BOOTSTRAP3_DEFCONFIG="at91sam9260eknf_uboot" BR2_TARGET_BAREBOX=y BR2_TARGET_BAREBOX_BOARD_DEFCONFIG="at91sam9260ek" +BR2_TARGET_UBOOT=y +BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG=y +BR2_TARGET_UBOOT_BOARD_DEFCONFIG="at91sam9260ek_nandflash" # Kernel BR2_LINUX_KERNEL=y -BR2_LINUX_KERNEL_CUSTOM_VERSION=y -BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="3.9.1" -BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG=y -BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE="board/atmel/at91sam9260ek/linux-3.9.config" +BR2_LINUX_KERNEL_DEFCONFIG="at91_dt" +BR2_LINUX_KERNEL_DTS_SUPPORT=y +BR2_LINUX_KERNEL_INTREE_DTS_NAME="at91sam9260ek" +
Bump to a recent version of AT91bootstrap and use mainline version of U-Boot and Linux. Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com> --- configs/at91sam9260eknf_defconfig | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)