[PATCH 2/2] xen: arm: more flexible scheme for specifying early printk device

[PATCH 2/2] xen: arm: more flexible scheme for specifying early printk device

[Ian Campbell]

This allows for early-printk to be specified (for existing UARTS at
least) without the need to edit Rules.mk.

The existing shortcuts are retained, but in a much more compact
fashion.

An unused EARLY_PRINTK_BAUD was removed from the zynqmq settings.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 docs/misc/arm/early-printk.txt |   34 +++++++++---
 xen/arch/arm/Rules.mk          |  114 ++++++++++++++--------------------------
 2 files changed, 65 insertions(+), 83 deletions(-)

diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-printk.txt
index 44df35e..7e03955 100644
--- a/docs/misc/arm/early-printk.txt
+++ b/docs/misc/arm/early-printk.txt
@@ -7,8 +7,32 @@ Note that selecting this option will limit Xen to a single UART definition.
 Attempting to boot Xen image on a different platform *will not work*, so this
 option should not be enable for Xens that are intended to be portable.
 
-CONFIG_EARLY_PRINTK=mach
-where mach is the name of the machine:
+CONFIG_EARLY_PRINTK=<INC>,<BASE_ADDRESS>,<OTHER_OPTIONS>
+
+<INC> and <BASE_ADDRESS> are mandatory arguments:
+
+  - <INC> is the name of the driver, see xen/arch/arm/arm{32,64}/debug-*.inc
+    (where <INC> corresponds to the wildcarded *).
+  - <BASE_ADDRESS> is the base physical address of the UART to use
+
+<OTHER_OPTIONS> varies depending on <INC>:
+
+  - 8250,<BASE_ADDRESS>,<REG_SHIFT>
+    - <REG_SHIFT> is, optionally, the left-shift to apply to the
+      register offsets within the uart.
+  - pl011,<BASE_ADDRESS>,<BAUD_RATE>
+    - <BAUD_RATE> is, optionally a baud rate which should be used to
+      configure the UART at start of day.
+
+      If <BAUD_RATE> is not given then the code will not try to
+      initialize the UART, so that bootloader or firmware settings can
+     be used for maximum compatibility.
+  - For all other uarts there are no additional options.
+
+As a convenience it is also possible to select from a list of
+predefined configurations using CONFIG_EARLY_PRINTK=mach where mach is
+the name of the machine:
+
   - brcm: printk with 8250 on Broadcom 7445D0 boards with A15 processors.
   - dra7: printk with 8250 on DRA7 platform
   - exynos5250: printk with the second UART
@@ -27,11 +51,7 @@ where mach is the name of the machine:
   - xgene-storm: printk with 820 on Xgene storm platform
   - zynqmp: printk with Cadence UART for Xilinx ZynqMP SoCs
 
-The base address and baud rate is hardcoded in xen/arch/arm/Rules.mk,
+These settings are is hardcoded in xen/arch/arm/Rules.mk,
 see there when adding support for new machines.
-If not explicitly requested with "EARLY_PRINTK_INIT_UART := y" in Rules.mk,
-the code will not try to initialize the UART, so that bootloader or
-firmware settings can be used for maximum compatibility. The baud rate
-parameter is ignored in this case.
 
 By default early printk is disabled.
diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index af3448b..b45cb69 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -42,84 +42,46 @@ EARLY_PRINTK := n
 
 ifeq ($(debug),y)
 
