From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 336D8C433EF for ; Thu, 14 Oct 2021 15:13:34 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B29D561039 for ; Thu, 14 Oct 2021 15:13:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org B29D561039 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 79C1C837D8; Thu, 14 Oct 2021 17:12:37 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="b5QQPisP"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id D4865837B7; Thu, 14 Oct 2021 17:10:33 +0200 (CEST) Received: from mail-ot1-x329.google.com (mail-ot1-x329.google.com [IPv6:2607:f8b0:4864:20::329]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 1D4C483794 for ; Thu, 14 Oct 2021 17:10:15 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-ot1-x329.google.com with SMTP id d21-20020a9d4f15000000b0054e677e0ac5so8598552otl.11 for ; Thu, 14 Oct 2021 08:10:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=+Bj3RgRkVN65viDIrgPz4Xffcss6k4fM4/vebbc7dy4=; b=b5QQPisPD4GuIkKTcBKoF0t1iFlimIx7gboIVOJw/owCearSUX2RqFVdRJxuKDQcmA HYZli+bcpgZYLmecbgx1Z1MQsmIkEU71q+scoRSAnBkI6ihlTfmT/Yc+RfUajHWVdpT6 rjbHjC8/hJMWmqe7/f261ov56UxS844ZOJ3oA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=+Bj3RgRkVN65viDIrgPz4Xffcss6k4fM4/vebbc7dy4=; b=FIJIf00uq4iqB4B9bIQXH6vatoIaiWFbgumUDqkMdKDktHBmlbHcaMphVhKAV213EC nPSac9DwwcJXGL48fN2xi18wbWYSiO0Bd7y2vxe3e+pk2HvN7vEVGVXxCBLtoxTm1DpI 6NjfhvE2kUffd1K184HBFk1jeLn26g7VAYf/Tp3nki2zfZPczBWgNseCVtNvNnGv8GxB d5Xgcj57GwEvxoNBTvg0s/VSjMj+DJ9N6PQk/Kd/YiGr54dARL2Ru+EMn3kKt1+U1jvE bd+/XhTeuDAaRBnKT9XvpS6k670giCLk5VNfYV9zlqximpqy/NW8uOnmwxYRzSXb5F0C wqQA== X-Gm-Message-State: AOAM5316A1/4watqOiWBd+TAZILUEZ7oAqwlvHiaxiqKZzgJphE1OtVF 7lJ6GDhFmhCmeTZ5X7y/8yfzXeDOQ0X+qvH8PMoVLA== X-Google-Smtp-Source: ABdhPJyfEP73L/D39hufaXhWPET5vCRL6hUWrohKqiSc9zJACkEN+bQ2WClaPrVb89eEcUBokf8ssgCVtsap4RQNTdQ= X-Received: by 2002:a9d:17c5:: with SMTP id j63mr3126674otj.191.1634224212527; Thu, 14 Oct 2021 08:10:12 -0700 (PDT) MIME-Version: 1.0 References: <20211008063715.13375-1-dsankouski@gmail.com> In-Reply-To: <20211008063715.13375-1-dsankouski@gmail.com> From: Simon Glass Date: Thu, 14 Oct 2021 09:09:58 -0600 Message-ID: Subject: Re: [PATCH 1/6 v4] serial: qcom: add support for GENI serial driver To: Dzmitry Sankouski Cc: U-Boot Mailing List , Ramon Fried , Tom Rini Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Hi Dzmitry, On Fri, 8 Oct 2021 at 00:46, Dzmitry Sankouski wrote: > > Generic Interface (GENI) Serial Engine (SE) based uart > can be found on newer qualcomm SOCs, starting from SDM845. > Tested on Samsung SM-G9600(starqltechn) > by chain-loading u-boot with stock bootloader. > > Signed-off-by: Dzmitry Sankouski > Cc: Ramon Fried > Cc: Tom Rini > --- > Changes for v2: > - change functions return type to void, where possible > - remove '.' from summary line > Changes for v3: > - move function open brace on new line > - use tab between define name and value > - define: wrap expression with braces, remove braces from constants > Changes for v4: > - add linux/delay.h header > > MAINTAINERS | 1 + > .../serial/msm-geni-serial.txt | 6 + > drivers/serial/Kconfig | 17 + > drivers/serial/Makefile | 1 + > drivers/serial/serial_msm_geni.c | 603 ++++++++++++++++++ > 5 files changed, 628 insertions(+) > create mode 100644 doc/device-tree-bindings/serial/msm-geni-serial.txt > create mode 100644 drivers/serial/serial_msm_geni.c Reviewed-by: Simon Glass Some nits below > > diff --git a/MAINTAINERS b/MAINTAINERS > index 776ff703b9..52ddc99cda 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -390,6 +390,7 @@ F: drivers/gpio/msm_gpio.c > F: drivers/mmc/msm_sdhci.c > F: drivers/phy/msm8916-usbh-phy.c > F: drivers/serial/serial_msm.c > +F: drivers/serial/serial_msm_geni.c > F: drivers/smem/msm_smem.c > F: drivers/usb/host/ehci-msm.c > > diff --git a/doc/device-tree-bindings/serial/msm-geni-serial.txt b/doc/device-tree-bindings/serial/msm-geni-serial.txt > new file mode 100644 > index 0000000000..9eadc2561b > --- /dev/null > +++ b/doc/device-tree-bindings/serial/msm-geni-serial.txt > @@ -0,0 +1,6 @@ > +Qualcomm GENI UART > + > +Required properties: > +- compatible: must be "qcom,msm-geni-uart" > +- reg: start address and size of the registers > +- clock: interface clock (must accept baudrate as a frequency) > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig > index 93348c0929..b420a5720d 100644 > --- a/drivers/serial/Kconfig > +++ b/drivers/serial/Kconfig > @@ -278,6 +278,14 @@ config DEBUG_UART_S5P > will need to provide parameters to make this work. The driver will > be available until the real driver-model serial is running. > > +config DEBUG_UART_MSM_GENI Do you need this? Most drivers just use the existing DEBUG_UART Kconfig. > + bool "Qualcomm snapdragon" > + depends on ARCH_SNAPDRAGON > + help > + Select this to enable a debug UART using the serial_msm driver. You > + will need to provide parameters to make this work. The driver will > + be available until the real driver-model serial is running. > + > config DEBUG_UART_MESON > bool "Amlogic Meson" > depends on MESON_SERIAL > @@ -783,6 +791,15 @@ config MSM_SERIAL > for example APQ8016 and MSM8916. > Single baudrate is supported in current implementation (115200). > > +config MSM_GENI_SERIAL > + bool "Qualcomm on-chip GENI UART" > + help > + Support UART based on Generic Interface (GENI) Serial Engine (SE), used on Qualcomm Snapdragon SoCs. > + Should support all qualcomm SOCs with Qualcomm Universal Peripheral (QUP) Wrapper cores, 80cols > + i.e. newer ones, starting from SDM845. > + Driver works in FIFO mode. > + Multiple baudrates supported. > + > config OCTEON_SERIAL_BOOTCMD > bool "MIPS Octeon PCI remote bootcmd input" > depends on ARCH_OCTEON > diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile > index 3cbea8156f..d44caf4ea2 100644 > --- a/drivers/serial/Makefile > +++ b/drivers/serial/Makefile > @@ -62,6 +62,7 @@ obj-$(CONFIG_PIC32_SERIAL) += serial_pic32.o > obj-$(CONFIG_BCM283X_MU_SERIAL) += serial_bcm283x_mu.o > obj-$(CONFIG_BCM283X_PL011_SERIAL) += serial_bcm283x_pl011.o > obj-$(CONFIG_MSM_SERIAL) += serial_msm.o > +obj-$(CONFIG_MSM_GENI_SERIAL) += serial_msm_geni.o > obj-$(CONFIG_MVEBU_A3700_UART) += serial_mvebu_a3700.o > obj-$(CONFIG_MPC8XX_CONS) += serial_mpc8xx.o > obj-$(CONFIG_NULLDEV_SERIAL) += serial_nulldev.o > diff --git a/drivers/serial/serial_msm_geni.c b/drivers/serial/serial_msm_geni.c > new file mode 100644 > index 0000000000..c656d54cbb > --- /dev/null > +++ b/drivers/serial/serial_msm_geni.c > @@ -0,0 +1,603 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Qualcomm GENI serial engine UART driver > + * > + * (C) Copyright 2021 Dzmitry Sankouski > + * > + * Based on Linux driver. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define UART_OVERSAMPLING 32 > +#define STALE_TIMEOUT 160 > +#define SE_UART_RX_STALE_CNT 0x294 > +#define S_GENI_CMD_ABORT (BIT(1)) > + > +#define SE_GENI_S_CMD_CTRL_REG 0x634 > +#define SE_GENI_M_CMD_CTRL_REG 0x604 > + > +/* GENI_M_CMD_CTRL_REG */ > +#define M_GENI_CMD_CANCEL (BIT(2)) > +#define M_GENI_CMD_ABORT (BIT(1)) > +#define M_GENI_DISABLE (BIT(0)) > + > +/* GENI_S_CMD0 fields */ > +#define S_OPCODE_MSK (GENMASK(31, 27)) > +#define S_OPCODE_SHFT 27 > +#define S_PARAMS_MSK (GENMASK(26, 0)) > + > +/* GENI_STATUS fields */ > +#define M_GENI_CMD_ACTIVE (BIT(0)) > +#define S_GENI_CMD_ACTIVE (BIT(12)) > +#define S_CMD_DONE_EN (BIT(0)) > +#define M_CMD_DONE_EN (BIT(0)) > + > +#define USEC_PER_SEC 1000000L > + > +#define SE_GENI_STATUS 0x40 > +#define GENI_SER_M_CLK_CFG 0x48 > +#define GENI_SER_S_CLK_CFG 0x4C > +#define SE_GENI_M_CMD0 0x600 > +#define SE_GENI_M_IRQ_CLEAR 0x618 > +#define SE_GENI_S_IRQ_STATUS 0x640 > +#define SE_GENI_S_IRQ_CLEAR 0x648 > +#define SE_GENI_S_IRQ_EN 0x644 > +#define SE_GENI_M_IRQ_EN 0x614 > +#define SE_GENI_TX_FIFOn 0x700 > +#define SE_GENI_RX_FIFOn 0x780 > +#define SE_GENI_TX_FIFO_STATUS 0x800 > +#define SE_GENI_RX_FIFO_STATUS 0x804 > +#define SE_GENI_TX_WATERMARK_REG 0x80C > +#define M_TX_FIFO_WATERMARK_EN (BIT(30)) > +#define DEF_TX_WM 2 > +#define SE_UART_TX_TRANS_LEN 0x270 > +#define SE_GENI_TX_PACKING_CFG0 0x260 > +#define SE_GENI_TX_PACKING_CFG1 0x264 > +#define SE_GENI_RX_PACKING_CFG0 0x284 > +#define SE_GENI_RX_PACKING_CFG1 0x288 > +#define SE_UART_TX_STOP_BIT_LEN 0x26c > +#define SE_UART_TX_WORD_LEN 0x268 > +#define SE_UART_RX_WORD_LEN 0x28c > +#define SE_UART_RX_TRANS_CFG 0x280 > +#define SE_UART_RX_PARITY_CFG 0x2a8 > +#define SE_UART_TX_TRANS_CFG 0x25c > +#define SE_UART_TX_PARITY_CFG 0x2a4 > + > +#define GENI_FORCE_DEFAULT_REG 0x20 > +/* GENI_FORCE_DEFAULT_REG fields */ > +#define FORCE_DEFAULT (BIT(0)) > + > +#define S_CMD_ABORT_EN (BIT(5)) > + > +#define SE_GENI_S_CMD0 0x630 > +#define UART_START_READ 0x1 > + > +/* GENI_M_CMD_CTRL_REG */ > +#define M_GENI_CMD_CANCEL (BIT(2)) > +#define M_GENI_CMD_ABORT (BIT(1)) > +#define M_GENI_DISABLE (BIT(0)) > + > +#define M_CMD_ABORT_EN (BIT(5)) > + > +#define M_CMD_DONE_EN (BIT(0)) > +#define M_CMD_DONE_DISABLE_MASK (~M_CMD_DONE_EN) > +#define SE_GENI_M_IRQ_STATUS 0x610 > + > +#define M_OPCODE_SHIFT 27 > +#define S_OPCODE_SHIFT 27 > +#define M_TX_FIFO_WATERMARK_EN (BIT(30)) > +#define UART_START_TX 0x1 > +#define UART_CTS_MASK (BIT(1)) > +#define M_SEC_IRQ_EN (BIT(31)) > +#define TX_FIFO_WC_MSK (GENMASK(27, 0)) > +#define RX_FIFO_WC_MSK (GENMASK(24, 0)) > + > +#define S_RX_FIFO_WATERMARK_EN (BIT(26)) > +#define S_RX_FIFO_LAST_EN (BIT(27)) > +#define M_RX_FIFO_WATERMARK_EN (BIT(26)) > +#define M_RX_FIFO_LAST_EN (BIT(27)) > + > +/* GENI_SER_M_CLK_CFG/GENI_SER_S_CLK_CFG */ > +#define SER_CLK_EN (BIT(0)) > +#define CLK_DIV_MSK (GENMASK(15, 4)) > +#define CLK_DIV_SHFT 4 > + > +#define SE_HW_PARAM_0 0xE24 > +/* SE_HW_PARAM_0 fields */ > +#define TX_FIFO_WIDTH_MSK (GENMASK(29, 24)) > +#define TX_FIFO_WIDTH_SHFT 24 > +#define TX_FIFO_DEPTH_MSK (GENMASK(21, 16)) > +#define TX_FIFO_DEPTH_SHFT 16 > + > +DECLARE_GLOBAL_DATA_PTR; > + > +struct msm_serial_data { > + phys_addr_t base; > + u32 baud; > +}; > + > +static int get_clk_cfg(unsigned long clk_freq, unsigned long *ser_clk) function comment...what does it return? > +{ > + unsigned long root_freq[] = {7372800, 14745600, 19200000, 29491200, > + 32000000, 48000000, 64000000, 80000000, > + 96000000, 100000000}; > + int i; > + int match = -1; > + > + for (i = 0; i < ARRAY_SIZE(root_freq); i++) { > + if (clk_freq > root_freq[i]) > + continue; > + > + if (!(root_freq[i] % clk_freq)) { > + match = i; > + break; > + } > + } > + if (match != -1) > + *ser_clk = root_freq[match]; > + else > + pr_err("clk_freq %ld\n", clk_freq); > + return match; > +} > + > +static int get_clk_div_rate(u32 baud, u64 *desired_clk_rate) function comment > +{ > + unsigned long ser_clk; > + int dfs_index; > + int clk_div = 0; > + > + *desired_clk_rate = baud * UART_OVERSAMPLING; > + dfs_index = get_clk_cfg(*desired_clk_rate, &ser_clk); > + if (dfs_index < 0) { > + pr_err("%s: Can't find matching DFS entry for baud %d\n", > + __func__, baud); > + clk_div = -EINVAL; > + goto exit_get_clk_div_rate; > + } > + > + clk_div = ser_clk / *desired_clk_rate; > + *desired_clk_rate = ser_clk; > +exit_get_clk_div_rate: > + return clk_div; > +} > + > +static int geni_serial_set_clock_rate(struct udevice *dev, u64 rate) > +{ > + struct clk *clk; > + int ret; > + > + clk = devm_clk_get(dev, "se-clk"); > + if (!clk) > + return -EINVAL; > + > + ret = clk_set_rate(clk, rate); > + return ret; > +} > + > +/** > + * geni_se_get_tx_fifo_depth() - Get the TX fifo depth of the serial engine > + * @base: Pointer to the concerned serial engine. > + * > + * This function is used to get the depth i.e. number of elements in the > + * TX fifo of the serial engine. > + * > + * Return: TX fifo depth in units of FIFO words. > + */ > +static inline u32 geni_se_get_tx_fifo_depth(long base) > +{ > + u32 tx_fifo_depth; > + > + tx_fifo_depth = ((readl(base + SE_HW_PARAM_0) & TX_FIFO_DEPTH_MSK) >> > + TX_FIFO_DEPTH_SHFT); > + return tx_fifo_depth; > +} > + > +/** > + * geni_se_get_tx_fifo_width() - Get the TX fifo width of the serial engine > + * @base: Pointer to the concerned serial engine. > + * > + * This function is used to get the width i.e. word size per element in the > + * TX fifo of the serial engine. > + * > + * Return: TX fifo width in bits > + */ > +static inline u32 geni_se_get_tx_fifo_width(long base) > +{ > + u32 tx_fifo_width; > + > + tx_fifo_width = ((readl(base + SE_HW_PARAM_0) & TX_FIFO_WIDTH_MSK) >> > + TX_FIFO_WIDTH_SHFT); > + return tx_fifo_width; > +} > + > +static inline void geni_serial_baud(phys_addr_t base_address, u64 uclk, > + int baud) > +{ > + u32 clk_div; > + u32 s_clk_cfg = 0; > + > + clk_div = get_clk_div_rate(baud, &uclk); > + > + s_clk_cfg |= SER_CLK_EN; > + s_clk_cfg |= (clk_div << CLK_DIV_SHFT); > + > + writel(s_clk_cfg, base_address + GENI_SER_M_CLK_CFG); > + writel(s_clk_cfg, base_address + GENI_SER_S_CLK_CFG); > +} > + > +int msm_serial_setbrg(struct udevice *dev, int baud) > +{ > + struct msm_serial_data *priv = dev_get_priv(dev); > + > + priv->baud = baud; > + u32 clk_div; > + u64 clk_rate; > + > + clk_div = get_clk_div_rate(baud, &clk_rate); > + geni_serial_set_clock_rate(dev, clk_rate); > + geni_serial_baud(priv->base, clk_rate, baud); > + > + return 0; > +} > + > +static bool qcom_geni_serial_poll_bit(const struct udevice *dev, int offset, > + int field, bool set) function comment > +{ > + u32 reg; > + struct msm_serial_data *priv = dev_get_priv(dev); > + unsigned int baud; > + unsigned int tx_fifo_depth; > + unsigned int tx_fifo_width; > + unsigned int fifo_bits; > + unsigned long timeout_us = 10000; > + > + baud = 115200; > + > + if (priv) { > + baud = priv->baud; > + if (!baud) > + baud = 115200; > + tx_fifo_depth = geni_se_get_tx_fifo_depth(priv->base); > + tx_fifo_width = geni_se_get_tx_fifo_width(priv->base); > + fifo_bits = tx_fifo_depth * tx_fifo_width; > + /* > + * Total polling iterations based on FIFO worth of bytes to be > + * sent at current baud. Add a little fluff to the wait. > + */ > + timeout_us = ((fifo_bits * USEC_PER_SEC) / baud) + 500; > + } > + > + /* > + * Use custom implementation instead of readl_poll_atomic since ktimer > + * is not ready at the time of early console. This comment seems to relate only to Linux? > + */ > + timeout_us = DIV_ROUND_UP(timeout_us, 10) * 10; > + while (timeout_us) { > + reg = readl(priv->base + offset); > + if ((bool)(reg & field) == set) > + return true; > + udelay(10); > + timeout_us -= 10; > + } > + return false; > +} > + > +static void qcom_geni_serial_setup_tx(u64 base, u32 xmit_size) > +{ > + u32 m_cmd; > + > + writel(xmit_size, base + SE_UART_TX_TRANS_LEN); > + m_cmd = UART_START_TX << M_OPCODE_SHIFT; > + writel(m_cmd, base + SE_GENI_M_CMD0); > +} > + > +static inline void qcom_geni_serial_poll_tx_done(const struct udevice *dev) > +{ > + struct msm_serial_data *priv = dev_get_priv(dev); > + int done = 0; > + u32 irq_clear = M_CMD_DONE_EN; > + > + done = qcom_geni_serial_poll_bit(dev, SE_GENI_M_IRQ_STATUS, > + M_CMD_DONE_EN, true); > + if (!done) { > + writel(M_GENI_CMD_ABORT, priv->base + SE_GENI_M_CMD_CTRL_REG); > + irq_clear |= M_CMD_ABORT_EN; > + qcom_geni_serial_poll_bit(dev, SE_GENI_M_IRQ_STATUS, > + M_CMD_ABORT_EN, true); > + } > + writel(irq_clear, priv->base + SE_GENI_M_IRQ_CLEAR); > +} > + > +static u32 qcom_geni_serial_tx_empty(u64 base) > +{ > + return !readl(base + SE_GENI_TX_FIFO_STATUS); > +} > + > +/** > + * geni_se_setup_s_cmd() - Setup the secondary sequencer > + * @se: Pointer to the concerned serial engine. > + * @cmd: Command/Operation to setup in the secondary sequencer. > + * @params: Parameter for the sequencer command. > + * > + * This function is used to configure the secondary sequencer with the > + * command and its associated parameters. > + */ > +static inline void geni_se_setup_s_cmd(u64 base, u32 cmd, u32 params) > +{ > + u32 s_cmd; > + > + s_cmd = readl(base + SE_GENI_S_CMD0); > + s_cmd &= ~(S_OPCODE_MSK | S_PARAMS_MSK); > + s_cmd |= (cmd << S_OPCODE_SHFT); > + s_cmd |= (params & S_PARAMS_MSK); > + writel(s_cmd, base + SE_GENI_S_CMD0); > +} > + > +static void qcom_geni_serial_start_tx(u64 base) > +{ > + u32 irq_en; > + u32 status; > + > + status = readl(base + SE_GENI_STATUS); > + if (status & M_GENI_CMD_ACTIVE) > + return; > + > + if (!qcom_geni_serial_tx_empty(base)) > + return; > + > + irq_en = readl(base + SE_GENI_M_IRQ_EN); > + irq_en |= M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN; > + > + writel(DEF_TX_WM, base + SE_GENI_TX_WATERMARK_REG); > + writel(irq_en, base + SE_GENI_M_IRQ_EN); > +} > + > +static void qcom_geni_serial_start_rx(struct udevice *dev) > +{ > + u32 irq_en; > + u32 status; > + struct msm_serial_data *priv = dev_get_priv(dev); > + > + status = readl(priv->base + SE_GENI_STATUS); > + > + geni_se_setup_s_cmd(priv->base, UART_START_READ, 0); > + > + irq_en = readl(priv->base + SE_GENI_S_IRQ_EN); > + irq_en |= S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN; > + writel(irq_en, priv->base + SE_GENI_S_IRQ_EN); setbits_le32() ? > + > + irq_en = readl(priv->base + SE_GENI_M_IRQ_EN); > + irq_en |= M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN; > + writel(irq_en, priv->base + SE_GENI_M_IRQ_EN); here too > +} > + > +static void msm_geni_serial_setup_rx(struct udevice *dev) > +{ > + u32 irq_clear = S_CMD_DONE_EN; > + u32 geni_s_irq_en; > + u32 geni_m_irq_en; > + u32 cfg0; > + u32 cfg1; > + struct msm_serial_data *priv = dev_get_priv(dev); > + > + irq_clear |= S_CMD_ABORT_EN; > + > + writel(S_GENI_CMD_ABORT, priv->base + SE_GENI_S_CMD_CTRL_REG); > + qcom_geni_serial_poll_bit(dev, SE_GENI_S_CMD_CTRL_REG, S_GENI_CMD_ABORT, > + false); > + writel(irq_clear, priv->base + SE_GENI_S_IRQ_CLEAR); > + writel(FORCE_DEFAULT, priv->base + GENI_FORCE_DEFAULT_REG); > + > + cfg0 = 0xf; What is this? Can we have #define/enum/comment? > + cfg1 = 0x0; > + writel(cfg0, priv->base + SE_GENI_RX_PACKING_CFG0); > + writel(cfg1, priv->base + SE_GENI_RX_PACKING_CFG1); > + > + geni_se_setup_s_cmd(priv->base, UART_START_READ, 0); > + > + geni_s_irq_en = readl(priv->base + SE_GENI_S_IRQ_EN); > + geni_m_irq_en = readl(priv->base + SE_GENI_M_IRQ_EN); > + > + geni_s_irq_en |= S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN; > + geni_m_irq_en |= M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN; > + > + writel(geni_s_irq_en, priv->base + SE_GENI_S_IRQ_EN); > + writel(geni_m_irq_en, priv->base + SE_GENI_M_IRQ_EN); > +} > + > +static int msm_serial_putc(struct udevice *dev, const char ch) > +{ > + struct msm_serial_data *priv = dev_get_priv(dev); > + > + writel(DEF_TX_WM, priv->base + SE_GENI_TX_WATERMARK_REG); > + qcom_geni_serial_setup_tx(priv->base, 1); > + > + qcom_geni_serial_poll_bit(dev, SE_GENI_M_IRQ_STATUS, > + M_TX_FIFO_WATERMARK_EN, true); > + > + writel(ch, priv->base + SE_GENI_TX_FIFOn); > + writel(M_TX_FIFO_WATERMARK_EN, priv->base + SE_GENI_M_IRQ_CLEAR); > + > + qcom_geni_serial_poll_tx_done(dev); > + > + return 0; > +} > + > +static int msm_serial_getc(struct udevice *dev) > +{ > + struct msm_serial_data *priv = dev_get_priv(dev); > + u32 rx_fifo; > + u32 m_irq_status; > + u32 s_irq_status; > + > + writel(1 << S_OPCODE_SHIFT, priv->base + SE_GENI_S_CMD0); > + > + qcom_geni_serial_poll_bit(dev, SE_GENI_M_IRQ_STATUS, M_SEC_IRQ_EN, > + true); > + > + m_irq_status = readl(priv->base + SE_GENI_M_IRQ_STATUS); > + s_irq_status = readl(priv->base + SE_GENI_S_IRQ_STATUS); > + writel(m_irq_status, priv->base + SE_GENI_M_IRQ_CLEAR); > + writel(s_irq_status, priv->base + SE_GENI_S_IRQ_CLEAR); > + qcom_geni_serial_poll_bit(dev, SE_GENI_RX_FIFO_STATUS, RX_FIFO_WC_MSK, > + true); > + > + if (!readl(priv->base + SE_GENI_RX_FIFO_STATUS)) > + return 0; > + > + rx_fifo = readl(priv->base + SE_GENI_RX_FIFOn); > + return rx_fifo & 0xff; > +} > + > +static int msm_serial_pending(struct udevice *dev, bool input) > +{ > + struct msm_serial_data *priv = dev_get_priv(dev); > + > + if (input) > + return readl(priv->base + SE_GENI_RX_FIFO_STATUS) & > + RX_FIFO_WC_MSK; > + else > + return readl(priv->base + SE_GENI_TX_FIFO_STATUS) & > + TX_FIFO_WC_MSK; > + > + return 0; > +} > + > +static const struct dm_serial_ops msm_serial_ops = { > + .putc = msm_serial_putc, > + .pending = msm_serial_pending, > + .getc = msm_serial_getc, > + .setbrg = msm_serial_setbrg, > +}; > + > +static inline void geni_serial_init(phys_addr_t base_address) > +{ > + u32 tx_trans_cfg; > + u32 tx_parity_cfg = 0; /* Disable Tx Parity */ > + u32 rx_trans_cfg = 0; > + u32 rx_parity_cfg = 0; /* Disable Rx Parity */ > + u32 stop_bit_len = 0; /* Default stop bit length - 1 bit */ > + u32 bits_per_char; > + > + /* > + * Ignore Flow control. > + * n = 8. > + */ > + tx_trans_cfg = UART_CTS_MASK; > + bits_per_char = BITS_PER_BYTE; > + > + u32 cfg0 = 0xf; > + > + u32 cfg1 = 0x0; > + > + /* > + * Make an unconditional cancel on the main sequencer to reset > + * it else we could end up in data loss scenarios. > + */ > + writel(cfg0, base_address + SE_GENI_TX_PACKING_CFG0); > + writel(cfg1, base_address + SE_GENI_TX_PACKING_CFG1); > + writel(cfg0, base_address + SE_GENI_RX_PACKING_CFG0); > + writel(cfg1, base_address + SE_GENI_RX_PACKING_CFG1); > + > + writel(tx_trans_cfg, base_address + SE_UART_TX_TRANS_CFG); > + writel(tx_parity_cfg, base_address + SE_UART_TX_PARITY_CFG); > + writel(rx_trans_cfg, base_address + SE_UART_RX_TRANS_CFG); > + writel(rx_parity_cfg, base_address + SE_UART_RX_PARITY_CFG); > + writel(bits_per_char, base_address + SE_UART_TX_WORD_LEN); > + writel(bits_per_char, base_address + SE_UART_RX_WORD_LEN); > + writel(stop_bit_len, base_address + SE_UART_TX_STOP_BIT_LEN); > +} > + > +static int msm_serial_probe(struct udevice *dev) > +{ > + struct msm_serial_data *priv = dev_get_priv(dev); > + > + /* No need to reinitialize the UART after relocation */ > + if (gd->flags & GD_FLG_RELOC) > + return 0; > + > + geni_serial_init(priv->base); > + msm_geni_serial_setup_rx(dev); > + qcom_geni_serial_start_rx(dev); > + qcom_geni_serial_start_tx(priv->base); > + > + pinctrl_select_state(dev, "uart"); Does this not happen automatically with the correct pinctrl properties in the devicetree? > + > + return 0; > +} > + > +static int msm_serial_ofdata_to_platdata(struct udevice *dev) > +{ > + struct msm_serial_data *priv = dev_get_priv(dev); > + > + priv->base = dev_read_addr(dev); > + if (priv->base == FDT_ADDR_T_NONE) > + return -EINVAL; > + > + return 0; > +} > + > +static const struct udevice_id msm_serial_ids[] = { > + {.compatible = "qcom,msm-geni-uart"}, {}}; > + > +U_BOOT_DRIVER(serial_msm_geni) = { > + .name = "serial_msm_geni", > + .id = UCLASS_SERIAL, > + .of_match = msm_serial_ids, > + .of_to_plat = msm_serial_ofdata_to_platdata, > + .priv_auto = sizeof(struct msm_serial_data), > + .probe = msm_serial_probe, > + .ops = &msm_serial_ops, > +}; > + > +#ifdef CONFIG_DEBUG_UART_MSM_GENI > + > +static struct msm_serial_data init_serial_data = { > + .base = CONFIG_DEBUG_UART_BASE > +}; > + > +/* Serial dumb device, to reuse driver code */ > +static struct udevice init_dev = { > + .priv_ = &init_serial_data, > +}; > + > +#include > + > +static inline void _debug_uart_init(void) > +{ > + phys_addr_t base = CONFIG_DEBUG_UART_BASE; > + > + geni_serial_init(base); > + geni_serial_baud(base, CONFIG_DEBUG_UART_CLOCK, CONFIG_BAUDRATE); > + qcom_geni_serial_start_tx(base); > +} > + > +static inline void _debug_uart_putc(int ch) > +{ > + phys_addr_t base = CONFIG_DEBUG_UART_BASE; > + > + writel(DEF_TX_WM, base + SE_GENI_TX_WATERMARK_REG); > + qcom_geni_serial_setup_tx(base, 1); > + qcom_geni_serial_poll_bit(&init_dev, SE_GENI_M_IRQ_STATUS, > + M_TX_FIFO_WATERMARK_EN, true); > + > + writel(ch, base + SE_GENI_TX_FIFOn); > + writel(M_TX_FIFO_WATERMARK_EN, base + SE_GENI_M_IRQ_CLEAR); > + qcom_geni_serial_poll_tx_done(&init_dev); > +} > + > +DEBUG_UART_FUNCS > + > +#endif > -- > 2.20.1 > Regards, Simon