[U-Boot] [PATCH 1/1] dm: sysreset: x86: missing build dependency

[U-Boot] [PATCH 1/1] dm: sysreset: x86: missing build dependency

[Heinrich Schuchardt]

make qemu-x86_defconfig
with additional

  #CONFIG_SYSRESET is not set

results in

  LD      u-boot
  lib/built-in.o: In function `efi_reset_system_boottime':
  lib/efi_loader/efi_runtime.c:122: undefined reference to `do_reset'

Fixes: fabb2b4c7f28 ("dm: sysreset: x86: Add a sysreset driver")
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 drivers/sysreset/Kconfig | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
index 9b2fda4d25..a5a58e6d83 100644
--- a/drivers/sysreset/Kconfig
+++ b/drivers/sysreset/Kconfig
@@ -36,6 +36,12 @@ config SYSRESET_PSCI
 	  Enable PSCI SYSTEM_RESET function call.  To use this, PSCI firmware
 	  must be running on your system.
 
+config SYSRESET_X86
+	bool "Enable support for x86 processor reboot driver"
+	depends on X86
+	help
+	  Reboot support for generic x86 processor reset.
+
 endif
 
 config SYSRESET_SYSCON
@@ -51,10 +57,4 @@ config SYSRESET_WATCHDOG
 	help
 	  Reboot support for generic watchdog reset.
 
-config SYSRESET_X86
-	bool "Enable support for x86 processor reboot driver"
-	depends on X86
-	help
-	  Reboot support for generic x86 processor reset.
-
 endmenu
-- 
2.18.0

[U-Boot] [PATCH 1/1] dm: sysreset: x86: missing build dependency

[Bin Meng]

Hi Heinrich,

On Sun, Jul 22, 2018 at 5:48 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> make qemu-x86_defconfig
> with additional
>
>   #CONFIG_SYSRESET is not set
>

I don't understand the changes. Why do you want to add
"#CONFIG_SYSRESET is not set"?

> results in
>
>   LD      u-boot
>   lib/built-in.o: In function `efi_reset_system_boottime':
>   lib/efi_loader/efi_runtime.c:122: undefined reference to `do_reset'
>
> Fixes: fabb2b4c7f28 ("dm: sysreset: x86: Add a sysreset driver")
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  drivers/sysreset/Kconfig | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
> index 9b2fda4d25..a5a58e6d83 100644
> --- a/drivers/sysreset/Kconfig
> +++ b/drivers/sysreset/Kconfig
> @@ -36,6 +36,12 @@ config SYSRESET_PSCI
>           Enable PSCI SYSTEM_RESET function call.  To use this, PSCI firmware
>           must be running on your system.
>
> +config SYSRESET_X86
> +       bool "Enable support for x86 processor reboot driver"
> +       depends on X86
> +       help
> +         Reboot support for generic x86 processor reset.
> +
>  endif
>
>  config SYSRESET_SYSCON
> @@ -51,10 +57,4 @@ config SYSRESET_WATCHDOG
>         help
>           Reboot support for generic watchdog reset.
>
> -config SYSRESET_X86
> -       bool "Enable support for x86 processor reboot driver"
> -       depends on X86
> -       help
> -         Reboot support for generic x86 processor reset.
> -
>  endmenu
> --

Regards,
Bin

[U-Boot] [PATCH 1/1] dm: sysreset: x86: missing build dependency

[Heinrich Schuchardt]

On 07/23/2018 03:51 AM, Bin Meng wrote:
> Hi Heinrich,
> 
> On Sun, Jul 22, 2018 at 5:48 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> make qemu-x86_defconfig
>> with additional
>>
>>   #CONFIG_SYSRESET is not set
>>
> 
> I don't understand the changes. Why do you want to add
> "#CONFIG_SYSRESET is not set"?

I was astonished that a configuration file I was using suddenly was
failing after your patch was merged. I analyzed the problem and found
the missing dependency.

In general it should not be possible to create a configuration that
cannot be built. One tool to verify this is the U-Boot randconfig target.

Best regards

Heinrich

> 
>> results in
>>
>>   LD      u-boot
>>   lib/built-in.o: In function `efi_reset_system_boottime':
>>   lib/efi_loader/efi_runtime.c:122: undefined reference to `do_reset'
>>
>> Fixes: fabb2b4c7f28 ("dm: sysreset: x86: Add a sysreset driver")
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>  drivers/sysreset/Kconfig | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
>> index 9b2fda4d25..a5a58e6d83 100644
>> --- a/drivers/sysreset/Kconfig
>> +++ b/drivers/sysreset/Kconfig
>> @@ -36,6 +36,12 @@ config SYSRESET_PSCI
>>           Enable PSCI SYSTEM_RESET function call.  To use this, PSCI firmware
>>           must be running on your system.
>>
>> +config SYSRESET_X86
>> +       bool "Enable support for x86 processor reboot driver"
>> +       depends on X86
>> +       help
>> +         Reboot support for generic x86 processor reset.
>> +
>>  endif
>>
>>  config SYSRESET_SYSCON
>> @@ -51,10 +57,4 @@ config SYSRESET_WATCHDOG
>>         help
>>           Reboot support for generic watchdog reset.
>>
>> -config SYSRESET_X86
>> -       bool "Enable support for x86 processor reboot driver"
>> -       depends on X86
>> -       help
>> -         Reboot support for generic x86 processor reset.
>> -
>>  endmenu
>> --
> 
> Regards,
> Bin
> 

[U-Boot] [PATCH 1/1] dm: sysreset: x86: missing build dependency

[Bin Meng]

Hi Heinrich,

On Sun, Jul 29, 2018 at 5:44 AM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 07/23/2018 03:51 AM, Bin Meng wrote:
>> Hi Heinrich,
>>
>> On Sun, Jul 22, 2018 at 5:48 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>> make qemu-x86_defconfig
>>> with additional
>>>
>>>   #CONFIG_SYSRESET is not set
>>>
>>
>> I don't understand the changes. Why do you want to add
>> "#CONFIG_SYSRESET is not set"?
>
> I was astonished that a configuration file I was using suddenly was
> failing after your patch was merged. I analyzed the problem and found
> the missing dependency.
>
> In general it should not be possible to create a configuration that
> cannot be built. One tool to verify this is the U-Boot randconfig target.
>

Yes, that's the goal, however I doubt currently we achieved such goal.

Using "Fixes: fabb2b4c7f28 ("dm: sysreset: x86: Add a sysreset
driver")" misleads people, as you are not using the default
configuration. If we want to support such random build for sysreset, I
think we should put the below driver under "if SYSRESET" too, not just
x86.

 41 config SYSRESET_SYSCON
 42         bool "Enable support for mfd syscon reboot driver"
 43         select REGMAP
 44         select SYSCON
 45         help
 46           Reboot support for generic SYSCON mapped register reset.
 47
 48 config SYSRESET_WATCHDOG
 49         bool "Enable support for watchdog reboot driver"
 50         select WDT
 51         help
 52           Reboot support for generic watchdog reset.
 53

Regards,
Bin