* Re: [RFC lora-next 5/5] HACK: net: lora: sx130x: Add PicoCell gateway shim for cdc-acm
From: Johan Hovold @ 2019-01-07 15:28 UTC (permalink / raw)
To: Andreas Färber
Cc: Rob Herring, linux-lpwan, linux-serial, Linux USB List,
devicetree, linux-kernel@vger.kernel.org, Johan Hovold,
David S. Miller, Oliver Neukum, Greg Kroah-Hartman, netdev,
linux-clk
In-Reply-To: <9d97fec5-31b0-8717-cd10-de4f36d1cf05@suse.de>
On Sat, Jan 05, 2019 at 12:43:48AM +0100, Andreas Färber wrote:
> Hi Rob,
>
> Am 04.01.19 um 18:07 schrieb Rob Herring:
> > On Fri, Jan 4, 2019 at 5:21 AM Andreas Färber <afaerber@suse.de> wrote:
> >>
> >> Ignore our device in cdc-acm probing and add a new driver for it,
> >> dispatching to cdc-acm for the actual implementation.
> >>
> >> WARNING: It is likely that this VID/PID is in use for unrelated devices.
> >> Only the Product string hints what this really is in current v0.2.1.
> >> Previous code v0.2.0 was using a Semtech VID/PID, but no card shipping
> >> with such firmware is known to me.
> >>
> >> While this may seem unorthodox, no internals of the driver are accessed,
> >> just the set of driver callbacks is duplicated as shim.
> >>
> >> Use this shim construct to fake DT nodes for serdev on probe.
> >> For testing purposes these nodes do not have a parent yet.
> >> This results in two "Error -2 creating of_node link" warnings on probe.
> >
> > It looks like the DT is pretty static. Rather than creating the nodes
> > at run-time, can't you create a dts file and build that into your
> > module.
>
> Heh, if that were the only issue with this patch... ;)
My thoughts exactly. ;)
This clearly is too much of a hack, but maintaining serdev compatible
information in the corresponding tty drivers is probably what we'll want
to do.
When the tty driver binds and registers its ports with tty core, we can
could match again on the USB descriptors, but since the device has
already been matched, we can just pass the equivalent of a compatible
string, or more generally dt-fragment, instead.
Without having had time to look into it myself yet, this sounds like
something we should be using the new software nodes for (software
generated fw nodes). That way, we don't depend on CONFIG_OF either.
Johan
^ permalink raw reply
* [PATCH v2 0/4] tty: serial: qcom_geni_serial: Assorted cleanups
From: Ryan Case @ 2019-01-08 1:58 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Evan Green, Doug Anderson, linux-kernel, linux-serial,
Stephen Boyd, Ryan Case
This is a series of cleanups for issues raised in prior reviews that
were unrelated to the patches at hand.
Changes in v2:
- Updated commit messages
- Removed CONSOLE from UART_CONSOLE_RX_WM
- Coalesced lines where possible
- Updated missed rxstale variable
Ryan Case (4):
tty: serial: qcom_geni_serial: Remove use of *_relaxed() and mb()
tty: serial: qcom_geni_serial: Remove set_rfr_wm() and related
variables
tty: serial: qcom_geni_serial: Remove xfer_mode variable
tty: serial: qcom_geni_serial: Use u32 for register variables
drivers/tty/serial/qcom_geni_serial.c | 279 ++++++++++----------------
1 file changed, 106 insertions(+), 173 deletions(-)
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply
* [PATCH v2 1/4] tty: serial: qcom_geni_serial: Remove use of *_relaxed() and mb()
From: Ryan Case @ 2019-01-08 1:58 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Evan Green, Doug Anderson, linux-kernel, linux-serial,
Stephen Boyd, Ryan Case
In-Reply-To: <20190108015838.166271-1-ryandcase@chromium.org>
A frequent side comment has been to remove the use of writel_relaxed,
readl_relaxed, and mb. This reduces driver complexity and the _relaxed
variants were not known to provide any noticeable performance benefit.
Signed-off-by: Ryan Case <ryandcase@chromium.org>
Reviewed-by: Evan Green <evgreen@chromium.org>
---
Changes in v2:
- Expanded commit message
- Coalesced lines where possible
drivers/tty/serial/qcom_geni_serial.c | 191 +++++++++++---------------
1 file changed, 80 insertions(+), 111 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 2b1e73193dad..e2cb3f7d1a7c 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -230,7 +230,7 @@ static unsigned int qcom_geni_serial_get_mctrl(struct uart_port *uport)
if (uart_console(uport) || !uart_cts_enabled(uport)) {
mctrl |= TIOCM_CTS;
} else {
- geni_ios = readl_relaxed(uport->membase + SE_GENI_IOS);
+ geni_ios = readl(uport->membase + SE_GENI_IOS);
if (!(geni_ios & IO2_DATA_IN))
mctrl |= TIOCM_CTS;
}
@@ -248,7 +248,7 @@ static void qcom_geni_serial_set_mctrl(struct uart_port *uport,
if (!(mctrl & TIOCM_RTS))
uart_manual_rfr = UART_MANUAL_RFR_EN | UART_RFR_NOT_READY;
- writel_relaxed(uart_manual_rfr, uport->membase + SE_UART_MANUAL_RFR);
+ writel(uart_manual_rfr, uport->membase + SE_UART_MANUAL_RFR);
}
static const char *qcom_geni_serial_get_type(struct uart_port *uport)
@@ -277,9 +277,6 @@ static bool qcom_geni_serial_poll_bit(struct uart_port *uport,
unsigned int fifo_bits;
unsigned long timeout_us = 20000;
- /* Ensure polling is not re-ordered before the prior writes/reads */
- mb();
-
if (uport->private_data) {
port = to_dev_port(uport, uport);
baud = port->baud;
@@ -299,7 +296,7 @@ static bool qcom_geni_serial_poll_bit(struct uart_port *uport,
*/
timeout_us = DIV_ROUND_UP(timeout_us, 10) * 10;
while (timeout_us) {
- reg = readl_relaxed(uport->membase + offset);
+ reg = readl(uport->membase + offset);
if ((bool)(reg & field) == set)
return true;
udelay(10);
@@ -312,7 +309,7 @@ static void qcom_geni_serial_setup_tx(struct uart_port *uport, u32 xmit_size)
{
u32 m_cmd;
- writel_relaxed(xmit_size, uport->membase + SE_UART_TX_TRANS_LEN);
+ writel(xmit_size, uport->membase + SE_UART_TX_TRANS_LEN);
m_cmd = UART_START_TX << M_OPCODE_SHFT;
writel(m_cmd, uport->membase + SE_GENI_M_CMD0);
}
@@ -325,13 +322,13 @@ static void qcom_geni_serial_poll_tx_done(struct uart_port *uport)
done = qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
M_CMD_DONE_EN, true);
if (!done) {
- writel_relaxed(M_GENI_CMD_ABORT, uport->membase +
+ writel(M_GENI_CMD_ABORT, uport->membase +
SE_GENI_M_CMD_CTRL_REG);
irq_clear |= M_CMD_ABORT_EN;
qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
M_CMD_ABORT_EN, true);
}
- writel_relaxed(irq_clear, uport->membase + SE_GENI_M_IRQ_CLEAR);
+ writel(irq_clear, uport->membase + SE_GENI_M_IRQ_CLEAR);
}
static void qcom_geni_serial_abort_rx(struct uart_port *uport)
@@ -341,8 +338,8 @@ static void qcom_geni_serial_abort_rx(struct uart_port *uport)
writel(S_GENI_CMD_ABORT, uport->membase + SE_GENI_S_CMD_CTRL_REG);
qcom_geni_serial_poll_bit(uport, SE_GENI_S_CMD_CTRL_REG,
S_GENI_CMD_ABORT, false);
- writel_relaxed(irq_clear, uport->membase + SE_GENI_S_IRQ_CLEAR);
- writel_relaxed(FORCE_DEFAULT, uport->membase + GENI_FORCE_DEFAULT_REG);
+ writel(irq_clear, uport->membase + SE_GENI_S_IRQ_CLEAR);
+ writel(FORCE_DEFAULT, uport->membase + GENI_FORCE_DEFAULT_REG);
}
#ifdef CONFIG_CONSOLE_POLL
@@ -351,19 +348,13 @@ static int qcom_geni_serial_get_char(struct uart_port *uport)
u32 rx_fifo;
u32 status;
- status = readl_relaxed(uport->membase + SE_GENI_M_IRQ_STATUS);
- writel_relaxed(status, uport->membase + SE_GENI_M_IRQ_CLEAR);
-
- status = readl_relaxed(uport->membase + SE_GENI_S_IRQ_STATUS);
- writel_relaxed(status, uport->membase + SE_GENI_S_IRQ_CLEAR);
+ status = readl(uport->membase + SE_GENI_M_IRQ_STATUS);
+ writel(status, uport->membase + SE_GENI_M_IRQ_CLEAR);
- /*
- * Ensure the writes to clear interrupts is not re-ordered after
- * reading the data.
- */
- mb();
+ status = readl(uport->membase + SE_GENI_S_IRQ_STATUS);
+ writel(status, uport->membase + SE_GENI_S_IRQ_CLEAR);
- status = readl_relaxed(uport->membase + SE_GENI_RX_FIFO_STATUS);
+ status = readl(uport->membase + SE_GENI_RX_FIFO_STATUS);
if (!(status & RX_FIFO_WC_MSK))
return NO_POLL_CHAR;
@@ -376,13 +367,12 @@ static void qcom_geni_serial_poll_put_char(struct uart_port *uport,
{
struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
- writel_relaxed(port->tx_wm, uport->membase + SE_GENI_TX_WATERMARK_REG);
+ writel(port->tx_wm, uport->membase + SE_GENI_TX_WATERMARK_REG);
qcom_geni_serial_setup_tx(uport, 1);
WARN_ON(!qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
M_TX_FIFO_WATERMARK_EN, true));
- writel_relaxed(c, uport->membase + SE_GENI_TX_FIFOn);
- writel_relaxed(M_TX_FIFO_WATERMARK_EN, uport->membase +
- SE_GENI_M_IRQ_CLEAR);
+ writel(c, uport->membase + SE_GENI_TX_FIFOn);
+ writel(M_TX_FIFO_WATERMARK_EN, uport->membase + SE_GENI_M_IRQ_CLEAR);
qcom_geni_serial_poll_tx_done(uport);
}
#endif
@@ -390,7 +380,7 @@ static void qcom_geni_serial_poll_put_char(struct uart_port *uport,
#ifdef CONFIG_SERIAL_QCOM_GENI_CONSOLE
static void qcom_geni_serial_wr_char(struct uart_port *uport, int ch)
{
- writel_relaxed(ch, uport->membase + SE_GENI_TX_FIFOn);
+ writel(ch, uport->membase + SE_GENI_TX_FIFOn);
}
static void
@@ -409,7 +399,7 @@ __qcom_geni_serial_console_write(struct uart_port *uport, const char *s,
bytes_to_send++;
}
- writel_relaxed(DEF_TX_WM, uport->membase + SE_GENI_TX_WATERMARK_REG);
+ writel(DEF_TX_WM, uport->membase + SE_GENI_TX_WATERMARK_REG);
qcom_geni_serial_setup_tx(uport, bytes_to_send);
for (i = 0; i < count; ) {
size_t chars_to_write = 0;
@@ -427,7 +417,7 @@ __qcom_geni_serial_console_write(struct uart_port *uport, const char *s,
chars_to_write = min_t(size_t, count - i, avail / 2);
uart_console_write(uport, s + i, chars_to_write,
qcom_geni_serial_wr_char);
- writel_relaxed(M_TX_FIFO_WATERMARK_EN, uport->membase +
+ writel(M_TX_FIFO_WATERMARK_EN, uport->membase +
SE_GENI_M_IRQ_CLEAR);
i += chars_to_write;
}
@@ -456,7 +446,7 @@ static void qcom_geni_serial_console_write(struct console *co, const char *s,
else
spin_lock_irqsave(&uport->lock, flags);
- geni_status = readl_relaxed(uport->membase + SE_GENI_STATUS);
+ geni_status = readl(uport->membase + SE_GENI_STATUS);
/* Cancel the current write to log the fault */
if (!locked) {
@@ -466,11 +456,10 @@ static void qcom_geni_serial_console_write(struct console *co, const char *s,
geni_se_abort_m_cmd(&port->se);
qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
M_CMD_ABORT_EN, true);
- writel_relaxed(M_CMD_ABORT_EN, uport->membase +
+ writel(M_CMD_ABORT_EN, uport->membase +
SE_GENI_M_IRQ_CLEAR);
}
- writel_relaxed(M_CMD_CANCEL_EN, uport->membase +
- SE_GENI_M_IRQ_CLEAR);
+ writel(M_CMD_CANCEL_EN, uport->membase + SE_GENI_M_IRQ_CLEAR);
} else if ((geni_status & M_GENI_CMD_ACTIVE) && !port->tx_remaining) {
/*
* It seems we can't interrupt existing transfers if all data
@@ -479,9 +468,8 @@ static void qcom_geni_serial_console_write(struct console *co, const char *s,
qcom_geni_serial_poll_tx_done(uport);
if (uart_circ_chars_pending(&uport->state->xmit)) {
- irq_en = readl_relaxed(uport->membase +
- SE_GENI_M_IRQ_EN);
- writel_relaxed(irq_en | M_TX_FIFO_WATERMARK_EN,
+ irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
+ writel(irq_en | M_TX_FIFO_WATERMARK_EN,
uport->membase + SE_GENI_M_IRQ_EN);
}
}
@@ -585,12 +573,12 @@ static void qcom_geni_serial_start_tx(struct uart_port *uport)
if (!qcom_geni_serial_tx_empty(uport))
return;
- irq_en = readl_relaxed(uport->membase + SE_GENI_M_IRQ_EN);
+ irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
irq_en |= M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN;
- writel_relaxed(port->tx_wm, uport->membase +
+ writel(port->tx_wm, uport->membase +
SE_GENI_TX_WATERMARK_REG);
- writel_relaxed(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
+ writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
}
}
@@ -600,35 +588,28 @@ static void qcom_geni_serial_stop_tx(struct uart_port *uport)
u32 status;
struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
- irq_en = readl_relaxed(uport->membase + SE_GENI_M_IRQ_EN);
+ irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
irq_en &= ~M_CMD_DONE_EN;
if (port->xfer_mode == GENI_SE_FIFO) {
irq_en &= ~M_TX_FIFO_WATERMARK_EN;
- writel_relaxed(0, uport->membase +
+ writel(0, uport->membase +
SE_GENI_TX_WATERMARK_REG);
}
- writel_relaxed(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
- status = readl_relaxed(uport->membase + SE_GENI_STATUS);
+ writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
+ status = readl(uport->membase + SE_GENI_STATUS);
/* Possible stop tx is called multiple times. */
if (!(status & M_GENI_CMD_ACTIVE))
return;
- /*
- * Ensure cancel command write is not re-ordered before checking
- * the status of the Primary Sequencer.
- */
- mb();
-
geni_se_cancel_m_cmd(&port->se);
if (!qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
M_CMD_CANCEL_EN, true)) {
geni_se_abort_m_cmd(&port->se);
qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
M_CMD_ABORT_EN, true);
- writel_relaxed(M_CMD_ABORT_EN, uport->membase +
- SE_GENI_M_IRQ_CLEAR);
+ writel(M_CMD_ABORT_EN, uport->membase + SE_GENI_M_IRQ_CLEAR);
}
- writel_relaxed(M_CMD_CANCEL_EN, uport->membase + SE_GENI_M_IRQ_CLEAR);
+ writel(M_CMD_CANCEL_EN, uport->membase + SE_GENI_M_IRQ_CLEAR);
}
static void qcom_geni_serial_start_rx(struct uart_port *uport)
@@ -637,26 +618,20 @@ static void qcom_geni_serial_start_rx(struct uart_port *uport)
u32 status;
struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
- status = readl_relaxed(uport->membase + SE_GENI_STATUS);
+ status = readl(uport->membase + SE_GENI_STATUS);
if (status & S_GENI_CMD_ACTIVE)
qcom_geni_serial_stop_rx(uport);
- /*
- * Ensure setup command write is not re-ordered before checking
- * the status of the Secondary Sequencer.
- */
- mb();
-
geni_se_setup_s_cmd(&port->se, UART_START_READ, 0);
if (port->xfer_mode == GENI_SE_FIFO) {
- irq_en = readl_relaxed(uport->membase + SE_GENI_S_IRQ_EN);
+ irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
irq_en |= S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN;
- writel_relaxed(irq_en, uport->membase + SE_GENI_S_IRQ_EN);
+ writel(irq_en, uport->membase + SE_GENI_S_IRQ_EN);
- irq_en = readl_relaxed(uport->membase + SE_GENI_M_IRQ_EN);
+ irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
irq_en |= M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN;
- writel_relaxed(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
+ writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
}
}
@@ -668,31 +643,25 @@ static void qcom_geni_serial_stop_rx(struct uart_port *uport)
u32 irq_clear = S_CMD_DONE_EN;
if (port->xfer_mode == GENI_SE_FIFO) {
- irq_en = readl_relaxed(uport->membase + SE_GENI_S_IRQ_EN);
+ irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
irq_en &= ~(S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN);
- writel_relaxed(irq_en, uport->membase + SE_GENI_S_IRQ_EN);
+ writel(irq_en, uport->membase + SE_GENI_S_IRQ_EN);
- irq_en = readl_relaxed(uport->membase + SE_GENI_M_IRQ_EN);
+ irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
irq_en &= ~(M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN);
- writel_relaxed(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
+ writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
}
- status = readl_relaxed(uport->membase + SE_GENI_STATUS);
+ status = readl(uport->membase + SE_GENI_STATUS);
/* Possible stop rx is called multiple times. */
if (!(status & S_GENI_CMD_ACTIVE))
return;
- /*
- * Ensure cancel command write is not re-ordered before checking
- * the status of the Secondary Sequencer.
- */
- mb();
-
geni_se_cancel_s_cmd(&port->se);
qcom_geni_serial_poll_bit(uport, SE_GENI_S_CMD_CTRL_REG,
S_GENI_CMD_CANCEL, false);
- status = readl_relaxed(uport->membase + SE_GENI_STATUS);
- writel_relaxed(irq_clear, uport->membase + SE_GENI_S_IRQ_CLEAR);
+ status = readl(uport->membase + SE_GENI_STATUS);
+ writel(irq_clear, uport->membase + SE_GENI_S_IRQ_CLEAR);
if (status & S_GENI_CMD_ACTIVE)
qcom_geni_serial_abort_rx(uport);
}
@@ -706,7 +675,7 @@ static void qcom_geni_serial_handle_rx(struct uart_port *uport, bool drop)
u32 total_bytes;
struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
- status = readl_relaxed(uport->membase + SE_GENI_RX_FIFO_STATUS);
+ status = readl(uport->membase + SE_GENI_RX_FIFO_STATUS);
word_cnt = status & RX_FIFO_WC_MSK;
last_word_partial = status & RX_LAST;
last_word_byte_cnt = (status & RX_LAST_BYTE_VALID_MSK) >>
@@ -736,7 +705,7 @@ static void qcom_geni_serial_handle_tx(struct uart_port *uport, bool done,
unsigned int chunk;
int tail;
- status = readl_relaxed(uport->membase + SE_GENI_TX_FIFO_STATUS);
+ status = readl(uport->membase + SE_GENI_TX_FIFO_STATUS);
/* Complete the current tx command before taking newly added data */
if (active)
@@ -762,9 +731,9 @@ static void qcom_geni_serial_handle_tx(struct uart_port *uport, bool done,
qcom_geni_serial_setup_tx(uport, pending);
port->tx_remaining = pending;
- irq_en = readl_relaxed(uport->membase + SE_GENI_M_IRQ_EN);
+ irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
if (!(irq_en & M_TX_FIFO_WATERMARK_EN))
- writel_relaxed(irq_en | M_TX_FIFO_WATERMARK_EN,
+ writel(irq_en | M_TX_FIFO_WATERMARK_EN,
uport->membase + SE_GENI_M_IRQ_EN);
}
@@ -797,14 +766,14 @@ static void qcom_geni_serial_handle_tx(struct uart_port *uport, bool done,
* cleared it in qcom_geni_serial_isr it will have already reasserted
* so we must clear it again here after our writes.
*/
- writel_relaxed(M_TX_FIFO_WATERMARK_EN,
+ writel(M_TX_FIFO_WATERMARK_EN,
uport->membase + SE_GENI_M_IRQ_CLEAR);
out_write_wakeup:
if (!port->tx_remaining) {
- irq_en = readl_relaxed(uport->membase + SE_GENI_M_IRQ_EN);
+ irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
if (irq_en & M_TX_FIFO_WATERMARK_EN)
- writel_relaxed(irq_en & ~M_TX_FIFO_WATERMARK_EN,
+ writel(irq_en & ~M_TX_FIFO_WATERMARK_EN,
uport->membase + SE_GENI_M_IRQ_EN);
}
@@ -828,12 +797,12 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
return IRQ_NONE;
spin_lock_irqsave(&uport->lock, flags);
- m_irq_status = readl_relaxed(uport->membase + SE_GENI_M_IRQ_STATUS);
- s_irq_status = readl_relaxed(uport->membase + SE_GENI_S_IRQ_STATUS);
- geni_status = readl_relaxed(uport->membase + SE_GENI_STATUS);
- m_irq_en = readl_relaxed(uport->membase + SE_GENI_M_IRQ_EN);
- writel_relaxed(m_irq_status, uport->membase + SE_GENI_M_IRQ_CLEAR);
- writel_relaxed(s_irq_status, uport->membase + SE_GENI_S_IRQ_CLEAR);
+ m_irq_status = readl(uport->membase + SE_GENI_M_IRQ_STATUS);
+ s_irq_status = readl(uport->membase + SE_GENI_S_IRQ_STATUS);
+ geni_status = readl(uport->membase + SE_GENI_STATUS);
+ m_irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
+ writel(m_irq_status, uport->membase + SE_GENI_M_IRQ_CLEAR);
+ writel(s_irq_status, uport->membase + SE_GENI_S_IRQ_CLEAR);
if (WARN_ON(m_irq_status & M_ILLEGAL_CMD_EN))
goto out_unlock;
@@ -931,7 +900,7 @@ static int qcom_geni_serial_port_setup(struct uart_port *uport)
get_tx_fifo_size(port);
set_rfr_wm(port);
- writel_relaxed(rxstale, uport->membase + SE_UART_RX_STALE_CNT);
+ writel(rxstale, uport->membase + SE_UART_RX_STALE_CNT);
/*
* Make an unconditional cancel on the main sequencer to reset
* it else we could end up in data loss scenarios.
@@ -1035,10 +1004,10 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
ser_clk_cfg |= clk_div << CLK_DIV_SHFT;
/* parity */
- tx_trans_cfg = readl_relaxed(uport->membase + SE_UART_TX_TRANS_CFG);
- tx_parity_cfg = readl_relaxed(uport->membase + SE_UART_TX_PARITY_CFG);
- rx_trans_cfg = readl_relaxed(uport->membase + SE_UART_RX_TRANS_CFG);
- rx_parity_cfg = readl_relaxed(uport->membase + SE_UART_RX_PARITY_CFG);
+ tx_trans_cfg = readl(uport->membase + SE_UART_TX_TRANS_CFG);
+ tx_parity_cfg = readl(uport->membase + SE_UART_TX_PARITY_CFG);
+ rx_trans_cfg = readl(uport->membase + SE_UART_RX_TRANS_CFG);
+ rx_parity_cfg = readl(uport->membase + SE_UART_RX_PARITY_CFG);
if (termios->c_cflag & PARENB) {
tx_trans_cfg |= UART_TX_PAR_EN;
rx_trans_cfg |= UART_RX_PAR_EN;
@@ -1094,17 +1063,17 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
uart_update_timeout(uport, termios->c_cflag, baud);
if (!uart_console(uport))
- writel_relaxed(port->loopback,
+ writel(port->loopback,
uport->membase + SE_UART_LOOPBACK_CFG);
- writel_relaxed(tx_trans_cfg, uport->membase + SE_UART_TX_TRANS_CFG);
- writel_relaxed(tx_parity_cfg, uport->membase + SE_UART_TX_PARITY_CFG);
- writel_relaxed(rx_trans_cfg, uport->membase + SE_UART_RX_TRANS_CFG);
- writel_relaxed(rx_parity_cfg, uport->membase + SE_UART_RX_PARITY_CFG);
- writel_relaxed(bits_per_char, uport->membase + SE_UART_TX_WORD_LEN);
- writel_relaxed(bits_per_char, uport->membase + SE_UART_RX_WORD_LEN);
- writel_relaxed(stop_bit_len, uport->membase + SE_UART_TX_STOP_BIT_LEN);
- writel_relaxed(ser_clk_cfg, uport->membase + GENI_SER_M_CLK_CFG);
- writel_relaxed(ser_clk_cfg, uport->membase + GENI_SER_S_CLK_CFG);
+ writel(tx_trans_cfg, uport->membase + SE_UART_TX_TRANS_CFG);
+ writel(tx_parity_cfg, uport->membase + SE_UART_TX_PARITY_CFG);
+ writel(rx_trans_cfg, uport->membase + SE_UART_RX_TRANS_CFG);
+ writel(rx_parity_cfg, uport->membase + SE_UART_RX_PARITY_CFG);
+ writel(bits_per_char, uport->membase + SE_UART_TX_WORD_LEN);
+ writel(bits_per_char, uport->membase + SE_UART_RX_WORD_LEN);
+ writel(stop_bit_len, uport->membase + SE_UART_TX_STOP_BIT_LEN);
+ writel(ser_clk_cfg, uport->membase + GENI_SER_M_CLK_CFG);
+ writel(ser_clk_cfg, uport->membase + GENI_SER_S_CLK_CFG);
out_restart_rx:
qcom_geni_serial_start_rx(uport);
}
@@ -1195,13 +1164,13 @@ static int __init qcom_geni_serial_earlycon_setup(struct earlycon_device *dev,
geni_se_init(&se, DEF_FIFO_DEPTH_WORDS / 2, DEF_FIFO_DEPTH_WORDS - 2);
geni_se_select_mode(&se, GENI_SE_FIFO);
- writel_relaxed(tx_trans_cfg, uport->membase + SE_UART_TX_TRANS_CFG);
- writel_relaxed(tx_parity_cfg, uport->membase + SE_UART_TX_PARITY_CFG);
- writel_relaxed(rx_trans_cfg, uport->membase + SE_UART_RX_TRANS_CFG);
- writel_relaxed(rx_parity_cfg, uport->membase + SE_UART_RX_PARITY_CFG);
- writel_relaxed(bits_per_char, uport->membase + SE_UART_TX_WORD_LEN);
- writel_relaxed(bits_per_char, uport->membase + SE_UART_RX_WORD_LEN);
- writel_relaxed(stop_bit_len, uport->membase + SE_UART_TX_STOP_BIT_LEN);
+ writel(tx_trans_cfg, uport->membase + SE_UART_TX_TRANS_CFG);
+ writel(tx_parity_cfg, uport->membase + SE_UART_TX_PARITY_CFG);
+ writel(rx_trans_cfg, uport->membase + SE_UART_RX_TRANS_CFG);
+ writel(rx_parity_cfg, uport->membase + SE_UART_RX_PARITY_CFG);
+ writel(bits_per_char, uport->membase + SE_UART_TX_WORD_LEN);
+ writel(bits_per_char, uport->membase + SE_UART_RX_WORD_LEN);
+ writel(stop_bit_len, uport->membase + SE_UART_TX_STOP_BIT_LEN);
dev->con->write = qcom_geni_serial_earlycon_write;
dev->con->setup = NULL;
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related
* [PATCH v2 2/4] tty: serial: qcom_geni_serial: Remove set_rfr_wm() and related variables
From: Ryan Case @ 2019-01-08 1:58 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Evan Green, Doug Anderson, linux-kernel, linux-serial,
Stephen Boyd, Ryan Case
In-Reply-To: <20190108015838.166271-1-ryandcase@chromium.org>
The variables of tx_wm and rx_wm were set to the same define value in
all cases, never updated, and the define was sometimes used
interchangably. Remove the variables/function and use the fixed value.
Signed-off-by: Ryan Case <ryandcase@chromium.org>
Reviewed-by: Evan Green <evgreen@chromium.org>
---
Changes in v2:
- Removed CONSOLE from UART_CONSOLE_RX_WM
drivers/tty/serial/qcom_geni_serial.c | 25 ++++---------------------
1 file changed, 4 insertions(+), 21 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index e2cb3f7d1a7c..22d3cb4b3b39 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -89,7 +89,7 @@
#define DEF_FIFO_DEPTH_WORDS 16
#define DEF_TX_WM 2
#define DEF_FIFO_WIDTH_BITS 32
-#define UART_CONSOLE_RX_WM 2
+#define UART_RX_WM 2
#define MAX_LOOPBACK_CFG 3
#ifdef CONFIG_CONSOLE_POLL
@@ -105,9 +105,6 @@ struct qcom_geni_serial_port {
u32 tx_fifo_depth;
u32 tx_fifo_width;
u32 rx_fifo_depth;
- u32 tx_wm;
- u32 rx_wm;
- u32 rx_rfr;
enum geni_se_xfer_mode xfer_mode;
bool setup;
int (*handle_rx)(struct uart_port *uport, u32 bytes, bool drop);
@@ -365,9 +362,7 @@ static int qcom_geni_serial_get_char(struct uart_port *uport)
static void qcom_geni_serial_poll_put_char(struct uart_port *uport,
unsigned char c)
{
- struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
-
- writel(port->tx_wm, uport->membase + SE_GENI_TX_WATERMARK_REG);
+ writel(DEF_TX_WM, uport->membase + SE_GENI_TX_WATERMARK_REG);
qcom_geni_serial_setup_tx(uport, 1);
WARN_ON(!qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
M_TX_FIFO_WATERMARK_EN, true));
@@ -576,7 +571,7 @@ static void qcom_geni_serial_start_tx(struct uart_port *uport)
irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
irq_en |= M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN;
- writel(port->tx_wm, uport->membase +
+ writel(DEF_TX_WM, uport->membase +
SE_GENI_TX_WATERMARK_REG);
writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
}
@@ -848,17 +843,6 @@ static void get_tx_fifo_size(struct qcom_geni_serial_port *port)
(port->tx_fifo_depth * port->tx_fifo_width) / BITS_PER_BYTE;
}
-static void set_rfr_wm(struct qcom_geni_serial_port *port)
-{
- /*
- * Set RFR (Flow off) to FIFO_DEPTH - 2.
- * RX WM level at 10% RX_FIFO_DEPTH.
- * TX WM level at 10% TX_FIFO_DEPTH.
- */
- port->rx_rfr = port->rx_fifo_depth - 2;
- port->rx_wm = UART_CONSOLE_RX_WM;
- port->tx_wm = DEF_TX_WM;
-}
static void qcom_geni_serial_shutdown(struct uart_port *uport)
{
@@ -899,7 +883,6 @@ static int qcom_geni_serial_port_setup(struct uart_port *uport)
get_tx_fifo_size(port);
- set_rfr_wm(port);
writel(rxstale, uport->membase + SE_UART_RX_STALE_CNT);
/*
* Make an unconditional cancel on the main sequencer to reset
@@ -912,7 +895,7 @@ static int qcom_geni_serial_port_setup(struct uart_port *uport)
false, true, false);
geni_se_config_packing(&port->se, BITS_PER_BYTE, port->rx_bytes_pw,
false, false, true);
- geni_se_init(&port->se, port->rx_wm, port->rx_rfr);
+ geni_se_init(&port->se, UART_RX_WM, port->rx_fifo_depth - 2);
geni_se_select_mode(&port->se, port->xfer_mode);
if (!uart_console(uport)) {
port->rx_fifo = devm_kcalloc(uport->dev,
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related
* [PATCH v2 3/4] tty: serial: qcom_geni_serial: Remove xfer_mode variable
From: Ryan Case @ 2019-01-08 1:58 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Evan Green, Doug Anderson, linux-kernel, linux-serial,
Stephen Boyd, Ryan Case
In-Reply-To: <20190108015838.166271-1-ryandcase@chromium.org>
The driver only supports FIFO mode so setting and checking this variable
is unnecessary. If DMA support is ever added then such checks can be
introduced.
Signed-off-by: Ryan Case <ryandcase@chromium.org>
Reviewed-by: Evan Green <evgreen@chromium.org>
---
Changes in v2:
- Fixed commit message typo
- Coalesced lines where possible
drivers/tty/serial/qcom_geni_serial.c | 67 ++++++++++-----------------
1 file changed, 24 insertions(+), 43 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 22d3cb4b3b39..626bf7d399aa 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -105,7 +105,6 @@ struct qcom_geni_serial_port {
u32 tx_fifo_depth;
u32 tx_fifo_width;
u32 rx_fifo_depth;
- enum geni_se_xfer_mode xfer_mode;
bool setup;
int (*handle_rx)(struct uart_port *uport, u32 bytes, bool drop);
unsigned int baud;
@@ -552,29 +551,20 @@ static int handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop)
static void qcom_geni_serial_start_tx(struct uart_port *uport)
{
u32 irq_en;
- struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
u32 status;
- if (port->xfer_mode == GENI_SE_FIFO) {
- /*
- * readl ensures reading & writing of IRQ_EN register
- * is not re-ordered before checking the status of the
- * Serial Engine.
- */
- status = readl(uport->membase + SE_GENI_STATUS);
- if (status & M_GENI_CMD_ACTIVE)
- return;
+ status = readl(uport->membase + SE_GENI_STATUS);
+ if (status & M_GENI_CMD_ACTIVE)
+ return;
- if (!qcom_geni_serial_tx_empty(uport))
- return;
+ if (!qcom_geni_serial_tx_empty(uport))
+ return;
- irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
- irq_en |= M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN;
+ irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
+ irq_en |= M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN;
- writel(DEF_TX_WM, uport->membase +
- SE_GENI_TX_WATERMARK_REG);
- writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
- }
+ writel(DEF_TX_WM, uport->membase + SE_GENI_TX_WATERMARK_REG);
+ writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
}
static void qcom_geni_serial_stop_tx(struct uart_port *uport)
@@ -584,12 +574,8 @@ static void qcom_geni_serial_stop_tx(struct uart_port *uport)
struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
- irq_en &= ~M_CMD_DONE_EN;
- if (port->xfer_mode == GENI_SE_FIFO) {
- irq_en &= ~M_TX_FIFO_WATERMARK_EN;
- writel(0, uport->membase +
- SE_GENI_TX_WATERMARK_REG);
- }
+ irq_en &= ~(M_CMD_DONE_EN | M_TX_FIFO_WATERMARK_EN);
+ writel(0, uport->membase + SE_GENI_TX_WATERMARK_REG);
writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
status = readl(uport->membase + SE_GENI_STATUS);
/* Possible stop tx is called multiple times. */
@@ -619,15 +605,13 @@ static void qcom_geni_serial_start_rx(struct uart_port *uport)
geni_se_setup_s_cmd(&port->se, UART_START_READ, 0);
- if (port->xfer_mode == GENI_SE_FIFO) {
- irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
- irq_en |= S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN;
- writel(irq_en, uport->membase + SE_GENI_S_IRQ_EN);
+ irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
+ irq_en |= S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN;
+ writel(irq_en, uport->membase + SE_GENI_S_IRQ_EN);
- irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
- irq_en |= M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN;
- writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
- }
+ irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
+ irq_en |= M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN;
+ writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
}
static void qcom_geni_serial_stop_rx(struct uart_port *uport)
@@ -637,15 +621,13 @@ static void qcom_geni_serial_stop_rx(struct uart_port *uport)
struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
u32 irq_clear = S_CMD_DONE_EN;
- if (port->xfer_mode == GENI_SE_FIFO) {
- irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
- irq_en &= ~(S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN);
- writel(irq_en, uport->membase + SE_GENI_S_IRQ_EN);
+ irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
+ irq_en &= ~(S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN);
+ writel(irq_en, uport->membase + SE_GENI_S_IRQ_EN);
- irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
- irq_en &= ~(M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN);
- writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
- }
+ irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
+ irq_en &= ~(M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN);
+ writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
status = readl(uport->membase + SE_GENI_STATUS);
/* Possible stop rx is called multiple times. */
@@ -888,7 +870,6 @@ static int qcom_geni_serial_port_setup(struct uart_port *uport)
* Make an unconditional cancel on the main sequencer to reset
* it else we could end up in data loss scenarios.
*/
- port->xfer_mode = GENI_SE_FIFO;
if (uart_console(uport))
qcom_geni_serial_poll_tx_done(uport);
geni_se_config_packing(&port->se, BITS_PER_BYTE, port->tx_bytes_pw,
@@ -896,7 +877,7 @@ static int qcom_geni_serial_port_setup(struct uart_port *uport)
geni_se_config_packing(&port->se, BITS_PER_BYTE, port->rx_bytes_pw,
false, false, true);
geni_se_init(&port->se, UART_RX_WM, port->rx_fifo_depth - 2);
- geni_se_select_mode(&port->se, port->xfer_mode);
+ geni_se_select_mode(&port->se, GENI_SE_FIFO);
if (!uart_console(uport)) {
port->rx_fifo = devm_kcalloc(uport->dev,
port->rx_fifo_depth, sizeof(u32), GFP_KERNEL);
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related
* [PATCH v2 4/4] tty: serial: qcom_geni_serial: Use u32 for register variables
From: Ryan Case @ 2019-01-08 1:58 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Evan Green, Doug Anderson, linux-kernel, linux-serial,
Stephen Boyd, Ryan Case
In-Reply-To: <20190108015838.166271-1-ryandcase@chromium.org>
Use u32 rather than unsigned long for register variables for clarity and
consistency.
Signed-off-by: Ryan Case <ryandcase@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-by: Evan Green <evgreen@chromium.org>
---
Changes in v2:
- Updated commit message
- Updated missed rxstale variable
drivers/tty/serial/qcom_geni_serial.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 626bf7d399aa..6d8a44c12596 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -760,12 +760,12 @@ static void qcom_geni_serial_handle_tx(struct uart_port *uport, bool done,
static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
{
- unsigned int m_irq_status;
- unsigned int s_irq_status;
- unsigned int geni_status;
+ u32 m_irq_en;
+ u32 m_irq_status;
+ u32 s_irq_status;
+ u32 geni_status;
struct uart_port *uport = dev;
unsigned long flags;
- unsigned int m_irq_en;
bool drop_rx = false;
struct tty_port *tport = &uport->state->port;
struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
@@ -844,7 +844,7 @@ static void qcom_geni_serial_shutdown(struct uart_port *uport)
static int qcom_geni_serial_port_setup(struct uart_port *uport)
{
struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
- unsigned int rxstale = DEFAULT_BITS_PER_CHAR * STALE_TIMEOUT;
+ u32 rxstale = DEFAULT_BITS_PER_CHAR * STALE_TIMEOUT;
u32 proto;
if (uart_console(uport)) {
@@ -943,14 +943,14 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
struct ktermios *termios, struct ktermios *old)
{
unsigned int baud;
- unsigned int bits_per_char;
- unsigned int tx_trans_cfg;
- unsigned int tx_parity_cfg;
- unsigned int rx_trans_cfg;
- unsigned int rx_parity_cfg;
- unsigned int stop_bit_len;
+ u32 bits_per_char;
+ u32 tx_trans_cfg;
+ u32 tx_parity_cfg;
+ u32 rx_trans_cfg;
+ u32 rx_parity_cfg;
+ u32 stop_bit_len;
unsigned int clk_div;
- unsigned long ser_clk_cfg;
+ u32 ser_clk_cfg;
struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
unsigned long clk_rate;
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related
* [PATCH AUTOSEL 4.20 014/117] serial: set suppress_bind_attrs flag only if builtin
From: Sasha Levin @ 2019-01-08 19:24 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Anders Roxell, Arnd Bergmann, Greg Kroah-Hartman, Sasha Levin,
linux-serial
In-Reply-To: <20190108192628.121270-1-sashal@kernel.org>
From: Anders Roxell <anders.roxell@linaro.org>
[ Upstream commit 646097940ad35aa2c1f2012af932d55976a9f255 ]
When the test 'CONFIG_DEBUG_TEST_DRIVER_REMOVE=y' is enabled,
arch_initcall(pl011_init) came before subsys_initcall(default_bdi_init).
devtmpfs gets killed because we try to remove a file and decrement the
wb reference count before the noop_backing_device_info gets initialized.
[ 0.332075] Serial: AMBA PL011 UART driver
[ 0.485276] 9000000.pl011: ttyAMA0 at MMIO 0x9000000 (irq = 39, base_baud = 0) is a PL011 rev1
[ 0.502382] console [ttyAMA0] enabled
[ 0.515710] Unable to handle kernel paging request at virtual address 0000800074c12000
[ 0.516053] Mem abort info:
[ 0.516222] ESR = 0x96000004
[ 0.516417] Exception class = DABT (current EL), IL = 32 bits
[ 0.516641] SET = 0, FnV = 0
[ 0.516826] EA = 0, S1PTW = 0
[ 0.516984] Data abort info:
[ 0.517149] ISV = 0, ISS = 0x00000004
[ 0.517339] CM = 0, WnR = 0
[ 0.517553] [0000800074c12000] user address but active_mm is swapper
[ 0.517928] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[ 0.518305] Modules linked in:
[ 0.518839] CPU: 0 PID: 13 Comm: kdevtmpfs Not tainted 4.19.0-rc5-next-20180928-00002-g2ba39ab0cd01-dirty #82
[ 0.519307] Hardware name: linux,dummy-virt (DT)
[ 0.519681] pstate: 80000005 (Nzcv daif -PAN -UAO)
[ 0.519959] pc : __destroy_inode+0x94/0x2a8
[ 0.520212] lr : __destroy_inode+0x78/0x2a8
[ 0.520401] sp : ffff0000098c3b20
[ 0.520590] x29: ffff0000098c3b20 x28: 00000000087a3714
[ 0.520904] x27: 0000000000002000 x26: 0000000000002000
[ 0.521179] x25: ffff000009583000 x24: 0000000000000000
[ 0.521467] x23: ffff80007bb52000 x22: ffff80007bbaa7c0
[ 0.521737] x21: ffff0000093f9338 x20: 0000000000000000
[ 0.522033] x19: ffff80007bbb05d8 x18: 0000000000000400
[ 0.522376] x17: 0000000000000000 x16: 0000000000000000
[ 0.522727] x15: 0000000000000400 x14: 0000000000000400
[ 0.523068] x13: 0000000000000001 x12: 0000000000000001
[ 0.523421] x11: 0000000000000000 x10: 0000000000000970
[ 0.523749] x9 : ffff0000098c3a60 x8 : ffff80007bbab190
[ 0.524017] x7 : ffff80007bbaa880 x6 : 0000000000000c88
[ 0.524305] x5 : ffff0000093d96c8 x4 : 61c8864680b583eb
[ 0.524567] x3 : ffff0000093d6180 x2 : ffffffffffffffff
[ 0.524872] x1 : 0000800074c12000 x0 : 0000800074c12000
[ 0.525207] Process kdevtmpfs (pid: 13, stack limit = 0x(____ptrval____))
[ 0.525529] Call trace:
[ 0.525806] __destroy_inode+0x94/0x2a8
[ 0.526108] destroy_inode+0x34/0x88
[ 0.526370] evict+0x144/0x1c8
[ 0.526636] iput+0x184/0x230
[ 0.526871] dentry_unlink_inode+0x118/0x130
[ 0.527152] d_delete+0xd8/0xe0
[ 0.527420] vfs_unlink+0x240/0x270
[ 0.527665] handle_remove+0x1d8/0x330
[ 0.527875] devtmpfsd+0x138/0x1c8
[ 0.528085] kthread+0x14c/0x158
[ 0.528291] ret_from_fork+0x10/0x18
[ 0.528720] Code: 92800002 aa1403e0 d538d081 8b010000 (c85f7c04)
[ 0.529367] ---[ end trace 5a3dee47727f877c ]---
Rework to set suppress_bind_attrs flag to avoid removing the device when
CONFIG_DEBUG_TEST_DRIVER_REMOVE=y. This applies for pic32_uart and
xilinx_uartps as well.
Co-developed-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/tty/serial/amba-pl011.c | 2 ++
drivers/tty/serial/pic32_uart.c | 1 +
drivers/tty/serial/xilinx_uartps.c | 1 +
3 files changed, 4 insertions(+)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index ebd33c0232e6..89ade213a1a9 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2780,6 +2780,7 @@ static struct platform_driver arm_sbsa_uart_platform_driver = {
.name = "sbsa-uart",
.of_match_table = of_match_ptr(sbsa_uart_of_match),
.acpi_match_table = ACPI_PTR(sbsa_uart_acpi_match),
+ .suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_AMBA_PL011),
},
};
@@ -2808,6 +2809,7 @@ static struct amba_driver pl011_driver = {
.drv = {
.name = "uart-pl011",
.pm = &pl011_dev_pm_ops,
+ .suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_AMBA_PL011),
},
.id_table = pl011_ids,
.probe = pl011_probe,
diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
index fd80d999308d..0bdf1687983f 100644
--- a/drivers/tty/serial/pic32_uart.c
+++ b/drivers/tty/serial/pic32_uart.c
@@ -919,6 +919,7 @@ static struct platform_driver pic32_uart_platform_driver = {
.driver = {
.name = PIC32_DEV_NAME,
.of_match_table = of_match_ptr(pic32_serial_dt_ids),
+ .suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_PIC32),
},
};
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 57c66d2c3471..379242b96790 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1719,6 +1719,7 @@ static struct platform_driver cdns_uart_platform_driver = {
.name = CDNS_UART_NAME,
.of_match_table = cdns_uart_of_match,
.pm = &cdns_uart_dev_pm_ops,
+ .suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_XILINX_PS_UART),
},
};
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.20 077/117] tty/serial: do not free trasnmit buffer page under port lock
From: Sasha Levin @ 2019-01-08 19:25 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sergey Senozhatsky, Sergey Senozhatsky, Greg Kroah-Hartman,
Jiri Slaby, Andrew Morton, Waiman Long, Dmitry Safonov,
Steven Rostedt, Sasha Levin, linux-serial
In-Reply-To: <20190108192628.121270-1-sashal@kernel.org>
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
[ Upstream commit d72402145ace0697a6a9e8e75a3de5bf3375f78d ]
LKP has hit yet another circular locking dependency between uart
console drivers and debugobjects [1]:
CPU0 CPU1
rhltable_init()
__init_work()
debug_object_init
uart_shutdown() /* db->lock */
/* uart_port->lock */ debug_print_object()
free_page() printk()
call_console_drivers()
debug_check_no_obj_freed() /* uart_port->lock */
/* db->lock */
debug_print_object()
So there are two dependency chains:
uart_port->lock -> db->lock
And
db->lock -> uart_port->lock
This particular circular locking dependency can be addressed in several
ways:
a) One way would be to move debug_print_object() out of db->lock scope
and, thus, break the db->lock -> uart_port->lock chain.
b) Another one would be to free() transmit buffer page out of db->lock
in UART code; which is what this patch does.
It makes sense to apply a) and b) independently: there are too many things
going on behind free(), none of which depend on uart_port->lock.
The patch fixes transmit buffer page free() in uart_shutdown() and,
additionally, in uart_port_startup() (as was suggested by Dmitry Safonov).
[1] https://lore.kernel.org/lkml/20181211091154.GL23332@shao2-debian/T/#u
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Dmitry Safonov <dima@arista.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/tty/serial/serial_core.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index c439a5a1e6c0..d4cca5bdaf1c 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -205,10 +205,15 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
if (!state->xmit.buf) {
state->xmit.buf = (unsigned char *) page;
uart_circ_clear(&state->xmit);
+ uart_port_unlock(uport, flags);
} else {
+ uart_port_unlock(uport, flags);
+ /*
+ * Do not free() the page under the port lock, see
+ * uart_shutdown().
+ */
free_page(page);
}
- uart_port_unlock(uport, flags);
retval = uport->ops->startup(uport);
if (retval == 0) {
@@ -268,6 +273,7 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
struct uart_port *uport = uart_port_check(state);
struct tty_port *port = &state->port;
unsigned long flags = 0;
+ char *xmit_buf = NULL;
/*
* Set the TTY IO error marker
@@ -298,14 +304,18 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
tty_port_set_suspended(port, 0);
/*
- * Free the transmit buffer page.
+ * Do not free() the transmit buffer page under the port lock since
+ * this can create various circular locking scenarios. For instance,
+ * console driver may need to allocate/free a debug object, which
+ * can endup in printk() recursion.
*/
uart_port_lock(state, flags);
- if (state->xmit.buf) {
- free_page((unsigned long)state->xmit.buf);
- state->xmit.buf = NULL;
- }
+ xmit_buf = state->xmit.buf;
+ state->xmit.buf = NULL;
uart_port_unlock(uport, flags);
+
+ if (xmit_buf)
+ free_page((unsigned long)xmit_buf);
}
/**
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 12/97] serial: set suppress_bind_attrs flag only if builtin
From: Sasha Levin @ 2019-01-08 19:28 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Anders Roxell, Arnd Bergmann, Greg Kroah-Hartman, Sasha Levin,
linux-serial
In-Reply-To: <20190108192949.122407-1-sashal@kernel.org>
From: Anders Roxell <anders.roxell@linaro.org>
[ Upstream commit 646097940ad35aa2c1f2012af932d55976a9f255 ]
When the test 'CONFIG_DEBUG_TEST_DRIVER_REMOVE=y' is enabled,
arch_initcall(pl011_init) came before subsys_initcall(default_bdi_init).
devtmpfs gets killed because we try to remove a file and decrement the
wb reference count before the noop_backing_device_info gets initialized.
[ 0.332075] Serial: AMBA PL011 UART driver
[ 0.485276] 9000000.pl011: ttyAMA0 at MMIO 0x9000000 (irq = 39, base_baud = 0) is a PL011 rev1
[ 0.502382] console [ttyAMA0] enabled
[ 0.515710] Unable to handle kernel paging request at virtual address 0000800074c12000
[ 0.516053] Mem abort info:
[ 0.516222] ESR = 0x96000004
[ 0.516417] Exception class = DABT (current EL), IL = 32 bits
[ 0.516641] SET = 0, FnV = 0
[ 0.516826] EA = 0, S1PTW = 0
[ 0.516984] Data abort info:
[ 0.517149] ISV = 0, ISS = 0x00000004
[ 0.517339] CM = 0, WnR = 0
[ 0.517553] [0000800074c12000] user address but active_mm is swapper
[ 0.517928] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[ 0.518305] Modules linked in:
[ 0.518839] CPU: 0 PID: 13 Comm: kdevtmpfs Not tainted 4.19.0-rc5-next-20180928-00002-g2ba39ab0cd01-dirty #82
[ 0.519307] Hardware name: linux,dummy-virt (DT)
[ 0.519681] pstate: 80000005 (Nzcv daif -PAN -UAO)
[ 0.519959] pc : __destroy_inode+0x94/0x2a8
[ 0.520212] lr : __destroy_inode+0x78/0x2a8
[ 0.520401] sp : ffff0000098c3b20
[ 0.520590] x29: ffff0000098c3b20 x28: 00000000087a3714
[ 0.520904] x27: 0000000000002000 x26: 0000000000002000
[ 0.521179] x25: ffff000009583000 x24: 0000000000000000
[ 0.521467] x23: ffff80007bb52000 x22: ffff80007bbaa7c0
[ 0.521737] x21: ffff0000093f9338 x20: 0000000000000000
[ 0.522033] x19: ffff80007bbb05d8 x18: 0000000000000400
[ 0.522376] x17: 0000000000000000 x16: 0000000000000000
[ 0.522727] x15: 0000000000000400 x14: 0000000000000400
[ 0.523068] x13: 0000000000000001 x12: 0000000000000001
[ 0.523421] x11: 0000000000000000 x10: 0000000000000970
[ 0.523749] x9 : ffff0000098c3a60 x8 : ffff80007bbab190
[ 0.524017] x7 : ffff80007bbaa880 x6 : 0000000000000c88
[ 0.524305] x5 : ffff0000093d96c8 x4 : 61c8864680b583eb
[ 0.524567] x3 : ffff0000093d6180 x2 : ffffffffffffffff
[ 0.524872] x1 : 0000800074c12000 x0 : 0000800074c12000
[ 0.525207] Process kdevtmpfs (pid: 13, stack limit = 0x(____ptrval____))
[ 0.525529] Call trace:
[ 0.525806] __destroy_inode+0x94/0x2a8
[ 0.526108] destroy_inode+0x34/0x88
[ 0.526370] evict+0x144/0x1c8
[ 0.526636] iput+0x184/0x230
[ 0.526871] dentry_unlink_inode+0x118/0x130
[ 0.527152] d_delete+0xd8/0xe0
[ 0.527420] vfs_unlink+0x240/0x270
[ 0.527665] handle_remove+0x1d8/0x330
[ 0.527875] devtmpfsd+0x138/0x1c8
[ 0.528085] kthread+0x14c/0x158
[ 0.528291] ret_from_fork+0x10/0x18
[ 0.528720] Code: 92800002 aa1403e0 d538d081 8b010000 (c85f7c04)
[ 0.529367] ---[ end trace 5a3dee47727f877c ]---
Rework to set suppress_bind_attrs flag to avoid removing the device when
CONFIG_DEBUG_TEST_DRIVER_REMOVE=y. This applies for pic32_uart and
xilinx_uartps as well.
Co-developed-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/tty/serial/amba-pl011.c | 2 ++
drivers/tty/serial/pic32_uart.c | 1 +
drivers/tty/serial/xilinx_uartps.c | 1 +
3 files changed, 4 insertions(+)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index ebd33c0232e6..89ade213a1a9 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2780,6 +2780,7 @@ static struct platform_driver arm_sbsa_uart_platform_driver = {
.name = "sbsa-uart",
.of_match_table = of_match_ptr(sbsa_uart_of_match),
.acpi_match_table = ACPI_PTR(sbsa_uart_acpi_match),
+ .suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_AMBA_PL011),
},
};
@@ -2808,6 +2809,7 @@ static struct amba_driver pl011_driver = {
.drv = {
.name = "uart-pl011",
.pm = &pl011_dev_pm_ops,
+ .suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_AMBA_PL011),
},
.id_table = pl011_ids,
.probe = pl011_probe,
diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
index fd80d999308d..0bdf1687983f 100644
--- a/drivers/tty/serial/pic32_uart.c
+++ b/drivers/tty/serial/pic32_uart.c
@@ -919,6 +919,7 @@ static struct platform_driver pic32_uart_platform_driver = {
.driver = {
.name = PIC32_DEV_NAME,
.of_match_table = of_match_ptr(pic32_serial_dt_ids),
+ .suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_PIC32),
},
};
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index a48f19b1b88f..e5e9924ed7fb 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1608,6 +1608,7 @@ static struct platform_driver cdns_uart_platform_driver = {
.name = CDNS_UART_NAME,
.of_match_table = cdns_uart_of_match,
.pm = &cdns_uart_dev_pm_ops,
+ .suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_XILINX_PS_UART),
},
};
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 62/97] tty/serial: do not free trasnmit buffer page under port lock
From: Sasha Levin @ 2019-01-08 19:29 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sergey Senozhatsky, Sergey Senozhatsky, Greg Kroah-Hartman,
Jiri Slaby, Andrew Morton, Waiman Long, Dmitry Safonov,
Steven Rostedt, Sasha Levin, linux-serial
In-Reply-To: <20190108192949.122407-1-sashal@kernel.org>
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
[ Upstream commit d72402145ace0697a6a9e8e75a3de5bf3375f78d ]
LKP has hit yet another circular locking dependency between uart
console drivers and debugobjects [1]:
CPU0 CPU1
rhltable_init()
__init_work()
debug_object_init
uart_shutdown() /* db->lock */
/* uart_port->lock */ debug_print_object()
free_page() printk()
call_console_drivers()
debug_check_no_obj_freed() /* uart_port->lock */
/* db->lock */
debug_print_object()
So there are two dependency chains:
uart_port->lock -> db->lock
And
db->lock -> uart_port->lock
This particular circular locking dependency can be addressed in several
ways:
a) One way would be to move debug_print_object() out of db->lock scope
and, thus, break the db->lock -> uart_port->lock chain.
b) Another one would be to free() transmit buffer page out of db->lock
in UART code; which is what this patch does.
It makes sense to apply a) and b) independently: there are too many things
going on behind free(), none of which depend on uart_port->lock.
The patch fixes transmit buffer page free() in uart_shutdown() and,
additionally, in uart_port_startup() (as was suggested by Dmitry Safonov).
[1] https://lore.kernel.org/lkml/20181211091154.GL23332@shao2-debian/T/#u
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Dmitry Safonov <dima@arista.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/tty/serial/serial_core.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 80bb56facfb6..ad126f51d549 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -205,10 +205,15 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
if (!state->xmit.buf) {
state->xmit.buf = (unsigned char *) page;
uart_circ_clear(&state->xmit);
+ uart_port_unlock(uport, flags);
} else {
+ uart_port_unlock(uport, flags);
+ /*
+ * Do not free() the page under the port lock, see
+ * uart_shutdown().
+ */
free_page(page);
}
- uart_port_unlock(uport, flags);
retval = uport->ops->startup(uport);
if (retval == 0) {
@@ -268,6 +273,7 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
struct uart_port *uport = uart_port_check(state);
struct tty_port *port = &state->port;
unsigned long flags = 0;
+ char *xmit_buf = NULL;
/*
* Set the TTY IO error marker
@@ -298,14 +304,18 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
tty_port_set_suspended(port, 0);
/*
- * Free the transmit buffer page.
+ * Do not free() the transmit buffer page under the port lock since
+ * this can create various circular locking scenarios. For instance,
+ * console driver may need to allocate/free a debug object, which
+ * can endup in printk() recursion.
*/
uart_port_lock(state, flags);
- if (state->xmit.buf) {
- free_page((unsigned long)state->xmit.buf);
- state->xmit.buf = NULL;
- }
+ xmit_buf = state->xmit.buf;
+ state->xmit.buf = NULL;
uart_port_unlock(uport, flags);
+
+ if (xmit_buf)
+ free_page((unsigned long)xmit_buf);
}
/**
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 05/53] serial: set suppress_bind_attrs flag only if builtin
From: Sasha Levin @ 2019-01-08 19:31 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Anders Roxell, Arnd Bergmann, Greg Kroah-Hartman, Sasha Levin,
linux-serial
In-Reply-To: <20190108193222.123316-1-sashal@kernel.org>
From: Anders Roxell <anders.roxell@linaro.org>
[ Upstream commit 646097940ad35aa2c1f2012af932d55976a9f255 ]
When the test 'CONFIG_DEBUG_TEST_DRIVER_REMOVE=y' is enabled,
arch_initcall(pl011_init) came before subsys_initcall(default_bdi_init).
devtmpfs gets killed because we try to remove a file and decrement the
wb reference count before the noop_backing_device_info gets initialized.
[ 0.332075] Serial: AMBA PL011 UART driver
[ 0.485276] 9000000.pl011: ttyAMA0 at MMIO 0x9000000 (irq = 39, base_baud = 0) is a PL011 rev1
[ 0.502382] console [ttyAMA0] enabled
[ 0.515710] Unable to handle kernel paging request at virtual address 0000800074c12000
[ 0.516053] Mem abort info:
[ 0.516222] ESR = 0x96000004
[ 0.516417] Exception class = DABT (current EL), IL = 32 bits
[ 0.516641] SET = 0, FnV = 0
[ 0.516826] EA = 0, S1PTW = 0
[ 0.516984] Data abort info:
[ 0.517149] ISV = 0, ISS = 0x00000004
[ 0.517339] CM = 0, WnR = 0
[ 0.517553] [0000800074c12000] user address but active_mm is swapper
[ 0.517928] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[ 0.518305] Modules linked in:
[ 0.518839] CPU: 0 PID: 13 Comm: kdevtmpfs Not tainted 4.19.0-rc5-next-20180928-00002-g2ba39ab0cd01-dirty #82
[ 0.519307] Hardware name: linux,dummy-virt (DT)
[ 0.519681] pstate: 80000005 (Nzcv daif -PAN -UAO)
[ 0.519959] pc : __destroy_inode+0x94/0x2a8
[ 0.520212] lr : __destroy_inode+0x78/0x2a8
[ 0.520401] sp : ffff0000098c3b20
[ 0.520590] x29: ffff0000098c3b20 x28: 00000000087a3714
[ 0.520904] x27: 0000000000002000 x26: 0000000000002000
[ 0.521179] x25: ffff000009583000 x24: 0000000000000000
[ 0.521467] x23: ffff80007bb52000 x22: ffff80007bbaa7c0
[ 0.521737] x21: ffff0000093f9338 x20: 0000000000000000
[ 0.522033] x19: ffff80007bbb05d8 x18: 0000000000000400
[ 0.522376] x17: 0000000000000000 x16: 0000000000000000
[ 0.522727] x15: 0000000000000400 x14: 0000000000000400
[ 0.523068] x13: 0000000000000001 x12: 0000000000000001
[ 0.523421] x11: 0000000000000000 x10: 0000000000000970
[ 0.523749] x9 : ffff0000098c3a60 x8 : ffff80007bbab190
[ 0.524017] x7 : ffff80007bbaa880 x6 : 0000000000000c88
[ 0.524305] x5 : ffff0000093d96c8 x4 : 61c8864680b583eb
[ 0.524567] x3 : ffff0000093d6180 x2 : ffffffffffffffff
[ 0.524872] x1 : 0000800074c12000 x0 : 0000800074c12000
[ 0.525207] Process kdevtmpfs (pid: 13, stack limit = 0x(____ptrval____))
[ 0.525529] Call trace:
[ 0.525806] __destroy_inode+0x94/0x2a8
[ 0.526108] destroy_inode+0x34/0x88
[ 0.526370] evict+0x144/0x1c8
[ 0.526636] iput+0x184/0x230
[ 0.526871] dentry_unlink_inode+0x118/0x130
[ 0.527152] d_delete+0xd8/0xe0
[ 0.527420] vfs_unlink+0x240/0x270
[ 0.527665] handle_remove+0x1d8/0x330
[ 0.527875] devtmpfsd+0x138/0x1c8
[ 0.528085] kthread+0x14c/0x158
[ 0.528291] ret_from_fork+0x10/0x18
[ 0.528720] Code: 92800002 aa1403e0 d538d081 8b010000 (c85f7c04)
[ 0.529367] ---[ end trace 5a3dee47727f877c ]---
Rework to set suppress_bind_attrs flag to avoid removing the device when
CONFIG_DEBUG_TEST_DRIVER_REMOVE=y. This applies for pic32_uart and
xilinx_uartps as well.
Co-developed-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/tty/serial/amba-pl011.c | 2 ++
drivers/tty/serial/pic32_uart.c | 1 +
drivers/tty/serial/xilinx_uartps.c | 1 +
3 files changed, 4 insertions(+)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index c9f701aca677..4a4a9f33715c 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2800,6 +2800,7 @@ static struct platform_driver arm_sbsa_uart_platform_driver = {
.name = "sbsa-uart",
.of_match_table = of_match_ptr(sbsa_uart_of_match),
.acpi_match_table = ACPI_PTR(sbsa_uart_acpi_match),
+ .suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_AMBA_PL011),
},
};
@@ -2828,6 +2829,7 @@ static struct amba_driver pl011_driver = {
.drv = {
.name = "uart-pl011",
.pm = &pl011_dev_pm_ops,
+ .suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_AMBA_PL011),
},
.id_table = pl011_ids,
.probe = pl011_probe,
diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
index 00a33eb859d3..e3d7d9d6c599 100644
--- a/drivers/tty/serial/pic32_uart.c
+++ b/drivers/tty/serial/pic32_uart.c
@@ -920,6 +920,7 @@ static struct platform_driver pic32_uart_platform_driver = {
.driver = {
.name = PIC32_DEV_NAME,
.of_match_table = of_match_ptr(pic32_serial_dt_ids),
+ .suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_PIC32),
},
};
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 21c35ad72b99..772d07f85b3d 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1644,6 +1644,7 @@ static struct platform_driver cdns_uart_platform_driver = {
.name = CDNS_UART_NAME,
.of_match_table = cdns_uart_of_match,
.pm = &cdns_uart_dev_pm_ops,
+ .suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_XILINX_PS_UART),
},
};
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 33/53] tty/serial: do not free trasnmit buffer page under port lock
From: Sasha Levin @ 2019-01-08 19:32 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sergey Senozhatsky, Sergey Senozhatsky, Greg Kroah-Hartman,
Jiri Slaby, Andrew Morton, Waiman Long, Dmitry Safonov,
Steven Rostedt, Sasha Levin, linux-serial
In-Reply-To: <20190108193222.123316-1-sashal@kernel.org>
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
[ Upstream commit d72402145ace0697a6a9e8e75a3de5bf3375f78d ]
LKP has hit yet another circular locking dependency between uart
console drivers and debugobjects [1]:
CPU0 CPU1
rhltable_init()
__init_work()
debug_object_init
uart_shutdown() /* db->lock */
/* uart_port->lock */ debug_print_object()
free_page() printk()
call_console_drivers()
debug_check_no_obj_freed() /* uart_port->lock */
/* db->lock */
debug_print_object()
So there are two dependency chains:
uart_port->lock -> db->lock
And
db->lock -> uart_port->lock
This particular circular locking dependency can be addressed in several
ways:
a) One way would be to move debug_print_object() out of db->lock scope
and, thus, break the db->lock -> uart_port->lock chain.
b) Another one would be to free() transmit buffer page out of db->lock
in UART code; which is what this patch does.
It makes sense to apply a) and b) independently: there are too many things
going on behind free(), none of which depend on uart_port->lock.
The patch fixes transmit buffer page free() in uart_shutdown() and,
additionally, in uart_port_startup() (as was suggested by Dmitry Safonov).
[1] https://lore.kernel.org/lkml/20181211091154.GL23332@shao2-debian/T/#u
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Dmitry Safonov <dima@arista.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/tty/serial/serial_core.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 6db8844ef3ec..543d0f95f094 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -218,10 +218,15 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
if (!state->xmit.buf) {
state->xmit.buf = (unsigned char *) page;
uart_circ_clear(&state->xmit);
+ uart_port_unlock(uport, flags);
} else {
+ uart_port_unlock(uport, flags);
+ /*
+ * Do not free() the page under the port lock, see
+ * uart_shutdown().
+ */
free_page(page);
}
- uart_port_unlock(uport, flags);
retval = uport->ops->startup(uport);
if (retval == 0) {
@@ -281,6 +286,7 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
struct uart_port *uport = uart_port_check(state);
struct tty_port *port = &state->port;
unsigned long flags = 0;
+ char *xmit_buf = NULL;
/*
* Set the TTY IO error marker
@@ -311,14 +317,18 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
tty_port_set_suspended(port, 0);
/*
- * Free the transmit buffer page.
+ * Do not free() the transmit buffer page under the port lock since
+ * this can create various circular locking scenarios. For instance,
+ * console driver may need to allocate/free a debug object, which
+ * can endup in printk() recursion.
*/
uart_port_lock(state, flags);
- if (state->xmit.buf) {
- free_page((unsigned long)state->xmit.buf);
- state->xmit.buf = NULL;
- }
+ xmit_buf = state->xmit.buf;
+ state->xmit.buf = NULL;
uart_port_unlock(uport, flags);
+
+ if (xmit_buf)
+ free_page((unsigned long)xmit_buf);
}
/**
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 04/36] serial: set suppress_bind_attrs flag only if builtin
From: Sasha Levin @ 2019-01-08 19:33 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Anders Roxell, Arnd Bergmann, Greg Kroah-Hartman, Sasha Levin,
linux-serial
In-Reply-To: <20190108193348.123880-1-sashal@kernel.org>
From: Anders Roxell <anders.roxell@linaro.org>
[ Upstream commit 646097940ad35aa2c1f2012af932d55976a9f255 ]
When the test 'CONFIG_DEBUG_TEST_DRIVER_REMOVE=y' is enabled,
arch_initcall(pl011_init) came before subsys_initcall(default_bdi_init).
devtmpfs gets killed because we try to remove a file and decrement the
wb reference count before the noop_backing_device_info gets initialized.
[ 0.332075] Serial: AMBA PL011 UART driver
[ 0.485276] 9000000.pl011: ttyAMA0 at MMIO 0x9000000 (irq = 39, base_baud = 0) is a PL011 rev1
[ 0.502382] console [ttyAMA0] enabled
[ 0.515710] Unable to handle kernel paging request at virtual address 0000800074c12000
[ 0.516053] Mem abort info:
[ 0.516222] ESR = 0x96000004
[ 0.516417] Exception class = DABT (current EL), IL = 32 bits
[ 0.516641] SET = 0, FnV = 0
[ 0.516826] EA = 0, S1PTW = 0
[ 0.516984] Data abort info:
[ 0.517149] ISV = 0, ISS = 0x00000004
[ 0.517339] CM = 0, WnR = 0
[ 0.517553] [0000800074c12000] user address but active_mm is swapper
[ 0.517928] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[ 0.518305] Modules linked in:
[ 0.518839] CPU: 0 PID: 13 Comm: kdevtmpfs Not tainted 4.19.0-rc5-next-20180928-00002-g2ba39ab0cd01-dirty #82
[ 0.519307] Hardware name: linux,dummy-virt (DT)
[ 0.519681] pstate: 80000005 (Nzcv daif -PAN -UAO)
[ 0.519959] pc : __destroy_inode+0x94/0x2a8
[ 0.520212] lr : __destroy_inode+0x78/0x2a8
[ 0.520401] sp : ffff0000098c3b20
[ 0.520590] x29: ffff0000098c3b20 x28: 00000000087a3714
[ 0.520904] x27: 0000000000002000 x26: 0000000000002000
[ 0.521179] x25: ffff000009583000 x24: 0000000000000000
[ 0.521467] x23: ffff80007bb52000 x22: ffff80007bbaa7c0
[ 0.521737] x21: ffff0000093f9338 x20: 0000000000000000
[ 0.522033] x19: ffff80007bbb05d8 x18: 0000000000000400
[ 0.522376] x17: 0000000000000000 x16: 0000000000000000
[ 0.522727] x15: 0000000000000400 x14: 0000000000000400
[ 0.523068] x13: 0000000000000001 x12: 0000000000000001
[ 0.523421] x11: 0000000000000000 x10: 0000000000000970
[ 0.523749] x9 : ffff0000098c3a60 x8 : ffff80007bbab190
[ 0.524017] x7 : ffff80007bbaa880 x6 : 0000000000000c88
[ 0.524305] x5 : ffff0000093d96c8 x4 : 61c8864680b583eb
[ 0.524567] x3 : ffff0000093d6180 x2 : ffffffffffffffff
[ 0.524872] x1 : 0000800074c12000 x0 : 0000800074c12000
[ 0.525207] Process kdevtmpfs (pid: 13, stack limit = 0x(____ptrval____))
[ 0.525529] Call trace:
[ 0.525806] __destroy_inode+0x94/0x2a8
[ 0.526108] destroy_inode+0x34/0x88
[ 0.526370] evict+0x144/0x1c8
[ 0.526636] iput+0x184/0x230
[ 0.526871] dentry_unlink_inode+0x118/0x130
[ 0.527152] d_delete+0xd8/0xe0
[ 0.527420] vfs_unlink+0x240/0x270
[ 0.527665] handle_remove+0x1d8/0x330
[ 0.527875] devtmpfsd+0x138/0x1c8
[ 0.528085] kthread+0x14c/0x158
[ 0.528291] ret_from_fork+0x10/0x18
[ 0.528720] Code: 92800002 aa1403e0 d538d081 8b010000 (c85f7c04)
[ 0.529367] ---[ end trace 5a3dee47727f877c ]---
Rework to set suppress_bind_attrs flag to avoid removing the device when
CONFIG_DEBUG_TEST_DRIVER_REMOVE=y. This applies for pic32_uart and
xilinx_uartps as well.
Co-developed-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/tty/serial/amba-pl011.c | 2 ++
drivers/tty/serial/pic32_uart.c | 1 +
drivers/tty/serial/xilinx_uartps.c | 1 +
3 files changed, 4 insertions(+)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 41b0dd67fcce..2d8089fc2139 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2712,6 +2712,7 @@ static struct platform_driver arm_sbsa_uart_platform_driver = {
.name = "sbsa-uart",
.of_match_table = of_match_ptr(sbsa_uart_of_match),
.acpi_match_table = ACPI_PTR(sbsa_uart_acpi_match),
+ .suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_AMBA_PL011),
},
};
@@ -2740,6 +2741,7 @@ static struct amba_driver pl011_driver = {
.drv = {
.name = "uart-pl011",
.pm = &pl011_dev_pm_ops,
+ .suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_AMBA_PL011),
},
.id_table = pl011_ids,
.probe = pl011_probe,
diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
index 7f8e99bbcb73..fb77d55a8d95 100644
--- a/drivers/tty/serial/pic32_uart.c
+++ b/drivers/tty/serial/pic32_uart.c
@@ -920,6 +920,7 @@ static struct platform_driver pic32_uart_platform_driver = {
.driver = {
.name = PIC32_DEV_NAME,
.of_match_table = of_match_ptr(pic32_serial_dt_ids),
+ .suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_PIC32),
},
};
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 7497f1d4a818..f8b09dc4a583 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1602,6 +1602,7 @@ static struct platform_driver cdns_uart_platform_driver = {
.name = CDNS_UART_NAME,
.of_match_table = cdns_uart_of_match,
.pm = &cdns_uart_dev_pm_ops,
+ .suppress_bind_attrs = IS_BUILTIN(CONFIG_SERIAL_XILINX_PS_UART),
},
};
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 23/36] tty/serial: do not free trasnmit buffer page under port lock
From: Sasha Levin @ 2019-01-08 19:33 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sergey Senozhatsky, Sergey Senozhatsky, Greg Kroah-Hartman,
Jiri Slaby, Andrew Morton, Waiman Long, Dmitry Safonov,
Steven Rostedt, Sasha Levin, linux-serial
In-Reply-To: <20190108193348.123880-1-sashal@kernel.org>
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
[ Upstream commit d72402145ace0697a6a9e8e75a3de5bf3375f78d ]
LKP has hit yet another circular locking dependency between uart
console drivers and debugobjects [1]:
CPU0 CPU1
rhltable_init()
__init_work()
debug_object_init
uart_shutdown() /* db->lock */
/* uart_port->lock */ debug_print_object()
free_page() printk()
call_console_drivers()
debug_check_no_obj_freed() /* uart_port->lock */
/* db->lock */
debug_print_object()
So there are two dependency chains:
uart_port->lock -> db->lock
And
db->lock -> uart_port->lock
This particular circular locking dependency can be addressed in several
ways:
a) One way would be to move debug_print_object() out of db->lock scope
and, thus, break the db->lock -> uart_port->lock chain.
b) Another one would be to free() transmit buffer page out of db->lock
in UART code; which is what this patch does.
It makes sense to apply a) and b) independently: there are too many things
going on behind free(), none of which depend on uart_port->lock.
The patch fixes transmit buffer page free() in uart_shutdown() and,
additionally, in uart_port_startup() (as was suggested by Dmitry Safonov).
[1] https://lore.kernel.org/lkml/20181211091154.GL23332@shao2-debian/T/#u
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Dmitry Safonov <dima@arista.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/tty/serial/serial_core.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 53e6db8b0330..bcfdaf6ddbb2 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -195,10 +195,15 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
if (!state->xmit.buf) {
state->xmit.buf = (unsigned char *) page;
uart_circ_clear(&state->xmit);
+ uart_port_unlock(uport, flags);
} else {
+ uart_port_unlock(uport, flags);
+ /*
+ * Do not free() the page under the port lock, see
+ * uart_shutdown().
+ */
free_page(page);
}
- uart_port_unlock(uport, flags);
retval = uport->ops->startup(uport);
if (retval == 0) {
@@ -258,6 +263,7 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
struct uart_port *uport = uart_port_check(state);
struct tty_port *port = &state->port;
unsigned long flags = 0;
+ char *xmit_buf = NULL;
/*
* Set the TTY IO error marker
@@ -288,14 +294,18 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
tty_port_set_suspended(port, 0);
/*
- * Free the transmit buffer page.
+ * Do not free() the transmit buffer page under the port lock since
+ * this can create various circular locking scenarios. For instance,
+ * console driver may need to allocate/free a debug object, which
+ * can endup in printk() recursion.
*/
uart_port_lock(state, flags);
- if (state->xmit.buf) {
- free_page((unsigned long)state->xmit.buf);
- state->xmit.buf = NULL;
- }
+ xmit_buf = state->xmit.buf;
+ state->xmit.buf = NULL;
uart_port_unlock(uport, flags);
+
+ if (xmit_buf)
+ free_page((unsigned long)xmit_buf);
}
/**
--
2.19.1
^ permalink raw reply related
* [PATCH 0/3] 8250_omap: use clk APIs to get fclk freqeuncy
From: Vignesh R @ 2019-01-09 9:12 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rob Herring
Cc: Jiri Slaby, Vignesh R, Tony Lindgren, Lokesh Vutla, linux-serial,
linux-omap, devicetree, linux-kernel
This series adds support obtain clk frequency of functional clk via
clocks phandle instead of hard coding it in DT as part of
clock-frequency DT parameter. This eliminates need to calculate
frequency offline and populate it.
Vignesh R (3):
serial: 8250_omap: Drop check for of_node
dt-bindings: serial: omap_serial: add clocks entry
serial: 8250_omap: Use clk_get_rate() to obtain fclk frequency
.../bindings/serial/omap_serial.txt | 2 +
drivers/tty/serial/8250/8250_omap.c | 75 ++++++++++---------
2 files changed, 42 insertions(+), 35 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH 1/3] serial: 8250_omap: Drop check for of_node
From: Vignesh R @ 2019-01-09 9:12 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rob Herring
Cc: Jiri Slaby, Vignesh R, Tony Lindgren, Lokesh Vutla, linux-serial,
linux-omap, devicetree, linux-kernel
In-Reply-To: <20190109091206.25759-1-vigneshr@ti.com>
8250_omap is DT only driver so dev->of_node always exists. Drop check
for existence of valid dev->of_node to simplify omap8250_probe().
Signed-off-by: Vignesh R <vigneshr@ti.com>
---
drivers/tty/serial/8250/8250_omap.c | 63 +++++++++++++----------------
1 file changed, 28 insertions(+), 35 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index ad7ba7d0f28d..a74126569785 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -1134,10 +1134,12 @@ static int omap8250_probe(struct platform_device *pdev)
{
struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
struct resource *irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ struct device_node *np = pdev->dev.of_node;
struct omap8250_priv *priv;
struct uart_8250_port up;
int ret;
void __iomem *membase;
+ const struct of_device_id *id;
if (!regs || !irq) {
dev_err(&pdev->dev, "missing registers or irq\n");
@@ -1194,27 +1196,20 @@ static int omap8250_probe(struct platform_device *pdev)
up.port.unthrottle = omap_8250_unthrottle;
up.port.rs485_config = omap_8250_rs485_config;
- if (pdev->dev.of_node) {
- const struct of_device_id *id;
-
- ret = of_alias_get_id(pdev->dev.of_node, "serial");
-
- of_property_read_u32(pdev->dev.of_node, "clock-frequency",
- &up.port.uartclk);
- priv->wakeirq = irq_of_parse_and_map(pdev->dev.of_node, 1);
-
- id = of_match_device(of_match_ptr(omap8250_dt_ids), &pdev->dev);
- if (id && id->data)
- priv->habit |= *(u8 *)id->data;
- } else {
- ret = pdev->id;
- }
+ ret = of_alias_get_id(np, "serial");
if (ret < 0) {
- dev_err(&pdev->dev, "failed to get alias/pdev id\n");
+ dev_err(&pdev->dev, "failed to get alias\n");
return ret;
}
up.port.line = ret;
+ of_property_read_u32(np, "clock-frequency", &up.port.uartclk);
+ priv->wakeirq = irq_of_parse_and_map(np, 1);
+
+ id = of_match_device(of_match_ptr(omap8250_dt_ids), &pdev->dev);
+ if (id && id->data)
+ priv->habit |= *(u8 *)id->data;
+
if (!up.port.uartclk) {
up.port.uartclk = DEFAULT_CLK_SPEED;
dev_warn(&pdev->dev,
@@ -1242,25 +1237,23 @@ static int omap8250_probe(struct platform_device *pdev)
omap_serial_fill_features_erratas(&up, priv);
up.port.handle_irq = omap8250_no_handle_irq;
#ifdef CONFIG_SERIAL_8250_DMA
- if (pdev->dev.of_node) {
- /*
- * Oh DMA support. If there are no DMA properties in the DT then
- * we will fall back to a generic DMA channel which does not
- * really work here. To ensure that we do not get a generic DMA
- * channel assigned, we have the the_no_dma_filter_fn() here.
- * To avoid "failed to request DMA" messages we check for DMA
- * properties in DT.
- */
- ret = of_property_count_strings(pdev->dev.of_node, "dma-names");
- if (ret == 2) {
- up.dma = &priv->omap8250_dma;
- priv->omap8250_dma.fn = the_no_dma_filter_fn;
- priv->omap8250_dma.tx_dma = omap_8250_tx_dma;
- priv->omap8250_dma.rx_dma = omap_8250_rx_dma;
- priv->omap8250_dma.rx_size = RX_TRIGGER;
- priv->omap8250_dma.rxconf.src_maxburst = RX_TRIGGER;
- priv->omap8250_dma.txconf.dst_maxburst = TX_TRIGGER;
- }
+ /*
+ * Oh DMA support. If there are no DMA properties in the DT then
+ * we will fall back to a generic DMA channel which does not
+ * really work here. To ensure that we do not get a generic DMA
+ * channel assigned, we have the the_no_dma_filter_fn() here.
+ * To avoid "failed to request DMA" messages we check for DMA
+ * properties in DT.
+ */
+ ret = of_property_count_strings(np, "dma-names");
+ if (ret == 2) {
+ up.dma = &priv->omap8250_dma;
+ priv->omap8250_dma.fn = the_no_dma_filter_fn;
+ priv->omap8250_dma.tx_dma = omap_8250_tx_dma;
+ priv->omap8250_dma.rx_dma = omap_8250_rx_dma;
+ priv->omap8250_dma.rx_size = RX_TRIGGER;
+ priv->omap8250_dma.rxconf.src_maxburst = RX_TRIGGER;
+ priv->omap8250_dma.txconf.dst_maxburst = TX_TRIGGER;
}
#endif
ret = serial8250_register_8250_port(&up);
--
2.20.1
^ permalink raw reply related
* [PATCH 2/3] dt-bindings: serial: omap_serial: add clocks entry
From: Vignesh R @ 2019-01-09 9:12 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rob Herring
Cc: Jiri Slaby, Vignesh R, Tony Lindgren, Lokesh Vutla, linux-serial,
linux-omap, devicetree, linux-kernel
In-Reply-To: <20190109091206.25759-1-vigneshr@ti.com>
Document clocks property used to pass phandle to functional clk.
Signed-off-by: Vignesh R <vigneshr@ti.com>
---
Documentation/devicetree/bindings/serial/omap_serial.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/serial/omap_serial.txt b/Documentation/devicetree/bindings/serial/omap_serial.txt
index c35d5ece1156..0a9b5444f4e6 100644
--- a/Documentation/devicetree/bindings/serial/omap_serial.txt
+++ b/Documentation/devicetree/bindings/serial/omap_serial.txt
@@ -22,6 +22,8 @@ Optional properties:
- dma-names : "rx" for receive channel, "tx" for transmit channel.
- rs485-rts-delay, rs485-rx-during-tx, linux,rs485-enabled-at-boot-time: see rs485.txt
- rs485-rts-active-high: drive RTS high when sending (default is low).
+- clocks: phandle to the functional clock as per
+ Documentation/devicetree/bindings/clock/clock-bindings.txt
Example:
--
2.20.1
^ permalink raw reply related
* [PATCH 3/3] serial: 8250_omap: Use clk_get_rate() to obtain fclk frequency
From: Vignesh R @ 2019-01-09 9:12 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rob Herring
Cc: Jiri Slaby, Vignesh R, Tony Lindgren, Lokesh Vutla, linux-serial,
linux-omap, devicetree, linux-kernel
In-Reply-To: <20190109091206.25759-1-vigneshr@ti.com>
8250_omap driver uses clock-frequency DT property to obtain functional
clk frequency. This is not ideal as users need to calculate functional
clk frequency offline and populate it in DT.
Therefore add support to obtain functional clock frequency using clk
APIs when clock-frequency DT property is not defined.
Suggested-by: Lokesh Vutla <lokeshvutla@ti.com>
Signed-off-by: Vignesh R <vigneshr@ti.com>
---
drivers/tty/serial/8250/8250_omap.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index a74126569785..0a8316632d75 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -12,6 +12,7 @@
#define SUPPORT_SYSRQ
#endif
+#include <linux/clk.h>
#include <linux/device.h>
#include <linux/io.h>
#include <linux/module.h>
@@ -1203,7 +1204,18 @@ static int omap8250_probe(struct platform_device *pdev)
}
up.port.line = ret;
- of_property_read_u32(np, "clock-frequency", &up.port.uartclk);
+ if (of_property_read_u32(np, "clock-frequency", &up.port.uartclk)) {
+ struct clk *clk;
+
+ clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(clk)) {
+ if (PTR_ERR(clk) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ } else {
+ up.port.uartclk = clk_get_rate(clk);
+ }
+ }
+
priv->wakeirq = irq_of_parse_and_map(np, 1);
id = of_match_device(of_match_ptr(omap8250_dt_ids), &pdev->dev);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 1/3] serial: 8250_omap: Drop check for of_node
From: Tony Lindgren @ 2019-01-09 21:44 UTC (permalink / raw)
To: Vignesh R
Cc: Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Lokesh Vutla,
linux-serial, linux-omap, devicetree, linux-kernel
In-Reply-To: <20190109091206.25759-2-vigneshr@ti.com>
* Vignesh R <vigneshr@ti.com> [190109 09:11]:
> 8250_omap is DT only driver so dev->of_node always exists. Drop check
> for existence of valid dev->of_node to simplify omap8250_probe().
That part seems safe to me now.
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
...
> - const struct of_device_id *id;
> -
> - ret = of_alias_get_id(pdev->dev.of_node, "serial");
> -
> - of_property_read_u32(pdev->dev.of_node, "clock-frequency",
> - &up.port.uartclk);
> - priv->wakeirq = irq_of_parse_and_map(pdev->dev.of_node, 1);
> -
> - id = of_match_device(of_match_ptr(omap8250_dt_ids), &pdev->dev);
> - if (id && id->data)
> - priv->habit |= *(u8 *)id->data;
But this part it seems we still need to keep around
as we still have lots of clock-frequency references
in the *.dtsi files. Or am I missing something?
Regards
Tony
^ permalink raw reply
* [PATCH] 8250_pci.c: Only check for communication class in serial_pci_guess_board
From: Guan Yung Tseng @ 2019-01-10 8:12 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Guan Yung Tseng
Some multiport serial cards, such as the NI PXI-8430/2, NI PXI-8430/8,
and NI PXI-8432/4 use PCI_CLASS_COMMUNICATION_OTHER and this fail the
serial_pci_is_class_communication test added in the commit 7d8905d06405
("serial: 8250_pci: Enable device after we check black list").
Since these devices are correctly listed in serial_pci_tbl, we
shouldn't need to check the PCI class IDs. This change relocates the
class checking solely into "serial_pci_guess_board" where it had been
before so that the class-check doesn't hinder initialization.
Signed-off-by: Guan Yung Tseng <guan.yung.tseng@ni.com>
---
drivers/tty/serial/8250/8250_pci.c | 28 +++++++++-------------------
1 file changed, 9 insertions(+), 19 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 4986b4a..e33a869 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -3382,21 +3382,6 @@ static const struct pci_device_id blacklist[] = {
{ PCI_VDEVICE(COMMTECH, PCI_ANY_ID), },
};
-static int serial_pci_is_class_communication(struct pci_dev *dev)
-{
- /*
- * If it is not a communications device or the programming
- * interface is greater than 6, give up.
- */
- if ((((dev->class >> 8) != PCI_CLASS_COMMUNICATION_SERIAL) &&
- ((dev->class >> 8) != PCI_CLASS_COMMUNICATION_MULTISERIAL) &&
- ((dev->class >> 8) != PCI_CLASS_COMMUNICATION_MODEM)) ||
- (dev->class & 0xff) > 6)
- return -ENODEV;
-
- return 0;
-}
-
static int serial_pci_is_blacklisted(struct pci_dev *dev)
{
const struct pci_device_id *bldev;
@@ -3427,6 +3412,15 @@ serial_pci_guess_board(struct pci_dev *dev, struct pciserial_board *board)
int num_iomem, num_port, first_port = -1, i;
/*
+ * If it is not a communications device or the programming
+ * interface is greater than 6, give up.
+ *
+ */
+ if ((((dev->class >> 8) != PCI_CLASS_COMMUNICATION_SERIAL) &&
+ ((dev->class >> 8) != PCI_CLASS_COMMUNICATION_MODEM)) ||
+ (dev->class & 0xff) > 6)
+ return -ENODEV;
+ /*
* Should we try to make guesses for multiport serial devices later?
*/
if ((dev->class >> 8) == PCI_CLASS_COMMUNICATION_MULTISERIAL)
@@ -3652,10 +3646,6 @@ pciserial_init_one(struct pci_dev *dev, const struct pci_device_id *ent)
board = &pci_boards[ent->driver_data];
- rc = serial_pci_is_class_communication(dev);
- if (rc)
- return rc;
-
rc = serial_pci_is_blacklisted(dev);
if (rc)
return rc;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v9 1/2] dmaengine: 8250_mtk_dma: add MediaTek uart DMA support
From: Long Cheng @ 2019-01-10 8:24 UTC (permalink / raw)
To: Nicolas Boichat
Cc: Vinod Koul, Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee,
Sean Wang, Matthias Brugger, Dan Williams, Greg Kroah-Hartman,
Jiri Slaby, Sean Wang, dmaengine, devicetree,
linux-arm Mailing List, linux-mediatek, lkml, linux-serial,
srv_heupstream, Yingjoe Chen, YT Shen, Z
In-Reply-To: <CANMq1KB=yuEJxUdi8M9deDhAv24WZ-4h+26cFq7AzBi8uyd88Q@mail.gmail.com>
On Thu, 2019-01-03 at 09:39 +0800, Nicolas Boichat wrote:
> On Wed, Jan 2, 2019 at 10:13 AM Long Cheng <long.cheng@mediatek.com> wrote:
> >
.....
> > +/* interrupt trigger level for tx */
> > +#define VFF_TX_THRE(n) ((n) * 7 / 8)
> > +/* interrupt trigger level for rx */
> > +#define VFF_RX_THRE(n) ((n) * 3 / 4)
> > +
> > +#define VFF_RING_SIZE 0xffffU
>
> Well, the size is actually 0x10000. Maybe call this VFF_RING_SIZE_MASK?
>
the max length is 0xffff. the bit 16 is wrap bit. our buffer is ring
buffer. So not mask.
> > +/* invert this bit when wrap ring head again*/
> > +#define VFF_RING_WRAP 0x10000U
> > +
> > + writel(val, c->base + reg);
> > +}
> > +
......
> > +static void mtk_uart_apdma_start_tx(struct mtk_chan *c)
> > +{
> > + unsigned int len, send, left, wpt, d_wpt, tmp;
> > + int ret;
> > +
> > + left = mtk_uart_apdma_read(c, VFF_LEFT_SIZE);
> > + if (!left) {
> > + mtk_uart_apdma_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
> > + return;
> > + }
> > +
> > + /* Wait 1sec for flush, can't sleep*/
>
> nit: one space after ',', period after 'sleep', space before '*'.
>
> > + ret = readx_poll_timeout(readl, c->base + VFF_FLUSH, tmp,
> > + tmp != VFF_FLUSH_B, 0, 1000000);
> > + if (ret)
> > + dev_warn(c->vc.chan.device->dev, "tx: fail, debug=0x%x\n",
> > + mtk_uart_apdma_read(c, VFF_DEBUG_STATUS));
>
> Why do we need to wait for flush now? The previous implementation did
> not require this...
>
because our HW buffer is cyclic. at one tx, if the size of data is
bigger, data maybe cover. So must wait flush finish. confirm the data is
right. like 128 bytes size, small length can't reproduce the issue.
> > +
> > + send = min_t(unsigned int, left, c->desc->avail_len);
> > + wpt = mtk_uart_apdma_read(c, VFF_WPT);
> > + len = mtk_uart_apdma_read(c, VFF_LEN);
> > +
> > + d_wpt = wpt + send;
> > + if ((d_wpt & VFF_RING_SIZE) >= len) {
>
> I don't get why you need to add "& VFF_RING_SIZE". If wpt + send >
> VFF_RING_SIZE, don't you need to toggle VFF_RING_WRAP too?
the longest actual length is VFF_RING_SIZE. one cyclic, will set bit[16]
to ~bit[16]. the bit[0 ~ 15] is actual address. So need get rid of
bit[16]
>
> > + d_wpt = d_wpt - len;
> > + d_wpt = d_wpt ^ VFF_RING_WRAP;
> > + }
> > + mtk_uart_apdma_write(c, VFF_WPT, d_wpt);
> > +
> > + c->desc->avail_len -= send;
> > +
> > + mtk_uart_apdma_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
> > + if (mtk_uart_apdma_read(c, VFF_FLUSH) == 0U)
> > + mtk_uart_apdma_write(c, VFF_FLUSH, VFF_FLUSH_B);
> > +}
>
> (thanks for the rest of the changes, this looks much more readable)
>
> > +
......
> > +
> > +static int mtk_uart_apdma_device_pause(struct dma_chan *chan)
> > +{
> > + /* just for check caps pass */
> > + return 0;
> > +}
>
> This is still not right... Hopefully somebody more familiar with the
> DMA subsystem can weigh in, but maybe it's enough to wait for the
> current transfer to be flushed and temporarily disable interrupts?
> e.g. call mtk_uart_apdma_terminate_all above?
>
i had review the 8250 UART framework. just check the function pointer,
not use. and our HW can't support the feature. So i just keep it at
first version.
> > +
> > +static int mtk_uart_apdma_device_resume(struct dma_chan *chan)
> > +{
> > + /* just for check caps pass */
> > + return 0;
> > +}
>
> Drop this one since you don't really need it.
>
ok, i will remove it.
> > +
> > +static void mtk_uart_apdma_free(struct mtk_uart_apdmadev *mtkd)
> > +{
> > + while (list_empty(&mtkd->ddev.channels) == 0) {
>
> !list_empty(
>
> > + struct mtk_chan *c = list_first_entry(&mtkd->ddev.channels,
> > + struct mtk_chan, vc.chan.device_node);
> > +
> > + list_del(&c->vc.chan.device_node);
> > + tasklet_kill(&c->vc.task);
> > + }
> > +}
> > +
> > +static const struct of_device_id mtk_uart_apdma_match[] = {
> > + { .compatible = "mediatek,mt6577-uart-dma", },
> > + { /* sentinel */ },
> > +};
> > +MODULE_DEVICE_TABLE(of, mtk_uart_apdma_match);
> > +
> > +static int mtk_uart_apdma_probe(struct platform_device *pdev)
> > +{
> > + struct mtk_uart_apdmadev *mtkd;
> > + struct resource *res;
> > + struct mtk_chan *c;
> > + unsigned int i;
> > + int rc;
> > +
> > + mtkd = devm_kzalloc(&pdev->dev, sizeof(*mtkd), GFP_KERNEL);
> > + if (!mtkd)
> > + return -ENOMEM;
> > +
> > + mtkd->clk = devm_clk_get(&pdev->dev, NULL);
> > + if (IS_ERR(mtkd->clk)) {
> > + dev_err(&pdev->dev, "No clock specified\n");
> > + rc = PTR_ERR(mtkd->clk);
> > + return rc;
> > + }
> > +
> > + if (of_property_read_bool(pdev->dev.of_node, "dma-33bits"))
> > + mtkd->support_33bits = true;
>
> I don't think this should be a device tree property. Typically we'd
> have multiple compatible strings for (slightly) different HW blocks,
> and enable 33bits only on HW that have support.
>
> See how it's done in drivers/i2c/busses/i2c-mt65xx.c, for example.
>
in MediaTek, not all IC can support 33bit. So i want to configure by DT.
So don't need to modify code in New IC.
> > +
> > + rc = dma_set_mask_and_coherent(&pdev->dev,
> > + DMA_BIT_MASK(32 | mtkd->support_33bits));
>
> I'd feel a little more confortable if you used a variable instead:
>
> int dma_bits = 32;
>
> if (support_33bits)
> dma_bits = 33;
>
> ..., DMA_BIT_MASK(dma_bits));
>
OK, i can modify it. thanks.
> > + if (rc)
> > + return rc;
> > +
> > + dma_cap_set(DMA_SLAVE, mtkd->ddev.cap_mask);
> > + mtkd->ddev.device_alloc_chan_resources =
> > + mtk_uart_apdma_alloc_chan_resources;
> > + mtkd->ddev.device_free_chan_resources =
> > + mtk_uart_apdma_free_chan_resources;
> > + mtkd->ddev.device_tx_status = mtk_uart_apdma_tx_status;
> > + mtkd->ddev.device_issue_pending = mtk_uart_apdma_issue_pending;
> > + mtkd->ddev.device_prep_slave_sg = mtk_uart_apdma_prep_slave_sg;
> > + mtkd->ddev.device_config = mtk_uart_apdma_slave_config;
> > + mtkd->ddev.device_pause = mtk_uart_apdma_device_pause;
> > + mtkd->ddev.device_resume = mtk_uart_apdma_device_resume;
> > + mtkd->ddev.device_terminate_all = mtk_uart_apdma_terminate_all;
> > + mtkd->ddev.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
> > + mtkd->ddev.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
> > + mtkd->ddev.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
> > + mtkd->ddev.residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
> > + mtkd->ddev.dev = &pdev->dev;
> > + INIT_LIST_HEAD(&mtkd->ddev.channels);
> > +
> > + for (i = 0; i < MTK_UART_APDMA_CHANNELS; i++) {
> > + c = devm_kzalloc(mtkd->ddev.dev, sizeof(*c), GFP_KERNEL);
> > + if (!c) {
> > + rc = -ENODEV;
> > + goto err_no_dma;
> > + }
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> > + if (!res) {
> > + rc = -ENODEV;
> > + goto err_no_dma;
> > + }
> > +
> > + c->base = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(c->base)) {
> > + rc = PTR_ERR(c->base);
> > + goto err_no_dma;
> > + }
> > + c->requested = false;
> > + c->vc.desc_free = mtk_uart_apdma_desc_free;
> > + vchan_init(&c->vc, &mtkd->ddev);
> > +
> > + mtkd->dma_irq[i] = platform_get_irq(pdev, i);
> > + if ((int)mtkd->dma_irq[i] < 0) {
> > + dev_err(&pdev->dev, "failed to get IRQ[%d]\n", i);
> > + rc = -EINVAL;
> > + goto err_no_dma;
> > + }
> > + }
> > +
> > + pm_runtime_enable(&pdev->dev);
> > + pm_runtime_set_active(&pdev->dev);
> > +
> > + rc = dma_async_device_register(&mtkd->ddev);
> > + if (rc)
> > + goto rpm_disable;
> > +
> > + platform_set_drvdata(pdev, mtkd);
> > +
> > + if (pdev->dev.of_node) {
> > + /* Device-tree DMA controller registration */
> > + rc = of_dma_controller_register(pdev->dev.of_node,
> > + of_dma_xlate_by_chan_id,
> > + mtkd);
> > + if (rc)
> > + goto dma_remove;
> > + }
> > +
> > + return rc;
> > +
> > +dma_remove:
> > + dma_async_device_unregister(&mtkd->ddev);
> > +rpm_disable:
> > + pm_runtime_disable(&pdev->dev);
> > +err_no_dma:
> > + mtk_uart_apdma_free(mtkd);
> > + return rc;
> > +}
> > +
......
^ permalink raw reply
* Re: [PATCH v9 1/2] dmaengine: 8250_mtk_dma: add MediaTek uart DMA support
From: Long Cheng @ 2019-01-10 8:50 UTC (permalink / raw)
To: Randy Dunlap
Cc: Nicolas Boichat, Vinod Koul, Rob Herring, Mark Rutland, Ryder Lee,
Sean Wang, Matthias Brugger, Dan Williams, Greg Kroah-Hartman,
Jiri Slaby, Sean Wang, dmaengine, devicetree,
linux-arm Mailing List, linux-mediatek, lkml, linux-serial,
srv_heupstream, Yingjoe Chen, YT Shen, Zhen
In-Reply-To: <d60c25c2-efe4-9294-dc7e-98f0333bff76@infradead.org>
On Wed, 2019-01-02 at 18:03 -0800, Randy Dunlap wrote:
> Hi,
>
> While you are making changes, here are a few more:
>
hi
OK, thanks for your comments.
>
> On 1/2/19 5:39 PM, Nicolas Boichat wrote:
> > diff --git a/drivers/dma/mediatek/Kconfig b/drivers/dma/mediatek/Kconfig
> > index 27bac0b..1a523c87 100644
> > --- a/drivers/dma/mediatek/Kconfig
> > +++ b/drivers/dma/mediatek/Kconfig
> > @@ -1,4 +1,15 @@
> >
> > +config DMA_MTK_UART
> > + tristate "MediaTek SoCs APDMA support for UART"
> > + depends on OF && SERIAL_8250_MT6577
> > + select DMA_ENGINE
> > + select DMA_VIRTUAL_CHANNELS
> > + help
> > + Support for the UART DMA engine found on MediaTek MTK SoCs.
> > + when SERIAL_8250_MT6577 is enabled, and if you want to use DMA,
>
> When
>
> > + you can enable the config. the DMA engine can only be used
>
> The
>
> > + with MediaTek SoCs.
> > +
>
above, i will modify.
> Also, use tabs to indent instead of spaces.
> The lines (tristate, depends, select, and help) should be indented with one tab.
> The help text lines should be indented with one tab + 2 spaces.
>
in my patch, i already do this. So don't need modify.
> > config MTK_HSDMA
> > tristate "MediaTek High-Speed DMA controller support"
> > depends on ARCH_MEDIATEK || COMPILE_TEST
>
>
> thanks,
^ permalink raw reply
* Re: [PATCH] 8250_pci.c: Only check for communication class in serial_pci_guess_board
From: Greg KH @ 2019-01-10 9:36 UTC (permalink / raw)
To: Guan Yung Tseng; +Cc: linux-serial, linux-kernel
In-Reply-To: <1547107932-3714-1-git-send-email-guan.yung.tseng@ni.com>
On Thu, Jan 10, 2019 at 04:12:12PM +0800, Guan Yung Tseng wrote:
> Some multiport serial cards, such as the NI PXI-8430/2, NI PXI-8430/8,
> and NI PXI-8432/4 use PCI_CLASS_COMMUNICATION_OTHER and this fail the
> serial_pci_is_class_communication test added in the commit 7d8905d06405
> ("serial: 8250_pci: Enable device after we check black list").
>
> Since these devices are correctly listed in serial_pci_tbl, we
> shouldn't need to check the PCI class IDs. This change relocates the
> class checking solely into "serial_pci_guess_board" where it had been
> before so that the class-check doesn't hinder initialization.
>
> Signed-off-by: Guan Yung Tseng <guan.yung.tseng@ni.com>
> ---
> drivers/tty/serial/8250/8250_pci.c | 28 +++++++++-------------------
> 1 file changed, 9 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index 4986b4a..e33a869 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -3382,21 +3382,6 @@ static const struct pci_device_id blacklist[] = {
> { PCI_VDEVICE(COMMTECH, PCI_ANY_ID), },
> };
>
> -static int serial_pci_is_class_communication(struct pci_dev *dev)
> -{
> - /*
> - * If it is not a communications device or the programming
> - * interface is greater than 6, give up.
> - */
> - if ((((dev->class >> 8) != PCI_CLASS_COMMUNICATION_SERIAL) &&
> - ((dev->class >> 8) != PCI_CLASS_COMMUNICATION_MULTISERIAL) &&
> - ((dev->class >> 8) != PCI_CLASS_COMMUNICATION_MODEM)) ||
> - (dev->class & 0xff) > 6)
> - return -ENODEV;
> -
> - return 0;
> -}
Please leave this as a function, no need to move it inside another
function at this point in time.
> static int serial_pci_is_blacklisted(struct pci_dev *dev)
> {
> const struct pci_device_id *bldev;
> @@ -3427,6 +3412,15 @@ serial_pci_guess_board(struct pci_dev *dev, struct pciserial_board *board)
> int num_iomem, num_port, first_port = -1, i;
>
> /*
> + * If it is not a communications device or the programming
> + * interface is greater than 6, give up.
> + *
> + */
> + if ((((dev->class >> 8) != PCI_CLASS_COMMUNICATION_SERIAL) &&
> + ((dev->class >> 8) != PCI_CLASS_COMMUNICATION_MODEM)) ||
> + (dev->class & 0xff) > 6)
> + return -ENODEV;
> + /*
what happened to the multiserial check?
You should just call the function here.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v9 1/2] dmaengine: 8250_mtk_dma: add MediaTek uart DMA support
From: Long Cheng @ 2019-01-10 10:33 UTC (permalink / raw)
To: Vinod Koul
Cc: Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee, Sean Wang,
Nicolas Boichat, Matthias Brugger, Dan Williams,
Greg Kroah-Hartman, Jiri Slaby, Sean Wang, dmaengine, devicetree,
linux-arm-kernel, linux-mediatek, linux-kernel, linux-serial,
srv_heupstream, Yingjoe Chen, YT Shen, Zhenbao Liu
In-Reply-To: <20190104171953.GQ13372@vkoul-mobl.Dlink>
On Fri, 2019-01-04 at 22:49 +0530, Vinod Koul wrote:
> On 02-01-19, 10:12, Long Cheng wrote:
> > In DMA engine framework, add 8250 uart dma to support MediaTek uart.
> > If MediaTek uart enabled(SERIAL_8250_MT6577), and want to improve
> > the performance, can enable the function.
>
> Is the DMA controller UART specific, can it work with other controllers
> as well, if so you should get rid of uart name in patch
>
I don't know that it can work or not on other controller. but it's for
MediaTek SOC
> > +#define MTK_UART_APDMA_CHANNELS (CONFIG_SERIAL_8250_NR_UARTS * 2)
>
> Why are the channels not coming from DT?
>
i will using dma-requests install of it.
> > +
> > +#define VFF_EN_B BIT(0)
> > +#define VFF_STOP_B BIT(0)
> > +#define VFF_FLUSH_B BIT(0)
> > +#define VFF_4G_SUPPORT_B BIT(0)
> > +#define VFF_RX_INT_EN0_B BIT(0) /*rx valid size >= vff thre*/
> > +#define VFF_RX_INT_EN1_B BIT(1)
> > +#define VFF_TX_INT_EN_B BIT(0) /*tx left size >= vff thre*/
>
> space around /* space */ also run checkpatch to check for style errors
>
ok.
> > +static void mtk_uart_apdma_start_tx(struct mtk_chan *c)
> > +{
> > + unsigned int len, send, left, wpt, d_wpt, tmp;
> > + int ret;
> > +
> > + left = mtk_uart_apdma_read(c, VFF_LEFT_SIZE);
> > + if (!left) {
> > + mtk_uart_apdma_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
> > + return;
> > + }
> > +
> > + /* Wait 1sec for flush, can't sleep*/
> > + ret = readx_poll_timeout(readl, c->base + VFF_FLUSH, tmp,
> > + tmp != VFF_FLUSH_B, 0, 1000000);
> > + if (ret)
> > + dev_warn(c->vc.chan.device->dev, "tx: fail, debug=0x%x\n",
> > + mtk_uart_apdma_read(c, VFF_DEBUG_STATUS));
> > +
> > + send = min_t(unsigned int, left, c->desc->avail_len);
> > + wpt = mtk_uart_apdma_read(c, VFF_WPT);
> > + len = mtk_uart_apdma_read(c, VFF_LEN);
> > +
> > + d_wpt = wpt + send;
> > + if ((d_wpt & VFF_RING_SIZE) >= len) {
> > + d_wpt = d_wpt - len;
> > + d_wpt = d_wpt ^ VFF_RING_WRAP;
> > + }
> > + mtk_uart_apdma_write(c, VFF_WPT, d_wpt);
> > +
> > + c->desc->avail_len -= send;
> > +
> > + mtk_uart_apdma_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
> > + if (mtk_uart_apdma_read(c, VFF_FLUSH) == 0U)
> > + mtk_uart_apdma_write(c, VFF_FLUSH, VFF_FLUSH_B);
> > +}
> > +
> > +static void mtk_uart_apdma_start_rx(struct mtk_chan *c)
> > +{
> > + struct mtk_uart_apdma_desc *d = c->desc;
> > + unsigned int len, wg, rg, cnt;
> > +
> > + if ((mtk_uart_apdma_read(c, VFF_VALID_SIZE) == 0U) ||
> > + !d || !vchan_next_desc(&c->vc))
> > + return;
> > +
> > + len = mtk_uart_apdma_read(c, VFF_LEN);
> > + rg = mtk_uart_apdma_read(c, VFF_RPT);
> > + wg = mtk_uart_apdma_read(c, VFF_WPT);
> > + if ((rg ^ wg) & VFF_RING_WRAP)
> > + cnt = (wg & VFF_RING_SIZE) + len - (rg & VFF_RING_SIZE);
> > + else
> > + cnt = (wg & VFF_RING_SIZE) - (rg & VFF_RING_SIZE);
> > +
> > + c->rx_status = cnt;
> > + mtk_uart_apdma_write(c, VFF_RPT, wg);
> > +
> > + list_del(&d->vd.node);
> > + vchan_cookie_complete(&d->vd);
> > +}
>
> this looks odd, why do you have different rx and tx start routines?
>
Would you like explain it in more detail? thanks.
In tx function, will wait the last data flush done. and the count the
size that send.
In Rx function, will count the size that receive.
Any way, in rx / tx, need andle WPT or RPT.
> > +static int mtk_uart_apdma_alloc_chan_resources(struct dma_chan *chan)
> > +{
> > + struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
> > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > + u32 tmp;
> > + int ret;
> > +
> > + pm_runtime_get_sync(mtkd->ddev.dev);
> > +
> > + mtk_uart_apdma_write(c, VFF_ADDR, 0);
> > + mtk_uart_apdma_write(c, VFF_THRE, 0);
> > + mtk_uart_apdma_write(c, VFF_LEN, 0);
> > + mtk_uart_apdma_write(c, VFF_RST, VFF_WARM_RST_B);
> > +
> > + ret = readx_poll_timeout(readl, c->base + VFF_EN, tmp,
> > + tmp == 0, 10, 100);
> > + if (ret) {
> > + dev_err(chan->device->dev, "dma reset: fail, timeout\n");
> > + return ret;
> > + }
>
> register read does reset?
>
'mtk_uart_apdma_write(c, VFF_RST, VFF_WARM_RST_B);' is reset. resd just
poll reset done.
> > +
> > + if (!c->requested) {
> > + c->requested = true;
> > + ret = request_irq(mtkd->dma_irq[chan->chan_id],
> > + mtk_uart_apdma_irq_handler, IRQF_TRIGGER_NONE,
> > + KBUILD_MODNAME, chan);
>
> why is the irq not requested in driver probe?
>
I have explained in below,
http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016418.html
> > +static enum dma_status mtk_uart_apdma_tx_status(struct dma_chan *chan,
> > + dma_cookie_t cookie,
> > + struct dma_tx_state *txstate)
> > +{
> > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > + enum dma_status ret;
> > + unsigned long flags;
> > +
> > + if (!txstate)
> > + return DMA_ERROR;
> > +
> > + ret = dma_cookie_status(chan, cookie, txstate);
> > + spin_lock_irqsave(&c->vc.lock, flags);
> > + if (ret == DMA_IN_PROGRESS) {
> > + c->rx_status = mtk_uart_apdma_read(c, VFF_RPT) & VFF_RING_SIZE;
> > + dma_set_residue(txstate, c->rx_status);
> > + } else if (ret == DMA_COMPLETE && c->cfg.direction == DMA_DEV_TO_MEM) {
>
> why set reside when it is complete? also reside can be null, that should
> be checked as well
>
In different status, set different reside.
> > +static struct dma_async_tx_descriptor *mtk_uart_apdma_prep_slave_sg
> > + (struct dma_chan *chan, struct scatterlist *sgl,
> > + unsigned int sglen, enum dma_transfer_direction dir,
> > + unsigned long tx_flags, void *context)
> > +{
> > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > + struct mtk_uart_apdma_desc *d;
> > +
> > + if ((dir != DMA_DEV_TO_MEM) &&
> > + (dir != DMA_MEM_TO_DEV)) {
> > + dev_err(chan->device->dev, "bad direction\n");
> > + return NULL;
> > + }
>
> we have a macro for this
Thanks for your suggestion. i will modify it.
>
> > +
> > + /* Now allocate and setup the descriptor */
> > + d = kzalloc(sizeof(*d), GFP_ATOMIC);
> > + if (!d)
> > + return NULL;
> > +
> > + /* sglen is 1 */
>
> ?
>
> > +static int mtk_uart_apdma_slave_config(struct dma_chan *chan,
> > + struct dma_slave_config *cfg)
> > +{
> > + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > + struct mtk_uart_apdmadev *mtkd =
> > + to_mtk_uart_apdma_dev(c->vc.chan.device);
> > +
> > + c->cfg = *cfg;
> > +
> > + if (cfg->direction == DMA_DEV_TO_MEM) {
>
> fg->direction is deprecated, in fact I have removed all users recently,
> please do not use this
You will remove 'direction' in 'struct dma_slave_config'? if remove, how
do confirm direction?
>
> > + unsigned int rx_len = cfg->src_addr_width * 1024;
> > +
> > + mtk_uart_apdma_write(c, VFF_ADDR, cfg->src_addr);
> > + mtk_uart_apdma_write(c, VFF_LEN, rx_len);
> > + mtk_uart_apdma_write(c, VFF_THRE, VFF_RX_THRE(rx_len));
> > + mtk_uart_apdma_write(c, VFF_INT_EN,
> > + VFF_RX_INT_EN0_B | VFF_RX_INT_EN1_B);
> > + mtk_uart_apdma_write(c, VFF_RPT, 0);
> > + mtk_uart_apdma_write(c, VFF_INT_FLAG, VFF_RX_INT_FLAG_CLR_B);
> > + mtk_uart_apdma_write(c, VFF_EN, VFF_EN_B);
>
> why are we writing this here, we are supposed to do that when txn
> starts!
>
also you can review the link
http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016418.html
> > +static int mtk_uart_apdma_device_resume(struct dma_chan *chan)
> > +{
> > + /* just for check caps pass */
> > + return 0;
> > +}
>
> if you do not support this please remove this!
>
> > +static void mtk_uart_apdma_free(struct mtk_uart_apdmadev *mtkd)
> > +{
> > + while (list_empty(&mtkd->ddev.channels) == 0) {
> > + struct mtk_chan *c = list_first_entry(&mtkd->ddev.channels,
> > + struct mtk_chan, vc.chan.device_node);
> > +
> > + list_del(&c->vc.chan.device_node);
> > + tasklet_kill(&c->vc.task);
> > + }
> > +}
> > +
> > +static const struct of_device_id mtk_uart_apdma_match[] = {
> > + { .compatible = "mediatek,mt6577-uart-dma", },
>
> where is the binding document for ediatek,mt6577-uart-dma ??
>
http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016154.html
^ permalink raw reply
* Re: [PATCH 1/3] serial: 8250_omap: Drop check for of_node
From: Sebastian Reichel @ 2019-01-10 12:07 UTC (permalink / raw)
To: Tony Lindgren
Cc: Vignesh R, Greg Kroah-Hartman, Rob Herring, Jiri Slaby,
Lokesh Vutla, linux-serial, linux-omap, devicetree, linux-kernel
In-Reply-To: <20190109214403.GW5544@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]
Hi,
On Wed, Jan 09, 2019 at 01:44:03PM -0800, Tony Lindgren wrote:
> * Vignesh R <vigneshr@ti.com> [190109 09:11]:
> > 8250_omap is DT only driver so dev->of_node always exists. Drop check
> > for existence of valid dev->of_node to simplify omap8250_probe().
>
> That part seems safe to me now.
>
> > --- a/drivers/tty/serial/8250/8250_omap.c
> > +++ b/drivers/tty/serial/8250/8250_omap.c
> ...
> > - const struct of_device_id *id;
> > -
> > - ret = of_alias_get_id(pdev->dev.of_node, "serial");
> > -
> > - of_property_read_u32(pdev->dev.of_node, "clock-frequency",
> > - &up.port.uartclk);
> > - priv->wakeirq = irq_of_parse_and_map(pdev->dev.of_node, 1);
> > -
> > - id = of_match_device(of_match_ptr(omap8250_dt_ids), &pdev->dev);
> > - if (id && id->data)
> > - priv->habit |= *(u8 *)id->data;
>
> But this part it seems we still need to keep around
> as we still have lots of clock-frequency references
> in the *.dtsi files. Or am I missing something?
It's re-added a couple of lines later. Only the indent was removed.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ 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