diff mbox

[linux-review:James-Hogan/kbuild-Remove-stale-asm-generic-wrappers/20160119-183642] d979f99e9cc14e2667e9b6e268db695977e4197a BUILD DONE

Message ID 19656457.qoKNGRmV4Q@wuerfel (mailing list archive)
State New, archived
Headers show

Commit Message

Arnd Bergmann Jan. 29, 2016, 8:44 p.m. UTC 
On Friday 29 January 2016 09:01:31 Geert Uytterhoeven wrote:
> Hi Arnd,
> 
> On Fri, Jan 29, 2016 at 12:07 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > The other related issue is the DEBUG_UART_{VIRT,PHYS} setting,
> > where there is no safe platform-specific default. I have two
> > ideas for working around that, maybe one of them sounds ok to
> > you:
> >
> > a) find a way to warn and/or disable DEBUG_LL when no address
> >    is set, rather than failing the build
> >
> > b) add 'default 0 if COMPILE_TEST' to make it harder to get this
> >    wrong by accident (hopefully nobody tries to run a COMPILE_TEST
> >    kernel). Also maybe add a #warning if DEBUG_UART_VIRT is
> 
> Make sure to add it at the end of the list, so enabling COMPILE_TEST in a
> working .config should give another working .config.

Sure, I've just done a largish series of patches in 4.5 to fix that
bug where we had it already.

> Perhaps you can use 0xdeadbeef instead of 0, and add
> 
>     #if DEBUG_UART_PHYS == 0xdeadbeed
>     #warning Broken value of DEBUG_UART_PHYS.
>     #endif
> 
> somewhere?

I can do that, though I don't see much of an advantage, as zero
is no more likely to be a real address than 0xdeadbeed.

How about the version below?

	Arnd


--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Geert Uytterhoeven Jan. 29, 2016, 9:24 p.m. UTC | #1 
Hi Arnd,

On Fri, Jan 29, 2016 at 9:44 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
> index c6b6175d0203..6cc09cf8618f 100644
> --- a/arch/arm/Kconfig.debug
> +++ b/arch/arm/Kconfig.debug
> @@ -1526,6 +1526,7 @@ config DEBUG_UART_PHYS
>         default 0xfffb9800 if DEBUG_OMAP1UART3 || DEBUG_OMAP7XXUART3
>         default 0xfffe8600 if DEBUG_BCM63XX_UART
>         default 0xfffff700 if ARCH_IOP33X
> +       default 0xdeadbeef if DEBUG_LL_UART_8250 || DEBUG_LL_UART_PL01X || DEBUG_LL_UART_EFM32
>         depends on ARCH_EP93XX || \
>                 DEBUG_LL_UART_8250 || DEBUG_LL_UART_PL01X || \
>                 DEBUG_LL_UART_EFM32 || \
> @@ -1628,6 +1629,7 @@ config DEBUG_UART_VIRT
>         default 0xff003000 if DEBUG_U300_UART
>         default 0xffd01000 if DEBUG_HIP01_UART
>         default DEBUG_UART_PHYS if !MMU
> +       default 0xdeadbeef if DEBUG_LL_UART_8250 || DEBUG_LL_UART_PL01X || DEBUG_LL_UART_EFM32
>         depends on DEBUG_LL_UART_8250 || DEBUG_LL_UART_PL01X || \
>                 DEBUG_UART_8250 || DEBUG_UART_PL01X || DEBUG_MESON_UARTAO || \
>                 DEBUG_NETX_UART || \
> diff --git a/arch/arm/include/debug/8250.S b/arch/arm/include/debug/8250.S
> index 7f7446f6f806..1191b1458586 100644
> --- a/arch/arm/include/debug/8250.S
> +++ b/arch/arm/include/debug/8250.S
> @@ -9,6 +9,9 @@
>   */
>  #include <linux/serial_reg.h>
>
> +#if CONFIG_DEBUG_UART_PHYS == 0xdeadbeef || CONFIG_DEBUG_UART_VIRT < 0xe0000000

Any special reason for 0xe0000000 vs ...

> --- a/arch/arm/include/debug/efm32.S
> +++ b/arch/arm/include/debug/efm32.S
> @@ -6,6 +6,9 @@
>   * it under the terms of the GNU General Public License version 2 as
>   * published by the Free Software Foundation.
>   */
> +#if CONFIG_DEBUG_UART_PHYS == 0xdeadbeef || CONFIG_DEBUG_UART_VIRT < 0xf0000000

0xf0000000?

> --- a/arch/arm/include/debug/pl01x.S
> +++ b/arch/arm/include/debug/pl01x.S
> @@ -10,6 +10,9 @@
>   * published by the Free Software Foundation.
>   *
>  */
> +#if CONFIG_DEBUG_UART_PHYS == 0xdeadbeef || CONFIG_DEBUG_UART_VIRT < 0xf0000000

