Re: [PATCH] arm: Drop deprecated early printk platform options

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Michal,

> On 13 Sep 2024, at 08:15, Michal Orzel <michal.orzel@xxxxxxx> wrote:
> 
> The predefined configurations for early printk have been deprecated for
> a sufficient amount of time. Let's finally remove them.
> 
> Note:
> In order not to loose these predefined configurations, I wrote a wiki
> page: https://wiki.xenproject.org/wiki/Xen_on_ARM_Early_Printk
> 
> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> ---
> docs/misc/arm/early-printk.txt |   4 -
> xen/arch/arm/Kconfig.debug     | 155 ++-------------------------------
> 2 files changed, 9 insertions(+), 150 deletions(-)
> 
> diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-printk.txt
> index bc2d65aa2ea3..e6b9124a38e3 100644
> --- a/docs/misc/arm/early-printk.txt
> +++ b/docs/misc/arm/early-printk.txt
> @@ -35,8 +35,4 @@ Other options depends on the driver selected:
> 
>   - For all other uarts there are no additional options.
> 
> -As a convenience it is also possible to select from a list of
> -predefined configurations available in the list of choice for "Early
> -printk" for specific platform.
> -
> By default early printk is disabled.
> diff --git a/xen/arch/arm/Kconfig.debug b/xen/arch/arm/Kconfig.debug
> index eec860e88e0b..2fa0acd2a3f1 100644
> --- a/xen/arch/arm/Kconfig.debug
> +++ b/xen/arch/arm/Kconfig.debug
> @@ -13,177 +13,67 @@ choice
> Choose one of the UART drivers for early printk, then you'll
> have to specify the parameters, like the base address.
> 
> - Deprecated: Alternatively, there are platform specific options
> - which will have default values for the various parameters. But
> - such option will soon be removed.
> -
> config EARLY_UART_CHOICE_8250
> select EARLY_UART_8250
> bool "Early printk via 8250 UART"
> help
> Say Y here if you wish the early printk to direct their
> - output to a 8250 UART. You can use this option to
> - provide the parameters for the 8250 UART rather than
> - selecting one of the platform specific options below if
> - you know the parameters for the port.
> + output to a 8250 UART.
> 
> - This option is preferred over the platform specific
> - options; the platform specific options are deprecated
> - and will soon be removed.
> config EARLY_UART_CHOICE_CADENCE
> select EARLY_UART_CADENCE
> depends on ARM_64
> bool "Early printk via Cadence UART"
> help
> Say Y here if you wish the early printk to direct their
> - output to a Cadence UART. You can use this option to
> - provide the parameters for the Cadence UART rather than
> - selecting one of the platform specific options below if
> - you know the parameters for the port.
> + output to a Cadence UART.
> 
> - This option is preferred over the platform specific
> - options; the platform specific options are deprecated
> - and will soon be removed.
> config EARLY_UART_CHOICE_EXYNOS4210
> select EARLY_UART_EXYNOS4210
> depends on ARM_32
> bool "Early printk via Exynos4210 UART"
> help
> Say Y here if you wish the early printk to direct their
> - output to a Exynos 4210 UART. You can use this option to
> - provide the parameters for the Exynos 4210 UART rather than
> - selecting one of the platform specific options below if
> - you know the parameters for the port.
> + output to a Exynos 4210 UART.
> 
> - This option is preferred over the platform specific
> - options; the platform specific options are deprecated
> - and will soon be removed.
> config EARLY_UART_CHOICE_IMX_LPUART
> select EARLY_UART_IMX_LPUART
> depends on ARM_64
> bool "Early printk via i.MX LPUART"
> help
> Say Y here if you wish the early printk to direct their
> - output to a i.MX LPUART. You can use this option to
> - provide the parameters for the i.MX LPUART rather than
> - selecting one of the platform specific options below if
> - you know the parameters for the port.
> + output to a i.MX LPUART.
> +
> config EARLY_UART_CHOICE_MESON
> select EARLY_UART_MESON
> depends on ARM_64
> bool "Early printk via MESON UART"
> help
> Say Y here if you wish the early printk to direct their
> - output to a MESON UART. You can use this option to
> - provide the parameters for the MESON UART rather than
> - selecting one of the platform specific options below if
> - you know the parameters for the port.
> + output to a MESON UART.
> 
> - This option is preferred over the platform specific
> - options; the platform specific options are deprecated
> - and will soon be removed.
> config EARLY_UART_CHOICE_MVEBU
> select EARLY_UART_MVEBU
> depends on ARM_64
> bool "Early printk via MVEBU UART"
> help
> Say Y here if you wish the early printk to direct their
> - output to a MVEBU UART. You can use this option to
> - provide the parameters for the MVEBU UART rather than
> - selecting one of the platform specific options below if
> - you know the parameters for the port.
> + output to a MVEBU UART.
> 
> - This option is preferred over the platform specific
> - options; the platform specific options are deprecated
> - and will soon be removed.
> config EARLY_UART_CHOICE_PL011
> select EARLY_UART_PL011
> bool "Early printk via PL011 UART"
> help
> Say Y here if you wish the early printk to direct their
> - output to a PL011 UART. You can use this option to
> - provide the parameters for the PL011 UART rather than
> - selecting one of the platform specific options below if
> - you know the parameters for the port.
> + output to a PL011 UART.
> 
> - This option is preferred over the platform specific
> - options; the platform specific options are deprecated
> - and will soon be removed.
> config EARLY_UART_CHOICE_SCIF
> select EARLY_UART_SCIF
> bool "Early printk via SCIF UART"
> help
> Say Y here if you wish the early printk to direct their
> - output to a SCIF UART. You can use this option to
> - provide the parameters for the SCIF UART rather than
> - selecting one of the platform specific options below if
> - you know the parameters for the port.
> + output to a SCIF UART.
> 
> - This option is preferred over the platform specific
> - options; the platform specific options are deprecated
> - and will soon be removed.
> -
> - config EARLY_PRINTK_BRCM
> - bool "Early printk with 8250 on Broadcom 7445D0 boards with A15 processors"
> - select EARLY_UART_8250
> - config EARLY_PRINTK_DRA7
> - bool "Early printk with 8250 on DRA7 platform"
> - select EARLY_UART_8250
> - config EARLY_PRINTK_EXYNOS5250
> - bool "Early printk with the second UART on Exynos5250"
> - select EARLY_UART_EXYNOS4210
> - depends on ARM_32
> - config EARLY_PRINTK_FASTMODEL
> - bool "Early printk with pl011 on ARM Fastmodel software emulators"
> - select EARLY_UART_PL011
> - config EARLY_PRINTK_HIKEY960
> - bool "Early printk with pl011 with Hikey 960"
> - select EARLY_UART_PL011
> - config EARLY_PRINTK_JUNO
> - bool "Early printk with pl011 on Juno platform"
> - select EARLY_UART_PL011
> - config EARLY_PRINTK_LAGER
> - bool "Early printk with SCIF0 on Renesas Lager board (R-Car H2 processor)"
> - select EARLY_UART_SCIF
> - config EARLY_PRINTK_MIDWAY
> - bool "Early printk with pl011 on Calxeda Midway processors"
> - select EARLY_UART_PL011
> - config EARLY_PRINTK_MVEBU
> - bool "Early printk with MVEBU for Marvell Armada 3700 SoCs"
> - select EARLY_UART_MVEBU
> - depends on ARM_64
> - config EARLY_PRINTK_OMAP5432
> - bool "Early printk with UART3 on TI OMAP5432 processors"
> - select EARLY_UART_8250
> - config EARLY_PRINTK_RCAR3
> - bool "Early printk with SCIF2 on Renesas R-Car Gen3 processors"
> - select EARLY_UART_SCIF
> - config EARLY_PRINTK_SEATTLE
> - bool "Early printk with pl011 for AMD Seattle processor"
> - select EARLY_UART_PL011
> - config EARLY_PRINTK_SUN6I
> - bool "Early printk with 8250 on Allwinner A31 processors"
> - select EARLY_UART_8250
> - config EARLY_PRINTK_SUN7I
> - bool "Early printk with 8250 on Allwinner A20 processors"
> - select EARLY_UART_8250
> - config EARLY_PRINTK_THUNDERX
> - bool "Early printk with pl011 for Cavium ThunderX processor"
> - select EARLY_UART_PL011
> - depends on ARM_64
> - config EARLY_PRINTK_VEXPRESS
> - bool "Early printk with pl011 for versatile express"
> - select EARLY_UART_PL011
> - config EARLY_PRINTK_XGENE_MCDIVITT
> - bool "Early printk with 820 on Xgene mcdivitt platform"
> - select EARLY_UART_8250
> - config EARLY_PRINTK_XGENE_STORM
> - bool "Early printk with 820 on Xgene storm platform"
> - select EARLY_UART_8250
> - config EARLY_PRINTK_ZYNQMP
> - bool "Early printk with Cadence UART for Xilinx ZynqMP SoCs"
> - select EARLY_UART_CADENCE
> - depends on ARM_64
> endchoice
> 
> 
> @@ -219,25 +109,6 @@ config EARLY_UART_BASE_ADDRESS
> depends on EARLY_PRINTK
> hex "Early printk, physical base address of debug UART"
> range 0x0 0xffffffff if ARM_32
> - default 0xF040AB00 if EARLY_PRINTK_BRCM
> - default 0x4806A000 if EARLY_PRINTK_DRA7
> - default 0x1c090000 if EARLY_PRINTK_FASTMODEL
> - default 0x12c20000 if EARLY_PRINTK_EXYNOS5250
> - default 0xfff32000 if EARLY_PRINTK_HIKEY960
> - default 0x7ff80000 if EARLY_PRINTK_JUNO
> - default 0xe6e60000 if EARLY_PRINTK_LAGER
> - default 0xfff36000 if EARLY_PRINTK_MIDWAY
> - default 0xd0012000 if EARLY_PRINTK_MVEBU
> - default 0x48020000 if EARLY_PRINTK_OMAP5432
> - default 0xe6e88000 if EARLY_PRINTK_RCAR3
> - default 0xe1010000 if EARLY_PRINTK_SEATTLE
> - default 0x01c28000 if EARLY_PRINTK_SUN6I
> - default 0x01c28000 if EARLY_PRINTK_SUN7I
> - default 0x87e024000000 if EARLY_PRINTK_THUNDERX
> - default 0x1c090000 if EARLY_PRINTK_VEXPRESS
> - default 0x1c021000 if EARLY_PRINTK_XGENE_MCDIVITT
> - default 0x1c020000 if EARLY_PRINTK_XGENE_STORM
> - default 0xff000000 if EARLY_PRINTK_ZYNQMP

