Skip to contentSkip to conversation

WTF: patch "[PATCH] ARC: enable uboot support unconditionally" was seriously submitted to be applied to the 4.20-stable tree?

PATCH [email protected] 3 participants 6 messages Feb 24, 2019 – Feb 26, 2019

Subsystems (2)
Stable branch Synopsys ARC architecture
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below was submitted to be applied to the 4.20-stable tree.

I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst.

I could be totally wrong, and if so, please respond to <[email protected]> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 493a2f812446e92bcb1e69a77381b4d39808d730 Mon Sep 17 00:00:00 2001
From: Eugeniy Paltsev <[email protected]>
Date: Thu, 14 Feb 2019 18:07:45 +0300
Subject: [PATCH] ARC: enable uboot support unconditionally

After reworking U-boot args handling code and adding paranoid arguments check we can eliminate CONFIG_ARC_UBOOT_SUPPORT and enable uboot support unconditionally.

For JTAG case we can assume that core registers will come up reset value of 0 or in worst case we rely on user passing '-on=clear_regs' to Metaware debugger.

Cc [email protected]
Tested-by Corentin LABBE <[email protected]>
Signed-off-by Eugeniy Paltsev <[email protected]>
Signed-off-by Vineet Gupta <[email protected]>
arch/arc/Kconfig +0 -12
index 7215f52b3413..d750b302d5ab 100644
@@ -191,7 +191,6 @@ config NR_CPUS
 
 config ARC_SMP_HALT_ON_RESET
 	bool "Enable Halt-on-reset boot mode"
-	default y if ARC_UBOOT_SUPPORT
 	help
 	  In SMP configuration cores can be configured as Halt-on-reset
 	  or they could all start at same time. For Halt-on-reset, non

@@ -523,17 +522,6 @@ config ARC_DBG_TLB_PARANOIA
 
 endif
 
-config ARC_UBOOT_SUPPORT
-	bool "Support uboot arg Handling"
-	help
-	  ARC Linux by default checks for uboot provided args as pointers to
-	  external cmdline or DTB. This however breaks in absence of uboot,
-	  when booting from Metaware debugger directly, as the registers are
-	  not zeroed out on reset by mdb and/or ARCv2 based cores. The bogus
-	  registers look like uboot args to kernel which then chokes.
-	  So only enable the uboot arg checking/processing if users are sure
-	  of uboot being in play.
-
 config ARC_BUILTIN_DTB_NAME
 	string "Built in DTB"
 	help
arch/arc/configs/nps_defconfig +0 -1
index 6e84060e7c90..621f59407d76 100644
@@ -31,7 +31,6 @@ CONFIG_ARC_CACHE_LINE_SHIFT=5
 # CONFIG_ARC_HAS_LLSC is not set
 CONFIG_ARC_KVADDR_SIZE=402
 CONFIG_ARC_EMUL_UNALIGNED=y
-CONFIG_ARC_UBOOT_SUPPORT=y
 CONFIG_PREEMPT=y
 CONFIG_NET=y
 CONFIG_UNIX=y
arch/arc/configs/vdk_hs38_defconfig +0 -1
index 1e59a2e9c602..e447ace6fa1c 100644
@@ -13,7 +13,6 @@ CONFIG_PARTITION_ADVANCED=y
 CONFIG_ARC_PLAT_AXS10X=y
 CONFIG_AXS103=y
 CONFIG_ISA_ARCV2=y
-CONFIG_ARC_UBOOT_SUPPORT=y
 CONFIG_ARC_BUILTIN_DTB_NAME="vdk_hs38"
 CONFIG_PREEMPT=y
 CONFIG_NET=y
arch/arc/configs/vdk_hs38_smp_defconfig +0 -2
index b5c3f6c54b03..c82cdb10aaf4 100644
@@ -15,8 +15,6 @@ CONFIG_AXS103=y
 CONFIG_ISA_ARCV2=y
 CONFIG_SMP=y
 # CONFIG_ARC_TIMERS_64BIT is not set