Likewise.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Jan. 29, 2016, 9:54 p.m. UTC | #2 
On Friday 29 January 2016 22:24:31 Geert Uytterhoeven wrote:
> > diff --git a/arch/arm/include/debug/8250.S b/arch/arm/include/debug/8250.S
> > index 7f7446f6f806..1191b1458586 100644
> > --- a/arch/arm/include/debug/8250.S
> > +++ b/arch/arm/include/debug/8250.S
> > @@ -9,6 +9,9 @@
> >   */
> >  #include <linux/serial_reg.h>
> >
> > +#if CONFIG_DEBUG_UART_PHYS == 0xdeadbeef || CONFIG_DEBUG_UART_VIRT < 0xe0000000
> 
> Any special reason for 0xe0000000 vs ...
> 
> > --- a/arch/arm/include/debug/efm32.S
> > +++ b/arch/arm/include/debug/efm32.S
> > @@ -6,6 +6,9 @@
> >   * it under the terms of the GNU General Public License version 2 as
> >   * published by the Free Software Foundation.
> >   */
> > +#if CONFIG_DEBUG_UART_PHYS == 0xdeadbeef || CONFIG_DEBUG_UART_VIRT < 0xf0000000
> 
> 0xf0000000?

We have one platform that uses a 8250 at address 0xe0010fe0, and one using
the netx UART at virtual address 0xe0000a00, everything else uses 0xf0000000
or higher.

The 0xf0000000 address seems like a better default cutoff, because that
is close to the VMALLOC_START value for systems with 768MB of RAM or more.
Picking a lower number can easily get you in trouble here.

Now that I think about it, I guess platforms that use values above
0xfee00000 can also easily get into trouble as that conflicts with the
PCI I/O space, the fixmap or other special areas documented in
Documentation/arm/memory.txt. We have a bunch of those:

        default 0xfee003f8 if DEBUG_FOOTBRIDGE_COM1
        default 0xfee20000 if DEBUG_NSPIRE_CLASSIC_UART || DEBUG_NSPIRE_CX_UART
        default 0xfee82340 if ARCH_IOP13XX
        default 0xfef00000 if ARCH_IXP4XX && !CPU_BIG_ENDIAN
        default 0xfef00003 if ARCH_IXP4XX && CPU_BIG_ENDIAN
        default 0xfef36000 if DEBUG_HIGHBANK_UART
        default 0xfefb0000 if DEBUG_OMAP1UART1 || DEBUG_OMAP7XXUART1
        default 0xfefb0800 if DEBUG_OMAP1UART2 || DEBUG_OMAP7XXUART2
        default 0xfefb9800 if DEBUG_OMAP1UART3 || DEBUG_OMAP7XXUART3
        default 0xfefff700 if ARCH_IOP33X
        default 0xff003000 if DEBUG_U300_UART
        default 0xffd01000 if DEBUG_HIP01_UART

The HIP01 is the only one that looks actively dangerous here (clashes with
fixmap), the others are probably all fine but it would be nice to stay out
of that area completely.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux Jan. 29, 2016, 11:15 p.m. UTC | #3 
On Fri, Jan 29, 2016 at 10:54:02PM +0100, Arnd Bergmann wrote:
> Now that I think about it, I guess platforms that use values above
> 0xfee00000 can also easily get into trouble as that conflicts with the
> PCI I/O space, the fixmap or other special areas documented in
> Documentation/arm/memory.txt. We have a bunch of those:
> 
>         default 0xfee003f8 if DEBUG_FOOTBRIDGE_COM1
>         default 0xfee20000 if DEBUG_NSPIRE_CLASSIC_UART || DEBUG_NSPIRE_CX_UART
>         default 0xfee82340 if ARCH_IOP13XX
>         default 0xfef00000 if ARCH_IXP4XX && !CPU_BIG_ENDIAN
>         default 0xfef00003 if ARCH_IXP4XX && CPU_BIG_ENDIAN
>         default 0xfef36000 if DEBUG_HIGHBANK_UART
>         default 0xfefb0000 if DEBUG_OMAP1UART1 || DEBUG_OMAP7XXUART1
>         default 0xfefb0800 if DEBUG_OMAP1UART2 || DEBUG_OMAP7XXUART2
>         default 0xfefb9800 if DEBUG_OMAP1UART3 || DEBUG_OMAP7XXUART3
>         default 0xfefff700 if ARCH_IOP33X
>         default 0xff003000 if DEBUG_U300_UART
>         default 0xffd01000 if DEBUG_HIP01_UART
> 
> The HIP01 is the only one that looks actively dangerous here (clashes with
> fixmap), the others are probably all fine but it would be nice to stay out
> of that area completely.