-ifeq ($(CONFIG_EARLY_PRINTK), brcm)
-EARLY_PRINTK_INC := 8250
-EARLY_UART_BASE_ADDRESS := 0xF040AB00
-EARLY_UART_REG_SHIFT := 2
-endif
-ifeq ($(CONFIG_EARLY_PRINTK), dra7)
-EARLY_PRINTK_INC := 8250
-EARLY_UART_BASE_ADDRESS := 0x4806A000
-EARLY_UART_REG_SHIFT := 2
-endif
-ifeq ($(CONFIG_EARLY_PRINTK), fastmodel)
-EARLY_PRINTK_INC := pl011
+# See docs/misc/arm/early-printk.txt for syntax
+
+EARLY_PRINTK_brcm           := 8250,0xF040AB00,2
+EARLY_PRINTK_dra7           := 8250,0x4806A000,2
+EARLY_PRINTK_fastmodel      := pl011,0x1c090000,115200
+EARLY_PRINTK_exynos5250     := exynos4210,0x12c20000
+EARLY_PRINTK_hip04-d01      := 8250,0xE4007000,2
+EARLY_PRINTK_juno           := pl011,0x7ff80000
+EARLY_PRINTK_lager          := scif,0xe6e60000
+EARLY_PRINTK_midway         := pl011,0xfff36000
+EARLY_PRINTK_omap5432       := 8250,0x48020000,2
+EARLY_PRINTK_seattle        := pl011,0xe1010000
+EARLY_PRINTK_sun6i          := 8250,0x01c28000,2
+EARLY_PRINTK_sun7i          := 8250,0x01c28000,2
+EARLY_PRINTK_thunderx       := pl011,0x87e024000000
+EARLY_PRINTK_vexpress       := pl011,0x1c090000
+EARLY_PRINTK_xgene-mcdivitt := 8250,0x1c021000,2
+EARLY_PRINTK_xgene-storm    := 8250,0x1c020000,2
+EARLY_PRINTK_zynqmp         := cadence,0xff000000
+
+COMMA := ,
+ifneq ($(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)),)
+EARLY_PRINTK_CFG := $(subst $(COMMA), ,$(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)))
+else
+EARLY_PRINTK_CFG := $(subst $(COMMA), ,$(CONFIG_EARLY_PRINTK))
+endif
+
+# Extract configuration from string
+EARLY_PRINTK_INC := $(word 1,$(EARLY_PRINTK_CFG))
+EARLY_UART_BASE_ADDRESS := $(word 2,$(EARLY_PRINTK_CFG))
+
+# UART specific options
+ifeq ($(EARLY_PRINTK_INC),8250)
+EARLY_UART_REG_SHIFT := $(word 3,$(EARLY_PRINTK_CFG))
+endif
+ifeq ($(EARLY_PRINTK_INC),pl011)
+ifneq ($(word 3,$(EARLY_PRINTK_CFG)),)
 EARLY_PRINTK_INIT_UART := y
-EARLY_PRINTK_BAUD := 115200
-EARLY_UART_BASE_ADDRESS := 0x1c090000
-endif
-ifeq ($(CONFIG_EARLY_PRINTK), exynos5250)
-EARLY_PRINTK_INC := exynos4210
-EARLY_UART_BASE_ADDRESS := 0x12c20000
-endif
-ifeq ($(CONFIG_EARLY_PRINTK), hip04-d01)
-EARLY_PRINTK_INC := 8250
-EARLY_UART_BASE_ADDRESS := 0xE4007000
-EARLY_UART_REG_SHIFT := 2
-endif
-ifeq ($(CONFIG_EARLY_PRINTK), juno)
-EARLY_PRINTK_INC := pl011
-EARLY_UART_BASE_ADDRESS := 0x7ff80000
-endif
-ifeq ($(CONFIG_EARLY_PRINTK), lager)
-EARLY_PRINTK_INC := scif
-EARLY_UART_BASE_ADDRESS := 0xe6e60000
-endif
-ifeq ($(CONFIG_EARLY_PRINTK), midway)
-EARLY_PRINTK_INC := pl011
-EARLY_UART_BASE_ADDRESS := 0xfff36000
-endif
-ifeq ($(CONFIG_EARLY_PRINTK), omap5432)
-EARLY_PRINTK_INC := 8250
-EARLY_UART_BASE_ADDRESS := 0x48020000
-EARLY_UART_REG_SHIFT := 2
-endif
-ifeq ($(CONFIG_EARLY_PRINTK), seattle)
-EARLY_PRINTK_INC := pl011
-EARLY_UART_BASE_ADDRESS := 0xe1010000
-endif
-ifeq ($(CONFIG_EARLY_PRINTK), sun6i)
-EARLY_PRINTK_INC := 8250
-EARLY_UART_BASE_ADDRESS := 0x01c28000
-EARLY_UART_REG_SHIFT := 2
-endif
-ifeq ($(CONFIG_EARLY_PRINTK), sun7i)
-EARLY_PRINTK_INC := 8250
-EARLY_UART_BASE_ADDRESS := 0x01c28000
-EARLY_UART_REG_SHIFT := 2
-endif
-ifeq ($(CONFIG_EARLY_PRINTK), thunderx)
-EARLY_PRINTK_INC := pl011
-EARLY_UART_BASE_ADDRESS := 0x87e024000000
-endif
-ifeq ($(CONFIG_EARLY_PRINTK), vexpress)
-EARLY_PRINTK_INC := pl011
-EARLY_UART_BASE_ADDRESS := 0x1c090000
-endif
-ifeq ($(CONFIG_EARLY_PRINTK), xgene-mcdivitt)
-EARLY_PRINTK_INC := 8250
-EARLY_UART_BASE_ADDRESS := 0x1c021000
-EARLY_UART_REG_SHIFT := 2
-endif
-ifeq ($(CONFIG_EARLY_PRINTK), xgene-storm)
-EARLY_PRINTK_INC := 8250
-EARLY_UART_BASE_ADDRESS := 0x1c020000
-EARLY_UART_REG_SHIFT := 2
+EARLY_PRINTK_BAUD := $(word 3,$(EARLY_PRINTK_CFG))
 endif
