* [PATCH v3 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Joel Stanley @ 2017-04-10 4:04 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Mark Rutland, Rob Herring
Cc: Jeremy Kerr, linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Andy Shevchenko,
Benjamin Herrenschmidt, openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <20170410040400.5509-1-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
This change adds a driver for the 16550-based Aspeed virtual UART
device. We use a similar process to the of_serial driver for device
probe, but expose some VUART-specific functions through sysfs too.
The VUART is two UART 'front ends' connected by their FIFO (no actual
serial line in between). One is on the BMC side (management controller)
and one is on the host CPU side.
This driver is for the BMC side. The sysfs files allow the BMC
userspace, which owns the system configuration policy, to specify at
what IO port and interrupt number the host side will appear to the host
on the Host <-> BMC LPC bus. It could be different on a different system
(though most of them use 3f8/4).
OpenPOWER host firmware doesn't like it when the host-side of the
VUART's FIFO is not drained. This driver only disables host TX discard
mode when the port is in use. We set the VUART enabled bit when we bind
to the device, and clear it on unbind.
We don't want to do this on open/release, as the host may be using this
bit to configure serial output modes, which is independent of whether
the devices has been opened by BMC userspace.
Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
v3:
- remove whitespace in header
- reformat comment
- don't check for reg-io-width property; we don't need any special
accessors for reading/writing bytes
- move file to 8250_aspeed_vuart.c
v2:
- Use attribute groups and DEVICE_ATTR_RW
- Use platform_get_resource/devm_ioremap_resource
- of_find_property -> of_property_read_bool
- Drop unncessary 0xff mask
- Fix comment style
- Use BIT and GENMASK where pssible
- Move to 8250 directory
- Rename ast -> aspeed to match other Aspeed drivers
- Add documentation of sysfs file
- Add detail to the commit message
Documentation/ABI/stable/sysfs-driver-aspeed-vuart | 15 +
Documentation/devicetree/bindings/serial/8250.txt | 2 +
drivers/tty/serial/8250/8250_aspeed_vuart.c | 323 +++++++++++++++++++++
drivers/tty/serial/8250/Kconfig | 10 +
drivers/tty/serial/8250/Makefile | 1 +
5 files changed, 351 insertions(+)
create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-vuart
create mode 100644 drivers/tty/serial/8250/8250_aspeed_vuart.c
diff --git a/Documentation/ABI/stable/sysfs-driver-aspeed-vuart b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
new file mode 100644
index 000000000000..8062953ce77b
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-aspeed-vuart
@@ -0,0 +1,15 @@
+What: /sys/bus/platform/drivers/aspeed-vuart/*/lpc_address
+Date: April 2017
+Contact: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
+Description: Configures which IO port the host side of the UART
+ will appear on the host <-> BMC LPC bus.
+Users: OpenBMC. Proposed changes should be mailed to
+ openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
+
+What: /sys/bus/platform/drivers/aspeed-vuart*/sirq
+Date: April 2017
+Contact: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
+Description: Configures which interrupt number the host side of
+ the UART will appear on the host <-> BMC LPC bus.
+Users: OpenBMC. Proposed changes should be mailed to
+ openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
index 10276a46ecef..656733949309 100644
--- a/Documentation/devicetree/bindings/serial/8250.txt
+++ b/Documentation/devicetree/bindings/serial/8250.txt
@@ -20,6 +20,8 @@ Required properties:
- "fsl,16550-FIFO64"
- "fsl,ns16550"
- "ti,da830-uart"
+ - "aspeed,ast2400-vuart"
+ - "aspeed,ast2500-vuart"
- "serial" if the port type is unknown.
- reg : offset and length of the register set for the device.
- interrupts : should contain uart interrupt.
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
new file mode 100644
index 000000000000..5db0023e0225
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -0,0 +1,323 @@
+/*
+ * Serial Port driver for Aspeed VUART device
+ *
+ * Copyright (C) 2016 Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>, IBM Corp.
+ * Copyright (C) 2006 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>, IBM Corp.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/clk.h>
+
+#include "8250.h"
+
+#define ASPEED_VUART_GCRA 0x20
+#define ASPEED_VUART_GCRA_VUART_EN BIT(0)
+#define ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD BIT(5)
+#define ASPEED_VUART_GCRB 0x24
+#define ASPEED_VUART_GCRB_HOST_SIRQ_MASK GENMASK(7, 4)
+#define ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT 4
+#define ASPEED_VUART_ADDRL 0x28
+#define ASPEED_VUART_ADDRH 0x2c
+
+struct aspeed_vuart {
+ struct device *dev;
+ void __iomem *regs;
+ struct clk *clk;
+ int line;
+};
+
+/*
+ * The VUART is basically two UART 'front ends' connected by their FIFO
+ * (no actual serial line in between). One is on the BMC side (management
+ * controller) and one is on the host CPU side.
+ *
+ * It allows the BMC to provide to the host a "UART" that pipes into
+ * the BMC itself and can then be turned by the BMC into a network console
+ * of some sort for example.
+ *
+ * This driver is for the BMC side. The sysfs files allow the BMC
+ * userspace which owns the system configuration policy, to specify
+ * at what IO port and interrupt number the host side will appear
+ * to the host on the Host <-> BMC LPC bus. It could be different on a
+ * different system (though most of them use 3f8/4).
+ */
+
+static ssize_t lpc_address_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+ u16 addr;
+
+ addr = (readb(vuart->regs + ASPEED_VUART_ADDRH) << 8) |
+ (readb(vuart->regs + ASPEED_VUART_ADDRL));
+
+ return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
+}
+
+static ssize_t lpc_address_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+ unsigned long val;
+ int err;
+
+ err = kstrtoul(buf, 0, &val);
+ if (err)
+ return err;
+
+ writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
+ writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(lpc_address);
+
+static ssize_t sirq_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+ u8 reg;
+
+ reg = readb(vuart->regs + ASPEED_VUART_GCRB);
+ reg &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+ reg >>= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
+
+ return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
+}
+
+static ssize_t sirq_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct aspeed_vuart *vuart = dev_get_drvdata(dev);
+ unsigned long val;
+ int err;
+ u8 reg;
+
+ err = kstrtoul(buf, 0, &val);
+ if (err)
+ return err;
+
+ val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
+ val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+
+ reg = readb(vuart->regs + ASPEED_VUART_GCRB);
+ reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+ reg |= val;
+ writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(sirq);
+
+static struct attribute *aspeed_vuart_attrs[] = {
+ &dev_attr_sirq.attr,
+ &dev_attr_lpc_address.attr,
+ NULL,
+};
+
+static const struct attribute_group aspeed_vuart_attr_group = {
+ .attrs = aspeed_vuart_attrs,
+};
+
+static void aspeed_vuart_set_enabled(struct aspeed_vuart *vuart, bool enabled)
+{
+ u8 reg;
+
+ reg = readb(vuart->regs + ASPEED_VUART_GCRA);
+ reg &= ~ASPEED_VUART_GCRA_VUART_EN;
+ if (enabled)
+ reg |= ASPEED_VUART_GCRA_VUART_EN;
+ writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
+}
+
+static void aspeed_vuart_set_host_tx_discard(struct aspeed_vuart *vuart,
+ bool discard)
+{
+ u8 reg;
+
+ reg = readb(vuart->regs + ASPEED_VUART_GCRA);
+
+ /* If the DISABLE_HOST_TX_DISCARD bit is set, discard is disabled */
+ reg &= ~ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
+ if (!discard)
+ reg |= ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD;
+
+ writeb(reg, vuart->regs + ASPEED_VUART_GCRA);
+}
+
+static int aspeed_vuart_startup(struct uart_port *uart_port)
+{
+ struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
+ struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
+ int rc;
+
+ rc = serial8250_do_startup(uart_port);
+ if (rc)
+ return rc;
+
+ aspeed_vuart_set_host_tx_discard(vuart, false);
+
+ return 0;
+}
+
+static void aspeed_vuart_shutdown(struct uart_port *uart_port)
+{
+ struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
+ struct aspeed_vuart *vuart = uart_8250_port->port.private_data;
+
+ aspeed_vuart_set_host_tx_discard(vuart, true);
+
+ serial8250_do_shutdown(uart_port);
+}
+
+static int aspeed_vuart_probe(struct platform_device *pdev)
+{
+ struct uart_8250_port port;
+ struct aspeed_vuart *vuart;
+ struct device_node *np;
+ struct resource *res;
+ u32 clk, prop;
+ int rc;
+
+ np = pdev->dev.of_node;
+
+ vuart = devm_kzalloc(&pdev->dev, sizeof(*vuart), GFP_KERNEL);
+ if (!vuart)
+ return -ENOMEM;
+
+ vuart->dev = &pdev->dev;
+
+ /* The 8510 core creates the mapping, which we grab a reference to
+ * for VUART-specific registers */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ vuart->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(vuart->regs))
+ return PTR_ERR(vuart->regs);
+
+ memset(&port, 0, sizeof(port));
+ port.port.private_data = vuart;
+ port.port.membase = vuart->regs;
+ port.port.mapbase = res->start;
+ port.port.mapsize = resource_size(res);
+ port.port.startup = aspeed_vuart_startup;
+ port.port.shutdown = aspeed_vuart_shutdown;
+ port.port.dev = &pdev->dev;
+
+ rc = sysfs_create_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
+ if (rc < 0)
+ return rc;
+
+ if (of_property_read_u32(np, "clock-frequency", &clk)) {
+ vuart->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(vuart->clk)) {
+ dev_warn(&pdev->dev,
+ "clk or clock-frequency not defined\n");
+ return PTR_ERR(vuart->clk);
+ }
+
+ rc = clk_prepare_enable(vuart->clk);
+ if (rc < 0)
+ return rc;
+
+ clk = clk_get_rate(vuart->clk);
+ }
+
+ /* If current-speed was set, then try not to change it. */
+ if (of_property_read_u32(np, "current-speed", &prop) == 0)
+ port.port.custom_divisor = clk / (16 * prop);
+
+ /* Check for shifted address mapping */
+ if (of_property_read_u32(np, "reg-offset", &prop) == 0)
+ port.port.mapbase += prop;
+
+ /* Check for registers offset within the devices address range */
+ if (of_property_read_u32(np, "reg-shift", &prop) == 0)
+ port.port.regshift = prop;
+
+ /* Check for fifo size */
+ if (of_property_read_u32(np, "fifo-size", &prop) == 0)
+ port.port.fifosize = prop;
+
+ /* Check for a fixed line number */
+ rc = of_alias_get_id(np, "serial");
+ if (rc >= 0)
+ port.port.line = rc;
+
+ port.port.irq = irq_of_parse_and_map(np, 0);
+ port.port.irqflags = IRQF_SHARED;
+ port.port.iotype = UPIO_MEM;
+ port.port.type = PORT_16550A;
+ port.port.uartclk = clk;
+ port.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF
+ | UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_NO_THRE_TEST;
+
+ if (of_property_read_bool(np, "no-loopback-test"))
+ port.port.flags |= UPF_SKIP_TEST;
+
+ if (port.port.fifosize)
+ port.capabilities = UART_CAP_FIFO;
+
+ if (of_property_read_bool(np, "auto-flow-control"))
+ port.capabilities |= UART_CAP_AFE;
+
+ rc = serial8250_register_8250_port(&port);
+ if (rc < 0)
+ goto err_clk_disable;
+
+ vuart->line = rc;
+
+ aspeed_vuart_set_enabled(vuart, true);
+ aspeed_vuart_set_host_tx_discard(vuart, true);
+ platform_set_drvdata(pdev, vuart);
+
+ return 0;
+
+err_clk_disable:
+ clk_disable_unprepare(vuart->clk);
+ irq_dispose_mapping(port.port.irq);
+ return rc;
+}
+
+static int aspeed_vuart_remove(struct platform_device *pdev)
+{
+ struct aspeed_vuart *vuart = platform_get_drvdata(pdev);
+
+ aspeed_vuart_set_enabled(vuart, false);
+ serial8250_unregister_port(vuart->line);
+ sysfs_remove_group(&vuart->dev->kobj, &aspeed_vuart_attr_group);
+ clk_disable_unprepare(vuart->clk);
+
+ return 0;
+}
+
+static const struct of_device_id aspeed_vuart_table[] = {
+ { .compatible = "aspeed,ast2400-vuart" },
+ { .compatible = "aspeed,ast2500-vuart" },
+ { },
+};
+
+static struct platform_driver aspeed_vuart_driver = {
+ .driver = {
+ .name = "aspeed-vuart",
+ .of_match_table = aspeed_vuart_table,
+ },
+ .probe = aspeed_vuart_probe,
+ .remove = aspeed_vuart_remove,
+};
+
+module_platform_driver(aspeed_vuart_driver);
+
+MODULE_AUTHOR("Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Driver for Aspeed VUART device");
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index a65fb8197aec..fb217f02ec94 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -220,6 +220,16 @@ config SERIAL_8250_ACCENT
To compile this driver as a module, choose M here: the module
will be called 8250_accent.
+config SERIAL_8250_ASPEED_VUART
+ tristate "Aspeed Virtual UART"
+ depends on SERIAL_8250
+ depends on OF
+ help
+ If you want to use the virtual UART (VUART) device on Aspeed
+ BMC platforms, enable this option. This enables the 16550A-
+ compatible device on the local LPC bus, giving a UART device
+ with no physical RS232 connections.
+
config SERIAL_8250_BOCA
tristate "Support Boca cards"
depends on SERIAL_8250 != n && ISA && SERIAL_8250_MANY_PORTS
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index 2f30f9ecdb1b..a44a99a3e623 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_SERIAL_8250_EXAR) += 8250_exar.o
obj-$(CONFIG_SERIAL_8250_HP300) += 8250_hp300.o
obj-$(CONFIG_SERIAL_8250_CS) += serial_cs.o
obj-$(CONFIG_SERIAL_8250_ACORN) += 8250_acorn.o
+obj-$(CONFIG_SERIAL_8250_ASPEED_VUART) += 8250_aspeed_vuart.o
obj-$(CONFIG_SERIAL_8250_BCM2835AUX) += 8250_bcm2835aux.o
obj-$(CONFIG_SERIAL_8250_CONSOLE) += 8250_early.o
obj-$(CONFIG_SERIAL_8250_FOURPORT) += 8250_fourport.o
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 1/2] serial: 8250: Add flag so drivers can avoid THRE probe
From: Joel Stanley @ 2017-04-10 4:03 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Mark Rutland, Rob Herring
Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Andy Shevchenko,
Benjamin Herrenschmidt, Jeremy Kerr,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <20170410040400.5509-1-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
The probing of THRE irq behaviour assumes the other end will be reading
bytes out of the buffer in order to probe the port at driver init. In
some cases the other end cannot be relied upon to read these bytes, so
provide a flag for them to skip this step.
Bit 19 was chosen as the flags are a int and the top bits are taken.
Acked-by: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
---
v3:
- Correct the bit number in the changelog
v2:
- No change
drivers/tty/serial/8250/8250_port.c | 2 +-
include/linux/serial_core.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 6119516ef5fc..60a6c247340f 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2229,7 +2229,7 @@ int serial8250_do_startup(struct uart_port *port)
}
}
- if (port->irq) {
+ if (port->irq && !(up->port.flags & UPF_NO_THRE_TEST)) {
unsigned char iir1;
/*
* Test for UARTs that do not reassert THRE when the
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 58484fb35cc8..260245deec94 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -195,6 +195,7 @@ struct uart_port {
#define UPF_NO_TXEN_TEST ((__force upf_t) (1 << 15))
#define UPF_MAGIC_MULTIPLIER ((__force upf_t) ASYNC_MAGIC_MULTIPLIER /* 16 */ )
+#define UPF_NO_THRE_TEST ((__force upf_t) (1 << 19))
/* Port has hardware-assisted h/w flow control */
#define UPF_AUTO_CTS ((__force upf_t) (1 << 20))
#define UPF_AUTO_RTS ((__force upf_t) (1 << 21))
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 0/2] drivers: serial: Aspeed VUART driver
From: Joel Stanley @ 2017-04-10 4:03 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Mark Rutland, Rob Herring
Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, openbmc-uLR06cmDAlY/bJ5BZ2RsiQ,
Andy Shevchenko, Benjamin Herrenschmidt, Jeremy Kerr
This is v3 of a driver for the Aspeed VUART. This version addresses feedback
from Andy and Greg, and includes Rob's ack for the bindings change.
The VUART is a serial device on the BMC side of the LPC bus that connects a BMC
to it's host processor.
We add a flag to the serial core to allow the driver to skip probing of the
THRE irq behaviour, which could hang due to the host not reading bytes out of
the buffer.
We've been using this on systems for over a year, so it has seen a good amount
of testing.
Cheers,
Joel
Jeremy Kerr (1):
drivers/serial: Add driver for Aspeed virtual UART
Joel Stanley (1):
serial: 8250: Add flag so drivers can avoid THRE probe
Documentation/ABI/stable/sysfs-driver-aspeed-vuart | 15 +
Documentation/devicetree/bindings/serial/8250.txt | 2 +
drivers/tty/serial/8250/8250_aspeed_vuart.c | 323 +++++++++++++++++++++
drivers/tty/serial/8250/8250_port.c | 2 +-
drivers/tty/serial/8250/Kconfig | 10 +
drivers/tty/serial/8250/Makefile | 1 +
include/linux/serial_core.h | 1 +
7 files changed, 353 insertions(+), 1 deletion(-)
create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-vuart
create mode 100644 drivers/tty/serial/8250/8250_aspeed_vuart.c
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v3 1/2] serial: 8250: Add flag so drivers can avoid THRE probe
From: Joel Stanley @ 2017-04-10 3:59 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Mark Rutland, Rob Herring
Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Andy Shevchenko,
Benjamin Herrenschmidt, Jeremy Kerr,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <20170410035904.29443-1-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
The probing of THRE irq behaviour assumes the other end will be reading
bytes out of the buffer in order to probe the port at driver init. In
some cases the other end cannot be relied upon to read these bytes, so
provide a flag for them to skip this step.
Bit 19 was chosen as the flags are a int and the top bits are taken.
Acked-by: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
---
v3:
- Correct the bit number in the changelog
v2:
- No change
drivers/tty/serial/8250/8250_port.c | 2 +-
include/linux/serial_core.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 6119516ef5fc..60a6c247340f 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2229,7 +2229,7 @@ int serial8250_do_startup(struct uart_port *port)
}
}
- if (port->irq) {
+ if (port->irq && !(up->port.flags & UPF_NO_THRE_TEST)) {
unsigned char iir1;
/*
* Test for UARTs that do not reassert THRE when the
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 58484fb35cc8..260245deec94 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -195,6 +195,7 @@ struct uart_port {
#define UPF_NO_TXEN_TEST ((__force upf_t) (1 << 15))
#define UPF_MAGIC_MULTIPLIER ((__force upf_t) ASYNC_MAGIC_MULTIPLIER /* 16 */ )
+#define UPF_NO_THRE_TEST ((__force upf_t) (1 << 19))
/* Port has hardware-assisted h/w flow control */
#define UPF_AUTO_CTS ((__force upf_t) (1 << 20))
#define UPF_AUTO_RTS ((__force upf_t) (1 << 21))
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 0/2] drivers: serial: Aspeed VUART driver
From: Joel Stanley @ 2017-04-10 3:59 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Mark Rutland, Rob Herring
Cc: linux-serial, linux-kernel, devicetree, openbmc, Andy Shevchenko,
Benjamin Herrenschmidt, Jeremy Kerr
This is v3 of a driver for the Aspeed VUART. This version addresses feedback
from Andy and Greg, and includes Rob's ack for the bindings change.
The VUART is a serial device on the BMC side of the LPC bus that connects a BMC
to it's host processor.
We add a flag to the serial core to allow the driver to skip probing of the
THRE irq behaviour, which could hang due to the host not reading bytes out of
the buffer.
We've been using this on systems for over a year, so it has seen a good amount
of testing.
Cheers,
Joel
Jeremy Kerr (1):
drivers/serial: Add driver for Aspeed virtual UART
Joel Stanley (1):
serial: 8250: Add flag so drivers can avoid THRE probe
Documentation/ABI/stable/sysfs-driver-aspeed-vuart | 15 +
Documentation/devicetree/bindings/serial/8250.txt | 2 +
drivers/tty/serial/8250/8250_aspeed_vuart.c | 323 +++++++++++++++++++++
drivers/tty/serial/8250/8250_port.c | 2 +-
drivers/tty/serial/8250/Kconfig | 10 +
drivers/tty/serial/8250/Makefile | 1 +
include/linux/serial_core.h | 1 +
7 files changed, 353 insertions(+), 1 deletion(-)
create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-vuart
create mode 100644 drivers/tty/serial/8250/8250_aspeed_vuart.c
--
2.11.0
^ permalink raw reply
* Re: [PATCH v2 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Joel Stanley @ 2017-04-10 3:07 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, Jiri Slaby, Mark Rutland, Rob Herring,
Jeremy Kerr, linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree,
Benjamin Herrenschmidt, OpenBMC Maillist
In-Reply-To: <CAHp75VckYiBccVN+_K+Tm38kcdC10=sekQZg7Uz_HwS4xyW2UA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Apr 5, 2017 at 8:24 PM, Andy Shevchenko
<andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, Apr 5, 2017 at 7:03 AM, Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org> wrote:
>
>> + port.port.irq = irq_of_parse_and_map(np, 0);
>
> Isn't better to get this via platform_get_irq() ?
I can't see the benefit.
>
>> + port.port.irqflags = IRQF_SHARED;
>> + port.port.iotype = UPIO_MEM;
>
>> + if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {
>
> I would still go with usual pattern.
>
>> + switch (prop) {
>> + case 1:
>> + port.port.iotype = UPIO_MEM;
>> + break;
>> + case 4:
>
>> + port.port.iotype = of_device_is_big_endian(np) ?
>> + UPIO_MEM32BE : UPIO_MEM32;
>
> Hmm... And this one is not in align with IO accessors used in this
> driver. (readx()/writex() are little endian IO accessors).
We only perform readb/writeb, however you raise a good point that
we're assuming LE in the register layout. I will remove checking of this
optional property.
I will send v3 with the other cleanups you mentioned.
Cheers,
Joel
>
>> + break;
>> + default:
>> + dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n",
>> + prop);
>> + rc = -EINVAL;
>> + goto err_clk_disable;
>> + }
>> + }
>> +
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [patch 0/7] staging: speakup: introduce tty-based comms
From: Okash Khawaja @ 2017-04-09 16:54 UTC (permalink / raw)
To: Samuel Thibault, Rob Herring, Greg Kroah-Hartman, Jiri Slaby,
linux-kernel@vger.kernel.org, devel, linux-serial@vger.kernel.org,
Kirk Reiser, Chris Brannon, speakup
In-Reply-To: <20170322000524.7k653z7ui2zzgan6@var.youpi.perso.aquilenet.fr>
Hi,
Any updates on this? We can start working in
tty_port_register_serdev_device() if that's okay?
Thanks,
Okash
On Wed, Mar 22, 2017 at 01:05:24AM +0100, Samuel Thibault wrote:
> Hello,
>
> So Rob, how do you see this going? Shall we introduce a serdev_device
> *tty_port_register_serdev_device(tty, device)? Will you work on it or
> should I give it a try?
>
> Samuel
>
> Samuel Thibault, on mer. 15 mars 2017 16:03:23 +0100, wrote:
> > Rob Herring, on mer. 15 mars 2017 09:45:59 -0500, wrote:
> > > On Mon, Mar 13, 2017 at 8:18 PM, Samuel Thibault
> > > <samuel.thibault@ens-lyon.org> wrote:
> > > > Samuel Thibault, on mar. 14 mars 2017 01:47:01 +0100, wrote:
> > > >> Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> > > >> > On Mon, Mar 13, 2017 at 10:05:51PM +0000, okash.khawaja@gmail.com wrote:
> > > >> > > This patchset introduces a TTY-based way for the synths to communicate
> > > >> > > with devices as an alternate for direct serial comms used by the synths
> > > >> > > at the moment. It then migrates some of the synths to the TTY-based
> > > >> > > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> > > >> > > txprt.
> > > >> >
> > > >> > What about using the serbus code that is now in the tree? That should
> > > >> > make this a lot easier than your patchset from what I can see.
> > > >>
> > > >> Mmm... AIUI from reading tty_port_register_device_attr, one
> > > >> would have to have registered a speakup serdev device driver
> > > >> *before* tty_port_register_device_attr gets called, so that
> > > >> serdev_tty_port_register matches the driver in the loop of
> > > >> of_serdev_register_devices, and no TTY cdev is created?
> > >
> > > Not exactly. The driver doesn't have to be registered nor loaded, but
> > > the device does have to be present so no tty cdev is created. The only
> > > way a device is present currently is when a node is in the DT. I
> > > expect the x86 folks will be adding ACPI support soon.
> >
> > Ok, but in our case there is no hardware device that the system can
> > see/probe, it's just plugged externally.
> >
> > > >> That would mean that speakup can not be loaded as a module after ttyS0
> > > >> initialization, that won't fly for our use needs. The line discipline
> > > >> mechanism allows us to attach ourself to an existing tty. Could we
> > > >> imagine a tty_port function which removes the cdev and tries to register
> > > >> the tty port again to serdev?
> > > >>
> > > >> What we basically need to be able to say on speakup module load is
> > > >> e.g. "I'm now attaching a device to ttyS0, use this serdev_device_ops to
> > > >> discuss with it".
> > > >
> > > > That for_each_available_child_of_node loop is really way more complex
> > > > than what we need. And what's more, it's not working without CONFIG_OF
> > > > (!)
> > > >
> > > > It would really make sense to me to have a
> > > >
> > > > serdev_device *tty_port_register_serdev_device(tty, device)
> > > >
> > > > which unregisters the character device of the tty, and creates instead a
> > > > controler with the given device plugged to it. Really much like a line
> > > > discipline, but way simpler :)
> > >
> > > What would trigger calling this function?
> >
> > From the user point of view, loading the speakup module for the external
> > device, typically, with "ttyS0" as module parameter. Then the speakup
> > init function can do whatever it needs to achieve this :)
^ permalink raw reply
* Re: 8250: Possible race between console message vs DMA?
From: Andy Shevchenko @ 2017-04-09 11:07 UTC (permalink / raw)
To: Vignesh R
Cc: linux-serial@vger.kernel.org, Peter Hurley, Greg Kroah-Hartman,
linux-omap@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <7c75614c-71cb-0a4e-222c-713f65a89f23@ti.com>
On Fri, Apr 7, 2017 at 2:08 PM, Vignesh R <vigneshr@ti.com> wrote:
> Hi All,
>
> I seem to be hitting a race condition using 8250_dma (and 8250_omap
> specific dma) support:
>
> Kernel writes log messages to console via
> serial8250_console_write()->serial8250_console_putchar() which directly
> accesses UART_TX register with port->lock acquired.
>
> Now, if the same UART instance is being used by systemd/userspace,
> characters are written to UART_TX register by serial8250_tx_chars(). The
> concurrent access by serial8250_console_write() and
> serial8250_tx_chars() is serialized by the use of port->lock spinlock
> and hence there is no issue with` non DMA case.
>
> But when using DMA with 8250 UART, I see that port->lock is held before
> scheduling of DMA TX transfer and released as soon as the transfer is
> submitted. The lock is not held until the transfer actually completes
> See,
> uart_start()
> ->serial8250_start_tx()->
> __start_tx()
> ->up->dma->tx_dma(up)
> Or
> __dma_tx_complete() in 8250_dma.c that acquires and releases port->lock
> once TX DMA transfer is submitted in serial8250_tx_dma()
>
> So, when the port->lock is released, it is quite possible that DMA is
> still transferring data to UART TX FIFO and UART FIFO might be almost full.
> I see that when DMA is writing to UART TX FIFO,
> serial8250_console_write() may also write kernel log messages to UART TX
> FIFO(as port->lock is now free to be acquired), which is leading to
> overflow and lose of data. serial8250_console_write() checks for
> UART_LSR_THRE to check if Transmit hold register is empty but that may
> not be enough as DMA might put data before CPU write.
>
> It seems that both DMA and CPU might simultaneously put data to UART
> FIFO and lead to potential loss of data.
> Is the expectation that UART instance used to print kernel log messages
> is not intended to use DMA? Or am I missing something?
>
>
> Any help appreciated!
I have one patch in my tree for a long time already:
https://bitbucket.org/andy-shev/linux/commits/9f86c648e53bd25b8ec374933764577b2a340468?at=topic/uart/rpm
Besides that I have patch to disable power management on kernel
console (and non-hackish implementation of runtime PM for UART is
there in case you are wondering what that repository for).
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCHv3 02/10] serdev: add serdev_device_wait_until_sent
From: Greg Kroah-Hartman @ 2017-04-08 16:57 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, Rob Herring,
Samuel Thibault, Pavel Machek, Tony Lindgren, Jiri Slaby,
Mark Rutland, linux-bluetooth, linux-serial, devicetree,
linux-kernel
In-Reply-To: <20170328155939.31566-3-sre@kernel.org>
On Tue, Mar 28, 2017 at 05:59:31PM +0200, Sebastian Reichel wrote:
> Add method, which waits until the transmission buffer has been sent.
> Note, that the change in ttyport_write_wakeup is related, since
> tty_wait_until_sent will hang without that change.
>
> Acked-by: Rob Herring <robh@kernel.org>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> ---
> Changes since PATCHv2:
> * Avoid goto in ttyport_write_wakeup
> ---
> drivers/tty/serdev/core.c | 11 +++++++++++
> drivers/tty/serdev/serdev-ttyport.c | 18 ++++++++++++++----
> include/linux/serdev.h | 3 +++
> 3 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index f4c6c90add78..a63b74031e22 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -173,6 +173,17 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
> }
> EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
>
> +void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
> +{
> + struct serdev_controller *ctrl = serdev->ctrl;
> +
> + if (!ctrl || !ctrl->ops->wait_until_sent)
> + return;
> +
> + ctrl->ops->wait_until_sent(ctrl, timeout);
> +}
> +EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
Is this still needed now that we have serdev_device_write() with an
unlimited timeout available?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 31/38] Annotate hardware config module parameters in drivers/tty/
From: Greg KH @ 2017-04-08 15:19 UTC (permalink / raw)
To: David Howells
Cc: linux-kernel, gnomes, linux-security-module, keyrings,
linux-serial, Jiri Slaby
In-Reply-To: <149141168659.29162.541205367169653847.stgit@warthog.procyon.org.uk>
On Wed, Apr 05, 2017 at 06:01:26PM +0100, David Howells wrote:
> When the kernel is running in secure boot mode, we lock down the kernel to
> prevent userspace from modifying the running kernel image. Whilst this
> includes prohibiting access to things like /dev/mem, it must also prevent
> access by means of configuring driver modules in such a way as to cause a
> device to access or modify the kernel image.
>
> To this end, annotate module_param* statements that refer to hardware
> configuration and indicate for future reference what type of parameter they
> specify. The parameter parser in the core sees this information and can
> skip such parameters with an error message if the kernel is locked down.
> The module initialisation then runs as normal, but just sees whatever the
> default values for those parameters is.
>
> Note that we do still need to do the module initialisation because some
> drivers have viable defaults set in case parameters aren't specified and
> some drivers support automatic configuration (e.g. PNP or PCI) in addition
> to manually coded parameters.
>
> This patch annotates drivers in drivers/tty/.
>
> Suggested-by: Alan Cox <gnomes@lxorguk.ukuu.org.uk>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> cc: Jiri Slaby <jslaby@suse.com>
> cc: linux-serial@vger.kernel.org
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply
* Re: Supporting early console on a SERIAL_8250_RT288X platform
From: Mason @ 2017-04-07 12:57 UTC (permalink / raw)
To: Robin Murphy, linux-serial, Peter Hurley
Cc: Rob Herring, Mans Rullgard, Russell King, Greg Kroah-Hartman,
Thibaud Cornic, Jean Delvare, Jiri Slaby, Andreas Farber,
Linux ARM
In-Reply-To: <d797386a-009e-f21d-dec4-a7520b4f6a72@arm.com>
Thanks for answering so quickly! :-)
On 07/04/2017 14:18, Robin Murphy wrote:
> On 07/04/17 12:53, Mason wrote:
>
>> Did I implement something wrong?
>> Why am I not seeing the output of the panic() call, like I do
>> with earlyprintk?
>> Since tty0 is enabled, why isn't it picking up where palmchip0
>> left off?
>
> As far as I'm aware, tty0 is a purely virtual console: look further down
> still and you'll probably see a second switch over from tty0 to ttyS0,
> which will be when the buffered output *actually* starts coming out
> again. That's certainly what happens for me, albeit with
> "pl11"->"tty0"->"ttyAMA0", (and complete with not realising ttyAMA0 is
> still the same port and reprinting the entire log buffer from the top).
Right you are!
Also, I had not noticed the "print the entire log buffer from the top"
issue you mention.
[ 0.000000] Booting Linux on physical CPU 0x0
...
[ 0.000000] earlycon: palmchip0 at MMIO 0x00010700 (options '115200n8')
[ 0.000000] bootconsole [palmchip0] enabled
...
[ 0.014147] Console: colour dummy device 80x30
[ 0.018616] console [tty0] enabled
[ 0.022039] bootconsole [palmchip0] disabled
[ 0.000000] Booting Linux on physical CPU 0x0
...
[ 0.000000] earlycon: palmchip0 at MMIO 0x00010700 (options '115200n8')
[ 0.000000] bootconsole [palmchip0] enabled
[ 0.014147] Console: colour dummy device 80x30
[ 0.018616] console [tty0] enabled
[ 0.022039] bootconsole [palmchip0] disabled
...
[ 0.460630] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
[ 0.462278] 10700.serial: ttyS0 at MMIO 0x10700 (irq = 20, base_baud = 460800) is a Palmchip BK-3103
[ 1.084227] console [ttyS0] enabled
> I believe this is something to do with how the DT stdout-path property
> is implemented, because with an explicit "console=ttyAMA0" (or ttyS0 in
> your case) on the command line instead of relying on stdout-path, the
> boot console does switch directly to the real UART without the
> intermediate tty0 blind spot.
Indeed, an explicit console=ttyS0,115200 does avoid the "dupe the entire log"
issue. There is still some "stuttering" involved:
[ 0.000000] earlycon: palmchip0 at MMIO 0x00010700 (options '115200n8')
[ 0.000000] bootconsole [palmchip0] enabled
...
[ 0.745811] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
[ 0.753681] console [ttyS0] disabled
[ 0.757426] 10700.serial: ttyS0 at MMIO 0x10700 (irq = 20, base_baud = 460800) is a Palmchip BK-3103
[ 0.766672] console [ttyS0] enabled
[ 0.766672] console [ttyS0] enabled
[ 0.773726] bootconsole [palmchip0] disabled
[ 0.773726] bootconsole [palmchip0] disabled
More importantly, it also solves the "hang before panic message" issue:
...
[ 0.575937] io scheduler noop registered
[ 0.579888] io scheduler deadline registered
[ 0.584245] io scheduler cfq registered (default)
[ 0.589650] Kernel panic - not syncing: THIS IS A TEST
Hopefully, supporting earlycon on this platform can be included
in time for v4.12 :-)
Greg, Peter, what do you think? ;-)
Regards.
^ permalink raw reply
* Re: Supporting early console on a SERIAL_8250_RT288X platform
From: Robin Murphy @ 2017-04-07 12:18 UTC (permalink / raw)
To: Mason, linux-serial, Peter Hurley
Cc: Rob Herring, Mans Rullgard, Russell King, Greg Kroah-Hartman,
Thibaud Cornic, Jean Delvare, Jiri Slaby, Andreas Farber,
Linux ARM
In-Reply-To: <0ac351b9-46bd-3a35-fab3-73177cabb747@free.fr>
On 07/04/17 12:53, Mason wrote:
> Hello,
>
> Some drivers hang or panic before the normal console is available.
> In these situations, "earlyprintk" is a life-saver.
>
> My problem with earlyprintk is that the UART address needs to be
> hard-coded in the kernel binary, which means I can't use the same
> binary for two chips with different UART addresses, right?
>
> As far as I understand, "earlycon" helps in that situation, because
> it looks up the UART address at run-time in the device tree.
>
>
> QUESTION #1
>
> Is that the only difference between earlyprintk and earlycon?
> (Probably not, see below)
>
>
> I hacked kernel v4.9.20 to add early console support on my platform.
> The patch is provided below, I'd like to hear comments on what needs
> fixing... because something is not right. If I call panic() in one of
> my driver's probe function, I don't see the output of the panic()
> call, which makes the whole exercise rather pointless.
>
> This is what I see:
>
> [ 0.000000] Booting Linux on physical CPU 0x0
> [ 0.000000] Linux version 4.9.20-1-rc3 (gcc version 5.3.1 20160113 (Linaro GCC 5.3-2016.02) ) #4 SMP PREEMPT Fri Apr 7 12:57
> [ 0.000000] CPU: ARMv7 Processor [413fc090] revision 0 (ARMv7), cr=10c5387d
> [ 0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
> [ 0.000000] OF: fdt:Machine model: Sigma Designs SMP8758 Vantage-1172 Rev E1
> [ 0.000000] ENTER param_setup_earlycon
> [ 0.000000] ENTER early_init_dt_scan_chosen_stdout
> [ 0.000000] early_init_dt_scan_chosen_stdout: scan __earlycon_table
> [ 0.000000] name=palmchip compatible=ralink,rt2880-uart setup=early_palmchip_setup
> [ 0.000000] ENTER of_setup_earlycon
> [ 0.000000] earlycon: palmchip0 at MMIO 0x00010700 (options '115200n8')
> [ 0.000000] bootconsole [palmchip0] enabled
> [ 0.000000] Memory policy: Data cache writealloc
> [ 0.000000] percpu: Embedded 14 pages/cpu @cfdd6000 s24704 r8192 d24448 u57344
> [ 0.000000] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 65024
> [ 0.000000] Kernel command line: mem=256M earlycon
> [ 0.000000] PID hash table entries: 1024 (order: 0, 4096 bytes)
> [ 0.000000] Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)
> [ 0.000000] Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)
> [ 0.000000] Memory: 248064K/262144K available (4096K kernel code, 147K rwdata, 892K rodata, 6144K init, 233K bss, 14080K reserved, 0K cma-reserved, 0K highmem)
> [ 0.000000] Virtual kernel memory layout:
> [ 0.000000] vector : 0xffff0000 - 0xffff1000 ( 4 kB)
> [ 0.000000] fixmap : 0xffc00000 - 0xfff00000 (3072 kB)
> [ 0.000000] vmalloc : 0xd0800000 - 0xff800000 ( 752 MB)
> [ 0.000000] lowmem : 0xc0000000 - 0xd0000000 ( 256 MB)
> [ 0.000000] pkmap : 0xbfe00000 - 0xc0000000 ( 2 MB)
> [ 0.000000] modules : 0xbf000000 - 0xbfe00000 ( 14 MB)
> [ 0.000000] .text : 0xc0008000 - 0xc0500000 (5088 kB)
> [ 0.000000] .init : 0xc0600000 - 0xc0c00000 (6144 kB)
> [ 0.000000] .data : 0xc0c00000 - 0xc0c24c60 ( 148 kB)
> [ 0.000000] .bss : 0xc0c24c60 - 0xc0c5f140 ( 234 kB)
> [ 0.000000] Preemptible hierarchical RCU implementation.
> [ 0.000000] Build-time adjustment of leaf fanout to 32.
> [ 0.000000] RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=2.
> [ 0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=32, nr_cpu_ids=2
> [ 0.000000] NR_IRQS:16 nr_irqs:16 16
> [ 0.000000] L2C-310 enabling early BRESP for Cortex-A9
> [ 0.000000] L2C-310 ID prefetch enabled, offset 4 lines
> [ 0.000000] L2C-310 dynamic clock gating enabled, standby mode enabled
> [ 0.000000] L2C-310 cache controller enabled, 8 ways, 512 kB
> [ 0.000000] L2C-310: CACHE_ID 0x410000c8, AUX_CTRL 0x72860401
> [ 0.000000] clocksource: tango-xtal: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 70787423951 ns
> [ 0.000003] sched_clock: 32 bits at 27MHz, resolution 37ns, wraps every 79536431085ns
> [ 0.007873] Switching to timer-based delay loop, resolution 37ns
> [ 0.014146] Console: colour dummy device 80x30
> [ 0.018616] console [tty0] enabled
> [ 0.022039] bootconsole [palmchip0] disabled
>
> And it just hangs there...
>
> If I remove the panic() call, it boots normally, like this:
>
> ...
> [ 0.014147] Console: colour dummy device 80x30
> [ 0.018616] console [tty0] enabled
> [ 0.022039] bootconsole [palmchip0] disabled
> [ 0.026345] Calibrating delay loop (skipped), value calculated using timer frequency.. 54.25 BogoMIPS (lpj=90000)
> [ 0.026364] pid_max: default: 32768 minimum: 301
> ...
>
> Did I implement something wrong?
> Why am I not seeing the output of the panic() call, like I do
> with earlyprintk?
> Since tty0 is enabled, why isn't it picking up where palmchip0
> left off?
As far as I'm aware, tty0 is a purely virtual console: look further down
still and you'll probably see a second switch over from tty0 to ttyS0,
which will be when the buffered output *actually* starts coming out
again. That's certainly what happens for me, albeit with
"pl11"->"tty0"->"ttyAMA0", (and complete with not realising ttyAMA0 is
still the same port and reprinting the entire log buffer from the top).
I believe this is something to do with how the DT stdout-path property
is implemented, because with an explicit "console=ttyAMA0" (or ttyS0 in
your case) on the command line instead of relying on stdout-path, the
boot console does switch directly to the real UART without the
intermediate tty0 blind spot.
Robin.
>
>
> Earlier discussion:
> https://www.spinics.net/lists/linux-serial/msg16227.html
>
>
> This is the patch I'm using:
>
> diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
> index 85a12f032402..2adabe788c17 100644
> --- a/drivers/tty/serial/8250/8250_early.c
> +++ b/drivers/tty/serial/8250/8250_early.c
> @@ -172,3 +172,52 @@ static int __init early_omap8250_setup(struct earlycon_device *device,
> OF_EARLYCON_DECLARE(omap8250, "ti,omap4-uart", early_omap8250_setup);
>
> #endif
> +
> +#ifdef CONFIG_SERIAL_8250_RT288X
> +
> +extern unsigned int au_serial_in(struct uart_port *p, int offset);
> +extern void au_serial_out(struct uart_port *p, int offset, int value);
> +
> +/*** Copy serial_putc with s/serial8250_early/au_serial/g ***/
> +
> +static void __init palmchip_putc(struct uart_port *port, int c)
> +{
> + unsigned int status;
> +
> + au_serial_out(port, UART_TX, c);
> +
> + for (;;) {
> + status = au_serial_in(port, UART_LSR);
> + if ((status & BOTH_EMPTY) == BOTH_EMPTY)
> + break;
> + cpu_relax();
> + }
> +}
> +
> +/*** Copy early_serial8250_write with s/serial_putc/palmchip_putc/g ***/
> +
> +static void __init early_palmchip_write(struct console *console,
> + const char *s, unsigned int count)
> +{
> + struct earlycon_device *device = console->data;
> + struct uart_port *port = &device->port;
> +
> + //extern void printascii(const char *); printascii("EARLY: ");
> + uart_console_write(port, s, count, palmchip_putc);
> +}
> +
> +static int __init early_palmchip_setup(struct earlycon_device *device,
> + const char *options)
> +{
> + struct uart_port *port = &device->port;
> +
> + if (!(device->port.membase || device->port.iobase))
> + return -ENODEV;
> +
> + port->regshift = 2; /* Hard coding the value doesn't feel right */
> + device->con->write = early_palmchip_write;
> + return 0;
> +}
> +OF_EARLYCON_DECLARE(palmchip, "ralink,rt2880-uart", early_palmchip_setup);
> +
> +#endif
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 080d5a59d0a7..7eb60146cb3f 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -313,7 +313,7 @@ static void default_serial_dl_write(struct uart_8250_port *up, int value)
> -1, /* UART_SCR (unmapped) */
> };
>
> -static unsigned int au_serial_in(struct uart_port *p, int offset)
> +/*static*/ unsigned int au_serial_in(struct uart_port *p, int offset)
> {
> if (offset >= ARRAY_SIZE(au_io_in_map))
> return UINT_MAX;
> @@ -323,7 +323,7 @@ static unsigned int au_serial_in(struct uart_port *p, int offset)
> return __raw_readl(p->membase + (offset << p->regshift));
> }
>
> -static void au_serial_out(struct uart_port *p, int offset, int value)
> +/*static*/ void au_serial_out(struct uart_port *p, int offset, int value)
> {
> if (offset >= ARRAY_SIZE(au_io_out_map))
> return;
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* Supporting early console on a SERIAL_8250_RT288X platform
From: Mason @ 2017-04-07 11:53 UTC (permalink / raw)
To: linux-serial, Peter Hurley
Cc: Rob Herring, Mans Rullgard, Russell King, Greg Kroah-Hartman,
Thibaud Cornic, Linux ARM, Jiri Slaby, Andreas Farber,
Jean Delvare
Hello,
Some drivers hang or panic before the normal console is available.
In these situations, "earlyprintk" is a life-saver.
My problem with earlyprintk is that the UART address needs to be
hard-coded in the kernel binary, which means I can't use the same
binary for two chips with different UART addresses, right?
As far as I understand, "earlycon" helps in that situation, because
it looks up the UART address at run-time in the device tree.
QUESTION #1
Is that the only difference between earlyprintk and earlycon?
(Probably not, see below)
I hacked kernel v4.9.20 to add early console support on my platform.
The patch is provided below, I'd like to hear comments on what needs
fixing... because something is not right. If I call panic() in one of
my driver's probe function, I don't see the output of the panic()
call, which makes the whole exercise rather pointless.
This is what I see:
[ 0.000000] Booting Linux on physical CPU 0x0
[ 0.000000] Linux version 4.9.20-1-rc3 (gcc version 5.3.1 20160113 (Linaro GCC 5.3-2016.02) ) #4 SMP PREEMPT Fri Apr 7 12:57
[ 0.000000] CPU: ARMv7 Processor [413fc090] revision 0 (ARMv7), cr=10c5387d
[ 0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
[ 0.000000] OF: fdt:Machine model: Sigma Designs SMP8758 Vantage-1172 Rev E1
[ 0.000000] ENTER param_setup_earlycon
[ 0.000000] ENTER early_init_dt_scan_chosen_stdout
[ 0.000000] early_init_dt_scan_chosen_stdout: scan __earlycon_table
[ 0.000000] name=palmchip compatible=ralink,rt2880-uart setup=early_palmchip_setup
[ 0.000000] ENTER of_setup_earlycon
[ 0.000000] earlycon: palmchip0 at MMIO 0x00010700 (options '115200n8')
[ 0.000000] bootconsole [palmchip0] enabled
[ 0.000000] Memory policy: Data cache writealloc
[ 0.000000] percpu: Embedded 14 pages/cpu @cfdd6000 s24704 r8192 d24448 u57344
[ 0.000000] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 65024
[ 0.000000] Kernel command line: mem=256M earlycon
[ 0.000000] PID hash table entries: 1024 (order: 0, 4096 bytes)
[ 0.000000] Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)
[ 0.000000] Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)
[ 0.000000] Memory: 248064K/262144K available (4096K kernel code, 147K rwdata, 892K rodata, 6144K init, 233K bss, 14080K reserved, 0K cma-reserved, 0K highmem)
[ 0.000000] Virtual kernel memory layout:
[ 0.000000] vector : 0xffff0000 - 0xffff1000 ( 4 kB)
[ 0.000000] fixmap : 0xffc00000 - 0xfff00000 (3072 kB)
[ 0.000000] vmalloc : 0xd0800000 - 0xff800000 ( 752 MB)
[ 0.000000] lowmem : 0xc0000000 - 0xd0000000 ( 256 MB)
[ 0.000000] pkmap : 0xbfe00000 - 0xc0000000 ( 2 MB)
[ 0.000000] modules : 0xbf000000 - 0xbfe00000 ( 14 MB)
[ 0.000000] .text : 0xc0008000 - 0xc0500000 (5088 kB)
[ 0.000000] .init : 0xc0600000 - 0xc0c00000 (6144 kB)
[ 0.000000] .data : 0xc0c00000 - 0xc0c24c60 ( 148 kB)
[ 0.000000] .bss : 0xc0c24c60 - 0xc0c5f140 ( 234 kB)
[ 0.000000] Preemptible hierarchical RCU implementation.
[ 0.000000] Build-time adjustment of leaf fanout to 32.
[ 0.000000] RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=2.
[ 0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=32, nr_cpu_ids=2
[ 0.000000] NR_IRQS:16 nr_irqs:16 16
[ 0.000000] L2C-310 enabling early BRESP for Cortex-A9
[ 0.000000] L2C-310 ID prefetch enabled, offset 4 lines
[ 0.000000] L2C-310 dynamic clock gating enabled, standby mode enabled
[ 0.000000] L2C-310 cache controller enabled, 8 ways, 512 kB
[ 0.000000] L2C-310: CACHE_ID 0x410000c8, AUX_CTRL 0x72860401
[ 0.000000] clocksource: tango-xtal: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 70787423951 ns
[ 0.000003] sched_clock: 32 bits at 27MHz, resolution 37ns, wraps every 79536431085ns
[ 0.007873] Switching to timer-based delay loop, resolution 37ns
[ 0.014146] Console: colour dummy device 80x30
[ 0.018616] console [tty0] enabled
[ 0.022039] bootconsole [palmchip0] disabled
And it just hangs there...
If I remove the panic() call, it boots normally, like this:
...
[ 0.014147] Console: colour dummy device 80x30
[ 0.018616] console [tty0] enabled
[ 0.022039] bootconsole [palmchip0] disabled
[ 0.026345] Calibrating delay loop (skipped), value calculated using timer frequency.. 54.25 BogoMIPS (lpj=90000)
[ 0.026364] pid_max: default: 32768 minimum: 301
...
Did I implement something wrong?
Why am I not seeing the output of the panic() call, like I do
with earlyprintk?
Since tty0 is enabled, why isn't it picking up where palmchip0
left off?
Earlier discussion:
https://www.spinics.net/lists/linux-serial/msg16227.html
This is the patch I'm using:
diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index 85a12f032402..2adabe788c17 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -172,3 +172,52 @@ static int __init early_omap8250_setup(struct earlycon_device *device,
OF_EARLYCON_DECLARE(omap8250, "ti,omap4-uart", early_omap8250_setup);
#endif
+
+#ifdef CONFIG_SERIAL_8250_RT288X
+
+extern unsigned int au_serial_in(struct uart_port *p, int offset);
+extern void au_serial_out(struct uart_port *p, int offset, int value);
+
+/*** Copy serial_putc with s/serial8250_early/au_serial/g ***/
+
+static void __init palmchip_putc(struct uart_port *port, int c)
+{
+ unsigned int status;
+
+ au_serial_out(port, UART_TX, c);
+
+ for (;;) {
+ status = au_serial_in(port, UART_LSR);
+ if ((status & BOTH_EMPTY) == BOTH_EMPTY)
+ break;
+ cpu_relax();
+ }
+}
+
+/*** Copy early_serial8250_write with s/serial_putc/palmchip_putc/g ***/
+
+static void __init early_palmchip_write(struct console *console,
+ const char *s, unsigned int count)
+{
+ struct earlycon_device *device = console->data;
+ struct uart_port *port = &device->port;
+
+ //extern void printascii(const char *); printascii("EARLY: ");
+ uart_console_write(port, s, count, palmchip_putc);
+}
+
+static int __init early_palmchip_setup(struct earlycon_device *device,
+ const char *options)
+{
+ struct uart_port *port = &device->port;
+
+ if (!(device->port.membase || device->port.iobase))
+ return -ENODEV;
+
+ port->regshift = 2; /* Hard coding the value doesn't feel right */
+ device->con->write = early_palmchip_write;
+ return 0;
+}
+OF_EARLYCON_DECLARE(palmchip, "ralink,rt2880-uart", early_palmchip_setup);
+
+#endif
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 080d5a59d0a7..7eb60146cb3f 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -313,7 +313,7 @@ static void default_serial_dl_write(struct uart_8250_port *up, int value)
-1, /* UART_SCR (unmapped) */
};
-static unsigned int au_serial_in(struct uart_port *p, int offset)
+/*static*/ unsigned int au_serial_in(struct uart_port *p, int offset)
{
if (offset >= ARRAY_SIZE(au_io_in_map))
return UINT_MAX;
@@ -323,7 +323,7 @@ static unsigned int au_serial_in(struct uart_port *p, int offset)
return __raw_readl(p->membase + (offset << p->regshift));
}
-static void au_serial_out(struct uart_port *p, int offset, int value)
+/*static*/ void au_serial_out(struct uart_port *p, int offset, int value)
{
if (offset >= ARRAY_SIZE(au_io_out_map))
return;
^ permalink raw reply related
* 8250: Possible race between console message vs DMA?
From: Vignesh R @ 2017-04-07 11:08 UTC (permalink / raw)
To: linux-serial@vger.kernel.org
Cc: Peter Hurley, Greg Kroah-Hartman, linux-omap@vger.kernel.org,
Andy Shevchenko, linux-kernel@vger.kernel.org
Hi All,
I seem to be hitting a race condition using 8250_dma (and 8250_omap
specific dma) support:
Kernel writes log messages to console via
serial8250_console_write()->serial8250_console_putchar() which directly
accesses UART_TX register with port->lock acquired.
Now, if the same UART instance is being used by systemd/userspace,
characters are written to UART_TX register by serial8250_tx_chars(). The
concurrent access by serial8250_console_write() and
serial8250_tx_chars() is serialized by the use of port->lock spinlock
and hence there is no issue with` non DMA case.
But when using DMA with 8250 UART, I see that port->lock is held before
scheduling of DMA TX transfer and released as soon as the transfer is
submitted. The lock is not held until the transfer actually completes
See,
uart_start()
->serial8250_start_tx()->
__start_tx()
->up->dma->tx_dma(up)
Or
__dma_tx_complete() in 8250_dma.c that acquires and releases port->lock
once TX DMA transfer is submitted in serial8250_tx_dma()
So, when the port->lock is released, it is quite possible that DMA is
still transferring data to UART TX FIFO and UART FIFO might be almost full.
I see that when DMA is writing to UART TX FIFO,
serial8250_console_write() may also write kernel log messages to UART TX
FIFO(as port->lock is now free to be acquired), which is leading to
overflow and lose of data. serial8250_console_write() checks for
UART_LSR_THRE to check if Transmit hold register is empty but that may
not be enough as DMA might put data before CPU write.
It seems that both DMA and CPU might simultaneously put data to UART
FIFO and lead to potential loss of data.
Is the expectation that UART instance used to print kernel log messages
is not intended to use DMA? Or am I missing something?
Any help appreciated!
--
Regards
Vignesh
^ permalink raw reply
* [PATCH] imx-serial: Reduce RX DMA startup latency when opening for reading
From: Peter Senna Tschudin @ 2017-04-07 9:45 UTC (permalink / raw)
Cc: Peter Senna Tschudin, Greg Kroah-Hartman, Jiri Slaby,
open list:SERIAL DRIVERS, open list
Reduce RX DMA start latency for the first reception when port is opened
for reading. Instead of waiting for an interrupt signaling data on RX
FIFO or data too old on RX FIFO, start RX DMA immediately when the
serial port is opened for reading.
Before this patch, the average RX DMA latency for the first reception
was 42489 microseconds with a standard deviation of 25721 microseconds
in 36 samples.
After the patch the average RX DMA latency for the first reception, when
the serial port is opened for reading, is 653 microseconds with a
standard deviation of 294 microseconds in 36 samples.
Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
---
RX DMA latency for the first reception was calculated as the time
difference between the call to imx_startup() and the completion of
start_rx_dma(). I used this patch for the measurement:
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -238,6 +238,7 @@ struct imx_port {
wait_queue_head_t dma_wait;
unsigned int saved_reg[10];
bool context_saved;
+ struct timespec ts;
};
struct imx_port_ucrs {
@@ -1059,6 +1060,7 @@ static int start_rx_dma(struct imx_port *sport)
struct dma_chan *chan = sport->dma_chan_rx;
struct device *dev = sport->port.dev;
struct dma_async_tx_descriptor *desc;
+ struct timespec te;
int ret;
sport->rx_ring.head = 0;
@@ -1087,6 +1089,10 @@ static int start_rx_dma(struct imx_port *sport)
sport->dma_is_rxing = 1;
sport->rx_cookie = dmaengine_submit(desc);
dma_async_issue_pending(chan);
+
+ getnstimeofday(&te);
+ dev_err(sport->port.dev, "DMA delay(microseconds): %lu\n", (te.tv_nsec - sport->ts.tv_nsec)/1000);
+
return 0;
}
@@ -1286,6 +1292,8 @@ static int imx_startup(struct uart_port *port)
int retval, i, readcnt = 0;
unsigned long flags, temp;
+ getnstimeofday(&sport->ts);
+
retval = clk_prepare_enable(sport->clk_per);
if (retval)
return retval;
drivers/tty/serial/imx.c | 63 +++++++++++++++++++++++++++++++++++++-----------
1 file changed, 49 insertions(+), 14 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index e3e152c..b4340b5 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -719,6 +719,27 @@ static irqreturn_t imx_rxint(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static void imx_disable_rx_int(struct imx_port *sport)
+{
+ unsigned long temp;
+
+ sport->dma_is_rxing = 1;
+
+ /* disable the receiver ready and aging timer interrupts */
+ temp = readl(sport->port.membase + UCR1);
+ temp &= ~(UCR1_RRDYEN);
+ writel(temp, sport->port.membase + UCR1);
+
+ temp = readl(sport->port.membase + UCR2);
+ temp &= ~(UCR2_ATEN);
+ writel(temp, sport->port.membase + UCR2);
+
+ /* disable the rx errors interrupts */
+ temp = readl(sport->port.membase + UCR4);
+ temp &= ~UCR4_OREN;
+ writel(temp, sport->port.membase + UCR4);
+}
+
static void clear_rx_errors(struct imx_port *sport);
static int start_rx_dma(struct imx_port *sport);
/*
@@ -734,21 +755,8 @@ static void imx_dma_rxint(struct imx_port *sport)
temp = readl(sport->port.membase + USR2);
if ((temp & USR2_RDR) && !sport->dma_is_rxing) {
- sport->dma_is_rxing = 1;
- /* disable the receiver ready and aging timer interrupts */
- temp = readl(sport->port.membase + UCR1);
- temp &= ~(UCR1_RRDYEN);
- writel(temp, sport->port.membase + UCR1);
-
- temp = readl(sport->port.membase + UCR2);
- temp &= ~(UCR2_ATEN);
- writel(temp, sport->port.membase + UCR2);
-
- /* disable the rx errors interrupts */
- temp = readl(sport->port.membase + UCR4);
- temp &= ~UCR4_OREN;
- writel(temp, sport->port.membase + UCR4);
+ imx_disable_rx_int(sport);
/* tell the DMA to receive the data. */
start_rx_dma(sport);
@@ -1339,6 +1347,33 @@ static int imx_startup(struct uart_port *port)
* Enable modem status interrupts
*/
imx_enable_ms(&sport->port);
+
+ /*
+ * If the serial port is opened for reading start RX DMA immediately
+ * instead of waiting for RX FIFO interrupts. In our iMX53 the average
+ * delay for the first reception dropped from approximately 35000
+ * microseconds to 1000 microseconds.
+ */
+ if (sport->dma_is_enabled) {
+ struct tty_struct *tty = sport->port.state->port.tty;
+ struct tty_file_private *file_priv;
+ int readcnt = 0;
+
+ spin_lock(&tty->files_lock);
+
+ if (!list_empty(&tty->tty_files))
+ list_for_each_entry(file_priv, &tty->tty_files, list)
+ if (!(file_priv->file->f_flags & O_WRONLY))
+ readcnt++;
+
+ spin_unlock(&tty->files_lock);
+
+ if (readcnt > 0) {
+ imx_disable_rx_int(sport);
+ start_rx_dma(sport);
+ }
+ }
+
spin_unlock_irqrestore(&sport->port.lock, flags);
return 0;
--
2.9.3
^ permalink raw reply related
* Re: [PATCH] format-security: move static strings to const
From: Kees Cook @ 2017-04-07 3:21 UTC (permalink / raw)
To: Jani Nikula
Cc: Linux MIPS Mailing List, linux-decnet-user, Mugunthan V N,
Tony Lindgren, Viresh Kumar, Rasmus Villemoes,
Maling list - DRI developers, LKML, Eric Dumazet,
Maciej W. Rozycki, Christian Gromm, Daniel Vetter, Ingo Molnar,
devel, Kejian Yan, Felipe Balbi, Russell King, Jarod Wilson,
David Airlie, Daode Huang, linux-serial, Jiri Slaby,
Yisen Zhuang <yis>
In-Reply-To: <87mvbtzztb.fsf@intel.com>
On Thu, Apr 6, 2017 at 1:48 AM, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Thu, 06 Apr 2017, Kees Cook <keescook@chromium.org> wrote:
>> While examining output from trial builds with -Wformat-security enabled,
>> many strings were found that should be defined as "const", or as a char
>> array instead of char pointer. This makes some static analysis easier,
>> by producing fewer false positives.
>>
>> As these are all trivial changes, it seemed best to put them all in
>> a single patch rather than chopping them up per maintainer.
>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index f6d4d9700734..1ff9d5912b83 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -2331,7 +2331,7 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
>> int __init drm_fb_helper_modinit(void)
>> {
>> #if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT)
>> - const char *name = "fbcon";
>> + const char name[] = "fbcon";
>
> I'd always write the former out of habit. Why should I start using the
> latter? What makes it better?
For me, it's mainly two reasons: sizeof() and -Wformat-security behavior.
The compiler treats "sizeof" differently. E.g. "sizeof(var)" shows the
allocation size for the array, and pointer size for the char pointer.
When doing things like snprintf(buf, sizeof(buf), ...) will do the
right thing, etc. (This is a poor example for a _const_ string, but
the point is that some calculations still work better with the array
over the pointer.)
The other situation (which is why I noted this to change them) is that
gcc's handling of them is different when faced with -Wformat-security
since it doesn't like to believe that const char pointers are actually
const for the purposes of being a format string.
> What keeps the kernel from accumulating tons more of the former?
Right now, nothing. The good news is that they're relatively rare, and
I notice them when they're added (since I have a -Wformat-security
tree). We could add a warning to checkpatch for suggesting const char
var[] over const char *var, perhaps?
> Here's an interesting comparison of the generated code. I'm a bit
> surprised by what gcc does, I would have expected no difference, like
> clang. https://godbolt.org/g/OdqUvN
Here's your example with sizeof() added, if you're curious...
https://godbolt.org/g/U1zIZK
> The other changes adding const in this patch are, of course, good.
Thanks!
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [PATCH] format-security: move static strings to const
From: Jani Nikula @ 2017-04-06 8:48 UTC (permalink / raw)
To: Kees Cook, Andrew Morton
Cc: linux-mips, linux-decnet-user, kernel, Qianqian Xie,
Mugunthan V N, Tony Lindgren, Viresh Kumar, Rasmus Villemoes,
dri-devel, linux-kernel, Eric Dumazet, Maciej W. Rozycki,
Christian Gromm, Daniel Vetter, Ingo Molnar, Andrey Shvetsov,
devel, Kejian Yan, Felipe Balbi, Russell King, Jarod Wilson,
linux-hippi, Daode Huang, linux-serial, Jiri Slaby
In-Reply-To: <20170405214711.GA5711@beast>
On Thu, 06 Apr 2017, Kees Cook <keescook@chromium.org> wrote:
> While examining output from trial builds with -Wformat-security enabled,
> many strings were found that should be defined as "const", or as a char
> array instead of char pointer. This makes some static analysis easier,
> by producing fewer false positives.
>
> As these are all trivial changes, it seemed best to put them all in
> a single patch rather than chopping them up per maintainer.
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index f6d4d9700734..1ff9d5912b83 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2331,7 +2331,7 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
> int __init drm_fb_helper_modinit(void)
> {
> #if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT)
> - const char *name = "fbcon";
> + const char name[] = "fbcon";
I'd always write the former out of habit. Why should I start using the
latter? What makes it better?
What keeps the kernel from accumulating tons more of the former?
Here's an interesting comparison of the generated code. I'm a bit
surprised by what gcc does, I would have expected no difference, like
clang. https://godbolt.org/g/OdqUvN
The other changes adding const in this patch are, of course, good.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH] format-security: move static strings to const
From: Daniel Vetter @ 2017-04-06 8:27 UTC (permalink / raw)
To: Kees Cook
Cc: linux-mips, Dmitry Torokhov, kernel, Qianqian Xie, Jes Sorensen,
Tony Lindgren, Viresh Kumar, Rasmus Villemoes, dri-devel,
Patrice Chotard, Eric Dumazet, Maciej W. Rozycki, Christian Gromm,
Daniel Vetter, Ingo Molnar, Andrey Shvetsov, devel, Kejian Yan,
Arnd Bergmann, Russell King, Jarod Wilson, linux-hippi,
Daode Huang, linux-serial, Jiri Slaby <jsla>
In-Reply-To: <20170405214711.GA5711@beast>
On Wed, Apr 05, 2017 at 02:47:11PM -0700, Kees Cook wrote:
> While examining output from trial builds with -Wformat-security enabled,
> many strings were found that should be defined as "const", or as a char
> array instead of char pointer. This makes some static analysis easier,
> by producing fewer false positives.
>
> As these are all trivial changes, it seemed best to put them all in
> a single patch rather than chopping them up per maintainer.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Ack on the drm part.
-Daniel
> ---
> arch/arm/mach-omap2/board-n8x0.c | 2 +-
> arch/mips/dec/prom/init.c | 6 +++---
> arch/mips/kernel/traps.c | 4 ++--
> drivers/char/dsp56k.c | 2 +-
> drivers/cpufreq/powernow-k8.c | 3 ++-
> drivers/gpu/drm/drm_fb_helper.c | 2 +-
> drivers/net/ethernet/amd/atarilance.c | 4 ++--
> drivers/net/ethernet/amd/declance.c | 2 +-
> drivers/net/ethernet/amd/sun3lance.c | 3 ++-
> drivers/net/ethernet/cirrus/mac89x0.c | 2 +-
> drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h | 2 +-
> drivers/net/ethernet/natsemi/sonic.h | 2 +-
> drivers/net/ethernet/toshiba/tc35815.c | 2 +-
> drivers/net/fddi/defxx.c | 2 +-
> drivers/net/hippi/rrunner.c | 3 ++-
> drivers/staging/most/mostcore/core.c | 2 +-
> drivers/tty/n_hdlc.c | 10 +++++-----
> drivers/tty/serial/st-asc.c | 2 +-
> net/decnet/af_decnet.c | 3 ++-
> 19 files changed, 31 insertions(+), 27 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-n8x0.c b/arch/arm/mach-omap2/board-n8x0.c
> index 6b6fda65fb3b..91272db09fa3 100644
> --- a/arch/arm/mach-omap2/board-n8x0.c
> +++ b/arch/arm/mach-omap2/board-n8x0.c
> @@ -117,7 +117,7 @@ static struct musb_hdrc_platform_data tusb_data = {
> static void __init n8x0_usb_init(void)
> {
> int ret = 0;
> - static char announce[] __initdata = KERN_INFO "TUSB 6010\n";
> + static const char announce[] __initconst = KERN_INFO "TUSB 6010\n";
>
> /* PM companion chip power control pin */
> ret = gpio_request_one(TUSB6010_GPIO_ENABLE, GPIOF_OUT_INIT_LOW,
> diff --git a/arch/mips/dec/prom/init.c b/arch/mips/dec/prom/init.c
> index 4e1761e0a09a..d88eb7a6662b 100644
> --- a/arch/mips/dec/prom/init.c
> +++ b/arch/mips/dec/prom/init.c
> @@ -88,7 +88,7 @@ void __init which_prom(s32 magic, s32 *prom_vec)
> void __init prom_init(void)
> {
> extern void dec_machine_halt(void);
> - static char cpu_msg[] __initdata =
> + static const char cpu_msg[] __initconst =
> "Sorry, this kernel is compiled for a wrong CPU type!\n";
> s32 argc = fw_arg0;
> s32 *argv = (void *)fw_arg1;
> @@ -111,7 +111,7 @@ void __init prom_init(void)
> #if defined(CONFIG_CPU_R3000)
> if ((current_cpu_type() == CPU_R4000SC) ||
> (current_cpu_type() == CPU_R4400SC)) {
> - static char r4k_msg[] __initdata =
> + static const char r4k_msg[] __initconst =
> "Please recompile with \"CONFIG_CPU_R4x00 = y\".\n";
> printk(cpu_msg);
> printk(r4k_msg);
> @@ -122,7 +122,7 @@ void __init prom_init(void)
> #if defined(CONFIG_CPU_R4X00)
> if ((current_cpu_type() == CPU_R3000) ||
> (current_cpu_type() == CPU_R3000A)) {
> - static char r3k_msg[] __initdata =
> + static const char r3k_msg[] __initconst =
> "Please recompile with \"CONFIG_CPU_R3000 = y\".\n";
> printk(cpu_msg);
> printk(r3k_msg);
> diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
> index c7d17cfb32f6..2c717db50380 100644
> --- a/arch/mips/kernel/traps.c
> +++ b/arch/mips/kernel/traps.c
> @@ -2256,8 +2256,8 @@ void set_handler(unsigned long offset, void *addr, unsigned long size)
> local_flush_icache_range(ebase + offset, ebase + offset + size);
> }
>
> -static char panic_null_cerr[] =
> - "Trying to set NULL cache error exception handler";
> +static const char panic_null_cerr[] =
> + "Trying to set NULL cache error exception handler\n";
>
> /*
> * Install uncached CPU exception handler.
> diff --git a/drivers/char/dsp56k.c b/drivers/char/dsp56k.c
> index 50aa9ba91f25..0d7b577e0ff0 100644
> --- a/drivers/char/dsp56k.c
> +++ b/drivers/char/dsp56k.c
> @@ -489,7 +489,7 @@ static const struct file_operations dsp56k_fops = {
>
> /****** Init and module functions ******/
>
> -static char banner[] __initdata = KERN_INFO "DSP56k driver installed\n";
> +static const char banner[] __initconst = KERN_INFO "DSP56k driver installed\n";
>
> static int __init dsp56k_init_driver(void)
> {
> diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
> index 0b5bf135b090..062d71434e47 100644
> --- a/drivers/cpufreq/powernow-k8.c
> +++ b/drivers/cpufreq/powernow-k8.c
> @@ -1171,7 +1171,8 @@ static struct cpufreq_driver cpufreq_amd64_driver = {
>
> static void __request_acpi_cpufreq(void)
> {
> - const char *cur_drv, *drv = "acpi-cpufreq";
> + const char drv[] = "acpi-cpufreq";
> + const char *cur_drv;
>
> cur_drv = cpufreq_get_current_driver();
> if (!cur_drv)
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index f6d4d9700734..1ff9d5912b83 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2331,7 +2331,7 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
> int __init drm_fb_helper_modinit(void)
> {
> #if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT)
> - const char *name = "fbcon";
> + const char name[] = "fbcon";
> struct module *fbcon;
>
> mutex_lock(&module_mutex);
> diff --git a/drivers/net/ethernet/amd/atarilance.c b/drivers/net/ethernet/amd/atarilance.c
> index 796c37a5bbde..c5b81268c284 100644
> --- a/drivers/net/ethernet/amd/atarilance.c
> +++ b/drivers/net/ethernet/amd/atarilance.c
> @@ -42,8 +42,8 @@
>
> */
>
> -static char version[] = "atarilance.c: v1.3 04/04/96 "
> - "Roman.Hodek@informatik.uni-erlangen.de\n";
> +static const char version[] = "atarilance.c: v1.3 04/04/96 "
> + "Roman.Hodek@informatik.uni-erlangen.de\n";
>
> #include <linux/netdevice.h>
> #include <linux/etherdevice.h>
> diff --git a/drivers/net/ethernet/amd/declance.c b/drivers/net/ethernet/amd/declance.c
> index 6c98901f1b89..82cc81385033 100644
> --- a/drivers/net/ethernet/amd/declance.c
> +++ b/drivers/net/ethernet/amd/declance.c
> @@ -72,7 +72,7 @@
> #include <asm/dec/machtype.h>
> #include <asm/dec/system.h>
>
> -static char version[] =
> +static const char version[] =
> "declance.c: v0.011 by Linux MIPS DECstation task force\n";
>
> MODULE_AUTHOR("Linux MIPS DECstation task force");
> diff --git a/drivers/net/ethernet/amd/sun3lance.c b/drivers/net/ethernet/amd/sun3lance.c
> index 12bb4f1489fc..77b1db267730 100644
> --- a/drivers/net/ethernet/amd/sun3lance.c
> +++ b/drivers/net/ethernet/amd/sun3lance.c
> @@ -21,7 +21,8 @@
>
> */
>
> -static char *version = "sun3lance.c: v1.2 1/12/2001 Sam Creasey (sammy@sammy.net)\n";
> +static const char version[] =
> +"sun3lance.c: v1.2 1/12/2001 Sam Creasey (sammy@sammy.net)\n";
>
> #include <linux/module.h>
> #include <linux/stddef.h>
> diff --git a/drivers/net/ethernet/cirrus/mac89x0.c b/drivers/net/ethernet/cirrus/mac89x0.c
> index b600fbbbf679..f910f0f386d6 100644
> --- a/drivers/net/ethernet/cirrus/mac89x0.c
> +++ b/drivers/net/ethernet/cirrus/mac89x0.c
> @@ -56,7 +56,7 @@
> local_irq_{dis,en}able()
> */
>
> -static char *version =
> +static const char version[] =
> "cs89x0.c:v1.02 11/26/96 Russell Nelson <nelson@crynwr.com>\n";
>
> /* ======================= configure the driver here ======================= */
> diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h
> index 2bb3d1e93c64..1951b65c7a57 100644
> --- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h
> +++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h
> @@ -407,7 +407,7 @@ struct mac_driver {
> };
>
> struct mac_stats_string {
> - char desc[ETH_GSTRING_LEN];
> + const char desc[ETH_GSTRING_LEN];
> unsigned long offset;
> };
>
> diff --git a/drivers/net/ethernet/natsemi/sonic.h b/drivers/net/ethernet/natsemi/sonic.h
> index 07091dd27e5d..7b0a8db57af9 100644
> --- a/drivers/net/ethernet/natsemi/sonic.h
> +++ b/drivers/net/ethernet/natsemi/sonic.h
> @@ -444,7 +444,7 @@ static inline __u16 sonic_rra_get(struct net_device* dev, int entry,
> (entry * SIZEOF_SONIC_RR) + offset);
> }
>
> -static const char *version =
> +static const char version[] =
> "sonic.c:v0.92 20.9.98 tsbogend@alpha.franken.de\n";
>
> #endif /* SONIC_H */
> diff --git a/drivers/net/ethernet/toshiba/tc35815.c b/drivers/net/ethernet/toshiba/tc35815.c
> index a45f98fa4aa7..ad6db05b97d6 100644
> --- a/drivers/net/ethernet/toshiba/tc35815.c
> +++ b/drivers/net/ethernet/toshiba/tc35815.c
> @@ -23,7 +23,7 @@
> */
>
> #define DRV_VERSION "1.39"
> -static const char *version = "tc35815.c:v" DRV_VERSION "\n";
> +static const char version[] = "tc35815.c:v" DRV_VERSION "\n";
> #define MODNAME "tc35815"
>
> #include <linux/module.h>
> diff --git a/drivers/net/fddi/defxx.c b/drivers/net/fddi/defxx.c
> index b0de8ecd7fe8..f4a816cf012a 100644
> --- a/drivers/net/fddi/defxx.c
> +++ b/drivers/net/fddi/defxx.c
> @@ -228,7 +228,7 @@
> #define DRV_VERSION "v1.11"
> #define DRV_RELDATE "2014/07/01"
>
> -static char version[] =
> +static const char version[] =
> DRV_NAME ": " DRV_VERSION " " DRV_RELDATE
> " Lawrence V. Stefani and others\n";
>
> diff --git a/drivers/net/hippi/rrunner.c b/drivers/net/hippi/rrunner.c
> index dd7fc6659ad4..9b0d6148e994 100644
> --- a/drivers/net/hippi/rrunner.c
> +++ b/drivers/net/hippi/rrunner.c
> @@ -60,7 +60,8 @@ MODULE_AUTHOR("Jes Sorensen <jes@wildopensource.com>");
> MODULE_DESCRIPTION("Essential RoadRunner HIPPI driver");
> MODULE_LICENSE("GPL");
>
> -static char version[] = "rrunner.c: v0.50 11/11/2002 Jes Sorensen (jes@wildopensource.com)\n";
> +static const char version[] =
> +"rrunner.c: v0.50 11/11/2002 Jes Sorensen (jes@wildopensource.com)\n";
>
>
> static const struct net_device_ops rr_netdev_ops = {
> diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
> index 191404bc5906..892aae6e9c9a 100644
> --- a/drivers/staging/most/mostcore/core.c
> +++ b/drivers/staging/most/mostcore/core.c
> @@ -82,7 +82,7 @@ struct most_inst_obj {
>
> static const struct {
> int most_ch_data_type;
> - char *name;
> + const char *name;
> } ch_data_type[] = {
> { MOST_CH_CONTROL, "control\n" },
> { MOST_CH_ASYNC, "async\n" },
> diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
> index e94aea8c0d05..7b2a466616d6 100644
> --- a/drivers/tty/n_hdlc.c
> +++ b/drivers/tty/n_hdlc.c
> @@ -939,11 +939,11 @@ static struct n_hdlc_buf *n_hdlc_buf_get(struct n_hdlc_buf_list *buf_list)
> return buf;
> } /* end of n_hdlc_buf_get() */
>
> -static char hdlc_banner[] __initdata =
> +static const char hdlc_banner[] __initconst =
> KERN_INFO "HDLC line discipline maxframe=%u\n";
> -static char hdlc_register_ok[] __initdata =
> +static const char hdlc_register_ok[] __initconst =
> KERN_INFO "N_HDLC line discipline registered.\n";
> -static char hdlc_register_fail[] __initdata =
> +static const char hdlc_register_fail[] __initconst =
> KERN_ERR "error registering line discipline: %d\n";
>
> static int __init n_hdlc_init(void)
> @@ -968,9 +968,9 @@ static int __init n_hdlc_init(void)
>
> } /* end of init_module() */
>
> -static char hdlc_unregister_ok[] __exitdata =
> +static const char hdlc_unregister_ok[] __exitdata =
> KERN_INFO "N_HDLC: line discipline unregistered\n";
> -static char hdlc_unregister_fail[] __exitdata =
> +static const char hdlc_unregister_fail[] __exitdata =
> KERN_ERR "N_HDLC: can't unregister line discipline (err = %d)\n";
>
> static void __exit n_hdlc_exit(void)
> diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
> index bcf1d33e6ffe..ef9f47847f59 100644
> --- a/drivers/tty/serial/st-asc.c
> +++ b/drivers/tty/serial/st-asc.c
> @@ -985,7 +985,7 @@ static struct platform_driver asc_serial_driver = {
> static int __init asc_init(void)
> {
> int ret;
> - static char banner[] __initdata =
> + static const char banner[] __initconst =
> KERN_INFO "STMicroelectronics ASC driver initialized\n";
>
> printk(banner);
> diff --git a/net/decnet/af_decnet.c b/net/decnet/af_decnet.c
> index e6e79eda9763..c6ed5e9502e9 100644
> --- a/net/decnet/af_decnet.c
> +++ b/net/decnet/af_decnet.c
> @@ -2359,7 +2359,8 @@ MODULE_AUTHOR("Linux DECnet Project Team");
> MODULE_LICENSE("GPL");
> MODULE_ALIAS_NETPROTO(PF_DECnet);
>
> -static char banner[] __initdata = KERN_INFO "NET4: DECnet for Linux: V.2.5.68s (C) 1995-2003 Linux DECnet Project Team\n";
> +static const char banner[] __initconst = KERN_INFO
> +"NET4: DECnet for Linux: V.2.5.68s (C) 1995-2003 Linux DECnet Project Team\n";
>
> static int __init decnet_init(void)
> {
> --
> 2.7.4
>
>
> --
> Kees Cook
> Pixel Security
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v9 3/3] printk: fix double printing with earlycon
From: Aleksey Makarov @ 2017-04-06 4:44 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
Sudeep Holla, Greg Kroah-Hartman, Peter Hurley, Jiri Slaby,
Robin Murphy, Steven Rostedt, Nair, Jayachandran,
Sergey Senozhatsky, Petr Mladek
In-Reply-To: <CAHp75Ve1n7iK0zrUOdjkKxrj-ggbJXpau=CG8dam+1DkDL_fyw@mail.gmail.com>
On 04/06/2017 12:57 AM, Andy Shevchenko wrote:
> On Wed, Apr 5, 2017 at 11:20 PM, Aleksey Makarov
> <aleksey.makarov@linaro.org> wrote:
>> If a console was specified by ACPI SPCR table _and_ command line
>> parameters like "console=ttyAMA0" _and_ "earlycon" were specified,
>> then log messages appear twice.
>>
>> The root cause is that the code traverses the list of specified
>> consoles (the `console_cmdline` array) and stops at the first match.
>> But it may happen that the same console is referred by the elements
>> of this array twice:
>>
>> pl011,mmio,0x87e024000000,115200 -- from SPCR
>> ttyAMA0 -- from command line
>>
>> but in this case `preferred_console` points to the second entry and
>> the flag CON_CONSDEV is not set, so bootconsole is not deregistered.
>>
>> To fix that, introduce an invariant "The last non-braille console
>> is always the preferred one" on the entries of the console_cmdline
>> array. Then traverse it in reverse order to be sure that if
>> the console is preferred then it will be the first matching entry.
>> Introduce variable console_cmdline_cnt that keeps the number
>> of elements of the console_cmdline array (Petr Mladek). It helps
>> to get rid of the loop that searches for the end of this array.
>
>> #define MAX_CMDLINECONSOLES 8
>>
>> static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
>> +static int console_cmdline_cnt;
>
> This should be equal to -1 at the beginning, am I right?
No, this is not an index of the last element, this is count of
elements of cmdline_console array. So it is 0 initially.
>> static int preferred_console = -1;
>> int console_set_on_cmdline;
>> @@ -1905,12 +1906,26 @@ static int __add_preferred_console(char *name, int idx, char *options,
>> * See if this tty is not yet registered, and
>> * if we have a slot free.
>> */
>> - for (i = 0, c = console_cmdline;
>> - i < MAX_CMDLINECONSOLES && c->name[0];
>> - i++, c++) {
>> + for (i = 0, c = console_cmdline; i < console_cmdline_cnt; i++, c++) {
>> if (strcmp(c->name, name) == 0 && c->index == idx) {
>> - if (!brl_options)
>> - preferred_console = i;
>> +
>
>> + if (brl_options)
>> + return 0;
>
> Is it invariant or brl_options may appear while looping?
I am not sure I understand your question.
If we find that we are registering a braille console that
has already been registered, we just return without updating
preferred console (it is only about regular consoles) and
without swapping it with the last element of the array (because it
is explicitly mentioned in the invariant: The last
*non-braille* console is always the preferred one)
>> +
>> + /*
>> + * Maintain an invariant that will help to find if
>> + * the matching console is preferred, see
>> + * register_console():
>> + *
>> + * The last non-braille console is always
>> + * the preferred one.
>> + */
>> + if (i != console_cmdline_cnt - 1)
>> + swap(console_cmdline[i],
>> + console_cmdline[console_cmdline_cnt - 1]);
>
> i'm wondering if you can iterate from the end to beginning as you do below.
> It would simplify things.
You mean iterate to find the last element?
Yes I can and it is how this was implemented in v8,
Petr Mladek asked to introduce console_cmdline_cnt.
Thank you for review
Aleksey Makarov
>> +
>> + preferred_console = console_cmdline_cnt - 1;
>> +
>> return 0;
>> }
>> }
>> @@ -1923,6 +1938,7 @@ static int __add_preferred_console(char *name, int idx, char *options,
>> braille_set_options(c, brl_options);
>>
>> c->index = idx;
>> + console_cmdline_cnt++;
>> return 0;
>> }
>> /*
>> @@ -2457,12 +2473,24 @@ void register_console(struct console *newcon)
>> }
>>
>> /*
>> - * See if this console matches one we selected on
>> - * the command line.
>> + * See if this console matches one we selected on the command line.
>> + *
>> + * There may be several entries in the console_cmdline array matching
>> + * with the same console, one with newcon->match(), another by
>> + * name/index:
>> + *
>> + * pl011,mmio,0x87e024000000,115200 -- added from SPCR
>> + * ttyAMA0 -- added from command line
>> + *
>> + * Traverse the console_cmdline array in reverse order to be
>> + * sure that if this console is preferred then it will be the first
>> + * matching entry. We use the invariant that is maintained in
>> + * __add_preferred_console().
>> */
>> - for (i = 0, c = console_cmdline;
>> - i < MAX_CMDLINECONSOLES && c->name[0];
>> - i++, c++) {
>
>> + for (i = console_cmdline_cnt - 1; i >= 0; i--) {
>
>
>
>> +
>> + c = console_cmdline + i;
>> +
>> if (!newcon->match ||
>> newcon->match(newcon, c->name, c->index, c->options) != 0) {
>> /* default matching */
>> --
>> 2.12.1
>>
>
>
>
^ permalink raw reply
* Re: [PATCH v9 3/3] printk: fix double printing with earlycon
From: Andy Shevchenko @ 2017-04-05 21:57 UTC (permalink / raw)
To: Aleksey Makarov
Cc: linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
Sudeep Holla, Greg Kroah-Hartman, Peter Hurley, Jiri Slaby,
Robin Murphy, Steven Rostedt, Nair, Jayachandran,
Sergey Senozhatsky, Petr Mladek
In-Reply-To: <20170405202006.18234-1-aleksey.makarov@linaro.org>
On Wed, Apr 5, 2017 at 11:20 PM, Aleksey Makarov
<aleksey.makarov@linaro.org> wrote:
> If a console was specified by ACPI SPCR table _and_ command line
> parameters like "console=ttyAMA0" _and_ "earlycon" were specified,
> then log messages appear twice.
>
> The root cause is that the code traverses the list of specified
> consoles (the `console_cmdline` array) and stops at the first match.
> But it may happen that the same console is referred by the elements
> of this array twice:
>
> pl011,mmio,0x87e024000000,115200 -- from SPCR
> ttyAMA0 -- from command line
>
> but in this case `preferred_console` points to the second entry and
> the flag CON_CONSDEV is not set, so bootconsole is not deregistered.
>
> To fix that, introduce an invariant "The last non-braille console
> is always the preferred one" on the entries of the console_cmdline
> array. Then traverse it in reverse order to be sure that if
> the console is preferred then it will be the first matching entry.
> Introduce variable console_cmdline_cnt that keeps the number
> of elements of the console_cmdline array (Petr Mladek). It helps
> to get rid of the loop that searches for the end of this array.
> #define MAX_CMDLINECONSOLES 8
>
> static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
> +static int console_cmdline_cnt;
This should be equal to -1 at the beginning, am I right?
> static int preferred_console = -1;
> int console_set_on_cmdline;
> @@ -1905,12 +1906,26 @@ static int __add_preferred_console(char *name, int idx, char *options,
> * See if this tty is not yet registered, and
> * if we have a slot free.
> */
> - for (i = 0, c = console_cmdline;
> - i < MAX_CMDLINECONSOLES && c->name[0];
> - i++, c++) {
> + for (i = 0, c = console_cmdline; i < console_cmdline_cnt; i++, c++) {
> if (strcmp(c->name, name) == 0 && c->index == idx) {
> - if (!brl_options)
> - preferred_console = i;
> +
> + if (brl_options)
> + return 0;
Is it invariant or brl_options may appear while looping?
> +
> + /*
> + * Maintain an invariant that will help to find if
> + * the matching console is preferred, see
> + * register_console():
> + *
> + * The last non-braille console is always
> + * the preferred one.
> + */
> + if (i != console_cmdline_cnt - 1)
> + swap(console_cmdline[i],
> + console_cmdline[console_cmdline_cnt - 1]);
i'm wondering if you can iterate from the end to beginning as you do below.
It would simplify things.
> +
> + preferred_console = console_cmdline_cnt - 1;
> +
> return 0;
> }
> }
> @@ -1923,6 +1938,7 @@ static int __add_preferred_console(char *name, int idx, char *options,
> braille_set_options(c, brl_options);
>
> c->index = idx;
> + console_cmdline_cnt++;
> return 0;
> }
> /*
> @@ -2457,12 +2473,24 @@ void register_console(struct console *newcon)
> }
>
> /*
> - * See if this console matches one we selected on
> - * the command line.
> + * See if this console matches one we selected on the command line.
> + *
> + * There may be several entries in the console_cmdline array matching
> + * with the same console, one with newcon->match(), another by
> + * name/index:
> + *
> + * pl011,mmio,0x87e024000000,115200 -- added from SPCR
> + * ttyAMA0 -- added from command line
> + *
> + * Traverse the console_cmdline array in reverse order to be
> + * sure that if this console is preferred then it will be the first
> + * matching entry. We use the invariant that is maintained in
> + * __add_preferred_console().
> */
> - for (i = 0, c = console_cmdline;
> - i < MAX_CMDLINECONSOLES && c->name[0];
> - i++, c++) {
> + for (i = console_cmdline_cnt - 1; i >= 0; i--) {
> +
> + c = console_cmdline + i;
> +
> if (!newcon->match ||
> newcon->match(newcon, c->name, c->index, c->options) != 0) {
> /* default matching */
> --
> 2.12.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH] format-security: move static strings to const
From: Kees Cook @ 2017-04-05 21:47 UTC (permalink / raw)
To: Andrew Morton
Cc: Tony Lindgren, Russell King, Maciej W. Rozycki, Ralf Baechle,
Arnd Bergmann, Greg Kroah-Hartman, Rafael J. Wysocki,
Viresh Kumar, Daniel Vetter, Jani Nikula, Sean Paul, David Airlie,
Yisen Zhuang, Salil Mehta, Thomas Bogendoerfer, Jes Sorensen,
Jiri Slaby, Patrice Chotard, David S. Miller
While examining output from trial builds with -Wformat-security enabled,
many strings were found that should be defined as "const", or as a char
array instead of char pointer. This makes some static analysis easier,
by producing fewer false positives.
As these are all trivial changes, it seemed best to put them all in
a single patch rather than chopping them up per maintainer.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
arch/arm/mach-omap2/board-n8x0.c | 2 +-
arch/mips/dec/prom/init.c | 6 +++---
arch/mips/kernel/traps.c | 4 ++--
drivers/char/dsp56k.c | 2 +-
drivers/cpufreq/powernow-k8.c | 3 ++-
drivers/gpu/drm/drm_fb_helper.c | 2 +-
drivers/net/ethernet/amd/atarilance.c | 4 ++--
drivers/net/ethernet/amd/declance.c | 2 +-
drivers/net/ethernet/amd/sun3lance.c | 3 ++-
drivers/net/ethernet/cirrus/mac89x0.c | 2 +-
drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h | 2 +-
drivers/net/ethernet/natsemi/sonic.h | 2 +-
drivers/net/ethernet/toshiba/tc35815.c | 2 +-
drivers/net/fddi/defxx.c | 2 +-
drivers/net/hippi/rrunner.c | 3 ++-
drivers/staging/most/mostcore/core.c | 2 +-
drivers/tty/n_hdlc.c | 10 +++++-----
drivers/tty/serial/st-asc.c | 2 +-
net/decnet/af_decnet.c | 3 ++-
19 files changed, 31 insertions(+), 27 deletions(-)
diff --git a/arch/arm/mach-omap2/board-n8x0.c b/arch/arm/mach-omap2/board-n8x0.c
index 6b6fda65fb3b..91272db09fa3 100644
--- a/arch/arm/mach-omap2/board-n8x0.c
+++ b/arch/arm/mach-omap2/board-n8x0.c
@@ -117,7 +117,7 @@ static struct musb_hdrc_platform_data tusb_data = {
static void __init n8x0_usb_init(void)
{
int ret = 0;
- static char announce[] __initdata = KERN_INFO "TUSB 6010\n";
+ static const char announce[] __initconst = KERN_INFO "TUSB 6010\n";
/* PM companion chip power control pin */
ret = gpio_request_one(TUSB6010_GPIO_ENABLE, GPIOF_OUT_INIT_LOW,
diff --git a/arch/mips/dec/prom/init.c b/arch/mips/dec/prom/init.c
index 4e1761e0a09a..d88eb7a6662b 100644
--- a/arch/mips/dec/prom/init.c
+++ b/arch/mips/dec/prom/init.c
@@ -88,7 +88,7 @@ void __init which_prom(s32 magic, s32 *prom_vec)
void __init prom_init(void)
{
extern void dec_machine_halt(void);
- static char cpu_msg[] __initdata =
+ static const char cpu_msg[] __initconst =
"Sorry, this kernel is compiled for a wrong CPU type!\n";
s32 argc = fw_arg0;
s32 *argv = (void *)fw_arg1;
@@ -111,7 +111,7 @@ void __init prom_init(void)
#if defined(CONFIG_CPU_R3000)
if ((current_cpu_type() == CPU_R4000SC) ||
(current_cpu_type() == CPU_R4400SC)) {
- static char r4k_msg[] __initdata =
+ static const char r4k_msg[] __initconst =
"Please recompile with \"CONFIG_CPU_R4x00 = y\".\n";
printk(cpu_msg);
printk(r4k_msg);
@@ -122,7 +122,7 @@ void __init prom_init(void)
#if defined(CONFIG_CPU_R4X00)
if ((current_cpu_type() == CPU_R3000) ||
(current_cpu_type() == CPU_R3000A)) {
- static char r3k_msg[] __initdata =
+ static const char r3k_msg[] __initconst =
"Please recompile with \"CONFIG_CPU_R3000 = y\".\n";
printk(cpu_msg);
printk(r3k_msg);
diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index c7d17cfb32f6..2c717db50380 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -2256,8 +2256,8 @@ void set_handler(unsigned long offset, void *addr, unsigned long size)
local_flush_icache_range(ebase + offset, ebase + offset + size);
}
-static char panic_null_cerr[] =
- "Trying to set NULL cache error exception handler";
+static const char panic_null_cerr[] =
+ "Trying to set NULL cache error exception handler\n";
/*
* Install uncached CPU exception handler.
diff --git a/drivers/char/dsp56k.c b/drivers/char/dsp56k.c
index 50aa9ba91f25..0d7b577e0ff0 100644
--- a/drivers/char/dsp56k.c
+++ b/drivers/char/dsp56k.c
@@ -489,7 +489,7 @@ static const struct file_operations dsp56k_fops = {
/****** Init and module functions ******/
-static char banner[] __initdata = KERN_INFO "DSP56k driver installed\n";
+static const char banner[] __initconst = KERN_INFO "DSP56k driver installed\n";
static int __init dsp56k_init_driver(void)
{
diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
index 0b5bf135b090..062d71434e47 100644
--- a/drivers/cpufreq/powernow-k8.c
+++ b/drivers/cpufreq/powernow-k8.c
@@ -1171,7 +1171,8 @@ static struct cpufreq_driver cpufreq_amd64_driver = {
static void __request_acpi_cpufreq(void)
{
- const char *cur_drv, *drv = "acpi-cpufreq";
+ const char drv[] = "acpi-cpufreq";
+ const char *cur_drv;
cur_drv = cpufreq_get_current_driver();
if (!cur_drv)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index f6d4d9700734..1ff9d5912b83 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2331,7 +2331,7 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
int __init drm_fb_helper_modinit(void)
{
#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT)
- const char *name = "fbcon";
+ const char name[] = "fbcon";
struct module *fbcon;
mutex_lock(&module_mutex);
diff --git a/drivers/net/ethernet/amd/atarilance.c b/drivers/net/ethernet/amd/atarilance.c
index 796c37a5bbde..c5b81268c284 100644
--- a/drivers/net/ethernet/amd/atarilance.c
+++ b/drivers/net/ethernet/amd/atarilance.c
@@ -42,8 +42,8 @@
*/
-static char version[] = "atarilance.c: v1.3 04/04/96 "
- "Roman.Hodek@informatik.uni-erlangen.de\n";
+static const char version[] = "atarilance.c: v1.3 04/04/96 "
+ "Roman.Hodek@informatik.uni-erlangen.de\n";
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
diff --git a/drivers/net/ethernet/amd/declance.c b/drivers/net/ethernet/amd/declance.c
index 6c98901f1b89..82cc81385033 100644
--- a/drivers/net/ethernet/amd/declance.c
+++ b/drivers/net/ethernet/amd/declance.c
@@ -72,7 +72,7 @@
#include <asm/dec/machtype.h>
#include <asm/dec/system.h>
-static char version[] =
+static const char version[] =
"declance.c: v0.011 by Linux MIPS DECstation task force\n";
MODULE_AUTHOR("Linux MIPS DECstation task force");
diff --git a/drivers/net/ethernet/amd/sun3lance.c b/drivers/net/ethernet/amd/sun3lance.c
index 12bb4f1489fc..77b1db267730 100644
--- a/drivers/net/ethernet/amd/sun3lance.c
+++ b/drivers/net/ethernet/amd/sun3lance.c
@@ -21,7 +21,8 @@
*/
-static char *version = "sun3lance.c: v1.2 1/12/2001 Sam Creasey (sammy@sammy.net)\n";
+static const char version[] =
+"sun3lance.c: v1.2 1/12/2001 Sam Creasey (sammy@sammy.net)\n";
#include <linux/module.h>
#include <linux/stddef.h>
diff --git a/drivers/net/ethernet/cirrus/mac89x0.c b/drivers/net/ethernet/cirrus/mac89x0.c
index b600fbbbf679..f910f0f386d6 100644
--- a/drivers/net/ethernet/cirrus/mac89x0.c
+++ b/drivers/net/ethernet/cirrus/mac89x0.c
@@ -56,7 +56,7 @@
local_irq_{dis,en}able()
*/
-static char *version =
+static const char version[] =
"cs89x0.c:v1.02 11/26/96 Russell Nelson <nelson@crynwr.com>\n";
/* ======================= configure the driver here ======================= */
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h
index 2bb3d1e93c64..1951b65c7a57 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h
@@ -407,7 +407,7 @@ struct mac_driver {
};
struct mac_stats_string {
- char desc[ETH_GSTRING_LEN];
+ const char desc[ETH_GSTRING_LEN];
unsigned long offset;
};
diff --git a/drivers/net/ethernet/natsemi/sonic.h b/drivers/net/ethernet/natsemi/sonic.h
index 07091dd27e5d..7b0a8db57af9 100644
--- a/drivers/net/ethernet/natsemi/sonic.h
+++ b/drivers/net/ethernet/natsemi/sonic.h
@@ -444,7 +444,7 @@ static inline __u16 sonic_rra_get(struct net_device* dev, int entry,
(entry * SIZEOF_SONIC_RR) + offset);
}
-static const char *version =
+static const char version[] =
"sonic.c:v0.92 20.9.98 tsbogend@alpha.franken.de\n";
#endif /* SONIC_H */
diff --git a/drivers/net/ethernet/toshiba/tc35815.c b/drivers/net/ethernet/toshiba/tc35815.c
index a45f98fa4aa7..ad6db05b97d6 100644
--- a/drivers/net/ethernet/toshiba/tc35815.c
+++ b/drivers/net/ethernet/toshiba/tc35815.c
@@ -23,7 +23,7 @@
*/
#define DRV_VERSION "1.39"
-static const char *version = "tc35815.c:v" DRV_VERSION "\n";
+static const char version[] = "tc35815.c:v" DRV_VERSION "\n";
#define MODNAME "tc35815"
#include <linux/module.h>
diff --git a/drivers/net/fddi/defxx.c b/drivers/net/fddi/defxx.c
index b0de8ecd7fe8..f4a816cf012a 100644
--- a/drivers/net/fddi/defxx.c
+++ b/drivers/net/fddi/defxx.c
@@ -228,7 +228,7 @@
#define DRV_VERSION "v1.11"
#define DRV_RELDATE "2014/07/01"
-static char version[] =
+static const char version[] =
DRV_NAME ": " DRV_VERSION " " DRV_RELDATE
" Lawrence V. Stefani and others\n";
diff --git a/drivers/net/hippi/rrunner.c b/drivers/net/hippi/rrunner.c
index dd7fc6659ad4..9b0d6148e994 100644
--- a/drivers/net/hippi/rrunner.c
+++ b/drivers/net/hippi/rrunner.c
@@ -60,7 +60,8 @@ MODULE_AUTHOR("Jes Sorensen <jes@wildopensource.com>");
MODULE_DESCRIPTION("Essential RoadRunner HIPPI driver");
MODULE_LICENSE("GPL");
-static char version[] = "rrunner.c: v0.50 11/11/2002 Jes Sorensen (jes@wildopensource.com)\n";
+static const char version[] =
+"rrunner.c: v0.50 11/11/2002 Jes Sorensen (jes@wildopensource.com)\n";
static const struct net_device_ops rr_netdev_ops = {
diff --git a/drivers/staging/most/mostcore/core.c b/drivers/staging/most/mostcore/core.c
index 191404bc5906..892aae6e9c9a 100644
--- a/drivers/staging/most/mostcore/core.c
+++ b/drivers/staging/most/mostcore/core.c
@@ -82,7 +82,7 @@ struct most_inst_obj {
static const struct {
int most_ch_data_type;
- char *name;
+ const char *name;
} ch_data_type[] = {
{ MOST_CH_CONTROL, "control\n" },
{ MOST_CH_ASYNC, "async\n" },
diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index e94aea8c0d05..7b2a466616d6 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -939,11 +939,11 @@ static struct n_hdlc_buf *n_hdlc_buf_get(struct n_hdlc_buf_list *buf_list)
return buf;
} /* end of n_hdlc_buf_get() */
-static char hdlc_banner[] __initdata =
+static const char hdlc_banner[] __initconst =
KERN_INFO "HDLC line discipline maxframe=%u\n";
-static char hdlc_register_ok[] __initdata =
+static const char hdlc_register_ok[] __initconst =
KERN_INFO "N_HDLC line discipline registered.\n";
-static char hdlc_register_fail[] __initdata =
+static const char hdlc_register_fail[] __initconst =
KERN_ERR "error registering line discipline: %d\n";
static int __init n_hdlc_init(void)
@@ -968,9 +968,9 @@ static int __init n_hdlc_init(void)
} /* end of init_module() */
-static char hdlc_unregister_ok[] __exitdata =
+static const char hdlc_unregister_ok[] __exitdata =
KERN_INFO "N_HDLC: line discipline unregistered\n";
-static char hdlc_unregister_fail[] __exitdata =
+static const char hdlc_unregister_fail[] __exitdata =
KERN_ERR "N_HDLC: can't unregister line discipline (err = %d)\n";
static void __exit n_hdlc_exit(void)
diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
index bcf1d33e6ffe..ef9f47847f59 100644
--- a/drivers/tty/serial/st-asc.c
+++ b/drivers/tty/serial/st-asc.c
@@ -985,7 +985,7 @@ static struct platform_driver asc_serial_driver = {
static int __init asc_init(void)
{
int ret;
- static char banner[] __initdata =
+ static const char banner[] __initconst =
KERN_INFO "STMicroelectronics ASC driver initialized\n";
printk(banner);
diff --git a/net/decnet/af_decnet.c b/net/decnet/af_decnet.c
index e6e79eda9763..c6ed5e9502e9 100644
--- a/net/decnet/af_decnet.c
+++ b/net/decnet/af_decnet.c
@@ -2359,7 +2359,8 @@ MODULE_AUTHOR("Linux DECnet Project Team");
MODULE_LICENSE("GPL");
MODULE_ALIAS_NETPROTO(PF_DECnet);
-static char banner[] __initdata = KERN_INFO "NET4: DECnet for Linux: V.2.5.68s (C) 1995-2003 Linux DECnet Project Team\n";
+static const char banner[] __initconst = KERN_INFO
+"NET4: DECnet for Linux: V.2.5.68s (C) 1995-2003 Linux DECnet Project Team\n";
static int __init decnet_init(void)
{
--
2.7.4
--
Kees Cook
Pixel Security
^ permalink raw reply related
* Re: [PATCHv3 00/10] Nokia H4+ support
From: Pavel Machek @ 2017-04-05 20:28 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, Marcel Holtmann, Sebastian Reichel,
Gustavo F. Padovan, Johan Hedberg, Samuel Thibault, Tony Lindgren,
Jiri Slaby, Mark Rutland, open list:BLUETOOTH DRIVERS,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David S. Miller
In-Reply-To: <CAL_Jsq+NU3M5yuBpK1UGgzCVvq0eABMApCqEe3_d5+tDaABsgQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1764 bytes --]
On Wed 2017-04-05 13:16:58, Rob Herring wrote:
> On Fri, Mar 31, 2017 at 8:33 AM, Greg Kroah-Hartman
> <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
> > On Wed, Mar 29, 2017 at 11:33:26PM +0200, Marcel Holtmann wrote:
> >> Hi Rob,
> >>
> >> >> Here is PATCHv3 for the Nokia bluetooth patchset. I addressed all comments from
> >> >> Rob and Pavel regarding the serdev patches and dropped the *.dts patches, since
> >> >> they were queued by Tony. I also changed the patch order, so that the serdev
> >> >> patches come first. All of them have Acked-by from Rob, so I think it makes
> >> >> sense to merge them to serdev subsystem (now) and provide an immutable branch
> >> >> for the bluetooth subsystem.
> >> >
> >> > Greg doesn't read cover letters generally and since the serdev patches
> >> > are Cc rather than To him, he's probably not planning to pick them up.
> >>
> >> I wonder actually if we should merge all of these via bluetooth-next
> >> tree with proper Ack from Greg. However it would be good to also get
> >> buy in from Dave for merging this ultimately through net-next.
> >
> > I don't really care where it goes. I can take the whole thing in my
> > tty/serial tree now if no one objects and I get an ack from the relevant
> > maintainers {hint...}
>
> I think it is better if it goes thru BT tree. I have another driver
> converted that is dependent on this series. There's a couple other
> serdev changes on the list too, but this shouldn't depend on them.
I believe BT tree makes sense. Still it would be nice to get Greg's
ACK ...?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* [PATCH v9 3/3] printk: fix double printing with earlycon
From: Aleksey Makarov @ 2017-04-05 20:20 UTC (permalink / raw)
To: linux-serial
Cc: linux-kernel, Aleksey Makarov, Sudeep Holla, Greg Kroah-Hartman,
Peter Hurley, Jiri Slaby, Robin Murphy, Steven Rostedt,
Nair, Jayachandran, Sergey Senozhatsky, Petr Mladek
In-Reply-To: <20170315102854.1763-1-aleksey.makarov@linaro.org>
If a console was specified by ACPI SPCR table _and_ command line
parameters like "console=ttyAMA0" _and_ "earlycon" were specified,
then log messages appear twice.
The root cause is that the code traverses the list of specified
consoles (the `console_cmdline` array) and stops at the first match.
But it may happen that the same console is referred by the elements
of this array twice:
pl011,mmio,0x87e024000000,115200 -- from SPCR
ttyAMA0 -- from command line
but in this case `preferred_console` points to the second entry and
the flag CON_CONSDEV is not set, so bootconsole is not deregistered.
To fix that, introduce an invariant "The last non-braille console
is always the preferred one" on the entries of the console_cmdline
array. Then traverse it in reverse order to be sure that if
the console is preferred then it will be the first matching entry.
Introduce variable console_cmdline_cnt that keeps the number
of elements of the console_cmdline array (Petr Mladek). It helps
to get rid of the loop that searches for the end of this array.
Reported-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
v8 -> v9:
- Introduce variable console_cmdline_cnt that keeps the number
of elements of the console_cmdline array (Petr Mladek). It helps
to get rid of the loop that searches for the end of this array.
For the record: I think that this console_cmdline_cnt implementation
is worse than just a loop that finds the end of the array because
1) we have to support consistency between console_cmdline_cnt and
console_cmdline_cnt
2) it makes patch a bit more intrusive
v7 -> v8:
- add an explanation to the comment how console_cmdline can contain entries
referring to the same console
- move the body of the function introduced in the previous version to cycle
- don't panic() (Sergey Senozhatsky). Don't check this condition because
the loop condition guarantees that it holds.
- use swap() (Sergey Senozhatsky)
v6 -> v7:
- return back to v5
- leave the check for already registered entries and add a function that
maintains the invariant.
v5 -> v6:
- drop v5 and continue to work on v4:
- introduce _braille_is_braille_console(). It helps to split original loop
into three parts: 1) search for braille console, 2) check for
preferred_console, 3) match other entries so that these three parts do not
intersect.
- introduce for_each_console_cmdline() macros to traverse console_cmdline
(Petr Mladek)
kernel/printk/printk.c | 48 ++++++++++++++++++++++++++++++++++++++----------
1 file changed, 38 insertions(+), 10 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index fd752f0c8ef1..be657af45758 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -269,6 +269,7 @@ static struct console *exclusive_console;
#define MAX_CMDLINECONSOLES 8
static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
+static int console_cmdline_cnt;
static int preferred_console = -1;
int console_set_on_cmdline;
@@ -1905,12 +1906,26 @@ static int __add_preferred_console(char *name, int idx, char *options,
* See if this tty is not yet registered, and
* if we have a slot free.
*/
- for (i = 0, c = console_cmdline;
- i < MAX_CMDLINECONSOLES && c->name[0];
- i++, c++) {
+ for (i = 0, c = console_cmdline; i < console_cmdline_cnt; i++, c++) {
if (strcmp(c->name, name) == 0 && c->index == idx) {
- if (!brl_options)
- preferred_console = i;
+
+ if (brl_options)
+ return 0;
+
+ /*
+ * Maintain an invariant that will help to find if
+ * the matching console is preferred, see
+ * register_console():
+ *
+ * The last non-braille console is always
+ * the preferred one.
+ */
+ if (i != console_cmdline_cnt - 1)
+ swap(console_cmdline[i],
+ console_cmdline[console_cmdline_cnt - 1]);
+
+ preferred_console = console_cmdline_cnt - 1;
+
return 0;
}
}
@@ -1923,6 +1938,7 @@ static int __add_preferred_console(char *name, int idx, char *options,
braille_set_options(c, brl_options);
c->index = idx;
+ console_cmdline_cnt++;
return 0;
}
/*
@@ -2457,12 +2473,24 @@ void register_console(struct console *newcon)
}
/*
- * See if this console matches one we selected on
- * the command line.
+ * See if this console matches one we selected on the command line.
+ *
+ * There may be several entries in the console_cmdline array matching
+ * with the same console, one with newcon->match(), another by
+ * name/index:
+ *
+ * pl011,mmio,0x87e024000000,115200 -- added from SPCR
+ * ttyAMA0 -- added from command line
+ *
+ * Traverse the console_cmdline array in reverse order to be
+ * sure that if this console is preferred then it will be the first
+ * matching entry. We use the invariant that is maintained in
+ * __add_preferred_console().
*/
- for (i = 0, c = console_cmdline;
- i < MAX_CMDLINECONSOLES && c->name[0];
- i++, c++) {
+ for (i = console_cmdline_cnt - 1; i >= 0; i--) {
+
+ c = console_cmdline + i;
+
if (!newcon->match ||
newcon->match(newcon, c->name, c->index, c->options) != 0) {
/* default matching */
--
2.12.1
^ permalink raw reply related
* Re: [PATCH v8 3/3] printk: fix double printing with earlycon
From: Aleksey Makarov @ 2017-04-05 18:26 UTC (permalink / raw)
To: Petr Mladek, Sergey Senozhatsky
Cc: linux-serial, linux-kernel, Sudeep Holla, Greg Kroah-Hartman,
Peter Hurley, Jiri Slaby, Robin Murphy, Steven Rostedt,
Nair, Jayachandran, Sergey Senozhatsky
In-Reply-To: <20170404111202.GK29537@pathway.suse.cz>
On 04/04/2017 02:12 PM, Petr Mladek wrote:
> On Thu 2017-03-30 14:55:46, Sergey Senozhatsky wrote:
>> On (03/28/17 14:56), Petr Mladek wrote:
[..]
> Alekesey, any chance to use the global variable to count used or point
> to the last element?
>
> I know that you have already spent a lot of time with it. It was great
> work. But the current solution of the cycle looks weird to me.
Sorry for the delay. I will send next version soon.
Thank you
Aleksey Makarov
^ permalink raw reply
* Re: [PATCHv3 00/10] Nokia H4+ support
From: Rob Herring @ 2017-04-05 18:16 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Marcel Holtmann, Sebastian Reichel, Gustavo F. Padovan,
Johan Hedberg, Samuel Thibault, Pavel Machek, Tony Lindgren,
Jiri Slaby, Mark Rutland, open list:BLUETOOTH DRIVERS,
linux-serial@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, David S. Miller
In-Reply-To: <20170331133353.GA32267@kroah.com>
On Fri, Mar 31, 2017 at 8:33 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Wed, Mar 29, 2017 at 11:33:26PM +0200, Marcel Holtmann wrote:
>> Hi Rob,
>>
>> >> Here is PATCHv3 for the Nokia bluetooth patchset. I addressed all comments from
>> >> Rob and Pavel regarding the serdev patches and dropped the *.dts patches, since
>> >> they were queued by Tony. I also changed the patch order, so that the serdev
>> >> patches come first. All of them have Acked-by from Rob, so I think it makes
>> >> sense to merge them to serdev subsystem (now) and provide an immutable branch
>> >> for the bluetooth subsystem.
>> >
>> > Greg doesn't read cover letters generally and since the serdev patches
>> > are Cc rather than To him, he's probably not planning to pick them up.
>>
>> I wonder actually if we should merge all of these via bluetooth-next
>> tree with proper Ack from Greg. However it would be good to also get
>> buy in from Dave for merging this ultimately through net-next.
>
> I don't really care where it goes. I can take the whole thing in my
> tty/serial tree now if no one objects and I get an ack from the relevant
> maintainers {hint...}
I think it is better if it goes thru BT tree. I have another driver
converted that is dependent on this series. There's a couple other
serdev changes on the list too, but this shouldn't depend on them.
Rob
^ 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