That's wrong.  Given it's PCI bus architecture and it's IO space, at least
one of those above is perfectly valid.  0x3f8 into IO space might ring a
bell... if not the COM1 certainly should. :)
diff mbox

Patch

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index c6b6175d0203..6cc09cf8618f 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -1526,6 +1526,7 @@  config DEBUG_UART_PHYS
 	default 0xfffb9800 if DEBUG_OMAP1UART3 || DEBUG_OMAP7XXUART3
 	default 0xfffe8600 if DEBUG_BCM63XX_UART
 	default 0xfffff700 if ARCH_IOP33X
+	default 0xdeadbeef if DEBUG_LL_UART_8250 || DEBUG_LL_UART_PL01X || DEBUG_LL_UART_EFM32
 	depends on ARCH_EP93XX || \
 	        DEBUG_LL_UART_8250 || DEBUG_LL_UART_PL01X || \
 		DEBUG_LL_UART_EFM32 || \
@@ -1628,6 +1629,7 @@  config DEBUG_UART_VIRT
 	default 0xff003000 if DEBUG_U300_UART
 	default 0xffd01000 if DEBUG_HIP01_UART
 	default DEBUG_UART_PHYS if !MMU
+	default 0xdeadbeef if DEBUG_LL_UART_8250 || DEBUG_LL_UART_PL01X || DEBUG_LL_UART_EFM32
 	depends on DEBUG_LL_UART_8250 || DEBUG_LL_UART_PL01X || \
 		DEBUG_UART_8250 || DEBUG_UART_PL01X || DEBUG_MESON_UARTAO || \
 		DEBUG_NETX_UART || \
diff --git a/arch/arm/include/debug/8250.S b/arch/arm/include/debug/8250.S
index 7f7446f6f806..1191b1458586 100644
--- a/arch/arm/include/debug/8250.S
+++ b/arch/arm/include/debug/8250.S
@@ -9,6 +9,9 @@ 
  */
 #include <linux/serial_reg.h>
 
+#if CONFIG_DEBUG_UART_PHYS == 0xdeadbeef || CONFIG_DEBUG_UART_VIRT < 0xe0000000
+#include "none.S"
+#else
 		.macro	addruart, rp, rv, tmp
 		ldr	\rp, =CONFIG_DEBUG_UART_PHYS
 		ldr	\rv, =CONFIG_DEBUG_UART_VIRT
@@ -55,3 +58,5 @@ 
 		beq	1001b
 #endif
 		.endm
+
+#endif
diff --git a/arch/arm/include/debug/efm32.S b/arch/arm/include/debug/efm32.S
index 660fa1e4b77b..537021761e4a 100644
--- a/arch/arm/include/debug/efm32.S
+++ b/arch/arm/include/debug/efm32.S
@@ -6,6 +6,9 @@ 
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+#if CONFIG_DEBUG_UART_PHYS == 0xdeadbeef || CONFIG_DEBUG_UART_VIRT < 0xf0000000
+#include "none.S"
+#else
 
 #define UARTn_CMD		0x000c
 #define UARTn_CMD_TXEN			0x0004
@@ -43,3 +46,5 @@ 
 		tst	\rd, #UARTn_STATUS_TXC
 		bne	1001b
 		.endm
+
+#endif
diff --git a/arch/arm/include/debug/none.S b/arch/arm/include/debug/none.S
new file mode 100644
index 000000000000..75cd1bbee5c4
--- /dev/null
+++ b/arch/arm/include/debug/none.S
@@ -0,0 +1,16 @@ 
+
+#warning DEBUG_LL not configured, disabling
+
+		.macro	addruart, rp, rv, tmp
+		ldr	\rp, =0
+		ldr	\rv, =0
+		.endm
+
+		.macro	senduart,rd,rx
+		.endm
+
+		.macro	busyuart,rd,rx
+		.endm
+
+		.macro	waituart,rd,rx
+		.endm
diff --git a/arch/arm/include/debug/pl01x.S b/arch/arm/include/debug/pl01x.S
index f7d8323cefcc..fbe0cad32be0 100644
--- a/arch/arm/include/debug/pl01x.S
+++ b/arch/arm/include/debug/pl01x.S
@@ -10,6 +10,9 @@ 
  * published by the Free Software Foundation.
  *
 */
+#if CONFIG_DEBUG_UART_PHYS == 0xdeadbeef || CONFIG_DEBUG_UART_VIRT < 0xf0000000
+#include "none.S"
+#else
 #include <linux/amba/serial.h>
 
 #ifdef CONFIG_DEBUG_ZTE_ZX
@@ -43,3 +46,4 @@ 
 		tst	\rd, #UART01x_FR_BUSY
 		bne	1001b
 		.endm
+#endif