diff mbox series

[U-Boot] omap3: beagle: re-enable EFI support after the following commit removed it by mistake:

Message ID 20180718193844.29632-1-guillaume.gardet@free.fr
State Accepted
Commit 69ca343906766601672eb1dfafa6654bc88e3252
Delegated to: Tom Rini
Headers show
Series [U-Boot] omap3: beagle: re-enable EFI support after the following commit removed it by mistake: | expand

Commit Message

Guillaume GARDET July 18, 2018, 7:38 p.m. UTC
commit d233ccb1d84b901f7e23e6d9b4f2c6a57198b23b
  ARM: omap3: beagle: Enable DM_SERIAL, update distro usage and NAND layout
    
Signed-off-by: Guillaume GARDET <guillaume.gardet@free.fr>
Cc: Derald D. Woods <woods.technical@gmail.com>
Cc: Tom Rini <trini@konsulko.com> 

---
 configs/omap3_beagle_defconfig | 3 ---
 1 file changed, 3 deletions(-)

Comments

Derald Woods July 18, 2018, 11:47 p.m. UTC | #1
On Wed, Jul 18, 2018 at 09:38:44PM +0200, Guillaume GARDET wrote:
>   commit d233ccb1d84b901f7e23e6d9b4f2c6a57198b23b
>   ARM: omap3: beagle: Enable DM_SERIAL, update distro usage and NAND layout


I have found no compile or runtime issues, on my BeagleBoard Rev. C4
or xM, with the current default configuration. At least, for my non-EFI
boot environment.

The following items are brought in from the EFI config additions:

+CONFIG_SMBIOS_PRODUCT_NAME="beagle"
+CONFIG_CMD_BOOTEFI=y
+CONFIG_CMD_BOOTEFI_HELLO_COMPILE=y
+CONFIG_EFI_PARTITION=y
+CONFIG_EFI_PARTITION_ENTRIES_NUMBERS=128
+CONFIG_EFI_PARTITION_ENTRIES_OFF=0
+CONFIG_SPL_EFI_PARTITION=y
+CONFIG_OF_LIBFDT_OVERLAY=y
+CONFIG_GENERATE_SMBIOS_TABLE=y
+CONFIG_SMBIOS_MANUFACTURER="ti"
+CONFIG_EFI_LOADER=y

I would change the commit message though. It was not a mistake. It was
removed to provide a minimal set of configuration options for the base
BSP type of setup. Using EFI is a choice. In fact, I tested my changes
on 3 different OMAP3 boards. So there has been no period of not working
in the basic board configuration. Provably.

So if EFI is required, even though one may not use it, this is a don't
care for me. My boards boot just fine without EFI.

Derald

> Signed-off-by: Guillaume GARDET <guillaume.gardet@free.fr>
> Cc: Derald D. Woods <woods.technical@gmail.com>
> Cc: Tom Rini <trini@konsulko.com> 
> 
> ---
>  configs/omap3_beagle_defconfig | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/configs/omap3_beagle_defconfig b/configs/omap3_beagle_defconfig
> index a37a38f881..1e1a391d7f 100644
> --- a/configs/omap3_beagle_defconfig
> +++ b/configs/omap3_beagle_defconfig
> @@ -32,8 +32,6 @@ CONFIG_MTDIDS_DEFAULT="nand0=omap2-nand.0"
>  CONFIG_MTDPARTS_DEFAULT="mtdparts=omap2-nand.0:512k(spl),1920k(u-boot),128k(u-boot-env),128k(dtb),6m(kernel),-(rootfs)"
>  CONFIG_CMD_UBI=y
>  # CONFIG_ISO_PARTITION is not set
> -# CONFIG_EFI_PARTITION is not set
> -CONFIG_SPL_PARTITION_UUIDS=y
>  CONFIG_OF_CONTROL=y
>  CONFIG_ENV_IS_IN_NAND=y
>  CONFIG_SPL_DM=y
> @@ -79,4 +77,3 @@ CONFIG_USB_ETHER_SMSC95XX=y
>  CONFIG_FAT_WRITE=y
>  CONFIG_BCH=y
>  CONFIG_SPL_OF_LIBFDT=y
> -# CONFIG_EFI_LOADER is not set
> -- 
> 2.18.0
>
Guillaume GARDET July 19, 2018, 7:34 a.m. UTC | #2
Hi Derald,


Le 19/07/2018 à 01:47, Derald D. Woods a écrit :
> On Wed, Jul 18, 2018 at 09:38:44PM +0200, Guillaume GARDET wrote:
>>    commit d233ccb1d84b901f7e23e6d9b4f2c6a57198b23b
>>    ARM: omap3: beagle: Enable DM_SERIAL, update distro usage and NAND layout
>
> I have found no compile or runtime issues, on my BeagleBoard Rev. C4
> or xM, with the current default configuration. At least, for my non-EFI
> boot environment.