Shouldn't we put a default 0 value here ?

> 
> config EARLY_UART_PL011_BAUD_RATE
> depends on EARLY_UART_PL011
> @@ -250,7 +121,6 @@ config EARLY_UART_PL011_BAUD_RATE
> not try to initialize the UART, so that bootloader or firmware
> settings can be used for maximum compatibility.
> 
> - default 115200 if EARLY_PRINTK_FASTMODEL
> default 0
> 
> config EARLY_UART_PL011_MMIO32
> @@ -274,13 +144,6 @@ config EARLY_UART_8250_REG_SHIFT
> 
> Default to 0.
> 
> - default 2 if EARLY_PRINTK_BRCM
> - default 2 if EARLY_PRINTK_DRA7
> - default 2 if EARLY_PRINTK_OMAP5432
> - default 2 if EARLY_PRINTK_SUN6I
> - default 2 if EARLY_PRINTK_SUN7I
> - default 2 if EARLY_PRINTK_XGENE_MCDIVITT
> - default 2 if EARLY_PRINTK_XGENE_STORM
> default 0

Seeing that all know example are setting this to 2, wouldn't it
make sense to change the default to 2 ?

Cheers
Bertrand

> 
> choice EARLY_UART_SCIF_VERSION
> -- 
> 2.25.1
>