-# CONFIG_ARC_SMP_HALT_ON_RESET is not set
-CONFIG_ARC_UBOOT_SUPPORT=y
 CONFIG_ARC_BUILTIN_DTB_NAME="vdk_hs38_smp"
 CONFIG_PREEMPT=y
 CONFIG_NET=y
arch/arc/kernel/head.S +0 -2
index 1f945d0f40da..30e090625916 100644
@@ -100,7 +100,6 @@ ENTRY(stext)
 	st.ab   0, [r5, 4]
 1:
 
-#ifdef CONFIG_ARC_UBOOT_SUPPORT
 	; Uboot - kernel ABI
 	;    r0 = [0] No uboot interaction, [1] cmdline in r2, [2] DTB in r2
 	;    r1 = magic number (always zero as of now)

@@ -108,7 +107,6 @@ ENTRY(stext)
 	; These are handled later in handle_uboot_args()
 	st	r0, [@uboot_tag]
 	st	r2, [@uboot_arg]
-#endif
 
 	; setup "current" tsk and optionally cache it in dedicated r25
 	mov	r9, @init_task
arch/arc/kernel/setup.c +0 -2
index 8bb156164556..93d4d6639873 100644
@@ -487,7 +487,6 @@ void __init handle_uboot_args(void)
 	bool use_embedded_dtb = true;
 	bool append_cmdline = false;
 
-#ifdef CONFIG_ARC_UBOOT_SUPPORT
 	/* check that we know this tag */
 	if (uboot_tag != UBOOT_TAG_NONE &&
 	    uboot_tag != UBOOT_TAG_CMDLINE &&

@@ -514,7 +513,6 @@ void __init handle_uboot_args(void)
 		append_cmdline = true;
 
 ignore_uboot_args:
-#endif
 
 	if (use_embedded_dtb) {
 		machine_desc = setup_machine_fdt(__dtb_start);
#1 lore raw
Quoted 5 lines · On Sun, Feb 24, 2019 at 02:54:28PM +0100, [email protected] wrote:

The patch below was submitted to be applied to the 4.20-stable tree.

I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst.

Quoted 9 lines

I could be totally wrong, and if so, please respond to <[email protected]> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again.

thanks,

greg k-h

Hello

Without this patch, we cannot boot a defconfig on ARC, and this is critical for testing ARC devices in kernelCI.
Why this patch is not a real stable patch, it is a strong requirement for testing all future stable release on kernelCI. Note that another patch (touching defconfig) for booting defconfig is still missing and need to be also added to stable. (I will resend it today)

Regards

author maintainer
#2 lore raw
Quoted 13 lines · LABBE Corentin
Quoted 12 lines · On Sun, Feb 24, 2019 at 02:54:28PM +0100, [email protected] wrote:

The patch below was submitted to be applied to the 4.20-stable tree.

I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst.

I could be totally wrong, and if so, please respond to <[email protected]> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again.

thanks,

Quoted 7 lines
Quoted 2 lines

greg k-h

Hello

Without this patch, we cannot boot a defconfig on ARC, and this is critical for testing ARC devices in kernelCI.

But this means you have never been able to boot a defconfig on ARC, right? So this is a new "feature" you are adding :)

If you wait one more week, all is good, you can do this for 5.0, and when I drop 4.20 in another few weeks, all is fine.

Quoted 2 lines

Why this patch is not a real stable patch, it is a strong requirement for testing all future stable release on kernelCI.

Only for 4.20.

And normally kernelci uses a specific configuration for the board it is testing, not defconfig. Or am I mistaken?

This is a fine patch to have upstream, it just really doesn't fit the stable rules.

Quoted 3 lines

Note that another patch (touching defconfig) for booting defconfig is still missing and need to be also added to stable. (I will resend it today)

Again, looks like a new feature. Just wait a week and all will be fine for you :)

thanks,

greg k-h

#3 lore raw
Quoted 17 lines · Greg KH
Quoted 16 lines · LABBE Corentin
Quoted 14 lines · On Sun, Feb 24, 2019 at 02:54:28PM +0100, [email protected] wrote:

The patch below was submitted to be applied to the 4.20-stable tree.

I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst.