It is related to EFI options since I use GPT partitions (and EFI/Grub2 to boot). That is why you did not find anything.

>
> The following items are brought in from the EFI config additions:
>
> +CONFIG_SMBIOS_PRODUCT_NAME="beagle"
> +CONFIG_CMD_BOOTEFI=y
> +CONFIG_CMD_BOOTEFI_HELLO_COMPILE=y
> +CONFIG_EFI_PARTITION=y
> +CONFIG_EFI_PARTITION_ENTRIES_NUMBERS=128
> +CONFIG_EFI_PARTITION_ENTRIES_OFF=0
> +CONFIG_SPL_EFI_PARTITION=y
> +CONFIG_OF_LIBFDT_OVERLAY=y
> +CONFIG_GENERATE_SMBIOS_TABLE=y
> +CONFIG_SMBIOS_MANUFACTURER="ti"
> +CONFIG_EFI_LOADER=y
>
> I would change the commit message though. It was not a mistake. It was
> removed to provide a minimal set of configuration options for the base
> BSP type of setup. Using EFI is a choice. In fact, I tested my changes
> on 3 different OMAP3 boards. So there has been no period of not working
> in the basic board configuration. Provably.
>
> So if EFI is required, even though one may not use it, this is a don't
> care for me. My boards boot just fine without EFI.

The thing is you introduced a regression with your commit, since EFI was supported previously. And you did not explicitly stated that you disabled it on purpose. So, it sounds like a mistake to me.
If you don't care about EFI, it is your choice, but please do not break setup of others. A use case is openSUSE which relies on EFI/Grub2 to boot.
Please note that EFI is now a default feature in U-Boot, so, please, let it enabled.

Guillaume


>
> Derald
>
>> Signed-off-by: Guillaume GARDET <guillaume.gardet@free.fr>
>> Cc: Derald D. Woods <woods.technical@gmail.com>
>> Cc: Tom Rini <trini@konsulko.com>
>>
>> ---
>>   configs/omap3_beagle_defconfig | 3 ---
>>   1 file changed, 3 deletions(-)
>>
>> diff --git a/configs/omap3_beagle_defconfig b/configs/omap3_beagle_defconfig
>> index a37a38f881..1e1a391d7f 100644
>> --- a/configs/omap3_beagle_defconfig
>> +++ b/configs/omap3_beagle_defconfig
>> @@ -32,8 +32,6 @@ CONFIG_MTDIDS_DEFAULT="nand0=omap2-nand.0"
>>   CONFIG_MTDPARTS_DEFAULT="mtdparts=omap2-nand.0:512k(spl),1920k(u-boot),128k(u-boot-env),128k(dtb),6m(kernel),-(rootfs)"
>>   CONFIG_CMD_UBI=y
>>   # CONFIG_ISO_PARTITION is not set
>> -# CONFIG_EFI_PARTITION is not set
>> -CONFIG_SPL_PARTITION_UUIDS=y
>>   CONFIG_OF_CONTROL=y
>>   CONFIG_ENV_IS_IN_NAND=y
>>   CONFIG_SPL_DM=y
>> @@ -79,4 +77,3 @@ CONFIG_USB_ETHER_SMSC95XX=y
>>   CONFIG_FAT_WRITE=y
>>   CONFIG_BCH=y
>>   CONFIG_SPL_OF_LIBFDT=y
>> -# CONFIG_EFI_LOADER is not set
>> -- 
>> 2.18.0
>>
Derald Woods July 19, 2018, 12:12 p.m. UTC | #3
On Thu, Jul 19, 2018, 2:34 AM Guillaume Gardet <guillaume.gardet@free.fr>
wrote:

> Hi Derald,
>
>
> Le 19/07/2018 à 01:47, Derald D. Woods a écrit :
> > On Wed, Jul 18, 2018 at 09:38:44PM +0200, Guillaume GARDET wrote:
> >>    commit d233ccb1d84b901f7e23e6d9b4f2c6a57198b23b
> >>    ARM: omap3: beagle: Enable DM_SERIAL, update distro usage and NAND
> layout
> >
> > I have found no compile or runtime issues, on my BeagleBoard Rev. C4
> > or xM, with the current default configuration. At least, for my non-EFI
> > boot environment.
>
> It is related to EFI options since I use GPT partitions (and EFI/Grub2 to
> boot). That is why you did not find anything.
>


That is the piece that I do not use.