-ifeq ($(CONFIG_EARLY_PRINTK), zynqmp)
-EARLY_PRINTK_INC := cadence
-EARLY_PRINTK_BAUD := 115200
-EARLY_UART_BASE_ADDRESS := 0xff000000
 endif
 
 ifneq ($(EARLY_PRINTK_INC),)
-- 
1.7.10.4

Re: [PATCH 2/2] xen: arm: more flexible scheme for specifying early printk device

[Ian Campbell]

On Wed, 2015-03-11 at 14:11 +0000, Ian Campbell wrote:

Sorry, I was too lazy to write a cover letter for these changes and
forgot that would mean that git send-email wouldn't thread them.

> This allows for early-printk to be specified (for existing UARTS at
> least) without the need to edit Rules.mk.
> 
> The existing shortcuts are retained, but in a much more compact
> fashion.
> 
> An unused EARLY_PRINTK_BAUD was removed from the zynqmq settings.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  docs/misc/arm/early-printk.txt |   34 +++++++++---
>  xen/arch/arm/Rules.mk          |  114 ++++++++++++++--------------------------
>  2 files changed, 65 insertions(+), 83 deletions(-)
> 
> diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-printk.txt
> index 44df35e..7e03955 100644
> --- a/docs/misc/arm/early-printk.txt
> +++ b/docs/misc/arm/early-printk.txt
> @@ -7,8 +7,32 @@ Note that selecting this option will limit Xen to a single UART definition.
>  Attempting to boot Xen image on a different platform *will not work*, so this
>  option should not be enable for Xens that are intended to be portable.
>  
> -CONFIG_EARLY_PRINTK=mach
> -where mach is the name of the machine:
> +CONFIG_EARLY_PRINTK=<INC>,<BASE_ADDRESS>,<OTHER_OPTIONS>
> +
> +<INC> and <BASE_ADDRESS> are mandatory arguments:
> +
> +  - <INC> is the name of the driver, see xen/arch/arm/arm{32,64}/debug-*.inc
> +    (where <INC> corresponds to the wildcarded *).
> +  - <BASE_ADDRESS> is the base physical address of the UART to use
> +
> +<OTHER_OPTIONS> varies depending on <INC>:
> +
> +  - 8250,<BASE_ADDRESS>,<REG_SHIFT>
> +    - <REG_SHIFT> is, optionally, the left-shift to apply to the
> +      register offsets within the uart.
> +  - pl011,<BASE_ADDRESS>,<BAUD_RATE>
> +    - <BAUD_RATE> is, optionally a baud rate which should be used to
> +      configure the UART at start of day.
> +
> +      If <BAUD_RATE> is not given then the code will not try to
> +      initialize the UART, so that bootloader or firmware settings can
> +     be used for maximum compatibility.
> +  - For all other uarts there are no additional options.
> +
> +As a convenience it is also possible to select from a list of
> +predefined configurations using CONFIG_EARLY_PRINTK=mach where mach is
> +the name of the machine:
> +
>    - brcm: printk with 8250 on Broadcom 7445D0 boards with A15 processors.
>    - dra7: printk with 8250 on DRA7 platform
>    - exynos5250: printk with the second UART
> @@ -27,11 +51,7 @@ where mach is the name of the machine:
>    - xgene-storm: printk with 820 on Xgene storm platform
>    - zynqmp: printk with Cadence UART for Xilinx ZynqMP SoCs
>  
> -The base address and baud rate is hardcoded in xen/arch/arm/Rules.mk,
> +These settings are is hardcoded in xen/arch/arm/Rules.mk,
>  see there when adding support for new machines.
> -If not explicitly requested with "EARLY_PRINTK_INIT_UART := y" in Rules.mk,
> -the code will not try to initialize the UART, so that bootloader or
> -firmware settings can be used for maximum compatibility. The baud rate
> -parameter is ignored in this case.
>  
>  By default early printk is disabled.
> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> index af3448b..b45cb69 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -42,84 +42,46 @@ EARLY_PRINTK := n
>  
>  ifeq ($(debug),y)
>  
> -ifeq ($(CONFIG_EARLY_PRINTK), brcm)
> -EARLY_PRINTK_INC := 8250
> -EARLY_UART_BASE_ADDRESS := 0xF040AB00
> -EARLY_UART_REG_SHIFT := 2
> -endif
> -ifeq ($(CONFIG_EARLY_PRINTK), dra7)
> -EARLY_PRINTK_INC := 8250
> -EARLY_UART_BASE_ADDRESS := 0x4806A000
> -EARLY_UART_REG_SHIFT := 2
> -endif
> -ifeq ($(CONFIG_EARLY_PRINTK), fastmodel)
> -EARLY_PRINTK_INC := pl011
> +# See docs/misc/arm/early-printk.txt for syntax
> +
> +EARLY_PRINTK_brcm           := 8250,0xF040AB00,2
> +EARLY_PRINTK_dra7           := 8250,0x4806A000,2
> +EARLY_PRINTK_fastmodel      := pl011,0x1c090000,115200
> +EARLY_PRINTK_exynos5250     := exynos4210,0x12c20000
> +EARLY_PRINTK_hip04-d01      := 8250,0xE4007000,2
> +EARLY_PRINTK_juno           := pl011,0x7ff80000
> +EARLY_PRINTK_lager          := scif,0xe6e60000
> +EARLY_PRINTK_midway         := pl011,0xfff36000
> +EARLY_PRINTK_omap5432       := 8250,0x48020000,2
> +EARLY_PRINTK_seattle        := pl011,0xe1010000
> +EARLY_PRINTK_sun6i          := 8250,0x01c28000,2
> +EARLY_PRINTK_sun7i          := 8250,0x01c28000,2
> +EARLY_PRINTK_thunderx       := pl011,0x87e024000000
> +EARLY_PRINTK_vexpress       := pl011,0x1c090000
> +EARLY_PRINTK_xgene-mcdivitt := 8250,0x1c021000,2
> +EARLY_PRINTK_xgene-storm    := 8250,0x1c020000,2
> +EARLY_PRINTK_zynqmp         := cadence,0xff000000
> +
> +COMMA := ,
> +ifneq ($(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)),)
> +EARLY_PRINTK_CFG := $(subst $(COMMA), ,$(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)))
> +else
> +EARLY_PRINTK_CFG := $(subst $(COMMA), ,$(CONFIG_EARLY_PRINTK))
> +endif
> +
> +# Extract configuration from string
> +EARLY_PRINTK_INC := $(word 1,$(EARLY_PRINTK_CFG))
> +EARLY_UART_BASE_ADDRESS := $(word 2,$(EARLY_PRINTK_CFG))
> +
> +# UART specific options
> +ifeq ($(EARLY_PRINTK_INC),8250)
> +EARLY_UART_REG_SHIFT := $(word 3,$(EARLY_PRINTK_CFG))
> +endif
> +ifeq ($(EARLY_PRINTK_INC),pl011)
> +ifneq ($(word 3,$(EARLY_PRINTK_CFG)),)
>  EARLY_PRINTK_INIT_UART := y
> -EARLY_PRINTK_BAUD := 115200
> -EARLY_UART_BASE_ADDRESS := 0x1c090000
> -endif
> -ifeq ($(CONFIG_EARLY_PRINTK), exynos5250)
> -EARLY_PRINTK_INC := exynos4210
> -EARLY_UART_BASE_ADDRESS := 0x12c20000
> -endif
> -ifeq ($(CONFIG_EARLY_PRINTK), hip04-d01)
> -EARLY_PRINTK_INC := 8250
> -EARLY_UART_BASE_ADDRESS := 0xE4007000
> -EARLY_UART_REG_SHIFT := 2
> -endif
> -ifeq ($(CONFIG_EARLY_PRINTK), juno)
> -EARLY_PRINTK_INC := pl011
> -EARLY_UART_BASE_ADDRESS := 0x7ff80000
> -endif
> -ifeq ($(CONFIG_EARLY_PRINTK), lager)
> -EARLY_PRINTK_INC := scif
> -EARLY_UART_BASE_ADDRESS := 0xe6e60000
> -endif
> -ifeq ($(CONFIG_EARLY_PRINTK), midway)
> -EARLY_PRINTK_INC := pl011
> -EARLY_UART_BASE_ADDRESS := 0xfff36000
> -endif
> -ifeq ($(CONFIG_EARLY_PRINTK), omap5432)
> -EARLY_PRINTK_INC := 8250
> -EARLY_UART_BASE_ADDRESS := 0x48020000
> -EARLY_UART_REG_SHIFT := 2
> -endif
> -ifeq ($(CONFIG_EARLY_PRINTK), seattle)
> -EARLY_PRINTK_INC := pl011
> -EARLY_UART_BASE_ADDRESS := 0xe1010000
> -endif
> -ifeq ($(CONFIG_EARLY_PRINTK), sun6i)
> -EARLY_PRINTK_INC := 8250
> -EARLY_UART_BASE_ADDRESS := 0x01c28000
> -EARLY_UART_REG_SHIFT := 2
> -endif
> -ifeq ($(CONFIG_EARLY_PRINTK), sun7i)
> -EARLY_PRINTK_INC := 8250
> -EARLY_UART_BASE_ADDRESS := 0x01c28000
> -EARLY_UART_REG_SHIFT := 2
> -endif
> -ifeq ($(CONFIG_EARLY_PRINTK), thunderx)
> -EARLY_PRINTK_INC := pl011
> -EARLY_UART_BASE_ADDRESS := 0x87e024000000
> -endif
> -ifeq ($(CONFIG_EARLY_PRINTK), vexpress)
> -EARLY_PRINTK_INC := pl011
> -EARLY_UART_BASE_ADDRESS := 0x1c090000
> -endif
> -ifeq ($(CONFIG_EARLY_PRINTK), xgene-mcdivitt)
> -EARLY_PRINTK_INC := 8250
> -EARLY_UART_BASE_ADDRESS := 0x1c021000
> -EARLY_UART_REG_SHIFT := 2
> -endif
> -ifeq ($(CONFIG_EARLY_PRINTK), xgene-storm)
> -EARLY_PRINTK_INC := 8250
> -EARLY_UART_BASE_ADDRESS := 0x1c020000
> -EARLY_UART_REG_SHIFT := 2
> +EARLY_PRINTK_BAUD := $(word 3,$(EARLY_PRINTK_CFG))
>  endif
> -ifeq ($(CONFIG_EARLY_PRINTK), zynqmp)
> -EARLY_PRINTK_INC := cadence
> -EARLY_PRINTK_BAUD := 115200
> -EARLY_UART_BASE_ADDRESS := 0xff000000
>  endif
>  
>  ifneq ($(EARLY_PRINTK_INC),)

