[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • From: Michal Orzel <michal.orzel@xxxxxxx>
  • Date: Fri, 13 Sep 2024 09:28:45 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=arm.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=pTXHH1ir/t+Nht7gGRFT1ImLSvImThE6txZ9jRwFxcQ=; b=MDt0w5swV/DcD8xJwJh9e8I2GATEkwXdzv+0oy+3t5ObSolRMPZmA96WTCmXEM2st9hgDAdKJCyUkH/bafxFG63fI9CG9xtI0ZwgGo2Ao6tK9PBd5/WqHO9GlQj9s+TD+yhO/ZzZdERCBNYVinZO+tu/uV7bfpSXH3OQu6EMms1eeVBTIwb6OVf4dh9r2vzzWFBD9/wg9xa4/2nRC1GfrYzoJKEoXcpJqOUPqjAYJDDK5c9zOk0wpUMKwV0zmygOaTXvLIqpturASjBXb3rsRoaIdKWYVCviLeWtxnsoiealWYkzsgYivLhAUFAmg3TF/8pholgpbon7piRiwsRG/Q==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=qvWXupPewOeBJNpM6xCI/1NU0O3Lq1vqOWEkM7D5TcNydlIZWvXrWvU8mXC9ME1r5pEktwbL+22tOMyL/TAX+kknUSwmubo5dq8+uUbAHX/DQFkBaigoX6xXGkPqzlGPCzUc9YnwMLaLgFgKCZLSemxWptrvxeIZyM8Jf1jh8KWlnaazLG7E8J0XH9DszyYWnXKY7vFT4T1nYGVRuCOjNAduLh8b2ANqU+XMXEKNv2t8qwlyKNVI5Qs2DWagUVrhT6xZ9dw6jl54Dyy1oaFWbhm/Dn4bB/5BYMKqnSkxz4pAxhD8atSbRCBcPR/cLMkvpB3BzxBl71BLuPPun+3fSw==
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Fri, 13 Sep 2024 07:29:19 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Bertrand,

On 13/09/2024 08:54, Bertrand Marquis wrote:
> 
> 
> 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 ?
No, we should not. It's done like that on purpose to trigger build error
if user enables early printk but forgets to specify an address. Adding default 0
would remove this guard behavior.

> 
>>
>> 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 ?
It would be a change in behavior and would not fit the purpose of this patch.
Also, the EARLY_UART_8250_REG_SHIFT Kconfig help mentions 0 as a default.
There might be platforms 8250 based where we don't know the correct reg shift 
value.
Better not to make any assumptions.

~Michal



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.