* Re: [PATCH 4/5] dt-bindings: serial: sprd: Add dma properties to support DMA mode
From: Baolin Wang @ 2019-03-01 9:42 UTC (permalink / raw)
To: Rob Herring
Cc: Greg KH, jslaby, Mark Rutland, Orson Zhai, Chunyan Zhang,
Mark Brown, lanqing.liu, linux-serial, LKML, DTML
In-Reply-To: <20190228195319.GA13133@bogus>
On Fri, 1 Mar 2019 at 03:53, Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Feb 19, 2019 at 03:31:14PM +0800, Baolin Wang wrote:
> > From: Lanqing Liu <lanqing.liu@unisoc.com>
> >
> > This patch adds dmas and dma-names properties for the UART DMA mode.
> >
> > Signed-off-by: Lanqing Liu <lanqing.liu@unisoc.com>
> > Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> > ---
> > .../devicetree/bindings/serial/sprd-uart.txt | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/serial/sprd-uart.txt b/Documentation/devicetree/bindings/serial/sprd-uart.txt
> > index 6eb5863..9ac28f6 100644
> > --- a/Documentation/devicetree/bindings/serial/sprd-uart.txt
> > +++ b/Documentation/devicetree/bindings/serial/sprd-uart.txt
> > @@ -15,12 +15,18 @@ Required properties:
> > UART clock and source clock are optional properties, but enable clock
> > is required.
> >
> > +Optional properties:
> > +- dma-names: Should contain "tx" for transmit and "rx" for receive channels.
>
> The order here doesn't match the example.
Ah, yes, will update new version to fix this. Thanks.
>
> > +- dmas: A list of dma specifiers, one for each entry in dma-names.
> > +
> > Example:
> > uart0: serial@0 {
> > compatible = "sprd,sc9860-uart",
> > "sprd,sc9836-uart";
> > reg = <0x0 0x100>;
> > interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
> > + dma-names = "rx", "tx";
> > + dmas = <&ap_dma 19 19>, <&ap_dma 20 20>;
> > clock-names = "enable", "uart", "source";
> > clocks = <&clk_ap_apb_gates 9>, <&clk_uart0>, <&ext_26m>;
> > };
> > --
> > 1.7.9.5
> >
--
Baolin Wang
Best Regards
^ permalink raw reply
* [PATCH v2] tty: xilinx_uartps: Correct return value in probe
From: Rajan Vaja @ 2019-03-01 9:37 UTC (permalink / raw)
To: gregkh, jslaby, michals
Cc: rajan.vaja, linux-kernel, JOLLYS, anirudh, linux-serial,
linux-arm-kernel
In-Reply-To: <1543927882-15091-1-git-send-email-rajan.vaja@xilinx.com>
Existing driver checks for alternate clock if devm_clk_get() fails
and returns error code for last clock failure. If xilinx_uartps is
called before clock driver, devm_clk_get() returns -EPROBE_DEFER.
In this case, probe should not check for alternate clock as main
clock is already present in DTS and return -EPROBE_DEFER only.
This patch fixes it by not checking for alternate clock when main
clock get returns -EPROBE_DEFER.
Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
---
Changes in v2:
* Handle dynamic port allocation error cases
---
drivers/tty/serial/xilinx_uartps.c | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 094f295..f7ae73e 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1547,27 +1547,33 @@ static int cdns_uart_probe(struct platform_device *pdev)
}
cdns_uart_data->pclk = devm_clk_get(&pdev->dev, "pclk");
+ if (PTR_ERR(cdns_uart_data->pclk) == -EPROBE_DEFER) {
+ rc = PTR_ERR(cdns_uart_data->pclk);
+ goto err_out_unregister_driver;
+ }
+
if (IS_ERR(cdns_uart_data->pclk)) {
cdns_uart_data->pclk = devm_clk_get(&pdev->dev, "aper_clk");
- if (!IS_ERR(cdns_uart_data->pclk))
- dev_err(&pdev->dev, "clock name 'aper_clk' is deprecated.\n");
+ if (IS_ERR(cdns_uart_data->pclk)) {
+ rc = PTR_ERR(cdns_uart_data->pclk);
+ goto err_out_unregister_driver;
+ }
+ dev_err(&pdev->dev, "clock name 'aper_clk' is deprecated.\n");
}
- if (IS_ERR(cdns_uart_data->pclk)) {
- dev_err(&pdev->dev, "pclk clock not found.\n");
- rc = PTR_ERR(cdns_uart_data->pclk);
+
+ cdns_uart_data->uartclk = devm_clk_get(&pdev->dev, "uart_clk");
+ if (PTR_ERR(cdns_uart_data->uartclk) == -EPROBE_DEFER) {
+ rc = PTR_ERR(cdns_uart_data->uartclk);
goto err_out_unregister_driver;
}
- cdns_uart_data->uartclk = devm_clk_get(&pdev->dev, "uart_clk");
if (IS_ERR(cdns_uart_data->uartclk)) {
cdns_uart_data->uartclk = devm_clk_get(&pdev->dev, "ref_clk");
- if (!IS_ERR(cdns_uart_data->uartclk))
- dev_err(&pdev->dev, "clock name 'ref_clk' is deprecated.\n");
- }
- if (IS_ERR(cdns_uart_data->uartclk)) {
- dev_err(&pdev->dev, "uart_clk clock not found.\n");
- rc = PTR_ERR(cdns_uart_data->uartclk);
- goto err_out_unregister_driver;
+ if (IS_ERR(cdns_uart_data->uartclk)) {
+ rc = PTR_ERR(cdns_uart_data->uartclk);
+ goto err_out_unregister_driver;
+ }
+ dev_err(&pdev->dev, "clock name 'ref_clk' is deprecated.\n");
}
rc = clk_prepare_enable(cdns_uart_data->pclk);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 4/5] dt-bindings: serial: sprd: Add dma properties to support DMA mode
From: Rob Herring @ 2019-02-28 19:53 UTC (permalink / raw)
To: Baolin Wang
Cc: gregkh, jslaby, mark.rutland, orsonzhai, zhang.lyra, broonie,
lanqing.liu, linux-serial, linux-kernel, devicetree
In-Reply-To: <8f0a032f9bcce28bb0d147f9646950f64fc22a74.1550560916.git.baolin.wang@linaro.org>
On Tue, Feb 19, 2019 at 03:31:14PM +0800, Baolin Wang wrote:
> From: Lanqing Liu <lanqing.liu@unisoc.com>
>
> This patch adds dmas and dma-names properties for the UART DMA mode.
>
> Signed-off-by: Lanqing Liu <lanqing.liu@unisoc.com>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
> .../devicetree/bindings/serial/sprd-uart.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/serial/sprd-uart.txt b/Documentation/devicetree/bindings/serial/sprd-uart.txt
> index 6eb5863..9ac28f6 100644
> --- a/Documentation/devicetree/bindings/serial/sprd-uart.txt
> +++ b/Documentation/devicetree/bindings/serial/sprd-uart.txt
> @@ -15,12 +15,18 @@ Required properties:
> UART clock and source clock are optional properties, but enable clock
> is required.
>
> +Optional properties:
> +- dma-names: Should contain "tx" for transmit and "rx" for receive channels.
The order here doesn't match the example.
> +- dmas: A list of dma specifiers, one for each entry in dma-names.
> +
> Example:
> uart0: serial@0 {
> compatible = "sprd,sc9860-uart",
> "sprd,sc9836-uart";
> reg = <0x0 0x100>;
> interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
> + dma-names = "rx", "tx";
> + dmas = <&ap_dma 19 19>, <&ap_dma 20 20>;
> clock-names = "enable", "uart", "source";
> clocks = <&clk_ap_apb_gates 9>, <&clk_uart0>, <&ext_26m>;
> };
> --
> 1.7.9.5
>
^ permalink raw reply
* Re: [PATCH 2/5] dt-bindings: serial: sprd: Add clocks and clocks-names properties
From: Rob Herring @ 2019-02-28 19:52 UTC (permalink / raw)
Cc: gregkh, jslaby, robh+dt, mark.rutland, orsonzhai, zhang.lyra,
baolin.wang, broonie, lanqing.liu, linux-serial, linux-kernel,
devicetree
In-Reply-To: <c2b4db380c044bebe9fdf546b8a9f3aa351c6867.1550560916.git.baolin.wang@linaro.org>
On Tue, 19 Feb 2019 15:31:12 +0800, Baolin Wang wrote:
> From: Lanqing Liu <lanqing.liu@unisoc.com>
>
> This patch adds clocks and clocks-names properties, which are used to do
> power management for our UART driver.
>
> Signed-off-by: Lanqing Liu <lanqing.liu@unisoc.com>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
> .../devicetree/bindings/serial/sprd-uart.txt | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH] tty/serial: Add a serial port simulator
From: minyard @ 2019-02-28 19:39 UTC (permalink / raw)
To: Greg Kroah-Hartman, linux-serial; +Cc: minyard, linux-kernel, Corey Minyard
From: Corey Minyard <cminyard@mvista.com>
This creates simulated serial ports, both as echo devices and pipe
devices. The driver reasonably approximates the serial port speed
and simulates some modem control lines. It allows error injection
and dirct control of modem control lines.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
I wrote this for doing automated tests on ser2net and the gensio library,
but it's general purpose and other people wanting to test serial port
programs might find it handy. It has been invaluable for me.
Documentation/serial/serialsim.rst | 149 ++++
MAINTAINERS | 7 +
drivers/tty/serial/Kconfig | 10 +
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/serialsim.c | 1086 ++++++++++++++++++++++++++++
include/uapi/linux/serialsim.h | 28 +
6 files changed, 1281 insertions(+)
create mode 100644 Documentation/serial/serialsim.rst
create mode 100644 drivers/tty/serial/serialsim.c
create mode 100644 include/uapi/linux/serialsim.h
diff --git a/Documentation/serial/serialsim.rst b/Documentation/serial/serialsim.rst
new file mode 100644
index 000000000000..655e10b4908e
--- /dev/null
+++ b/Documentation/serial/serialsim.rst
@@ -0,0 +1,149 @@
+.. SPDX-License-Identifier: GPL-2.0+
+=====================================
+serialsim - A kernel serial simualtor
+=====================================
+
+:Author: Corey Minyard <minyard@mvista.com> / <minyard@acm.org>
+
+The serialsim device is a serial simulator with echo and pipe devices.
+It is quite useful for testing programs that use serial ports.
+
+This attempts to emulate a basic serial device. It uses the baud rate
+and sends the bytes through the loopback or pipe at approximately the
+speed it would on a normal serial device.
+
+There is a python interface to the special ioctls for controlling the
+remote end of the termios in addition to the standard ioctl interface
+documented below. See https://github.com/cminyard/serialsim
+
+=====
+Using
+=====
+
+The serialsim.ko module creates two types of devices. Echo devices
+simply echo back the data to the same device. These devices will
+appear as /dev/ttyEcho<n>.
+
+Pipe devices will transfer the data between two devices. The
+devices will appear as /dev/ttyPipeA<n> and /dev/ttyPipeB<n>. And
+data written to PipeA reads from PipeB, and vice-versa.
+
+You may create an arbitrary number of devices by setting the
+nr_echo ports and nr_pipe_ports module parameters. The default is
+four for both.
+
+This driver supports modifying the modem control lines and
+injecting various serial errors. It also supports a simulated null
+modem between the two pipes, or in a loopback on the echo device.
+
+By default a pipe or echo comes up in null modem configuration,
+meaning that the DTR line is hooked to the DSR and CD lines on the
+other side and the RTS line on one side is hooked to the CTS line
+on the other side.
+
+The RTS and CTS lines don't currently do anything for flow control.
+
+You can modify null modem and control the lines individually
+through an interface in /sys/class/tty/ttyECHO<n>/ctrl,
+/sys/class/tty/ttyPipeA<n>/ctrl, and
+/sys/class/tty/ttyPipeB<n>/ctrl. The following may be written to
+those files:
+
+[+-]nullmodem
+ enable/disable null modem
+
+[+-]cd
+ enable/disable Carrier Detect (no effect if +nullmodem)
+
+[+-]dsr
+ enable/disable Data Set Ready (no effect if +nullmodem)
+
+[+-]cts
+ enable/disable Clear To Send (no effect if +nullmodem)
+
+[+-]ring
+ enable/disable Ring
+
+frame
+ inject a frame error on the next byte
+
+parity
+ inject a parity error on the next byte
+
+overrun
+ inject an overrun error on the next byte
+
+The contents of the above files has the following format:
+
+tty[Echo|PipeA|PipeB]<n>
+ <mctrl values>
+
+where <mctrl values> is the modem control values above (not frame,
+parity, or overrun) with the following added:
+
+[+-]dtr
+ value of the Data Terminal Ready
+
+[+-]rts
+ value of the Request To Send
+
+The above values are not settable through this interface, they are
+set through the serial port interface itself.
+
+So, for instance, ttyEcho0 comes up in the following state::
+
+ # cat /sys/class/tty/ttyEcho0/ctrl
+ ttyEcho0: +nullmodem -cd -dsr -cts -ring -dtr -rts
+
+If something connects, it will become::
+
+ ttyEcho0: +nullmodem +cd +dsr +cts -ring +dtr +rts
+
+To enable ring::
+
+ # echo "+ring" >/sys/class/tty/ttyEcho0/ctrl
+ # cat /sys/class/tty/ttyEcho0/ctrl
+ ttyEcho0: +nullmodem +cd +dsr +cts +ring +dtr +rts
+
+Now disable NULL modem and the CD line::
+
+ # echo "-nullmodem -cd" >/sys/class/tty/ttyEcho0/ctrl
+ # cat /sys/class/tty/ttyEcho0/ctrl
+ ttyEcho0: -nullmodem -cd -dsr -cts +ring -dtr -rts
+
+Note that these settings are for the side you are modifying. So if
+you set nullmodem on ttyPipeA0, that controls whether the DTR/RTS
+lines from ttyPipeB0 affect ttyPipeA0. It doesn't affect ttyPipeB's
+modem control lines.
+
+The PIPEA and PIPEB devices also have the ability to set these
+values for the other end via an ioctl. The following ioctls are
+available:
+
+TIOCSERSNULLMODEM
+ Set the null modem value, the arg is a boolean.
+
+TIOCSERSREMMCTRL
+ Set the modem control lines, bits 16-31 of the arg is
+ a 16-bit mask telling which values to set, bits 0-15 are the
+ actual values. Settable values are TIOCM_CAR, TIOCM_CTS,
+ TIOCM_DSR, and TIOC_RNG. If NULLMODEM is set to true, then only
+ TIOC_RNG is settable. The DTR and RTS lines are not here, you can
+ set them through the normal interface.
+
+TIOCSERSREMERR
+ Send an error or errors on the next sent byte. arg is
+ a bitwise OR of (1 << TTY_xxx). Allowed errors are TTY_BREAK,
+ TTY_FRAME, TTY_PARITY, and TTY_OVERRUN.
+
+TIOCSERGREMTERMIOS
+ Return the termios structure for the other side of the pipe.
+ arg is a pointer to a standard termios struct.
+
+TIOCSERGREMRS485
+ Return the remote RS485 settings, arg is a pointer to a struct
+ serial_rs485.
+
+Note that unlike the sysfs interface, these ioctls affect the other
+end. So setting nullmodem on the ttyPipeB0 interface sets whether
+the DTR/RTS lines on ttyPipeB0 affect ttyPipeA0.
diff --git a/MAINTAINERS b/MAINTAINERS
index 9919840d54cd..aa1eb3c07f44 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13702,6 +13702,13 @@ L: linux-media@vger.kernel.org
S: Maintained
F: drivers/media/rc/serial_ir.c
+SERIAL PORT SIMULATOR
+M: Corey Minyard <cminyard@mvista.com>
+S: Maintained
+F: Documentation/serial/serialsim.rst
+F: drivers/tty/serial/serialsim.c
+F: include/uapi/linux/serialsim.h
+
SFC NETWORK DRIVER
M: Solarflare linux maintainers <linux-net-drivers@solarflare.com>
M: Edward Cree <ecree@solarflare.com>
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 089a6f285d5e..789113055e2f 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1565,4 +1565,14 @@ endmenu
config SERIAL_MCTRL_GPIO
tristate
+config SERIAL_SIMULATOR
+ tristate "Serial port simulator"
+ default n
+ help
+ Support for a simulated device that behaves like a normal
+ serial port. It has echo devices and pipe devices. This
+ is useful for testing programs that use serial ports.
+
+ If unsure, say N.
+
endif # TTY
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index 1511e8a9f856..dbd9ee0f1c6b 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -91,6 +91,7 @@ obj-$(CONFIG_SERIAL_PIC32) += pic32_uart.o
obj-$(CONFIG_SERIAL_MPS2_UART) += mps2-uart.o
obj-$(CONFIG_SERIAL_OWL) += owl-uart.o
obj-$(CONFIG_SERIAL_RDA) += rda-uart.o
+obj-$(CONFIG_SERIAL_SIMULATOR) += serialsim.o
# GPIOLIB helpers for modem control lines
obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.o
diff --git a/drivers/tty/serial/serialsim.c b/drivers/tty/serial/serialsim.c
new file mode 100644
index 000000000000..d4b3414a0df0
--- /dev/null
+++ b/drivers/tty/serial/serialsim.c
@@ -0,0 +1,1086 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * serialsim - Emulate a serial device in a loopback and/or pipe
+ *
+ * See the docs for this for more details.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/ioport.h>
+#include <linux/init.h>
+#include <linux/serial.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+#include <linux/device.h>
+#include <linux/serial_core.h>
+#include <linux/kthread.h>
+#include <linux/hrtimer.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/ctype.h>
+#include <linux/string.h>
+#include <linux/uaccess.h>
+
+#include <linux/serialsim.h>
+
+#define PORT_SERIALECHO 72549
+#define PORT_SERIALPIPEA 72550
+#define PORT_SERIALPIPEB 72551
+
+#ifdef CONFIG_HIGH_RES_TIMERS
+#define SERIALSIM_WAKES_PER_SEC 1000
+#else
+#define SERIALSIM_WAKES_PER_SEC HZ
+#endif
+
+#define SERIALSIM_XBUFSIZE 32
+
+/* For things to send on the line, in flags field. */
+#define DO_FRAME_ERR (1 << TTY_FRAME)
+#define DO_PARITY_ERR (1 << TTY_PARITY)
+#define DO_OVERRUN_ERR (1 << TTY_OVERRUN)
+#define DO_BREAK (1 << TTY_BREAK)
+#define FLAGS_MASK (DO_FRAME_ERR | DO_PARITY_ERR | DO_OVERRUN_ERR | DO_BREAK)
+
+struct serialsim_intf {
+ struct uart_port port;
+
+ /*
+ * This is my transmit buffer, my thread picks this up and
+ * injects them into the other port's uart.
+ */
+ unsigned char xmitbuf[SERIALSIM_XBUFSIZE];
+ struct circ_buf buf;
+
+ /* Error flags to send. */
+ bool break_reported;
+ unsigned int flags;
+
+ /* Modem state. */
+ unsigned int mctrl;
+ bool do_null_modem;
+ spinlock_t mctrl_lock;
+ struct tasklet_struct mctrl_tasklet;
+
+ /* My transmitter is enabled. */
+ bool tx_enabled;
+
+ /* I can receive characters. */
+ bool rx_enabled;
+
+ /* Is the port registered with the uart driver? */
+ bool registered;
+
+ /*
+ * The serial echo port on the other side of this pipe (or points
+ * to myself in loopback mode.
+ */
+ struct serialsim_intf *ointf;
+
+ unsigned int div;
+ unsigned int bytes_per_interval;
+ unsigned int per_interval_residual;
+
+ struct ktermios termios;
+
+ const char *threadname;
+ struct task_struct *thread;
+
+ struct serial_rs485 rs485;
+};
+
+#define circ_sbuf_space(buf) CIRC_SPACE((buf)->head, (buf)->tail, \
+ SERIALSIM_XBUFSIZE)
+#define circ_sbuf_empty(buf) ((buf)->head == (buf)->tail)
+#define circ_sbuf_next(idx) (((idx) + 1) % SERIALSIM_XBUFSIZE)
+
+static struct serialsim_intf *serialsim_port_to_intf(struct uart_port *port)
+{
+ return container_of(port, struct serialsim_intf, port);
+}
+
+static unsigned int serialsim_tx_empty(struct uart_port *port)
+{
+ struct serialsim_intf *intf = serialsim_port_to_intf(port);
+
+ if (circ_sbuf_empty(&intf->buf))
+ return TIOCSER_TEMT;
+ return 0;
+}
+
+/*
+ * We have to lock multiple locks, make sure to do it in the same order all
+ * the time.
+ */
+static void serialsim_null_modem_lock_irq(struct serialsim_intf *intf)
+{
+ spin_lock_irq(&intf->port.lock);
+ if (intf == intf->ointf) {
+ spin_lock(&intf->mctrl_lock);
+ } else if (intf < intf->ointf) {
+ spin_lock(&intf->mctrl_lock);
+ spin_lock(&intf->ointf->mctrl_lock);
+ } else {
+ spin_lock(&intf->ointf->mctrl_lock);
+ spin_lock(&intf->mctrl_lock);
+ }
+}
+
+static void serialsim_null_modem_unlock_irq(struct serialsim_intf *intf)
+{
+ if (intf == intf->ointf) {
+ spin_unlock(&intf->mctrl_lock);
+ } else {
+ /* Order doesn't matter here. */
+ spin_unlock(&intf->mctrl_lock);
+ spin_unlock(&intf->ointf->mctrl_lock);
+ }
+ spin_unlock_irq(&intf->port.lock);
+}
+
+/*
+ * This must be called holding intf->port.lock and intf->mctrl_lock.
+ */
+static void _serialsim_set_modem_lines(struct serialsim_intf *intf,
+ unsigned int mask,
+ unsigned int new_mctrl)
+{
+ unsigned int changes;
+ unsigned int mctrl = (intf->mctrl & ~mask) | (new_mctrl & mask);
+
+ if (mctrl == intf->mctrl)
+ return;
+
+ if (!intf->rx_enabled) {
+ intf->mctrl = mctrl;
+ return;
+ }
+
+ changes = mctrl ^ intf->mctrl;
+ intf->mctrl = mctrl;
+ if (changes & TIOCM_CAR)
+ uart_handle_dcd_change(&intf->port, mctrl & TIOCM_CAR);
+ if (changes & TIOCM_CTS)
+ uart_handle_cts_change(&intf->port, mctrl & TIOCM_CTS);
+ if (changes & TIOCM_RNG)
+ intf->port.icount.rng++;
+ if (changes & TIOCM_DSR)
+ intf->port.icount.dsr++;
+}
+
+#define NULL_MODEM_MCTRL (TIOCM_CAR | TIOCM_CTS | TIOCM_DSR)
+#define LOCAL_MCTRL (NULL_MODEM_MCTRL | TIOCM_RNG)
+
+/*
+ * Must be called holding intf->port.lock, intf->mctrl_lock, and
+ * intf->ointf.mctrl_lock.
+ */
+static void serialsim_handle_null_modem_update(struct serialsim_intf *intf)
+{
+ unsigned int mctrl = 0;
+
+ /* Pull the values from the remote side for myself. */
+ if (intf->ointf->mctrl & TIOCM_DTR)
+ mctrl |= TIOCM_CAR | TIOCM_DSR;
+ if (intf->ointf->mctrl & TIOCM_RTS)
+ mctrl |= TIOCM_CTS;
+
+ _serialsim_set_modem_lines(intf, NULL_MODEM_MCTRL, mctrl);
+}
+
+static void serialsim_set_null_modem(struct serialsim_intf *intf, bool val)
+{
+ serialsim_null_modem_lock_irq(intf);
+
+ if (!!val == !!intf->do_null_modem)
+ goto out_unlock;
+
+ if (!val) {
+ intf->do_null_modem = false;
+ goto out_unlock;
+ }
+
+ /* Enabling NULL modem. */
+ intf->do_null_modem = true;
+
+ serialsim_handle_null_modem_update(intf);
+
+out_unlock:
+ serialsim_null_modem_unlock_irq(intf);
+}
+
+static void serialsim_set_modem_lines(struct serialsim_intf *intf,
+ unsigned int mask,
+ unsigned int new_mctrl)
+{
+ mask &= LOCAL_MCTRL;
+
+ spin_lock_irq(&intf->port.lock);
+ spin_lock(&intf->mctrl_lock);
+
+ if (intf->do_null_modem)
+ mask &= ~NULL_MODEM_MCTRL;
+
+ _serialsim_set_modem_lines(intf, mask, new_mctrl);
+
+ spin_unlock(&intf->mctrl_lock);
+ spin_unlock_irq(&intf->port.lock);
+}
+
+static void mctrl_tasklet(unsigned long data)
+{
+ struct serialsim_intf *intf = (void *) data;
+
+ serialsim_null_modem_lock_irq(intf);
+ if (intf->ointf->do_null_modem)
+ serialsim_handle_null_modem_update(intf);
+ serialsim_null_modem_unlock_irq(intf);
+}
+
+#define SETTABLE_MCTRL (TIOCM_RTS | TIOCM_DTR)
+
+static void serialsim_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+ struct serialsim_intf *intf = serialsim_port_to_intf(port);
+
+ spin_lock(&intf->mctrl_lock);
+ intf->mctrl &= ~SETTABLE_MCTRL;
+ intf->mctrl |= mctrl & SETTABLE_MCTRL;
+ spin_unlock(&intf->mctrl_lock);
+
+ /*
+ * We are called holding port->lock, but we must be able to claim
+ * intf->ointf->port.lock, and that can result in deadlock. So
+ * we have to run this elsewhere. Note that we run the other
+ * end's tasklet.
+ */
+ tasklet_schedule(&intf->ointf->mctrl_tasklet);
+}
+
+static unsigned int serialsim_get_mctrl(struct uart_port *port)
+{
+ struct serialsim_intf *intf = serialsim_port_to_intf(port);
+ unsigned int rv;
+
+ spin_lock(&intf->mctrl_lock);
+ rv = intf->mctrl;
+ spin_unlock(&intf->mctrl_lock);
+
+ return rv;
+}
+
+static void serialsim_stop_tx(struct uart_port *port)
+{
+ struct serialsim_intf *intf = serialsim_port_to_intf(port);
+
+ intf->tx_enabled = false;
+}
+
+static void serialsim_set_baud_rate(struct serialsim_intf *intf,
+ unsigned int baud, unsigned int cflag)
+{
+ unsigned int bits_per_char;
+
+ switch (cflag & CSIZE) {
+ case CS5:
+ bits_per_char = 7;
+ break;
+ case CS6:
+ bits_per_char = 8;
+ break;
+ case CS7:
+ bits_per_char = 9;
+ break;
+ default:
+ bits_per_char = 10;
+ break; /* CS8 and others. */
+ }
+ if (cflag & CSTOPB)
+ bits_per_char++;
+
+ intf->div = SERIALSIM_WAKES_PER_SEC * bits_per_char;
+ intf->bytes_per_interval = baud / intf->div;
+ intf->per_interval_residual = baud % intf->div;
+}
+
+static void serialsim_transfer_data(struct uart_port *port,
+ struct circ_buf *tbuf)
+{
+ struct circ_buf *cbuf = &port->state->xmit;
+
+ while (!uart_circ_empty(cbuf) && circ_sbuf_space(tbuf)) {
+ unsigned char c = cbuf->buf[cbuf->tail];
+
+ cbuf->tail = (cbuf->tail + 1) % UART_XMIT_SIZE;
+ tbuf->buf[tbuf->head] = c;
+ tbuf->head = circ_sbuf_next(tbuf->head);
+ port->icount.tx++;
+ }
+ if (uart_circ_chars_pending(cbuf) < WAKEUP_CHARS)
+ uart_write_wakeup(port);
+}
+
+static unsigned int serialsim_get_flag(struct serialsim_intf *intf,
+ unsigned int *status)
+{
+ unsigned int flags = intf->flags;
+
+ *status = flags;
+
+ /* Overrun is always reported through a different way. */
+ if (flags & DO_OVERRUN_ERR) {
+ intf->port.icount.overrun++;
+ intf->flags &= ~DO_OVERRUN_ERR;
+ }
+
+ if (flags & DO_BREAK && !intf->break_reported) {
+ intf->port.icount.brk++;
+ intf->break_reported = true;
+ return TTY_BREAK;
+ }
+ if (flags & DO_FRAME_ERR) {
+ intf->port.icount.frame++;
+ intf->flags &= ~DO_FRAME_ERR;
+ return TTY_FRAME;
+ }
+ if (flags & DO_PARITY_ERR) {
+ intf->port.icount.parity++;
+ intf->flags &= ~DO_PARITY_ERR;
+ return TTY_PARITY;
+ }
+
+ return TTY_NORMAL;
+}
+
+static void serialsim_set_flags(struct serialsim_intf *intf,
+ unsigned int status)
+{
+ spin_lock_irq(&intf->port.lock);
+ intf->flags |= status;
+ spin_unlock_irq(&intf->port.lock);
+}
+
+static void serialsim_thread_delay(void)
+{
+#ifdef CONFIG_HIGH_RES_TIMERS
+ ktime_t timeout;
+
+ timeout = ns_to_ktime(1000000000 / SERIALSIM_WAKES_PER_SEC);
+ set_current_state(TASK_INTERRUPTIBLE);
+ schedule_hrtimeout(&timeout, HRTIMER_MODE_REL);
+#else
+ schedule_timeout_interruptible(1);
+#endif
+}
+
+static int serialsim_thread(void *data)
+{
+ struct serialsim_intf *intf = data;
+ struct serialsim_intf *ointf = intf->ointf;
+ struct uart_port *port = &intf->port;
+ struct uart_port *oport = &ointf->port;
+ struct circ_buf *tbuf = &intf->buf;
+ unsigned int residual = 0;
+
+ while (!kthread_should_stop()) {
+ unsigned int to_send;
+ unsigned int div;
+ unsigned char buf[SERIALSIM_XBUFSIZE];
+ unsigned int pos = 0;
+ unsigned int flag;
+ unsigned int status = 0;
+
+ spin_lock_irq(&oport->lock);
+ if (ointf->tx_enabled)
+ /*
+ * Move bytes from the other port's transmit buffer to
+ * the interface buffer.
+ */
+ serialsim_transfer_data(oport, tbuf);
+ spin_unlock_irq(&oport->lock);
+
+ /*
+ * Move from the interface buffer into the local
+ * buffer based on the simulated serial speed.
+ */
+ to_send = intf->bytes_per_interval;
+ residual += intf->per_interval_residual;
+ div = intf->div;
+ while (!circ_sbuf_empty(tbuf) && to_send) {
+ buf[pos++] = tbuf->buf[tbuf->tail];
+ tbuf->tail = circ_sbuf_next(tbuf->tail);
+ to_send--;
+ }
+ if (residual >= div) {
+ residual -= div;
+ if (!circ_sbuf_empty(tbuf)) {
+ buf[pos++] = tbuf->buf[tbuf->tail];
+ tbuf->tail = circ_sbuf_next(tbuf->tail);
+ }
+ }
+
+ /*
+ * Move from the internal buffer into my receive
+ * buffer.
+ */
+ spin_lock_irq(&port->lock);
+ flag = serialsim_get_flag(intf, &status);
+ if (intf->rx_enabled) {
+ for (to_send = 0; to_send < pos; to_send++) {
+ port->icount.rx++;
+ uart_insert_char(port, status,
+ DO_OVERRUN_ERR,
+ buf[to_send], flag);
+ flag = 0;
+ status = 0;
+ }
+ }
+ spin_unlock_irq(&port->lock);
+
+ if (pos)
+ tty_flip_buffer_push(&port->state->port);
+
+ serialsim_thread_delay();
+ }
+
+ return 0;
+}
+
+static void serialsim_start_tx(struct uart_port *port)
+{
+ struct serialsim_intf *intf = serialsim_port_to_intf(port);
+
+ intf->tx_enabled = true;
+}
+
+static void serialsim_stop_rx(struct uart_port *port)
+{
+ struct serialsim_intf *intf = serialsim_port_to_intf(port);
+
+ intf->rx_enabled = false;
+}
+
+static void serialsim_break_ctl(struct uart_port *port, int break_state)
+{
+ struct serialsim_intf *intf = serialsim_port_to_intf(port);
+ struct serialsim_intf *ointf = intf->ointf;
+
+ spin_lock_irq(&ointf->port.lock);
+ if (!break_state && ointf->flags & DO_BREAK) {
+ /* Turning break off. */
+ ointf->break_reported = false;
+ ointf->flags &= ~DO_BREAK;
+ } else if (break_state) {
+ ointf->flags |= DO_BREAK;
+ }
+ spin_unlock_irq(&ointf->port.lock);
+}
+
+static int serialsim_startup(struct uart_port *port)
+{
+ struct serialsim_intf *intf = serialsim_port_to_intf(port);
+ int rv = 0;
+ struct circ_buf *cbuf = &port->state->xmit;
+ unsigned long flags;
+
+ /*
+ * This is technically wrong, but otherwise tests using it
+ * can get stale data.
+ */
+ spin_lock_irqsave(&port->lock, flags);
+ cbuf->head = cbuf->tail = 0;
+ spin_unlock_irqrestore(&port->lock, flags);
+
+ intf->buf.head = intf->buf.tail = 0;
+ intf->thread = kthread_run(serialsim_thread, intf,
+ "%s%d", intf->threadname, port->line);
+ if (IS_ERR(intf->thread)) {
+ rv = PTR_ERR(intf->thread);
+ intf->thread = NULL;
+ pr_err("serialsim: Could not start thread: %d", rv);
+ } else {
+ spin_lock_irqsave(&port->lock, flags);
+ intf->tx_enabled = true;
+ intf->rx_enabled = true;
+
+ serialsim_set_baud_rate(intf, 9600, CS8);
+ spin_unlock_irqrestore(&port->lock, flags);
+ }
+
+ return rv;
+}
+
+static void serialsim_shutdown(struct uart_port *port)
+{
+ struct serialsim_intf *intf = serialsim_port_to_intf(port);
+ unsigned long flags;
+
+ spin_lock_irqsave(&port->lock, flags);
+ spin_unlock_irqrestore(&port->lock, flags);
+ kthread_stop(intf->thread);
+}
+
+static void serialsim_release_port(struct uart_port *port)
+{
+}
+
+static void
+serialsim_set_termios(struct uart_port *port, struct ktermios *termios,
+ struct ktermios *old)
+{
+ struct serialsim_intf *intf = serialsim_port_to_intf(port);
+ unsigned int baud = uart_get_baud_rate(port, termios, old,
+ 10, 100000000);
+ unsigned long flags;
+
+ spin_lock_irqsave(&port->lock, flags);
+ serialsim_set_baud_rate(intf, baud, termios->c_cflag);
+ intf->termios = *termios;
+ spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static int serialsim_rs485(struct uart_port *port,
+ struct serial_rs485 *newrs485)
+{
+ struct serialsim_intf *intf = serialsim_port_to_intf(port);
+
+ intf->rs485 = *newrs485;
+ return 0;
+}
+
+static const char *serialecho_type(struct uart_port *port)
+{
+ return "SerialEcho";
+}
+
+static const char *serialpipea_type(struct uart_port *port)
+{
+ return "SerialPipeA";
+}
+
+static const char *serialpipeb_type(struct uart_port *port)
+{
+ return "SerialPipeB";
+}
+
+static void serialecho_config_port(struct uart_port *port, int type)
+{
+ port->type = PORT_SERIALECHO;
+}
+
+static void serialpipea_config_port(struct uart_port *port, int type)
+{
+ port->type = PORT_SERIALPIPEA;
+}
+
+static void serialpipeb_config_port(struct uart_port *port, int type)
+{
+ port->type = PORT_SERIALPIPEB;
+}
+
+static int serialpipe_ioctl(struct uart_port *port, unsigned int cmd,
+ unsigned long arg)
+{
+ int rv = 0;
+ struct serialsim_intf *intf = serialsim_port_to_intf(port);
+ unsigned int mask;
+ int val;
+
+ switch (cmd) {
+ case TIOCSERSREMNULLMODEM:
+ serialsim_set_null_modem(intf->ointf, !!arg);
+ break;
+
+ case TIOCSERGREMNULLMODEM:
+ val = intf->ointf->do_null_modem;
+ if (copy_to_user((int __user *) arg, &val, sizeof(int)))
+ rv = -EFAULT;
+ break;
+
+ case TIOCSERSREMMCTRL:
+ mask = (arg >> 16) & 0xffff;
+ arg &= 0xffff;
+ if (mask & ~LOCAL_MCTRL || arg & ~LOCAL_MCTRL)
+ rv = -EINVAL;
+ else
+ serialsim_set_modem_lines(intf->ointf, mask, arg);
+ break;
+
+ case TIOCSERGREMMCTRL:
+ if (copy_to_user((unsigned int __user *) arg,
+ &intf->ointf->mctrl,
+ sizeof(unsigned int)))
+ rv = -EFAULT;
+ break;
+
+ case TIOCSERSREMERR:
+ if (arg & ~FLAGS_MASK)
+ rv = -EINVAL;
+ else
+ serialsim_set_flags(intf, arg);
+ break;
+
+ case TIOCSERGREMERR:
+ if (copy_to_user((unsigned int __user *) arg, &intf->flags,
+ sizeof(unsigned int)))
+ rv = -EFAULT;
+ break;
+
+ case TIOCSERGREMTERMIOS:
+ {
+ struct ktermios otermios;
+
+ spin_lock_irq(&intf->ointf->port.lock);
+ otermios = intf->ointf->termios;
+ spin_unlock_irq(&intf->ointf->port.lock);
+#ifdef TCGETS2
+ rv = kernel_termios_to_user_termios((struct termios2 __user *)
+ arg,
+ &otermios);
+#else
+ rv = kernel_termios_to_user_termios((struct termios __user *)
+ arg,
+ &otermios);
+#endif
+ if (rv)
+ rv = -EFAULT;
+ break;
+ }
+
+ case TIOCSERGREMRS485:
+ {
+ struct serial_rs485 ors485;
+
+ spin_lock_irq(&intf->ointf->port.lock);
+ ors485 = intf->ointf->rs485;
+ spin_unlock_irq(&intf->ointf->port.lock);
+
+ if (copy_to_user((struct serial_rs485 __user *) arg,
+ &ors485, sizeof(ors485)))
+ rv = -EFAULT;
+ break;
+ }
+
+ default:
+ rv = -ENOIOCTLCMD;
+ }
+
+ return rv;
+}
+
+static struct serialsim_intf *serialecho_ports;
+static struct serialsim_intf *serialpipe_ports;
+
+static unsigned int nr_echo_ports = 4;
+module_param(nr_echo_ports, uint, 0444);
+MODULE_PARM_DESC(nr_echo_ports,
+ "The number of echo ports to create. Defaults to 4");
+
+static unsigned int nr_pipe_ports = 4;
+module_param(nr_pipe_ports, uint, 0444);
+MODULE_PARM_DESC(nr_pipe_ports,
+ "The number of pipe ports to create. Defaults to 4");
+
+static char *gettok(char **s)
+{
+ char *t = skip_spaces(*s);
+ char *p = t;
+
+ while (*p && !isspace(*p))
+ p++;
+ if (*p)
+ *p++ = '\0';
+ *s = p;
+
+ return t;
+}
+
+static bool tokeq(const char *t, const char *m)
+{
+ return strcmp(t, m) == 0;
+}
+
+static unsigned int parse_modem_line(char op, unsigned int flag,
+ unsigned int *mctrl)
+{
+ if (op == '+')
+ *mctrl |= flag;
+ else
+ *mctrl &= ~flag;
+ return flag;
+}
+
+static void serialsim_ctrl_append(char **val, int *left, char *n, bool enabled)
+{
+ int count;
+
+ count = snprintf(*val, *left, " %c%s", enabled ? '+' : '-', n);
+ *left -= count;
+ *val += count;
+}
+
+static ssize_t serialsim_ctrl_read(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct tty_port *tport = dev_get_drvdata(dev);
+ struct uart_state *state = container_of(tport, struct uart_state, port);
+ struct uart_port *port = state->uart_port;
+ struct serialsim_intf *intf = serialsim_port_to_intf(port);
+ unsigned int mctrl = intf->mctrl;
+ char *val = buf;
+ int left = PAGE_SIZE;
+ int count;
+
+ count = snprintf(val, left, "%s:", dev->kobj.name);
+ val += count;
+ left -= count;
+ serialsim_ctrl_append(&val, &left, "nullmodem", intf->do_null_modem);
+ serialsim_ctrl_append(&val, &left, "cd", mctrl & TIOCM_CAR);
+ serialsim_ctrl_append(&val, &left, "dsr", mctrl & TIOCM_DSR);
+ serialsim_ctrl_append(&val, &left, "cts", mctrl & TIOCM_CTS);
+ serialsim_ctrl_append(&val, &left, "ring", mctrl & TIOCM_RNG);
+ serialsim_ctrl_append(&val, &left, "dtr", mctrl & TIOCM_DTR);
+ serialsim_ctrl_append(&val, &left, "rts", mctrl & TIOCM_RTS);
+ *val++ = '\n';
+
+ return val - buf;
+}
+
+static ssize_t serialsim_ctrl_write(struct device *dev,
+ struct device_attribute *attr,
+ const char *val, size_t count)
+{
+ struct tty_port *tport = dev_get_drvdata(dev);
+ struct uart_state *state = container_of(tport, struct uart_state, port);
+ struct uart_port *port = state->uart_port;
+ struct serialsim_intf *intf = serialsim_port_to_intf(port);
+ char *str = kstrndup(val, count, GFP_KERNEL);
+ char *p, *s = str;
+ int rv = count;
+ unsigned int flags = 0;
+ unsigned int nullmodem = 0;
+ unsigned int mctrl_mask = 0, mctrl = 0;
+
+ if (!str)
+ return -ENOMEM;
+
+ p = gettok(&s);
+ while (*p) {
+ char op = '\0';
+ int err = 0;
+
+ switch (*p) {
+ case '+':
+ case '-':
+ op = *p++;
+ break;
+ default:
+ break;
+ }
+
+ if (tokeq(p, "frame"))
+ flags |= DO_FRAME_ERR;
+ else if (tokeq(p, "parity"))
+ flags |= DO_PARITY_ERR;
+ else if (tokeq(p, "overrun"))
+ flags |= DO_OVERRUN_ERR;
+ else if (tokeq(p, "nullmodem"))
+ nullmodem = op;
+ else if (tokeq(p, "dsr"))
+ mctrl_mask |= parse_modem_line(op, TIOCM_DSR, &mctrl);
+ else if (tokeq(p, "cts"))
+ mctrl_mask |= parse_modem_line(op, TIOCM_CTS, &mctrl);
+ else if (tokeq(p, "cd"))
+ mctrl_mask |= parse_modem_line(op, TIOCM_CAR, &mctrl);
+ else if (tokeq(p, "ring"))
+ mctrl_mask |= parse_modem_line(op, TIOCM_RNG, &mctrl);
+ else
+ err = -EINVAL;
+
+ if (err) {
+ rv = err;
+ goto out;
+ }
+ p = gettok(&s);
+ }
+
+ if (flags)
+ serialsim_set_flags(intf, flags);
+ if (nullmodem)
+ serialsim_set_null_modem(intf, nullmodem == '+');
+ if (mctrl_mask)
+ serialsim_set_modem_lines(intf, mctrl_mask, mctrl);
+
+out:
+ kfree(str);
+
+ return rv;
+}
+
+static DEVICE_ATTR(ctrl, 0660, serialsim_ctrl_read, serialsim_ctrl_write);
+
+static struct attribute *serialsim_dev_attrs[] = {
+ &dev_attr_ctrl.attr,
+ NULL,
+};
+
+static struct attribute_group serialsim_dev_attr_group = {
+ .attrs = serialsim_dev_attrs,
+};
+
+static const struct uart_ops serialecho_ops = {
+ .tx_empty = serialsim_tx_empty,
+ .set_mctrl = serialsim_set_mctrl,
+ .get_mctrl = serialsim_get_mctrl,
+ .stop_tx = serialsim_stop_tx,
+ .start_tx = serialsim_start_tx,
+ .stop_rx = serialsim_stop_rx,
+ .break_ctl = serialsim_break_ctl,
+ .startup = serialsim_startup,
+ .shutdown = serialsim_shutdown,
+ .release_port = serialsim_release_port,
+ .set_termios = serialsim_set_termios,
+ .type = serialecho_type,
+ .config_port = serialecho_config_port
+};
+
+static const struct uart_ops serialpipea_ops = {
+ .tx_empty = serialsim_tx_empty,
+ .set_mctrl = serialsim_set_mctrl,
+ .get_mctrl = serialsim_get_mctrl,
+ .stop_tx = serialsim_stop_tx,
+ .start_tx = serialsim_start_tx,
+ .stop_rx = serialsim_stop_rx,
+ .break_ctl = serialsim_break_ctl,
+ .startup = serialsim_startup,
+ .shutdown = serialsim_shutdown,
+ .release_port = serialsim_release_port,
+ .set_termios = serialsim_set_termios,
+ .type = serialpipea_type,
+ .config_port = serialpipea_config_port,
+ .ioctl = serialpipe_ioctl
+};
+
+static const struct uart_ops serialpipeb_ops = {
+ .tx_empty = serialsim_tx_empty,
+ .set_mctrl = serialsim_set_mctrl,
+ .get_mctrl = serialsim_get_mctrl,
+ .stop_tx = serialsim_stop_tx,
+ .start_tx = serialsim_start_tx,
+ .stop_rx = serialsim_stop_rx,
+ .break_ctl = serialsim_break_ctl,
+ .startup = serialsim_startup,
+ .shutdown = serialsim_shutdown,
+ .release_port = serialsim_release_port,
+ .set_termios = serialsim_set_termios,
+ .type = serialpipeb_type,
+ .config_port = serialpipeb_config_port,
+ .ioctl = serialpipe_ioctl
+};
+
+static struct uart_driver serialecho_driver = {
+ .owner = THIS_MODULE,
+ .driver_name = "ttyEcho",
+ .dev_name = "ttyEcho"
+};
+
+static struct uart_driver serialpipea_driver = {
+ .owner = THIS_MODULE,
+ .driver_name = "ttyPipeA",
+ .dev_name = "ttyPipeA"
+};
+
+static struct uart_driver serialpipeb_driver = {
+ .owner = THIS_MODULE,
+ .driver_name = "ttyPipeB",
+ .dev_name = "ttyPipeB"
+};
+
+
+static int __init serialsim_init(void)
+{
+ unsigned int i;
+ int rv;
+
+ serialecho_ports = kcalloc(nr_echo_ports,
+ sizeof(*serialecho_ports),
+ GFP_KERNEL);
+ if (!serialecho_ports) {
+ pr_err("serialsim: Unable to allocate echo ports.\n");
+ rv = ENOMEM;
+ goto out;
+ }
+
+ serialpipe_ports = kcalloc(nr_pipe_ports * 2,
+ sizeof(*serialpipe_ports),
+ GFP_KERNEL);
+ if (!serialpipe_ports) {
+ kfree(serialecho_ports);
+ pr_err("serialsim: Unable to allocate pipe ports.\n");
+ rv = ENOMEM;
+ goto out;
+ }
+
+ serialecho_driver.nr = nr_echo_ports;
+ rv = uart_register_driver(&serialecho_driver);
+ if (rv) {
+ kfree(serialecho_ports);
+ kfree(serialpipe_ports);
+ pr_err("serialsim: Unable to register driver.\n");
+ goto out;
+ }
+
+ serialpipea_driver.nr = nr_pipe_ports;
+ rv = uart_register_driver(&serialpipea_driver);
+ if (rv) {
+ uart_unregister_driver(&serialecho_driver);
+ kfree(serialecho_ports);
+ kfree(serialpipe_ports);
+ pr_err("serialsim: Unable to register driver.\n");
+ goto out;
+ }
+
+ serialpipeb_driver.nr = nr_pipe_ports;
+ rv = uart_register_driver(&serialpipeb_driver);
+ if (rv) {
+ uart_unregister_driver(&serialpipea_driver);
+ uart_unregister_driver(&serialecho_driver);
+ kfree(serialecho_ports);
+ kfree(serialpipe_ports);
+ pr_err("serialsim: Unable to register driver.\n");
+ goto out;
+ }
+
+ for (i = 0; i < nr_echo_ports; i++) {
+ struct serialsim_intf *intf = &serialecho_ports[i];
+ struct uart_port *port = &intf->port;
+
+ intf->buf.buf = intf->xmitbuf;
+ intf->ointf = intf;
+ intf->threadname = "serialecho";
+ intf->do_null_modem = true;
+ spin_lock_init(&intf->mctrl_lock);
+ tasklet_init(&intf->mctrl_tasklet, mctrl_tasklet, (long) intf);
+ /* Won't configure without some I/O or mem address set. */
+ port->iobase = 1;
+ port->line = i;
+ port->flags = UPF_BOOT_AUTOCONF;
+ port->ops = &serialecho_ops;
+ spin_lock_init(&port->lock);
+ port->attr_group = &serialsim_dev_attr_group;
+ rv = uart_add_one_port(&serialecho_driver, port);
+ if (rv)
+ pr_err("serialsim: Unable to add uart port %d: %d\n",
+ i, rv);
+ else
+ intf->registered = true;
+ }
+
+ for (i = 0; i < nr_pipe_ports * 2; i += 2) {
+ struct serialsim_intf *intfa = &serialpipe_ports[i];
+ struct serialsim_intf *intfb = &serialpipe_ports[i + 1];
+ struct uart_port *porta = &intfa->port;
+ struct uart_port *portb = &intfb->port;
+
+ intfa->buf.buf = intfa->xmitbuf;
+ intfb->buf.buf = intfb->xmitbuf;
+ intfa->ointf = intfb;
+ intfb->ointf = intfa;
+ intfa->threadname = "serialpipea";
+ intfb->threadname = "serialpipeb";
+ intfa->do_null_modem = true;
+ intfb->do_null_modem = true;
+ spin_lock_init(&intfa->mctrl_lock);
+ spin_lock_init(&intfb->mctrl_lock);
+ tasklet_init(&intfa->mctrl_tasklet, mctrl_tasklet,
+ (long) intfa);
+ tasklet_init(&intfb->mctrl_tasklet, mctrl_tasklet,
+ (long) intfb);
+
+ /* Won't configure without some I/O or mem address set. */
+ porta->iobase = 1;
+ porta->line = i / 2;
+ porta->flags = UPF_BOOT_AUTOCONF;
+ porta->ops = &serialpipea_ops;
+ spin_lock_init(&porta->lock);
+ porta->attr_group = &serialsim_dev_attr_group;
+ porta->rs485_config = serialsim_rs485;
+ rv = uart_add_one_port(&serialpipea_driver, porta);
+ if (rv) {
+ pr_err("serialsim: Unable to add uart pipe aport %d: %d\n",
+ i, rv);
+ continue;
+ } else {
+ intfa->registered = true;
+ }
+
+ portb->iobase = 1;
+ portb->line = i / 2;
+ portb->flags = UPF_BOOT_AUTOCONF;
+ portb->ops = &serialpipeb_ops;
+ portb->attr_group = &serialsim_dev_attr_group;
+ spin_lock_init(&portb->lock);
+ portb->rs485_config = serialsim_rs485;
+ rv = uart_add_one_port(&serialpipeb_driver, portb);
+ if (rv) {
+ pr_err("serialsim: Unable to add uart pipe b port %d: %d\n",
+ i, rv);
+ intfa->registered = false;
+ uart_remove_one_port(&serialpipea_driver, porta);
+ } else {
+ intfb->registered = true;
+ }
+ }
+ rv = 0;
+
+ pr_info("serialsim ready\n");
+out:
+ return rv;
+}
+
+static void __exit serialsim_exit(void)
+{
+ unsigned int i;
+
+ for (i = 0; i < nr_echo_ports; i++) {
+ struct serialsim_intf *intf = &serialecho_ports[i];
+ struct uart_port *port = &intf->port;
+
+ if (intf->registered)
+ uart_remove_one_port(&serialecho_driver, port);
+ tasklet_kill(&intf->mctrl_tasklet);
+ }
+
+ for (i = 0; i < nr_pipe_ports * 2; i += 2) {
+ struct serialsim_intf *intfa = &serialpipe_ports[i];
+ struct serialsim_intf *intfb = &serialpipe_ports[i + 1];
+ struct uart_port *porta = &intfa->port;
+ struct uart_port *portb = &intfb->port;
+
+ if (intfa->registered)
+ uart_remove_one_port(&serialpipea_driver, porta);
+ if (intfb->registered)
+ uart_remove_one_port(&serialpipeb_driver, portb);
+ tasklet_kill(&intfa->mctrl_tasklet);
+ tasklet_kill(&intfb->mctrl_tasklet);
+ }
+ uart_unregister_driver(&serialecho_driver);
+ uart_unregister_driver(&serialpipea_driver);
+ uart_unregister_driver(&serialpipeb_driver);
+
+ kfree(serialecho_ports);
+ kfree(serialpipe_ports);
+
+ pr_info("serialsim unloaded\n");
+}
+
+module_init(serialsim_init);
+module_exit(serialsim_exit);
+
+MODULE_AUTHOR("Corey Minyard");
+MODULE_DESCRIPTION("Serial simulation device");
+MODULE_LICENSE("GPL");
diff --git a/include/uapi/linux/serialsim.h b/include/uapi/linux/serialsim.h
new file mode 100644
index 000000000000..7c62c7d148d1
--- /dev/null
+++ b/include/uapi/linux/serialsim.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+
+/*
+ * serialsim - Emulate a serial device in a loopback and/or pipe
+ */
+
+/*
+ * TTY IOCTLs for controlling the modem control and for error injection.
+ * See drivers/tty/serial/serialsim.c and Documentation/serial/serialsim.rst
+ * for details.
+ */
+
+#ifndef LINUX_SERIALSIM_H
+#define LINUX_SERIALSIM_H
+
+#include <linux/ioctl.h>
+#include <asm/termbits.h>
+
+#define TIOCSERSREMNULLMODEM 0x54e4
+#define TIOCSERSREMMCTRL 0x54e5
+#define TIOCSERSREMERR 0x54e6
+#define TIOCSERGREMTERMIOS _IOR('T', 0xe7, struct termios2)
+#define TIOCSERGREMNULLMODEM _IOR('T', 0xe8, int)
+#define TIOCSERGREMMCTRL _IOR('T', 0xe9, unsigned int)
+#define TIOCSERGREMERR _IOR('T', 0xea, unsigned int)
+#define TIOCSERGREMRS485 _IOR('T', 0xeb, struct serial_rs485)
+
+#endif
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] tty: serial: samsung: Enable baud clock during initialisation
From: Marek Szyprowski @ 2019-02-28 10:07 UTC (permalink / raw)
To: Stuart Menefy, Krzysztof Kozlowski, Greg Kroah-Hartman,
Jiri Slaby, linux-serial, linux-kernel, linux-samsung-soc
In-Reply-To: <20190212214022.9446-1-stuart.menefy@mathembedded.com>
Hi Stuart,
On 2019-02-12 22:40, Stuart Menefy wrote:
> The Exynos 5260, like the 5433, appears to require baud clock as
> well as pclk to be running before accessing any of the registers,
> otherwise an external abort is raised.
>
> The serial driver already enables baud clock when required, but only
> if it knows which clock is baud clock. On older SoCs baud clock may be
> selected from a number of possible clocks so to support this the driver
> only selects which clock to use for baud clock when a port is opened,
> at which point the desired baud rate is known and the best clock can be
> selected.
>
> The result is that there are a number of circumstances in which
> registers are accessed without first explicitly enabling baud clock:
> - while the driver is being initialised
> - the initial parts of opening a port for the first time
> - when resuming if the port hasn't been already opened
>
> The 5433 overcomes this currently by marking the baud clock as
> CLK_IGNORE_UNUSED, so the clock is always enabled, however
> for the 5260 I've been trying to avoid this.
>
> This change adds code to pick the first available clock to use
> as baud clock and enables it while initialising the driver.
>
> This code wouldn't be sufficient on a SoC which supports
> multiple possible baud clock sources _and_ requires the
> correct baud clock to be enabled before accessing any of the
> serial port registers (in particular the register which selects
> which clock to use as the baud clock). As far as I know
> such hardware doesn't exist.
I think that we can use a simpler approach. The driver can simply keep
baud clock enabled together with pclk always when there is only one baud
clock source (s3c24xx_serial_drv_data.num_clks == 1). This will work
both on Exynos 5260 and 5433.
> Signed-off-by: Stuart Menefy <stuart.menefy@mathembedded.com>
> ---
> drivers/tty/serial/samsung.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
> index 9fc3559f80d9..83fd51607741 100644
> --- a/drivers/tty/serial/samsung.c
> +++ b/drivers/tty/serial/samsung.c
> @@ -1694,6 +1694,42 @@ s3c24xx_serial_cpufreq_deregister(struct s3c24xx_uart_port *port)
> }
> #endif
>
> +static int s3c24xx_serial_enable_baudclk(struct s3c24xx_uart_port *ourport)
> +{
> + struct device *dev = ourport->port.dev;
> + struct s3c24xx_uart_info *info = ourport->info;
> + char clk_name[MAX_CLK_NAME_LENGTH];
> + unsigned int clk_sel;
> + struct clk *clk;
> + int clk_num;
> + int ret;
> +
> + clk_sel = ourport->cfg->clk_sel ? : info->def_clk_sel;
> + for (clk_num = 0; clk_num < info->num_clks; clk_num++) {
> + if (!(clk_sel & (1 << clk_num)))
> + continue;
> +
> + sprintf(clk_name, "clk_uart_baud%d", clk_num);
> + clk = clk_get(dev, clk_name);
> + if (IS_ERR(clk))
> + continue;
> +
> + ret = clk_prepare_enable(clk);
> + if (ret) {
> + clk_put(clk);
> + continue;
> + }
> +
> + ourport->baudclk = clk;
> + ourport->baudclk_rate = clk_get_rate(clk);
> + s3c24xx_serial_setsource(&ourport->port, clk_num);
> +
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> /* s3c24xx_serial_init_port
> *
> * initialise a single serial port from the platform device given
> @@ -1788,6 +1824,10 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
> goto err;
> }
>
> + ret = s3c24xx_serial_enable_baudclk(ourport);
> + if (ret)
> + pr_warn("uart: failed to enable baudclk\n");
> +
> /* Keep all interrupts masked and cleared */
> if (s3c24xx_serial_has_interrupt_mask(port)) {
> wr_regl(port, S3C64XX_UINTM, 0xf);
> @@ -1901,6 +1941,8 @@ static int s3c24xx_serial_probe(struct platform_device *pdev)
> * and keeps the clock enabled in this case.
> */
> clk_disable_unprepare(ourport->clk);
> + if (!IS_ERR(ourport->baudclk))
> + clk_disable_unprepare(ourport->baudclk);
>
> ret = s3c24xx_serial_cpufreq_register(ourport);
> if (ret < 0)
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* Re: [PATCH v4 07/10] dt-bindings: serial: Add Milbeaut serial driver description
From: Sugaya, Taichi @ 2019-02-28 2:35 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-serial, devicetree, linux-kernel, linux-arm-kernel, soc,
Rob Herring, Mark Rutland, Takao Orito, Kazuhiro Kasai,
Shinji Kanematsu, Jassi Brar, Masami Hiramatsu
In-Reply-To: <20190227150450.GA7056@kroah.com>
Hi,
On 2019/02/28 0:04, Greg Kroah-Hartman wrote:
> On Wed, Feb 27, 2019 at 01:53:30PM +0900, Sugaya Taichi wrote:
>> Add DT bindings document for Milbeaut serial driver.
>>
>> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> ---
>> .../devicetree/bindings/serial/milbeaut-uart.txt | 21 +++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/serial/milbeaut-uart.txt
>
> I have already queued this up in my tty tree for merging in 5.1-rc1.
>
> thanks,
>
> greg k-h
>
I see. Thank you!
^ permalink raw reply
* Re: [PATCH v4 07/10] dt-bindings: serial: Add Milbeaut serial driver description
From: Greg Kroah-Hartman @ 2019-02-27 15:04 UTC (permalink / raw)
To: Sugaya Taichi
Cc: linux-serial, devicetree, linux-kernel, linux-arm-kernel, soc,
Rob Herring, Mark Rutland, Takao Orito, Kazuhiro Kasai,
Shinji Kanematsu, Jassi Brar, Masami Hiramatsu
In-Reply-To: <1551243210-10826-1-git-send-email-sugaya.taichi@socionext.com>
On Wed, Feb 27, 2019 at 01:53:30PM +0900, Sugaya Taichi wrote:
> Add DT bindings document for Milbeaut serial driver.
>
> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> .../devicetree/bindings/serial/milbeaut-uart.txt | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/serial/milbeaut-uart.txt
I have already queued this up in my tty tree for merging in 5.1-rc1.
thanks,
greg k-h
^ permalink raw reply
* Re: [RFC PATCH v1 20/25] serial: 8250: implement write_atomic
From: Petr Mladek @ 2019-02-27 13:55 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <87zhqhed3u.fsf@linutronix.de>
On Wed 2019-02-27 11:32:05, John Ogness wrote:
> On 2019-02-27, Petr Mladek <pmladek@suse.com> wrote:
> >> Implement a non-sleeping NMI-safe write_atomic console function in
> >> order to support emergency printk messages.
> >
> > It uses console_atomic_lock() added in 18th patch. That one uses
> > prb_lock() added by 2nd patch.
> >
> > Now, prb_lock() allows recursion on the same CPU. But it still needs
> > to wait until it is released on another CPU.
> >
> > [...]
> >
> > OK, it would be safe when prb_lock() is the only lock taken
> > in the NMI handler.
>
> Which is the case. As I wrote to you already [0], NMI contexts are
> _never_ allowed to do things that rely on waiting forever for other
> CPUs.
Who says _never_? I agree that it is not reasonable. But the
history shows that it happens. In principle, there is nothing wrong
in using spinlock in NMI when it is used only in NMI.
> > Not to say, that we would most
> > likely need to add a lock back into nmi_cpu_backtrace()
> > to keep the output sane.
>
> No. That is why CPU-IDs were added to the output. It is quite sane and
> easy to read.
And I already wrote that they are not added by default and they
does not solve kmsg interface.
Also we might need to provide a userspace support in advance.
We could not release kernel that will make logs hard to read
without post processing. At least I do not have the balls
to do so.
> > Peter Zijlstra several times talked about fully lockless
> > consoles. He is using the early console for debugging, see
> > the patchset
> > https://lkml.kernel.org/r/20170928121823.430053219@infradead.org
>
> That is an interesting thread to quote. In that thread Peter actually
> wrote the exact implementation of prb_lock() as the method to
> synchronize access to the serial console.
The synchronization was added just for the thread. I am not sure
if Peter is using it in the real life.
> > I am not sure if it is always possible. I personally see
> > the following way:
> >
> > 1. Make the printk ring buffer fully lockless. Then we reduce
> > the problem only to console locking. And we could
> > have a per-console-driver lock (no the big lock like
> > prb_lock()).
>
> A fully lockless ring buffer is an option. But as you said, it only
> reduces the window, which is why I decided it is not so important (at
> least for now). Creating a per-console-driver lock would probably be a
> good idea anyway as long as we can guarantee the ordering (which
> shouldn't be a problem as long as emergency console ordering remains
> fixed and emergency writers always follow that ordering).
>
> > 2. I am afraid that we need to add some locking between CPUs
> > to avoid mixing characters from directly printed messages.
>
> That is exactly what console_atomic_lock() (actually prb_lock) is!
Sure. But it should not be a common lock for the ring buffer and
all consoles. Also there are still open questions with NMI
and the direct console handling itself.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH v1 15/25] printk: print history for new consoles
From: Petr Mladek @ 2019-02-27 13:12 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <874l8pft0y.fsf@linutronix.de>
On Wed 2019-02-27 11:02:53, John Ogness wrote:
> On 2019-02-27, Petr Mladek <pmladek@suse.com> wrote:
> > I mean that your patch does the reply on a very hidden location.
>
> Right. I understand that and I agree.
>
> > Regarding the per-console kthread. It would make sense if
> > we stop handling all consoles synchronously. For example,
> > when we push messages to fast consoles immediately and
> > offload the work for slow consoles.
>
> My per-console kthread suggestion relating to fast consoles is so that
> some consoles (such as netconsole, which is quite fast) could drop less
> messages than a slow console (such as uart).
OK, it was not clear from the context.
> > Anyway, we first need to make the offload reliable enough.
> > It is not acceptable to always offload all messages.
> > We have been there last few years. We must keep a high
> > chance to see the messages. Any warning might be important
> > when it causes the system to die. Nobody knows what message
> > is such an important.
>
> You seem to be missing the point of the series. It _is_ acceptable to
> offload all messages because they are being offloaded to non-emergency
> consoles. If messages are lost, it sucks (and the appropriate "dropped"
> messages are sent), but it isn't critical. Once we can agree to this
> point, printk becomes so much easier to work with.
>
> Emergency consoles exist for handling important messages. They will not
> drop messages. They are synchronous and immediate.
We might start thinking about this only when the most common consoles
support the emergency mode. This patchset implemented it only
for serial consoles that are often very slow. It is contradicting
the above statement about fast consoles.
Also the emergency messages from different CPUs are synchronized.
This slows down all affected CPUs. They are serialized and blocked
by the speed of the consoles. It was the reason to handle all pending
messages only by the current owner. I am sure that it would cause
regressions.
Not to say that the synchronization is done using an unfair lock.
One CPU can simply get starved by others for non-predictable time.
This is why ticket spinlocks were invented.
You might argue that the amount of emergency messages should
be small but see below.
> >> It is not necessary. It is desired. Why should _any_ task be punished
> >> with console writing? That is what the printk kthread is for.
> >
> > I do not know about any acceptable solution without punishing
> > the tasks. But we might find a better compromise between the
> > punishment and reliability.
>
> I do not want printk to compromise. That compromise is part of the
> problem. Let's partition printk to important and non-important so that
> we can optimize both. _That_ is the heart of this series.
No, this is just another compromise. Let's look at it from another
side.
The important and non-important messages already existed. The split
was done by console_loglevel. The emergency level just adds one
more category (show, show later, ignore). It allows more fine
grained setting but it does not remove the compromise.
People would still need to choose which messages should be seen
reliably and which might get lost. And the problem will be still
the same. The more messages will be printed reliably the more
delayed might get printk() callers. It might prevent softlockups
but only at the cost that all parallel writers are blocked by
waiting for the console.
Also note that printk configuration already is too complicated.
See the four numbers in /proc/sys/kernel/printk. Many people
would have troubles to set them reasonably even with description.
Fifth number would only make it worse.
And it is even more complicated because people are inconsistent
with using the log levels, see
https://lkml.kernel.org/r/20180619115726.3098-1-hdegoede@redhat.com
It is a lost fight. People always need to see messages from
the code that they work on. If we make it harder to see
some levels, people will just start using levels that are
not filtered.
This is why I suggest to split the work on the ring buffer
and consoles. The new ring buffer might be a clear win.
While the console handling is really complicated. But
I still think that we might and should do better even
in the consoles.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH v1 20/25] serial: 8250: implement write_atomic
From: John Ogness @ 2019-02-27 10:32 UTC (permalink / raw)
To: Petr Mladek
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190227094655.ecdwhsc2bf5spkqx@pathway.suse.cz>
On 2019-02-27, Petr Mladek <pmladek@suse.com> wrote:
>> Implement a non-sleeping NMI-safe write_atomic console function in
>> order to support emergency printk messages.
>
> It uses console_atomic_lock() added in 18th patch. That one uses
> prb_lock() added by 2nd patch.
>
> Now, prb_lock() allows recursion on the same CPU. But it still needs
> to wait until it is released on another CPU.
>
> [...]
>
> OK, it would be safe when prb_lock() is the only lock taken
> in the NMI handler.
Which is the case. As I wrote to you already [0], NMI contexts are
_never_ allowed to do things that rely on waiting forever for other
CPUs. I could not find any instances where that is the
case. nmi_cpu_backtrace() used to do this, but it does not anymore.
> But printk() should not make such limitation
> to the rest of the system.
That is something we have to decide. It is the one factor that makes
prb_lock() feel a hell of a lot like BKL.
> Not to say, that we would most
> likely need to add a lock back into nmi_cpu_backtrace()
> to keep the output sane.
No. That is why CPU-IDs were added to the output. It is quite sane and
easy to read.
> Peter Zijlstra several times talked about fully lockless
> consoles. He is using the early console for debugging, see
> the patchset
> https://lkml.kernel.org/r/20170928121823.430053219@infradead.org
That is an interesting thread to quote. In that thread Peter actually
wrote the exact implementation of prb_lock() as the method to
synchronize access to the serial console.
> I am not sure if it is always possible. I personally see
> the following way:
>
> 1. Make the printk ring buffer fully lockless. Then we reduce
> the problem only to console locking. And we could
> have a per-console-driver lock (no the big lock like
> prb_lock()).
A fully lockless ring buffer is an option. But as you said, it only
reduces the window, which is why I decided it is not so important (at
least for now). Creating a per-console-driver lock would probably be a
good idea anyway as long as we can guarantee the ordering (which
shouldn't be a problem as long as emergency console ordering remains
fixed and emergency writers always follow that ordering).
> 2. I am afraid that we need to add some locking between CPUs
> to avoid mixing characters from directly printed messages.
That is exactly what console_atomic_lock() (actually prb_lock) is!
John Ogness
[0] https://lkml.kernel.org/r/87pnrvs707.fsf@linutronix.de
^ permalink raw reply
* Re: [RFC PATCH v1 15/25] printk: print history for new consoles
From: John Ogness @ 2019-02-27 10:02 UTC (permalink / raw)
To: Petr Mladek
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190227090249.fzigc7r237emlg6k@pathway.suse.cz>
On 2019-02-27, Petr Mladek <pmladek@suse.com> wrote:
> I mean that your patch does the reply on a very hidden location.
Right. I understand that and I agree.
> Regarding the per-console kthread. It would make sense if
> we stop handling all consoles synchronously. For example,
> when we push messages to fast consoles immediately and
> offload the work for slow consoles.
My per-console kthread suggestion relating to fast consoles is so that
some consoles (such as netconsole, which is quite fast) could drop less
messages than a slow console (such as uart).
> Anyway, we first need to make the offload reliable enough.
> It is not acceptable to always offload all messages.
> We have been there last few years. We must keep a high
> chance to see the messages. Any warning might be important
> when it causes the system to die. Nobody knows what message
> is such an important.
You seem to be missing the point of the series. It _is_ acceptable to
offload all messages because they are being offloaded to non-emergency
consoles. If messages are lost, it sucks (and the appropriate "dropped"
messages are sent), but it isn't critical. Once we can agree to this
point, printk becomes so much easier to work with.
Emergency consoles exist for handling important messages. They will not
drop messages. They are synchronous and immediate.
>> It is not necessary. It is desired. Why should _any_ task be punished
>> with console writing? That is what the printk kthread is for.
>
> I do not know about any acceptable solution without punishing
> the tasks. But we might find a better compromise between the
> punishment and reliability.
I do not want printk to compromise. That compromise is part of the
problem. Let's partition printk to important and non-important so that
we can optimize both. _That_ is the heart of this series.
John Ogness
^ permalink raw reply
* Re: [RFC PATCH v1 20/25] serial: 8250: implement write_atomic
From: Petr Mladek @ 2019-02-27 9:46 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190212143003.48446-21-john.ogness@linutronix.de>
On Tue 2019-02-12 15:29:58, John Ogness wrote:
> Implement a non-sleeping NMI-safe write_atomic console function in
> order to support emergency printk messages.
It uses console_atomic_lock() added in 18th patch. That one uses
prb_lock() added by 2nd patch.
Now, prb_lock() allows recursion on the same CPU. But it still needs
to wait until it is released on another CPU.
It means that it is not completely save when NMIs happen on more CPUs in
parallel, for example, when calling nmi_trigger_cpumask_backtrace().
OK, it would be safe when prb_lock() is the only lock taken
in the NMI handler. But printk() should not make such limitation
to the rest of the system. Not to say, that we would most
likely need to add a lock back into nmi_cpu_backtrace()
to keep the output sane.
Peter Zijlstra several times talked about fully lockless
consoles. He is using the early console for debugging, see
the patchset
https://lkml.kernel.org/r/20170928121823.430053219@infradead.org
I am not sure if it is always possible. I personally see
the following way:
1. Make the printk ring buffer fully lockless. Then we reduce
the problem only to console locking. And we could
have a per-console-driver lock (no the big lock like
prb_lock()).
2. I am afraid that we need to add some locking between CPUs
to avoid mixing characters from directly printed messages.
This would be safe everywhere expect in NMI. Then we could
either risk ignoring the lock in NMI (there should be few
messages anyway, the backtraces would get synchronized
another way). Or we might need a compromise between
handling console using the current owner and offload.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH v1 15/25] printk: print history for new consoles
From: Petr Mladek @ 2019-02-27 9:02 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <87o96yziau.fsf@linutronix.de>
On Tue 2019-02-26 16:22:01, John Ogness wrote:
> On 2019-02-26, Petr Mladek <pmladek@suse.com> wrote:
> >> When new consoles register, they currently print how many messages
> >> they have missed. However, many (or all) of those messages may still
> >> be in the ring buffer. Add functionality to print as much of the
> >> history as available. This is a clean replacement of the old
> >> exclusive console hack.
> >>
> >> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> >> index 897219f34cab..6c875abd7b17 100644
> >> --- a/kernel/printk/printk.c
> >> +++ b/kernel/printk/printk.c
> >> @@ -1524,6 +1595,10 @@ static void call_console_drivers(u64 seq, const char *ext_text, size_t ext_len,
> >> for_each_console(con) {
> >> if (!(con->flags & CON_ENABLED))
> >> continue;
> >> + if (!con->wrote_history) {
> >> + printk_write_history(con, seq);
> >
> > This looks like an alien. The code is supposed to write one message
> > from the given buffer. And some huge job is well hidden there.
>
> This is a very simple implementation of a printk kthread. It probably
> makes more sense to have a printk kthread per console. That would allow
> fast consoles to not be penalized by slow consoles. Due to the
> per-console seq tracking, the code would already support it.
I mean that your patch does the reply on a very hidden location.
I think that a cleaned design would be to implement something like:
void console_check_and_reply(void)
{
struct console *con;
if (!console_drivers)
return;
for_each_console(con) {
if (con->flags & CON_PRINTBUFFER) {
printk_write_history(con, console_seq);
con->flags &= ~CON_PRINTBUFFER;
}
}
}
Then there is no recursion. Also you are much more flexible.
You could call this on any reasonable place. For example, you
could call this in the printk_kthread right after taking
console_lock and before processing new messages.
Regarding the per-console kthread. It would make sense if
we stop handling all consoles synchronously. For example,
when we push messages to fast consoles immediately and
offload the work for slow consoles.
Anyway, we first need to make the offload reliable enough.
It is not acceptable to always offload all messages.
We have been there last few years. We must keep a high
chance to see the messages. Any warning might be important
when it causes the system to die. Nobody knows what message
is such an important.
> > In addition, the code is actually recursive. It will become
> > clear when it is deduplicated as suggested above. We should
> > avoid it when it is not necessary. Note that recursive code
> > is always more prone to mistakes and it is harder to think of.
>
> Agreed.
>
> > I guess that the motivation is to do everything from the printk
> > kthread. Is it really necessary? register_console() takes
> > console_lock(). It has to be sleepable context by definition.
>
> It is not necessary. It is desired. Why should _any_ task be punished
> with console writing? That is what the printk kthread is for.
I do not know about any acceptable solution without punishing
the tasks. But we might find a better compromise between the
punishment and reliability.
Best Regards,
Petr
^ permalink raw reply
* [PATCH v4 07/10] dt-bindings: serial: Add Milbeaut serial driver description
From: Sugaya Taichi @ 2019-02-27 4:53 UTC (permalink / raw)
To: linux-serial, devicetree, linux-kernel, linux-arm-kernel, soc
Cc: Greg Kroah-Hartman, Rob Herring, Mark Rutland, Takao Orito,
Kazuhiro Kasai, Shinji Kanematsu, Jassi Brar, Masami Hiramatsu,
Sugaya Taichi
Add DT bindings document for Milbeaut serial driver.
Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/serial/milbeaut-uart.txt | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
create mode 100644 Documentation/devicetree/bindings/serial/milbeaut-uart.txt
diff --git a/Documentation/devicetree/bindings/serial/milbeaut-uart.txt b/Documentation/devicetree/bindings/serial/milbeaut-uart.txt
new file mode 100644
index 0000000..3d2fb1a
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/milbeaut-uart.txt
@@ -0,0 +1,21 @@
+Socionext Milbeaut UART controller
+
+Required properties:
+- compatible: should be "socionext,milbeaut-usio-uart".
+- reg: offset and length of the register set for the device.
+- interrupts: two interrupts specifier.
+- interrupt-names: should be "rx", "tx".
+- clocks: phandle to the input clock.
+
+Optional properties:
+- auto-flow-control: flow control enable.
+
+Example:
+ usio1: usio_uart@1e700010 {
+ compatible = "socionext,milbeaut-usio-uart";
+ reg = <0x1e700010 0x10>;
+ interrupts = <0 141 0x4>, <0 149 0x4>;
+ interrupt-names = "rx", "tx";
+ clocks = <&clk 2>;
+ auto-flow-control;
+ };
--
1.9.1
^ permalink raw reply related
* [PATCH v4 00/10] Add basic support for Socionext Milbeaut M10V SoC
From: Sugaya Taichi @ 2019-02-27 4:50 UTC (permalink / raw)
To: devicetree, linux-serial, linux-arm-kernel, linux-kernel, soc
Cc: Mark Rutland, Shinji Kanematsu, Masami Hiramatsu, Arnd Bergmann,
Jassi Brar, Sugaya Taichi, Greg Kroah-Hartman, Daniel Lezcano,
Russell King, Masahiro Yamada, Rob Herring, Takao Orito,
Thomas Gleixner, Kazuhiro Kasai
Hi,
Here is the series of patches the initial support for SC2000(M10V) of
Milbeaut SoCs. "M10V" is the internal name of SC2000, so commonly used in
source code.
SC2000 is a SoC of the Milbeaut series. equipped with a DSP optimized for
computer vision. It also features advanced functionalities such as 360-degree,
real-time spherical stitching with multi cameras, image stabilization for
without mechanical gimbals, and rolling shutter correction. More detail is
below:
https://www.socionext.com/en/products/assp/milbeaut/SC2000.html
Specifications for developers are below:
- Quad-core 32bit Cortex-A7 on ARMv7-A architecture
- NEON support
- DSP
- GPU
- MAX 3GB DDR3
- Cortex-M0 for power control
- NAND Flash Interface
- SD UHS-I
- SD UHS-II
- SDIO
- USB2.0 HOST / Device
- USB3.0 HOST / Device
- PCI express Gen2
- Ethernet Engine
- I2C
- UART
- SPI
- PWM
Support is quite minimal for now, since it only includes timer, clock,
pictrl and serial controller drivers, so we can only boot to userspace
through initramfs. Support for the other peripherals will come eventually.
Changes since v3:
* Add "Reviewed" tag to the dt-binding of the serial driver.
* Fix schema string in milbeaut soc binding.
* Refine defconfig for milbeaut M10V.
* Add milbeaut architecture config to multi_v7_defconfig.
Changes since v2:
* Drop clk, pinctrl, and serial driver.
* Drop unneeded options from defconfig.
* Convert milbeaut soc binding to yaml.
* Fix serial driver binding.
* Change serial id of aliases in DT.
* Add platform checking when entering suspend/resume.
* Drop pr_err()s.
Changes since v1:
* Change file names.
* Change #define names.
* Refine cpu-enable-method and bindigs.
* Add documentation for Milbeaut SoCs.
* Add more infomation for timer driver.
* Add sched_clock to timer driver.
* Refine whole of clk driver.
* Add earlycon instead of earlyprintk.
* Refine Device Tree.
Sugaya Taichi (10):
dt-bindings: sram: milbeaut: Add binding for Milbeaut smp-sram
dt-bindings: arm: Add SMP enable-method for Milbeaut
dt-bindings: Add documentation for Milbeaut SoCs
ARM: milbeaut: Add basic support for Milbeaut m10v SoC
dt-bindings: timer: Add Milbeaut M10V timer description
clocksource/drivers/timer-milbeaut: Introduce timer for Milbeaut SoCs
dt-bindings: serial: Add Milbeaut serial driver description
ARM: dts: milbeaut: Add device tree set for the Milbeaut M10V board
ARM: configs: Add Milbeaut M10V defconfig
ARM: multi_v7_defconfig: add ARCH_MILBEAUT and ARCH_MILBEAUT_M10V
Documentation/devicetree/bindings/arm/cpus.yaml | 1 +
.../bindings/arm/socionext/milbeaut.yaml | 22 +++
.../devicetree/bindings/serial/milbeaut-uart.txt | 21 +++
.../devicetree/bindings/sram/milbeaut-smp-sram.txt | 24 +++
.../bindings/timer/socionext,milbeaut-timer.txt | 17 +++
arch/arm/Kconfig | 2 +
arch/arm/Makefile | 1 +
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/milbeaut-m10v-evb.dts | 32 ++++
arch/arm/boot/dts/milbeaut-m10v.dtsi | 95 ++++++++++++
arch/arm/configs/milbeaut_m10v_defconfig | 119 +++++++++++++++
arch/arm/configs/multi_v7_defconfig | 2 +
arch/arm/mach-milbeaut/Kconfig | 20 +++
arch/arm/mach-milbeaut/Makefile | 1 +
arch/arm/mach-milbeaut/platsmp.c | 143 ++++++++++++++++++
drivers/clocksource/Kconfig | 9 ++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-milbeaut.c | 161 +++++++++++++++++++++
18 files changed, 672 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/socionext/milbeaut.yaml
create mode 100644 Documentation/devicetree/bindings/serial/milbeaut-uart.txt
create mode 100644 Documentation/devicetree/bindings/sram/milbeaut-smp-sram.txt
create mode 100644 Documentation/devicetree/bindings/timer/socionext,milbeaut-timer.txt
create mode 100644 arch/arm/boot/dts/milbeaut-m10v-evb.dts
create mode 100644 arch/arm/boot/dts/milbeaut-m10v.dtsi
create mode 100644 arch/arm/configs/milbeaut_m10v_defconfig
create mode 100644 arch/arm/mach-milbeaut/Kconfig
create mode 100644 arch/arm/mach-milbeaut/Makefile
create mode 100644 arch/arm/mach-milbeaut/platsmp.c
create mode 100644 drivers/clocksource/timer-milbeaut.c
--
1.9.1
^ permalink raw reply
* Re: [RFC PATCH v1 16/25] printk: implement CON_PRINTBUFFER
From: Petr Mladek @ 2019-02-26 15:38 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190212143003.48446-17-john.ogness@linutronix.de>
On Tue 2019-02-12 15:29:54, John Ogness wrote:
> If the CON_PRINTBUFFER flag is not set, do not replay the history
> for that console.
This patch fixes a regression caused by previous patches.
We need to do it a way that does not cause the regression
and do not break bisection.
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 6c875abd7b17..b97d4195b09a 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1596,8 +1592,12 @@ static void call_console_drivers(u64 seq, const char *ext_text, size_t ext_len,
> if (!(con->flags & CON_ENABLED))
> continue;
> if (!con->wrote_history) {
> - printk_write_history(con, seq);
> - continue;
> + if (con->flags & CON_PRINTBUFFER) {
> + printk_write_history(con, seq);
> + continue;
> + }
> + con->wrote_history = 1;
I have just got an idea that we do not need a new flag.
We could clear CON_PRINTBUFFER bit instead.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH v1 15/25] printk: print history for new consoles
From: John Ogness @ 2019-02-26 15:22 UTC (permalink / raw)
To: Petr Mladek
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190226145837.wl54fr7rn2ii5oxc@pathway.suse.cz>
On 2019-02-26, Petr Mladek <pmladek@suse.com> wrote:
>> When new consoles register, they currently print how many messages
>> they have missed. However, many (or all) of those messages may still
>> be in the ring buffer. Add functionality to print as much of the
>> history as available. This is a clean replacement of the old
>> exclusive console hack.
>>
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index 897219f34cab..6c875abd7b17 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -1506,6 +1506,77 @@ static void format_text(struct printk_log *msg, u64 seq,
>> }
>> }
>>
>> +static void printk_write_history(struct console *con, u64 master_seq)
>> +{
>> + struct prb_iterator iter;
>> + bool time = printk_time;
>> + static char *ext_text;
>> + static char *text;
>> + static char *buf;
>> + u64 seq;
>> +
>> + ext_text = kmalloc(CONSOLE_EXT_LOG_MAX, GFP_KERNEL);
>> + text = kmalloc(PRINTK_SPRINT_MAX, GFP_KERNEL);
>> + buf = kmalloc(PRINTK_RECORD_MAX, GFP_KERNEL);
>> + if (!ext_text || !text || !buf)
>> + return;
>
> We need to free buffers that were successfully allocated.
Ouch. You just found some crazy garbage. The char-pointers are
static. The bug is that it allocates each time a console is
registered. It was supposed to be lazy allocation:
if (!ext_text)
ext_text = kmalloc(CONSOLE_EXT_LOG_MAX, GFP_KERNEL);
>> + if (!(con->flags & CON_ENABLED))
>> + goto out;
>> +
>> + if (!con->write)
>> + goto out;
>> +
>> + if (!cpu_online(raw_smp_processor_id()) &&
>> + !(con->flags & CON_ANYTIME))
>> + goto out;
>> +
>> + prb_iter_init(&iter, &printk_rb, NULL);
>> +
>> + for (;;) {
>> + struct printk_log *msg;
>> + size_t ext_len;
>> + size_t len;
>> + int ret;
>> +
>> + ret = prb_iter_next(&iter, buf, PRINTK_RECORD_MAX, &seq);
>> + if (ret == 0) {
>> + break;
>> + } else if (ret < 0) {
>> + prb_iter_init(&iter, &printk_rb, NULL);
>> + continue;
>> + }
>> +
>> + if (seq > master_seq)
>> + break;
>> +
>> + con->printk_seq++;
>> + if (con->printk_seq < seq) {
>> + print_console_dropped(con, seq - con->printk_seq);
>> + con->printk_seq = seq;
>> + }
>> +
>> + msg = (struct printk_log *)buf;
>> + format_text(msg, master_seq, ext_text, &ext_len, text,
>> + &len, time);
>> +
>> + if (len == 0 && ext_len == 0)
>> + continue;
>> +
>> + if (con->flags & CON_EXTENDED)
>> + con->write(con, ext_text, ext_len);
>> + else
>> + con->write(con, text, len);
>> +
>> + printk_delay(msg->level);
>
> Hmm, this duplicates a lot of code from call_console_drivers() and
> maybe also from printk_kthread_func(). It is error prone. People
> will forget to update this function when working on the main one.
>
> We need to put the shared parts into separate functions.
Agreed.
>> + }
>> +out:
>> + con->wrote_history = 1;
>> + kfree(ext_text);
>> + kfree(text);
>> + kfree(buf);
>> +}
>> +
>> /*
>> * Call the console drivers, asking them to write out
>> * log_buf[start] to log_buf[end - 1].
>> @@ -1524,6 +1595,10 @@ static void call_console_drivers(u64 seq, const char *ext_text, size_t ext_len,
>> for_each_console(con) {
>> if (!(con->flags & CON_ENABLED))
>> continue;
>> + if (!con->wrote_history) {
>> + printk_write_history(con, seq);
>
> This looks like an alien. The code is supposed to write one message
> from the given buffer. And some huge job is well hidden there.
This is a very simple implementation of a printk kthread. It probably
makes more sense to have a printk kthread per console. That would allow
fast consoles to not be penalized by slow consoles. Due to the
per-console seq tracking, the code would already support it.
> In addition, the code is actually recursive. It will become
> clear when it is deduplicated as suggested above. We should
> avoid it when it is not necessary. Note that recursive code
> is always more prone to mistakes and it is harder to think of.
Agreed.
> I guess that the motivation is to do everything from the printk
> kthread. Is it really necessary? register_console() takes
> console_lock(). It has to be sleepable context by definition.
It is not necessary. It is desired. Why should _any_ task be punished
with console writing? That is what the printk kthread is for.
John Ogness
^ permalink raw reply
* Re: [RFC PATCH v1 15/25] printk: print history for new consoles
From: Petr Mladek @ 2019-02-26 14:58 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190212143003.48446-16-john.ogness@linutronix.de>
On Tue 2019-02-12 15:29:53, John Ogness wrote:
> When new consoles register, they currently print how many messages
> they have missed. However, many (or all) of those messages may still
> be in the ring buffer. Add functionality to print as much of the
> history as available. This is a clean replacement of the old
> exclusive console hack.
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 897219f34cab..6c875abd7b17 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1506,6 +1506,77 @@ static void format_text(struct printk_log *msg, u64 seq,
> }
> }
>
> +static void printk_write_history(struct console *con, u64 master_seq)
> +{
> + struct prb_iterator iter;
> + bool time = printk_time;
> + static char *ext_text;
> + static char *text;
> + static char *buf;
> + u64 seq;
> +
> + ext_text = kmalloc(CONSOLE_EXT_LOG_MAX, GFP_KERNEL);
> + text = kmalloc(PRINTK_SPRINT_MAX, GFP_KERNEL);
> + buf = kmalloc(PRINTK_RECORD_MAX, GFP_KERNEL);
> + if (!ext_text || !text || !buf)
> + return;
We need to free buffers that were successfully allocated.
> + if (!(con->flags & CON_ENABLED))
> + goto out;
> +
> + if (!con->write)
> + goto out;
> +
> + if (!cpu_online(raw_smp_processor_id()) &&
> + !(con->flags & CON_ANYTIME))
> + goto out;
> +
> + prb_iter_init(&iter, &printk_rb, NULL);
> +
> + for (;;) {
> + struct printk_log *msg;
> + size_t ext_len;
> + size_t len;
> + int ret;
> +
> + ret = prb_iter_next(&iter, buf, PRINTK_RECORD_MAX, &seq);
> + if (ret == 0) {
> + break;
> + } else if (ret < 0) {
> + prb_iter_init(&iter, &printk_rb, NULL);
> + continue;
> + }
> +
> + if (seq > master_seq)
> + break;
> +
> + con->printk_seq++;
> + if (con->printk_seq < seq) {
> + print_console_dropped(con, seq - con->printk_seq);
> + con->printk_seq = seq;
> + }
> +
> + msg = (struct printk_log *)buf;
> + format_text(msg, master_seq, ext_text, &ext_len, text,
> + &len, time);
> +
> + if (len == 0 && ext_len == 0)
> + continue;
> +
> + if (con->flags & CON_EXTENDED)
> + con->write(con, ext_text, ext_len);
> + else
> + con->write(con, text, len);
> +
> + printk_delay(msg->level);
Hmm, this duplicates a lot of code from call_console_drivers() and
maybe also from printk_kthread_func(). It is error prone. People
will forget to update this function when working on the main one.
We need to put the shared parts into separate functions.
For example, the per-console code might be moved to
call_console_write(struct console *con, ...). Then
call_console_drivers() might look like:
static void call_console_drivers(u64 seq, const char *ext_text, size_t ext_len,
const char *text, size_t len, int level)
{
struct console *con;
trace_console_rcuidle(text, len);
if (!console_drivers)
return;
for_each_console(con) {
call_console_write(con, seq, ext_text, ext_len,
text, len, level);
}
}
And you could call call_console_write() for the particular console
from printk_write_history().
> + }
> +out:
> + con->wrote_history = 1;
> + kfree(ext_text);
> + kfree(text);
> + kfree(buf);
> +}
> +
> /*
> * Call the console drivers, asking them to write out
> * log_buf[start] to log_buf[end - 1].
> @@ -1524,6 +1595,10 @@ static void call_console_drivers(u64 seq, const char *ext_text, size_t ext_len,
> for_each_console(con) {
> if (!(con->flags & CON_ENABLED))
> continue;
> + if (!con->wrote_history) {
> + printk_write_history(con, seq);
This looks like an alien. The code is supposed to write one message
from the given buffer. And some huge job is well hidden there.
In addition, the code is actually recursive. It will become
clear when it is deduplicated as suggested above. We should
avoid it when it is not necessary. Note that recursive code
is always more prone to mistakes and it is harder to think of.
I guess that the motivation is to do everything from the printk
kthread. Is it really necessary? register_console() takes
console_lock(). It has to be sleepable context by definition.
Anyway, the new console is added under console_lock().
Any new console_lock owner could always check which new
consoles need to replay the log before it starts processing
new messages.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH v1 13/25] printk: track seq per console
From: Petr Mladek @ 2019-02-26 13:11 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <87lg230whd.fsf@linutronix.de>
On Tue 2019-02-26 09:45:02, John Ogness wrote:
> On 2019-02-25, Petr Mladek <pmladek@suse.com> wrote:
> >> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> >> index ece54c24ea0d..ebd9aac06323 100644
> >> --- a/kernel/printk/printk.c
> >> +++ b/kernel/printk/printk.c
> >> @@ -1504,6 +1514,19 @@ static void call_console_drivers(const char *ext_text, size_t ext_len,
> >> if (!cpu_online(raw_smp_processor_id()) &&
> >> !(con->flags & CON_ANYTIME))
> >> continue;
> >> + if (con->printk_seq >= seq)
> >> + continue;
> >> +
> >> + con->printk_seq++;
> >> + if (con->printk_seq < seq) {
> >> + print_console_dropped(con, seq - con->printk_seq);
> >> + con->printk_seq = seq;
> >
> > It would be great to print this message only when the real one
> > is not superseded.
>
> You mean if there was some function to check if "seq" is the newest
> entry. And only in that situation would any dropped information be
> presented?
Not newest but not suppressed.
Example: Only every 10th message is important enough to reach
console (see suppress_message_printing).
Instead of seeing:
message 10
message 20
7 messages dropped
20 another messages dropped because of printing the warning
20 another messages dropped because of printing the warning
20 another messages dropped because of printing the warning
message 100
see something like:
message 10
message 20
13 messages dropped
message 40
13 messages dropped
message 60
13 messages dropped
message 80
message 90
message100
The original code would print only warnings because the important
lines are lost when printing the warnings.
A better code would show more messages because the warning
is printed when a visible message is already buffered.
You might wonder why there are only 13 messages dropped in
the new state. It is because the other 6 messages are
quickly read and suspended (filtered out). They are
skipped by intention.
I hope that it will be more clear with the patch that I have
just sent, see
https://lkml.kernel.org/r/20190226124945.7078-1-pmladek@suse.com
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH v1 10/25] printk: redirect emit/store to new ringbuffer
From: Petr Mladek @ 2019-02-26 9:45 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <875zt7bz1t.fsf@linutronix.de>
On Mon 2019-02-25 17:41:50, John Ogness wrote:
> On 2019-02-25, Petr Mladek <pmladek@suse.com> wrote:
> >> >> vprintk_emit and vprintk_store are the main functions that all printk
> >> >> variants eventually go through. Change these to store the message in
> >> >> the new printk ring buffer that the printk kthread is reading.
> >> >
> >> > Please, are there any candidates or plans to reuse the new ring
> >> > buffer implementation?
> >>
> >> As you pointed out below, this patch already uses the ring buffer
> >> implementation for a totally different purpose: NMI safe dynamic memory
> >> allocation.
> >
> > I have found an alternative solution. We could calculate the length
> > of the formatted string without any buffer:
> >
> > va_list args_copy;
> >
> > va_copy(args_copy, args);
> > len = vscprintf(NULL, fmt, args_copy);
> > va_end(args_copy);
> >
> > This vsprintf() mode was implemented for exactly this purpose.
>
> For vprintk_emit() that would work. As you will see in later (patch 23),
> the sprint_rb ringbuffer is used for dynamic memory allocation for
> kmsg_dump functions as well.
It looks dangerous to share a limited buffer between core kernel
functionality and user-space triggered one. I mean that an unlimited
number of devkmsg operations must not cause loosing printk() messages.
> The current printk implementation allows readers to read directly from
> the ringbuffer. The proposed ringbuffer requires the reader (printk) to
> have its own buffers.
>
> We may be able to find an alternate solution here as well if that is
> desired.
I hope that we will be able to find one. The previous implementation
needed some buffers as well. We should be able to use the same
approach.
I guess that one problem is that the new ringbuffer API is not
able to copy just the text directly into the user-provided buffer.
It might get solved by extending the API.
Anyway, I still have to look at the remaining patches.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH v1 13/25] printk: track seq per console
From: John Ogness @ 2019-02-26 8:45 UTC (permalink / raw)
To: Petr Mladek
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190225145954.62zl3tndmo2t372h@pathway.suse.cz>
On 2019-02-25, Petr Mladek <pmladek@suse.com> wrote:
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index ece54c24ea0d..ebd9aac06323 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -1504,6 +1514,19 @@ static void call_console_drivers(const char *ext_text, size_t ext_len,
>> if (!cpu_online(raw_smp_processor_id()) &&
>> !(con->flags & CON_ANYTIME))
>> continue;
>> + if (con->printk_seq >= seq)
>> + continue;
>> +
>> + con->printk_seq++;
>> + if (con->printk_seq < seq) {
>> + print_console_dropped(con, seq - con->printk_seq);
>> + con->printk_seq = seq;
>
> It would be great to print this message only when the real one
> is not superseded.
You mean if there was some function to check if "seq" is the newest
entry. And only in that situation would any dropped information be
presented?
> The suppressed messages can be proceed quickly. They allow consoles
> to catch up with the message producers. But if the spend time
> with printing the warning, we just risk loosing more messages
> again and again.
So instead of:
message A
message B
3 messages dropped
message F
message G
2 messages dropped
message J
message K
you would prefer to see:
message A
message B
message F
message G
message J
message K
5 messages dropped
... with the hope that maybe we are able to print messages H and/or I by
not spending time on the first dropped message?
If there are a lof printk's coming (sysrq-z) then probably there will be
many dropped during output. With your proposal, it wouldn't be seen that
so many intermediate messages are dropped. Only at the end of the output
the user is presented with some huge dropped number.
With my implementation if you see 2 printk lines together you can be
sure that nothing was dropped between those two lines. I think that is
more valuable than having the possibility of maybe losing a few less
messages in a flood situation.
John Ogness
^ permalink raw reply
* Re: [PATCH 1/6] dt-bindings: soc: qcom: Add interconnect binding for GENI QUP
From: Georgi Djakov @ 2019-02-25 17:39 UTC (permalink / raw)
To: Rob Herring
Cc: Bjorn Andersson, Alok Chauhan, linux-arm-msm, devicetree,
linux-kernel, linux-i2c, linux-spi, linux-serial, Andy Gross,
David Brown, Mark Rutland, dianders, swboyd
In-Reply-To: <20190223002655.GA6631@bogus>
On 2/23/19 02:26, Rob Herring wrote:
> On Wed, Jan 23, 2019 at 08:41:20PM +0200, Georgi Djakov wrote:
>> Hi,
>>
>> On 1/23/19 19:07, Bjorn Andersson wrote:
>>> On Mon 21 Jan 22:33 PST 2019, Alok Chauhan wrote:
>>>
>>>> Add documentation for the interconnect and interconnect-names bindings
>>>> for the GENI QUP as detailed by bindings/interconnect/interconnect.txt.
>>>>
>>>> Signed-off-by: Alok Chauhan <alokc@codeaurora.org>
>>>> ---
>>>> Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++++++++++
>>>> 1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>>>> index dab7ca9..44d7e02 100644
>>>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>>>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>>>> @@ -17,6 +17,12 @@ Required properties if child node exists:
>>>> - #address-cells: Must be <1> for Serial Engine Address
>>>> - #size-cells: Must be <1> for Serial Engine Address Size
>>>> - ranges: Must be present
>>>> +- interconnects: phandle to a interconnect provider. Please refer
>>>> + ../interconnect/interconnect.txt for details.
>>>> + Must be 2 paths corresponding to 2 AXI ports.
>>>> +- interconnect-names: Port names to differentiate between the
>>>
>>> s/Port names/Path names/
>>>
>>>> + 2 interconnect paths defined with interconnect
>>>> + specifier.
>>>
>>> These two names are significant in that they must match what the driver
>>> expects, hence you must actually specify them here.
>>>
>>> And as the scope of these strings are local to the QUP node you can omit
>>> "qup" from them, so make them "memory" and "config" (or perhaps iface,
>>> to match the clock naming?).
>>
>> Actually there was a discussion in the past where we decided include
>> both the src and dst endpoint names in this property so that there is
>> some symmetry with the "interconnects" property. It would be nice to be
>> consistent across different drivers at least for now.
>> If we want to denote the master and slave ports here, my two cents would
>> be for "qup-mem" and "cpu-qup" or something similar?
>
> Well, there's a proposal from Maxime to add 'dma-memory' or something.
> You all need to sort this out.
Agree. I haven't commented on the latest MBUS patches yet. Meanwhile i
am trying to get a better understanding of the hardware. In general if
there are no better suggestions, i am fine with adding a specific name
to handle this kind of dma transfers.
> I assume config or cpu-qup is for register access? Why is this needed?
> That should get described thru the DT tree. The interconnect stuff was
> supposed to be for the non-cpu centric view (i.e. DMA masters). Maybe
> it's fine, but that's not my initial reaction.
Yes, cpu-qup should be for register access. Alok, please correct me if i
am wrong, but I believe this is a separate bus used only for
configuration access to some HW blocks. This bus might be disabled by
default and we may need to request some bandwidth in order to enable it.
It's described as a separate path in DT and we are trying to give it a
meaningful name.
Thanks,
Georgi
^ permalink raw reply
* Re: [RFC PATCH v1 10/25] printk: redirect emit/store to new ringbuffer
From: John Ogness @ 2019-02-25 16:41 UTC (permalink / raw)
To: Petr Mladek
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190225121106.oig7s6odqikcn4xw@pathway.suse.cz>
On 2019-02-25, Petr Mladek <pmladek@suse.com> wrote:
>> >> vprintk_emit and vprintk_store are the main functions that all printk
>> >> variants eventually go through. Change these to store the message in
>> >> the new printk ring buffer that the printk kthread is reading.
>> >
>> > Please, are there any candidates or plans to reuse the new ring
>> > buffer implementation?
>>
>> As you pointed out below, this patch already uses the ring buffer
>> implementation for a totally different purpose: NMI safe dynamic memory
>> allocation.
>
> I have found an alternative solution. We could calculate the length
> of the formatted string without any buffer:
>
> va_list args_copy;
>
> va_copy(args_copy, args);
> len = vscprintf(NULL, fmt, args_copy);
> va_end(args_copy);
>
> This vsprintf() mode was implemented for exactly this purpose.
For vprintk_emit() that would work. As you will see in later (patch 23),
the sprint_rb ringbuffer is used for dynamic memory allocation for
kmsg_dump functions as well.
The current printk implementation allows readers to read directly from
the ringbuffer. The proposed ringbuffer requires the reader (printk) to
have its own buffers.
We may be able to find an alternate solution here as well if that is
desired.
John Ogness
^ permalink raw reply
* Re: [RFC PATCH v1 13/25] printk: track seq per console
From: Petr Mladek @ 2019-02-25 14:59 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190212143003.48446-14-john.ogness@linutronix.de>
On Tue 2019-02-12 15:29:51, John Ogness wrote:
> Allow each console to track which seq record was last printed. This
> simplifies identifying dropped records.
I would like to see a better justification here. And I think
that it is part of this patchset.
One reason is to implement console reply a cleaner way.
Another reason is to allows calling lockless consoles
directly in emergency situations.
Both reasons are independent on the log buffer implementation.
Therefore I suggest to move it into another patchset.
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index ece54c24ea0d..ebd9aac06323 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1504,6 +1514,19 @@ static void call_console_drivers(const char *ext_text, size_t ext_len,
> if (!cpu_online(raw_smp_processor_id()) &&
> !(con->flags & CON_ANYTIME))
> continue;
> + if (con->printk_seq >= seq)
> + continue;
> +
> + con->printk_seq++;
> + if (con->printk_seq < seq) {
> + print_console_dropped(con, seq - con->printk_seq);
> + con->printk_seq = seq;
It would be great to print this message only when the real one
is not superseded.
The suppressed messages can be proceed quickly. They allow consoles
to catch up with the message producers. But if the spend time
with printing the warning, we just risk loosing more messages
again and again.
Heh, there is a bug in the current code. The warning is not
printed when the currently proceed message is superseded.
I am going to prepare a patch for this.
Best Regards,
Petr
^ 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