Re: [PATCH 2/2] xen: arm: more flexible scheme for specifying early printk device

[Andrew Cooper]

On 11/03/15 14:11, Ian Campbell wrote:
> +
> +COMMA := ,

COMMA is available from the root Config.mk as 'comma'

~Andrew

Re: [PATCH 2/2] xen: arm: more flexible scheme for specifying early printk device

[Ian Campbell]

On Wed, 2015-03-11 at 14:19 +0000, Andrew Cooper wrote:
> On 11/03/15 14:11, Ian Campbell wrote:
> > +
> > +COMMA := ,
> 
> COMMA is available from the root Config.mk as 'comma'

I had missed the -i off my grep, thanks!

Ian.

Re: [PATCH 2/2] xen: arm: more flexible scheme for specifying early printk device

[Julien Grall]

Hi Ian,

On 11/03/2015 14:11, Ian Campbell wrote:
> This allows for early-printk to be specified (for existing UARTS at
> least) without the need to edit Rules.mk.
>
> The existing shortcuts are retained, but in a much more compact
> fashion.
>
> An unused EARLY_PRINTK_BAUD was removed from the zynqmq settings.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

With the change suggested by Andrew:

Reviewed-by: Julien Grall <julien.grall@linaro.org>

Although, I haven't check closely that all the values are still 
correctly. I guess it was mechanical changes.