I could be totally wrong, and if so, please respond to <[email protected]> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again.

thanks,

greg k-h

Quoted 7 lines
Quoted 4 lines

Hello

Without this patch, we cannot boot a defconfig on ARC, and this is critical for testing ARC devices in kernelCI.

But this means you have never been able to boot a defconfig on ARC, right? So this is a new "feature" you are adding :)

No I was able to boot them, but by adding CONFIG_ARC_UBOOT_SUPPORT.
This patch remove the assertion that ARC devices will be booted by default for debugging purpose.

Quoted 11 lines

If you wait one more week, all is good, you can do this for 5.0, and when I drop 4.20 in another few weeks, all is fine.

Quoted 2 lines

Why this patch is not a real stable patch, it is a strong requirement for testing all future stable release on kernelCI.

Only for 4.20.

And normally kernelci uses a specific configuration for the board it is testing, not defconfig. Or am I mistaken?

No, kernelci only build defconfig without touching them.

Quoted 11 lines

This is a fine patch to have upstream, it just really doesn't fit the stable rules.

Quoted 3 lines

Note that another patch (touching defconfig) for booting defconfig is still missing and need to be also added to stable. (I will resend it today)

Again, looks like a new feature. Just wait a week and all will be fine for you :)

#4 lore raw
In reply to Greg KH
Quoted 17 lines · Greg KH
Quoted 16 lines · LABBE Corentin
Quoted 14 lines · On Sun, Feb 24, 2019 at 02:54:28PM +0100, [email protected] wrote:

The patch below was submitted to be applied to the 4.20-stable tree.

I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst.

I could be totally wrong, and if so, please respond to <[email protected]> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again.

thanks,

greg k-h

Quoted 7 lines
Quoted 4 lines

Hello

Without this patch, we cannot boot a defconfig on ARC, and this is critical for testing ARC devices in kernelCI.

But this means you have never been able to boot a defconfig on ARC, right? So this is a new "feature" you are adding :)

Correct, we've never been able to boot an *upstream* defconfig in kernelCI for ARC.

Quoted 10 lines

If you wait one more week, all is good, you can do this for 5.0, and when I drop 4.20 in another few weeks, all is fine.

Quoted 2 lines

Why this patch is not a real stable patch, it is a strong requirement for testing all future stable release on kernelCI.

Only for 4.20.

And normally kernelci uses a specific configuration for the board it is testing, not defconfig. Or am I mistaken?

We only boot upstream defconfigs, hence the backport request I guess.

Without it, we'll survive. It just means we can only boot v5.1+ on ARC in kernelCI.

Kevin

author maintainer
#5 lore raw
Quoted 38 lines · Kevin Hilman
Quoted 17 lines · Greg KH
Quoted 16 lines · LABBE Corentin
Quoted 14 lines · On Sun, Feb 24, 2019 at 02:54:28PM +0100, [email protected] wrote:

The patch below was submitted to be applied to the 4.20-stable tree.

I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst.

I could be totally wrong, and if so, please respond to <[email protected]> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again.

thanks,

greg k-h

Quoted 7 lines
Quoted 4 lines

Hello

Without this patch, we cannot boot a defconfig on ARC, and this is critical for testing ARC devices in kernelCI.

But this means you have never been able to boot a defconfig on ARC, right? So this is a new "feature" you are adding :)

Correct, we've never been able to boot an *upstream* defconfig in kernelCI for ARC.

Quoted 8 lines

If you wait one more week, all is good, you can do this for 5.0, and when I drop 4.20 in another few weeks, all is fine.

Quoted 2 lines

Why this patch is not a real stable patch, it is a strong requirement for testing all future stable release on kernelCI.

Only for 4.20.

Quoted 7 lines
Quoted 2 lines

And normally kernelci uses a specific configuration for the board it is testing, not defconfig. Or am I mistaken?

We only boot upstream defconfigs, hence the backport request I guess.

Without it, we'll survive. It just means we can only boot v5.1+ on ARC in kernelCI.

5.0+ in kernelCI, this was for a stable patch in 4.20 only, it's already in Linus's tree for 5.0 :)

thanks,

greg k-h