Hi, On 02/04/2024 12:25, Michal Orzel wrote: > > > Hello, > > On 29/03/2024 01:08, Volodymyr Babchuk wrote: >> >> >> Generic Interface (GENI) is a newer interface for low speed interfaces >> like UART, I2C or SPI. This patch adds the simple driver for the UART >> instance of GENI. Code is based on similar drivers in U-Boot and Linux >> kernel. > Do you have a link to a manual? > >> >> This driver implements only simple synchronous mode, because although >> GENI supports FIFO mode, it needs to know number of >> characters **before** starting TX transaction. This is a stark >> contrast when compared to other UART peripherals, which allow adding >> characters to a FIFO while TX operation is running. >> >> The patch adds both normal UART driver and earlyprintk version. >> >> Signed-off-by: Volodymyr Babchuk <volodymyr_babc...@epam.com> >> --- >> xen/arch/arm/Kconfig.debug | 19 +- >> xen/arch/arm/arm64/debug-qcom.inc | 76 +++++++ > Shouldn't all the files (+ other places) have geni in their names? Could qcom > refer to other type of serial device? > > >> xen/arch/arm/include/asm/qcom-uart.h | 48 +++++ >> xen/drivers/char/Kconfig | 8 + >> xen/drivers/char/Makefile | 1 + >> xen/drivers/char/qcom-uart.c | 288 +++++++++++++++++++++++++++ >> 6 files changed, 439 insertions(+), 1 deletion(-) >> create mode 100644 xen/arch/arm/arm64/debug-qcom.inc >> create mode 100644 xen/arch/arm/include/asm/qcom-uart.h >> create mode 100644 xen/drivers/char/qcom-uart.c >> >> diff --git a/xen/arch/arm/Kconfig.debug b/xen/arch/arm/Kconfig.debug >> index eec860e88e..f6ab0bb30e 100644 >> --- a/xen/arch/arm/Kconfig.debug >> +++ b/xen/arch/arm/Kconfig.debug >> @@ -119,6 +119,19 @@ choice >> selecting one of the platform specific options below >> if >> you know the parameters for the port. >> >> + This option is preferred over the platform specific >> + options; the platform specific options are deprecated >> + and will soon be removed. >> + config EARLY_UART_CHOICE_QCOM > The list is sorted alphabetically. Please adhere to that. > >> + select EARLY_UART_QCOM >> + bool "Early printk via Qualcomm debug UART" > Shouldn't you add depends on ARM_64? Isn't this device Arm64 specific like in > Linux? > >> + help >> + Say Y here if you wish the early printk to direct >> their > help text here should be indented by 2 tabs and 2 spaces (do not take example > from surrounding code) > >> + output to a Qualcomm debug UART. You can use this >> option to >> + provide the parameters for the debug UART rather than >> + selecting one of the platform specific options below >> if >> + you know the parameters for the port. >> + >> This option is preferred over the platform specific >> options; the platform specific options are deprecated >> and will soon be removed. >> @@ -211,6 +224,9 @@ config EARLY_UART_PL011 >> config EARLY_UART_SCIF >> select EARLY_PRINTK >> bool >> +config EARLY_UART_QCOM >> + select EARLY_PRINTK >> + bool > The list is sorted alphabetically. Please adhere to that. > >> >> config EARLY_PRINTK >> bool >> @@ -261,7 +277,7 @@ config EARLY_UART_PL011_MMIO32 >> will be done using 32-bit only accessors. >> >> config EARLY_UART_INIT >> - depends on EARLY_UART_PL011 && EARLY_UART_PL011_BAUD_RATE != 0 >> + depends on (EARLY_UART_PL011 && EARLY_UART_PL011_BAUD_RATE != 0) || >> EARLY_UART_QCOM >> def_bool y >> >> config EARLY_UART_8250_REG_SHIFT >> @@ -308,3 +324,4 @@ config EARLY_PRINTK_INC >> default "debug-mvebu.inc" if EARLY_UART_MVEBU >> default "debug-pl011.inc" if EARLY_UART_PL011 >> default "debug-scif.inc" if EARLY_UART_SCIF >> + default "debug-qcom.inc" if EARLY_UART_QCOM >> diff --git a/xen/arch/arm/arm64/debug-qcom.inc >> b/xen/arch/arm/arm64/debug-qcom.inc >> new file mode 100644 >> index 0000000000..130d08d6e9 >> --- /dev/null >> +++ b/xen/arch/arm/arm64/debug-qcom.inc >> @@ -0,0 +1,76 @@ >> +/* >> + * xen/arch/arm/arm64/debug-qcom.inc >> + * >> + * Qualcomm debug UART specific debug code >> + * >> + * Volodymyr Babchuk <volodymyr_babc...@epam.com> >> + * Copyright (C) 2024, EPAM Systems. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. > No need for the license text. You should use SPDX identifier instead at the > top of the file: > /* SPDX-License-Identifier: GPL-2.0-or-later */ I chatted with Julien and it would be best to use GPL-2.0-only.
~Michal