Regards,

-- 
Julien Grall

Re: [PATCH 2/2] xen: arm: more flexible scheme for specifying early printk device

[Ian Campbell]

On Wed, 2015-03-11 at 14:47 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 11/03/2015 14:11, Ian Campbell wrote:
> > This allows for early-printk to be specified (for existing UARTS at
> > least) without the need to edit Rules.mk.
> >
> > The existing shortcuts are retained, but in a much more compact
> > fashion.
> >
> > An unused EARLY_PRINTK_BAUD was removed from the zynqmq settings.
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> 
> With the change suggested by Andrew:
> 
> Reviewed-by: Julien Grall <julien.grall@linaro.org>

Thanks.

> Although, I haven't check closely that all the values are still 
> correctly. I guess it was mechanical changes.

It was a series of emacs search-and-replace of s/^MCONFIG_FOO := /,/ and
then some manual tidy up.

Ian.

Re: [PATCH 2/2] xen: arm: more flexible scheme for specifying early printk device

[Tim Deegan]

At 14:11 +0000 on 11 Mar (1426079515), Ian Campbell wrote:
> +EARLY_PRINTK_brcm           := 8250,0xF040AB00,2
> +EARLY_PRINTK_dra7           := 8250,0x4806A000,2
> +EARLY_PRINTK_fastmodel      := pl011,0x1c090000,115200
> +EARLY_PRINTK_exynos5250     := exynos4210,0x12c20000
> +EARLY_PRINTK_hip04-d01      := 8250,0xE4007000,2
> +EARLY_PRINTK_juno           := pl011,0x7ff80000
> +EARLY_PRINTK_lager          := scif,0xe6e60000
> +EARLY_PRINTK_midway         := pl011,0xfff36000
> +EARLY_PRINTK_omap5432       := 8250,0x48020000,2
> +EARLY_PRINTK_seattle        := pl011,0xe1010000
> +EARLY_PRINTK_sun6i          := 8250,0x01c28000,2
> +EARLY_PRINTK_sun7i          := 8250,0x01c28000,2
> +EARLY_PRINTK_thunderx       := pl011,0x87e024000000
> +EARLY_PRINTK_vexpress       := pl011,0x1c090000
> +EARLY_PRINTK_xgene-mcdivitt := 8250,0x1c021000,2
> +EARLY_PRINTK_xgene-storm    := 8250,0x1c020000,2
> +EARLY_PRINTK_zynqmp         := cadence,0xff000000
> +
> +COMMA := ,
> +ifneq ($(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)),)
> +EARLY_PRINTK_CFG := $(subst $(COMMA), ,$(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)))
> +else
> +EARLY_PRINTK_CFG := $(subst $(COMMA), ,$(CONFIG_EARLY_PRINTK))
> +endif

