On 2/8/19 1:47 PM, Oleksandr wrote: > > On 05.02.19 20:56, Marek Vasut wrote: > > Hi Marek
Hi, >> On 1/31/19 6:38 PM, Oleksandr Tyshchenko wrote: >>> From: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com> >>> >>> Also take care of the fact that Lager and Stout boards use >>> different serial interface for console. >> This describes something else than $subject , please split the patchset >> such that one patch does one thing and describes that one thing in the >> commit message. > > Yes, make sense. Will split. > >>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com> >>> --- >>> arch/arm/mach-rmobile/debug.h | 91 >>> +++++++++++++++++++++++++++++++++++++++++++ >>> arch/arm/mach-rmobile/psci.c | 23 +++++++++++ >>> 2 files changed, 114 insertions(+) >>> create mode 100644 arch/arm/mach-rmobile/debug.h >>> >>> diff --git a/arch/arm/mach-rmobile/debug.h >>> b/arch/arm/mach-rmobile/debug.h >>> new file mode 100644 >>> index 0000000..5d4ec77 >>> --- /dev/null >>> +++ b/arch/arm/mach-rmobile/debug.h >>> @@ -0,0 +1,91 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 */ >>> +/* >>> + * Contains functions used for PSCI debug. >>> + * >>> + * Copyright (C) 2018 EPAM Systems Inc. >>> + * >>> + * Based on arch/arm/mach-uniphier/debug.h >>> + */ >>> + >>> +#ifndef __DEBUG_H__ >>> +#define __DEBUG_H__ >>> + >>> +#include <asm/io.h> >>> + >>> +/* SCIFA definitions */ >>> +#define SCIFA_BASE 0xe6c40000 >> Should come from DT > > I have just found that rcar-base.h already contains #define-s for all > SCIF(A)s. So does the DT, and the addresses should come from DT, not from ad-hoc constants in U-Boot. Those will likely be removed at some point. >>> +#define SCIFA_SCASSR 0x14 /* Serial status register */ >>> +#define SCIFA_SCAFTDR 0x20 /* Transmit FIFO data register */ >>> + >>> +/* SCIF definitions */ >>> +#define SCIF_BASE 0xe6e60000 >>> + >>> +#define SCIF_SCFSR 0x10 /* Serial status register */ >>> +#define SCIF_SCFTDR 0x0c /* Transmit FIFO data register */ >>> + >>> +/* Common for both interfaces definitions */ >>> +#define SCFSR_TDFE BIT(5) /* Transmit FIFO Data Empty */ >>> +#define SCFSR_TEND BIT(6) /* Transmission End */ >>> + >>> +#ifdef CONFIG_SCIF_A >>> +#define UART_BASE SCIFA_BASE >>> +#define UART_STATUS_REG SCIFA_SCASSR >>> +#define UART_TX_FIFO_REG SCIFA_SCAFTDR >>> +#else >>> +#define UART_BASE SCIF_BASE >>> +#define UART_STATUS_REG SCIF_SCFSR >>> +#define UART_TX_FIFO_REG SCIF_SCFTDR >>> +#endif >>> + >>> +/* All functions are inline so that they can be called from .secure >>> section. */ >>> + >>> +#ifdef DEBUG >>> +static inline void debug_putc(int c) >>> +{ >>> + void __iomem *base = (void __iomem *)UART_BASE; >>> + >>> + while (!(readw(base + UART_STATUS_REG) & SCFSR_TDFE)) >>> + ; >>> + >>> + writeb(c, base + UART_TX_FIFO_REG); >>> + writew(readw(base + UART_STATUS_REG) & ~(SCFSR_TEND | SCFSR_TDFE), >>> + base + UART_STATUS_REG); >> Is this some implementation of debug uart API or is this a >> reimplementation of the serial driver ? > > I would say it is some implementation of debug uart API. > > Let me explain why it is needed. We need something very simple to be > called from the PSCI backend > > in order to have possibility for debugging it. Actually the only thing > we need is a simple function to write a char into uart TX register. > > Actually I borrowed that idea from the implementation for mach-uniphier > and modified according to the SCIF(A) usage. > > These are examples, how it looks like: > > 1. > > [ 3.193974] psci_checker: Trying to turn off and on again group 1 > (CPUs 4-7) > [U-Boot PSCI] cpu_off: cpu=00000004 > [ 3.233648] Retrying again to check for CPU kill > [ 3.238269] CPU4 killed. > [U-Boot PSCI] cpu_off: cpu=00000005 > [ 3.263678] Retrying again to check for CPU kill > [ 3.268298] CPU5 killed. > [U-Boot PSCI] cpu_off: cpu=00000006 > [ 3.293648] Retrying again to check for CPU kill > [ 3.298268] CPU6 killed. > [U-Boot PSCI] cpu_off: cpu=00000007 > [ 3.323691] Retrying again to check for CPU kill > [ 3.328310] CPU7 killed. > [U-Boot PSCI] cpu_on: cpu=00000001, target_cpu=00000100, > entry_point=48102440, context_id=00000000 > [U-Boot PSCI] cpu_on: cpu=00000002, target_cpu=00000101, > entry_point=48102440, context_id=00000000 > [U-Boot PSCI] cpu_on: cpu=00000001, target_cpu=00000102, > entry_point=48102440, context_id=00000000 > [U-Boot PSCI] cpu_on: cpu=00000001, target_cpu=00000103, > entry_point=48102440, context_id=00000000 > > > 2. > (XEN) Bringing up CPU4 > [U-Boot PSCI] cpu_on: cpu=00000000, target_cpu=00000100, > entry_point=4800004c, context_id=0030e208 > - CPU 00000100 booting - > - Xen starting in Hyp mode - > - Setting up control registers - > - Turning on paging - > - Ready - > (XEN) Adding cpu 4 to runqueue 0 > (XEN) CPU 4 booted. > (XEN) Bringing up CPU5 > [U-Boot PSCI] cpu_on: cpu=00000000, target_cpu=00000101, > entry_point=4800004c, context_id=0030e208 > - CPU 00000101 booting - > - Xen starting in Hyp mode - > - Setting up control registers - > - Turning on paging - > - Ready - > (XEN) Adding cpu 5 to runqueue 0 > (XEN) CPU 5 booted. > (XEN) Bringing up CPU6 > [U-Boot PSCI] cpu_on: cpu=00000000, target_cpu=00000102, > entry_point=4800004c, context_id=0030e208 > - CPU 00000102 booting - > - Xen starting in Hyp mode - > - Setting up control registers - > - Turning on paging - > - Ready - > (XEN) Adding cpu 6 to runqueue 0 > (XEN) CPU 6 booted. > (XEN) Bringing up CPU7 > [U-Boot PSCI] cpu_on: cpu=00000000, target_cpu=00000103, > entry_point=4800004c, context_id=0030e208 > - CPU 00000103 booting - > - Xen starting in Hyp mode - > - Setting up control registers - > - Turning on paging - > - Ready - > (XEN) Adding cpu 7 to runqueue 0 > (XEN) Brought up 8 CPUs > (XEN) CPU 7 booted. OK, this should then sit in drivers/serial/ . The UART base address and properties should be selected via DT, not via some ad-hoc constant hard-coded into U-Boot. [...] -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot