[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v1 2/2] xen/arm: Add MESON UART driver for Amlogic Meson SoCs
Hi Amit,
I will leave Andre to properly review the driver. But see the some comments
below.
On 26/01/2019 08:53, Amit Singh Tomar wrote:
This patch adds driver for UART controller present on Amlogic Meson
SoCs and it has been tested on Nanopi K2 board based on S905 SoC.
Controller registers defination is taken from Linux 4.20.
defination/definition/
Also, please add a pointer to the driver in Linux.
Signed-off-by: Amit Singh Tomar <amittomer25@xxxxxxxxx>
---
Changes since RFC:
* Removed S905 reference as other Amlogic SoCs
have this uart controller.
A link to the specification would have still been nice.
* Replaced meson_s905_read/write helper
with clrsetbit and friends helper.
* Followed proper UART reset sequence.
* List all UART compatible strings same as Linux
driver.
---
xen/drivers/char/Kconfig | 8 ++
xen/drivers/char/Makefile | 1 +
xen/drivers/char/meson-uart.c | 282 ++++++++++++++++++++++++++++++++++++++++++
As Jan mentioned in the previous version, you want to update MAINTAINERS to add
the file meson-uart.c under "ARM".
3 files changed, 291 insertions(+)
create mode 100644 xen/drivers/char/meson-uart.c
diff --git a/xen/drivers/char/Kconfig b/xen/drivers/char/Kconfig
index b1f07f8..d4add7f 100644
--- a/xen/drivers/char/Kconfig
+++ b/xen/drivers/char/Kconfig
@@ -20,6 +20,14 @@ config HAS_MVEBU
This selects the Marvell MVEBU UART. If you have a ARMADA 3700
based board, say Y.
+config HAS_MESON
+ bool "Amlogic MESON UART driver"
+ default y
+ depends on ARM_64
+ help
+ This selects the Amlogic MESON UART. If you have a Amlogic based
+ board, say Y.
The indentation still looks wrong to me. You want to use hard tab for Kconfig.
+
config HAS_PL011
bool "ARM PL011 UART driver"
default y
diff --git a/xen/drivers/char/Makefile b/xen/drivers/char/Makefile
index b68c330..7c646d7 100644
--- a/xen/drivers/char/Makefile
+++ b/xen/drivers/char/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_HAS_NS16550) += ns16550.o
obj-$(CONFIG_HAS_CADENCE_UART) += cadence-uart.o
obj-$(CONFIG_HAS_PL011) += pl011.o
obj-$(CONFIG_HAS_EXYNOS4210) += exynos4210-uart.o
+obj-$(CONFIG_HAS_MESON) += meson-uart.o
obj-$(CONFIG_HAS_MVEBU) += mvebu-uart.o
obj-$(CONFIG_HAS_OMAP) += omap-uart.o
obj-$(CONFIG_HAS_SCIF) += scif-uart.o
diff --git a/xen/drivers/char/meson-uart.c b/xen/drivers/char/meson-uart.c
new file mode 100644
index 0000000..990ae95
--- /dev/null
+++ b/xen/drivers/char/meson-uart.c
@@ -0,0 +1,282 @@
+/*
+ * xen/drivers/char/meson-uart.c
+ *
+ * Driver for Amlogic MESON UART
+ *
+ * Copyright (c) 2019, Amit Singh Tomar <amittomer25@xxxxxxxxx>.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms and conditions of the GNU General Public
+ * License, version 2, as published by the Free Software Foundation.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/irq.h>
+#include <xen/serial.h>
+#include <xen/vmap.h>
+#include <asm/io.h>
+
+/* Register offsets */
+#define AML_UART_WFIFO_REG 0x00
+#define AML_UART_RFIFO_REG 0x04
+#define AML_UART_CONTROL_REG 0x08
+#define AML_UART_STATUS_REG 0x0c
+#define AML_UART_MISC_REG 0x10
+
+/* UART_CONTROL bits */
+#define AML_UART_TX_RST BIT(22)
+#define AML_UART_RX_RST BIT(23)
+#define AML_UART_CLEAR_ERR BIT(24)
+#define AML_UART_RX_INT_EN BIT(27)
+#define AML_UART_TX_INT_EN BIT(28)
+
+/* UART_STATUS bits */
+#define AML_UART_RX_FIFO_EMPTY BIT(20)
+#define AML_UART_TX_FIFO_FULL BIT(21)
+#define AML_UART_TX_FIFO_EMPTY BIT(22)
+#define AML_UART_TX_CNT_MASK GENMASK(14, 8)
+
+/* AML_UART_MISC bits */
+#define AML_UART_XMIT_IRQ(c) (((c) & 0xff) << 8)
+#define AML_UART_RECV_IRQ(c) ((c) & 0xff)
+
+#define TX_FIFO_SIZE 64
+
+#define setbits(addr, set) writel((readl(addr) | (set)), (addr))
+#define clrbits(addr, clear) writel((readl(addr) & ~(clear)),
(addr))
+#define clrsetbits(addr, clear, set) writel(((readl(addr) & ~(clear)) \
+ | (set)), (addr))
I would prefer if you provide static inline helpers from *bits.
+
+static struct meson_uart {
+ unsigned int irq;
+ void __iomem *regs;
+ struct irqaction irqaction;
+ struct vuart_info vuart;
+} meson_com;
+
+static void meson_uart_interrupt(int irq, void *data,
+ struct cpu_user_regs *regs)
The indentation looks wrong here.
+{
+ struct serial_port *port = data;
+ struct meson_uart *uart = port->uart;
+ uint32_t st = readl(uart->regs + AML_UART_STATUS_REG);
+
+ if ( !(st & AML_UART_RX_FIFO_EMPTY) )
+ {
+ serial_rx_interrupt(port, regs);
+ }
The {} are not necessary.
+
+ if ( !(st & AML_UART_TX_FIFO_FULL) )
+ {
+ serial_tx_interrupt(port, regs);
+ }
Same here.
+}
+
+static void __init meson_uart_init_preirq(struct serial_port *port)
+{
+ struct meson_uart *uart = port->uart;
+
+ /* Reset UART */
+ setbits(uart->regs + AML_UART_CONTROL_REG,
+ (AML_UART_RX_RST | AML_UART_TX_RST | AML_UART_CLEAR_ERR));
+
+ clrbits(uart->regs + AML_UART_CONTROL_REG,
+ (AML_UART_RX_RST | AML_UART_TX_RST | AML_UART_CLEAR_ERR));
+
+ /* Disable Rx/Tx interrupts */
+ clrsetbits(uart->regs + AML_UART_CONTROL_REG,
+ (AML_UART_RX_INT_EN | AML_UART_TX_INT_EN), 0);
What is 0?
+}
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
