* [PATCH v6 2/2] tty/serial: Add Spreadtrum sc9836-uart driver support
From: Chunyan Zhang @ 2015-01-22 13:35 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: mark.rutland-5wv7dgnIgG8, arnd-r2nGTMty4D4,
gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io,
peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8,
shawn.guo-QSEj5FYQhm4dnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, jslaby-AlSwsSmVLrQ,
grant.likely-QSEj5FYQhm4dnm+yROfE0A, heiko-4mtYJXux2i+zQB+pC5nmwQ,
jason-NLaQJdtUoK4Be96aLqz0jA, florian.vaussard-p8DiymsW2f8,
andrew-g2DYL2Zd6BY, hytszk-Re5JQEeQqe8AvxtiuMwx3w,
antonynpavlov-Re5JQEeQqe8AvxtiuMwx3w,
orsonzhai-Re5JQEeQqe8AvxtiuMwx3w, geng.ren-lxIno14LUO0EEoCn2XhGlw,
zhizhou.zhang-lxIno14LUO0EEoCn2XhGlw,
lanqing.liu-lxIno14LUO0EEoCn2XhGlw,
zhang.lyra-Re5JQEeQqe8AvxtiuMwx3w,
wei.qiao-lxIno14LUO0EEoCn2XhGlw,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1421933706-4277-1-git-send-email-chunyan.zhang-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
Add a full sc9836-uart driver for SC9836 SoC which is based on the
spreadtrum sharkl64 platform.
This driver also support earlycon.
This patch also replaced the spaces between the macros and their
values with the tabs in serial_core.h
Originally-by: Lanqing Liu <lanqing.liu-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
Signed-off-by: Orson Zhai <orson.zhai-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
Signed-off-by: Chunyan Zhang <chunyan.zhang-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
---
drivers/tty/serial/Kconfig | 18 +
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/sprd_serial.c | 801 ++++++++++++++++++++++++++++++++++++++
include/uapi/linux/serial_core.h | 3 +
4 files changed, 823 insertions(+)
create mode 100644 drivers/tty/serial/sprd_serial.c
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index c79b43c..13211f7 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1577,6 +1577,24 @@ config SERIAL_MEN_Z135
This driver can also be build as a module. If so, the module will be called
men_z135_uart.ko
+config SERIAL_SPRD
+ tristate "Support for Spreadtrum serial"
+ depends on ARCH_SPRD
+ select SERIAL_CORE
+ help
+ This enables the driver for the Spreadtrum's serial.
+
+config SERIAL_SPRD_CONSOLE
+ bool "Spreadtrum UART console support"
+ depends on SERIAL_SPRD=y
+ select SERIAL_CORE_CONSOLE
+ select SERIAL_EARLYCON
+ help
+ Support for early debug console using Spreadtrum's serial. This enables
+ the console before standard serial driver is probed. This is enabled
+ with "earlycon" on the kernel command line. The console is
+ enabled when early_param is processed.
+
endmenu
config SERIAL_MCTRL_GPIO
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index 9a548ac..4801aca 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -93,6 +93,7 @@ obj-$(CONFIG_SERIAL_ARC) += arc_uart.o
obj-$(CONFIG_SERIAL_RP2) += rp2.o
obj-$(CONFIG_SERIAL_FSL_LPUART) += fsl_lpuart.o
obj-$(CONFIG_SERIAL_MEN_Z135) += men_z135_uart.o
+obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
# GPIOLIB helpers for modem control lines
obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.o
diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
new file mode 100644
index 0000000..fd98541
--- /dev/null
+++ b/drivers/tty/serial/sprd_serial.c
@@ -0,0 +1,801 @@
+/*
+ * Copyright (C) 2012-2015 Spreadtrum Communications Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ */
+
+#include <linux/clk.h>
+#include <linux/console.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/serial_core.h>
+#include <linux/serial.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+
+/* device name */
+#define UART_NR_MAX 8
+#define SPRD_TTY_NAME "ttyS"
+#define SPRD_FIFO_SIZE 128
+#define SPRD_DEF_RATE 26000000
+#define SPRD_TIMEOUT 2048
+
+/* the offset of serial registers and BITs for them */
+/* data registers */
+#define SPRD_TXD 0x0000
+#define SPRD_RXD 0x0004
+
+/* line status register and its BITs */
+#define SPRD_LSR 0x0008
+#define SPRD_LSR_OE BIT(4)
+#define SPRD_LSR_FE BIT(3)
+#define SPRD_LSR_PE BIT(2)
+#define SPRD_LSR_BI BIT(7)
+#define SPRD_LSR_TX_OVER BIT(15)
+
+/* data number in TX and RX fifo */
+#define SPRD_STS1 0x000C
+
+/* interrupt enable register and its BITs */
+#define SPRD_IEN 0x0010
+#define SPRD_IEN_RX_FULL BIT(0)
+#define SPRD_IEN_TX_EMPTY BIT(1)
+#define SPRD_IEN_BREAK_DETECT BIT(7)
+#define SPRD_IEN_TIMEOUT BIT(13)
+
+/* interrupt clear register */
+#define SPRD_ICLR 0x0014
+
+/* line control register */
+#define SPRD_LCR 0x0018
+#define SPRD_LCR_STOP_1BIT 0x10
+#define SPRD_LCR_STOP_2BIT 0x30
+#define SPRD_LCR_DATA_LEN (BIT(2) | BIT(3))
+#define SPRD_LCR_DATA_LEN5 0x0
+#define SPRD_LCR_DATA_LEN6 0x4
+#define SPRD_LCR_DATA_LEN7 0x8
+#define SPRD_LCR_DATA_LEN8 0xc
+#define SPRD_LCR_PARITY (BIT(0) | BIT(1))
+#define SPRD_LCR_PARITY_EN 0x2
+#define SPRD_LCR_EVEN_PAR 0x0
+#define SPRD_LCR_ODD_PAR 0x1
+
+/* control register 1 */
+#define SPRD_CTL1 0x001C
+#define RX_HW_FLOW_CTL_THLD BIT(6)
+#define RX_HW_FLOW_CTL_EN BIT(7)
+#define TX_HW_FLOW_CTL_EN BIT(8)
+
+/* fifo threshold register */
+#define SPRD_CTL2 0x0020
+#define THLD_TX_EMPTY 0x40
+#define THLD_RX_FULL 0x40
+
+/* config baud rate register */
+#define SPRD_CLKD0 0x0024
+#define SPRD_CLKD1 0x0028
+
+/* interrupt mask status register */
+#define SPRD_IMSR 0x002C
+#define SPRD_IMSR_RX_FIFO_FULL BIT(0)
+#define SPRD_IMSR_TX_FIFO_EMPTY BIT(1)
+#define SPRD_IMSR_BREAK_DETECT BIT(7)
+#define SPRD_IMSR_TIMEOUT BIT(13)
+
+struct reg_backup {
+ uint32_t ien;
+ uint32_t ctrl0;
+ uint32_t ctrl1;
+ uint32_t ctrl2;
+ uint32_t clkd0;
+ uint32_t clkd1;
+ uint32_t dspwait;
+};
+
+struct sprd_uart_port {
+ struct uart_port port;
+ struct reg_backup reg_bak;
+ char name[16];
+};
+
+static struct sprd_uart_port *sprd_port[UART_NR_MAX] = { NULL };
+
+static inline unsigned int serial_in(struct uart_port *port, int offset)
+{
+ return readl_relaxed(port->membase + offset);
+}
+
+static inline void serial_out(struct uart_port *port, int offset, int value)
+{
+ writel_relaxed(value, port->membase + offset);
+}
+
+static unsigned int sprd_tx_empty(struct uart_port *port)
+{
+ if (serial_in(port, SPRD_STS1) & 0xff00)
+ return 0;
+ else
+ return TIOCSER_TEMT;
+}
+
+static unsigned int sprd_get_mctrl(struct uart_port *port)
+{
+ return TIOCM_DSR | TIOCM_CTS;
+}
+
+static void sprd_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+ /* nothing to do */
+}
+
+static void sprd_stop_tx(struct uart_port *port)
+{
+ unsigned int ien, iclr;
+
+ iclr = serial_in(port, SPRD_ICLR);
+ ien = serial_in(port, SPRD_IEN);
+
+ iclr |= SPRD_IEN_TX_EMPTY;
+ ien &= ~SPRD_IEN_TX_EMPTY;
+
+ serial_out(port, SPRD_ICLR, iclr);
+ serial_out(port, SPRD_IEN, ien);
+}
+
+static void sprd_start_tx(struct uart_port *port)
+{
+ unsigned int ien;
+
+ ien = serial_in(port, SPRD_IEN);
+ if (!(ien & SPRD_IEN_TX_EMPTY)) {
+ ien |= SPRD_IEN_TX_EMPTY;
+ serial_out(port, SPRD_IEN, ien);
+ }
+}
+
+static void sprd_stop_rx(struct uart_port *port)
+{
+ unsigned int ien, iclr;
+
+ iclr = serial_in(port, SPRD_ICLR);
+ ien = serial_in(port, SPRD_IEN);
+
+ ien &= ~(SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT);
+ iclr |= SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT;
+
+ serial_out(port, SPRD_IEN, ien);
+ serial_out(port, SPRD_ICLR, iclr);
+}
+
+/* The Sprd serial does not support this function. */
+static void sprd_break_ctl(struct uart_port *port, int break_state)
+{
+ /* nothing to do */
+}
+
+static inline int handle_lsr_errors(struct uart_port *port,
+ unsigned int *flag,
+ unsigned int *lsr)
+{
+ int ret = 0;
+
+ /* statistics */
+ if (*lsr & SPRD_LSR_BI) {
+ *lsr &= ~(SPRD_LSR_FE | SPRD_LSR_PE);
+ port->icount.brk++;
+ ret = uart_handle_break(port);
+ if (ret)
+ return ret;
+ } else if (*lsr & SPRD_LSR_PE)
+ port->icount.parity++;
+ else if (*lsr & SPRD_LSR_FE)
+ port->icount.frame++;
+ if (*lsr & SPRD_LSR_OE)
+ port->icount.overrun++;
+
+ /* mask off conditions which should be ignored */
+ *lsr &= port->read_status_mask;
+ if (*lsr & SPRD_LSR_BI)
+ *flag = TTY_BREAK;
+ else if (*lsr & SPRD_LSR_PE)
+ *flag = TTY_PARITY;
+ else if (*lsr & SPRD_LSR_FE)
+ *flag = TTY_FRAME;
+
+ return ret;
+}
+
+static inline void sprd_rx(int irq, void *dev_id)
+{
+ struct uart_port *port = dev_id;
+ struct tty_port *tty = &port->state->port;
+ unsigned int ch, flag, lsr, max_count = SPRD_TIMEOUT;
+
+ while ((serial_in(port, SPRD_STS1) & 0x00ff) && max_count--) {
+ lsr = serial_in(port, SPRD_LSR);
+ ch = serial_in(port, SPRD_RXD);
+ flag = TTY_NORMAL;
+ port->icount.rx++;
+
+ if (lsr & (SPRD_LSR_BI | SPRD_LSR_PE
+ | SPRD_LSR_FE | SPRD_LSR_OE))
+ if (handle_lsr_errors(port, &lsr, &flag))
+ continue;
+ if (uart_handle_sysrq_char(port, ch))
+ continue;
+
+ uart_insert_char(port, lsr, SPRD_LSR_OE, ch, flag);
+ }
+
+ tty_flip_buffer_push(tty);
+}
+
+static inline void sprd_tx(int irq, void *dev_id)
+{
+ struct uart_port *port = dev_id;
+ struct circ_buf *xmit = &port->state->xmit;
+ int count;
+
+ if (port->x_char) {
+ serial_out(port, SPRD_TXD, port->x_char);
+ port->icount.tx++;
+ port->x_char = 0;
+ return;
+ }
+
+ if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
+ sprd_stop_tx(port);
+ return;
+ }
+
+ count = THLD_TX_EMPTY;
+ do {
+ serial_out(port, SPRD_TXD, xmit->buf[xmit->tail]);
+ xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
+ port->icount.tx++;
+ if (uart_circ_empty(xmit))
+ break;
+ } while (--count > 0);
+
+ if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+ uart_write_wakeup(port);
+
+ if (uart_circ_empty(xmit))
+ sprd_stop_tx(port);
+}
+
+/* this handles the interrupt from one port */
+static irqreturn_t sprd_handle_irq(int irq, void *dev_id)
+{
+ struct uart_port *port = (struct uart_port *)dev_id;
+ unsigned int ims;
+
+ spin_lock(&port->lock);
+
+ ims = serial_in(port, SPRD_IMSR);
+
+ if (!ims)
+ return IRQ_NONE;
+
+ serial_out(port, SPRD_ICLR, ~0);
+
+ if (ims & (SPRD_IMSR_RX_FIFO_FULL |
+ SPRD_IMSR_BREAK_DETECT | SPRD_IMSR_TIMEOUT))
+ sprd_rx(irq, port);
+
+ if (ims & SPRD_IMSR_TX_FIFO_EMPTY)
+ sprd_tx(irq, port);
+
+ spin_unlock(&port->lock);
+
+ return IRQ_HANDLED;
+}
+
+static int sprd_startup(struct uart_port *port)
+{
+ int ret = 0;
+ unsigned int ien, ctrl1;
+ unsigned int timeout;
+ struct sprd_uart_port *sp;
+
+ serial_out(port, SPRD_CTL2, ((THLD_TX_EMPTY << 8) | THLD_RX_FULL));
+
+ /* clear rx fifo */
+ timeout = SPRD_TIMEOUT;
+ while (timeout-- && serial_in(port, SPRD_STS1) & 0x00ff)
+ serial_in(port, SPRD_RXD);
+
+ /* clear tx fifo */
+ timeout = SPRD_TIMEOUT;
+ while (timeout-- && serial_in(port, SPRD_STS1) & 0xff00)
+ cpu_relax();
+
+ /* clear interrupt */
+ serial_out(port, SPRD_IEN, 0x0);
+ serial_out(port, SPRD_ICLR, ~0);
+
+ /* allocate irq */
+ sp = container_of(port, struct sprd_uart_port, port);
+ snprintf(sp->name, sizeof(sp->name), "sprd_serial%d", port->line);
+ ret = devm_request_irq(port->dev, port->irq, sprd_handle_irq,
+ IRQF_SHARED, sp->name, port);
+ if (ret) {
+ dev_err(port->dev, "fail to request serial irq %d, ret=%d\n",
+ port->irq, ret);
+ return ret;
+ }
+ ctrl1 = serial_in(port, SPRD_CTL1);
+ ctrl1 |= 0x3e00 | THLD_RX_FULL;
+ serial_out(port, SPRD_CTL1, ctrl1);
+
+ /* enable interrupt */
+ spin_lock(&port->lock);
+ ien = serial_in(port, SPRD_IEN);
+ ien |= SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT | SPRD_IEN_TIMEOUT;
+ serial_out(port, SPRD_IEN, ien);
+ spin_unlock(&port->lock);
+
+ return 0;
+}
+
+static void sprd_shutdown(struct uart_port *port)
+{
+ serial_out(port, SPRD_IEN, 0x0);
+ serial_out(port, SPRD_ICLR, ~0);
+ devm_free_irq(port->dev, port->irq, port);
+}
+
+static void sprd_set_termios(struct uart_port *port,
+ struct ktermios *termios,
+ struct ktermios *old)
+{
+ unsigned int baud, quot;
+ unsigned int lcr, fc;
+ unsigned long flags;
+
+ /* ask the core to calculate the divisor for us */
+ baud = uart_get_baud_rate(port, termios, old, 1200, 3000000);
+
+ quot = (unsigned int)((port->uartclk + baud / 2) / baud);
+
+ /* set data length */
+ lcr = serial_in(port, SPRD_LCR);
+ lcr &= ~SPRD_LCR_DATA_LEN;
+ switch (termios->c_cflag & CSIZE) {
+ case CS5:
+ lcr |= SPRD_LCR_DATA_LEN5;
+ break;
+ case CS6:
+ lcr |= SPRD_LCR_DATA_LEN6;
+ break;
+ case CS7:
+ lcr |= SPRD_LCR_DATA_LEN7;
+ break;
+ case CS8:
+ default:
+ lcr |= SPRD_LCR_DATA_LEN8;
+ break;
+ }
+
+ /* calculate stop bits */
+ lcr &= ~(SPRD_LCR_STOP_1BIT | SPRD_LCR_STOP_2BIT);
+ if (termios->c_cflag & CSTOPB)
+ lcr |= SPRD_LCR_STOP_2BIT;
+ else
+ lcr |= SPRD_LCR_STOP_1BIT;
+
+ /* calculate parity */
+ lcr &= ~SPRD_LCR_PARITY;
+ termios->c_cflag &= ~CMSPAR; /* no support mark/space */
+ if (termios->c_cflag & PARENB) {
+ lcr |= SPRD_LCR_PARITY_EN;
+ if (termios->c_cflag & PARODD)
+ lcr |= SPRD_LCR_ODD_PAR;
+ else
+ lcr |= SPRD_LCR_EVEN_PAR;
+ }
+
+ spin_lock_irqsave(&port->lock, flags);
+
+ /* update the per-port timeout */
+ uart_update_timeout(port, termios->c_cflag, baud);
+
+ port->read_status_mask = SPRD_LSR_OE;
+ if (termios->c_iflag & INPCK)
+ port->read_status_mask |= SPRD_LSR_FE | SPRD_LSR_PE;
+ if (termios->c_iflag & (BRKINT | PARMRK))
+ port->read_status_mask |= SPRD_LSR_BI;
+
+ /* characters to ignore */
+ port->ignore_status_mask = 0;
+ if (termios->c_iflag & IGNPAR)
+ port->ignore_status_mask |= SPRD_LSR_PE | SPRD_LSR_FE;
+ if (termios->c_iflag & IGNBRK) {
+ port->ignore_status_mask |= SPRD_LSR_BI;
+ /*
+ * If we're ignoring parity and break indicators,
+ * ignore overruns too (for real raw support).
+ */
+ if (termios->c_iflag & IGNPAR)
+ port->ignore_status_mask |= SPRD_LSR_OE;
+ }
+
+ /* flow control */
+ fc = serial_in(port, SPRD_CTL1);
+ fc &= ~(RX_HW_FLOW_CTL_THLD | RX_HW_FLOW_CTL_EN | TX_HW_FLOW_CTL_EN);
+ if (termios->c_cflag & CRTSCTS) {
+ fc |= RX_HW_FLOW_CTL_THLD;
+ fc |= RX_HW_FLOW_CTL_EN;
+ fc |= TX_HW_FLOW_CTL_EN;
+ }
+
+ /* clock divider bit0~bit15 */
+ serial_out(port, SPRD_CLKD0, quot & 0xffff);
+
+ /* clock divider bit16~bit20 */
+ serial_out(port, SPRD_CLKD1, (quot & 0x1f0000) >> 16);
+ serial_out(port, SPRD_LCR, lcr);
+ fc |= 0x3e00 | THLD_RX_FULL;
+ serial_out(port, SPRD_CTL1, fc);
+
+ spin_unlock_irqrestore(&port->lock, flags);
+
+ /* Don't rewrite B0 */
+ if (tty_termios_baud_rate(termios))
+ tty_termios_encode_baud_rate(termios, baud, baud);
+}
+
+static const char *sprd_type(struct uart_port *port)
+{
+ return "SPX";
+}
+
+static void sprd_release_port(struct uart_port *port)
+{
+ /* nothing to do */
+}
+
+static int sprd_request_port(struct uart_port *port)
+{
+ return 0;
+}
+
+static void sprd_config_port(struct uart_port *port, int flags)
+{
+ if (flags & UART_CONFIG_TYPE)
+ port->type = PORT_SPRD;
+}
+
+static int sprd_verify_port(struct uart_port *port,
+ struct serial_struct *ser)
+{
+ if (ser->type != PORT_SPRD)
+ return -EINVAL;
+ if (port->irq != ser->irq)
+ return -EINVAL;
+ return 0;
+}
+
+static struct uart_ops serial_sprd_ops = {
+ .tx_empty = sprd_tx_empty,
+ .get_mctrl = sprd_get_mctrl,
+ .set_mctrl = sprd_set_mctrl,
+ .stop_tx = sprd_stop_tx,
+ .start_tx = sprd_start_tx,
+ .stop_rx = sprd_stop_rx,
+ .break_ctl = sprd_break_ctl,
+ .startup = sprd_startup,
+ .shutdown = sprd_shutdown,
+ .set_termios = sprd_set_termios,
+ .type = sprd_type,
+ .release_port = sprd_release_port,
+ .request_port = sprd_request_port,
+ .config_port = sprd_config_port,
+ .verify_port = sprd_verify_port,
+};
+
+#ifdef CONFIG_SERIAL_SPRD_CONSOLE
+static inline void wait_for_xmitr(struct uart_port *port)
+{
+ unsigned int status, tmout = 10000;
+
+ /* wait up to 10ms for the character(s) to be sent */
+ do {
+ status = serial_in(port, SPRD_STS1);
+ if (--tmout == 0)
+ break;
+ udelay(1);
+ } while (status & 0xff00);
+}
+
+static void sprd_console_putchar(struct uart_port *port, int ch)
+{
+ wait_for_xmitr(port);
+ serial_out(port, SPRD_TXD, ch);
+}
+
+static void sprd_console_write(struct console *co, const char *s,
+ unsigned int count)
+{
+ struct uart_port *port = &sprd_port[co->index]->port;
+ int locked = 1;
+ unsigned long flags;
+
+ if (oops_in_progress)
+ locked = spin_trylock(&port->lock);
+ else
+ spin_lock_irqsave(&port->lock, flags);
+
+ uart_console_write(port, s, count, sprd_console_putchar);
+
+ /* wait for transmitter to become empty */
+ wait_for_xmitr(port);
+
+ if (locked)
+ spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static int __init sprd_console_setup(struct console *co, char *options)
+{
+ struct uart_port *port;
+ int baud = 115200;
+ int bits = 8;
+ int parity = 'n';
+ int flow = 'n';
+
+ if (co->index >= UART_NR_MAX || co->index < 0)
+ co->index = 0;
+
+ port = &sprd_port[co->index]->port;
+ if (port == NULL) {
+ pr_info("serial port %d not yet initialized\n", co->index);
+ return -ENODEV;
+ }
+ if (options)
+ uart_parse_options(options, &baud, &parity, &bits, &flow);
+
+ return uart_set_options(port, co, baud, parity, bits, flow);
+}
+
+static struct uart_driver sprd_uart_driver;
+static struct console sprd_console = {
+ .name = SPRD_TTY_NAME,
+ .write = sprd_console_write,
+ .device = uart_console_device,
+ .setup = sprd_console_setup,
+ .flags = CON_PRINTBUFFER,
+ .index = -1,
+ .data = &sprd_uart_driver,
+};
+
+#define SPRD_CONSOLE (&sprd_console)
+
+/* Support for earlycon */
+static void sprd_putc(struct uart_port *port, int c)
+{
+ unsigned int timeout = SPRD_TIMEOUT;
+
+ while (timeout-- &&
+ !(readl(port->membase + SPRD_LSR) & SPRD_LSR_TX_OVER))
+ cpu_relax();
+
+ writeb(c, port->membase + SPRD_TXD);
+}
+
+static void sprd_early_write(struct console *con, const char *s,
+ unsigned n)
+{
+ struct earlycon_device *dev = con->data;
+
+ uart_console_write(&dev->port, s, n, sprd_putc);
+}
+
+static int __init sprd_early_console_setup(
+ struct earlycon_device *device,
+ const char *opt)
+{
+ if (!device->port.membase)
+ return -ENODEV;
+
+ device->con->write = sprd_early_write;
+ return 0;
+}
+
+EARLYCON_DECLARE(sprd_serial, sprd_early_console_setup);
+OF_EARLYCON_DECLARE(sprd_serial, "sprd,sc9836-uart",
+ sprd_early_console_setup);
+
+#else /* !CONFIG_SERIAL_SPRD_CONSOLE */
+#define SPRD_CONSOLE NULL
+#endif
+
+static struct uart_driver sprd_uart_driver = {
+ .owner = THIS_MODULE,
+ .driver_name = "sprd_serial",
+ .dev_name = SPRD_TTY_NAME,
+ .major = 0,
+ .minor = 0,
+ .nr = UART_NR_MAX,
+ .cons = SPRD_CONSOLE,
+};
+
+static int sprd_probe_dt_alias(int index, struct device *dev)
+{
+ struct device_node *np;
+ static bool seen_dev_with_alias;
+ static bool seen_dev_without_alias;
+ int ret = index;
+
+ if (!IS_ENABLED(CONFIG_OF))
+ return ret;
+
+ np = dev->of_node;
+ if (!np)
+ return ret;
+
+ ret = of_alias_get_id(np, "serial");
+ if (IS_ERR_VALUE(ret)) {
+ seen_dev_without_alias = true;
+ ret = index;
+ } else {
+ seen_dev_with_alias = true;
+ if (ret >= ARRAY_SIZE(sprd_port) || sprd_port[ret] != NULL) {
+ dev_warn(dev, "requested serial port %d not available.\n", ret);
+ ret = index;
+ }
+ }
+
+ if (seen_dev_with_alias && seen_dev_without_alias)
+ dev_warn(dev, "aliased and non-aliased serial devices found in device tree. Serial port enumeration may be unpredictable.\n");
+
+ return ret;
+}
+
+static int sprd_remove(struct platform_device *dev)
+{
+ struct sprd_uart_port *sup = platform_get_drvdata(dev);
+ bool busy = false;
+ int i;
+
+ if (sup)
+ uart_remove_one_port(&sprd_uart_driver, &sup->port);
+
+ for (i = 0; i < ARRAY_SIZE(sprd_port); i++)
+ if (sprd_port[i] == sup)
+ sprd_port[i] = NULL;
+ else if (sprd_port[i])
+ busy = true;
+
+ if (!busy)
+ uart_unregister_driver(&sprd_uart_driver);
+
+ return 0;
+}
+
+static int sprd_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ struct uart_port *up;
+ struct clk *clk;
+ int irq;
+ int index;
+ int ret;
+
+ for (index = 0; index < ARRAY_SIZE(sprd_port); index++)
+ if (sprd_port[index] == NULL)
+ break;
+
+ if (index == ARRAY_SIZE(sprd_port))
+ return -EBUSY;
+
+ index = sprd_probe_dt_alias(index, &pdev->dev);
+
+ sprd_port[index] = devm_kzalloc(&pdev->dev,
+ sizeof(*sprd_port[index]), GFP_KERNEL);
+ if (!sprd_port[index])
+ return -ENOMEM;
+
+ up = &sprd_port[index]->port;
+ up->dev = &pdev->dev;
+ up->line = index;
+ up->type = PORT_SPRD;
+ up->iotype = SERIAL_IO_PORT;
+ up->uartclk = SPRD_DEF_RATE;
+ up->fifosize = SPRD_FIFO_SIZE;
+ up->ops = &serial_sprd_ops;
+ up->flags = ASYNC_BOOT_AUTOCONF;
+
+ clk = devm_clk_get(&pdev->dev, NULL);
+ if (!IS_ERR(clk))
+ up->uartclk = clk_get_rate(clk);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "not provide mem resource\n");
+ return -ENODEV;
+ }
+ up->mapbase = res->start;
+ up->membase = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(up->membase))
+ return PTR_ERR(up->membase);
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "not provide irq resource\n");
+ return -ENODEV;
+ }
+ up->irq = irq;
+
+ if (!sprd_uart_driver.state) {
+ ret = uart_register_driver(&sprd_uart_driver);
+ if (ret < 0) {
+ pr_err("Failed to register SPRD-UART driver\n");
+ return ret;
+ }
+ }
+
+ ret = uart_add_one_port(&sprd_uart_driver, up);
+ if (ret) {
+ sprd_port[index] = NULL;
+ sprd_remove(pdev);
+ }
+
+ platform_set_drvdata(pdev, up);
+
+ return ret;
+}
+
+static int sprd_suspend(struct device *dev)
+{
+ int id = to_platform_device(dev)->id;
+ struct uart_port *port = &sprd_port[id]->port;
+
+ uart_suspend_port(&sprd_uart_driver, port);
+
+ return 0;
+}
+
+static int sprd_resume(struct device *dev)
+{
+ int id = to_platform_device(dev)->id;
+ struct uart_port *port = &sprd_port[id]->port;
+
+ uart_resume_port(&sprd_uart_driver, port);
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(sprd_pm_ops, sprd_suspend, sprd_resume);
+
+static const struct of_device_id serial_ids[] = {
+ {.compatible = "sprd,sc9836-uart",},
+ {}
+};
+
+static struct platform_driver sprd_platform_driver = {
+ .probe = sprd_probe,
+ .remove = sprd_remove,
+ .driver = {
+ .name = "sprd_serial",
+ .of_match_table = of_match_ptr(serial_ids),
+ .pm = &sprd_pm_ops,
+ },
+};
+
+module_platform_driver(sprd_platform_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Spreadtrum SoC serial driver series");
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index c172180..7e6eb39 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -248,4 +248,7 @@
/* MESON */
#define PORT_MESON 109
+/* SPRD SERIAL */
+#define PORT_SPRD 110
+
#endif /* _UAPILINUX_SERIAL_CORE_H */
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 01/13] kdbus: add documentation
From: David Herrmann @ 2015-01-22 13:46 UTC (permalink / raw)
To: Michael Kerrisk (man-pages)
Cc: Daniel Mack, Greg Kroah-Hartman, Arnd Bergmann, Eric W. Biederman,
One Thousand Gnomes, Tom Gundersen, Jiri Kosina, Andy Lutomirski,
Linux API, linux-kernel, Djalal Harouni, Johannes Stezenbach,
Theodore T'so
In-Reply-To: <54C0CE8A.5080805-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi Michael
On Thu, Jan 22, 2015 at 11:18 AM, Michael Kerrisk (man-pages)
<mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 01/21/2015 05:58 PM, Daniel Mack wrote:
>>>> Also, the context the kdbus commands operate on originate from a
>>>> mountable special-purpose file system.
>>>
>>> It's not clear to me how this point implies any particular API design
>>> choice.
>>
>> It emphasizes the fact that our ioctl API can only be used with the
>> nodes exposed by kdbusfs, and vice versa. I think operations on
>> driver-specific files do not justify a new 'generic' syscall API.
>
> I see your (repeated) use of words like "driver" as just a distraction,
> I'm sorry. "Driver-specific files" is an implementation detail. The
> point is that kdbus provides a complex, general-purpose feature for all
> of userland. It is core infrastructure that will be used by key pieces
> of the plumbing layer, and likely by many other applications as well.
> *Please* stop suggesting that it is not core infrastructure: kdbus
> has the potential to be a great IPC that will be very useful to many,
> many user-space developers.
We called it an 'ipc driver' so far. It is in no way meant as
distraction. Feel free to call it 'core infrastructure'. I think we
can agree that we want it to be generically useful, like other ipc
mechanisms, including UDS and netlink.
> (By the way, we have precedents for device/filesystem-specific system
> calls. Even a recent one, in memfd_create().)
memfd_create() is in no way file-system specific. Internally, it uses
shmem, but that's an implementation detail. The API does not expose
this in any way. If you were referring to sealing, it's implemented as
fcntl(), not as a separate syscall. Furthermore, sealing is only
limited to shmem as it's the only volatile storage. It's not an API
restriction. Other volatile file-systems are free to implement
sealing.
>>> ioctl() is a get-out-of-jail free card when it comes to API design.
>>
>> And how are syscalls different in that regard when they would both
>> transport the same data structures?
>
> My general impression is that when it comes to system calls,
> there's usually a lot more up front thought about API design.
This is no technical reason why to use syscalls over ioctls. Imho,
it's rather a reason to improve the kernel's ioctl-review process.
>> Also note that all kdbus ioctls
>> necessarily operate on a file descriptor context, which an ioctl passes
>> along by default.
>
> I fail to see your point here. The same statement applies to multiple
> special-purpose system calls (epoll, inotify, various shared memory APIs,
> and so on).
epoll and inotify don't have a 'parent' object living in the
file-system. They *need* an entry-point. We can use open() for that.
You're right, from a technical viewpoint, there's no restriction.
There're examples for both (eg., see Solaris /dev/poll, as
ioctl()-based 'epoll').
>>> Rather
>>> than thinking carefully and long about a set of coherent, stable APIs that
>>> provide some degree of type-safety to user-space, one just adds/changes/removes
>>> an ioctl.
>>
>> Adding another ioctl in the future for cases we didn't think about
>> earlier would of course be considered a workaround; and even though such
>> things happen all the time, it's certainly something we'd like to avoid.
>>
>> However, we would also need to add another syscall in such cases, which
>> is even worse IMO. So that's really not an argument against ioctls after
>> all. What fundamental difference between a raw syscall and a ioctl,
>> especially with regards to type safety, is there which would help us here?
>
> Type safety helps user-space application developers. System calls have
> it, ioctl() does not.
This is simply not true. There is no type-safety in:
syscall(__NR_xyz, some, random, arguments)
The only way a syscall gets 'type-safe', is to provide a wrapper
function. Same applies to ioctls. But people tend to not do that for
ioctls, which is, again, not a technical argument against ioctls. It's
a matter of psychology, though.
I still don't see a technical reason to use syscalls. API proposals welcome!
We're now working on a small kdbus helper library, which provides
type-safe ioctl wrappers, item-iterators and documented examples. But,
like syscalls, nobody is forced to use the wrappers. The API design is
not affected by this.
>>> And that process seems to be frequent and ongoing even now. (And
>>> it's to your great credit that the API/ABI breaks are clearly and honestly
>>> marked in the kdbus.h changelog.) All of this lightens the burden of API
>>> design for kernel developers, but I'm concerned that the long-term pain
>>> for user-space developers who use an API which (in my estimation) may
>>> come to be widely used will be enormous.
>>
>> Yes, we've jointly reviewed the API details again until just recently to
>> unify structs and enums etc, and added fields to make the ioctls structs
>> more versatile for possible future additions. By that, we effectively
>> broke the ABI, but we did that because we know we can't do such things
>> again in the future.
>>
>> But again - I don't see how this would be different when using syscalls
>> rather than ioctls to transport information between the driver and
>> userspace. Could you elaborate?
>
> My suspicion is that not nearly enough thinking has yet been done about
> the design of the API. That's based on these observations:
>
> * Documentation that is, considering the size of the API, *way* too thin.
Ok, working on that.
> * Some parts of the API not documented at all (various kdbus_item blobs)
All public structures have documentation in kdbus.h. It may need
improvements, though.
> * ABI changes happening even quite recently
Please elaborate why 'recent ABI-changes' are a sign of a premature API.
I seriously doubt any API can be called 'perfect'. On the contrary, I
believe that all APIs could be improved continuously. The fact that
we, at one point, settle on an API is an admission of
backwards-compatibility. I in no way think it's a sign of
'perfection'.
With kdbus our plan is to give API-guarantees starting with upstream
inclusion. We know, that our API will not be perfect, none is. But we
will try hard to fix anything that comes up, as long as we can. And
this effort will manifest in ABI-breaks.
> * API oddities such as the 'kernel_flags' fields. Why do I need to
> be told what flags the kernel supports on *every* operation?
If we only returned EINVAL on invalid arguments, user-space had to
probe for each flag to see whether it's supported. By returning the
set of supported flags, user-space can cache those and _reliably_ know
which flags are supported.
We decided the overhead of a single u64 copy on each ioctl is
preferred over a separate syscall/ioctl to query kernel flags. If you
disagree, please elaborate (preferably with a suggestion how to do it
better).
> The above is just after a day of looking hard at kdbus.txt. I strongly
> suspect I'd find a lot of other issues if I spent more time on kdbus.
If you find the time, please do! Any hints how a specific part of the
API could be done better, is highly appreciated. A lot of the more or
less recent changes were done due to reviews from glib developers.
More help is always welcome!
Thanks
David
^ permalink raw reply
* Re: [PATCH 01/13] kdbus: add documentation
From: Austin S Hemmelgarn @ 2015-01-22 14:49 UTC (permalink / raw)
To: David Herrmann, Michael Kerrisk (man-pages)
Cc: Daniel Mack, Greg Kroah-Hartman, Arnd Bergmann, Eric W. Biederman,
One Thousand Gnomes, Tom Gundersen, Jiri Kosina, Andy Lutomirski,
Linux API, linux-kernel, Djalal Harouni, Johannes Stezenbach,
Theodore T'so
In-Reply-To: <CANq1E4S=3qNw599L85uj-8aXwxeV+mcurB_Nu_rHH8opAeePjw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1515 bytes --]
On 2015-01-22 08:46, David Herrmann wrote:
> Hi Michael
>
> On Thu, Jan 22, 2015 at 11:18 AM, Michael Kerrisk (man-pages)
> <mtk.manpages@gmail.com> wrote:
>> * API oddities such as the 'kernel_flags' fields. Why do I need to
>> be told what flags the kernel supports on *every* operation?
>
> If we only returned EINVAL on invalid arguments, user-space had to
> probe for each flag to see whether it's supported. By returning the
> set of supported flags, user-space can cache those and _reliably_ know
> which flags are supported.
> We decided the overhead of a single u64 copy on each ioctl is
> preferred over a separate syscall/ioctl to query kernel flags. If you
> disagree, please elaborate (preferably with a suggestion how to do it
> better).
>
While I agree that there should be a way for userspace to get the list
of supported operations, userspace apps will only actually care about
that once, when they begin talking to kdbus, because (ignoring the live
kernel patching that people have been working on recently) the list of
supported operations isn't going to change while the system is running.
While a u64 copy has relatively low overhead, it does have overhead,
and that is very significant when you consider part of the reason some
people want kdbus is for the performance gain. Especially for those
automotive applications that have been mentioned which fire off
thousands of messages during start-up, every little bit of performance
is significant.
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 2455 bytes --]
^ permalink raw reply
* Re: [v8 4/5] ext4: adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface support
From: Konstantin Khlebnikov @ 2015-01-22 15:20 UTC (permalink / raw)
To: Dave Chinner, Li Xi
Cc: linux-fsdevel, linux-ext4, linux-api, tytso, adilger, jack, viro,
hch, dmonakhov
In-Reply-To: <20141209225758.GB9756@dastard>
On 10.12.2014 01:57, Dave Chinner wrote:
> On Tue, Dec 09, 2014 at 01:22:27PM +0800, Li Xi wrote:
>> This patch adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR ioctl interface
>> support for ext4. The interface is kept consistent with
>> XFS_IOC_FSGETXATTR/XFS_IOC_FSGETXATTR.
>>
>> Signed-off-by: Li Xi <lixi@ddn.com>
>> ---
>> fs/ext4/ext4.h | 111 ++++++++++++++++
>> fs/ext4/ioctl.c | 330 +++++++++++++++++++++++++++++++++--------------
>> fs/xfs/xfs_fs.h | 47 +++-----
>> include/uapi/linux/fs.h | 58 ++++++++
>> 4 files changed, 418 insertions(+), 128 deletions(-)
>>
>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>> index 136e18c..43a2a88 100644
>> --- a/fs/ext4/ext4.h
>> +++ b/fs/ext4/ext4.h
>> @@ -384,6 +384,115 @@ struct flex_groups {
>> #define EXT4_FL_USER_VISIBLE 0x204BDFFF /* User visible flags */
>> #define EXT4_FL_USER_MODIFIABLE 0x204380FF /* User modifiable flags */
>>
>> +/* Transfer internal flags to xflags */
>> +static inline __u32 ext4_iflags_to_xflags(unsigned long iflags)
>> +{
>> + __u32 xflags = 0;
>> +
>> + if (iflags & EXT4_SECRM_FL)
>> + xflags |= FS_XFLAG_SECRM;
>> + if (iflags & EXT4_UNRM_FL)
>> + xflags |= FS_XFLAG_UNRM;
>> + if (iflags & EXT4_COMPR_FL)
>> + xflags |= FS_XFLAG_COMPR;
> ....
>
> NACK.
>
>> + if (iflags & EXT4_SYNC_FL)
>> + xflags |= FS_XFLAG_SYNC;
>> + if (iflags & EXT4_IMMUTABLE_FL)
>> + xflags |= FS_XFLAG_IMMUTABLE;
>> + if (iflags & EXT4_APPEND_FL)
>> + xflags |= FS_XFLAG_APPEND;
>> + if (iflags & EXT4_NODUMP_FL)
>> + xflags |= FS_XFLAG_NODUMP;
>> + if (iflags & EXT4_NOATIME_FL)
>> + xflags |= FS_XFLAG_NOATIME;
>
> These are OK because they already exist in the interface, but all
> the ext4 specific flags you've added have no place in this patchset.
>
>
>> +
>> /* Flags that should be inherited by new inodes from their parent. */
>> #define EXT4_FL_INHERITED (EXT4_SECRM_FL | EXT4_UNRM_FL | EXT4_COMPR_FL |\
>> EXT4_SYNC_FL | EXT4_NODUMP_FL | EXT4_NOATIME_FL |\
>> @@ -606,6 +715,8 @@ enum {
>> #define EXT4_IOC_RESIZE_FS _IOW('f', 16, __u64)
>> #define EXT4_IOC_SWAP_BOOT _IO('f', 17)
>> #define EXT4_IOC_PRECACHE_EXTENTS _IO('f', 18)
>> +#define EXT4_IOC_FSGETXATTR FS_IOC_FSGETXATTR
>> +#define EXT4_IOC_FSSETXATTR FS_IOC_FSSETXATTR
>
> NACK. Userspace only needs FS_IOC_FS[GS]ETXATTR so that it works across
> all filesystems. We need to retain XFS_IOC_FS[GS]ETXATTR so we
> don't break existing userspace tools, but we do not need new
> filesystem specific definitions anywhere.
>
>> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
>> index fcbf647..872fed5 100644
>> --- a/include/uapi/linux/fs.h
>> +++ b/include/uapi/linux/fs.h
>> @@ -58,6 +58,62 @@ struct inodes_stat_t {
>> long dummy[5]; /* padding for sysctl ABI compatibility */
>> };
>>
>> +/*
>> + * Extend attribute flags. These should be or-ed together to figure out what
>> + * is valid.
>> + */
>> +#define FSX_XFLAGS (1 << 0)
>> +#define FSX_EXTSIZE (1 << 1)
>> +#define FSX_NEXTENTS (1 << 2)
>> +#define FSX_PROJID (1 << 3)
>
> NACK.
>
> I've said this more than once: these are *private to XFS's
> implementation* and are not be part of the user interface. Do not
> move them from their existing location.
>
>
>> +
>> +/*
>> + * Structure for FS_IOC_FSGETXATTR and FS_IOC_FSSETXATTR.
>> + */
>> +struct fsxattr {
>> + __u32 fsx_xflags; /* xflags field value (get/set) */
>> + __u32 fsx_extsize; /* extsize field value (get/set)*/
>> + __u32 fsx_nextents; /* nextents field value (get) */
>> + __u32 fsx_projid; /* project identifier (get/set) */
>> + unsigned char fsx_pad[12];
>> +};
>> +
>> +/*
>> + * Flags for the fsx_xflags field
>> + */
>> +#define FS_XFLAG_REALTIME 0x00000001 /* data in realtime volume */
>> +#define FS_XFLAG_PREALLOC 0x00000002 /* preallocated file extents */
>> +#define FS_XFLAG_SECRM 0x00000004 /* secure deletion */
>
> NACK - ext4 specific.
>
>> +#define FS_XFLAG_IMMUTABLE 0x00000008 /* file cannot be modified */
>> +#define FS_XFLAG_APPEND 0x00000010 /* all writes append */
>> +#define FS_XFLAG_SYNC 0x00000020 /* all writes synchronous */
>> +#define FS_XFLAG_NOATIME 0x00000040 /* do not update access time */
>> +#define FS_XFLAG_NODUMP 0x00000080 /* do not include in backups */
>> +#define FS_XFLAG_RTINHERIT 0x00000100 /* create with rt bit set */
>> +#define FS_XFLAG_PROJINHERIT 0x00000200 /* create with parents projid */
>> +#define FS_XFLAG_NOSYMLINKS 0x00000400 /* disallow symlink creation */
>> +#define FS_XFLAG_EXTSIZE 0x00000800 /* extent size allocator hint */
>> +#define FS_XFLAG_EXTSZINHERIT 0x00001000 /* inherit inode extent size */
>> +#define FS_XFLAG_NODEFRAG 0x00002000 /* do not defragment */
>> +#define FS_XFLAG_FILESTREAM 0x00004000 /* use filestream allocator */
>
> existing flags.
>
>> +#define FS_XFLAG_UNRM 0x00008000 /* undelete */
>> +#define FS_XFLAG_COMPR 0x00010000 /* compress file */
>> +#define FS_XFLAG_COMPRBLK 0x00020000 /* one or more compressed clusters */
>> +#define FS_XFLAG_NOCOMPR 0x00040000 /* don't compress */
>> +#define FS_XFLAG_ECOMPR 0x00080000 /* compression error */
>> +#define FS_XFLAG_INDEX 0x00100000 /* hash-indexed directory */
>> +#define FS_XFLAG_IMAGIC 0x00200000 /* AFS directory */
>> +#define FS_XFLAG_JOURNAL_DATA 0x00400000 /* file data should be journaled */
>> +#define FS_XFLAG_NOTAIL 0x00800000 /* file tail should not be merged */
>> +#define FS_XFLAG_DIRSYNC 0x01000000 /* dirsync behaviour (directories only) */
>> +#define FS_XFLAG_TOPDIR 0x02000000 /* top of directory hierarchies*/
>> +#define FS_XFLAG_HUGE_FILE 0x04000000 /* set to each huge file */
>> +#define FS_XFLAG_EXTENTS 0x08000000 /* inode uses extents */
>> +#define FS_XFLAG_EA_INODE 0x10000000 /* inode used for large EA */
>> +#define FS_XFLAG_EOFBLOCKS 0x20000000 /* blocks allocated beyond EOF */
>> +#define FS_XFLAG_INLINE_DATA 0x40000000 /* inode has inline data. */
>
> And a bunch more ext4 specific flags that *uses all the remaining
> flag space*. At minimum, we need to keep space in this existing
> flags field for flags to future indication of how the padding is
> used, so that's yet another NACK.
>
> Further, none of these have any relevance to project quotas so
> should not be a part of this patchset. Nor are they relevant to any
> other filesystem, and many are duplicated by information you can get
> from FIEMAP and other interfaces. NACK, again.
>
> Because I'm getting annoyed at being repeatedly ignored about what
> needs to be done here, I'm now going to shout a bit. DO NOT CHANGE
> THE INTERFACE. DO NOT ADD any new flags to the interface. DO NOT
> REDEFINE how the interface works. DO NOT "ext4-ise" the interface.
>
> The only changes to the interface code should be moving the XFS
> definitions and renaming them so as to provide the new generic
> ioctl definition as well as the historic XFS definitions. The ext4
> implementation needs to be done in a separate patch to the interface
> rename, and it must only implement the functionality the interface
> already provides. Anything else is outside the scope of this
> patchset and requires separate discussion.
What reason for reusing XFS ioctl?
As I see quota tools from xfsprogs checks filesystem name and seems
like they wouldn't work without upgrade. e2fsprogs have to be updated
updated anyway to support changes in layout. So, in this case we could
design new generic ioctl/syscall interface for that. For example add
new commands to quotactl() instead of yet another obscure ioctl.
Also: is quota for project-id '0' really required for something?
It adds overhead but I don't see any use-cases for that.
>
> Cheers,
>
> Dave.
>
^ permalink raw reply
* Re: [v8 4/5] ext4: adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface support
From: Konstantin Khlebnikov @ 2015-01-22 15:28 UTC (permalink / raw)
To: Li Xi, linux-fsdevel, linux-ext4, linux-api, tytso, adilger, jack,
viro, hch, dmonakhov
In-Reply-To: <1418102548-5469-5-git-send-email-lixi@ddn.com>
On 09.12.2014 08:22, Li Xi wrote:
> This patch adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR ioctl interface
> support for ext4. The interface is kept consistent with
> XFS_IOC_FSGETXATTR/XFS_IOC_FSGETXATTR.
>
> Signed-off-by: Li Xi <lixi@ddn.com>
> ---
> fs/ext4/ext4.h | 111 ++++++++++++++++
> fs/ext4/ioctl.c | 330 +++++++++++++++++++++++++++++++++--------------
> fs/xfs/xfs_fs.h | 47 +++-----
> include/uapi/linux/fs.h | 58 ++++++++
> 4 files changed, 418 insertions(+), 128 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 136e18c..43a2a88 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -384,6 +384,115 @@ struct flex_groups {
> #define EXT4_FL_USER_VISIBLE 0x204BDFFF /* User visible flags */
> #define EXT4_FL_USER_MODIFIABLE 0x204380FF /* User modifiable flags */
>
> +/* Transfer internal flags to xflags */
> +static inline __u32 ext4_iflags_to_xflags(unsigned long iflags)
> +{
> + __u32 xflags = 0;
> +
> + if (iflags & EXT4_SECRM_FL)
> + xflags |= FS_XFLAG_SECRM;
> + if (iflags & EXT4_UNRM_FL)
> + xflags |= FS_XFLAG_UNRM;
> + if (iflags & EXT4_COMPR_FL)
> + xflags |= FS_XFLAG_COMPR;
> + if (iflags & EXT4_SYNC_FL)
> + xflags |= FS_XFLAG_SYNC;
> + if (iflags & EXT4_IMMUTABLE_FL)
> + xflags |= FS_XFLAG_IMMUTABLE;
> + if (iflags & EXT4_APPEND_FL)
> + xflags |= FS_XFLAG_APPEND;
> + if (iflags & EXT4_NODUMP_FL)
> + xflags |= FS_XFLAG_NODUMP;
> + if (iflags & EXT4_NOATIME_FL)
> + xflags |= FS_XFLAG_NOATIME;
> + if (iflags & EXT4_COMPRBLK_FL)
> + xflags |= FS_XFLAG_COMPRBLK;
> + if (iflags & EXT4_NOCOMPR_FL)
> + xflags |= FS_XFLAG_NOCOMPR;
> + if (iflags & EXT4_ECOMPR_FL)
> + xflags |= FS_XFLAG_ECOMPR;
> + if (iflags & EXT4_INDEX_FL)
> + xflags |= FS_XFLAG_INDEX;
> + if (iflags & EXT4_IMAGIC_FL)
> + xflags |= FS_XFLAG_IMAGIC;
> + if (iflags & EXT4_JOURNAL_DATA_FL)
> + xflags |= FS_XFLAG_JOURNAL_DATA;
> + if (iflags & EXT4_NOTAIL_FL)
> + xflags |= FS_XFLAG_NOTAIL;
> + if (iflags & EXT4_DIRSYNC_FL)
> + xflags |= FS_XFLAG_DIRSYNC;
> + if (iflags & EXT4_TOPDIR_FL)
> + xflags |= FS_XFLAG_TOPDIR;
> + if (iflags & EXT4_HUGE_FILE_FL)
> + xflags |= FS_XFLAG_HUGE_FILE;
> + if (iflags & EXT4_EXTENTS_FL)
> + xflags |= FS_XFLAG_EXTENTS;
> + if (iflags & EXT4_EA_INODE_FL)
> + xflags |= FS_XFLAG_EA_INODE;
> + if (iflags & EXT4_EOFBLOCKS_FL)
> + xflags |= FS_XFLAG_EOFBLOCKS;
> + if (iflags & EXT4_INLINE_DATA_FL)
> + xflags |= FS_XFLAG_INLINE_DATA;
> + if (iflags & EXT4_PROJINHERIT_FL)
> + xflags |= FS_XFLAG_PROJINHERIT;
> + return xflags;
> +}
> +
> +/* Transfer xflags flags to internal */
> +static inline unsigned long ext4_xflags_to_iflags(__u32 xflags)
> +{
> + unsigned long iflags = 0;
> +
> + if (xflags & FS_XFLAG_SECRM)
> + iflags |= EXT4_SECRM_FL;
> + if (xflags & FS_XFLAG_UNRM)
> + iflags |= EXT4_UNRM_FL;
> + if (xflags & FS_XFLAG_COMPR)
> + iflags |= EXT4_COMPR_FL;
> + if (xflags & FS_XFLAG_SYNC)
> + iflags |= EXT4_SYNC_FL;
> + if (xflags & FS_XFLAG_IMMUTABLE)
> + iflags |= EXT4_IMMUTABLE_FL;
> + if (xflags & FS_XFLAG_APPEND)
> + iflags |= EXT4_APPEND_FL;
> + if (xflags & FS_XFLAG_NODUMP)
> + iflags |= EXT4_NODUMP_FL;
> + if (xflags & FS_XFLAG_NOATIME)
> + iflags |= EXT4_NOATIME_FL;
> + if (xflags & FS_XFLAG_COMPRBLK)
> + iflags |= EXT4_COMPRBLK_FL;
> + if (xflags & FS_XFLAG_NOCOMPR)
> + iflags |= EXT4_NOCOMPR_FL;
> + if (xflags & FS_XFLAG_ECOMPR)
> + iflags |= EXT4_ECOMPR_FL;
> + if (xflags & FS_XFLAG_INDEX)
> + iflags |= EXT4_INDEX_FL;
> + if (xflags & FS_XFLAG_IMAGIC)
> + iflags |= EXT4_IMAGIC_FL;
> + if (xflags & FS_XFLAG_JOURNAL_DATA)
> + iflags |= EXT4_JOURNAL_DATA_FL;
> + if (xflags & FS_XFLAG_IMAGIC)
> + iflags |= FS_XFLAG_NOTAIL;
> + if (xflags & FS_XFLAG_DIRSYNC)
> + iflags |= EXT4_DIRSYNC_FL;
> + if (xflags & FS_XFLAG_TOPDIR)
> + iflags |= EXT4_TOPDIR_FL;
> + if (xflags & FS_XFLAG_HUGE_FILE)
> + iflags |= EXT4_HUGE_FILE_FL;
> + if (xflags & FS_XFLAG_EXTENTS)
> + iflags |= EXT4_EXTENTS_FL;
> + if (xflags & FS_XFLAG_EA_INODE)
> + iflags |= EXT4_EA_INODE_FL;
> + if (xflags & FS_XFLAG_EOFBLOCKS)
> + iflags |= EXT4_EOFBLOCKS_FL;
> + if (xflags & FS_XFLAG_INLINE_DATA)
> + iflags |= EXT4_INLINE_DATA_FL;
> + if (xflags & FS_XFLAG_PROJINHERIT)
> + iflags |= EXT4_PROJINHERIT_FL;
> +
> + return iflags;
> +}
> +
> /* Flags that should be inherited by new inodes from their parent. */
> #define EXT4_FL_INHERITED (EXT4_SECRM_FL | EXT4_UNRM_FL | EXT4_COMPR_FL |\
> EXT4_SYNC_FL | EXT4_NODUMP_FL | EXT4_NOATIME_FL |\
> @@ -606,6 +715,8 @@ enum {
> #define EXT4_IOC_RESIZE_FS _IOW('f', 16, __u64)
> #define EXT4_IOC_SWAP_BOOT _IO('f', 17)
> #define EXT4_IOC_PRECACHE_EXTENTS _IO('f', 18)
> +#define EXT4_IOC_FSGETXATTR FS_IOC_FSGETXATTR
> +#define EXT4_IOC_FSSETXATTR FS_IOC_FSSETXATTR
>
> #if defined(__KERNEL__) && defined(CONFIG_COMPAT)
> /*
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index f58a0d1..8332476 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -14,6 +14,8 @@
> #include <linux/compat.h>
> #include <linux/mount.h>
> #include <linux/file.h>
> +#include <linux/quotaops.h>
> +#include <linux/quota.h>
> #include <asm/uaccess.h>
> #include "ext4_jbd2.h"
> #include "ext4.h"
> @@ -196,126 +198,220 @@ journal_err_out:
> return err;
> }
>
> -long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> +static int ext4_ioctl_setflags(struct file *filp, unsigned int flags)
> {
> struct inode *inode = file_inode(filp);
> - struct super_block *sb = inode->i_sb;
> struct ext4_inode_info *ei = EXT4_I(inode);
> - unsigned int flags;
> + handle_t *handle = NULL;
> + int err, migrate = 0;
> + struct ext4_iloc iloc;
> + unsigned int oldflags, mask, i;
> + unsigned int jflag;
>
> - ext4_debug("cmd = %u, arg = %lu\n", cmd, arg);
> + if (!inode_owner_or_capable(inode))
> + return -EACCES;
>
> - switch (cmd) {
> - case EXT4_IOC_GETFLAGS:
> - ext4_get_inode_flags(ei);
> - flags = ei->i_flags & EXT4_FL_USER_VISIBLE;
> - return put_user(flags, (int __user *) arg);
> - case EXT4_IOC_SETFLAGS: {
> - handle_t *handle = NULL;
> - int err, migrate = 0;
> - struct ext4_iloc iloc;
> - unsigned int oldflags, mask, i;
> - unsigned int jflag;
> + err = mnt_want_write_file(filp);
> + if (err)
> + return err;
>
> - if (!inode_owner_or_capable(inode))
> - return -EACCES;
> + flags = ext4_mask_flags(inode->i_mode, flags);
>
> - if (get_user(flags, (int __user *) arg))
> - return -EFAULT;
> + err = -EPERM;
> + mutex_lock(&inode->i_mutex);
> + /* Is it quota file? Do not allow user to mess with it */
> + if (IS_NOQUOTA(inode))
> + goto flags_out;
>
> - err = mnt_want_write_file(filp);
> - if (err)
> - return err;
> + oldflags = ei->i_flags;
>
> - flags = ext4_mask_flags(inode->i_mode, flags);
> + /* The JOURNAL_DATA flag is modifiable only by root */
> + jflag = flags & EXT4_JOURNAL_DATA_FL;
>
> - err = -EPERM;
> - mutex_lock(&inode->i_mutex);
> - /* Is it quota file? Do not allow user to mess with it */
> - if (IS_NOQUOTA(inode))
> + /*
> + * The IMMUTABLE and APPEND_ONLY flags can only be changed by
> + * the relevant capability.
> + *
> + * This test looks nicer. Thanks to Pauline Middelink
> + */
> + if ((flags ^ oldflags) & (EXT4_APPEND_FL | EXT4_IMMUTABLE_FL)) {
> + if (!capable(CAP_LINUX_IMMUTABLE))
> goto flags_out;
> + }
>
> - oldflags = ei->i_flags;
> -
> - /* The JOURNAL_DATA flag is modifiable only by root */
> - jflag = flags & EXT4_JOURNAL_DATA_FL;
> -
> - /*
> - * The IMMUTABLE and APPEND_ONLY flags can only be changed by
> - * the relevant capability.
> - *
> - * This test looks nicer. Thanks to Pauline Middelink
> - */
> - if ((flags ^ oldflags) & (EXT4_APPEND_FL | EXT4_IMMUTABLE_FL)) {
> - if (!capable(CAP_LINUX_IMMUTABLE))
> - goto flags_out;
> - }
> -
> - /*
> - * The JOURNAL_DATA flag can only be changed by
> - * the relevant capability.
> - */
> - if ((jflag ^ oldflags) & (EXT4_JOURNAL_DATA_FL)) {
> - if (!capable(CAP_SYS_RESOURCE))
> - goto flags_out;
> - }
> - if ((flags ^ oldflags) & EXT4_EXTENTS_FL)
> - migrate = 1;
> -
> + /*
> + * The JOURNAL_DATA flag can only be changed by
> + * the relevant capability.
> + */
> + if ((jflag ^ oldflags) & (EXT4_JOURNAL_DATA_FL)) {
> + if (!capable(CAP_SYS_RESOURCE))
> + goto flags_out;
> + }
> + if ((flags ^ oldflags) & EXT4_EXTENTS_FL)
> + migrate = 1;
> if (flags & EXT4_EOFBLOCKS_FL) {
> - /* we don't support adding EOFBLOCKS flag */
> - if (!(oldflags & EXT4_EOFBLOCKS_FL)) {
> - err = -EOPNOTSUPP;
> - goto flags_out;
> - }
> - } else if (oldflags & EXT4_EOFBLOCKS_FL)
> - ext4_truncate(inode);
> -
> - handle = ext4_journal_start(inode, EXT4_HT_INODE, 1);
> - if (IS_ERR(handle)) {
> - err = PTR_ERR(handle);
> + /* we don't support adding EOFBLOCKS flag */
> + if (!(oldflags & EXT4_EOFBLOCKS_FL)) {
> + err = -EOPNOTSUPP;
> goto flags_out;
> }
> - if (IS_SYNC(inode))
> - ext4_handle_sync(handle);
> - err = ext4_reserve_inode_write(handle, inode, &iloc);
> - if (err)
> - goto flags_err;
> -
> - for (i = 0, mask = 1; i < 32; i++, mask <<= 1) {
> - if (!(mask & EXT4_FL_USER_MODIFIABLE))
> - continue;
> - if (mask & flags)
> - ext4_set_inode_flag(inode, i);
> - else
> - ext4_clear_inode_flag(inode, i);
> - }
> + } else if (oldflags & EXT4_EOFBLOCKS_FL)
> + ext4_truncate(inode);
>
> - ext4_set_inode_flags(inode);
> - inode->i_ctime = ext4_current_time(inode);
> + handle = ext4_journal_start(inode, EXT4_HT_INODE, 1);
> + if (IS_ERR(handle)) {
> + err = PTR_ERR(handle);
> + goto flags_out;
> + }
> + if (IS_SYNC(inode))
> + ext4_handle_sync(handle);
> + err = ext4_reserve_inode_write(handle, inode, &iloc);
> + if (err)
> + goto flags_err;
> +
> + for (i = 0, mask = 1; i < 32; i++, mask <<= 1) {
> + if (!(mask & EXT4_FL_USER_MODIFIABLE))
> + continue;
> + if (mask & flags)
> + ext4_set_inode_flag(inode, i);
> + else
> + ext4_clear_inode_flag(inode, i);
> + }
>
> - err = ext4_mark_iloc_dirty(handle, inode, &iloc);
> -flags_err:
> - ext4_journal_stop(handle);
> - if (err)
> - goto flags_out;
> + ext4_set_inode_flags(inode);
> + inode->i_ctime = ext4_current_time(inode);
>
> - if ((jflag ^ oldflags) & (EXT4_JOURNAL_DATA_FL))
> - err = ext4_change_inode_journal_flag(inode, jflag);
> - if (err)
> - goto flags_out;
> - if (migrate) {
> - if (flags & EXT4_EXTENTS_FL)
> - err = ext4_ext_migrate(inode);
> - else
> - err = ext4_ind_migrate(inode);
> - }
> + err = ext4_mark_iloc_dirty(handle, inode, &iloc);
> +flags_err:
> + ext4_journal_stop(handle);
> + if (err)
> + goto flags_out;
> +
> + if ((jflag ^ oldflags) & (EXT4_JOURNAL_DATA_FL))
> + err = ext4_change_inode_journal_flag(inode, jflag);
> + if (err)
> + goto flags_out;
> + if (migrate) {
> + if (flags & EXT4_EXTENTS_FL)
> + err = ext4_ext_migrate(inode);
> + else
> + err = ext4_ind_migrate(inode);
> + }
>
> flags_out:
> - mutex_unlock(&inode->i_mutex);
> - mnt_drop_write_file(filp);
> + mutex_unlock(&inode->i_mutex);
> + mnt_drop_write_file(filp);
> + return err;
> +}
> +
> +static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
> +{
> + struct inode *inode = file_inode(filp);
> + struct super_block *sb = inode->i_sb;
> + struct ext4_inode_info *ei = EXT4_I(inode);
> + int err;
> + handle_t *handle;
> + kprojid_t kprojid;
> + struct ext4_iloc iloc;
> + struct ext4_inode *raw_inode;
> +
> + struct dquot *transfer_to[EXT4_MAXQUOTAS] = { };
> +
> + /* Make sure caller can change project. */
> + if (!capable(CAP_SYS_ADMIN))
> + return -EACCES;
> +
> + if (projid != EXT4_DEF_PROJID
> + && !EXT4_HAS_RO_COMPAT_FEATURE(sb,
> + EXT4_FEATURE_RO_COMPAT_PROJECT))
> + return -EOPNOTSUPP;
> +
> + if (!EXT4_HAS_RO_COMPAT_FEATURE(sb,
> + EXT4_FEATURE_RO_COMPAT_PROJECT)) {
> + BUG_ON(__kprojid_val(EXT4_I(inode)->i_projid)
> + != EXT4_DEF_PROJID);
> + if (projid != EXT4_DEF_PROJID)
> + return -EOPNOTSUPP;
> + else
> + return 0;
> + }
> +
> + kprojid = make_kprojid(&init_user_ns, (projid_t)projid);
Maybe current_user_ns()?
This code should be user-namespace aware from the beginning.
> +
> + if (projid_eq(kprojid, EXT4_I(inode)->i_projid))
> + return 0;
> +
> + err = mnt_want_write_file(filp);
> + if (err)
> return err;
> +
> + err = -EPERM;
> + mutex_lock(&inode->i_mutex);
> + /* Is it quota file? Do not allow user to mess with it */
> + if (IS_NOQUOTA(inode))
> + goto project_out;
> +
> + dquot_initialize(inode);
> +
> + handle = ext4_journal_start(inode, EXT4_HT_QUOTA,
> + EXT4_QUOTA_INIT_BLOCKS(sb) +
> + EXT4_QUOTA_DEL_BLOCKS(sb) + 3);
> + if (IS_ERR(handle)) {
> + err = PTR_ERR(handle);
> + goto project_out;
> + }
> +
> + err = ext4_reserve_inode_write(handle, inode, &iloc);
> + if (err)
> + goto project_stop;
> +
> + raw_inode = ext4_raw_inode(&iloc);
> + if ((EXT4_INODE_SIZE(sb) <=
> + EXT4_GOOD_OLD_INODE_SIZE) ||
> + (!EXT4_FITS_IN_INODE(raw_inode, ei, i_projid))) {
> + err = -EFBIG;
> + goto project_stop;
> }
> +
> + transfer_to[PRJQUOTA] = dqget(sb, make_kqid_projid(kprojid));
> + if (!transfer_to[PRJQUOTA])
> + goto project_set;
> +
> + err = __dquot_transfer(inode, transfer_to);
> + dqput(transfer_to[PRJQUOTA]);
> + if (err)
> + goto project_stop;
> +
> +project_set:
> + EXT4_I(inode)->i_projid = kprojid;
> + inode->i_ctime = ext4_current_time(inode);
> + err = ext4_mark_iloc_dirty(handle, inode, &iloc);
> +project_stop:
> + ext4_journal_stop(handle);
> +project_out:
> + mutex_unlock(&inode->i_mutex);
> + mnt_drop_write_file(filp);
> + return err;
> +}
> +
> +long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> +{
> + struct inode *inode = file_inode(filp);
> + struct super_block *sb = inode->i_sb;
> + struct ext4_inode_info *ei = EXT4_I(inode);
> + unsigned int flags;
> +
> + ext4_debug("cmd = %u, arg = %lu\n", cmd, arg);
> +
> + switch (cmd) {
> + case EXT4_IOC_GETFLAGS:
> + ext4_get_inode_flags(ei);
> + flags = ei->i_flags & EXT4_FL_USER_VISIBLE;
> + return put_user(flags, (int __user *) arg);
> + case EXT4_IOC_SETFLAGS:
> + if (get_user(flags, (int __user *) arg))
> + return -EFAULT;
> + return ext4_ioctl_setflags(filp, flags);
> case EXT4_IOC_GETVERSION:
> case EXT4_IOC_GETVERSION_OLD:
> return put_user(inode->i_generation, (int __user *) arg);
> @@ -615,7 +711,45 @@ resizefs_out:
> }
> case EXT4_IOC_PRECACHE_EXTENTS:
> return ext4_ext_precache(inode);
> + case EXT4_IOC_FSGETXATTR:
> + {
> + struct fsxattr fa;
> +
> + memset(&fa, 0, sizeof(struct fsxattr));
> +
> + ext4_get_inode_flags(ei);
> + fa.fsx_xflags = ext4_iflags_to_xflags(ei->i_flags & EXT4_FL_USER_VISIBLE);
> +
> + if (EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
> + EXT4_FEATURE_RO_COMPAT_PROJECT)) {
> + fa.fsx_projid = (__u32)from_kprojid(&init_user_ns,
> + EXT4_I(inode)->i_projid);
Same here.
> + }
> +
> + if (copy_to_user((struct fsxattr __user *)arg,
> + &fa, sizeof(fa)))
> + return -EFAULT;
> + return 0;
> + }
> + case EXT4_IOC_FSSETXATTR:
> + {
> + struct fsxattr fa;
> + int err;
> +
> + if (copy_from_user(&fa, (struct fsxattr __user *)arg,
> + sizeof(fa)))
> + return -EFAULT;
>
> + err = ext4_ioctl_setflags(filp, ext4_xflags_to_iflags(fa.fsx_xflags));
> + if (err)
> + return err;
> +
> + err = ext4_ioctl_setproject(filp, fa.fsx_projid);
> + if (err)
> + return err;
> +
> + return 0;
> + }
> default:
> return -ENOTTY;
> }
> diff --git a/fs/xfs/xfs_fs.h b/fs/xfs/xfs_fs.h
> index 18dc721..64c7ae6 100644
> --- a/fs/xfs/xfs_fs.h
> +++ b/fs/xfs/xfs_fs.h
> @@ -36,38 +36,25 @@ struct dioattr {
> #endif
>
> /*
> - * Structure for XFS_IOC_FSGETXATTR[A] and XFS_IOC_FSSETXATTR.
> - */
> -#ifndef HAVE_FSXATTR
> -struct fsxattr {
> - __u32 fsx_xflags; /* xflags field value (get/set) */
> - __u32 fsx_extsize; /* extsize field value (get/set)*/
> - __u32 fsx_nextents; /* nextents field value (get) */
> - __u32 fsx_projid; /* project identifier (get/set) */
> - unsigned char fsx_pad[12];
> -};
> -#endif
> -
> -/*
> * Flags for the bs_xflags/fsx_xflags field
> * There should be a one-to-one correspondence between these flags and the
> * XFS_DIFLAG_s.
> */
> -#define XFS_XFLAG_REALTIME 0x00000001 /* data in realtime volume */
> -#define XFS_XFLAG_PREALLOC 0x00000002 /* preallocated file extents */
> -#define XFS_XFLAG_IMMUTABLE 0x00000008 /* file cannot be modified */
> -#define XFS_XFLAG_APPEND 0x00000010 /* all writes append */
> -#define XFS_XFLAG_SYNC 0x00000020 /* all writes synchronous */
> -#define XFS_XFLAG_NOATIME 0x00000040 /* do not update access time */
> -#define XFS_XFLAG_NODUMP 0x00000080 /* do not include in backups */
> -#define XFS_XFLAG_RTINHERIT 0x00000100 /* create with rt bit set */
> -#define XFS_XFLAG_PROJINHERIT 0x00000200 /* create with parents projid */
> -#define XFS_XFLAG_NOSYMLINKS 0x00000400 /* disallow symlink creation */
> -#define XFS_XFLAG_EXTSIZE 0x00000800 /* extent size allocator hint */
> -#define XFS_XFLAG_EXTSZINHERIT 0x00001000 /* inherit inode extent size */
> -#define XFS_XFLAG_NODEFRAG 0x00002000 /* do not defragment */
> -#define XFS_XFLAG_FILESTREAM 0x00004000 /* use filestream allocator */
> -#define XFS_XFLAG_HASATTR 0x80000000 /* no DIFLAG for this */
> +#define XFS_XFLAG_REALTIME FS_XFLAG_REALTIME /* data in realtime volume */
> +#define XFS_XFLAG_PREALLOC FS_XFLAG_PREALLOC /* preallocated file extents */
> +#define XFS_XFLAG_IMMUTABLE FS_XFLAG_IMMUTABLE /* file cannot be modified */
> +#define XFS_XFLAG_APPEND FS_XFLAG_APPEND /* all writes append */
> +#define XFS_XFLAG_SYNC FS_XFLAG_SYNC /* all writes synchronous */
> +#define XFS_XFLAG_NOATIME FS_XFLAG_NOATIME /* do not update access time */
> +#define XFS_XFLAG_NODUMP FS_XFLAG_NODUMP /* do not include in backups */
> +#define XFS_XFLAG_RTINHERIT FS_XFLAG_RTINHERIT /* create with rt bit set */
> +#define XFS_XFLAG_PROJINHERIT FS_XFLAG_PROJINHERIT /* create with parents projid */
> +#define XFS_XFLAG_NOSYMLINKS FS_XFLAG_NOSYMLINKS /* disallow symlink creation */
> +#define XFS_XFLAG_EXTSIZE FS_XFLAG_EXTSIZE /* extent size allocator hint */
> +#define XFS_XFLAG_EXTSZINHERIT FS_XFLAG_EXTSZINHERIT /* inherit inode extent size */
> +#define XFS_XFLAG_NODEFRAG FS_XFLAG_NODEFRAG /* do not defragment */
> +#define XFS_XFLAG_FILESTREAM FS_XFLAG_FILESTREAM /* use filestream allocator */
> +#define XFS_XFLAG_HASATTR FS_XFLAG_HASATTR /* no DIFLAG for this */
>
> /*
> * Structure for XFS_IOC_GETBMAP.
> @@ -503,8 +490,8 @@ typedef struct xfs_swapext
> #define XFS_IOC_ALLOCSP _IOW ('X', 10, struct xfs_flock64)
> #define XFS_IOC_FREESP _IOW ('X', 11, struct xfs_flock64)
> #define XFS_IOC_DIOINFO _IOR ('X', 30, struct dioattr)
> -#define XFS_IOC_FSGETXATTR _IOR ('X', 31, struct fsxattr)
> -#define XFS_IOC_FSSETXATTR _IOW ('X', 32, struct fsxattr)
> +#define XFS_IOC_FSGETXATTR FS_IOC_FSGETXATTR
> +#define XFS_IOC_FSSETXATTR FS_IOC_FSSETXATTR
> #define XFS_IOC_ALLOCSP64 _IOW ('X', 36, struct xfs_flock64)
> #define XFS_IOC_FREESP64 _IOW ('X', 37, struct xfs_flock64)
> #define XFS_IOC_GETBMAP _IOWR('X', 38, struct getbmap)
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index fcbf647..872fed5 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -58,6 +58,62 @@ struct inodes_stat_t {
> long dummy[5]; /* padding for sysctl ABI compatibility */
> };
>
> +/*
> + * Extend attribute flags. These should be or-ed together to figure out what
> + * is valid.
> + */
> +#define FSX_XFLAGS (1 << 0)
> +#define FSX_EXTSIZE (1 << 1)
> +#define FSX_NEXTENTS (1 << 2)
> +#define FSX_PROJID (1 << 3)
> +
> +/*
> + * Structure for FS_IOC_FSGETXATTR and FS_IOC_FSSETXATTR.
> + */
> +struct fsxattr {
> + __u32 fsx_xflags; /* xflags field value (get/set) */
> + __u32 fsx_extsize; /* extsize field value (get/set)*/
> + __u32 fsx_nextents; /* nextents field value (get) */
> + __u32 fsx_projid; /* project identifier (get/set) */
> + unsigned char fsx_pad[12];
> +};
> +
> +/*
> + * Flags for the fsx_xflags field
> + */
> +#define FS_XFLAG_REALTIME 0x00000001 /* data in realtime volume */
> +#define FS_XFLAG_PREALLOC 0x00000002 /* preallocated file extents */
> +#define FS_XFLAG_SECRM 0x00000004 /* secure deletion */
> +#define FS_XFLAG_IMMUTABLE 0x00000008 /* file cannot be modified */
> +#define FS_XFLAG_APPEND 0x00000010 /* all writes append */
> +#define FS_XFLAG_SYNC 0x00000020 /* all writes synchronous */
> +#define FS_XFLAG_NOATIME 0x00000040 /* do not update access time */
> +#define FS_XFLAG_NODUMP 0x00000080 /* do not include in backups */
> +#define FS_XFLAG_RTINHERIT 0x00000100 /* create with rt bit set */
> +#define FS_XFLAG_PROJINHERIT 0x00000200 /* create with parents projid */
> +#define FS_XFLAG_NOSYMLINKS 0x00000400 /* disallow symlink creation */
> +#define FS_XFLAG_EXTSIZE 0x00000800 /* extent size allocator hint */
> +#define FS_XFLAG_EXTSZINHERIT 0x00001000 /* inherit inode extent size */
> +#define FS_XFLAG_NODEFRAG 0x00002000 /* do not defragment */
> +#define FS_XFLAG_FILESTREAM 0x00004000 /* use filestream allocator */
> +#define FS_XFLAG_UNRM 0x00008000 /* undelete */
> +#define FS_XFLAG_COMPR 0x00010000 /* compress file */
> +#define FS_XFLAG_COMPRBLK 0x00020000 /* one or more compressed clusters */
> +#define FS_XFLAG_NOCOMPR 0x00040000 /* don't compress */
> +#define FS_XFLAG_ECOMPR 0x00080000 /* compression error */
> +#define FS_XFLAG_INDEX 0x00100000 /* hash-indexed directory */
> +#define FS_XFLAG_IMAGIC 0x00200000 /* AFS directory */
> +#define FS_XFLAG_JOURNAL_DATA 0x00400000 /* file data should be journaled */
> +#define FS_XFLAG_NOTAIL 0x00800000 /* file tail should not be merged */
> +#define FS_XFLAG_DIRSYNC 0x01000000 /* dirsync behaviour (directories only) */
> +#define FS_XFLAG_TOPDIR 0x02000000 /* top of directory hierarchies*/
> +#define FS_XFLAG_HUGE_FILE 0x04000000 /* set to each huge file */
> +#define FS_XFLAG_EXTENTS 0x08000000 /* inode uses extents */
> +#define FS_XFLAG_EA_INODE 0x10000000 /* inode used for large EA */
> +#define FS_XFLAG_EOFBLOCKS 0x20000000 /* blocks allocated beyond EOF */
> +#define FS_XFLAG_INLINE_DATA 0x40000000 /* inode has inline data. */
> +#define FS_XFLAG_HASATTR 0x80000000 /* no DIFLAG for this */
> +
>
> #define NR_FILE 8192 /* this can well be larger on a larger system */
>
> @@ -163,6 +219,8 @@ struct inodes_stat_t {
> #define FS_IOC_GETVERSION _IOR('v', 1, long)
> #define FS_IOC_SETVERSION _IOW('v', 2, long)
> #define FS_IOC_FIEMAP _IOWR('f', 11, struct fiemap)
> +#define FS_IOC_FSGETXATTR _IOR('f', 31, struct fsxattr)
> +#define FS_IOC_FSSETXATTR _IOW('f', 32, struct fsxattr)
> #define FS_IOC32_GETFLAGS _IOR('f', 1, int)
> #define FS_IOC32_SETFLAGS _IOW('f', 2, int)
> #define FS_IOC32_GETVERSION _IOR('v', 1, int)
>
^ permalink raw reply
* Re: [v8 4/5] ext4: adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface support
From: Jan Kara @ 2015-01-22 15:59 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: Dave Chinner, Li Xi, linux-fsdevel, linux-ext4, linux-api, tytso,
adilger, jack, viro, hch, dmonakhov
In-Reply-To: <54C1152F.1080101@yandex-team.ru>
On Thu 22-01-15 18:20:15, Konstantin Khlebnikov wrote:
> On 10.12.2014 01:57, Dave Chinner wrote:
> >On Tue, Dec 09, 2014 at 01:22:27PM +0800, Li Xi wrote:
> >>This patch adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR ioctl interface
> >>support for ext4. The interface is kept consistent with
> >>XFS_IOC_FSGETXATTR/XFS_IOC_FSGETXATTR.
> >>
> >>Signed-off-by: Li Xi <lixi@ddn.com>
> >>---
> >> fs/ext4/ext4.h | 111 ++++++++++++++++
> >> fs/ext4/ioctl.c | 330 +++++++++++++++++++++++++++++++++--------------
> >> fs/xfs/xfs_fs.h | 47 +++-----
> >> include/uapi/linux/fs.h | 58 ++++++++
> >> 4 files changed, 418 insertions(+), 128 deletions(-)
> >>
> >>diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> >>index 136e18c..43a2a88 100644
> >>--- a/fs/ext4/ext4.h
> >>+++ b/fs/ext4/ext4.h
> >>@@ -384,6 +384,115 @@ struct flex_groups {
> >> #define EXT4_FL_USER_VISIBLE 0x204BDFFF /* User visible flags */
> >> #define EXT4_FL_USER_MODIFIABLE 0x204380FF /* User modifiable flags */
> >>
> >>+/* Transfer internal flags to xflags */
> >>+static inline __u32 ext4_iflags_to_xflags(unsigned long iflags)
> >>+{
> >>+ __u32 xflags = 0;
> >>+
> >>+ if (iflags & EXT4_SECRM_FL)
> >>+ xflags |= FS_XFLAG_SECRM;
> >>+ if (iflags & EXT4_UNRM_FL)
> >>+ xflags |= FS_XFLAG_UNRM;
> >>+ if (iflags & EXT4_COMPR_FL)
> >>+ xflags |= FS_XFLAG_COMPR;
> >....
> >
> >NACK.
> >
> >>+ if (iflags & EXT4_SYNC_FL)
> >>+ xflags |= FS_XFLAG_SYNC;
> >>+ if (iflags & EXT4_IMMUTABLE_FL)
> >>+ xflags |= FS_XFLAG_IMMUTABLE;
> >>+ if (iflags & EXT4_APPEND_FL)
> >>+ xflags |= FS_XFLAG_APPEND;
> >>+ if (iflags & EXT4_NODUMP_FL)
> >>+ xflags |= FS_XFLAG_NODUMP;
> >>+ if (iflags & EXT4_NOATIME_FL)
> >>+ xflags |= FS_XFLAG_NOATIME;
> >
> >These are OK because they already exist in the interface, but all
> >the ext4 specific flags you've added have no place in this patchset.
> >
> >
> >>+
> >> /* Flags that should be inherited by new inodes from their parent. */
> >> #define EXT4_FL_INHERITED (EXT4_SECRM_FL | EXT4_UNRM_FL | EXT4_COMPR_FL |\
> >> EXT4_SYNC_FL | EXT4_NODUMP_FL | EXT4_NOATIME_FL |\
> >>@@ -606,6 +715,8 @@ enum {
> >> #define EXT4_IOC_RESIZE_FS _IOW('f', 16, __u64)
> >> #define EXT4_IOC_SWAP_BOOT _IO('f', 17)
> >> #define EXT4_IOC_PRECACHE_EXTENTS _IO('f', 18)
> >>+#define EXT4_IOC_FSGETXATTR FS_IOC_FSGETXATTR
> >>+#define EXT4_IOC_FSSETXATTR FS_IOC_FSSETXATTR
> >
> >NACK. Userspace only needs FS_IOC_FS[GS]ETXATTR so that it works across
> >all filesystems. We need to retain XFS_IOC_FS[GS]ETXATTR so we
> >don't break existing userspace tools, but we do not need new
> >filesystem specific definitions anywhere.
> >
> >>diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> >>index fcbf647..872fed5 100644
> >>--- a/include/uapi/linux/fs.h
> >>+++ b/include/uapi/linux/fs.h
> >>@@ -58,6 +58,62 @@ struct inodes_stat_t {
> >> long dummy[5]; /* padding for sysctl ABI compatibility */
> >> };
> >>
> >>+/*
> >>+ * Extend attribute flags. These should be or-ed together to figure out what
> >>+ * is valid.
> >>+ */
> >>+#define FSX_XFLAGS (1 << 0)
> >>+#define FSX_EXTSIZE (1 << 1)
> >>+#define FSX_NEXTENTS (1 << 2)
> >>+#define FSX_PROJID (1 << 3)
> >
> >NACK.
> >
> >I've said this more than once: these are *private to XFS's
> >implementation* and are not be part of the user interface. Do not
> >move them from their existing location.
> >
> >
> >>+
> >>+/*
> >>+ * Structure for FS_IOC_FSGETXATTR and FS_IOC_FSSETXATTR.
> >>+ */
> >>+struct fsxattr {
> >>+ __u32 fsx_xflags; /* xflags field value (get/set) */
> >>+ __u32 fsx_extsize; /* extsize field value (get/set)*/
> >>+ __u32 fsx_nextents; /* nextents field value (get) */
> >>+ __u32 fsx_projid; /* project identifier (get/set) */
> >>+ unsigned char fsx_pad[12];
> >>+};
> >>+
> >>+/*
> >>+ * Flags for the fsx_xflags field
> >>+ */
> >>+#define FS_XFLAG_REALTIME 0x00000001 /* data in realtime volume */
> >>+#define FS_XFLAG_PREALLOC 0x00000002 /* preallocated file extents */
> >>+#define FS_XFLAG_SECRM 0x00000004 /* secure deletion */
> >
> >NACK - ext4 specific.
> >
> >>+#define FS_XFLAG_IMMUTABLE 0x00000008 /* file cannot be modified */
> >>+#define FS_XFLAG_APPEND 0x00000010 /* all writes append */
> >>+#define FS_XFLAG_SYNC 0x00000020 /* all writes synchronous */
> >>+#define FS_XFLAG_NOATIME 0x00000040 /* do not update access time */
> >>+#define FS_XFLAG_NODUMP 0x00000080 /* do not include in backups */
> >>+#define FS_XFLAG_RTINHERIT 0x00000100 /* create with rt bit set */
> >>+#define FS_XFLAG_PROJINHERIT 0x00000200 /* create with parents projid */
> >>+#define FS_XFLAG_NOSYMLINKS 0x00000400 /* disallow symlink creation */
> >>+#define FS_XFLAG_EXTSIZE 0x00000800 /* extent size allocator hint */
> >>+#define FS_XFLAG_EXTSZINHERIT 0x00001000 /* inherit inode extent size */
> >>+#define FS_XFLAG_NODEFRAG 0x00002000 /* do not defragment */
> >>+#define FS_XFLAG_FILESTREAM 0x00004000 /* use filestream allocator */
> >
> >existing flags.
> >
> >>+#define FS_XFLAG_UNRM 0x00008000 /* undelete */
> >>+#define FS_XFLAG_COMPR 0x00010000 /* compress file */
> >>+#define FS_XFLAG_COMPRBLK 0x00020000 /* one or more compressed clusters */
> >>+#define FS_XFLAG_NOCOMPR 0x00040000 /* don't compress */
> >>+#define FS_XFLAG_ECOMPR 0x00080000 /* compression error */
> >>+#define FS_XFLAG_INDEX 0x00100000 /* hash-indexed directory */
> >>+#define FS_XFLAG_IMAGIC 0x00200000 /* AFS directory */
> >>+#define FS_XFLAG_JOURNAL_DATA 0x00400000 /* file data should be journaled */
> >>+#define FS_XFLAG_NOTAIL 0x00800000 /* file tail should not be merged */
> >>+#define FS_XFLAG_DIRSYNC 0x01000000 /* dirsync behaviour (directories only) */
> >>+#define FS_XFLAG_TOPDIR 0x02000000 /* top of directory hierarchies*/
> >>+#define FS_XFLAG_HUGE_FILE 0x04000000 /* set to each huge file */
> >>+#define FS_XFLAG_EXTENTS 0x08000000 /* inode uses extents */
> >>+#define FS_XFLAG_EA_INODE 0x10000000 /* inode used for large EA */
> >>+#define FS_XFLAG_EOFBLOCKS 0x20000000 /* blocks allocated beyond EOF */
> >>+#define FS_XFLAG_INLINE_DATA 0x40000000 /* inode has inline data. */
> >
> >And a bunch more ext4 specific flags that *uses all the remaining
> >flag space*. At minimum, we need to keep space in this existing
> >flags field for flags to future indication of how the padding is
> >used, so that's yet another NACK.
> >
> >Further, none of these have any relevance to project quotas so
> >should not be a part of this patchset. Nor are they relevant to any
> >other filesystem, and many are duplicated by information you can get
> >from FIEMAP and other interfaces. NACK, again.
> >
> >Because I'm getting annoyed at being repeatedly ignored about what
> >needs to be done here, I'm now going to shout a bit. DO NOT CHANGE
> >THE INTERFACE. DO NOT ADD any new flags to the interface. DO NOT
> >REDEFINE how the interface works. DO NOT "ext4-ise" the interface.
> >
> >The only changes to the interface code should be moving the XFS
> >definitions and renaming them so as to provide the new generic
> >ioctl definition as well as the historic XFS definitions. The ext4
> >implementation needs to be done in a separate patch to the interface
> >rename, and it must only implement the functionality the interface
> >already provides. Anything else is outside the scope of this
> >patchset and requires separate discussion.
>
> What reason for reusing XFS ioctl?
>
> As I see quota tools from xfsprogs checks filesystem name and seems
> like they wouldn't work without upgrade. e2fsprogs have to be updated
> updated anyway to support changes in layout. So, in this case we could
> design new generic ioctl/syscall interface for that. For example add
> new commands to quotactl() instead of yet another obscure ioctl.
Using quotactl() for setting / getting project ID is IMO a wrong fit.
quotactl() is used to manipulate quota usage & limits but not file
properties. And reusing XFS ioctl is IMHO better than inventing a new
ioctl - ext4 can use the same ioctl as XFS just fine. It's only that Li Xi
mixed in unrelated changes to the ext4 support for that ioctl.
> Also: is quota for project-id '0' really required for something?
> It adds overhead but I don't see any use-cases for that.
But only if filesystem has project quota feature enabled, no? That
doesn't concern me too much since the overhead doesn't seem to big and when
you enable project quotas you likely want to use them ;-). But if you are
concerned, I'm not strictly opposed to special casing of project id 0. But
I'd like to see how much speed you gain by that before complicating the
code...
Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply
* Re: [v8 4/5] ext4: adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface support
From: Konstantin Khlebnikov @ 2015-01-22 18:35 UTC (permalink / raw)
To: Jan Kara
Cc: Konstantin Khlebnikov, Dave Chinner, Li Xi, linux-fsdevel,
linux-ext4, Linux API, tytso, adilger, Al Viro, hch,
Дмитрий Монахов
In-Reply-To: <20150122155900.GB3062@quack.suse.cz>
On Thu, Jan 22, 2015 at 6:59 PM, Jan Kara <jack@suse.cz> wrote:
> On Thu 22-01-15 18:20:15, Konstantin Khlebnikov wrote:
>> On 10.12.2014 01:57, Dave Chinner wrote:
>> >On Tue, Dec 09, 2014 at 01:22:27PM +0800, Li Xi wrote:
>> >>This patch adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR ioctl interface
>> >>support for ext4. The interface is kept consistent with
>> >>XFS_IOC_FSGETXATTR/XFS_IOC_FSGETXATTR.
>> >>
>> >>Signed-off-by: Li Xi <lixi@ddn.com>
>> >>---
>> >> fs/ext4/ext4.h | 111 ++++++++++++++++
>> >> fs/ext4/ioctl.c | 330 +++++++++++++++++++++++++++++++++--------------
>> >> fs/xfs/xfs_fs.h | 47 +++-----
>> >> include/uapi/linux/fs.h | 58 ++++++++
>> >> 4 files changed, 418 insertions(+), 128 deletions(-)
>> >>
>> >>diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>> >>index 136e18c..43a2a88 100644
>> >>--- a/fs/ext4/ext4.h
>> >>+++ b/fs/ext4/ext4.h
>> >>@@ -384,6 +384,115 @@ struct flex_groups {
>> >> #define EXT4_FL_USER_VISIBLE 0x204BDFFF /* User visible flags */
>> >> #define EXT4_FL_USER_MODIFIABLE 0x204380FF /* User modifiable flags */
>> >>
>> >>+/* Transfer internal flags to xflags */
>> >>+static inline __u32 ext4_iflags_to_xflags(unsigned long iflags)
>> >>+{
>> >>+ __u32 xflags = 0;
>> >>+
>> >>+ if (iflags & EXT4_SECRM_FL)
>> >>+ xflags |= FS_XFLAG_SECRM;
>> >>+ if (iflags & EXT4_UNRM_FL)
>> >>+ xflags |= FS_XFLAG_UNRM;
>> >>+ if (iflags & EXT4_COMPR_FL)
>> >>+ xflags |= FS_XFLAG_COMPR;
>> >....
>> >
>> >NACK.
>> >
>> >>+ if (iflags & EXT4_SYNC_FL)
>> >>+ xflags |= FS_XFLAG_SYNC;
>> >>+ if (iflags & EXT4_IMMUTABLE_FL)
>> >>+ xflags |= FS_XFLAG_IMMUTABLE;
>> >>+ if (iflags & EXT4_APPEND_FL)
>> >>+ xflags |= FS_XFLAG_APPEND;
>> >>+ if (iflags & EXT4_NODUMP_FL)
>> >>+ xflags |= FS_XFLAG_NODUMP;
>> >>+ if (iflags & EXT4_NOATIME_FL)
>> >>+ xflags |= FS_XFLAG_NOATIME;
>> >
>> >These are OK because they already exist in the interface, but all
>> >the ext4 specific flags you've added have no place in this patchset.
>> >
>> >
>> >>+
>> >> /* Flags that should be inherited by new inodes from their parent. */
>> >> #define EXT4_FL_INHERITED (EXT4_SECRM_FL | EXT4_UNRM_FL | EXT4_COMPR_FL |\
>> >> EXT4_SYNC_FL | EXT4_NODUMP_FL | EXT4_NOATIME_FL |\
>> >>@@ -606,6 +715,8 @@ enum {
>> >> #define EXT4_IOC_RESIZE_FS _IOW('f', 16, __u64)
>> >> #define EXT4_IOC_SWAP_BOOT _IO('f', 17)
>> >> #define EXT4_IOC_PRECACHE_EXTENTS _IO('f', 18)
>> >>+#define EXT4_IOC_FSGETXATTR FS_IOC_FSGETXATTR
>> >>+#define EXT4_IOC_FSSETXATTR FS_IOC_FSSETXATTR
>> >
>> >NACK. Userspace only needs FS_IOC_FS[GS]ETXATTR so that it works across
>> >all filesystems. We need to retain XFS_IOC_FS[GS]ETXATTR so we
>> >don't break existing userspace tools, but we do not need new
>> >filesystem specific definitions anywhere.
>> >
>> >>diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
>> >>index fcbf647..872fed5 100644
>> >>--- a/include/uapi/linux/fs.h
>> >>+++ b/include/uapi/linux/fs.h
>> >>@@ -58,6 +58,62 @@ struct inodes_stat_t {
>> >> long dummy[5]; /* padding for sysctl ABI compatibility */
>> >> };
>> >>
>> >>+/*
>> >>+ * Extend attribute flags. These should be or-ed together to figure out what
>> >>+ * is valid.
>> >>+ */
>> >>+#define FSX_XFLAGS (1 << 0)
>> >>+#define FSX_EXTSIZE (1 << 1)
>> >>+#define FSX_NEXTENTS (1 << 2)
>> >>+#define FSX_PROJID (1 << 3)
>> >
>> >NACK.
>> >
>> >I've said this more than once: these are *private to XFS's
>> >implementation* and are not be part of the user interface. Do not
>> >move them from their existing location.
>> >
>> >
>> >>+
>> >>+/*
>> >>+ * Structure for FS_IOC_FSGETXATTR and FS_IOC_FSSETXATTR.
>> >>+ */
>> >>+struct fsxattr {
>> >>+ __u32 fsx_xflags; /* xflags field value (get/set) */
>> >>+ __u32 fsx_extsize; /* extsize field value (get/set)*/
>> >>+ __u32 fsx_nextents; /* nextents field value (get) */
>> >>+ __u32 fsx_projid; /* project identifier (get/set) */
>> >>+ unsigned char fsx_pad[12];
>> >>+};
>> >>+
>> >>+/*
>> >>+ * Flags for the fsx_xflags field
>> >>+ */
>> >>+#define FS_XFLAG_REALTIME 0x00000001 /* data in realtime volume */
>> >>+#define FS_XFLAG_PREALLOC 0x00000002 /* preallocated file extents */
>> >>+#define FS_XFLAG_SECRM 0x00000004 /* secure deletion */
>> >
>> >NACK - ext4 specific.
>> >
>> >>+#define FS_XFLAG_IMMUTABLE 0x00000008 /* file cannot be modified */
>> >>+#define FS_XFLAG_APPEND 0x00000010 /* all writes append */
>> >>+#define FS_XFLAG_SYNC 0x00000020 /* all writes synchronous */
>> >>+#define FS_XFLAG_NOATIME 0x00000040 /* do not update access time */
>> >>+#define FS_XFLAG_NODUMP 0x00000080 /* do not include in backups */
>> >>+#define FS_XFLAG_RTINHERIT 0x00000100 /* create with rt bit set */
>> >>+#define FS_XFLAG_PROJINHERIT 0x00000200 /* create with parents projid */
>> >>+#define FS_XFLAG_NOSYMLINKS 0x00000400 /* disallow symlink creation */
>> >>+#define FS_XFLAG_EXTSIZE 0x00000800 /* extent size allocator hint */
>> >>+#define FS_XFLAG_EXTSZINHERIT 0x00001000 /* inherit inode extent size */
>> >>+#define FS_XFLAG_NODEFRAG 0x00002000 /* do not defragment */
>> >>+#define FS_XFLAG_FILESTREAM 0x00004000 /* use filestream allocator */
>> >
>> >existing flags.
>> >
>> >>+#define FS_XFLAG_UNRM 0x00008000 /* undelete */
>> >>+#define FS_XFLAG_COMPR 0x00010000 /* compress file */
>> >>+#define FS_XFLAG_COMPRBLK 0x00020000 /* one or more compressed clusters */
>> >>+#define FS_XFLAG_NOCOMPR 0x00040000 /* don't compress */
>> >>+#define FS_XFLAG_ECOMPR 0x00080000 /* compression error */
>> >>+#define FS_XFLAG_INDEX 0x00100000 /* hash-indexed directory */
>> >>+#define FS_XFLAG_IMAGIC 0x00200000 /* AFS directory */
>> >>+#define FS_XFLAG_JOURNAL_DATA 0x00400000 /* file data should be journaled */
>> >>+#define FS_XFLAG_NOTAIL 0x00800000 /* file tail should not be merged */
>> >>+#define FS_XFLAG_DIRSYNC 0x01000000 /* dirsync behaviour (directories only) */
>> >>+#define FS_XFLAG_TOPDIR 0x02000000 /* top of directory hierarchies*/
>> >>+#define FS_XFLAG_HUGE_FILE 0x04000000 /* set to each huge file */
>> >>+#define FS_XFLAG_EXTENTS 0x08000000 /* inode uses extents */
>> >>+#define FS_XFLAG_EA_INODE 0x10000000 /* inode used for large EA */
>> >>+#define FS_XFLAG_EOFBLOCKS 0x20000000 /* blocks allocated beyond EOF */
>> >>+#define FS_XFLAG_INLINE_DATA 0x40000000 /* inode has inline data. */
>> >
>> >And a bunch more ext4 specific flags that *uses all the remaining
>> >flag space*. At minimum, we need to keep space in this existing
>> >flags field for flags to future indication of how the padding is
>> >used, so that's yet another NACK.
>> >
>> >Further, none of these have any relevance to project quotas so
>> >should not be a part of this patchset. Nor are they relevant to any
>> >other filesystem, and many are duplicated by information you can get
>> >from FIEMAP and other interfaces. NACK, again.
>> >
>> >Because I'm getting annoyed at being repeatedly ignored about what
>> >needs to be done here, I'm now going to shout a bit. DO NOT CHANGE
>> >THE INTERFACE. DO NOT ADD any new flags to the interface. DO NOT
>> >REDEFINE how the interface works. DO NOT "ext4-ise" the interface.
>> >
>> >The only changes to the interface code should be moving the XFS
>> >definitions and renaming them so as to provide the new generic
>> >ioctl definition as well as the historic XFS definitions. The ext4
>> >implementation needs to be done in a separate patch to the interface
>> >rename, and it must only implement the functionality the interface
>> >already provides. Anything else is outside the scope of this
>> >patchset and requires separate discussion.
>>
>> What reason for reusing XFS ioctl?
>>
>> As I see quota tools from xfsprogs checks filesystem name and seems
>> like they wouldn't work without upgrade. e2fsprogs have to be updated
>> updated anyway to support changes in layout. So, in this case we could
>> design new generic ioctl/syscall interface for that. For example add
>> new commands to quotactl() instead of yet another obscure ioctl.
> Using quotactl() for setting / getting project ID is IMO a wrong fit.
> quotactl() is used to manipulate quota usage & limits but not file
> properties. And reusing XFS ioctl is IMHO better than inventing a new
> ioctl - ext4 can use the same ioctl as XFS just fine. It's only that Li Xi
> mixed in unrelated changes to the ext4 support for that ioctl.
XFS interface looks really strange for that:
struct fsxattr {
__u32 fsx_xflags; /* xflags field value (get/set) */
__u32 fsx_extsize; /* extsize field value (get/set)*/
__u32 fsx_nextents; /* nextents field value (get) */
__u32 fsx_projid; /* project identifier (get/set) */
unsigned char fsx_pad[12];
};
Where we need only one flag and one field, everything else is just a legacy.
Moreover that flag: FS_PROJINHERIT_FL is redundant. For files it's meaningless.
For directories only difference between clearing it and changing
project_id to zero is
in accounting directory itself into inode/space quota. I'm not sure if there any
use-case for accounting directory itself but not charging its content.
Well. Maybe pair of fcntl()? fcntl(fd, F_GET_PROJECT/F_SET_PROJECT, ...);
>
>> Also: is quota for project-id '0' really required for something?
>> It adds overhead but I don't see any use-cases for that.
> But only if filesystem has project quota feature enabled, no? That
> doesn't concern me too much since the overhead doesn't seem to big and when
> you enable project quotas you likely want to use them ;-). But if you are
> concerned, I'm not strictly opposed to special casing of project id 0. But
> I'd like to see how much speed you gain by that before complicating the
> code...
For non-journalled quota it's nothing but for journalled difference
might be significant.
This might be implemented without any complication as a fake inactive
quota block
which just plugs particular id.
>
> Honza
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH RFC 5/6] epoll: Add implementation for epoll_mod_wait
From: Andy Lutomirski @ 2015-01-22 21:12 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Fam Zheng, Michael Kerrisk (man-pages),
linux-kernel@vger.kernel.org, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, X86 ML, Alexander Viro, Andrew Morton, Kees Cook,
David Herrmann, Alexei Starovoitov, Miklos Szeredi,
David Drysdale, Oleg Nesterov, David S. Miller, Vivek Goyal,
Mike Frysinger, Theodore Ts'o, Heiko Carstens,
Rasmus Villemoes, Rashika Kheria, Hugh Dickins <hugh>
In-Reply-To: <54BF9294.3070902@redhat.com>
On Wed, Jan 21, 2015 at 3:50 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 21/01/2015 12:14, Fam Zheng wrote:
>> > My take for simplicity will be leaving epoll_ctl as-is, and my take for
>> > performance will be epoll_pwait1. And I don't really like putting my time on
>> > epoll_ctl_batch, thinking it as a ambivalent compromise in between.
>>
>> > I agree with Michael actually. The big change is going from O(n)
>> > epoll_ctl calls to O(1), and epoll_ctl_batch achieves that just fine.
>> > Changing 2 syscalls to 1 is the icing on the cake, but we're talking of
>> > a fraction of a microsecond.
>>
>> Maybe I'm missing something, but in common cases, the set of fds for epoll_wait
>> doesn't change that radically from one iteration to another, does it?
>
> That depends on the application.
In my application, the set of fds almost never changes, but the set of
events I want changes all the time. The main thing that changes is
whether I care about EPOLLOUT. If I'm ready to send something, then I
want EPOLLOUT. If I'm not ready, then I don't want EPOLLOUT.
--Andy
^ permalink raw reply
* Re: [v8 4/5] ext4: adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface support
From: Dave Chinner @ 2015-01-23 1:39 UTC (permalink / raw)
To: Jan Kara
Cc: Konstantin Khlebnikov, Li Xi,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-ext4-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, tytso-3s7WtUTddSA,
adilger-m1MBpc4rdrD3fQ9qLvQP4Q,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, hch-wEGCiKHe2LqWVfeAwA7xHQ,
dmonakhov-GEFAQzZX7r8dnm+yROfE0A
In-Reply-To: <20150122155900.GB3062-+0h/O2h83AeN3ZZ/Hiejyg@public.gmane.org>
On Thu, Jan 22, 2015 at 04:59:00PM +0100, Jan Kara wrote:
> On Thu 22-01-15 18:20:15, Konstantin Khlebnikov wrote:
> > Also: is quota for project-id '0' really required for something?
> > It adds overhead but I don't see any use-cases for that.
> But only if filesystem has project quota feature enabled, no? That
> doesn't concern me too much since the overhead doesn't seem to big and when
> you enable project quotas you likely want to use them ;-). But if you are
> concerned, I'm not strictly opposed to special casing of project id 0. But
> I'd like to see how much speed you gain by that before complicating the
> code...
Except that doing so will result in different behaviour between
filesystems. XFS always *accounts* inodes when quotas are enabled,
but does not allow limits to be placed on quota ID 0. Hence projid
= 0 accounts all the space not used by other project groups so
admins can easily see how much uncontrolled space is being used.
I've had admins tell me this is one of the features they liked most
about using project quotas because you can't hide from it, even as
root....
Cheers,
Dave.
--
Dave Chinner
david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org
^ permalink raw reply
* Re: [v8 4/5] ext4: adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface support
From: Dave Chinner @ 2015-01-23 1:53 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: Li Xi, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-ext4-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, tytso-3s7WtUTddSA,
adilger-m1MBpc4rdrD3fQ9qLvQP4Q, jack-AlSwsSmVLrQ,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, hch-wEGCiKHe2LqWVfeAwA7xHQ,
dmonakhov-GEFAQzZX7r8dnm+yROfE0A
In-Reply-To: <54C11733.7080801-XoJtRXgx1JseBXzfvpsJ4g@public.gmane.org>
On Thu, Jan 22, 2015 at 06:28:51PM +0300, Konstantin Khlebnikov wrote:
> On 09.12.2014 08:22, Li Xi wrote:
> >+static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
> >+{
> >+ struct inode *inode = file_inode(filp);
> >+ struct super_block *sb = inode->i_sb;
> >+ struct ext4_inode_info *ei = EXT4_I(inode);
> >+ int err;
> >+ handle_t *handle;
> >+ kprojid_t kprojid;
> >+ struct ext4_iloc iloc;
> >+ struct ext4_inode *raw_inode;
> >+
> >+ struct dquot *transfer_to[EXT4_MAXQUOTAS] = { };
> >+
> >+ /* Make sure caller can change project. */
> >+ if (!capable(CAP_SYS_ADMIN))
> >+ return -EACCES;
> >+
> >+ if (projid != EXT4_DEF_PROJID
> >+ && !EXT4_HAS_RO_COMPAT_FEATURE(sb,
> >+ EXT4_FEATURE_RO_COMPAT_PROJECT))
> >+ return -EOPNOTSUPP;
> >+
> >+ if (!EXT4_HAS_RO_COMPAT_FEATURE(sb,
> >+ EXT4_FEATURE_RO_COMPAT_PROJECT)) {
> >+ BUG_ON(__kprojid_val(EXT4_I(inode)->i_projid)
> >+ != EXT4_DEF_PROJID);
> >+ if (projid != EXT4_DEF_PROJID)
> >+ return -EOPNOTSUPP;
> >+ else
> >+ return 0;
> >+ }
> >+
> >+ kprojid = make_kprojid(&init_user_ns, (projid_t)projid);
>
> Maybe current_user_ns()?
> This code should be user-namespace aware from the beginning.
No, the code is correct. Project quotas have nothing to do with
UIDs and so should never have been included in the uid/gid
namespace mapping infrastructure in the first place.
Point in case: directory subtree quotas can be used as a resource
controller for limiting space usage within separate containers that
share the same underlying (large) filesystem via mount namespaces.
Cheers,
Dave.
PS: can you please trim your reply to just the sections of the
patch you are commenting to? Finding replies like this in a large
patch is like searching for a needle in a haystack...
--
Dave Chinner
david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org
^ permalink raw reply
* Re: [PATCH v6 2/2] tty/serial: Add Spreadtrum sc9836-uart driver support
From: Peter Hurley @ 2015-01-23 3:57 UTC (permalink / raw)
To: Chunyan Zhang, gregkh, robh+dt
Cc: mark.rutland, arnd, gnomes, shawn.guo, pawel.moll, ijc+devicetree,
galak, jslaby, grant.likely, heiko, jason, florian.vaussard,
andrew, hytszk, antonynpavlov, orsonzhai, geng.ren, zhizhou.zhang,
lanqing.liu, zhang.lyra, wei.qiao, devicetree, linux-kernel,
linux-serial, linux-api
In-Reply-To: <1421933706-4277-3-git-send-email-chunyan.zhang@spreadtrum.com>
Hi Chunyan,
On 01/22/2015 08:35 AM, Chunyan Zhang wrote:
> Add a full sc9836-uart driver for SC9836 SoC which is based on the
> spreadtrum sharkl64 platform.
> This driver also support earlycon.
Looking good. Comments below.
> This patch also replaced the spaces between the macros and their
> values with the tabs in serial_core.h
Notes about patch changes goes...
>
> Originally-by: Lanqing Liu <lanqing.liu@spreadtrum.com>
> Signed-off-by: Orson Zhai <orson.zhai@spreadtrum.com>
> Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
> ---
...here. For example,
* Replaced spaces with tab for PORT_SPRD define in serial_core.h
> drivers/tty/serial/Kconfig | 18 +
> drivers/tty/serial/Makefile | 1 +
> drivers/tty/serial/sprd_serial.c | 801 ++++++++++++++++++++++++++++++++++++++
> include/uapi/linux/serial_core.h | 3 +
> 4 files changed, 823 insertions(+)
> create mode 100644 drivers/tty/serial/sprd_serial.c
>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index c79b43c..13211f7 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1577,6 +1577,24 @@ config SERIAL_MEN_Z135
> This driver can also be build as a module. If so, the module will be called
> men_z135_uart.ko
>
> +config SERIAL_SPRD
> + tristate "Support for Spreadtrum serial"
> + depends on ARCH_SPRD
> + select SERIAL_CORE
> + help
> + This enables the driver for the Spreadtrum's serial.
> +
> +config SERIAL_SPRD_CONSOLE
> + bool "Spreadtrum UART console support"
> + depends on SERIAL_SPRD=y
> + select SERIAL_CORE_CONSOLE
> + select SERIAL_EARLYCON
> + help
> + Support for early debug console using Spreadtrum's serial. This enables
> + the console before standard serial driver is probed. This is enabled
> + with "earlycon" on the kernel command line. The console is
> + enabled when early_param is processed.
> +
> endmenu
>
> config SERIAL_MCTRL_GPIO
> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> index 9a548ac..4801aca 100644
> --- a/drivers/tty/serial/Makefile
> +++ b/drivers/tty/serial/Makefile
> @@ -93,6 +93,7 @@ obj-$(CONFIG_SERIAL_ARC) += arc_uart.o
> obj-$(CONFIG_SERIAL_RP2) += rp2.o
> obj-$(CONFIG_SERIAL_FSL_LPUART) += fsl_lpuart.o
> obj-$(CONFIG_SERIAL_MEN_Z135) += men_z135_uart.o
> +obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
>
> # GPIOLIB helpers for modem control lines
> obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.o
> diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
> new file mode 100644
> index 0000000..fd98541
> --- /dev/null
> +++ b/drivers/tty/serial/sprd_serial.c
> @@ -0,0 +1,801 @@
> +/*
> + * Copyright (C) 2012-2015 Spreadtrum Communications Inc.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * 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.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/console.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/serial_core.h>
> +#include <linux/serial.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +#include <linux/tty_flip.h>
> +
> +/* device name */
> +#define UART_NR_MAX 8
> +#define SPRD_TTY_NAME "ttyS"
> +#define SPRD_FIFO_SIZE 128
> +#define SPRD_DEF_RATE 26000000
> +#define SPRD_TIMEOUT 2048
> +
> +/* the offset of serial registers and BITs for them */
> +/* data registers */
> +#define SPRD_TXD 0x0000
> +#define SPRD_RXD 0x0004
> +
> +/* line status register and its BITs */
> +#define SPRD_LSR 0x0008
> +#define SPRD_LSR_OE BIT(4)
> +#define SPRD_LSR_FE BIT(3)
> +#define SPRD_LSR_PE BIT(2)
> +#define SPRD_LSR_BI BIT(7)
> +#define SPRD_LSR_TX_OVER BIT(15)
> +
> +/* data number in TX and RX fifo */
> +#define SPRD_STS1 0x000C
> +
> +/* interrupt enable register and its BITs */
> +#define SPRD_IEN 0x0010
> +#define SPRD_IEN_RX_FULL BIT(0)
> +#define SPRD_IEN_TX_EMPTY BIT(1)
> +#define SPRD_IEN_BREAK_DETECT BIT(7)
> +#define SPRD_IEN_TIMEOUT BIT(13)
> +
> +/* interrupt clear register */
> +#define SPRD_ICLR 0x0014
> +
> +/* line control register */
> +#define SPRD_LCR 0x0018
> +#define SPRD_LCR_STOP_1BIT 0x10
> +#define SPRD_LCR_STOP_2BIT 0x30
> +#define SPRD_LCR_DATA_LEN (BIT(2) | BIT(3))
> +#define SPRD_LCR_DATA_LEN5 0x0
> +#define SPRD_LCR_DATA_LEN6 0x4
> +#define SPRD_LCR_DATA_LEN7 0x8
> +#define SPRD_LCR_DATA_LEN8 0xc
> +#define SPRD_LCR_PARITY (BIT(0) | BIT(1))
> +#define SPRD_LCR_PARITY_EN 0x2
> +#define SPRD_LCR_EVEN_PAR 0x0
> +#define SPRD_LCR_ODD_PAR 0x1
> +
> +/* control register 1 */
> +#define SPRD_CTL1 0x001C
> +#define RX_HW_FLOW_CTL_THLD BIT(6)
> +#define RX_HW_FLOW_CTL_EN BIT(7)
> +#define TX_HW_FLOW_CTL_EN BIT(8)
> +
> +/* fifo threshold register */
> +#define SPRD_CTL2 0x0020
> +#define THLD_TX_EMPTY 0x40
> +#define THLD_RX_FULL 0x40
> +
> +/* config baud rate register */
> +#define SPRD_CLKD0 0x0024
> +#define SPRD_CLKD1 0x0028
> +
> +/* interrupt mask status register */
> +#define SPRD_IMSR 0x002C
> +#define SPRD_IMSR_RX_FIFO_FULL BIT(0)
> +#define SPRD_IMSR_TX_FIFO_EMPTY BIT(1)
> +#define SPRD_IMSR_BREAK_DETECT BIT(7)
> +#define SPRD_IMSR_TIMEOUT BIT(13)
> +
> +struct reg_backup {
> + uint32_t ien;
u32 ien;
same for others
> + uint32_t ctrl0;
> + uint32_t ctrl1;
> + uint32_t ctrl2;
> + uint32_t clkd0;
> + uint32_t clkd1;
> + uint32_t dspwait;
> +};
> +
> +struct sprd_uart_port {
> + struct uart_port port;
> + struct reg_backup reg_bak;
> + char name[16];
> +};
> +
> +static struct sprd_uart_port *sprd_port[UART_NR_MAX] = { NULL };
^^^^^^^^^^
Don't need the initializer.
> +
> +static inline unsigned int serial_in(struct uart_port *port, int offset)
> +{
> + return readl_relaxed(port->membase + offset);
> +}
> +
> +static inline void serial_out(struct uart_port *port, int offset, int value)
> +{
> + writel_relaxed(value, port->membase + offset);
> +}
> +
> +static unsigned int sprd_tx_empty(struct uart_port *port)
> +{
> + if (serial_in(port, SPRD_STS1) & 0xff00)
> + return 0;
> + else
> + return TIOCSER_TEMT;
> +}
> +
> +static unsigned int sprd_get_mctrl(struct uart_port *port)
> +{
> + return TIOCM_DSR | TIOCM_CTS;
> +}
> +
> +static void sprd_set_mctrl(struct uart_port *port, unsigned int mctrl)
> +{
> + /* nothing to do */
> +}
> +
> +static void sprd_stop_tx(struct uart_port *port)
> +{
> + unsigned int ien, iclr;
> +
> + iclr = serial_in(port, SPRD_ICLR);
> + ien = serial_in(port, SPRD_IEN);
> +
> + iclr |= SPRD_IEN_TX_EMPTY;
> + ien &= ~SPRD_IEN_TX_EMPTY;
> +
> + serial_out(port, SPRD_ICLR, iclr);
> + serial_out(port, SPRD_IEN, ien);
> +}
> +
> +static void sprd_start_tx(struct uart_port *port)
> +{
> + unsigned int ien;
> +
> + ien = serial_in(port, SPRD_IEN);
> + if (!(ien & SPRD_IEN_TX_EMPTY)) {
> + ien |= SPRD_IEN_TX_EMPTY;
> + serial_out(port, SPRD_IEN, ien);
> + }
> +}
> +
> +static void sprd_stop_rx(struct uart_port *port)
> +{
> + unsigned int ien, iclr;
> +
> + iclr = serial_in(port, SPRD_ICLR);
> + ien = serial_in(port, SPRD_IEN);
> +
> + ien &= ~(SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT);
> + iclr |= SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT;
> +
> + serial_out(port, SPRD_IEN, ien);
> + serial_out(port, SPRD_ICLR, iclr);
> +}
> +
> +/* The Sprd serial does not support this function. */
> +static void sprd_break_ctl(struct uart_port *port, int break_state)
> +{
> + /* nothing to do */
> +}
> +
> +static inline int handle_lsr_errors(struct uart_port *port,
> + unsigned int *flag,
> + unsigned int *lsr)
Don't declare this inline. gcc will inline single call site
functions.
> +{
> + int ret = 0;
> +
> + /* statistics */
> + if (*lsr & SPRD_LSR_BI) {
> + *lsr &= ~(SPRD_LSR_FE | SPRD_LSR_PE);
> + port->icount.brk++;
> + ret = uart_handle_break(port);
> + if (ret)
> + return ret;
> + } else if (*lsr & SPRD_LSR_PE)
> + port->icount.parity++;
> + else if (*lsr & SPRD_LSR_FE)
> + port->icount.frame++;
> + if (*lsr & SPRD_LSR_OE)
> + port->icount.overrun++;
> +
> + /* mask off conditions which should be ignored */
> + *lsr &= port->read_status_mask;
> + if (*lsr & SPRD_LSR_BI)
> + *flag = TTY_BREAK;
> + else if (*lsr & SPRD_LSR_PE)
> + *flag = TTY_PARITY;
> + else if (*lsr & SPRD_LSR_FE)
> + *flag = TTY_FRAME;
> +
> + return ret;
> +}
> +
> +static inline void sprd_rx(int irq, void *dev_id)
^^^^^^^^^^^^
struct uart_port *port
The 'irq' parameter is unused, remove it.
Don't declare inline.
> +{
> + struct uart_port *port = dev_id;
^^^^^^^^^
delete this line.
> + struct tty_port *tty = &port->state->port;
> + unsigned int ch, flag, lsr, max_count = SPRD_TIMEOUT;
^^^^^^^^^
== 2048?
That's a lot. Most (all?) other drivers use 256.
> +
> + while ((serial_in(port, SPRD_STS1) & 0x00ff) && max_count--) {
> + lsr = serial_in(port, SPRD_LSR);
> + ch = serial_in(port, SPRD_RXD);
> + flag = TTY_NORMAL;
> + port->icount.rx++;
> +
> + if (lsr & (SPRD_LSR_BI | SPRD_LSR_PE
> + | SPRD_LSR_FE | SPRD_LSR_OE))
operators should trail on the previous line, like
if (lsr & (SPRD_LSR_BI | SPRD_LSR_PE |
SPRD_LSR_FE | SPRD_LSR_OE))
> + if (handle_lsr_errors(port, &lsr, &flag))
> + continue;
> + if (uart_handle_sysrq_char(port, ch))
> + continue;
> +
> + uart_insert_char(port, lsr, SPRD_LSR_OE, ch, flag);
> + }
> +
> + tty_flip_buffer_push(tty);
> +}
> +
> +static inline void sprd_tx(int irq, void *dev_id)
^^^^^^^^^^^^^
struct uart_port *port
The 'irq' parameter is unused, remove it.
Don't declare inline.
> +{
> + struct uart_port *port = dev_id;
^^^^^^^^^
delete this line.
> + struct circ_buf *xmit = &port->state->xmit;
> + int count;
> +
> + if (port->x_char) {
> + serial_out(port, SPRD_TXD, port->x_char);
> + port->icount.tx++;
> + port->x_char = 0;
> + return;
> + }
> +
> + if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
> + sprd_stop_tx(port);
> + return;
> + }
> +
> + count = THLD_TX_EMPTY;
> + do {
> + serial_out(port, SPRD_TXD, xmit->buf[xmit->tail]);
> + xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
> + port->icount.tx++;
> + if (uart_circ_empty(xmit))
> + break;
> + } while (--count > 0);
> +
> + if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> + uart_write_wakeup(port);
> +
> + if (uart_circ_empty(xmit))
> + sprd_stop_tx(port);
> +}
> +
> +/* this handles the interrupt from one port */
> +static irqreturn_t sprd_handle_irq(int irq, void *dev_id)
> +{
> + struct uart_port *port = (struct uart_port *)dev_id;
^^^^^^^^^^^
Don't need the type cast.
> + unsigned int ims;
> +
> + spin_lock(&port->lock);
> +
> + ims = serial_in(port, SPRD_IMSR);
> +
> + if (!ims)
> + return IRQ_NONE;
> +
> + serial_out(port, SPRD_ICLR, ~0);
> +
> + if (ims & (SPRD_IMSR_RX_FIFO_FULL |
> + SPRD_IMSR_BREAK_DETECT | SPRD_IMSR_TIMEOUT))
> + sprd_rx(irq, port);
> +
> + if (ims & SPRD_IMSR_TX_FIFO_EMPTY)
> + sprd_tx(irq, port);
> +
> + spin_unlock(&port->lock);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int sprd_startup(struct uart_port *port)
> +{
> + int ret = 0;
> + unsigned int ien, ctrl1;
> + unsigned int timeout;
> + struct sprd_uart_port *sp;
unsigned long flags;
> +
> + serial_out(port, SPRD_CTL2, ((THLD_TX_EMPTY << 8) | THLD_RX_FULL));
> +
> + /* clear rx fifo */
> + timeout = SPRD_TIMEOUT;
> + while (timeout-- && serial_in(port, SPRD_STS1) & 0x00ff)
> + serial_in(port, SPRD_RXD);
> +
> + /* clear tx fifo */
> + timeout = SPRD_TIMEOUT;
> + while (timeout-- && serial_in(port, SPRD_STS1) & 0xff00)
> + cpu_relax();
> +
> + /* clear interrupt */
> + serial_out(port, SPRD_IEN, 0x0);
^^^
just 0
> + serial_out(port, SPRD_ICLR, ~0);
> +
> + /* allocate irq */
> + sp = container_of(port, struct sprd_uart_port, port);
> + snprintf(sp->name, sizeof(sp->name), "sprd_serial%d", port->line);
> + ret = devm_request_irq(port->dev, port->irq, sprd_handle_irq,
> + IRQF_SHARED, sp->name, port);
> + if (ret) {
> + dev_err(port->dev, "fail to request serial irq %d, ret=%d\n",
> + port->irq, ret);
> + return ret;
> + }
> + ctrl1 = serial_in(port, SPRD_CTL1);
> + ctrl1 |= 0x3e00 | THLD_RX_FULL;
^^^^^
What is this magic number?
> + serial_out(port, SPRD_CTL1, ctrl1);
> +
> + /* enable interrupt */
> + spin_lock(&port->lock);
spin_lock_irqsave(&port->lock, flags);
> + ien = serial_in(port, SPRD_IEN);
> + ien |= SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT | SPRD_IEN_TIMEOUT;
> + serial_out(port, SPRD_IEN, ien);
> + spin_unlock(&port->lock);
spin_unlock_irqrestore(&port->lock, flags);
> +
> + return 0;
> +}
> +
> +static void sprd_shutdown(struct uart_port *port)
> +{
> + serial_out(port, SPRD_IEN, 0x0);
^^^
just 0
> + serial_out(port, SPRD_ICLR, ~0);
> + devm_free_irq(port->dev, port->irq, port);
> +}
> +
> +static void sprd_set_termios(struct uart_port *port,
> + struct ktermios *termios,
> + struct ktermios *old)
> +{
> + unsigned int baud, quot;
> + unsigned int lcr, fc;
> + unsigned long flags;
> +
> + /* ask the core to calculate the divisor for us */
> + baud = uart_get_baud_rate(port, termios, old, 1200, 3000000);
^^^^ ^^^^^^
mabye derive these from uartclk?
> +
> + quot = (unsigned int)((port->uartclk + baud / 2) / baud);
> +
> + /* set data length */
> + lcr = serial_in(port, SPRD_LCR);
> + lcr &= ~SPRD_LCR_DATA_LEN;
What bits are being preserved in SPRD_LCR that you need to read the
current value? IOW, why can't SPRD_LCR be defined only by the termios
c_cflag? Like,
lcr = 0;
> + switch (termios->c_cflag & CSIZE) {
> + case CS5:
> + lcr |= SPRD_LCR_DATA_LEN5;
> + break;
> + case CS6:
> + lcr |= SPRD_LCR_DATA_LEN6;
> + break;
> + case CS7:
> + lcr |= SPRD_LCR_DATA_LEN7;
> + break;
> + case CS8:
> + default:
> + lcr |= SPRD_LCR_DATA_LEN8;
> + break;
> + }
> +
> + /* calculate stop bits */
> + lcr &= ~(SPRD_LCR_STOP_1BIT | SPRD_LCR_STOP_2BIT);
> + if (termios->c_cflag & CSTOPB)
> + lcr |= SPRD_LCR_STOP_2BIT;
> + else
> + lcr |= SPRD_LCR_STOP_1BIT;
> +
> + /* calculate parity */
> + lcr &= ~SPRD_LCR_PARITY;
> + termios->c_cflag &= ~CMSPAR; /* no support mark/space */
> + if (termios->c_cflag & PARENB) {
> + lcr |= SPRD_LCR_PARITY_EN;
> + if (termios->c_cflag & PARODD)
> + lcr |= SPRD_LCR_ODD_PAR;
> + else
> + lcr |= SPRD_LCR_EVEN_PAR;
> + }
> +
> + spin_lock_irqsave(&port->lock, flags);
> +
> + /* update the per-port timeout */
> + uart_update_timeout(port, termios->c_cflag, baud);
> +
> + port->read_status_mask = SPRD_LSR_OE;
> + if (termios->c_iflag & INPCK)
> + port->read_status_mask |= SPRD_LSR_FE | SPRD_LSR_PE;
> + if (termios->c_iflag & (BRKINT | PARMRK))
if (termios->c_iflag & (IGNBRK | BRKINT | PARMRK))
Because if IGNBRK is set and a BRK is received, sprd_rx() should operate
like this:
/* - RESULT - */
lsr = serial_in(SPRD_LSR) /* lsr = SPRD_LSR_BI */
ch = serial_in(SPRD_RXD) /* ch = 0 */
lsr & SPRD_LSR_BI ? yes
handle_lsr_errors(lsr, flag)
lsr &= read_status_mask /* lsr = SPRD_LSR_BI */
flag = TTY_BREAK
uart_insert_char(lsr, ch, flag)
lsr & ignore_status_mask == 0? no /* ch _not_ inserted */
> + port->read_status_mask |= SPRD_LSR_BI;
> +
> + /* characters to ignore */
> + port->ignore_status_mask = 0;
> + if (termios->c_iflag & IGNPAR)
> + port->ignore_status_mask |= SPRD_LSR_PE | SPRD_LSR_FE;
> + if (termios->c_iflag & IGNBRK) {
> + port->ignore_status_mask |= SPRD_LSR_BI;
> + /*
> + * If we're ignoring parity and break indicators,
> + * ignore overruns too (for real raw support).
> + */
> + if (termios->c_iflag & IGNPAR)
> + port->ignore_status_mask |= SPRD_LSR_OE;
> + }
> +
> + /* flow control */
> + fc = serial_in(port, SPRD_CTL1);
> + fc &= ~(RX_HW_FLOW_CTL_THLD | RX_HW_FLOW_CTL_EN | TX_HW_FLOW_CTL_EN);
> + if (termios->c_cflag & CRTSCTS) {
> + fc |= RX_HW_FLOW_CTL_THLD;
> + fc |= RX_HW_FLOW_CTL_EN;
> + fc |= TX_HW_FLOW_CTL_EN;
> + }
> +
> + /* clock divider bit0~bit15 */
> + serial_out(port, SPRD_CLKD0, quot & 0xffff);
> +
> + /* clock divider bit16~bit20 */
> + serial_out(port, SPRD_CLKD1, (quot & 0x1f0000) >> 16);
> + serial_out(port, SPRD_LCR, lcr);
> + fc |= 0x3e00 | THLD_RX_FULL;
> + serial_out(port, SPRD_CTL1, fc);
> +
> + spin_unlock_irqrestore(&port->lock, flags);
> +
> + /* Don't rewrite B0 */
> + if (tty_termios_baud_rate(termios))
> + tty_termios_encode_baud_rate(termios, baud, baud);
> +}
> +
> +static const char *sprd_type(struct uart_port *port)
> +{
> + return "SPX";
> +}
> +
> +static void sprd_release_port(struct uart_port *port)
> +{
> + /* nothing to do */
> +}
> +
> +static int sprd_request_port(struct uart_port *port)
> +{
> + return 0;
> +}
> +
> +static void sprd_config_port(struct uart_port *port, int flags)
> +{
> + if (flags & UART_CONFIG_TYPE)
> + port->type = PORT_SPRD;
> +}
> +
> +static int sprd_verify_port(struct uart_port *port,
> + struct serial_struct *ser)
> +{
> + if (ser->type != PORT_SPRD)
> + return -EINVAL;
> + if (port->irq != ser->irq)
> + return -EINVAL;
> + return 0;
> +}
> +
> +static struct uart_ops serial_sprd_ops = {
> + .tx_empty = sprd_tx_empty,
> + .get_mctrl = sprd_get_mctrl,
> + .set_mctrl = sprd_set_mctrl,
> + .stop_tx = sprd_stop_tx,
> + .start_tx = sprd_start_tx,
> + .stop_rx = sprd_stop_rx,
> + .break_ctl = sprd_break_ctl,
> + .startup = sprd_startup,
> + .shutdown = sprd_shutdown,
> + .set_termios = sprd_set_termios,
> + .type = sprd_type,
> + .release_port = sprd_release_port,
> + .request_port = sprd_request_port,
> + .config_port = sprd_config_port,
> + .verify_port = sprd_verify_port,
> +};
> +
> +#ifdef CONFIG_SERIAL_SPRD_CONSOLE
> +static inline void wait_for_xmitr(struct uart_port *port)
> +{
> + unsigned int status, tmout = 10000;
> +
> + /* wait up to 10ms for the character(s) to be sent */
> + do {
> + status = serial_in(port, SPRD_STS1);
> + if (--tmout == 0)
> + break;
> + udelay(1);
> + } while (status & 0xff00);
> +}
> +
> +static void sprd_console_putchar(struct uart_port *port, int ch)
> +{
> + wait_for_xmitr(port);
> + serial_out(port, SPRD_TXD, ch);
> +}
> +
> +static void sprd_console_write(struct console *co, const char *s,
> + unsigned int count)
> +{
> + struct uart_port *port = &sprd_port[co->index]->port;
> + int locked = 1;
> + unsigned long flags;
> +
> + if (oops_in_progress)
> + locked = spin_trylock(&port->lock);
if (port->sysrq)
locked = 0;
else if (oops_in_progress)
locked = spin_trylock_irqsave(&port->lock, flags);
> + else
> + spin_lock_irqsave(&port->lock, flags);
> +
> + uart_console_write(port, s, count, sprd_console_putchar);
> +
> + /* wait for transmitter to become empty */
> + wait_for_xmitr(port);
> +
> + if (locked)
> + spin_unlock_irqrestore(&port->lock, flags);
> +}
> +
> +static int __init sprd_console_setup(struct console *co, char *options)
> +{
> + struct uart_port *port;
> + int baud = 115200;
> + int bits = 8;
> + int parity = 'n';
> + int flow = 'n';
> +
> + if (co->index >= UART_NR_MAX || co->index < 0)
> + co->index = 0;
> +
> + port = &sprd_port[co->index]->port;
> + if (port == NULL) {
> + pr_info("serial port %d not yet initialized\n", co->index);
> + return -ENODEV;
> + }
> + if (options)
> + uart_parse_options(options, &baud, &parity, &bits, &flow);
> +
> + return uart_set_options(port, co, baud, parity, bits, flow);
> +}
> +
> +static struct uart_driver sprd_uart_driver;
> +static struct console sprd_console = {
> + .name = SPRD_TTY_NAME,
> + .write = sprd_console_write,
> + .device = uart_console_device,
> + .setup = sprd_console_setup,
> + .flags = CON_PRINTBUFFER,
> + .index = -1,
> + .data = &sprd_uart_driver,
> +};
> +
> +#define SPRD_CONSOLE (&sprd_console)
> +
> +/* Support for earlycon */
> +static void sprd_putc(struct uart_port *port, int c)
> +{
> + unsigned int timeout = SPRD_TIMEOUT;
> +
> + while (timeout-- &&
> + !(readl(port->membase + SPRD_LSR) & SPRD_LSR_TX_OVER))
> + cpu_relax();
> +
> + writeb(c, port->membase + SPRD_TXD);
> +}
> +
> +static void sprd_early_write(struct console *con, const char *s,
> + unsigned n)
> +{
> + struct earlycon_device *dev = con->data;
> +
> + uart_console_write(&dev->port, s, n, sprd_putc);
> +}
> +
> +static int __init sprd_early_console_setup(
> + struct earlycon_device *device,
> + const char *opt)
> +{
> + if (!device->port.membase)
> + return -ENODEV;
> +
> + device->con->write = sprd_early_write;
> + return 0;
> +}
> +
> +EARLYCON_DECLARE(sprd_serial, sprd_early_console_setup);
> +OF_EARLYCON_DECLARE(sprd_serial, "sprd,sc9836-uart",
> + sprd_early_console_setup);
> +
> +#else /* !CONFIG_SERIAL_SPRD_CONSOLE */
> +#define SPRD_CONSOLE NULL
> +#endif
> +
> +static struct uart_driver sprd_uart_driver = {
> + .owner = THIS_MODULE,
> + .driver_name = "sprd_serial",
> + .dev_name = SPRD_TTY_NAME,
> + .major = 0,
> + .minor = 0,
> + .nr = UART_NR_MAX,
> + .cons = SPRD_CONSOLE,
> +};
> +
> +static int sprd_probe_dt_alias(int index, struct device *dev)
> +{
> + struct device_node *np;
> + static bool seen_dev_with_alias;
> + static bool seen_dev_without_alias;
> + int ret = index;
> +
> + if (!IS_ENABLED(CONFIG_OF))
> + return ret;
> +
> + np = dev->of_node;
> + if (!np)
> + return ret;
> +
> + ret = of_alias_get_id(np, "serial");
> + if (IS_ERR_VALUE(ret)) {
> + seen_dev_without_alias = true;
> + ret = index;
> + } else {
> + seen_dev_with_alias = true;
> + if (ret >= ARRAY_SIZE(sprd_port) || sprd_port[ret] != NULL) {
> + dev_warn(dev, "requested serial port %d not available.\n", ret);
> + ret = index;
> + }
> + }
> +
> + if (seen_dev_with_alias && seen_dev_without_alias)
> + dev_warn(dev, "aliased and non-aliased serial devices found in device tree. Serial port enumeration may be unpredictable.\n");
Is this necessary? Why does a user want to know this?
> +
> + return ret;
> +}
> +
> +static int sprd_remove(struct platform_device *dev)
> +{
> + struct sprd_uart_port *sup = platform_get_drvdata(dev);
> + bool busy = false;
> + int i;
> +
> + if (sup)
> + uart_remove_one_port(&sprd_uart_driver, &sup->port);
see comment in sprd_probe()
if (sup) {
uart_remove_one_port();
sprd_port[sup->line] = NULL;
sprd_ports--;
}
if (!sprd_ports)
uart_unregister_driver();
> +
> + for (i = 0; i < ARRAY_SIZE(sprd_port); i++)
> + if (sprd_port[i] == sup)
> + sprd_port[i] = NULL;
> + else if (sprd_port[i])
> + busy = true;
> +
> + if (!busy)
> + uart_unregister_driver(&sprd_uart_driver);
> +
> + return 0;
> +}
> +
> +static int sprd_probe(struct platform_device *pdev)
> +{
> + struct resource *res;
> + struct uart_port *up;
> + struct clk *clk;
> + int irq;
> + int index;
> + int ret;
> +
> + for (index = 0; index < ARRAY_SIZE(sprd_port); index++)
> + if (sprd_port[index] == NULL)
> + break;
> +
> + if (index == ARRAY_SIZE(sprd_port))
> + return -EBUSY;
> +
> + index = sprd_probe_dt_alias(index, &pdev->dev);
> +
> + sprd_port[index] = devm_kzalloc(&pdev->dev,
> + sizeof(*sprd_port[index]), GFP_KERNEL);
> + if (!sprd_port[index])
> + return -ENOMEM;
> +
> + up = &sprd_port[index]->port;
> + up->dev = &pdev->dev;
> + up->line = index;
> + up->type = PORT_SPRD;
> + up->iotype = SERIAL_IO_PORT;
> + up->uartclk = SPRD_DEF_RATE;
> + up->fifosize = SPRD_FIFO_SIZE;
> + up->ops = &serial_sprd_ops;
> + up->flags = ASYNC_BOOT_AUTOCONF;
^^^^^^^^^
UPF_BOOT_AUTOCONF
sparse will catch errors like this. See Documentation/sparse.txt
> +
> + clk = devm_clk_get(&pdev->dev, NULL);
> + if (!IS_ERR(clk))
> + up->uartclk = clk_get_rate(clk);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "not provide mem resource\n");
> + return -ENODEV;
> + }
> + up->mapbase = res->start;
> + up->membase = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(up->membase))
> + return PTR_ERR(up->membase);
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "not provide irq resource\n");
> + return -ENODEV;
> + }
> + up->irq = irq;
> +
> + if (!sprd_uart_driver.state) {
^^^^^^^^^^^^^^^^^^^^^^
I know Rob said this was ok, but it's not.
Just use a global counter.
if (!sprd_ports) {
> + ret = uart_register_driver(&sprd_uart_driver);
> + if (ret < 0) {
> + pr_err("Failed to register SPRD-UART driver\n");
> + return ret;
> + }
> + }
> +
sprd_ports++;
> + ret = uart_add_one_port(&sprd_uart_driver, up);
> + if (ret) {
> + sprd_port[index] = NULL;
> + sprd_remove(pdev);
> + }
> +
> + platform_set_drvdata(pdev, up);
> +
> + return ret;
> +}
> +
> +static int sprd_suspend(struct device *dev)
> +{
> + int id = to_platform_device(dev)->id;
> + struct uart_port *port = &sprd_port[id]->port;
I'm a little confused regarding the port indexing;
is platform_device->id == line ? Where did that happen?
> +
> + uart_suspend_port(&sprd_uart_driver, port);
> +
> + return 0;
> +}
> +
> +static int sprd_resume(struct device *dev)
> +{
> + int id = to_platform_device(dev)->id;
> + struct uart_port *port = &sprd_port[id]->port;
> +
> + uart_resume_port(&sprd_uart_driver, port);
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(sprd_pm_ops, sprd_suspend, sprd_resume);
> +
> +static const struct of_device_id serial_ids[] = {
> + {.compatible = "sprd,sc9836-uart",},
> + {}
> +};
> +
> +static struct platform_driver sprd_platform_driver = {
> + .probe = sprd_probe,
> + .remove = sprd_remove,
> + .driver = {
> + .name = "sprd_serial",
> + .of_match_table = of_match_ptr(serial_ids),
> + .pm = &sprd_pm_ops,
> + },
> +};
> +
> +module_platform_driver(sprd_platform_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Spreadtrum SoC serial driver series");
> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> index c172180..7e6eb39 100644
> --- a/include/uapi/linux/serial_core.h
> +++ b/include/uapi/linux/serial_core.h
> @@ -248,4 +248,7 @@
> /* MESON */
> #define PORT_MESON 109
>
> +/* SPRD SERIAL */
> +#define PORT_SPRD 110
> +
> #endif /* _UAPILINUX_SERIAL_CORE_H */
>
^ permalink raw reply
* Re: [PATCH] selftests/exec: Check if the syscall exists and bail if not
From: Michael Ellerman @ 2015-01-23 6:01 UTC (permalink / raw)
To: David Drysdale
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux API,
Geert Uytterhoeven, Shuah Khan,
davej-rdkfGonbjUTCLXcRTR1eJlpr/1R2p/CL
In-Reply-To: <CAHse=S_AWuY4YG7DJeU+Rma-kPuQLGZH2oqVV27RvF4unu61Aw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, 2015-01-21 at 10:22 +0000, David Drysdale wrote:
> On Wed, Jan 21, 2015 at 7:41 AM, Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org> wrote:
> > On systems which don't implement sys_execveat(), this test produces a
> > lot of output.
> >
> > Add a check at the beginning to see if the syscall is present, and if
> > not just note one error and return.
>
> Good point, thanks.
>
> > diff --git a/tools/testing/selftests/exec/execveat.c b/tools/testing/selftests/exec/execveat.c
> > index e238c9559caf..b87e4a843bea 100644
> > --- a/tools/testing/selftests/exec/execveat.c
> > +++ b/tools/testing/selftests/exec/execveat.c
> > @@ -234,6 +234,14 @@ static int run_tests(void)
> > int fd_cloexec = open_or_die("execveat", O_RDONLY|O_CLOEXEC);
> > int fd_script_cloexec = open_or_die("script", O_RDONLY|O_CLOEXEC);
> >
> > + /* Check if we have execveat at all, and bail early if not */
> > + errno = 0;
> > + execveat_(-1, NULL, NULL, NULL, 0);
> > + if (errno == -ENOSYS) {
>
> Could we change this to ENOSYS (no minus) and also change
> the execveat_() function similarly, so that a binary built where
> __NR_execveat is available but running where it isn't also exits
> early? (My bad for having the minus sign in execveat_() in the
> first place -- fingers too used to kernel mode.)
Ah yeah, me too, -ENOSYS just came naturally.
Will fix up and retest and resend.
cheers
^ permalink raw reply
* Re: [PATCH RFC 5/6] epoll: Add implementation for epoll_mod_wait
From: Fam Zheng @ 2015-01-23 6:20 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Paolo Bonzini, Michael Kerrisk (man-pages),
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, X86 ML,
Alexander Viro, Andrew Morton, Kees Cook, David Herrmann,
Alexei Starovoitov, Miklos Szeredi, David Drysdale, Oleg Nesterov,
David S. Miller, Vivek Goyal, Mike Frysinger, Theodore Ts'o,
Heiko Carstens, Rasmus Villemoes, Rashika Kheria, Hugh
In-Reply-To: <CALCETrWwuJpFK+38mBxxTQCu7Oig22Nr+mAuO++Y+0CdAhfzkw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Thu, 01/22 13:12, Andy Lutomirski wrote:
> On Wed, Jan 21, 2015 at 3:50 AM, Paolo Bonzini <pbonzini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> >
> >
> > On 21/01/2015 12:14, Fam Zheng wrote:
> >> > My take for simplicity will be leaving epoll_ctl as-is, and my take for
> >> > performance will be epoll_pwait1. And I don't really like putting my time on
> >> > epoll_ctl_batch, thinking it as a ambivalent compromise in between.
> >>
> >> > I agree with Michael actually. The big change is going from O(n)
> >> > epoll_ctl calls to O(1), and epoll_ctl_batch achieves that just fine.
> >> > Changing 2 syscalls to 1 is the icing on the cake, but we're talking of
> >> > a fraction of a microsecond.
> >>
> >> Maybe I'm missing something, but in common cases, the set of fds for epoll_wait
> >> doesn't change that radically from one iteration to another, does it?
> >
> > That depends on the application.
>
> In my application, the set of fds almost never changes, but the set of
> events I want changes all the time. The main thing that changes is
> whether I care about EPOLLOUT. If I'm ready to send something, then I
> want EPOLLOUT. If I'm not ready, then I don't want EPOLLOUT.
>
OK, I'll split it to epoll_ctl_batch and epoll_pwait1 as Micheal suggested in
v2.
Fam
^ permalink raw reply
* Re: [PATCH 01/13] kdbus: add documentation
From: Ahmed S. Darwish @ 2015-01-23 6:28 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: arnd-r2nGTMty4D4, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io, teg-B22kvLQNl6c,
jkosina-AlSwsSmVLrQ, luto-kltTT9wpgjJwATOyAt5JVQ,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
daniel-cYrQPVfZooxQFI55V6+gNQ, dh.herrmann-Re5JQEeQqe8AvxtiuMwx3w,
tixxdz-Umm1ozX2/EEdnm+yROfE0A, Michael Kerrisk (man-pages),
Linus Torvalds, Daniel Mack
In-Reply-To: <1421435777-25306-2-git-send-email-gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
On Fri, Jan 16, 2015 at 11:16:05AM -0800, Greg Kroah-Hartman wrote:
> From: Daniel Mack <daniel-cYrQPVfZoowdnm+yROfE0A@public.gmane.org>
>
> kdbus is a system for low-latency, low-overhead, easy to use
> interprocess communication (IPC).
>
> The interface to all functions in this driver is implemented via ioctls
> on files exposed through a filesystem called 'kdbusfs'. The default
> mount point of kdbusfs is /sys/fs/kdbus.
Pardon my ignorance, but we've always been told that adding
new ioctl()s to the kernel is a very big no-no. But given
the seniority of the folks stewarding this kdbus effort,
there must be a good rationale ;-)
So, can the rationale behind introducing new ioctl()s be
further explained? It would be even better if it's included
in the documentation patch itself.
Thanks,
Darwish
^ permalink raw reply
* Re: [PATCH v6 2/2] tty/serial: Add Spreadtrum sc9836-uart driver support
From: Lyra Zhang @ 2015-01-23 7:23 UTC (permalink / raw)
To: Peter Hurley
Cc: Chunyan Zhang,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Mark Rutland,
Arnd Bergmann,
gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org,
Shawn Guo, Pawel Moll,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
Kumar Gala, jslaby-AlSwsSmVLrQ@public.gmane.org, Grant Likely,
Heiko Stübner, jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org,
florian.vaussard-p8DiymsW2f8@public.gmane.org,
andrew-g2DYL2Zd6BY@public.gmane.org, Hayato Suzuki,
antonynpavlov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, Orson Zhai,
geng.ren-lxIno14LUO0EEoCn2XhGlw@public.gmane.org
In-Reply-To: <54C1C69B.6010100-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
Hi, Peter
Many thanks to you for reviewing so carefully and giving us so many
suggestions and so clear explanations.
I'll address all of your comments and send an updated patch soon.
On Fri, Jan 23, 2015 at 11:57 AM, Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org> wrote:
> Hi Chunyan,
>
> On 01/22/2015 08:35 AM, Chunyan Zhang wrote:
>> Add a full sc9836-uart driver for SC9836 SoC which is based on the
>> spreadtrum sharkl64 platform.
>> This driver also support earlycon.
>
> Looking good. Comments below.
>
>> This patch also replaced the spaces between the macros and their
>> values with the tabs in serial_core.h
>
> Notes about patch changes goes...
>
>>
>> Originally-by: Lanqing Liu <lanqing.liu-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
>> Signed-off-by: Orson Zhai <orson.zhai-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
>> Signed-off-by: Chunyan Zhang <chunyan.zhang-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
>> ---
>
> ...here. For example,
>
> * Replaced spaces with tab for PORT_SPRD define in serial_core.h
>
ok
>> drivers/tty/serial/Kconfig | 18 +
>> drivers/tty/serial/Makefile | 1 +
>> drivers/tty/serial/sprd_serial.c | 801 ++++++++++++++++++++++++++++++++++++++
>> include/uapi/linux/serial_core.h | 3 +
>> 4 files changed, 823 insertions(+)
>> create mode 100644 drivers/tty/serial/sprd_serial.c
>>
>> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
>> index c79b43c..13211f7 100644
>> --- a/drivers/tty/serial/Kconfig
>> +++ b/drivers/tty/serial/Kconfig
>> @@ -1577,6 +1577,24 @@ config SERIAL_MEN_Z135
>> This driver can also be build as a module. If so, the module will be called
>> men_z135_uart.ko
>>
>> +config SERIAL_SPRD
>> + tristate "Support for Spreadtrum serial"
>> + depends on ARCH_SPRD
>> + select SERIAL_CORE
>> + help
>> + This enables the driver for the Spreadtrum's serial.
>> +
>> +config SERIAL_SPRD_CONSOLE
>> + bool "Spreadtrum UART console support"
>> + depends on SERIAL_SPRD=y
>> + select SERIAL_CORE_CONSOLE
>> + select SERIAL_EARLYCON
>> + help
>> + Support for early debug console using Spreadtrum's serial. This enables
>> + the console before standard serial driver is probed. This is enabled
>> + with "earlycon" on the kernel command line. The console is
>> + enabled when early_param is processed.
>> +
>> endmenu
>>
>> config SERIAL_MCTRL_GPIO
>> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
>> index 9a548ac..4801aca 100644
>> --- a/drivers/tty/serial/Makefile
>> +++ b/drivers/tty/serial/Makefile
>> @@ -93,6 +93,7 @@ obj-$(CONFIG_SERIAL_ARC) += arc_uart.o
>> obj-$(CONFIG_SERIAL_RP2) += rp2.o
>> obj-$(CONFIG_SERIAL_FSL_LPUART) += fsl_lpuart.o
>> obj-$(CONFIG_SERIAL_MEN_Z135) += men_z135_uart.o
>> +obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
>>
>> # GPIOLIB helpers for modem control lines
>> obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.o
>> diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
>> new file mode 100644
>> index 0000000..fd98541
>> --- /dev/null
>> +++ b/drivers/tty/serial/sprd_serial.c
>> @@ -0,0 +1,801 @@
>> +/*
>> + * Copyright (C) 2012-2015 Spreadtrum Communications Inc.
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * 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.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/console.h>
>> +#include <linux/delay.h>
>> +#include <linux/io.h>
>> +#include <linux/ioport.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/serial_core.h>
>> +#include <linux/serial.h>
>> +#include <linux/slab.h>
>> +#include <linux/tty.h>
>> +#include <linux/tty_flip.h>
>> +
>> +/* device name */
>> +#define UART_NR_MAX 8
>> +#define SPRD_TTY_NAME "ttyS"
>> +#define SPRD_FIFO_SIZE 128
>> +#define SPRD_DEF_RATE 26000000
>> +#define SPRD_TIMEOUT 2048
>> +
>> +/* the offset of serial registers and BITs for them */
>> +/* data registers */
>> +#define SPRD_TXD 0x0000
>> +#define SPRD_RXD 0x0004
>> +
>> +/* line status register and its BITs */
>> +#define SPRD_LSR 0x0008
>> +#define SPRD_LSR_OE BIT(4)
>> +#define SPRD_LSR_FE BIT(3)
>> +#define SPRD_LSR_PE BIT(2)
>> +#define SPRD_LSR_BI BIT(7)
>> +#define SPRD_LSR_TX_OVER BIT(15)
>> +
>> +/* data number in TX and RX fifo */
>> +#define SPRD_STS1 0x000C
>> +
>> +/* interrupt enable register and its BITs */
>> +#define SPRD_IEN 0x0010
>> +#define SPRD_IEN_RX_FULL BIT(0)
>> +#define SPRD_IEN_TX_EMPTY BIT(1)
>> +#define SPRD_IEN_BREAK_DETECT BIT(7)
>> +#define SPRD_IEN_TIMEOUT BIT(13)
>> +
>> +/* interrupt clear register */
>> +#define SPRD_ICLR 0x0014
>> +
>> +/* line control register */
>> +#define SPRD_LCR 0x0018
>> +#define SPRD_LCR_STOP_1BIT 0x10
>> +#define SPRD_LCR_STOP_2BIT 0x30
>> +#define SPRD_LCR_DATA_LEN (BIT(2) | BIT(3))
>> +#define SPRD_LCR_DATA_LEN5 0x0
>> +#define SPRD_LCR_DATA_LEN6 0x4
>> +#define SPRD_LCR_DATA_LEN7 0x8
>> +#define SPRD_LCR_DATA_LEN8 0xc
>> +#define SPRD_LCR_PARITY (BIT(0) | BIT(1))
>> +#define SPRD_LCR_PARITY_EN 0x2
>> +#define SPRD_LCR_EVEN_PAR 0x0
>> +#define SPRD_LCR_ODD_PAR 0x1
>> +
>> +/* control register 1 */
>> +#define SPRD_CTL1 0x001C
>> +#define RX_HW_FLOW_CTL_THLD BIT(6)
>> +#define RX_HW_FLOW_CTL_EN BIT(7)
>> +#define TX_HW_FLOW_CTL_EN BIT(8)
>> +
>> +/* fifo threshold register */
>> +#define SPRD_CTL2 0x0020
>> +#define THLD_TX_EMPTY 0x40
>> +#define THLD_RX_FULL 0x40
>> +
>> +/* config baud rate register */
>> +#define SPRD_CLKD0 0x0024
>> +#define SPRD_CLKD1 0x0028
>> +
>> +/* interrupt mask status register */
>> +#define SPRD_IMSR 0x002C
>> +#define SPRD_IMSR_RX_FIFO_FULL BIT(0)
>> +#define SPRD_IMSR_TX_FIFO_EMPTY BIT(1)
>> +#define SPRD_IMSR_BREAK_DETECT BIT(7)
>> +#define SPRD_IMSR_TIMEOUT BIT(13)
>> +
>> +struct reg_backup {
>> + uint32_t ien;
>
> u32 ien;
>
> same for others
>
>> + uint32_t ctrl0;
>> + uint32_t ctrl1;
>> + uint32_t ctrl2;
>> + uint32_t clkd0;
>> + uint32_t clkd1;
>> + uint32_t dspwait;
>> +};
>> +
>> +struct sprd_uart_port {
>> + struct uart_port port;
>> + struct reg_backup reg_bak;
>> + char name[16];
>> +};
>> +
>> +static struct sprd_uart_port *sprd_port[UART_NR_MAX] = { NULL };
> ^^^^^^^^^^
> Don't need the initializer.
>
>> +
>> +static inline unsigned int serial_in(struct uart_port *port, int offset)
>> +{
>> + return readl_relaxed(port->membase + offset);
>> +}
>> +
>> +static inline void serial_out(struct uart_port *port, int offset, int value)
>> +{
>> + writel_relaxed(value, port->membase + offset);
>> +}
>> +
>> +static unsigned int sprd_tx_empty(struct uart_port *port)
>> +{
>> + if (serial_in(port, SPRD_STS1) & 0xff00)
>> + return 0;
>> + else
>> + return TIOCSER_TEMT;
>> +}
>> +
>> +static unsigned int sprd_get_mctrl(struct uart_port *port)
>> +{
>> + return TIOCM_DSR | TIOCM_CTS;
>> +}
>> +
>> +static void sprd_set_mctrl(struct uart_port *port, unsigned int mctrl)
>> +{
>> + /* nothing to do */
>> +}
>> +
>> +static void sprd_stop_tx(struct uart_port *port)
>> +{
>> + unsigned int ien, iclr;
>> +
>> + iclr = serial_in(port, SPRD_ICLR);
>> + ien = serial_in(port, SPRD_IEN);
>> +
>> + iclr |= SPRD_IEN_TX_EMPTY;
>> + ien &= ~SPRD_IEN_TX_EMPTY;
>> +
>> + serial_out(port, SPRD_ICLR, iclr);
>> + serial_out(port, SPRD_IEN, ien);
>> +}
>> +
>> +static void sprd_start_tx(struct uart_port *port)
>> +{
>> + unsigned int ien;
>> +
>> + ien = serial_in(port, SPRD_IEN);
>> + if (!(ien & SPRD_IEN_TX_EMPTY)) {
>> + ien |= SPRD_IEN_TX_EMPTY;
>> + serial_out(port, SPRD_IEN, ien);
>> + }
>> +}
>> +
>> +static void sprd_stop_rx(struct uart_port *port)
>> +{
>> + unsigned int ien, iclr;
>> +
>> + iclr = serial_in(port, SPRD_ICLR);
>> + ien = serial_in(port, SPRD_IEN);
>> +
>> + ien &= ~(SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT);
>> + iclr |= SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT;
>> +
>> + serial_out(port, SPRD_IEN, ien);
>> + serial_out(port, SPRD_ICLR, iclr);
>> +}
>> +
>> +/* The Sprd serial does not support this function. */
>> +static void sprd_break_ctl(struct uart_port *port, int break_state)
>> +{
>> + /* nothing to do */
>> +}
>> +
>> +static inline int handle_lsr_errors(struct uart_port *port,
>> + unsigned int *flag,
>> + unsigned int *lsr)
>
> Don't declare this inline. gcc will inline single call site
> functions.
>
>> +{
>> + int ret = 0;
>> +
>> + /* statistics */
>> + if (*lsr & SPRD_LSR_BI) {
>> + *lsr &= ~(SPRD_LSR_FE | SPRD_LSR_PE);
>> + port->icount.brk++;
>> + ret = uart_handle_break(port);
>> + if (ret)
>> + return ret;
>> + } else if (*lsr & SPRD_LSR_PE)
>> + port->icount.parity++;
>> + else if (*lsr & SPRD_LSR_FE)
>> + port->icount.frame++;
>> + if (*lsr & SPRD_LSR_OE)
>> + port->icount.overrun++;
>> +
>> + /* mask off conditions which should be ignored */
>> + *lsr &= port->read_status_mask;
>> + if (*lsr & SPRD_LSR_BI)
>> + *flag = TTY_BREAK;
>> + else if (*lsr & SPRD_LSR_PE)
>> + *flag = TTY_PARITY;
>> + else if (*lsr & SPRD_LSR_FE)
>> + *flag = TTY_FRAME;
>> +
>> + return ret;
>> +}
>> +
>> +static inline void sprd_rx(int irq, void *dev_id)
> ^^^^^^^^^^^^
> struct uart_port *port
>
> The 'irq' parameter is unused, remove it.
> Don't declare inline.
>
>> +{
>> + struct uart_port *port = dev_id;
> ^^^^^^^^^
> delete this line.
>
>> + struct tty_port *tty = &port->state->port;
>> + unsigned int ch, flag, lsr, max_count = SPRD_TIMEOUT;
> ^^^^^^^^^
> == 2048?
>
> That's a lot. Most (all?) other drivers use 256.
>
>> +
>> + while ((serial_in(port, SPRD_STS1) & 0x00ff) && max_count--) {
>> + lsr = serial_in(port, SPRD_LSR);
>> + ch = serial_in(port, SPRD_RXD);
>> + flag = TTY_NORMAL;
>> + port->icount.rx++;
>> +
>> + if (lsr & (SPRD_LSR_BI | SPRD_LSR_PE
>> + | SPRD_LSR_FE | SPRD_LSR_OE))
>
> operators should trail on the previous line, like
>
> if (lsr & (SPRD_LSR_BI | SPRD_LSR_PE |
> SPRD_LSR_FE | SPRD_LSR_OE))
>
>
>> + if (handle_lsr_errors(port, &lsr, &flag))
>> + continue;
>> + if (uart_handle_sysrq_char(port, ch))
>> + continue;
>> +
>> + uart_insert_char(port, lsr, SPRD_LSR_OE, ch, flag);
>> + }
>> +
>> + tty_flip_buffer_push(tty);
>> +}
>> +
>> +static inline void sprd_tx(int irq, void *dev_id)
> ^^^^^^^^^^^^^
> struct uart_port *port
>
> The 'irq' parameter is unused, remove it.
> Don't declare inline.
>
>> +{
>> + struct uart_port *port = dev_id;
> ^^^^^^^^^
> delete this line.
>
>> + struct circ_buf *xmit = &port->state->xmit;
>> + int count;
>> +
>> + if (port->x_char) {
>> + serial_out(port, SPRD_TXD, port->x_char);
>> + port->icount.tx++;
>> + port->x_char = 0;
>> + return;
>> + }
>> +
>> + if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
>> + sprd_stop_tx(port);
>> + return;
>> + }
>> +
>> + count = THLD_TX_EMPTY;
>> + do {
>> + serial_out(port, SPRD_TXD, xmit->buf[xmit->tail]);
>> + xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
>> + port->icount.tx++;
>> + if (uart_circ_empty(xmit))
>> + break;
>> + } while (--count > 0);
>> +
>> + if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
>> + uart_write_wakeup(port);
>> +
>> + if (uart_circ_empty(xmit))
>> + sprd_stop_tx(port);
>> +}
>> +
>> +/* this handles the interrupt from one port */
>> +static irqreturn_t sprd_handle_irq(int irq, void *dev_id)
>> +{
>> + struct uart_port *port = (struct uart_port *)dev_id;
> ^^^^^^^^^^^
> Don't need the type cast.
>
>> + unsigned int ims;
>> +
>> + spin_lock(&port->lock);
>> +
>> + ims = serial_in(port, SPRD_IMSR);
>> +
>> + if (!ims)
>> + return IRQ_NONE;
>> +
>> + serial_out(port, SPRD_ICLR, ~0);
>> +
>> + if (ims & (SPRD_IMSR_RX_FIFO_FULL |
>> + SPRD_IMSR_BREAK_DETECT | SPRD_IMSR_TIMEOUT))
>> + sprd_rx(irq, port);
>> +
>> + if (ims & SPRD_IMSR_TX_FIFO_EMPTY)
>> + sprd_tx(irq, port);
>> +
>> + spin_unlock(&port->lock);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int sprd_startup(struct uart_port *port)
>> +{
>> + int ret = 0;
>> + unsigned int ien, ctrl1;
>> + unsigned int timeout;
>> + struct sprd_uart_port *sp;
>
> unsigned long flags;
>
>> +
>> + serial_out(port, SPRD_CTL2, ((THLD_TX_EMPTY << 8) | THLD_RX_FULL));
>> +
>> + /* clear rx fifo */
>> + timeout = SPRD_TIMEOUT;
>> + while (timeout-- && serial_in(port, SPRD_STS1) & 0x00ff)
>> + serial_in(port, SPRD_RXD);
>> +
>> + /* clear tx fifo */
>> + timeout = SPRD_TIMEOUT;
>> + while (timeout-- && serial_in(port, SPRD_STS1) & 0xff00)
>> + cpu_relax();
>> +
>> + /* clear interrupt */
>> + serial_out(port, SPRD_IEN, 0x0);
> ^^^
> just 0
>
>> + serial_out(port, SPRD_ICLR, ~0);
>> +
>> + /* allocate irq */
>> + sp = container_of(port, struct sprd_uart_port, port);
>> + snprintf(sp->name, sizeof(sp->name), "sprd_serial%d", port->line);
>> + ret = devm_request_irq(port->dev, port->irq, sprd_handle_irq,
>> + IRQF_SHARED, sp->name, port);
>> + if (ret) {
>> + dev_err(port->dev, "fail to request serial irq %d, ret=%d\n",
>> + port->irq, ret);
>> + return ret;
>> + }
>> + ctrl1 = serial_in(port, SPRD_CTL1);
>> + ctrl1 |= 0x3e00 | THLD_RX_FULL;
> ^^^^^
> What is this magic number?
>
>> + serial_out(port, SPRD_CTL1, ctrl1);
>> +
>> + /* enable interrupt */
>> + spin_lock(&port->lock);
>
> spin_lock_irqsave(&port->lock, flags);
>
>> + ien = serial_in(port, SPRD_IEN);
>> + ien |= SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT | SPRD_IEN_TIMEOUT;
>> + serial_out(port, SPRD_IEN, ien);
>> + spin_unlock(&port->lock);
>
> spin_unlock_irqrestore(&port->lock, flags);
>
>> +
>> + return 0;
>> +}
>> +
>> +static void sprd_shutdown(struct uart_port *port)
>> +{
>> + serial_out(port, SPRD_IEN, 0x0);
> ^^^
> just 0
>
>> + serial_out(port, SPRD_ICLR, ~0);
>> + devm_free_irq(port->dev, port->irq, port);
>> +}
>> +
>> +static void sprd_set_termios(struct uart_port *port,
>> + struct ktermios *termios,
>> + struct ktermios *old)
>> +{
>> + unsigned int baud, quot;
>> + unsigned int lcr, fc;
>> + unsigned long flags;
>> +
>> + /* ask the core to calculate the divisor for us */
>> + baud = uart_get_baud_rate(port, termios, old, 1200, 3000000);
> ^^^^ ^^^^^^
> mabye derive these from uartclk?
I'm afraid I can't understand very clearly, Could you explain more
details please?
>
>> +
>> + quot = (unsigned int)((port->uartclk + baud / 2) / baud);
>> +
>> + /* set data length */
>> + lcr = serial_in(port, SPRD_LCR);
>> + lcr &= ~SPRD_LCR_DATA_LEN;
>
> What bits are being preserved in SPRD_LCR that you need to read the
> current value? IOW, why can't SPRD_LCR be defined only by the termios
> c_cflag? Like,
>
> lcr = 0;
>
yes, I think you're right. I'll change it.
>> + switch (termios->c_cflag & CSIZE) {
>> + case CS5:
>> + lcr |= SPRD_LCR_DATA_LEN5;
>> + break;
>> + case CS6:
>> + lcr |= SPRD_LCR_DATA_LEN6;
>> + break;
>> + case CS7:
>> + lcr |= SPRD_LCR_DATA_LEN7;
>> + break;
>> + case CS8:
>> + default:
>> + lcr |= SPRD_LCR_DATA_LEN8;
>> + break;
>> + }
>> +
>> + /* calculate stop bits */
>> + lcr &= ~(SPRD_LCR_STOP_1BIT | SPRD_LCR_STOP_2BIT);
>> + if (termios->c_cflag & CSTOPB)
>> + lcr |= SPRD_LCR_STOP_2BIT;
>> + else
>> + lcr |= SPRD_LCR_STOP_1BIT;
>> +
>> + /* calculate parity */
>> + lcr &= ~SPRD_LCR_PARITY;
>> + termios->c_cflag &= ~CMSPAR; /* no support mark/space */
>> + if (termios->c_cflag & PARENB) {
>> + lcr |= SPRD_LCR_PARITY_EN;
>> + if (termios->c_cflag & PARODD)
>> + lcr |= SPRD_LCR_ODD_PAR;
>> + else
>> + lcr |= SPRD_LCR_EVEN_PAR;
>> + }
>> +
>> + spin_lock_irqsave(&port->lock, flags);
>> +
>> + /* update the per-port timeout */
>> + uart_update_timeout(port, termios->c_cflag, baud);
>> +
>> + port->read_status_mask = SPRD_LSR_OE;
>> + if (termios->c_iflag & INPCK)
>> + port->read_status_mask |= SPRD_LSR_FE | SPRD_LSR_PE;
>> + if (termios->c_iflag & (BRKINT | PARMRK))
>
> if (termios->c_iflag & (IGNBRK | BRKINT | PARMRK))
>
> Because if IGNBRK is set and a BRK is received, sprd_rx() should operate
> like this:
> /* - RESULT - */
> lsr = serial_in(SPRD_LSR) /* lsr = SPRD_LSR_BI */
> ch = serial_in(SPRD_RXD) /* ch = 0 */
>
> lsr & SPRD_LSR_BI ? yes
> handle_lsr_errors(lsr, flag)
>
> lsr &= read_status_mask /* lsr = SPRD_LSR_BI */
> flag = TTY_BREAK
>
> uart_insert_char(lsr, ch, flag)
> lsr & ignore_status_mask == 0? no /* ch _not_ inserted */
>
>
>> + port->read_status_mask |= SPRD_LSR_BI;
>> +
>> + /* characters to ignore */
>> + port->ignore_status_mask = 0;
>> + if (termios->c_iflag & IGNPAR)
>> + port->ignore_status_mask |= SPRD_LSR_PE | SPRD_LSR_FE;
>> + if (termios->c_iflag & IGNBRK) {
>> + port->ignore_status_mask |= SPRD_LSR_BI;
>> + /*
>> + * If we're ignoring parity and break indicators,
>> + * ignore overruns too (for real raw support).
>> + */
>> + if (termios->c_iflag & IGNPAR)
>> + port->ignore_status_mask |= SPRD_LSR_OE;
>> + }
>> +
>> + /* flow control */
>> + fc = serial_in(port, SPRD_CTL1);
>> + fc &= ~(RX_HW_FLOW_CTL_THLD | RX_HW_FLOW_CTL_EN | TX_HW_FLOW_CTL_EN);
>> + if (termios->c_cflag & CRTSCTS) {
>> + fc |= RX_HW_FLOW_CTL_THLD;
>> + fc |= RX_HW_FLOW_CTL_EN;
>> + fc |= TX_HW_FLOW_CTL_EN;
>> + }
>> +
>> + /* clock divider bit0~bit15 */
>> + serial_out(port, SPRD_CLKD0, quot & 0xffff);
>> +
>> + /* clock divider bit16~bit20 */
>> + serial_out(port, SPRD_CLKD1, (quot & 0x1f0000) >> 16);
>> + serial_out(port, SPRD_LCR, lcr);
>> + fc |= 0x3e00 | THLD_RX_FULL;
>> + serial_out(port, SPRD_CTL1, fc);
>> +
>> + spin_unlock_irqrestore(&port->lock, flags);
>> +
>> + /* Don't rewrite B0 */
>> + if (tty_termios_baud_rate(termios))
>> + tty_termios_encode_baud_rate(termios, baud, baud);
>> +}
>> +
>> +static const char *sprd_type(struct uart_port *port)
>> +{
>> + return "SPX";
>> +}
>> +
>> +static void sprd_release_port(struct uart_port *port)
>> +{
>> + /* nothing to do */
>> +}
>> +
>> +static int sprd_request_port(struct uart_port *port)
>> +{
>> + return 0;
>> +}
>> +
>> +static void sprd_config_port(struct uart_port *port, int flags)
>> +{
>> + if (flags & UART_CONFIG_TYPE)
>> + port->type = PORT_SPRD;
>> +}
>> +
>> +static int sprd_verify_port(struct uart_port *port,
>> + struct serial_struct *ser)
>> +{
>> + if (ser->type != PORT_SPRD)
>> + return -EINVAL;
>> + if (port->irq != ser->irq)
>> + return -EINVAL;
>> + return 0;
>> +}
>> +
>> +static struct uart_ops serial_sprd_ops = {
>> + .tx_empty = sprd_tx_empty,
>> + .get_mctrl = sprd_get_mctrl,
>> + .set_mctrl = sprd_set_mctrl,
>> + .stop_tx = sprd_stop_tx,
>> + .start_tx = sprd_start_tx,
>> + .stop_rx = sprd_stop_rx,
>> + .break_ctl = sprd_break_ctl,
>> + .startup = sprd_startup,
>> + .shutdown = sprd_shutdown,
>> + .set_termios = sprd_set_termios,
>> + .type = sprd_type,
>> + .release_port = sprd_release_port,
>> + .request_port = sprd_request_port,
>> + .config_port = sprd_config_port,
>> + .verify_port = sprd_verify_port,
>> +};
>> +
>> +#ifdef CONFIG_SERIAL_SPRD_CONSOLE
>> +static inline void wait_for_xmitr(struct uart_port *port)
>> +{
>> + unsigned int status, tmout = 10000;
>> +
>> + /* wait up to 10ms for the character(s) to be sent */
>> + do {
>> + status = serial_in(port, SPRD_STS1);
>> + if (--tmout == 0)
>> + break;
>> + udelay(1);
>> + } while (status & 0xff00);
>> +}
>> +
>> +static void sprd_console_putchar(struct uart_port *port, int ch)
>> +{
>> + wait_for_xmitr(port);
>> + serial_out(port, SPRD_TXD, ch);
>> +}
>> +
>> +static void sprd_console_write(struct console *co, const char *s,
>> + unsigned int count)
>> +{
>> + struct uart_port *port = &sprd_port[co->index]->port;
>> + int locked = 1;
>> + unsigned long flags;
>> +
>> + if (oops_in_progress)
>> + locked = spin_trylock(&port->lock);
>
> if (port->sysrq)
> locked = 0;
> else if (oops_in_progress)
> locked = spin_trylock_irqsave(&port->lock, flags);
>
>> + else
>> + spin_lock_irqsave(&port->lock, flags);
>> +
>> + uart_console_write(port, s, count, sprd_console_putchar);
>> +
>> + /* wait for transmitter to become empty */
>> + wait_for_xmitr(port);
>> +
>> + if (locked)
>> + spin_unlock_irqrestore(&port->lock, flags);
>> +}
>> +
>> +static int __init sprd_console_setup(struct console *co, char *options)
>> +{
>> + struct uart_port *port;
>> + int baud = 115200;
>> + int bits = 8;
>> + int parity = 'n';
>> + int flow = 'n';
>> +
>> + if (co->index >= UART_NR_MAX || co->index < 0)
>> + co->index = 0;
>> +
>> + port = &sprd_port[co->index]->port;
>> + if (port == NULL) {
>> + pr_info("serial port %d not yet initialized\n", co->index);
>> + return -ENODEV;
>> + }
>> + if (options)
>> + uart_parse_options(options, &baud, &parity, &bits, &flow);
>> +
>> + return uart_set_options(port, co, baud, parity, bits, flow);
>> +}
>> +
>> +static struct uart_driver sprd_uart_driver;
>> +static struct console sprd_console = {
>> + .name = SPRD_TTY_NAME,
>> + .write = sprd_console_write,
>> + .device = uart_console_device,
>> + .setup = sprd_console_setup,
>> + .flags = CON_PRINTBUFFER,
>> + .index = -1,
>> + .data = &sprd_uart_driver,
>> +};
>> +
>> +#define SPRD_CONSOLE (&sprd_console)
>> +
>> +/* Support for earlycon */
>> +static void sprd_putc(struct uart_port *port, int c)
>> +{
>> + unsigned int timeout = SPRD_TIMEOUT;
>> +
>> + while (timeout-- &&
>> + !(readl(port->membase + SPRD_LSR) & SPRD_LSR_TX_OVER))
>> + cpu_relax();
>> +
>> + writeb(c, port->membase + SPRD_TXD);
>> +}
>> +
>> +static void sprd_early_write(struct console *con, const char *s,
>> + unsigned n)
>> +{
>> + struct earlycon_device *dev = con->data;
>> +
>> + uart_console_write(&dev->port, s, n, sprd_putc);
>> +}
>> +
>> +static int __init sprd_early_console_setup(
>> + struct earlycon_device *device,
>> + const char *opt)
>> +{
>> + if (!device->port.membase)
>> + return -ENODEV;
>> +
>> + device->con->write = sprd_early_write;
>> + return 0;
>> +}
>> +
>> +EARLYCON_DECLARE(sprd_serial, sprd_early_console_setup);
>> +OF_EARLYCON_DECLARE(sprd_serial, "sprd,sc9836-uart",
>> + sprd_early_console_setup);
>> +
>> +#else /* !CONFIG_SERIAL_SPRD_CONSOLE */
>> +#define SPRD_CONSOLE NULL
>> +#endif
>> +
>> +static struct uart_driver sprd_uart_driver = {
>> + .owner = THIS_MODULE,
>> + .driver_name = "sprd_serial",
>> + .dev_name = SPRD_TTY_NAME,
>> + .major = 0,
>> + .minor = 0,
>> + .nr = UART_NR_MAX,
>> + .cons = SPRD_CONSOLE,
>> +};
>> +
>> +static int sprd_probe_dt_alias(int index, struct device *dev)
>> +{
>> + struct device_node *np;
>> + static bool seen_dev_with_alias;
>> + static bool seen_dev_without_alias;
>> + int ret = index;
>> +
>> + if (!IS_ENABLED(CONFIG_OF))
>> + return ret;
>> +
>> + np = dev->of_node;
>> + if (!np)
>> + return ret;
>> +
>> + ret = of_alias_get_id(np, "serial");
>> + if (IS_ERR_VALUE(ret)) {
>> + seen_dev_without_alias = true;
>> + ret = index;
>> + } else {
>> + seen_dev_with_alias = true;
>> + if (ret >= ARRAY_SIZE(sprd_port) || sprd_port[ret] != NULL) {
>> + dev_warn(dev, "requested serial port %d not available.\n", ret);
>> + ret = index;
>> + }
>> + }
>> +
>> + if (seen_dev_with_alias && seen_dev_without_alias)
>> + dev_warn(dev, "aliased and non-aliased serial devices found in device tree. Serial port enumeration may be unpredictable.\n");
>
> Is this necessary? Why does a user want to know this?
I just followed pl011, but I think I can remove if nobody care it.
>
>> +
>> + return ret;
>> +}
>> +
>> +static int sprd_remove(struct platform_device *dev)
>> +{
>> + struct sprd_uart_port *sup = platform_get_drvdata(dev);
>> + bool busy = false;
>> + int i;
>> +
>> + if (sup)
>> + uart_remove_one_port(&sprd_uart_driver, &sup->port);
>
> see comment in sprd_probe()
ok, I see.
>
> if (sup) {
> uart_remove_one_port();
> sprd_port[sup->line] = NULL;
> sprd_ports--;
> }
>
> if (!sprd_ports)
> uart_unregister_driver();
>
>> +
>> + for (i = 0; i < ARRAY_SIZE(sprd_port); i++)
>> + if (sprd_port[i] == sup)
>> + sprd_port[i] = NULL;
>> + else if (sprd_port[i])
>> + busy = true;
>> +
>> + if (!busy)
>> + uart_unregister_driver(&sprd_uart_driver);
>> +
>> + return 0;
>> +}
>> +
>> +static int sprd_probe(struct platform_device *pdev)
>> +{
>> + struct resource *res;
>> + struct uart_port *up;
>> + struct clk *clk;
>> + int irq;
>> + int index;
>> + int ret;
>> +
>> + for (index = 0; index < ARRAY_SIZE(sprd_port); index++)
>> + if (sprd_port[index] == NULL)
>> + break;
>> +
>> + if (index == ARRAY_SIZE(sprd_port))
>> + return -EBUSY;
>> +
>> + index = sprd_probe_dt_alias(index, &pdev->dev);
>> +
>> + sprd_port[index] = devm_kzalloc(&pdev->dev,
>> + sizeof(*sprd_port[index]), GFP_KERNEL);
>> + if (!sprd_port[index])
>> + return -ENOMEM;
>> +
>> + up = &sprd_port[index]->port;
>> + up->dev = &pdev->dev;
>> + up->line = index;
>> + up->type = PORT_SPRD;
>> + up->iotype = SERIAL_IO_PORT;
>> + up->uartclk = SPRD_DEF_RATE;
>> + up->fifosize = SPRD_FIFO_SIZE;
>> + up->ops = &serial_sprd_ops;
>> + up->flags = ASYNC_BOOT_AUTOCONF;
> ^^^^^^^^^
> UPF_BOOT_AUTOCONF
>
> sparse will catch errors like this. See Documentation/sparse.txt
you mean we should use UPF_BOOT_AUTOCONF, right?
>
>> +
>> + clk = devm_clk_get(&pdev->dev, NULL);
>> + if (!IS_ERR(clk))
>> + up->uartclk = clk_get_rate(clk);
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (!res) {
>> + dev_err(&pdev->dev, "not provide mem resource\n");
>> + return -ENODEV;
>> + }
>> + up->mapbase = res->start;
>> + up->membase = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(up->membase))
>> + return PTR_ERR(up->membase);
>> +
>> + irq = platform_get_irq(pdev, 0);
>> + if (irq < 0) {
>> + dev_err(&pdev->dev, "not provide irq resource\n");
>> + return -ENODEV;
>> + }
>> + up->irq = irq;
>> +
>> + if (!sprd_uart_driver.state) {
> ^^^^^^^^^^^^^^^^^^^^^^
> I know Rob said this was ok, but it's not.
>
> Just use a global counter.
ok.
>
> if (!sprd_ports) {
>
>> + ret = uart_register_driver(&sprd_uart_driver);
>> + if (ret < 0) {
>> + pr_err("Failed to register SPRD-UART driver\n");
>> + return ret;
>> + }
>> + }
>> +
>
> sprd_ports++;
>
>> + ret = uart_add_one_port(&sprd_uart_driver, up);
>> + if (ret) {
>> + sprd_port[index] = NULL;
>> + sprd_remove(pdev);
>> + }
>> +
>> + platform_set_drvdata(pdev, up);
>> +
>> + return ret;
>> +}
>> +
>> +static int sprd_suspend(struct device *dev)
>> +{
>> + int id = to_platform_device(dev)->id;
>> + struct uart_port *port = &sprd_port[id]->port;
>
> I'm a little confused regarding the port indexing;
> is platform_device->id == line ? Where did that happen?
>
Oh, I'll change to assign platform_device->id with port->line in probe()
>
>> +
>> + uart_suspend_port(&sprd_uart_driver, port);
>> +
>> + return 0;
>> +}
>> +
>> +static int sprd_resume(struct device *dev)
>> +{
>> + int id = to_platform_device(dev)->id;
>> + struct uart_port *port = &sprd_port[id]->port;
>> +
>> + uart_resume_port(&sprd_uart_driver, port);
>> +
>> + return 0;
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(sprd_pm_ops, sprd_suspend, sprd_resume);
>> +
>> +static const struct of_device_id serial_ids[] = {
>> + {.compatible = "sprd,sc9836-uart",},
>> + {}
>> +};
>> +
>> +static struct platform_driver sprd_platform_driver = {
>> + .probe = sprd_probe,
>> + .remove = sprd_remove,
>> + .driver = {
>> + .name = "sprd_serial",
>> + .of_match_table = of_match_ptr(serial_ids),
>> + .pm = &sprd_pm_ops,
>> + },
>> +};
>> +
>> +module_platform_driver(sprd_platform_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("Spreadtrum SoC serial driver series");
>> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
>> index c172180..7e6eb39 100644
>> --- a/include/uapi/linux/serial_core.h
>> +++ b/include/uapi/linux/serial_core.h
>> @@ -248,4 +248,7 @@
>> /* MESON */
>> #define PORT_MESON 109
>>
>> +/* SPRD SERIAL */
>> +#define PORT_SPRD 110
>> +
>> #endif /* _UAPILINUX_SERIAL_CORE_H */
>>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] ipv6: Provide definitions for GNU libc 2.8 and later
From: Thierry Reding @ 2015-01-23 7:46 UTC (permalink / raw)
To: David S. Miller, Andrew Morton
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Starting with GNU libc 2.8 and later, the definitions of the in6_pktinfo
and ip6_mtuinfo structures is guarded by __USE_GNU. Make sure to provide
the structures in the linux/ipv6.h when building against GNU libc 2.8 or
later and __USE_GNU is undefined to avoid breaking build issues.
One example where such failure to build can be observed is dhcpcd 6.6.7.
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
This is based on next-20150122.
include/uapi/linux/libc-compat.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/include/uapi/linux/libc-compat.h b/include/uapi/linux/libc-compat.h
index fa673e9cc040..40bd54df06e3 100644
--- a/include/uapi/linux/libc-compat.h
+++ b/include/uapi/linux/libc-compat.h
@@ -70,8 +70,17 @@
#define __UAPI_DEF_IPV6_MREQ 0
#define __UAPI_DEF_IPPROTO_V6 0
#define __UAPI_DEF_IPV6_OPTIONS 0
+/*
+ * The GNU C library 2.8 and later define the in6_pktinfo and ip6_mtuinfo
+ * structures only for __USE_GNU.
+ */
+#if __GLIBC_PREREQ(2, 8) && !defined(__USE_GNU)
+#define __UAPI_DEF_IN6_PKTINFO 1
+#define __UAPI_DEF_IP6_MTUINFO 1
+#else
#define __UAPI_DEF_IN6_PKTINFO 0
#define __UAPI_DEF_IP6_MTUINFO 0
+#endif
#else
--
2.1.3
^ permalink raw reply related
* Re: [PATCH RFC 5/6] epoll: Add implementation for epoll_mod_wait
From: Paolo Bonzini @ 2015-01-23 9:56 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Fam Zheng, Michael Kerrisk (man-pages),
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, X86 ML,
Alexander Viro, Andrew Morton, Kees Cook, David Herrmann,
Alexei Starovoitov, Miklos Szeredi, David Drysdale, Oleg Nesterov,
David S. Miller, Vivek Goyal, Mike Frysinger, Theodore Ts'o,
Heiko Carstens, Rasmus Villemoes, Rashika Kheria,
Hugh Dickins <hugh>
In-Reply-To: <CALCETrWwuJpFK+38mBxxTQCu7Oig22Nr+mAuO++Y+0CdAhfzkw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 22/01/2015 22:12, Andy Lutomirski wrote:
> In my application, the set of fds almost never changes, but the set of
> events I want changes all the time. The main thing that changes is
> whether I care about EPOLLOUT. If I'm ready to send something, then I
> want EPOLLOUT. If I'm not ready, then I don't want EPOLLOUT.
Yes, this is almost always the case unless you use EPOLLET.
Paolo
^ permalink raw reply
* Re: [PATCH 01/13] kdbus: add documentation
From: Michael Kerrisk (man-pages) @ 2015-01-23 11:47 UTC (permalink / raw)
To: David Herrmann
Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w, Daniel Mack,
Greg Kroah-Hartman, Arnd Bergmann, Eric W. Biederman,
One Thousand Gnomes, Tom Gundersen, Jiri Kosina, Andy Lutomirski,
Linux API, linux-kernel, Djalal Harouni, Johannes Stezenbach,
Theodore T'so, Christoph Hellwig
In-Reply-To: <CANq1E4S=3qNw599L85uj-8aXwxeV+mcurB_Nu_rHH8opAeePjw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi David,
On 01/22/2015 02:46 PM, David Herrmann wrote:
> Hi Michael
>
> On Thu, Jan 22, 2015 at 11:18 AM, Michael Kerrisk (man-pages)
> <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 01/21/2015 05:58 PM, Daniel Mack wrote:
>>>>> Also, the context the kdbus commands operate on originate from a
>>>>> mountable special-purpose file system.
>>>>
>>>> It's not clear to me how this point implies any particular API design
>>>> choice.
>>>
>>> It emphasizes the fact that our ioctl API can only be used with the
>>> nodes exposed by kdbusfs, and vice versa. I think operations on
>>> driver-specific files do not justify a new 'generic' syscall API.
>>
>> I see your (repeated) use of words like "driver" as just a distraction,
>> I'm sorry. "Driver-specific files" is an implementation detail. The
>> point is that kdbus provides a complex, general-purpose feature for all
>> of userland. It is core infrastructure that will be used by key pieces
>> of the plumbing layer, and likely by many other applications as well.
>> *Please* stop suggesting that it is not core infrastructure: kdbus
>> has the potential to be a great IPC that will be very useful to many,
>> many user-space developers.
>
> We called it an 'ipc driver' so far. It is in no way meant as
> distraction. Feel free to call it 'core infrastructure'. I think we
> can agree that we want it to be generically useful, like other ipc
> mechanisms, including UDS and netlink.
Yes.
>> (By the way, we have precedents for device/filesystem-specific system
>> calls. Even a recent one, in memfd_create().)
>
> memfd_create() is in no way file-system specific. Internally, it uses
> shmem, but that's an implementation detail. The API does not expose
> this in any way. If you were referring to sealing, it's implemented as
> fcntl(), not as a separate syscall. Furthermore, sealing is only
> limited to shmem as it's the only volatile storage. It's not an API
> restriction. Other volatile file-systems are free to implement
> sealing.
My bad. I mispoke there.
>>>> ioctl() is a get-out-of-jail free card when it comes to API design.
>>>
>>> And how are syscalls different in that regard when they would both
>>> transport the same data structures?
>>
>> My general impression is that when it comes to system calls,
>> there's usually a lot more up front thought about API design.
>
> This is no technical reason why to use syscalls over ioctls. Imho,
> it's rather a reason to improve the kernel's ioctl-review process.
Agreed it's not a technical reason. But the distinction I make
does capture how things usually work.
[...]
>>>> Rather
>>>> than thinking carefully and long about a set of coherent, stable APIs that
>>>> provide some degree of type-safety to user-space, one just adds/changes/removes
>>>> an ioctl.
>>>
>>> Adding another ioctl in the future for cases we didn't think about
>>> earlier would of course be considered a workaround; and even though such
>>> things happen all the time, it's certainly something we'd like to avoid.
>>>
>>> However, we would also need to add another syscall in such cases, which
>>> is even worse IMO. So that's really not an argument against ioctls after
>>> all. What fundamental difference between a raw syscall and a ioctl,
>>> especially with regards to type safety, is there which would help us here?
>>
>> Type safety helps user-space application developers. System calls have
>> it, ioctl() does not.
>
> This is simply not true. There is no type-safety in:
> syscall(__NR_xyz, some, random, arguments)
>
> The only way a syscall gets 'type-safe', is to provide a wrapper
> function. Same applies to ioctls. But people tend to not do that for
> ioctls, which is, again, not a technical argument against ioctls. It's
> a matter of psychology, though.
Yes, I see I wasn't quite clear enough. I should have said:
Type safety helps user-space application developers.
*As typically provided to user-space via libc wrappers*,
system calls have it, ioctl() does not.
And system call wrappers are generally provided pretty much automatically
by libcs (at least, mostly, there's some to-ing and fro-ing about this in
glibc-land these days as you are aware from the memfd_create() story;
http://thread.gmane.org/gmane.comp.lib.glibc.alpha/45884/focus=46937).
So, in practice user-space programmers typically automatically get type
safety with system calls, but not with ioctl().
> I still don't see a technical reason to use syscalls. API proposals welcome!
> We're now working on a small kdbus helper library, which provides
> type-safe ioctl wrappers, item-iterators and documented examples. But,
> like syscalls, nobody is forced to use the wrappers. The API design is
> not affected by this.
>
>>>> And that process seems to be frequent and ongoing even now. (And
>>>> it's to your great credit that the API/ABI breaks are clearly and honestly
>>>> marked in the kdbus.h changelog.) All of this lightens the burden of API
>>>> design for kernel developers, but I'm concerned that the long-term pain
>>>> for user-space developers who use an API which (in my estimation) may
>>>> come to be widely used will be enormous.
>>>
>>> Yes, we've jointly reviewed the API details again until just recently to
>>> unify structs and enums etc, and added fields to make the ioctls structs
>>> more versatile for possible future additions. By that, we effectively
>>> broke the ABI, but we did that because we know we can't do such things
>>> again in the future.
>>>
>>> But again - I don't see how this would be different when using syscalls
>>> rather than ioctls to transport information between the driver and
>>> userspace. Could you elaborate?
>>
>> My suspicion is that not nearly enough thinking has yet been done about
>> the design of the API. That's based on these observations:
>>
>> * Documentation that is, considering the size of the API, *way* too thin.
>
> Ok, working on that.
>
>> * Some parts of the API not documented at all (various kdbus_item blobs)
>
> All public structures have documentation in kdbus.h. It may need
> improvements, though.
Again -- that's very thin--one liners aand sentence fragments for the most
part. (Not that I think that needs to be fixed, just that that doesn't
fit with my definition of "documented", which should be something like
what I've requested for kdbus.txt.)
>> * ABI changes happening even quite recently
>
> Please elaborate why 'recent ABI-changes' are a sign of a premature API.
>
> I seriously doubt any API can be called 'perfect'. On the contrary, I
> believe that all APIs could be improved continuously. The fact that
> we, at one point, settle on an API is an admission of
> backwards-compatibility. I in no way think it's a sign of
> 'perfection'.
I agree that no API is perfect. But some emerge from the starting gate
in much better state than others. kdbus will likely be an important API,
and it's important that it's well designed one at the outset.
> With kdbus our plan is to give API-guarantees starting with upstream
> inclusion. We know, that our API will not be perfect, none is. But we
> will try hard to fix anything that comes up, as long as we can. And
> this effort will manifest in ABI-breaks.
Fair enough. My concern is that upstream inclusion is being rushed before
the API design can be well assessed.
>> * API oddities such as the 'kernel_flags' fields. Why do I need to
>> be told what flags the kernel supports on *every* operation?
>
> If we only returned EINVAL on invalid arguments, user-space had to
> probe for each flag to see whether it's supported. By returning the
> set of supported flags, user-space can cache those and _reliably_ know
(Not sure why you emphasize "reliably" here...)
> which flags are supported.
> We decided the overhead of a single u64 copy on each ioctl is
> preferred over a separate syscall/ioctl to query kernel flags. If you
> disagree, please elaborate (preferably with a suggestion how to do it
> better).
Well that's a quite unconventional design choice. Determining the set
of supported flags in an API is a one-time operation. The natural--and
I would say, *obviously* better--approach to this would be either the
traditional EINVAL approach or an API that is called once to retrieve
the set of supported flags. Instead, kdbus clutters the APIs with a
mostly unneeded extra piece of information on *every* call, and
fractionally increases the run time, for no good reason.
Now, you might say that my suggested alternatives are not obviously
better. My response is that, at the very least, in the documentation
I'd expect to see this unconventional approach clearly highlighted
and accompanied with a sound reason for choosing it. (Note: so far I
still haven't actually seen a sound reason...)
>> The above is just after a day of looking hard at kdbus.txt. I strongly
>> suspect I'd find a lot of other issues if I spent more time on kdbus.
>
> If you find the time, please do! Any hints how a specific part of the
> API could be done better, is highly appreciated. A lot of the more or
> less recent changes were done due to reviews from glib developers.
Could you point me at those reviews (mailing list archives?)?
So, I'm thinking about things such as the following:
* The odd choice of ioctl() as the API mechanism for what should become
a key user-space API. (BTW, which other widely used IPC API ever
took the approach of ioctl() as the mechanism?)
* Weak justifications for unconventional API design choices such
as the 'kernel_flags' above.
* Thin documentation that doesn't provide nearly enough detail,
has no worked examples of the use of the APIs (when it should
contain a multitude of such examples), and has no rationale
for the API design choices [1].
* An API design that consists of 16 ioctl() requests and 20+
structures exchanged between user and kernel space being called
"simple". (Clearly it is not.)
Given a list of points like that, I worry that not nearly enough
thought has been put into design of the API, and certainly would be
very concerned to think that it might be merged into mainline
in the near future.
At this point, I think the onus is on the kdbus developers to
provide strong evidence that they have a good API design, one that
will well serve the needs of thousands of user-space programmers for
the next few decades. Such evidence would include at least:
* Detailed documentation that fully described all facets of the API
* A number of working, well documented example programs that start
(very) simple, and ramp up to demonstrate more complex pieces
of the API.
* Documented rationale for API design choices.
To date, much of that sort of evidence is lacking, and I worry that
the job of proper API design will be left to someone else, someone
who devises a user-space library that provides a suitable
abstraction on top of the current ioctl() API (but may be forced to
make design compromises because of design failings in the underlying
kernel API).
> More help is always welcome!
That's what I'm trying to do at the moment...
Cheers,
Michael
[1] Elsewhere in this thread, you've said that "with bus-proxy and
systemd we have two huge users of kdbus that put a lot of
pressure on API design." I would have expected that the results
of that pressure should be captured in documentation of the
rationale for the API design. I haven't found any of that,
so far.
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: [v8 4/5] ext4: adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface support
From: Konstantin Khlebnikov @ 2015-01-23 11:58 UTC (permalink / raw)
To: Dave Chinner
Cc: Li Xi, linux-fsdevel, linux-ext4, linux-api, tytso, adilger, jack,
viro, hch, dmonakhov, Eric W. Biederman
In-Reply-To: <20150123015307.GD24722@dastard>
On 23.01.2015 04:53, Dave Chinner wrote:
> On Thu, Jan 22, 2015 at 06:28:51PM +0300, Konstantin Khlebnikov wrote:
>> On 09.12.2014 08:22, Li Xi wrote:
>>> +static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
>>> +{
>>> + struct inode *inode = file_inode(filp);
>>> + struct super_block *sb = inode->i_sb;
>>> + struct ext4_inode_info *ei = EXT4_I(inode);
>>> + int err;
>>> + handle_t *handle;
>>> + kprojid_t kprojid;
>>> + struct ext4_iloc iloc;
>>> + struct ext4_inode *raw_inode;
>>> +
>>> + struct dquot *transfer_to[EXT4_MAXQUOTAS] = { };
>>> +
>>> + /* Make sure caller can change project. */
>>> + if (!capable(CAP_SYS_ADMIN))
>>> + return -EACCES;
>>> +
>>> + if (projid != EXT4_DEF_PROJID
>>> + && !EXT4_HAS_RO_COMPAT_FEATURE(sb,
>>> + EXT4_FEATURE_RO_COMPAT_PROJECT))
>>> + return -EOPNOTSUPP;
>>> +
>>> + if (!EXT4_HAS_RO_COMPAT_FEATURE(sb,
>>> + EXT4_FEATURE_RO_COMPAT_PROJECT)) {
>>> + BUG_ON(__kprojid_val(EXT4_I(inode)->i_projid)
>>> + != EXT4_DEF_PROJID);
>>> + if (projid != EXT4_DEF_PROJID)
>>> + return -EOPNOTSUPP;
>>> + else
>>> + return 0;
>>> + }
>>> +
>>> + kprojid = make_kprojid(&init_user_ns, (projid_t)projid);
>>
>> Maybe current_user_ns()?
>> This code should be user-namespace aware from the beginning.
>
> No, the code is correct. Project quotas have nothing to do with
> UIDs and so should never have been included in the uid/gid
> namespace mapping infrastructure in the first place.
Right, but user-namespace provides id mapping for project-id too.
This infrastructure adds support for nested project quotas with
virtualized ids in sub-containers. I couldn't say that this is
must have feature but implementation is trivial because whole
infrastructure is already here.
>
> Point in case: directory subtree quotas can be used as a resource
> controller for limiting space usage within separate containers that
> share the same underlying (large) filesystem via mount namespaces.
That's exactly my use-case: 'sub-volumes' for containers with
quota for space usage/inodes count.
^ permalink raw reply
* [PATCH v7 0/2] Add Spreadtrum SoC bindings and serial driver support
From: Chunyan Zhang @ 2015-01-23 13:01 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
arnd-r2nGTMty4D4, gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io,
peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8, pawel.moll-5wv7dgnIgG8,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, grant.likely-QSEj5FYQhm4dnm+yROfE0A
Cc: jslaby-AlSwsSmVLrQ, heiko-4mtYJXux2i+zQB+pC5nmwQ,
jason-NLaQJdtUoK4Be96aLqz0jA, florian.vaussard-p8DiymsW2f8,
andrew-g2DYL2Zd6BY, hytszk-Re5JQEeQqe8AvxtiuMwx3w,
antonynpavlov-Re5JQEeQqe8AvxtiuMwx3w,
shawn.guo-QSEj5FYQhm4dnm+yROfE0A,
orsonzhai-Re5JQEeQqe8AvxtiuMwx3w, geng.ren-lxIno14LUO0EEoCn2XhGlw,
zhizhou.zhang-lxIno14LUO0EEoCn2XhGlw,
lanqing.liu-lxIno14LUO0EEoCn2XhGlw,
zhang.lyra-Re5JQEeQqe8AvxtiuMwx3w,
wei.qiao-lxIno14LUO0EEoCn2XhGlw,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <sprd-serial-v7>
This patch-set split the last version, and addressed the review comments from
last version on serial driver code.
Changes from v6:
- Setted SPRD_TIMEOUT with 256 rather than 2048
- Used u32 instead of uint32_t
- Removed inline of handle_lsr_errors which is a single call site function
- Removed unused parameter of sprd_tx & sprd_rx
- Used spin_lock_irqsave in sprd_startup() instead of spin_lock
- Added a check for port->sysrq in sprd_console_write()
- Used a global counter as a condition of calling uart_{un}register_driver
- Added pdev->id assignment in probe()
- Setted port->flags with UPF_BOOT_AUTOCONF instead of ASYNC_BOOT_AUTOCONF
Changes from v5:
- Used Spreadtrum instead of SPRD in menus
- Changed TTY name to 'ttyS'
- Moved uart_register_driver() to probe()
- Added spinlock as needed
- Removed register states saving and restoring in suspend() and resume()
Chunyan Zhang (2):
Documentation: DT: Add bindings for Spreadtrum SoC Platform
tty/serial: Add Spreadtrum sc9836-uart driver support
Documentation/devicetree/bindings/arm/sprd.txt | 11 +
.../devicetree/bindings/serial/sprd-uart.txt | 7 +
.../devicetree/bindings/vendor-prefixes.txt | 1 +
drivers/tty/serial/Kconfig | 18 +
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/sprd_serial.c | 799 ++++++++++++++++++++
include/uapi/linux/serial_core.h | 3 +
7 files changed, 840 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/sprd.txt
create mode 100644 Documentation/devicetree/bindings/serial/sprd-uart.txt
create mode 100644 drivers/tty/serial/sprd_serial.c
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v7 1/2] Documentation: DT: Add bindings for Spreadtrum SoC Platform
From: Chunyan Zhang @ 2015-01-23 13:01 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
arnd-r2nGTMty4D4, gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io,
peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8, pawel.moll-5wv7dgnIgG8,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, grant.likely-QSEj5FYQhm4dnm+yROfE0A
Cc: jslaby-AlSwsSmVLrQ, heiko-4mtYJXux2i+zQB+pC5nmwQ,
jason-NLaQJdtUoK4Be96aLqz0jA, florian.vaussard-p8DiymsW2f8,
andrew-g2DYL2Zd6BY, hytszk-Re5JQEeQqe8AvxtiuMwx3w,
antonynpavlov-Re5JQEeQqe8AvxtiuMwx3w,
shawn.guo-QSEj5FYQhm4dnm+yROfE0A,
orsonzhai-Re5JQEeQqe8AvxtiuMwx3w, geng.ren-lxIno14LUO0EEoCn2XhGlw,
zhizhou.zhang-lxIno14LUO0EEoCn2XhGlw,
lanqing.liu-lxIno14LUO0EEoCn2XhGlw,
zhang.lyra-Re5JQEeQqe8AvxtiuMwx3w,
wei.qiao-lxIno14LUO0EEoCn2XhGlw,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1422018066-24997-1-git-send-email-chunyan.zhang-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
Adds Spreadtrum's prefix "sprd" to vendor-prefixes file.
Adds the devicetree binding documentations for Spreadtrum's sc9836-uart
and SC9836 SoC based on the Sharkl64 Platform which is a 64-bit SoC
Platform of Spreadtrum.
Signed-off-by: Chunyan Zhang <chunyan.zhang-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
---
Documentation/devicetree/bindings/arm/sprd.txt | 11 +++++++++++
.../devicetree/bindings/serial/sprd-uart.txt | 7 +++++++
.../devicetree/bindings/vendor-prefixes.txt | 1 +
3 files changed, 19 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/sprd.txt
create mode 100644 Documentation/devicetree/bindings/serial/sprd-uart.txt
diff --git a/Documentation/devicetree/bindings/arm/sprd.txt b/Documentation/devicetree/bindings/arm/sprd.txt
new file mode 100644
index 0000000..31a629d
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/sprd.txt
@@ -0,0 +1,11 @@
+Spreadtrum SoC Platforms Device Tree Bindings
+----------------------------------------------------
+
+Sharkl64 is a Spreadtrum's SoC Platform which is based
+on ARM 64-bit processor.
+
+SC9836 openphone board with SC9836 SoC based on the
+Sharkl64 Platform shall have the following properties.
+
+Required root node properties:
+ - compatible = "sprd,sc9836-openphone", "sprd,sc9836";
diff --git a/Documentation/devicetree/bindings/serial/sprd-uart.txt b/Documentation/devicetree/bindings/serial/sprd-uart.txt
new file mode 100644
index 0000000..2aff0f2
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/sprd-uart.txt
@@ -0,0 +1,7 @@
+* Spreadtrum serial UART
+
+Required properties:
+- compatible: must be "sprd,sc9836-uart"
+- reg: offset and length of the register set for the device
+- interrupts: exactly one interrupt specifier
+- clocks: phandles to input clocks.
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index b1df0ad..0a8384f 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -153,6 +153,7 @@ snps Synopsys, Inc.
solidrun SolidRun
sony Sony Corporation
spansion Spansion Inc.
+sprd Spreadtrum Communications Inc.
st STMicroelectronics
ste ST-Ericsson
stericsson ST-Ericsson
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v7 2/2] tty/serial: Add Spreadtrum sc9836-uart driver support
From: Chunyan Zhang @ 2015-01-23 13:01 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
arnd-r2nGTMty4D4, gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io,
peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8, pawel.moll-5wv7dgnIgG8,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, grant.likely-QSEj5FYQhm4dnm+yROfE0A
Cc: jslaby-AlSwsSmVLrQ, heiko-4mtYJXux2i+zQB+pC5nmwQ,
jason-NLaQJdtUoK4Be96aLqz0jA, florian.vaussard-p8DiymsW2f8,
andrew-g2DYL2Zd6BY, hytszk-Re5JQEeQqe8AvxtiuMwx3w,
antonynpavlov-Re5JQEeQqe8AvxtiuMwx3w,
shawn.guo-QSEj5FYQhm4dnm+yROfE0A,
orsonzhai-Re5JQEeQqe8AvxtiuMwx3w, geng.ren-lxIno14LUO0EEoCn2XhGlw,
zhizhou.zhang-lxIno14LUO0EEoCn2XhGlw,
lanqing.liu-lxIno14LUO0EEoCn2XhGlw,
zhang.lyra-Re5JQEeQqe8AvxtiuMwx3w,
wei.qiao-lxIno14LUO0EEoCn2XhGlw,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1422018066-24997-1-git-send-email-chunyan.zhang-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
Add a full sc9836-uart driver for SC9836 SoC which is based on the
spreadtrum sharkl64 platform.
This driver also support earlycon.
Originally-by: Lanqing Liu <lanqing.liu-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
Signed-off-by: Orson Zhai <orson.zhai-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
Signed-off-by: Chunyan Zhang <chunyan.zhang-lxIno14LUO0EEoCn2XhGlw@public.gmane.org>
Acked-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
drivers/tty/serial/Kconfig | 18 +
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/sprd_serial.c | 799 ++++++++++++++++++++++++++++++++++++++
include/uapi/linux/serial_core.h | 3 +
4 files changed, 821 insertions(+)
create mode 100644 drivers/tty/serial/sprd_serial.c
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index c79b43c..13211f7 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1577,6 +1577,24 @@ config SERIAL_MEN_Z135
This driver can also be build as a module. If so, the module will be called
men_z135_uart.ko
+config SERIAL_SPRD
+ tristate "Support for Spreadtrum serial"
+ depends on ARCH_SPRD
+ select SERIAL_CORE
+ help
+ This enables the driver for the Spreadtrum's serial.
+
+config SERIAL_SPRD_CONSOLE
+ bool "Spreadtrum UART console support"
+ depends on SERIAL_SPRD=y
+ select SERIAL_CORE_CONSOLE
+ select SERIAL_EARLYCON
+ help
+ Support for early debug console using Spreadtrum's serial. This enables
+ the console before standard serial driver is probed. This is enabled
+ with "earlycon" on the kernel command line. The console is
+ enabled when early_param is processed.
+
endmenu
config SERIAL_MCTRL_GPIO
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index 9a548ac..4801aca 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -93,6 +93,7 @@ obj-$(CONFIG_SERIAL_ARC) += arc_uart.o
obj-$(CONFIG_SERIAL_RP2) += rp2.o
obj-$(CONFIG_SERIAL_FSL_LPUART) += fsl_lpuart.o
obj-$(CONFIG_SERIAL_MEN_Z135) += men_z135_uart.o
+obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
# GPIOLIB helpers for modem control lines
obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.o
diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
new file mode 100644
index 0000000..e4b0793
--- /dev/null
+++ b/drivers/tty/serial/sprd_serial.c
@@ -0,0 +1,799 @@
+/*
+ * Copyright (C) 2012-2015 Spreadtrum Communications Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ */
+
+#include <linux/clk.h>
+#include <linux/console.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/serial_core.h>
+#include <linux/serial.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+
+/* device name */
+#define UART_NR_MAX 8
+#define SPRD_TTY_NAME "ttyS"
+#define SPRD_FIFO_SIZE 128
+#define SPRD_DEF_RATE 26000000
+#define SPRD_BAUD_IO_LIMIT 3000000
+#define SPRD_TIMEOUT 256
+
+/* the offset of serial registers and BITs for them */
+/* data registers */
+#define SPRD_TXD 0x0000
+#define SPRD_RXD 0x0004
+
+/* line status register and its BITs */
+#define SPRD_LSR 0x0008
+#define SPRD_LSR_OE BIT(4)
+#define SPRD_LSR_FE BIT(3)
+#define SPRD_LSR_PE BIT(2)
+#define SPRD_LSR_BI BIT(7)
+#define SPRD_LSR_TX_OVER BIT(15)
+
+/* data number in TX and RX fifo */
+#define SPRD_STS1 0x000C
+
+/* interrupt enable register and its BITs */
+#define SPRD_IEN 0x0010
+#define SPRD_IEN_RX_FULL BIT(0)
+#define SPRD_IEN_TX_EMPTY BIT(1)
+#define SPRD_IEN_BREAK_DETECT BIT(7)
+#define SPRD_IEN_TIMEOUT BIT(13)
+
+/* interrupt clear register */
+#define SPRD_ICLR 0x0014
+
+/* line control register */
+#define SPRD_LCR 0x0018
+#define SPRD_LCR_STOP_1BIT 0x10
+#define SPRD_LCR_STOP_2BIT 0x30
+#define SPRD_LCR_DATA_LEN (BIT(2) | BIT(3))
+#define SPRD_LCR_DATA_LEN5 0x0
+#define SPRD_LCR_DATA_LEN6 0x4
+#define SPRD_LCR_DATA_LEN7 0x8
+#define SPRD_LCR_DATA_LEN8 0xc
+#define SPRD_LCR_PARITY (BIT(0) | BIT(1))
+#define SPRD_LCR_PARITY_EN 0x2
+#define SPRD_LCR_EVEN_PAR 0x0
+#define SPRD_LCR_ODD_PAR 0x1
+
+/* control register 1 */
+#define SPRD_CTL1 0x001C
+#define RX_HW_FLOW_CTL_THLD BIT(6)
+#define RX_HW_FLOW_CTL_EN BIT(7)
+#define TX_HW_FLOW_CTL_EN BIT(8)
+#define RX_TOUT_THLD_DEF 0x3E00
+#define RX_HFC_THLD_DEF 0x40
+
+/* fifo threshold register */
+#define SPRD_CTL2 0x0020
+#define THLD_TX_EMPTY 0x40
+#define THLD_RX_FULL 0x40
+
+/* config baud rate register */
+#define SPRD_CLKD0 0x0024
+#define SPRD_CLKD1 0x0028
+
+/* interrupt mask status register */
+#define SPRD_IMSR 0x002C
+#define SPRD_IMSR_RX_FIFO_FULL BIT(0)
+#define SPRD_IMSR_TX_FIFO_EMPTY BIT(1)
+#define SPRD_IMSR_BREAK_DETECT BIT(7)
+#define SPRD_IMSR_TIMEOUT BIT(13)
+
+struct reg_backup {
+ u32 ien;
+ u32 ctrl0;
+ u32 ctrl1;
+ u32 ctrl2;
+ u32 clkd0;
+ u32 clkd1;
+ u32 dspwait;
+};
+
+struct sprd_uart_port {
+ struct uart_port port;
+ struct reg_backup reg_bak;
+ char name[16];
+};
+
+static struct sprd_uart_port *sprd_port[UART_NR_MAX];
+static int sprd_ports_num;
+
+static inline unsigned int serial_in(struct uart_port *port, int offset)
+{
+ return readl_relaxed(port->membase + offset);
+}
+
+static inline void serial_out(struct uart_port *port, int offset, int value)
+{
+ writel_relaxed(value, port->membase + offset);
+}
+
+static unsigned int sprd_tx_empty(struct uart_port *port)
+{
+ if (serial_in(port, SPRD_STS1) & 0xff00)
+ return 0;
+ else
+ return TIOCSER_TEMT;
+}
+
+static unsigned int sprd_get_mctrl(struct uart_port *port)
+{
+ return TIOCM_DSR | TIOCM_CTS;
+}
+
+static void sprd_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+ /* nothing to do */
+}
+
+static void sprd_stop_tx(struct uart_port *port)
+{
+ unsigned int ien, iclr;
+
+ iclr = serial_in(port, SPRD_ICLR);
+ ien = serial_in(port, SPRD_IEN);
+
+ iclr |= SPRD_IEN_TX_EMPTY;
+ ien &= ~SPRD_IEN_TX_EMPTY;
+
+ serial_out(port, SPRD_ICLR, iclr);
+ serial_out(port, SPRD_IEN, ien);
+}
+
+static void sprd_start_tx(struct uart_port *port)
+{
+ unsigned int ien;
+
+ ien = serial_in(port, SPRD_IEN);
+ if (!(ien & SPRD_IEN_TX_EMPTY)) {
+ ien |= SPRD_IEN_TX_EMPTY;
+ serial_out(port, SPRD_IEN, ien);
+ }
+}
+
+static void sprd_stop_rx(struct uart_port *port)
+{
+ unsigned int ien, iclr;
+
+ iclr = serial_in(port, SPRD_ICLR);
+ ien = serial_in(port, SPRD_IEN);
+
+ ien &= ~(SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT);
+ iclr |= SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT;
+
+ serial_out(port, SPRD_IEN, ien);
+ serial_out(port, SPRD_ICLR, iclr);
+}
+
+/* The Sprd serial does not support this function. */
+static void sprd_break_ctl(struct uart_port *port, int break_state)
+{
+ /* nothing to do */
+}
+
+static int handle_lsr_errors(struct uart_port *port,
+ unsigned int *flag,
+ unsigned int *lsr)
+{
+ int ret = 0;
+
+ /* statistics */
+ if (*lsr & SPRD_LSR_BI) {
+ *lsr &= ~(SPRD_LSR_FE | SPRD_LSR_PE);
+ port->icount.brk++;
+ ret = uart_handle_break(port);
+ if (ret)
+ return ret;
+ } else if (*lsr & SPRD_LSR_PE)
+ port->icount.parity++;
+ else if (*lsr & SPRD_LSR_FE)
+ port->icount.frame++;
+ if (*lsr & SPRD_LSR_OE)
+ port->icount.overrun++;
+
+ /* mask off conditions which should be ignored */
+ *lsr &= port->read_status_mask;
+ if (*lsr & SPRD_LSR_BI)
+ *flag = TTY_BREAK;
+ else if (*lsr & SPRD_LSR_PE)
+ *flag = TTY_PARITY;
+ else if (*lsr & SPRD_LSR_FE)
+ *flag = TTY_FRAME;
+
+ return ret;
+}
+
+static inline void sprd_rx(struct uart_port *port)
+{
+ struct tty_port *tty = &port->state->port;
+ unsigned int ch, flag, lsr, max_count = SPRD_TIMEOUT;
+
+ while ((serial_in(port, SPRD_STS1) & 0x00ff) && max_count--) {
+ lsr = serial_in(port, SPRD_LSR);
+ ch = serial_in(port, SPRD_RXD);
+ flag = TTY_NORMAL;
+ port->icount.rx++;
+
+ if (lsr & (SPRD_LSR_BI | SPRD_LSR_PE |
+ SPRD_LSR_FE | SPRD_LSR_OE))
+ if (handle_lsr_errors(port, &lsr, &flag))
+ continue;
+ if (uart_handle_sysrq_char(port, ch))
+ continue;
+
+ uart_insert_char(port, lsr, SPRD_LSR_OE, ch, flag);
+ }
+
+ tty_flip_buffer_push(tty);
+}
+
+static inline void sprd_tx(struct uart_port *port)
+{
+ struct circ_buf *xmit = &port->state->xmit;
+ int count;
+
+ if (port->x_char) {
+ serial_out(port, SPRD_TXD, port->x_char);
+ port->icount.tx++;
+ port->x_char = 0;
+ return;
+ }
+
+ if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
+ sprd_stop_tx(port);
+ return;
+ }
+
+ count = THLD_TX_EMPTY;
+ do {
+ serial_out(port, SPRD_TXD, xmit->buf[xmit->tail]);
+ xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
+ port->icount.tx++;
+ if (uart_circ_empty(xmit))
+ break;
+ } while (--count > 0);
+
+ if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+ uart_write_wakeup(port);
+
+ if (uart_circ_empty(xmit))
+ sprd_stop_tx(port);
+}
+
+/* this handles the interrupt from one port */
+static irqreturn_t sprd_handle_irq(int irq, void *dev_id)
+{
+ struct uart_port *port = dev_id;
+ unsigned int ims;
+
+ spin_lock(&port->lock);
+
+ ims = serial_in(port, SPRD_IMSR);
+
+ if (!ims)
+ return IRQ_NONE;
+
+ serial_out(port, SPRD_ICLR, ~0);
+
+ if (ims & (SPRD_IMSR_RX_FIFO_FULL |
+ SPRD_IMSR_BREAK_DETECT | SPRD_IMSR_TIMEOUT))
+ sprd_rx(port);
+
+ if (ims & SPRD_IMSR_TX_FIFO_EMPTY)
+ sprd_tx(port);
+
+ spin_unlock(&port->lock);
+
+ return IRQ_HANDLED;
+}
+
+static int sprd_startup(struct uart_port *port)
+{
+ int ret = 0;
+ unsigned int ien, fc;
+ unsigned int timeout;
+ struct sprd_uart_port *sp;
+ unsigned long flags;
+
+ serial_out(port, SPRD_CTL2, ((THLD_TX_EMPTY << 8) | THLD_RX_FULL));
+
+ /* clear rx fifo */
+ timeout = SPRD_TIMEOUT;
+ while (timeout-- && serial_in(port, SPRD_STS1) & 0x00ff)
+ serial_in(port, SPRD_RXD);
+
+ /* clear tx fifo */
+ timeout = SPRD_TIMEOUT;
+ while (timeout-- && serial_in(port, SPRD_STS1) & 0xff00)
+ cpu_relax();
+
+ /* clear interrupt */
+ serial_out(port, SPRD_IEN, 0);
+ serial_out(port, SPRD_ICLR, ~0);
+
+ /* allocate irq */
+ sp = container_of(port, struct sprd_uart_port, port);
+ snprintf(sp->name, sizeof(sp->name), "sprd_serial%d", port->line);
+ ret = devm_request_irq(port->dev, port->irq, sprd_handle_irq,
+ IRQF_SHARED, sp->name, port);
+ if (ret) {
+ dev_err(port->dev, "fail to request serial irq %d, ret=%d\n",
+ port->irq, ret);
+ return ret;
+ }
+ fc = serial_in(port, SPRD_CTL1);
+ fc |= RX_TOUT_THLD_DEF | RX_HFC_THLD_DEF;
+ serial_out(port, SPRD_CTL1, fc);
+
+ /* enable interrupt */
+ spin_lock_irqsave(&port->lock, flags);
+ ien = serial_in(port, SPRD_IEN);
+ ien |= SPRD_IEN_RX_FULL | SPRD_IEN_BREAK_DETECT | SPRD_IEN_TIMEOUT;
+ serial_out(port, SPRD_IEN, ien);
+ spin_unlock_irqrestore(&port->lock, flags);
+
+ return 0;
+}
+
+static void sprd_shutdown(struct uart_port *port)
+{
+ serial_out(port, SPRD_IEN, 0);
+ serial_out(port, SPRD_ICLR, ~0);
+ devm_free_irq(port->dev, port->irq, port);
+}
+
+static void sprd_set_termios(struct uart_port *port,
+ struct ktermios *termios,
+ struct ktermios *old)
+{
+ unsigned int baud, quot;
+ unsigned int lcr, fc;
+ unsigned long flags;
+
+ /* ask the core to calculate the divisor for us */
+ baud = uart_get_baud_rate(port, termios, old, 0, SPRD_BAUD_IO_LIMIT);
+
+ quot = (unsigned int)((port->uartclk + baud / 2) / baud);
+
+ /* set data length */
+ switch (termios->c_cflag & CSIZE) {
+ case CS5:
+ lcr |= SPRD_LCR_DATA_LEN5;
+ break;
+ case CS6:
+ lcr |= SPRD_LCR_DATA_LEN6;
+ break;
+ case CS7:
+ lcr |= SPRD_LCR_DATA_LEN7;
+ break;
+ case CS8:
+ default:
+ lcr |= SPRD_LCR_DATA_LEN8;
+ break;
+ }
+
+ /* calculate stop bits */
+ lcr &= ~(SPRD_LCR_STOP_1BIT | SPRD_LCR_STOP_2BIT);
+ if (termios->c_cflag & CSTOPB)
+ lcr |= SPRD_LCR_STOP_2BIT;
+ else
+ lcr |= SPRD_LCR_STOP_1BIT;
+
+ /* calculate parity */
+ lcr &= ~SPRD_LCR_PARITY;
+ termios->c_cflag &= ~CMSPAR; /* no support mark/space */
+ if (termios->c_cflag & PARENB) {
+ lcr |= SPRD_LCR_PARITY_EN;
+ if (termios->c_cflag & PARODD)
+ lcr |= SPRD_LCR_ODD_PAR;
+ else
+ lcr |= SPRD_LCR_EVEN_PAR;
+ }
+
+ spin_lock_irqsave(&port->lock, flags);
+
+ /* update the per-port timeout */
+ uart_update_timeout(port, termios->c_cflag, baud);
+
+ port->read_status_mask = SPRD_LSR_OE;
+ if (termios->c_iflag & INPCK)
+ port->read_status_mask |= SPRD_LSR_FE | SPRD_LSR_PE;
+ if (termios->c_iflag & (IGNBRK | BRKINT | PARMRK))
+ port->read_status_mask |= SPRD_LSR_BI;
+
+ /* characters to ignore */
+ port->ignore_status_mask = 0;
+ if (termios->c_iflag & IGNPAR)
+ port->ignore_status_mask |= SPRD_LSR_PE | SPRD_LSR_FE;
+ if (termios->c_iflag & IGNBRK) {
+ port->ignore_status_mask |= SPRD_LSR_BI;
+ /*
+ * If we're ignoring parity and break indicators,
+ * ignore overruns too (for real raw support).
+ */
+ if (termios->c_iflag & IGNPAR)
+ port->ignore_status_mask |= SPRD_LSR_OE;
+ }
+
+ /* flow control */
+ fc = serial_in(port, SPRD_CTL1);
+ fc &= ~(RX_HW_FLOW_CTL_THLD | RX_HW_FLOW_CTL_EN | TX_HW_FLOW_CTL_EN);
+ if (termios->c_cflag & CRTSCTS) {
+ fc |= RX_HW_FLOW_CTL_THLD;
+ fc |= RX_HW_FLOW_CTL_EN;
+ fc |= TX_HW_FLOW_CTL_EN;
+ }
+
+ /* clock divider bit0~bit15 */
+ serial_out(port, SPRD_CLKD0, quot & 0xffff);
+
+ /* clock divider bit16~bit20 */
+ serial_out(port, SPRD_CLKD1, (quot & 0x1f0000) >> 16);
+ serial_out(port, SPRD_LCR, lcr);
+ fc |= RX_TOUT_THLD_DEF | RX_HFC_THLD_DEF;
+ serial_out(port, SPRD_CTL1, fc);
+
+ spin_unlock_irqrestore(&port->lock, flags);
+
+ /* Don't rewrite B0 */
+ if (tty_termios_baud_rate(termios))
+ tty_termios_encode_baud_rate(termios, baud, baud);
+}
+
+static const char *sprd_type(struct uart_port *port)
+{
+ return "SPX";
+}
+
+static void sprd_release_port(struct uart_port *port)
+{
+ /* nothing to do */
+}
+
+static int sprd_request_port(struct uart_port *port)
+{
+ return 0;
+}
+
+static void sprd_config_port(struct uart_port *port, int flags)
+{
+ if (flags & UART_CONFIG_TYPE)
+ port->type = PORT_SPRD;
+}
+
+static int sprd_verify_port(struct uart_port *port,
+ struct serial_struct *ser)
+{
+ if (ser->type != PORT_SPRD)
+ return -EINVAL;
+ if (port->irq != ser->irq)
+ return -EINVAL;
+ return 0;
+}
+
+static struct uart_ops serial_sprd_ops = {
+ .tx_empty = sprd_tx_empty,
+ .get_mctrl = sprd_get_mctrl,
+ .set_mctrl = sprd_set_mctrl,
+ .stop_tx = sprd_stop_tx,
+ .start_tx = sprd_start_tx,
+ .stop_rx = sprd_stop_rx,
+ .break_ctl = sprd_break_ctl,
+ .startup = sprd_startup,
+ .shutdown = sprd_shutdown,
+ .set_termios = sprd_set_termios,
+ .type = sprd_type,
+ .release_port = sprd_release_port,
+ .request_port = sprd_request_port,
+ .config_port = sprd_config_port,
+ .verify_port = sprd_verify_port,
+};
+
+#ifdef CONFIG_SERIAL_SPRD_CONSOLE
+static inline void wait_for_xmitr(struct uart_port *port)
+{
+ unsigned int status, tmout = 10000;
+
+ /* wait up to 10ms for the character(s) to be sent */
+ do {
+ status = serial_in(port, SPRD_STS1);
+ if (--tmout == 0)
+ break;
+ udelay(1);
+ } while (status & 0xff00);
+}
+
+static void sprd_console_putchar(struct uart_port *port, int ch)
+{
+ wait_for_xmitr(port);
+ serial_out(port, SPRD_TXD, ch);
+}
+
+static void sprd_console_write(struct console *co, const char *s,
+ unsigned int count)
+{
+ struct uart_port *port = &sprd_port[co->index]->port;
+ int locked = 1;
+ unsigned long flags;
+
+ if (port->sysrq)
+ locked = 0;
+ else if (oops_in_progress)
+ locked = spin_trylock_irqsave(&port->lock, flags);
+ else
+ spin_lock_irqsave(&port->lock, flags);
+
+ uart_console_write(port, s, count, sprd_console_putchar);
+
+ /* wait for transmitter to become empty */
+ wait_for_xmitr(port);
+
+ if (locked)
+ spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static int __init sprd_console_setup(struct console *co, char *options)
+{
+ struct uart_port *port;
+ int baud = 115200;
+ int bits = 8;
+ int parity = 'n';
+ int flow = 'n';
+
+ if (co->index >= UART_NR_MAX || co->index < 0)
+ co->index = 0;
+
+ port = &sprd_port[co->index]->port;
+ if (port == NULL) {
+ pr_info("serial port %d not yet initialized\n", co->index);
+ return -ENODEV;
+ }
+ if (options)
+ uart_parse_options(options, &baud, &parity, &bits, &flow);
+
+ return uart_set_options(port, co, baud, parity, bits, flow);
+}
+
+static struct uart_driver sprd_uart_driver;
+static struct console sprd_console = {
+ .name = SPRD_TTY_NAME,
+ .write = sprd_console_write,
+ .device = uart_console_device,
+ .setup = sprd_console_setup,
+ .flags = CON_PRINTBUFFER,
+ .index = -1,
+ .data = &sprd_uart_driver,
+};
+
+#define SPRD_CONSOLE (&sprd_console)
+
+/* Support for earlycon */
+static void sprd_putc(struct uart_port *port, int c)
+{
+ unsigned int timeout = SPRD_TIMEOUT;
+
+ while (timeout-- &&
+ !(readl(port->membase + SPRD_LSR) & SPRD_LSR_TX_OVER))
+ cpu_relax();
+
+ writeb(c, port->membase + SPRD_TXD);
+}
+
+static void sprd_early_write(struct console *con, const char *s,
+ unsigned n)
+{
+ struct earlycon_device *dev = con->data;
+
+ uart_console_write(&dev->port, s, n, sprd_putc);
+}
+
+static int __init sprd_early_console_setup(
+ struct earlycon_device *device,
+ const char *opt)
+{
+ if (!device->port.membase)
+ return -ENODEV;
+
+ device->con->write = sprd_early_write;
+ return 0;
+}
+
+EARLYCON_DECLARE(sprd_serial, sprd_early_console_setup);
+OF_EARLYCON_DECLARE(sprd_serial, "sprd,sc9836-uart",
+ sprd_early_console_setup);
+
+#else /* !CONFIG_SERIAL_SPRD_CONSOLE */
+#define SPRD_CONSOLE NULL
+#endif
+
+static struct uart_driver sprd_uart_driver = {
+ .owner = THIS_MODULE,
+ .driver_name = "sprd_serial",
+ .dev_name = SPRD_TTY_NAME,
+ .major = 0,
+ .minor = 0,
+ .nr = UART_NR_MAX,
+ .cons = SPRD_CONSOLE,
+};
+
+static int sprd_probe_dt_alias(int index, struct device *dev)
+{
+ struct device_node *np;
+ static bool seen_dev_with_alias;
+ static bool seen_dev_without_alias;
+ int ret = index;
+
+ if (!IS_ENABLED(CONFIG_OF))
+ return ret;
+
+ np = dev->of_node;
+ if (!np)
+ return ret;
+
+ ret = of_alias_get_id(np, "serial");
+ if (IS_ERR_VALUE(ret)) {
+ seen_dev_without_alias = true;
+ ret = index;
+ } else {
+ seen_dev_with_alias = true;
+ if (ret >= ARRAY_SIZE(sprd_port) || sprd_port[ret] != NULL) {
+ dev_warn(dev, "requested serial port %d not available.\n", ret);
+ ret = index;
+ }
+ }
+
+ return ret;
+}
+
+static int sprd_remove(struct platform_device *dev)
+{
+ struct sprd_uart_port *sup = platform_get_drvdata(dev);
+
+ if (sup) {
+ uart_remove_one_port(&sprd_uart_driver, &sup->port);
+ sprd_port[sup->port.line] = NULL;
+ sprd_ports_num--;
+ }
+
+ if (!sprd_ports_num)
+ uart_unregister_driver(&sprd_uart_driver);
+
+ return 0;
+}
+
+static int sprd_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ struct uart_port *up;
+ struct clk *clk;
+ int irq;
+ int index;
+ int ret;
+
+ for (index = 0; index < ARRAY_SIZE(sprd_port); index++)
+ if (sprd_port[index] == NULL)
+ break;
+
+ if (index == ARRAY_SIZE(sprd_port))
+ return -EBUSY;
+
+ index = sprd_probe_dt_alias(index, &pdev->dev);
+
+ sprd_port[index] = devm_kzalloc(&pdev->dev,
+ sizeof(*sprd_port[index]), GFP_KERNEL);
+ if (!sprd_port[index])
+ return -ENOMEM;
+
+ pdev->id = index;
+
+ up = &sprd_port[index]->port;
+ up->dev = &pdev->dev;
+ up->line = index;
+ up->type = PORT_SPRD;
+ up->iotype = SERIAL_IO_PORT;
+ up->uartclk = SPRD_DEF_RATE;
+ up->fifosize = SPRD_FIFO_SIZE;
+ up->ops = &serial_sprd_ops;
+ up->flags = UPF_BOOT_AUTOCONF;
+
+ clk = devm_clk_get(&pdev->dev, NULL);
+ if (!IS_ERR(clk))
+ up->uartclk = clk_get_rate(clk);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "not provide mem resource\n");
+ return -ENODEV;
+ }
+ up->mapbase = res->start;
+ up->membase = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(up->membase))
+ return PTR_ERR(up->membase);
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "not provide irq resource\n");
+ return -ENODEV;
+ }
+ up->irq = irq;
+
+ if (!sprd_ports_num) {
+ ret = uart_register_driver(&sprd_uart_driver);
+ if (ret < 0) {
+ pr_err("Failed to register SPRD-UART driver\n");
+ return ret;
+ }
+ }
+ sprd_ports_num++;
+
+ ret = uart_add_one_port(&sprd_uart_driver, up);
+ if (ret) {
+ sprd_port[index] = NULL;
+ sprd_remove(pdev);
+ }
+
+ platform_set_drvdata(pdev, up);
+
+ return ret;
+}
+
+static int sprd_suspend(struct device *dev)
+{
+ int id = to_platform_device(dev)->id;
+ struct uart_port *port = &sprd_port[id]->port;
+
+ uart_suspend_port(&sprd_uart_driver, port);
+
+ return 0;
+}
+
+static int sprd_resume(struct device *dev)
+{
+ int id = to_platform_device(dev)->id;
+ struct uart_port *port = &sprd_port[id]->port;
+
+ uart_resume_port(&sprd_uart_driver, port);
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(sprd_pm_ops, sprd_suspend, sprd_resume);
+
+static const struct of_device_id serial_ids[] = {
+ {.compatible = "sprd,sc9836-uart",},
+ {}
+};
+
+static struct platform_driver sprd_platform_driver = {
+ .probe = sprd_probe,
+ .remove = sprd_remove,
+ .driver = {
+ .name = "sprd_serial",
+ .of_match_table = of_match_ptr(serial_ids),
+ .pm = &sprd_pm_ops,
+ },
+};
+
+module_platform_driver(sprd_platform_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Spreadtrum SoC serial driver series");
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index c172180..7e6eb39 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -248,4 +248,7 @@
/* MESON */
#define PORT_MESON 109
+/* SPRD SERIAL */
+#define PORT_SPRD 110
+
#endif /* _UAPILINUX_SERIAL_CORE_H */
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v6 2/2] tty/serial: Add Spreadtrum sc9836-uart driver support
From: Peter Hurley @ 2015-01-23 13:12 UTC (permalink / raw)
To: Lyra Zhang
Cc: Chunyan Zhang, gregkh@linuxfoundation.org, robh+dt@kernel.org,
Mark Rutland, Arnd Bergmann, gnomes@lxorguk.ukuu.org.uk,
Shawn Guo, Pawel Moll, ijc+devicetree@hellion.org.uk, Kumar Gala,
jslaby@suse.cz, Grant Likely, Heiko Stübner,
jason@lakedaemon.net, florian.vaussard@epfl.ch, andrew@lunn.ch,
Hayato Suzuki, antonynpavlov@gmail.com, Orson Zhai,
geng.ren@spreadtrum.com
In-Reply-To: <CAAfSe-s2Y_JSiR+HX4OOF_62q3JYN5k81JZuNe9CP9=shBpZzA@mail.gmail.com>
On 01/23/2015 02:23 AM, Lyra Zhang wrote:
> Hi, Peter
>
> Many thanks to you for reviewing so carefully and giving us so many
> suggestions and so clear explanations.
:)
> I'll address all of your comments and send an updated patch soon.
>
> On Fri, Jan 23, 2015 at 11:57 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
[...]
>>> +static void sprd_set_termios(struct uart_port *port,
>>> + struct ktermios *termios,
>>> + struct ktermios *old)
>>> +{
>>> + unsigned int baud, quot;
>>> + unsigned int lcr, fc;
>>> + unsigned long flags;
>>> +
>>> + /* ask the core to calculate the divisor for us */
>>> + baud = uart_get_baud_rate(port, termios, old, 1200, 3000000);
>> ^^^^ ^^^^^^
>> mabye derive these from uartclk?
>
> I'm afraid I can't understand very clearly, Could you explain more
> details please?
Is the fixed clock divider == 8 and the uartclk == 26000000 ?
If so,
baud = uartclk / 8 = 3250000
I see now this is clamping baud inside the maximum, so this is fine.
Please disregard my comment.
[...]
>>> +static int sprd_probe(struct platform_device *pdev)
>>> +{
>>> + struct resource *res;
>>> + struct uart_port *up;
>>> + struct clk *clk;
>>> + int irq;
>>> + int index;
>>> + int ret;
>>> +
>>> + for (index = 0; index < ARRAY_SIZE(sprd_port); index++)
>>> + if (sprd_port[index] == NULL)
>>> + break;
>>> +
>>> + if (index == ARRAY_SIZE(sprd_port))
>>> + return -EBUSY;
>>> +
>>> + index = sprd_probe_dt_alias(index, &pdev->dev);
>>> +
>>> + sprd_port[index] = devm_kzalloc(&pdev->dev,
>>> + sizeof(*sprd_port[index]), GFP_KERNEL);
>>> + if (!sprd_port[index])
>>> + return -ENOMEM;
>>> +
>>> + up = &sprd_port[index]->port;
>>> + up->dev = &pdev->dev;
>>> + up->line = index;
>>> + up->type = PORT_SPRD;
>>> + up->iotype = SERIAL_IO_PORT;
>>> + up->uartclk = SPRD_DEF_RATE;
>>> + up->fifosize = SPRD_FIFO_SIZE;
>>> + up->ops = &serial_sprd_ops;
>>> + up->flags = ASYNC_BOOT_AUTOCONF;
>> ^^^^^^^^^
>> UPF_BOOT_AUTOCONF
>>
>> sparse will catch errors like this. See Documentation/sparse.txt
>
> you mean we should use UPF_BOOT_AUTOCONF, right?
Yes. Only UPF_* flag definitions should be used with the uart_port.flags
field.
My comment regarding the sparse tool and documentation is because the
flags field and UPF_* definitions use a type mechanism to generate
warnings using the sparse tool if regular integer values are used
with the flags field.
The type mechanism was specifically introduced to catch using ASYNC_*
definitions with the uart_port.flags field.
[...]
>>> +static int sprd_suspend(struct device *dev)
>>> +{
>>> + int id = to_platform_device(dev)->id;
>>> + struct uart_port *port = &sprd_port[id]->port;
>>
>> I'm a little confused regarding the port indexing;
>> is platform_device->id == line ? Where did that happen?
>>
>
> Oh, I'll change to assign platform_device->id with port->line in probe()
I apologize; I should have made my comment clearer.
The ->id should not be assigned.
Replace
int id = to_platform_device(dev)->id;
struct uart_port *port = &sprd_port[id]->port;
uart_suspend_port(&sprd_uart_driver, port);
with
struct sprd_uart_port *sup = dev_get_drvdata(dev);
uart_suspend_port(&sprd_uart_driver, &sup->port);
I know it's not obvious but platform_get/set_drvdata() is really
dev_get/set_drvdata() using the embedded struct device dev.
>
>>
>>> +
>>> + uart_suspend_port(&sprd_uart_driver, port);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int sprd_resume(struct device *dev)
>>> +{
>>> + int id = to_platform_device(dev)->id;
>>> + struct uart_port *port = &sprd_port[id]->port;
>>> +
>>> + uart_resume_port(&sprd_uart_driver, port);
same here
>>> + return 0;
^ permalink raw reply
* Re: [PATCH 01/13] kdbus: add documentation
From: Greg Kroah-Hartman @ 2015-01-23 13:19 UTC (permalink / raw)
To: Ahmed S. Darwish
Cc: arnd, ebiederm, gnomes, teg, jkosina, luto, linux-api,
linux-kernel, daniel, dh.herrmann, tixxdz,
Michael Kerrisk (man-pages), Linus Torvalds, Daniel Mack
In-Reply-To: <20150123062820.GB7949@Darwish.PC>
On Fri, Jan 23, 2015 at 08:28:20AM +0200, Ahmed S. Darwish wrote:
> On Fri, Jan 16, 2015 at 11:16:05AM -0800, Greg Kroah-Hartman wrote:
> > From: Daniel Mack <daniel@zonque.org>
> >
> > kdbus is a system for low-latency, low-overhead, easy to use
> > interprocess communication (IPC).
> >
> > The interface to all functions in this driver is implemented via ioctls
> > on files exposed through a filesystem called 'kdbusfs'. The default
> > mount point of kdbusfs is /sys/fs/kdbus.
>
> Pardon my ignorance, but we've always been told that adding
> new ioctl()s to the kernel is a very big no-no. But given
> the seniority of the folks stewarding this kdbus effort,
> there must be a good rationale ;-)
>
> So, can the rationale behind introducing new ioctl()s be
> further explained? It would be even better if it's included
> in the documentation patch itself.
The main reason to use an ioctl is that you want to atomically set
and/or get something "complex" through the user/kernel boundary. For
simple device attributes, sysfs works great, for configuring devices,
configfs works great, but for data streams / structures / etc. an ioctl
is the correct thing to use.
Examples of new ioctls being added to the kernel are all over the
place, look at all of the special-purpose ioctls the filesystems keep
creating (they aren't adding new syscalls), look at the monstrosity that
is the DRM layer, look at other complex things like openvswitch, or
"simpler" device-specific interfaces like the MEI one, or even more
complex ones like the MMC interface. These are all valid uses of ioctls
as they are device/filesystem specific ways to interact with the kernel.
The thing is, almost no one pays attention to these new ioctls as they
are domain-specific interfaces, with open userspace programs talking to
them, and they work well. ioctl is a powerful and useful interface, and
if we were to suddenly require no new ioctls, and require everything to
be a syscall, we would do nothing except make apis more complex (hint,
you now have to do extra validation on your file descriptor passed to
you to determine if it really is what you can properly operate your
ioctl on), and cause no real benefit at all.
Yes, people abuse ioctls at times, but really, they are needed.
And remember, I was one of the people who long ago thought we should not
be adding more ioctls, but I have seen the folly of my ways, and chalk
it up to youthful ignorance :)
Hope this helps explain things better,
greg k-h
^ permalink raw reply
* Re: [PATCH 01/13] kdbus: add documentation
From: Greg Kroah-Hartman @ 2015-01-23 13:29 UTC (permalink / raw)
To: Ahmed S. Darwish
Cc: arnd-r2nGTMty4D4, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io, teg-B22kvLQNl6c,
jkosina-AlSwsSmVLrQ, luto-kltTT9wpgjJwATOyAt5JVQ,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
daniel-cYrQPVfZooxQFI55V6+gNQ, dh.herrmann-Re5JQEeQqe8AvxtiuMwx3w,
tixxdz-Umm1ozX2/EEdnm+yROfE0A, Michael Kerrisk (man-pages),
Linus Torvalds, Daniel Mack
In-Reply-To: <20150123131946.GA26302-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
On Fri, Jan 23, 2015 at 09:19:46PM +0800, Greg Kroah-Hartman wrote:
> On Fri, Jan 23, 2015 at 08:28:20AM +0200, Ahmed S. Darwish wrote:
> > On Fri, Jan 16, 2015 at 11:16:05AM -0800, Greg Kroah-Hartman wrote:
> > > From: Daniel Mack <daniel-cYrQPVfZoowdnm+yROfE0A@public.gmane.org>
> > >
> > > kdbus is a system for low-latency, low-overhead, easy to use
> > > interprocess communication (IPC).
> > >
> > > The interface to all functions in this driver is implemented via ioctls
> > > on files exposed through a filesystem called 'kdbusfs'. The default
> > > mount point of kdbusfs is /sys/fs/kdbus.
> >
> > Pardon my ignorance, but we've always been told that adding
> > new ioctl()s to the kernel is a very big no-no. But given
> > the seniority of the folks stewarding this kdbus effort,
> > there must be a good rationale ;-)
> >
> > So, can the rationale behind introducing new ioctl()s be
> > further explained? It would be even better if it's included
> > in the documentation patch itself.
>
> The main reason to use an ioctl is that you want to atomically set
> and/or get something "complex" through the user/kernel boundary. For
> simple device attributes, sysfs works great, for configuring devices,
> configfs works great, but for data streams / structures / etc. an ioctl
> is the correct thing to use.
>
> Examples of new ioctls being added to the kernel are all over the
> place, look at all of the special-purpose ioctls the filesystems keep
> creating (they aren't adding new syscalls), look at the monstrosity that
> is the DRM layer, look at other complex things like openvswitch, or
> "simpler" device-specific interfaces like the MEI one, or even more
> complex ones like the MMC interface.
Oops, I meant, MIC, not MMC, sorry about that.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox