Messages in this thread | | | Subject | Re: [PATCH] tty: serial: Add earlycon driver for Tegra Combined UART | From | Mikko Perttunen <> | Date | Mon, 15 Feb 2021 12:35:31 +0200 |
| |
On 2/15/21 12:25 PM, Thierry Reding wrote: > On Sat, Feb 13, 2021 at 01:58:24PM +0200, Mikko Perttunen wrote: >> Add an earlycon driver for platforms with TCU, namely Tegra194. >> The driver is compatible with boot parameters passed by NVIDIA >> boot chains. > > I'm not sure I understand the latter part of this description. What boot > parameters is this compatible with? Looking at the setup function there > doesn't seem to be anything out of the ordinary here, so I'm wondering > if that's just confusing. If there's anything special, it might be worth > specifically pointing out what that is. Perhaps both in the commit > message and in a code comment, so it's properly documented.
It's that the name of the driver 'tegra_comb_uart' matches what the boot chain passes; and that OF_EARLYCON_DECLARE is not used. (OF_EARLYCON_DECLARE cannot anyway be used due to the mailbox indirection in device tree).
> >> >> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com> >> --- >> drivers/tty/serial/Kconfig | 12 +++++ >> drivers/tty/serial/Makefile | 1 + >> drivers/tty/serial/tegra-tcu-earlycon.c | 72 +++++++++++++++++++++++++ >> 3 files changed, 85 insertions(+) >> create mode 100644 drivers/tty/serial/tegra-tcu-earlycon.c >> >> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig >> index 34a2899e69c0..d941785e3f46 100644 >> --- a/drivers/tty/serial/Kconfig >> +++ b/drivers/tty/serial/Kconfig >> @@ -331,6 +331,18 @@ config SERIAL_TEGRA_TCU_CONSOLE >> >> If unsure, say Y. >> >> +config SERIAL_TEGRA_TCU_EARLYCON >> + bool "Earlycon on NVIDIA Tegra Combined UART" >> + depends on ARCH_TEGRA || COMPILE_TEST >> + select SERIAL_EARLYCON >> + select SERIAL_CORE_CONSOLE >> + default y if SERIAL_TEGRA_TCU_CONSOLE >> + help >> + If you say Y here, TCU output will be supported during the earlycon >> + phase of the boot. >> + >> + If unsure, say Y. >> + >> config SERIAL_MAX3100 >> tristate "MAX3100 support" >> depends on SPI >> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile >> index b85d53f9e9ff..408144326fed 100644 >> --- a/drivers/tty/serial/Makefile >> +++ b/drivers/tty/serial/Makefile >> @@ -72,6 +72,7 @@ obj-$(CONFIG_SERIAL_XILINX_PS_UART) += xilinx_uartps.o >> obj-$(CONFIG_SERIAL_SIRFSOC) += sirfsoc_uart.o >> obj-$(CONFIG_SERIAL_TEGRA) += serial-tegra.o >> obj-$(CONFIG_SERIAL_TEGRA_TCU) += tegra-tcu.o >> +obj-$(CONFIG_SERIAL_TEGRA_TCU_EARLYCON) += tegra-tcu-earlycon.o >> obj-$(CONFIG_SERIAL_AR933X) += ar933x_uart.o >> obj-$(CONFIG_SERIAL_EFM32_UART) += efm32-uart.o >> obj-$(CONFIG_SERIAL_ARC) += arc_uart.o >> diff --git a/drivers/tty/serial/tegra-tcu-earlycon.c b/drivers/tty/serial/tegra-tcu-earlycon.c >> new file mode 100644 >> index 000000000000..9decfbced0a7 >> --- /dev/null >> +++ b/drivers/tty/serial/tegra-tcu-earlycon.c >> @@ -0,0 +1,72 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) 2017-2021, NVIDIA CORPORATION. All rights reserved. >> + */ >> + >> +#include <linux/console.h> >> +#include <linux/io.h> >> +#include <linux/serial_core.h> >> + >> +#define NUM_BYTES_FIELD_BIT 24 >> +#define FLUSH_BIT 26 > > This one seems to be unused.
True, I'll remove it.
> >> +#define INTR_TRIGGER_BIT 31 > > I wonder if this could somehow be integrated with the existing TCU > driver since we have these bits defined there already. And really this > is basically a skeleton version of the same driver. > >> +/* >> + * This function splits the string to be printed (const char *s) into multiple >> + * packets. Each packet contains a max of 3 characters. Packets are sent to the >> + * SPE-based combined UART server for printing. Communication with SPE is done >> + * through mailbox registers which can generate interrupts for SPE. >> + */ >> +static void early_tcu_write(struct console *console, const char *s, unsigned int count) >> +{ >> + struct earlycon_device *device = console->data; >> + u8 __iomem *addr = device->port.membase; >> + u32 mbox_val = BIT(INTR_TRIGGER_BIT); >> + unsigned int i; >> + >> + /* Loop for processing each 3 char packet */ >> + for (i = 0; i < count; i++) { >> + if (s[i] == '\n') >> + mbox_val = update_and_send_mbox(addr, mbox_val, '\r'); >> + mbox_val = update_and_send_mbox(addr, mbox_val, s[i]); >> + } >> + >> + if ((mbox_val >> NUM_BYTES_FIELD_BIT) & 0x3) { >> + while (readl(addr) & BIT(INTR_TRIGGER_BIT)) >> + cpu_relax(); >> + writel(mbox_val, addr); >> + } >> +} > > For example this function already exists in the Tegra TCU driver and > perhaps some of that could be refactored to work for both cases.
This is very similar to the main tegra_tcu driver, but considering how simple this driver is, and the main driver using the mailbox framework making the actual implementation incompatible, I was thinking that it's easier to just have this be independent.
> > Thierry >
Thanks, Mikko
| |