If you're respinning this, it might be neater as

CONFIG_EARLY_PRINTK := $(or $(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)), $(CONFIG_EARLY_PRINTK)
EARLY_PRINTK_CFG := $(subst $(COMMA), ,$(CONFIG_EARLY_PRINTK))

Or maybe not, now that I've written it out. :) Either way is probably fine.

Tim.

Re: [PATCH 2/2] xen: arm: more flexible scheme for specifying early printk device

[Ian Campbell]

On Thu, 2015-03-12 at 12:03 +0100, Tim Deegan wrote:
> At 14:11 +0000 on 11 Mar (1426079515), Ian Campbell wrote:
> > +EARLY_PRINTK_brcm           := 8250,0xF040AB00,2
> > +EARLY_PRINTK_dra7           := 8250,0x4806A000,2
> > +EARLY_PRINTK_fastmodel      := pl011,0x1c090000,115200
> > +EARLY_PRINTK_exynos5250     := exynos4210,0x12c20000
> > +EARLY_PRINTK_hip04-d01      := 8250,0xE4007000,2
> > +EARLY_PRINTK_juno           := pl011,0x7ff80000
> > +EARLY_PRINTK_lager          := scif,0xe6e60000
> > +EARLY_PRINTK_midway         := pl011,0xfff36000
> > +EARLY_PRINTK_omap5432       := 8250,0x48020000,2
> > +EARLY_PRINTK_seattle        := pl011,0xe1010000
> > +EARLY_PRINTK_sun6i          := 8250,0x01c28000,2
> > +EARLY_PRINTK_sun7i          := 8250,0x01c28000,2
> > +EARLY_PRINTK_thunderx       := pl011,0x87e024000000
> > +EARLY_PRINTK_vexpress       := pl011,0x1c090000
> > +EARLY_PRINTK_xgene-mcdivitt := 8250,0x1c021000,2
> > +EARLY_PRINTK_xgene-storm    := 8250,0x1c020000,2
> > +EARLY_PRINTK_zynqmp         := cadence,0xff000000
> > +
> > +COMMA := ,
> > +ifneq ($(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)),)
> > +EARLY_PRINTK_CFG := $(subst $(COMMA), ,$(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)))
> > +else
> > +EARLY_PRINTK_CFG := $(subst $(COMMA), ,$(CONFIG_EARLY_PRINTK))
> > +endif
> 
> If you're respinning this, it might be neater as
> 
> CONFIG_EARLY_PRINTK := $(or $(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)), $(CONFIG_EARLY_PRINTK)
> EARLY_PRINTK_CFG := $(subst $(COMMA), ,$(CONFIG_EARLY_PRINTK))

