* [PATCH i2c-next v9 1/5] dt-bindings: i2c: Add 'bus-timeout-ms' and '#retries' properties as common optional
From: Jae Hyun Yoo @ 2018-10-30 21:09 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181030210917.32711-1-jae.hyun.yoo@linux.intel.com>
This commit adds 'bus-timeout-ms' and '#retries' properties as
common optional properties that can be used for setting 'timeout'
and 'retries' values of 'struct i2c_adapter'. With this patch, the
bus timeout value and the master transfer retries count can be set
through these properties at the registration time of an adapter.
Still the values can be set by I2C_TIMEOUT and I2C_RETRIES ioctls
on cdev at runtime too.
These properties may not be supported by all drivers. However, if
a driver wants to support one of them, it should adapt the
bindings in this document.
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/i2c/i2c.txt | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index 11263982470e..bdead91f82a4 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -80,6 +80,12 @@ wants to support one of the below features, it should adapt the bindings below.
Names of map programmable addresses.
It can contain any map needing another address than default one.
+- bus-timeout-ms
+ Bus timeout in milliseconds.
+
+- #retries
+ Number of retries for master transfer.
+
Binding may contain optional "interrupts" property, describing interrupts
used by the device. I2C core will assign "irq" interrupt (or the very first
interrupt if not using interrupt names) as primary interrupt for the slave.
--
2.19.1
^ permalink raw reply related
* [PATCH i2c-next v9 2/5] i2c: core: Add support reading of 'bus-timeout-ms' and '#retries' properties
From: Jae Hyun Yoo @ 2018-10-30 21:09 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181030210917.32711-1-jae.hyun.yoo@linux.intel.com>
This commit adds support for 'bus-timeout-ms' and '#retries'
properties to set 'timeout' and 'retries' values in
'struct i2c_adapter' in case an adapter node has the properties.
Still the values can be set by I2C_TIMEOUT and I2C_RETRIES ioctls
on cdev at runtime too.
These properties may not be supported by all drivers. However, if
a driver wants to support one of them, it should adapt the
bindings in the dt-bindings document.
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
drivers/i2c/i2c-core-base.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index dc78aa7369de..abf8e78bdb82 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1214,6 +1214,7 @@ EXPORT_SYMBOL_GPL(i2c_handle_smbus_host_notify);
static int i2c_register_adapter(struct i2c_adapter *adap)
{
int res = -EINVAL;
+ u32 bus_timeout_ms = 0;
/* Can't register until after driver model init */
if (WARN_ON(!is_registered)) {
@@ -1239,8 +1240,15 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
INIT_LIST_HEAD(&adap->userspace_clients);
/* Set default timeout to 1 second if not already set */
- if (adap->timeout == 0)
- adap->timeout = HZ;
+ if (adap->timeout == 0) {
+ device_property_read_u32(&adap->dev, "bus-timeout-ms",
+ &bus_timeout_ms);
+ adap->timeout = bus_timeout_ms ?
+ msecs_to_jiffies(bus_timeout_ms) : HZ;
+ }
+
+ /* Set retries count if it has the property setting */
+ device_property_read_u32(&adap->dev, "#retries", &adap->retries);
/* register soft irqs for Host Notify */
res = i2c_setup_host_notify_irq_domain(adap);
--
2.19.1
^ permalink raw reply related
* [PATCH i2c-next v9 3/5] dt-bindings: i2c: aspeed: Add 'bus-timeout-ms' property as an optional property
From: Jae Hyun Yoo @ 2018-10-30 21:09 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181030210917.32711-1-jae.hyun.yoo@linux.intel.com>
This commit adds 'bus-timeout-ms' property as an optional property
which can be used for setting the bus timeout value of an adapter.
With this patch, the bus timeout value can be set through this
property at the probing time of this module. Still the bus timeout
value can be set by an I2C_TIMEOUT ioctl on cdev at runtime too.
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/i2c/i2c-aspeed.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
index 8fbd8633a387..ce1f07620368 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
@@ -17,6 +17,9 @@ Optional Properties:
specified
- multi-master : states that there is another master active on this bus.
+- bus-timeout-ms: bus timeout in milliseconds defaults to 1 second when not
+ specified.
+
Example:
i2c {
--
2.19.1
^ permalink raw reply related
* [PATCH i2c-next v9 4/5] i2c: aspeed: Remove hard-coded bus timeout value setting
From: Jae Hyun Yoo @ 2018-10-30 21:09 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181030210917.32711-1-jae.hyun.yoo@linux.intel.com>
This commit removes hard-coded bus timeout value setting so that
it can be set by i2c-core-base.
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
---
drivers/i2c/busses/i2c-aspeed.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 8dc9161ced38..833b6b6a4c7e 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -930,7 +930,6 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
init_completion(&bus->cmd_complete);
bus->adap.owner = THIS_MODULE;
bus->adap.retries = 0;
- bus->adap.timeout = 5 * HZ;
bus->adap.algo = &aspeed_i2c_algo;
bus->adap.dev.parent = &pdev->dev;
bus->adap.dev.of_node = pdev->dev.of_node;
--
2.19.1
^ permalink raw reply related
* [PATCH i2c-next v9 5/5] i2c: aspeed: Add bus idle waiting logic for multi-master use cases
From: Jae Hyun Yoo @ 2018-10-30 21:09 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181030210917.32711-1-jae.hyun.yoo@linux.intel.com>
In multi-master environment, this driver's master cannot know
exactly when a peer master sends data to this driver's slave so a
case can be happened that this master tries to send data through
the master_xfer function but slave data from peer master is still
being processed by this driver.
To prevent any state corruption in the case, this patch adds
checking code if any slave operation is ongoing and it waits up to
the bus timeout duration before starting a master_xfer operation.
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
---
drivers/i2c/busses/i2c-aspeed.c | 55 ++++++++++++++++++++++++---------
1 file changed, 40 insertions(+), 15 deletions(-)
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 833b6b6a4c7e..30c3ab3a4844 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -12,6 +12,7 @@
#include <linux/clk.h>
#include <linux/completion.h>
+#include <linux/delay.h>
#include <linux/err.h>
#include <linux/errno.h>
#include <linux/i2c.h>
@@ -115,6 +116,9 @@
/* 0x18 : I2CD Slave Device Address Register */
#define ASPEED_I2CD_DEV_ADDR_MASK GENMASK(6, 0)
+/* Busy checking */
+#define ASPEED_I2C_BUS_BUSY_CHECK_INTERVAL_US (10 * 1000)
+
enum aspeed_i2c_master_state {
ASPEED_I2C_MASTER_INACTIVE,
ASPEED_I2C_MASTER_START,
@@ -156,6 +160,8 @@ struct aspeed_i2c_bus {
int cmd_err;
/* Protected only by i2c_lock_bus */
int master_xfer_result;
+ /* Multi-master */
+ bool multi_master;
#if IS_ENABLED(CONFIG_I2C_SLAVE)
struct i2c_client *slave;
enum aspeed_i2c_slave_state slave_state;
@@ -596,27 +602,44 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
return irq_remaining ? IRQ_NONE : IRQ_HANDLED;
}
+static int aspeed_i2c_check_bus_busy(struct aspeed_i2c_bus *bus)
+{
+ unsigned long timeout;
+
+ if (bus->multi_master) {
+ might_sleep();
+ /* Initialize it only when multi_master is set */
+ timeout = jiffies + bus->adap.timeout;
+ }
+
+ for (;;) {
+ if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
+ ASPEED_I2CD_BUS_BUSY_STS))
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ if (bus->slave_state == ASPEED_I2C_SLAVE_STOP)
+#endif
+ return 0;
+ if (!bus->multi_master)
+ break;
+ if (time_after(jiffies, timeout))
+ break;
+ usleep_range((ASPEED_I2C_BUS_BUSY_CHECK_INTERVAL_US >> 2) + 1,
+ ASPEED_I2C_BUS_BUSY_CHECK_INTERVAL_US);
+ }
+
+ return aspeed_i2c_recover_bus(bus);
+}
+
static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
struct i2c_msg *msgs, int num)
{
struct aspeed_i2c_bus *bus = i2c_get_adapdata(adap);
unsigned long time_left, flags;
- int ret = 0;
- spin_lock_irqsave(&bus->lock, flags);
- bus->cmd_err = 0;
-
- /* If bus is busy, attempt recovery. We assume a single master
- * environment.
- */
- if (readl(bus->base + ASPEED_I2C_CMD_REG) & ASPEED_I2CD_BUS_BUSY_STS) {
- spin_unlock_irqrestore(&bus->lock, flags);
- ret = aspeed_i2c_recover_bus(bus);
- if (ret)
- return ret;
- spin_lock_irqsave(&bus->lock, flags);
- }
+ if (aspeed_i2c_check_bus_busy(bus))
+ return -EAGAIN;
+ spin_lock_irqsave(&bus->lock, flags);
bus->cmd_err = 0;
bus->msgs = msgs;
bus->msgs_index = 0;
@@ -827,7 +850,9 @@ static int aspeed_i2c_init(struct aspeed_i2c_bus *bus,
if (ret < 0)
return ret;
- if (!of_property_read_bool(pdev->dev.of_node, "multi-master"))
+ if (of_property_read_bool(pdev->dev.of_node, "multi-master"))
+ bus->multi_master = true;
+ else
fun_ctrl_reg |= ASPEED_I2CD_MULTI_MASTER_DIS;
/* Enable Master Mode */
--
2.19.1
^ permalink raw reply related
* [PATCH v2 0/5] serial: Finish sysrq on qcom_geni; fix sysrq vs. lockdep on 8250
From: Douglas Anderson @ 2018-10-30 22:11 UTC (permalink / raw)
To: linux-aspeed
I started out this series trying to make sysrq work over the serial
console on qcom_geni_serial, then fell into a rat's nest.
To solve the deadlock I faced when enabling sysrq I tried to borrow
code from '8250_port.c' which avoided grabbing the port lock in
console_write(). ...but since these days I try to run with lockdep on
all the time, I found it caused an annoying lockdep splat (which I
also reproduced on my rk3399 board). ...so I instead changed my
qcom_geni_serial solution to borrow code from 'msm_serial.c'
I wasn't super happy with the solution in 'msm_serial.c' though. I
don't like releasing the spinlock there. Not only is it ugly but it
means we are unlocking / re-locking _all the time_ even though sysrq
characters are rare. ...so I came up with what I think is a better
solution and then implemented it for qcom_geni_serial.
Since I had a good way to test 8250-based UARTs, I also fixed that
driver to use my new method. When doing so, I ran into a missing
include in serial_core.h. NOTE: I didn't have a way to test
msm_serial.c at all, so I didn't switch that (or all other serial
drivers for that matter) to the new method.
NOTE: from a serial point of view v2 is the same as v1 but I've
removed the extra kgdb-related patches and made it obvious that this
is really for all sysrq, not just kgdb. I've also generally tried to
curate the CCs more properly.
Douglas Anderson (5):
serial: qcom_geni_serial: Finish supporting sysrq
serial: core: Allow processing sysrq at port unlock time
serial: qcom_geni_serial: Process sysrq at port unlock time
serial: core: Include console.h from serial_core.h
serial: 8250: Process sysrq at port unlock time
drivers/tty/serial/8250/8250_aspeed_vuart.c | 6 +++-
drivers/tty/serial/8250/8250_fsl.c | 6 +++-
drivers/tty/serial/8250/8250_omap.c | 6 +++-
drivers/tty/serial/8250/8250_port.c | 8 ++---
drivers/tty/serial/qcom_geni_serial.c | 10 ++++--
include/linux/serial_core.h | 38 ++++++++++++++++++++-
6 files changed, 63 insertions(+), 11 deletions(-)
--
2.19.1.568.g152ad8e336-goog
^ permalink raw reply
* [PATCH v2 1/5] serial: qcom_geni_serial: Finish supporting sysrq
From: Douglas Anderson @ 2018-10-30 22:11 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181030221107.79758-1-dianders@chromium.org>
The geni serial driver already had some sysrq code in it, but since
SUPPORT_SYSRQ wasn't defined the code didn't do anything useful.
Let's make it useful by adding that define using the same formula
found in other serial drivers.
In order to prevent deadlock, we'll take a page from the
'msm_serial.c' where the spinlock is released around
uart_handle_sysrq_char(). This seemed better than copying from
'8250_port.c' where we skip locking in the console_write function
since the '8250_port.c' method can cause lockdep warnings when
dropping into kgdb.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
drivers/tty/serial/qcom_geni_serial.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 1515074e18fb..b83e3554bced 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -1,6 +1,10 @@
// SPDX-License-Identifier: GPL-2.0
// Copyright (c) 2017-2018, The Linux foundation. All rights reserved.
+#if defined(CONFIG_SERIAL_QCOM_GENI_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
+# define SUPPORT_SYSRQ
+#endif
+
#include <linux/clk.h>
#include <linux/console.h>
#include <linux/io.h>
@@ -495,7 +499,10 @@ static int handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
continue;
}
+ spin_unlock(&uport->lock);
sysrq = uart_handle_sysrq_char(uport, buf[c]);
+ spin_lock(&uport->lock);
+
if (!sysrq)
tty_insert_flip_char(tport, buf[c], TTY_NORMAL);
}
--
2.19.1.568.g152ad8e336-goog
^ permalink raw reply related
* [PATCH v2 2/5] serial: core: Allow processing sysrq at port unlock time
From: Douglas Anderson @ 2018-10-30 22:11 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181030221107.79758-1-dianders@chromium.org>
Right now serial drivers process sysrq keys deep in their character
receiving code. This means that they've already grabbed their
port->lock spinlock. This can end up getting in the way if we've go
to do serial stuff (especially kgdb) in response to the sysrq.
Serial drivers have various hacks in them to handle this. Looking at
'8250_port.c' you can see that the console_write() skips locking if
we're in the sysrq handler. Looking at 'msm_serial.c' you can see
that the port lock is dropped around uart_handle_sysrq_char().
It turns out that these hacks aren't exactly perfect. If you have
lockdep turned on and use something like the 8250_port hack you'll get
a splat that looks like:
WARNING: possible circular locking dependency detected
[...] is trying to acquire lock:
... (console_owner){-.-.}, at: console_unlock+0x2e0/0x5e4
but task is already holding lock:
... (&port_lock_key){-.-.}, at: serial8250_handle_irq+0x30/0xe4
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&port_lock_key){-.-.}:
_raw_spin_lock_irqsave+0x58/0x70
serial8250_console_write+0xa8/0x250
univ8250_console_write+0x40/0x4c
console_unlock+0x528/0x5e4
register_console+0x2c4/0x3b0
uart_add_one_port+0x350/0x478
serial8250_register_8250_port+0x350/0x3a8
dw8250_probe+0x67c/0x754
platform_drv_probe+0x58/0xa4
really_probe+0x150/0x294
driver_probe_device+0xac/0xe8
__driver_attach+0x98/0xd0
bus_for_each_dev+0x84/0xc8
driver_attach+0x2c/0x34
bus_add_driver+0xf0/0x1ec
driver_register+0xb4/0x100
__platform_driver_register+0x60/0x6c
dw8250_platform_driver_init+0x20/0x28
...
-> #0 (console_owner){-.-.}:
lock_acquire+0x1e8/0x214
console_unlock+0x35c/0x5e4
vprintk_emit+0x230/0x274
vprintk_default+0x7c/0x84
vprintk_func+0x190/0x1bc
printk+0x80/0xa0
__handle_sysrq+0x104/0x21c
handle_sysrq+0x30/0x3c
serial8250_read_char+0x15c/0x18c
serial8250_rx_chars+0x34/0x74
serial8250_handle_irq+0x9c/0xe4
dw8250_handle_irq+0x98/0xcc
serial8250_interrupt+0x50/0xe8
...
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&port_lock_key);
lock(console_owner);
lock(&port_lock_key);
lock(console_owner);
*** DEADLOCK ***
The hack used in 'msm_serial.c' doesn't cause the above splats but it
seems a bit ugly to unlock / lock our spinlock deep in our irq
handler.
It seems like we could defer processing the sysrq until the end of the
interrupt handler right after we've unlocked the port. With this
scheme if a whole batch of sysrq characters comes in one irq then we
won't handle them all, but that seems like it should be a fine
compromise.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
include/linux/serial_core.h | 37 ++++++++++++++++++++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 406edae44ca3..3460b15a2607 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -173,6 +173,7 @@ struct uart_port {
struct console *cons; /* struct console, if any */
#if defined(CONFIG_SERIAL_CORE_CONSOLE) || defined(SUPPORT_SYSRQ)
unsigned long sysrq; /* sysrq timeout */
+ unsigned int sysrq_ch; /* char for sysrq */
#endif
/* flags must be updated while holding port mutex */
@@ -482,8 +483,42 @@ uart_handle_sysrq_char(struct uart_port *port, unsigned int ch)
}
return 0;
}
+static inline int
+uart_prepare_sysrq_char(struct uart_port *port, unsigned int ch)
+{
+ if (port->sysrq) {
+ if (ch && time_before(jiffies, port->sysrq)) {
+ port->sysrq_ch = ch;
+ port->sysrq = 0;
+ return 1;
+ }
+ port->sysrq = 0;
+ }
+ return 0;
+}
+static inline void
+uart_unlock_and_check_sysrq(struct uart_port *port, unsigned long irqflags)
+{
+ int sysrq_ch;
+
+ sysrq_ch = port->sysrq_ch;
+ port->sysrq_ch = 0;
+
+ spin_unlock_irqrestore(&port->lock, irqflags);
+
+ if (sysrq_ch)
+ handle_sysrq(sysrq_ch);
+}
#else
-#define uart_handle_sysrq_char(port,ch) ({ (void)port; 0; })
+static inline int
+uart_handle_sysrq_char(struct uart_port *port, unsigned int ch) { return 0; }
+static inline int
+uart_prepare_sysrq_char(struct uart_port *port, unsigned int ch) { return 0; }
+static inline void
+uart_unlock_and_check_sysrq(struct uart_port *port, unsigned long irqflags)
+{
+ spin_unlock_irqrestore(&port->lock, irqflags);
+}
#endif
/*
--
2.19.1.568.g152ad8e336-goog
^ permalink raw reply related
* [PATCH v2 3/5] serial: qcom_geni_serial: Process sysrq at port unlock time
From: Douglas Anderson @ 2018-10-30 22:11 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181030221107.79758-1-dianders@chromium.org>
Let's take advantage of the new ("serial: core: Allow processing sysrq
at port unlock time") to handle sysrqs more cleanly.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
drivers/tty/serial/qcom_geni_serial.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index b83e3554bced..29ddc1fc65f8 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -499,9 +499,7 @@ static int handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
continue;
}
- spin_unlock(&uport->lock);
- sysrq = uart_handle_sysrq_char(uport, buf[c]);
- spin_lock(&uport->lock);
+ sysrq = uart_prepare_sysrq_char(uport, buf[c]);
if (!sysrq)
tty_insert_flip_char(tport, buf[c], TTY_NORMAL);
@@ -811,7 +809,8 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
qcom_geni_serial_handle_rx(uport, drop_rx);
out_unlock:
- spin_unlock_irqrestore(&uport->lock, flags);
+ uart_unlock_and_check_sysrq(uport, flags);
+
return IRQ_HANDLED;
}
--
2.19.1.568.g152ad8e336-goog
^ permalink raw reply related
* [PATCH v2 4/5] serial: core: Include console.h from serial_core.h
From: Douglas Anderson @ 2018-10-30 22:11 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181030221107.79758-1-dianders@chromium.org>
In the static inline function uart_handle_break() in serial_core.h we
dereference port->cons. That gives an error unless console.h is also
included.
This error hasn't shown up till now because everyone who has defined
SUPPORT_SYSRQ has also included console.h, but it's a bit ugly to make
this requirement. Let's make the include explicit.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
include/linux/serial_core.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 3460b15a2607..ff9d0ee32f11 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -22,6 +22,7 @@
#include <linux/bitops.h>
#include <linux/compiler.h>
+#include <linux/console.h>
#include <linux/interrupt.h>
#include <linux/circ_buf.h>
#include <linux/spinlock.h>
--
2.19.1.568.g152ad8e336-goog
^ permalink raw reply related
* [PATCH v2 5/5] serial: 8250: Process sysrq at port unlock time
From: Douglas Anderson @ 2018-10-30 22:11 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181030221107.79758-1-dianders@chromium.org>
Let's take advantage of the new ("serial: core: Allow processing sysrq
at port unlock time") to handle sysrqs more cleanly.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
I don't have a great way to test all 8250 variants, but I've at least
tested rk3288 / rk3399 and they seem to work. Hopefully I got the
aspeed_vuart / fsl / omap variants right (I only compile tested
those).
drivers/tty/serial/8250/8250_aspeed_vuart.c | 6 +++++-
drivers/tty/serial/8250/8250_fsl.c | 6 +++++-
drivers/tty/serial/8250/8250_omap.c | 6 +++++-
drivers/tty/serial/8250/8250_port.c | 8 +++-----
4 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
index 435bec40dee6..0438d9a905ce 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -5,6 +5,10 @@
* Copyright (C) 2016 Jeremy Kerr <jk@ozlabs.org>, IBM Corp.
* Copyright (C) 2006 Arnd Bergmann <arnd@arndb.de>, IBM Corp.
*/
+#if defined(CONFIG_SERIAL_8250_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
+#define SUPPORT_SYSRQ
+#endif
+
#include <linux/device.h>
#include <linux/module.h>
#include <linux/of_address.h>
@@ -293,7 +297,7 @@ static int aspeed_vuart_handle_irq(struct uart_port *port)
if (lsr & UART_LSR_THRE)
serial8250_tx_chars(up);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_unlock_and_check_sysrq(port, flags);
return 1;
}
diff --git a/drivers/tty/serial/8250/8250_fsl.c b/drivers/tty/serial/8250/8250_fsl.c
index 6640a4c7ddd1..ff3dcaea5d93 100644
--- a/drivers/tty/serial/8250/8250_fsl.c
+++ b/drivers/tty/serial/8250/8250_fsl.c
@@ -1,4 +1,8 @@
// SPDX-License-Identifier: GPL-2.0
+#if defined(CONFIG_SERIAL_8250_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
+#define SUPPORT_SYSRQ
+#endif
+
#include <linux/serial_reg.h>
#include <linux/serial_8250.h>
@@ -54,7 +58,7 @@ int fsl8250_handle_irq(struct uart_port *port)
serial8250_tx_chars(up);
up->lsr_saved_flags = orig_lsr;
- spin_unlock_irqrestore(&up->port.lock, flags);
+ uart_unlock_and_check_sysrq(&up->port, flags);
return 1;
}
EXPORT_SYMBOL_GPL(fsl8250_handle_irq);
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index a019286f8bb6..ad7ba7d0f28d 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -8,6 +8,10 @@
*
*/
+#if defined(CONFIG_SERIAL_8250_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
+#define SUPPORT_SYSRQ
+#endif
+
#include <linux/device.h>
#include <linux/io.h>
#include <linux/module.h>
@@ -1085,7 +1089,7 @@ static int omap_8250_dma_handle_irq(struct uart_port *port)
}
}
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_unlock_and_check_sysrq(port, flags);
serial8250_rpm_put(up);
return 1;
}
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 3f779d25ec0c..d2f3310abe54 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1736,7 +1736,7 @@ void serial8250_read_char(struct uart_8250_port *up, unsigned char lsr)
else if (lsr & UART_LSR_FE)
flag = TTY_FRAME;
}
- if (uart_handle_sysrq_char(port, ch))
+ if (uart_prepare_sysrq_char(port, ch))
return;
uart_insert_char(port, lsr, UART_LSR_OE, ch, flag);
@@ -1878,7 +1878,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
if ((!up->dma || up->dma->tx_err) && (status & UART_LSR_THRE))
serial8250_tx_chars(up);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_unlock_and_check_sysrq(port, flags);
return 1;
}
EXPORT_SYMBOL_GPL(serial8250_handle_irq);
@@ -3239,9 +3239,7 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s,
serial8250_rpm_get(up);
- if (port->sysrq)
- locked = 0;
- else if (oops_in_progress)
+ if (oops_in_progress)
locked = spin_trylock_irqsave(&port->lock, flags);
else
spin_lock_irqsave(&port->lock, flags);
--
2.19.1.568.g152ad8e336-goog
^ permalink raw reply related
* [PATCH 0/7] serial: Finish kgdb on qcom_geni; fix many lockdep splats w/ kgdb
From: Doug Anderson @ 2018-10-30 22:20 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181030115628.eqtyqdugkpkxvyr2@holly.lan>
Hi,
On Tue, Oct 30, 2018 at 4:56 AM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Mon, Oct 29, 2018 at 11:07:00AM -0700, Douglas Anderson wrote:
> > Looking back, this is pretty much two series squashed that could be
> > treated indepdently. The first is a serial series and the second is a
> > kgdb series.
>
> Indeed.
>
> I couldn't work out the link between the first 5 patches and the last 2
> until I read this...
>
> Is anything in the 01->05 patch set even related to kgdb? From the stack
> traces it looks to me like the lock dep warning would trigger for any
> sysrq. I think separating into two threads for v2 would be sensible.
Yes, sorry about the mess. Splitting this into two series makes the
most sense. Then I can focus more on trying to get the CCs right and
people can just get the patches that matter to them.
OK, I've sent v2 of both series out now. Please yell if you can't
find them for whatever reason.
-Doug
-Doug
^ permalink raw reply
* [PATCH i2c-next v9 5/5] i2c: aspeed: Add bus idle waiting logic for multi-master use cases
From: kbuild test robot @ 2018-10-30 22:22 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181030210917.32711-6-jae.hyun.yoo@linux.intel.com>
Hi Jae,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.19 next-20181030]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jae-Hyun-Yoo/dt-bindings-i2c-Add-bus-timeout-ms-and-retries-properties-as-common-optional/20181031-051152
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=xtensa
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
In file included from include/linux/ktime.h:25,
from include/linux/rcutiny.h:28,
from include/linux/rcupdate.h:209,
from include/linux/srcu.h:33,
from include/linux/notifier.h:16,
from include/linux/clk.h:17,
from drivers/i2c/busses/i2c-aspeed.c:13:
drivers/i2c/busses/i2c-aspeed.c: In function 'aspeed_i2c_master_xfer':
>> include/linux/jiffies.h:108:15: warning: 'timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
((long)((b) - (a)) < 0))
^
drivers/i2c/busses/i2c-aspeed.c:607:16: note: 'timeout' was declared here
unsigned long timeout;
^~~~~~~
--
In file included from include/linux/ktime.h:25,
from include/linux/rcutiny.h:28,
from include/linux/rcupdate.h:209,
from include/linux/srcu.h:33,
from include/linux/notifier.h:16,
from include/linux/clk.h:17,
from drivers/i2c//busses/i2c-aspeed.c:13:
drivers/i2c//busses/i2c-aspeed.c: In function 'aspeed_i2c_master_xfer':
>> include/linux/jiffies.h:108:15: warning: 'timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
((long)((b) - (a)) < 0))
^
drivers/i2c//busses/i2c-aspeed.c:607:16: note: 'timeout' was declared here
unsigned long timeout;
^~~~~~~
vim +/timeout +108 include/linux/jiffies.h
^1da177e Linus Torvalds 2005-04-16 91
^1da177e Linus Torvalds 2005-04-16 92 /*
^1da177e Linus Torvalds 2005-04-16 93 * These inlines deal with timer wrapping correctly. You are
^1da177e Linus Torvalds 2005-04-16 94 * strongly encouraged to use them
^1da177e Linus Torvalds 2005-04-16 95 * 1. Because people otherwise forget
^1da177e Linus Torvalds 2005-04-16 96 * 2. Because if the timer wrap changes in future you won't have to
^1da177e Linus Torvalds 2005-04-16 97 * alter your driver code.
^1da177e Linus Torvalds 2005-04-16 98 *
^1da177e Linus Torvalds 2005-04-16 99 * time_after(a,b) returns true if the time a is after time b.
^1da177e Linus Torvalds 2005-04-16 100 *
^1da177e Linus Torvalds 2005-04-16 101 * Do this with "<0" and ">=0" to only test the sign of the result. A
^1da177e Linus Torvalds 2005-04-16 102 * good compiler would generate better code (and a really good compiler
^1da177e Linus Torvalds 2005-04-16 103 * wouldn't care). Gcc is currently neither.
^1da177e Linus Torvalds 2005-04-16 104 */
^1da177e Linus Torvalds 2005-04-16 105 #define time_after(a,b) \
^1da177e Linus Torvalds 2005-04-16 106 (typecheck(unsigned long, a) && \
^1da177e Linus Torvalds 2005-04-16 107 typecheck(unsigned long, b) && \
5a581b36 Paul E. McKenney 2013-07-27 @108 ((long)((b) - (a)) < 0))
^1da177e Linus Torvalds 2005-04-16 109 #define time_before(a,b) time_after(b,a)
^1da177e Linus Torvalds 2005-04-16 110
:::::: The code at line 108 was first introduced by commit
:::::: 5a581b367b5df0531265311fc681c2abd377e5e6 jiffies: Avoid undefined behavior from signed overflow
:::::: TO: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
:::::: CC: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 55213 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/linux-aspeed/attachments/20181031/ce27fe2c/attachment-0001.gz>
^ permalink raw reply
* [PATCH i2c-next v9 5/5] i2c: aspeed: Add bus idle waiting logic for multi-master use cases
From: Jae Hyun Yoo @ 2018-10-30 23:04 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <201810310601.Odxb7qSY%fengguang.wu@intel.com>
On 10/30/2018 3:22 PM, kbuild test robot wrote:
> Hi Jae,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on wsa/i2c/for-next]
> [also build test WARNING on v4.19 next-20181030]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Jae-Hyun-Yoo/dt-bindings-i2c-Add-bus-timeout-ms-and-retries-properties-as-common-optional/20181031-051152
> base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
> config: xtensa-allyesconfig (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 8.1.0
> reproduce:
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> GCC_VERSION=8.1.0 make.cross ARCH=xtensa
>
> Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
> http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
>
> All warnings (new ones prefixed by >>):
>
> In file included from include/linux/ktime.h:25,
> from include/linux/rcutiny.h:28,
> from include/linux/rcupdate.h:209,
> from include/linux/srcu.h:33,
> from include/linux/notifier.h:16,
> from include/linux/clk.h:17,
> from drivers/i2c/busses/i2c-aspeed.c:13:
> drivers/i2c/busses/i2c-aspeed.c: In function 'aspeed_i2c_master_xfer':
>>> include/linux/jiffies.h:108:15: warning: 'timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
> ((long)((b) - (a)) < 0))
> ^
> drivers/i2c/busses/i2c-aspeed.c:607:16: note: 'timeout' was declared here
> unsigned long timeout;
> ^~~~~~~
> --
> In file included from include/linux/ktime.h:25,
> from include/linux/rcutiny.h:28,
> from include/linux/rcupdate.h:209,
> from include/linux/srcu.h:33,
> from include/linux/notifier.h:16,
> from include/linux/clk.h:17,
> from drivers/i2c//busses/i2c-aspeed.c:13:
> drivers/i2c//busses/i2c-aspeed.c: In function 'aspeed_i2c_master_xfer':
>>> include/linux/jiffies.h:108:15: warning: 'timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
> ((long)((b) - (a)) < 0))
> ^
> drivers/i2c//busses/i2c-aspeed.c:607:16: note: 'timeout' was declared here
> unsigned long timeout;
> ^~~~~~~
>
> vim +/timeout +108 include/linux/jiffies.h
>
> ^1da177e Linus Torvalds 2005-04-16 91
> ^1da177e Linus Torvalds 2005-04-16 92 /*
> ^1da177e Linus Torvalds 2005-04-16 93 * These inlines deal with timer wrapping correctly. You are
> ^1da177e Linus Torvalds 2005-04-16 94 * strongly encouraged to use them
> ^1da177e Linus Torvalds 2005-04-16 95 * 1. Because people otherwise forget
> ^1da177e Linus Torvalds 2005-04-16 96 * 2. Because if the timer wrap changes in future you won't have to
> ^1da177e Linus Torvalds 2005-04-16 97 * alter your driver code.
> ^1da177e Linus Torvalds 2005-04-16 98 *
> ^1da177e Linus Torvalds 2005-04-16 99 * time_after(a,b) returns true if the time a is after time b.
> ^1da177e Linus Torvalds 2005-04-16 100 *
> ^1da177e Linus Torvalds 2005-04-16 101 * Do this with "<0" and ">=0" to only test the sign of the result. A
> ^1da177e Linus Torvalds 2005-04-16 102 * good compiler would generate better code (and a really good compiler
> ^1da177e Linus Torvalds 2005-04-16 103 * wouldn't care). Gcc is currently neither.
> ^1da177e Linus Torvalds 2005-04-16 104 */
> ^1da177e Linus Torvalds 2005-04-16 105 #define time_after(a,b) \
> ^1da177e Linus Torvalds 2005-04-16 106 (typecheck(unsigned long, a) && \
> ^1da177e Linus Torvalds 2005-04-16 107 typecheck(unsigned long, b) && \
> 5a581b36 Paul E. McKenney 2013-07-27 @108 ((long)((b) - (a)) < 0))
> ^1da177e Linus Torvalds 2005-04-16 109 #define time_before(a,b) time_after(b,a)
> ^1da177e Linus Torvalds 2005-04-16 110
>
> :::::: The code at line 108 was first introduced by commit
> :::::: 5a581b367b5df0531265311fc681c2abd377e5e6 jiffies: Avoid undefined behavior from signed overflow
>
> :::::: TO: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> :::::: CC: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
This is a false warning.
The 'timeout' local variable will be initialized properly if it's gonna
be used when the multi-master property is set.
-Jae
^ permalink raw reply
* [PATCH i2c-next v9 5/5] i2c: aspeed: Add bus idle waiting logic for multi-master use cases
From: kbuild test robot @ 2018-11-01 17:44 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181030210917.32711-6-jae.hyun.yoo@linux.intel.com>
Hi Jae,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.19 next-20181101]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jae-Hyun-Yoo/dt-bindings-i2c-Add-bus-timeout-ms-and-retries-properties-as-common-optional/20181031-051152
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
drivers/i2c/busses/i2c-aspeed.c: In function 'aspeed_i2c_master_xfer':
>> drivers/i2c/busses/i2c-aspeed.c:624:210: warning: 'timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
if (time_after(jiffies, timeout))
^
drivers/i2c/busses/i2c-aspeed.c:607:16: note: 'timeout' was declared here
unsigned long timeout;
^~~~~~~
vim +/timeout +624 drivers/i2c/busses/i2c-aspeed.c
604
605 static int aspeed_i2c_check_bus_busy(struct aspeed_i2c_bus *bus)
606 {
607 unsigned long timeout;
608
609 if (bus->multi_master) {
610 might_sleep();
611 /* Initialize it only when multi_master is set */
612 timeout = jiffies + bus->adap.timeout;
613 }
614
615 for (;;) {
616 if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
617 ASPEED_I2CD_BUS_BUSY_STS))
618 #if IS_ENABLED(CONFIG_I2C_SLAVE)
619 if (bus->slave_state == ASPEED_I2C_SLAVE_STOP)
620 #endif
621 return 0;
622 if (!bus->multi_master)
623 break;
> 624 if (time_after(jiffies, timeout))
625 break;
626 usleep_range((ASPEED_I2C_BUS_BUSY_CHECK_INTERVAL_US >> 2) + 1,
627 ASPEED_I2C_BUS_BUSY_CHECK_INTERVAL_US);
628 }
629
630 return aspeed_i2c_recover_bus(bus);
631 }
632
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 64563 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/linux-aspeed/attachments/20181102/9bc6f6c4/attachment-0001.gz>
^ permalink raw reply
* [PATCH] i2c: aspeed: Fix an incorrect error log printing
From: Jae Hyun Yoo @ 2018-11-01 17:46 UTC (permalink / raw)
To: linux-aspeed
Usually a SLAVE_MATCH and an RX_DONE intterupts come together at
the same time in most cases but rarely these come separately like
SLAVE_MATCH first and then RX_DONE right after that.
Current slave irq handler prints out an error message if the
latter case happens but this is not an actual error, so this commit
fixes the incorrect error log printing and makes the handler
initialize slave state if an incorrect state actually happens.
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
drivers/i2c/busses/i2c-aspeed.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 8dc9161ced38..e9312d90ff7d 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -311,9 +311,13 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
case ASPEED_I2C_SLAVE_STOP:
i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
break;
+ case ASPEED_I2C_SLAVE_START:
+ /* Slave was just started. Waiting for the next irq. */;
+ break;
default:
- dev_err(bus->dev, "unhandled slave_state: %d\n",
+ dev_err(bus->dev, "unknown slave_state: %d\n",
bus->slave_state);
+ bus->slave_state = ASPEED_I2C_SLAVE_STOP;
break;
}
--
2.19.1
^ permalink raw reply related
* [PATCH i2c-next v9 5/5] i2c: aspeed: Add bus idle waiting logic for multi-master use cases
From: Jae Hyun Yoo @ 2018-11-01 18:09 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <201811020154.eziM0emr%fengguang.wu@intel.com>
On 11/1/2018 10:44 AM, kbuild test robot wrote:
> Hi Jae,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on wsa/i2c/for-next]
> [also build test WARNING on v4.19 next-20181101]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Jae-Hyun-Yoo/dt-bindings-i2c-Add-bus-timeout-ms-and-retries-properties-as-common-optional/20181031-051152
> base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
> config: i386-allyesconfig (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
> http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
>
> All warnings (new ones prefixed by >>):
>
> drivers/i2c/busses/i2c-aspeed.c: In function 'aspeed_i2c_master_xfer':
>>> drivers/i2c/busses/i2c-aspeed.c:624:210: warning: 'timeout' may be used uninitialized in this function [-Wmaybe-uninitialized]
> if (time_after(jiffies, timeout))
> ^
> drivers/i2c/busses/i2c-aspeed.c:607:16: note: 'timeout' was declared here
> unsigned long timeout;
> ^~~~~~~
>
> vim +/timeout +624 drivers/i2c/busses/i2c-aspeed.c
>
> 604
> 605 static int aspeed_i2c_check_bus_busy(struct aspeed_i2c_bus *bus)
> 606 {
> 607 unsigned long timeout;
> 608
> 609 if (bus->multi_master) {
> 610 might_sleep();
> 611 /* Initialize it only when multi_master is set */
> 612 timeout = jiffies + bus->adap.timeout;
The 'timeout' variable will be initialized only when bus->multi_master
is true to save some OPs in case this variable isn't used. The saved OPs
are not expensive but it makes a meaning because this function is called
very frequently on every master transfer.
> 613 }
> 614
> 615 for (;;) {
> 616 if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
> 617 ASPEED_I2CD_BUS_BUSY_STS))
> 618 #if IS_ENABLED(CONFIG_I2C_SLAVE)
> 619 if (bus->slave_state == ASPEED_I2C_SLAVE_STOP)
> 620 #endif
> 621 return 0;
> 622 if (!bus->multi_master)
> 623 break;
If bus->multi_master is false then it doesn't fall through so the
'timeout' variable in below will not be used when itself is
uninitialized. Also, bus->multi_master is fixed at probing time and it
doesn't change at runtime so the warning case never happens.
-Jae
> > 624 if (time_after(jiffies, timeout))
> 625 break;
> 626 usleep_range((ASPEED_I2C_BUS_BUSY_CHECK_INTERVAL_US >> 2) + 1,
> 627 ASPEED_I2C_BUS_BUSY_CHECK_INTERVAL_US);
> 628 }
> 629
> 630 return aspeed_i2c_recover_bus(bus);
> 631 }
> 632
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
^ permalink raw reply
* [PATCH v2] clocksource/drivers/fttmr010: fix invalid interrupt register access
From: Tao Ren @ 2018-11-05 18:43 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CACRpkdbbUO9_VOtnZL+Xf_0nTTgSruaaPOLnvZ4M+NQfPLzvWQ@mail.gmail.com>
On 10/7/18, 2:03 PM, "Linus Walleij" <linus.walleij@linaro.org> wrote:
>
> On Wed, Oct 3, 2018 at 11:54 PM Tao Ren <taoren@fb.com> wrote:
>
>> TIMER_INTR_MASK register (Base Address of Timer + 0x38) is not designed
>> for masking interrupts on ast2500 chips, and it's not even listed in
>> ast2400 datasheet, so it's not safe to access TIMER_INTR_MASK on aspeed
>> chips.
>>
>> Similarly, TIMER_INTR_STATE register (Base Address of Timer + 0x34) is
>> not interrupt status register on ast2400 and ast2500 chips. Although
>> there is no side effect to reset the register in fttmr010_common_init(),
>> it's just misleading to do so.
>>
>> Besides, "count_down" is renamed to "is_aspeed" in "fttmr010" structure,
>> and more comments are added so the code is more readble.
>>
>> Signed-off-by: Tao Ren <taoren@fb.com>
>> ---
>> Changes in v2:
>> - "count_down" is renamed to "is_aspeed" in "fttmr010" structure.
>> - more comments are added to make the code more readable.
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Hi Daniel / Thomas,
Any further comments on the patch? Or is there anything needed from my side?
Thanks,
Tao Ren
^ permalink raw reply
* [PATCH v2] clocksource/drivers/fttmr010: fix invalid interrupt register access
From: Daniel Lezcano @ 2018-11-05 18:50 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <BC1ADE3F-F31F-4E1E-A6EE-3BD83C5F2634@fb.com>
On 05/11/2018 19:43, Tao Ren wrote:
> On 10/7/18, 2:03 PM, "Linus Walleij" <linus.walleij@linaro.org> wrote:
>>
>> On Wed, Oct 3, 2018 at 11:54 PM Tao Ren <taoren@fb.com> wrote:
>>
>>> TIMER_INTR_MASK register (Base Address of Timer + 0x38) is not designed
>>> for masking interrupts on ast2500 chips, and it's not even listed in
>>> ast2400 datasheet, so it's not safe to access TIMER_INTR_MASK on aspeed
>>> chips.
>>>
>>> Similarly, TIMER_INTR_STATE register (Base Address of Timer + 0x34) is
>>> not interrupt status register on ast2400 and ast2500 chips. Although
>>> there is no side effect to reset the register in fttmr010_common_init(),
>>> it's just misleading to do so.
>>>
>>> Besides, "count_down" is renamed to "is_aspeed" in "fttmr010" structure,
>>> and more comments are added so the code is more readble.
>>>
>>> Signed-off-by: Tao Ren <taoren@fb.com>
>>> ---
>>> Changes in v2:
>>> - "count_down" is renamed to "is_aspeed" in "fttmr010" structure.
>>> - more comments are added to make the code more readable.
>>
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> Hi Daniel / Thomas,
>
> Any further comments on the patch? Or is there anything needed from my side?
Oh right, sorry. Should it go to stable also ? Is there a Fixes tag it
can apply ?
--
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* [PATCH v2] clocksource/drivers/fttmr010: fix invalid interrupt register access
From: Tao Ren @ 2018-11-05 19:00 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <279e6a06-3f2f-e736-f28e-4d099729a517@linaro.org>
On 11/5/18, 10:51 AM, "Daniel Lezcano" <daniel.lezcano@linaro.org> wrote:
> Oh right, sorry. Should it go to stable also ? Is there a Fixes tag it can apply ?
Personally I don't think it needs to go to stable, because I don't see any functional failures caused by this invalid register access.
Thanks,
Tao Ren
^ permalink raw reply
* [PATCH v2 1/2] ARM: dts: Add Facebook BMC flash layout
From: Tao Ren @ 2018-11-06 4:28 UTC (permalink / raw)
To: linux-aspeed
This is the layout used by Facebook BMC systems. It describes the fixed
flash layout of a 32MB mtd device.
Signed-off-by: Tao Ren <taoren@fb.com>
---
Changes since v1:
- adding facebook copyright.
.../boot/dts/facebook-bmc-flash-layout.dtsi | 33 +++++++++++++++++++
1 file changed, 33 insertions(+)
create mode 100644 arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi
diff --git a/arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi b/arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi
new file mode 100644
index 000000000000..94039d7b7de5
--- /dev/null
+++ b/arch/arm/boot/dts/facebook-bmc-flash-layout.dtsi
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2018 Facebook Inc.
+
+partitions {
+ compatible = "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ u-boot at 0 {
+ reg = <0x0 0x60000>;
+ label = "u-boot";
+ };
+
+ u-boot-env at 60000 {
+ reg = <0x60000 0x20000>;
+ label = "env";
+ };
+
+ fit at 80000 {
+ reg = <0x80000 0x1b80000>;
+ label = "fit";
+ };
+
+ data0 at 1c00000 {
+ reg = <0x1c00000 0x400000>;
+ label = "data0";
+ };
+
+ flash0 at 0 {
+ reg = <0x0 0x2000000>;
+ label = "flash0";
+ };
+};
--
2.17.1
^ permalink raw reply related
* [PATCH v2 2/2] ARM: dts: aspeed: Add Facebook Backpack-CMM BMC
From: Tao Ren @ 2018-11-06 4:28 UTC (permalink / raw)
To: linux-aspeed
Add initial version of device tree file for Facebook Backpack CMM
(Chasis Management Module) ast2500 BMC.
Note: I2C devices on Backpack Line Cards and Fabric Cards are not
listed in the device tree file because Line/Fabric Cards may be
unplugged.
Signed-off-by: Tao Ren <taoren@fb.com>
---
Changes since v1:
- including "aspeed-bmc-facebook-cmm.dtb" in Makefile.
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts | 317 ++++++++++++++++++
2 files changed, 318 insertions(+)
create mode 100644 arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index a7c313bfe490..2239e6a30d7a 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1200,6 +1200,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
aspeed-ast2500-evb.dtb \
aspeed-bmc-arm-centriq2400-rep.dtb \
aspeed-bmc-arm-stardragon4800-rep2.dtb \
+ aspeed-bmc-facebook-cmm.dtb \
aspeed-bmc-facebook-tiogapass.dtb \
aspeed-bmc-intel-s2600wf.dtb \
aspeed-bmc-opp-lanyang.dtb \
diff --git a/arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts b/arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts
new file mode 100644
index 000000000000..a259d8a2426a
--- /dev/null
+++ b/arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts
@@ -0,0 +1,317 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2018 Facebook Inc.
+/dts-v1/;
+
+#include "aspeed-g5.dtsi"
+
+/ {
+ model = "Facebook Backpack CMM BMC";
+ compatible = "facebook,cmm-bmc", "aspeed,ast2500";
+
+ aliases {
+ /*
+ * Override the default uart aliases to avoid breaking
+ * the legacy applications.
+ */
+ serial0 = &uart5;
+ serial1 = &uart1;
+ serial2 = &uart3;
+ serial3 = &uart4;
+
+ /*
+ * Hardcode the bus number of i2c switches' channels to
+ * avoid breaking the legacy applications.
+ */
+ i2c16 = &imux16;
+ i2c17 = &imux17;
+ i2c18 = &imux18;
+ i2c19 = &imux19;
+ i2c20 = &imux20;
+ i2c21 = &imux21;
+ i2c22 = &imux22;
+ i2c23 = &imux23;
+ i2c24 = &imux24;
+ i2c25 = &imux25;
+ i2c26 = &imux26;
+ i2c27 = &imux27;
+ i2c28 = &imux28;
+ i2c29 = &imux29;
+ i2c30 = &imux30;
+ i2c31 = &imux31;
+ i2c32 = &imux32;
+ i2c33 = &imux33;
+ i2c34 = &imux34;
+ i2c35 = &imux35;
+ i2c36 = &imux36;
+ i2c37 = &imux37;
+ i2c38 = &imux38;
+ i2c39 = &imux39;
+ };
+
+ chosen {
+ stdout-path = &uart1;
+ bootargs = "console=ttyS1,9600n8 root=/dev/ram rw earlyprintk";
+ };
+
+ memory at 80000000 {
+ reg = <0x80000000 0x20000000>;
+ };
+};
+
+/*
+ * Update reset type to "system" (full chip) to fix warm reboot hang issue
+ * when reset type is set to default ("soc", gated by reset mask registers).
+ */
+&wdt1 {
+ status = "okay";
+ aspeed,reset-type = "system";
+};
+
+/*
+ * wdt2 is not used by Backpack CMM.
+ */
+&wdt2 {
+ status = "disabled";
+};
+
+&fmc {
+ status = "okay";
+ flash at 0 {
+ status = "okay";
+ m25p,fast-read;
+ label = "bmc";
+#include "facebook-bmc-flash-layout.dtsi"
+ };
+};
+
+&uart1 {
+ status = "okay";
+};
+
+&uart3 {
+ status = "okay";
+};
+
+&uart4 {
+ status = "okay";
+};
+
+&uart5 {
+ status = "okay";
+};
+
+&mac1 {
+ status = "okay";
+ no-hw-checksum;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_rgmii2_default &pinctrl_mdio2_default>;
+};
+
+&i2c0 {
+ status = "okay";
+};
+
+&i2c1 {
+ status = "okay";
+
+ i2c-switch at 77 {
+ compatible = "nxp,pca9548";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x77>;
+
+ imux16: i2c at 0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ };
+
+ imux17: i2c at 1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+ };
+
+ imux18: i2c at 2 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <2>;
+ };
+
+ imux19: i2c at 3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <3>;
+ };
+
+ imux20: i2c at 4 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <4>;
+ };
+
+ imux21: i2c at 5 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <5>;
+ };
+
+ imux22: i2c at 6 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <6>;
+ };
+
+ imux23: i2c at 7 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <7>;
+ };
+ };
+};
+
+&i2c2 {
+ status = "okay";
+
+ i2c-switch at 71 {
+ compatible = "nxp,pca9548";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x71>;
+
+ imux24: i2c at 0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ };
+
+ imux25: i2c at 1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+ };
+
+ imux26: i2c at 2 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <2>;
+ };
+
+ imux27: i2c at 3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <3>;
+ };
+
+ imux28: i2c at 4 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <4>;
+ };
+
+ imux29: i2c at 5 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <5>;
+ };
+
+ imux30: i2c at 6 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <6>;
+ };
+
+ imux31: i2c at 7 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <7>;
+ };
+ };
+};
+
+&i2c3 {
+ status = "okay";
+};
+
+&i2c4 {
+ status = "okay";
+};
+
+&i2c5 {
+ status = "okay";
+};
+
+&i2c6 {
+ status = "okay";
+};
+
+&i2c7 {
+ status = "okay";
+};
+
+&i2c8 {
+ status = "okay";
+
+ i2c-switch at 77 {
+ compatible = "nxp,pca9548";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x77>;
+
+ imux32: i2c at 0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ };
+
+ imux33: i2c at 1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+ };
+
+ imux34: i2c at 2 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <2>;
+ };
+
+ imux35: i2c at 3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <3>;
+ };
+
+ imux36: i2c at 4 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <4>;
+ };
+
+ imux37: i2c at 5 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <5>;
+ };
+
+ imux38: i2c at 6 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <6>;
+ };
+
+ imux39: i2c at 7 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <7>;
+ };
+ };
+};
+
+&i2c13 {
+ status = "okay";
+};
+
+&adc {
+ status = "okay";
+};
--
2.17.1
^ permalink raw reply related
* [PATCH v2 0/5] serial: Finish sysrq on qcom_geni; fix sysrq vs. lockdep on 8250
From: Andy Shevchenko @ 2018-11-07 18:23 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181030221107.79758-1-dianders@chromium.org>
On Tue, Oct 30, 2018 at 03:11:02PM -0700, Douglas Anderson wrote:
> I started out this series trying to make sysrq work over the serial
> console on qcom_geni_serial, then fell into a rat's nest.
>
> To solve the deadlock I faced when enabling sysrq I tried to borrow
> code from '8250_port.c' which avoided grabbing the port lock in
> console_write(). ...but since these days I try to run with lockdep on
> all the time, I found it caused an annoying lockdep splat (which I
> also reproduced on my rk3399 board). ...so I instead changed my
> qcom_geni_serial solution to borrow code from 'msm_serial.c'
>
> I wasn't super happy with the solution in 'msm_serial.c' though. I
> don't like releasing the spinlock there. Not only is it ugly but it
> means we are unlocking / re-locking _all the time_ even though sysrq
> characters are rare. ...so I came up with what I think is a better
> solution and then implemented it for qcom_geni_serial.
>
> Since I had a good way to test 8250-based UARTs, I also fixed that
> driver to use my new method. When doing so, I ran into a missing
> include in serial_core.h. NOTE: I didn't have a way to test
> msm_serial.c at all, so I didn't switch that (or all other serial
> drivers for that matter) to the new method.
>
> NOTE: from a serial point of view v2 is the same as v1 but I've
> removed the extra kgdb-related patches and made it obvious that this
> is really for all sysrq, not just kgdb. I've also generally tried to
> curate the CCs more properly.
It seems your forgot console people to Cc.
>
>
> Douglas Anderson (5):
> serial: qcom_geni_serial: Finish supporting sysrq
> serial: core: Allow processing sysrq at port unlock time
> serial: qcom_geni_serial: Process sysrq at port unlock time
> serial: core: Include console.h from serial_core.h
> serial: 8250: Process sysrq at port unlock time
>
> drivers/tty/serial/8250/8250_aspeed_vuart.c | 6 +++-
> drivers/tty/serial/8250/8250_fsl.c | 6 +++-
> drivers/tty/serial/8250/8250_omap.c | 6 +++-
> drivers/tty/serial/8250/8250_port.c | 8 ++---
> drivers/tty/serial/qcom_geni_serial.c | 10 ++++--
> include/linux/serial_core.h | 38 ++++++++++++++++++++-
> 6 files changed, 63 insertions(+), 11 deletions(-)
>
> --
> 2.19.1.568.g152ad8e336-goog
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v2 0/5] serial: Finish sysrq on qcom_geni; fix sysrq vs. lockdep on 8250
From: Doug Anderson @ 2018-11-07 19:26 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20181107182349.GP10650@smile.fi.intel.com>
Hi,
On Wed, Nov 7, 2018 at 10:23 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Oct 30, 2018 at 03:11:02PM -0700, Douglas Anderson wrote:
> > I started out this series trying to make sysrq work over the serial
> > console on qcom_geni_serial, then fell into a rat's nest.
> >
> > To solve the deadlock I faced when enabling sysrq I tried to borrow
> > code from '8250_port.c' which avoided grabbing the port lock in
> > console_write(). ...but since these days I try to run with lockdep on
> > all the time, I found it caused an annoying lockdep splat (which I
> > also reproduced on my rk3399 board). ...so I instead changed my
> > qcom_geni_serial solution to borrow code from 'msm_serial.c'
> >
> > I wasn't super happy with the solution in 'msm_serial.c' though. I
> > don't like releasing the spinlock there. Not only is it ugly but it
> > means we are unlocking / re-locking _all the time_ even though sysrq
> > characters are rare. ...so I came up with what I think is a better
> > solution and then implemented it for qcom_geni_serial.
> >
> > Since I had a good way to test 8250-based UARTs, I also fixed that
> > driver to use my new method. When doing so, I ran into a missing
> > include in serial_core.h. NOTE: I didn't have a way to test
> > msm_serial.c at all, so I didn't switch that (or all other serial
> > drivers for that matter) to the new method.
> >
> > NOTE: from a serial point of view v2 is the same as v1 but I've
> > removed the extra kgdb-related patches and made it obvious that this
> > is really for all sysrq, not just kgdb. I've also generally tried to
> > curate the CCs more properly.
>
> It seems your forgot console people to Cc.
Can you be more specific, please? Which section of the "MAINTAINERS"
file should I be looking at for the "console" you are thinking of?
Certainly there are lots of hits for "console" in MAINTAINERS but I
don't think I see any that are relevant that I missed. Grepping:
$ grep -i console MAINTAINERS
HYPERVISOR VIRTUAL CONSOLE DRIVER
F: drivers/video/console/sti*
PCDP - PRIMARY CONSOLE AND DEBUG PORT
SGI SN-IA64 (Altix) SERIAL CONSOLE DRIVER
STAGING - SPEAKUP CONSOLE SPEECH DRIVER
VIRTIO CONSOLE DRIVER
F: drivers/char/virtio_console.c
F: include/linux/virtio_console.h
F: include/uapi/linux/virtio_console.h
...none of those seem relevant upon first glance but I'm happy to
stand corrected.
Ah! Based on who you added to the CC list I guess you meant to CC
"printk" folks?
PRINTK
M: Petr Mladek <pmladek@suse.com>
M: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
R: Steven Rostedt <rostedt@goodmis.org>
S: Maintained
F: kernel/printk/
F: include/linux/printk.h
I'd be happy to CC those folks on future spins (if there are any).
I'm not convinced that these patches are directly relevant to the
printk subsystem, but I'm always happy for more people to have a
chance to review patches.
Hopefully anyone who needs this patch can find it on one of the
relevant mailing lists. I screwed up and missed LKML this time
around, but there are plenty of other mailing lists here that it could
be found on. If requested I'm also happy to re-post the same series
adding those 3 people if that's what everyone wants.
-Doug
^ permalink raw reply
* [PATCH v2 0/5] serial: Finish sysrq on qcom_geni; fix sysrq vs. lockdep on 8250
From: Andy Shevchenko @ 2018-11-07 19:54 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CAD=FV=Xyra4+cLB0J64+SWqCF=YKdEJpAuybQboYxOcfrdTeQg@mail.gmail.com>
On Wed, Nov 07, 2018 at 11:26:56AM -0800, Doug Anderson wrote:
> Hi,
>
> On Wed, Nov 7, 2018 at 10:23 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Tue, Oct 30, 2018 at 03:11:02PM -0700, Douglas Anderson wrote:
> > > I started out this series trying to make sysrq work over the serial
> > > console on qcom_geni_serial, then fell into a rat's nest.
> > >
> > > To solve the deadlock I faced when enabling sysrq I tried to borrow
> > > code from '8250_port.c' which avoided grabbing the port lock in
> > > console_write(). ...but since these days I try to run with lockdep on
> > > all the time, I found it caused an annoying lockdep splat (which I
> > > also reproduced on my rk3399 board). ...so I instead changed my
> > > qcom_geni_serial solution to borrow code from 'msm_serial.c'
> > >
> > > I wasn't super happy with the solution in 'msm_serial.c' though. I
> > > don't like releasing the spinlock there. Not only is it ugly but it
> > > means we are unlocking / re-locking _all the time_ even though sysrq
> > > characters are rare. ...so I came up with what I think is a better
> > > solution and then implemented it for qcom_geni_serial.
> > >
> > > Since I had a good way to test 8250-based UARTs, I also fixed that
> > > driver to use my new method. When doing so, I ran into a missing
> > > include in serial_core.h. NOTE: I didn't have a way to test
> > > msm_serial.c at all, so I didn't switch that (or all other serial
> > > drivers for that matter) to the new method.
> > >
> > > NOTE: from a serial point of view v2 is the same as v1 but I've
> > > removed the extra kgdb-related patches and made it obvious that this
> > > is really for all sysrq, not just kgdb. I've also generally tried to
> > > curate the CCs more properly.
> >
> > It seems your forgot console people to Cc.
>
> Can you be more specific, please? Which section of the "MAINTAINERS"
> file should I be looking at for the "console" you are thinking of?
I have added them to Cc list: Petr, Sergey, and Steven.
> Certainly there are lots of hits for "console" in MAINTAINERS but I
> don't think I see any that are relevant that I missed. Grepping:
> Ah! Based on who you added to the CC list I guess you meant to CC
> "printk" folks?
Correct.
> PRINTK
> M: Petr Mladek <pmladek@suse.com>
> M: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> R: Steven Rostedt <rostedt@goodmis.org>
> S: Maintained
> F: kernel/printk/
> F: include/linux/printk.h
> I'd be happy to CC those folks on future spins (if there are any).
> I'm not convinced that these patches are directly relevant to the
> printk subsystem, but I'm always happy for more people to have a
> chance to review patches.
If you look retrospectively in the mailing lists, you can find that they are
doing most of the console core work, which I believe includes SysRq behaviour.
So, don't be confused their names are listed under PRINTK.
> Hopefully anyone who needs this patch can find it on one of the
> relevant mailing lists. I screwed up and missed LKML this time
> around, but there are plenty of other mailing lists here that it could
> be found on. If requested I'm also happy to re-post the same series
> adding those 3 people if that's what everyone wants.
--
With Best Regards,
Andy Shevchenko
^ 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