> >
> > The following items are brought in from the EFI config additions:
> >
> > +CONFIG_SMBIOS_PRODUCT_NAME="beagle"
> > +CONFIG_CMD_BOOTEFI=y
> > +CONFIG_CMD_BOOTEFI_HELLO_COMPILE=y
> > +CONFIG_EFI_PARTITION=y
> > +CONFIG_EFI_PARTITION_ENTRIES_NUMBERS=128
> > +CONFIG_EFI_PARTITION_ENTRIES_OFF=0
> > +CONFIG_SPL_EFI_PARTITION=y
> > +CONFIG_OF_LIBFDT_OVERLAY=y
> > +CONFIG_GENERATE_SMBIOS_TABLE=y
> > +CONFIG_SMBIOS_MANUFACTURER="ti"
> > +CONFIG_EFI_LOADER=y
> >
> > I would change the commit message though. It was not a mistake. It was
> > removed to provide a minimal set of configuration options for the base
> > BSP type of setup. Using EFI is a choice. In fact, I tested my changes
> > on 3 different OMAP3 boards. So there has been no period of not working
> > in the basic board configuration. Provably.
> >
> > So if EFI is required, even though one may not use it, this is a don't
> > care for me. My boards boot just fine without EFI.
>
> The thing is you introduced a regression with your commit, since EFI was
> supported previously. And you did not explicitly stated that you disabled
> it on purpose. So, it sounds like a mistake to me.
> If you don't care about EFI, it is your choice, but please do not break
> setup of others. A use case is openSUSE which relies on EFI/Grub2 to boot.
> Please note that EFI is now a default feature in U-Boot, so, please, let
> it enabled.
>
> Guillaume
>

I am not a novice and my series received the usual mailing list oversight.
Maybe even more than one iteration. As I stated, this is a don't care for
me. EFI is not mandatory to function/boot. The bare minimum case was the
target of the series.

I acknowledge your commit.

Derald


>
>
> >
> > Derald
> >
> >> Signed-off-by: Guillaume GARDET <guillaume.gardet@free.fr>
> >> Cc: Derald D. Woods <woods.technical@gmail.com>
> >> Cc: Tom Rini <trini@konsulko.com>
> >>
> >> ---
> >>   configs/omap3_beagle_defconfig | 3 ---
> >>   1 file changed, 3 deletions(-)
> >>
> >> diff --git a/configs/omap3_beagle_defconfig
> b/configs/omap3_beagle_defconfig
> >> index a37a38f881..1e1a391d7f 100644
> >> --- a/configs/omap3_beagle_defconfig
> >> +++ b/configs/omap3_beagle_defconfig
> >> @@ -32,8 +32,6 @@ CONFIG_MTDIDS_DEFAULT="nand0=omap2-nand.0"
> >>
>  CONFIG_MTDPARTS_DEFAULT="mtdparts=omap2-nand.0:512k(spl),1920k(u-boot),128k(u-boot-env),128k(dtb),6m(kernel),-(rootfs)"
> >>   CONFIG_CMD_UBI=y
> >>   # CONFIG_ISO_PARTITION is not set
> >> -# CONFIG_EFI_PARTITION is not set
> >> -CONFIG_SPL_PARTITION_UUIDS=y
> >>   CONFIG_OF_CONTROL=y
> >>   CONFIG_ENV_IS_IN_NAND=y
> >>   CONFIG_SPL_DM=y
> >> @@ -79,4 +77,3 @@ CONFIG_USB_ETHER_SMSC95XX=y
> >>   CONFIG_FAT_WRITE=y
> >>   CONFIG_BCH=y
> >>   CONFIG_SPL_OF_LIBFDT=y
> >> -# CONFIG_EFI_LOADER is not set
> >> --
> >> 2.18.0
> >>
>
>
Tom Rini July 26, 2018, 7:54 p.m. UTC | #4
On Wed, Jul 18, 2018 at 09:38:44PM +0200, Guillaume GARDET wrote:

> commit d233ccb1d84b901f7e23e6d9b4f2c6a57198b23b
>   ARM: omap3: beagle: Enable DM_SERIAL, update distro usage and NAND layout
>     
> Signed-off-by: Guillaume GARDET <guillaume.gardet@free.fr>
> Cc: Derald D. Woods <woods.technical@gmail.com>
> Cc: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/configs/omap3_beagle_defconfig b/configs/omap3_beagle_defconfig
index a37a38f881..1e1a391d7f 100644
--- a/configs/omap3_beagle_defconfig
+++ b/configs/omap3_beagle_defconfig
@@ -32,8 +32,6 @@  CONFIG_MTDIDS_DEFAULT="nand0=omap2-nand.0"
 CONFIG_MTDPARTS_DEFAULT="mtdparts=omap2-nand.0:512k(spl),1920k(u-boot),128k(u-boot-env),128k(dtb),6m(kernel),-(rootfs)"
 CONFIG_CMD_UBI=y
 # CONFIG_ISO_PARTITION is not set
-# CONFIG_EFI_PARTITION is not set
-CONFIG_SPL_PARTITION_UUIDS=y
 CONFIG_OF_CONTROL=y
 CONFIG_ENV_IS_IN_NAND=y
 CONFIG_SPL_DM=y
@@ -79,4 +77,3 @@  CONFIG_USB_ETHER_SMSC95XX=y
 CONFIG_FAT_WRITE=y
 CONFIG_BCH=y
 CONFIG_SPL_OF_LIBFDT=y
-# CONFIG_EFI_LOADER is not set