I didn't know about $(or ...) (but I suppose I could have guessed it
existed, I wonder since when though).

In my early testing I found that 
	CONFIG_EARLY_PRINTK := $(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK))
didn't do as I expected with CONFIG_EARLY_PRINTK on the make command
line, but it's possible I messed up the trial.

> Or maybe not, now that I've written it out. :) Either way is probably fine.

:-)

I could do:

EARLY_PRINTK_CFG := $(subst $(COMMA), ,$(or $(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)),
                                            $(CONFIG_EARLY_PRINTK)))

Not sure that's better either.

Ian.

Re: [PATCH 2/2] xen: arm: more flexible scheme for specifying early printk device

[Tim Deegan]

At 16:51 +0000 on 12 Mar (1426175517), Ian Campbell wrote:
> On Thu, 2015-03-12 at 12:03 +0100, Tim Deegan wrote:
> > CONFIG_EARLY_PRINTK := $(or $(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)), $(CONFIG_EARLY_PRINTK)
> > EARLY_PRINTK_CFG := $(subst $(COMMA), ,$(CONFIG_EARLY_PRINTK))
> 
> I didn't know about $(or ...) (but I suppose I could have guessed it
> existed, I wonder since when though).

Since 3.81 (April 2006), it seems, so OK for debians back to squeeze.
We don't specify a gmake version, but that's very slightly newer than
GCC 4.1.

> In my early testing I found that 
> 	CONFIG_EARLY_PRINTK := $(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK))
> didn't do as I expected with CONFIG_EARLY_PRINTK on the make command
> line, but it's possible I messed up the trial.
> 
> > Or maybe not, now that I've written it out. :) Either way is probably fine.
> 
> :-)
> 
> I could do:
> 
> EARLY_PRINTK_CFG := $(subst $(COMMA), ,$(or $(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)),
>                                             $(CONFIG_EARLY_PRINTK)))
> 
> Not sure that's better either.

Yep, or use another intermediate variable.  The bikeshed will look
lovely.

Tim.

Re: [PATCH 2/2] xen: arm: more flexible scheme for specifying early printk device

[Ian Campbell]

On Wed, 2015-03-11 at 14:47 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 11/03/2015 14:11, Ian Campbell wrote:
> > This allows for early-printk to be specified (for existing UARTS at
> > least) without the need to edit Rules.mk.
> >
> > The existing shortcuts are retained, but in a much more compact
> > fashion.
> >
> > An unused EARLY_PRINTK_BAUD was removed from the zynqmq settings.
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> 
> With the change suggested by Andrew:
> 
> Reviewed-by: Julien Grall <julien.grall@linaro.org>

Applied with the use of the right comma thing.

Tim, I decided to leave the bikeshed the colour it already was.

Ian.