* [PATCH v4 4/7] dmaengine: imx-sdma: remove useless 'lock' and 'enabled' in 'struct sdma_channel'
From: Robin Gong @ 2018-06-14 14:02 UTC (permalink / raw)
To: vkoul, s.hauer, l.stach, dan.j.williams, gregkh, jslaby
Cc: linux-serial, dmaengine, linux-kernel, linux-arm-kernel,
linux-imx
In-Reply-To: <1528984982-5074-1-git-send-email-yibin.gong@nxp.com>
Since 'sdmac->vc.lock' and 'sdmac->desc' can be used as 'lock' and
'enabled' in 'struct sdma_channel sdmac', remove them.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 23 -----------------------
1 file changed, 23 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 719bf9f..27b76eb 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -349,10 +349,8 @@ struct sdma_channel {
unsigned long event_mask[2];
unsigned long watermark_level;
u32 shp_addr, per_addr;
- spinlock_t lock;
enum dma_status status;
struct imx_dma_data data;
- bool enabled;
};
#define IMX_DMA_SG_LOOP BIT(0)
@@ -613,14 +611,7 @@ static int sdma_config_ownership(struct sdma_channel *sdmac,
static void sdma_enable_channel(struct sdma_engine *sdma, int channel)
{
- unsigned long flags;
- struct sdma_channel *sdmac = &sdma->channel[channel];
-
writel(BIT(channel), sdma->regs + SDMA_H_START);
-
- spin_lock_irqsave(&sdmac->lock, flags);
- sdmac->enabled = true;
- spin_unlock_irqrestore(&sdmac->lock, flags);
}
/*
@@ -738,14 +729,6 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
struct sdma_buffer_descriptor *bd;
int error = 0;
enum dma_status old_status = sdmac->status;
- unsigned long flags;
-
- spin_lock_irqsave(&sdmac->lock, flags);
- if (!sdmac->enabled) {
- spin_unlock_irqrestore(&sdmac->lock, flags);
- return;
- }
- spin_unlock_irqrestore(&sdmac->lock, flags);
/*
* loop mode. Iterate over descriptors, re-setup them and
@@ -1007,15 +990,10 @@ static int sdma_disable_channel(struct dma_chan *chan)
struct sdma_channel *sdmac = to_sdma_chan(chan);
struct sdma_engine *sdma = sdmac->sdma;
int channel = sdmac->channel;
- unsigned long flags;
writel_relaxed(BIT(channel), sdma->regs + SDMA_H_STATSTOP);
sdmac->status = DMA_ERROR;
- spin_lock_irqsave(&sdmac->lock, flags);
- sdmac->enabled = false;
- spin_unlock_irqrestore(&sdmac->lock, flags);
-
return 0;
}
@@ -1929,7 +1907,6 @@ static int sdma_probe(struct platform_device *pdev)
struct sdma_channel *sdmac = &sdma->channel[i];
sdmac->sdma = sdma;
- spin_lock_init(&sdmac->lock);
sdmac->channel = i;
sdmac->vc.desc_free = sdma_desc_free;
--
2.7.4
^ permalink raw reply related
* [PATCH v4 5/7] dmaengine: imx-sdma: remove the maximum limitation for bd numbers
From: Robin Gong @ 2018-06-14 14:03 UTC (permalink / raw)
To: vkoul, s.hauer, l.stach, dan.j.williams, gregkh, jslaby
Cc: linux-serial, dmaengine, linux-kernel, linux-arm-kernel,
linux-imx
In-Reply-To: <1528984982-5074-1-git-send-email-yibin.gong@nxp.com>
No this limitation now after virtual dma used since bd is allocated
dynamically instead of static.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 27b76eb..f56226f 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -292,7 +292,6 @@ struct sdma_context_data {
u32 scratch7;
} __attribute__ ((packed));
-#define NUM_BD (int)(PAGE_SIZE / sizeof(struct sdma_buffer_descriptor))
struct sdma_engine;
@@ -1297,13 +1296,6 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
if (ret)
goto err_bd_out;
- if (sg_len > NUM_BD) {
- dev_err(sdma->dev, "SDMA channel %d: maximum number of sg exceeded: %d > %d\n",
- channel, sg_len, NUM_BD);
- ret = -EINVAL;
- goto err_bd_out;
- }
-
desc->chn_count = 0;
for_each_sg(sgl, sg, sg_len, i) {
struct sdma_buffer_descriptor *bd = &desc->bd[i];
@@ -1413,12 +1405,6 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
if (ret)
goto err_bd_out;
- if (num_periods > NUM_BD) {
- dev_err(sdma->dev, "SDMA channel %d: maximum number of sg exceeded: %d > %d\n",
- channel, num_periods, NUM_BD);
- goto err_bd_out;
- }
-
if (period_len > 0xffff) {
dev_err(sdma->dev, "SDMA channel %d: maximum period size exceeded: %zu > %d\n",
channel, period_len, 0xffff);
--
2.7.4
^ permalink raw reply related
* [PATCH v4 6/7] dmaengine: imx-sdma: add sdma_transfer_init to decrease code overlap
From: Robin Gong @ 2018-06-14 14:03 UTC (permalink / raw)
To: vkoul, s.hauer, l.stach, dan.j.williams, gregkh, jslaby
Cc: linux-serial, dmaengine, linux-kernel, linux-arm-kernel,
linux-imx
In-Reply-To: <1528984982-5074-1-git-send-email-yibin.gong@nxp.com>
There are lot of codes overlap between prep_sg and prep_cyclic function.
Add sdma_transfer_init() function to elimated the code overlap.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 83 ++++++++++++++++++++++----------------------------
1 file changed, 37 insertions(+), 46 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index f56226f..e0783a2 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1255,6 +1255,40 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
clk_disable(sdma->clk_ahb);
}
+static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac,
+ enum dma_transfer_direction direction, u32 bds)
+{
+ struct sdma_desc *desc;
+
+ desc = kzalloc((sizeof(*desc)), GFP_KERNEL);
+ if (!desc)
+ goto err_out;
+
+ sdmac->status = DMA_IN_PROGRESS;
+ sdmac->direction = direction;
+ sdmac->flags = 0;
+
+ desc->chn_count = 0;
+ desc->chn_real_count = 0;
+ desc->buf_tail = 0;
+ desc->buf_ptail = 0;
+ desc->sdmac = sdmac;
+ desc->num_bd = bds;
+
+ if (sdma_alloc_bd(desc))
+ goto err_desc_out;
+
+ if (sdma_load_context(sdmac))
+ goto err_desc_out;
+
+ return desc;
+
+err_desc_out:
+ kfree(desc);
+err_out:
+ return NULL;
+}
+
static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
struct dma_chan *chan, struct scatterlist *sgl,
unsigned int sg_len, enum dma_transfer_direction direction,
@@ -1267,36 +1301,13 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
struct scatterlist *sg;
struct sdma_desc *desc;
- if (sdmac->status == DMA_IN_PROGRESS)
- return NULL;
- sdmac->status = DMA_IN_PROGRESS;
-
- sdmac->flags = 0;
-
- desc = kzalloc((sizeof(*desc)), GFP_KERNEL);
+ desc = sdma_transfer_init(sdmac, direction, sg_len);
if (!desc)
goto err_out;
- desc->buf_tail = 0;
- desc->buf_ptail = 0;
- desc->sdmac = sdmac;
- desc->num_bd = sg_len;
- desc->chn_real_count = 0;
-
- if (sdma_alloc_bd(desc)) {
- kfree(desc);
- goto err_out;
- }
-
dev_dbg(sdma->dev, "setting up %d entries for channel %d.\n",
sg_len, channel);
- sdmac->direction = direction;
- ret = sdma_load_context(sdmac);
- if (ret)
- goto err_bd_out;
-
- desc->chn_count = 0;
for_each_sg(sgl, sg, sg_len, i) {
struct sdma_buffer_descriptor *bd = &desc->bd[i];
int param;
@@ -1372,38 +1383,18 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
struct sdma_engine *sdma = sdmac->sdma;
int num_periods = buf_len / period_len;
int channel = sdmac->channel;
- int ret, i = 0, buf = 0;
+ int i = 0, buf = 0;
struct sdma_desc *desc;
dev_dbg(sdma->dev, "%s channel: %d\n", __func__, channel);
- if (sdmac->status == DMA_IN_PROGRESS)
- return NULL;
-
- sdmac->status = DMA_IN_PROGRESS;
-
- desc = kzalloc((sizeof(*desc)), GFP_KERNEL);
+ desc = sdma_transfer_init(sdmac, direction, num_periods);
if (!desc)
goto err_out;
- desc->buf_tail = 0;
- desc->buf_ptail = 0;
- desc->sdmac = sdmac;
- desc->num_bd = num_periods;
- desc->chn_real_count = 0;
desc->period_len = period_len;
sdmac->flags |= IMX_DMA_SG_LOOP;
- sdmac->direction = direction;
-
- if (sdma_alloc_bd(desc)) {
- kfree(desc);
- goto err_bd_out;
- }
-
- ret = sdma_load_context(sdmac);
- if (ret)
- goto err_bd_out;
if (period_len > 0xffff) {
dev_err(sdma->dev, "SDMA channel %d: maximum period size exceeded: %zu > %d\n",
--
2.7.4
^ permalink raw reply related
* [PATCH v4 7/7] tty: serial: imx: split all dma setup operations out of 'port.lock' protector
From: Robin Gong @ 2018-06-14 14:03 UTC (permalink / raw)
To: vkoul, s.hauer, l.stach, dan.j.williams, gregkh, jslaby
Cc: linux-serial, dmaengine, linux-kernel, linux-arm-kernel,
linux-imx
In-Reply-To: <1528984982-5074-1-git-send-email-yibin.gong@nxp.com>
After sdma driver change to virt-dma, all bds will be allocated
dynamically with 'port.lock' acquired instead of statically allocated
before. That means the lock sequence is 'port.lock' -> 'fs_reclaim_acquire'
.But in case uart rx/tx dma callback coming after other kernel code which
have already acquired 'fs_reclaim_acquire' lock, which means the above lock
sequence reverted as 'fs_reclaim_acquire' -> 'port.lock'(acquired in uart
dma callback), thus, lockdep warning comes as beow. Actually don't need to
spinlock all DMA operations in UART driver with 'port.lock', because dma
driver can wipe off race condition by commone virt-dma lock . Split all dma
operations out of the code areas which protected by 'port.lock'.
[ 46.155406] =====================================================
[ 46.161503] WARNING: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
[ 46.168122] 4.17.0-rc6-00008-g7caafa3-dirty #48 Not tainted
[ 46.173696] -----------------------------------------------------
[ 46.179795] mxc_uart_stress/419 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
[ 46.186934] fa7c1440 (fs_reclaim){+.+.}, at: fs_reclaim_acquire.part.3+0x0/0x48
[ 46.194270]
[ 46.194270] and this task is already holding:
[ 46.200106] 09a17fda (&port_lock_key){-.-.}, at: uart_write+0x84/0x190
[ 46.206658] which would create a new lock dependency:
[ 46.211710] (&port_lock_key){-.-.} -> (fs_reclaim){+.+.}
[ 46.217132]
[ 46.217132] but this new dependency connects a HARDIRQ-irq-safe lock:
[ 46.225051] (&port_lock_key){-.-.}
[ 46.225062]
[ 46.225062] ... which became HARDIRQ-irq-safe at:
[ 46.234740] lock_acquire+0x70/0x90
[ 46.238326] _raw_spin_lock_irqsave+0x40/0x54
[ 46.242777] imx_uart_console_write+0x1bc/0x1e0
[ 46.247402] console_unlock+0x320/0x5f0
[ 46.251329] vprintk_emit+0x22c/0x3fc
[ 46.255082] vprintk_default+0x28/0x30
[ 46.258923] vprintk_func+0x78/0xcc
[ 46.262503] printk+0x34/0x54
[ 46.265566] crng_fast_load+0xf8/0x138
[ 46.269407] add_interrupt_randomness+0x21c/0x24c
[ 46.274204] handle_irq_event_percpu+0x40/0x84
[ 46.278739] handle_irq_event+0x40/0x64
[ 46.282667] handle_fasteoi_irq+0xbc/0x178
[ 46.286854] generic_handle_irq+0x28/0x3c
[ 46.290954] __handle_domain_irq+0x6c/0xe8
[ 46.295148] gic_handle_irq+0x64/0xc4
[ 46.298904] __irq_svc+0x70/0x98
[ 46.302225] _raw_spin_unlock_irq+0x30/0x34
[ 46.306505] finish_task_switch+0xc0/0x27c
[ 46.310693] __schedule+0x2c0/0x79c
[ 46.314272] schedule_idle+0x40/0x84
[ 46.317941] do_idle+0x178/0x2b4
[ 46.321259] cpu_startup_entry+0x20/0x24
[ 46.325278] rest_init+0x214/0x264
[ 46.328775] start_kernel+0x39c/0x424
[ 46.332527] (null)
[ 46.334891]
[ 46.334891] to a HARDIRQ-irq-unsafe lock:
[ 46.340379] (fs_reclaim){+.+.}
[ 46.340391]
[ 46.340391] ... which became HARDIRQ-irq-unsafe at:
[ 46.349885] ...
[ 46.349895] lock_acquire+0x70/0x90
[ 46.355225] fs_reclaim_acquire.part.3+0x38/0x48
[ 46.359933] fs_reclaim_acquire+0x1c/0x20
[ 46.364036] kmem_cache_alloc+0x2c/0x174
[ 46.368051] alloc_worker.constprop.10+0x1c/0x58
[ 46.372759] init_rescuer.part.4+0x18/0xa4
[ 46.376952] workqueue_init+0xc0/0x210
[ 46.380793] kernel_init_freeable+0x58/0x1d8
[ 46.385156] kernel_init+0x10/0x11c
[ 46.388736] ret_from_fork+0x14/0x20
[ 46.392399] (null)
[ 46.394762]
[ 46.394762] other info that might help us debug this:
[ 46.394762]
[ 46.402769] Possible interrupt unsafe locking scenario:
[ 46.402769]
[ 46.409560] CPU0 CPU1
[ 46.414092] ---- ----
[ 46.418622] lock(fs_reclaim);
[ 46.421772] local_irq_disable();
[ 46.427693] lock(&port_lock_key);
[ 46.433707] lock(fs_reclaim);
[ 46.439372] <Interrupt>
[ 46.441993] lock(&port_lock_key);
[ 46.445661]
[ 46.445661] *** DEADLOCK ***
[ 46.445661]
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/tty/serial/imx.c | 97 ++++++++++++++++++++++++++----------------------
1 file changed, 53 insertions(+), 44 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index b83bc2c..f2a2966 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -223,6 +223,7 @@ struct imx_port {
dma_cookie_t rx_cookie;
unsigned int tx_bytes;
unsigned int dma_tx_nents;
+ struct work_struct tsk_dma_tx;
unsigned int saved_reg[10];
bool context_saved;
};
@@ -491,8 +492,6 @@ static void imx_uart_enable_ms(struct uart_port *port)
mctrl_gpio_enable_ms(sport->gpios);
}
-static void imx_uart_dma_tx(struct imx_port *sport);
-
/* called with port.lock taken and irqs off */
static inline void imx_uart_transmit_buffer(struct imx_port *sport)
{
@@ -524,7 +523,7 @@ static inline void imx_uart_transmit_buffer(struct imx_port *sport)
imx_uart_writel(sport, ucr1, UCR1);
} else {
imx_uart_writel(sport, ucr1, UCR1);
- imx_uart_dma_tx(sport);
+ schedule_work(&sport->tsk_dma_tx);
}
return;
@@ -574,7 +573,7 @@ static void imx_uart_dma_tx_callback(void *data)
uart_write_wakeup(&sport->port);
if (!uart_circ_empty(xmit) && !uart_tx_stopped(&sport->port))
- imx_uart_dma_tx(sport);
+ schedule_work(&sport->tsk_dma_tx);
else if (sport->port.rs485.flags & SER_RS485_ENABLED) {
u32 ucr4 = imx_uart_readl(sport, UCR4);
ucr4 |= UCR4_TCEN;
@@ -584,19 +583,21 @@ static void imx_uart_dma_tx_callback(void *data)
spin_unlock_irqrestore(&sport->port.lock, flags);
}
-/* called with port.lock taken and irqs off */
-static void imx_uart_dma_tx(struct imx_port *sport)
+static void dma_tx_work(struct work_struct *w)
{
+ struct imx_port *sport = container_of(w, struct imx_port, tsk_dma_tx);
struct circ_buf *xmit = &sport->port.state->xmit;
struct scatterlist *sgl = sport->tx_sgl;
struct dma_async_tx_descriptor *desc;
struct dma_chan *chan = sport->dma_chan_tx;
struct device *dev = sport->port.dev;
+ unsigned long flags;
u32 ucr1, ucr4;
int ret;
+ spin_lock_irqsave(&sport->port.lock, flags);
if (sport->dma_is_txing)
- return;
+ goto work_out;
ucr4 = imx_uart_readl(sport, UCR4);
ucr4 &= ~UCR4_TCEN;
@@ -604,45 +605,51 @@ static void imx_uart_dma_tx(struct imx_port *sport)
sport->tx_bytes = uart_circ_chars_pending(xmit);
- if (xmit->tail < xmit->head) {
- sport->dma_tx_nents = 1;
- sg_init_one(sgl, xmit->buf + xmit->tail, sport->tx_bytes);
- } else {
- sport->dma_tx_nents = 2;
- sg_init_table(sgl, 2);
- sg_set_buf(sgl, xmit->buf + xmit->tail,
- UART_XMIT_SIZE - xmit->tail);
- sg_set_buf(sgl + 1, xmit->buf, xmit->head);
- }
+ if (sport->tx_bytes > 0) {
+ if (xmit->tail < xmit->head) {
+ sport->dma_tx_nents = 1;
+ sg_init_one(sgl, xmit->buf + xmit->tail,
+ sport->tx_bytes);
+ } else {
+ sport->dma_tx_nents = 2;
+ sg_init_table(sgl, 2);
+ sg_set_buf(sgl, xmit->buf + xmit->tail,
+ UART_XMIT_SIZE - xmit->tail);
+ sg_set_buf(sgl + 1, xmit->buf, xmit->head);
+ }
+ spin_unlock_irqrestore(&sport->port.lock, flags);
- ret = dma_map_sg(dev, sgl, sport->dma_tx_nents, DMA_TO_DEVICE);
- if (ret == 0) {
- dev_err(dev, "DMA mapping error for TX.\n");
- return;
- }
- desc = dmaengine_prep_slave_sg(chan, sgl, sport->dma_tx_nents,
+ ret = dma_map_sg(dev, sgl, sport->dma_tx_nents, DMA_TO_DEVICE);
+ if (ret == 0) {
+ dev_err(dev, "DMA mapping error for TX.\n");
+ return;
+ }
+ desc = dmaengine_prep_slave_sg(chan, sgl, sport->dma_tx_nents,
DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT);
- if (!desc) {
- dma_unmap_sg(dev, sgl, sport->dma_tx_nents,
- DMA_TO_DEVICE);
- dev_err(dev, "We cannot prepare for the TX slave dma!\n");
- return;
- }
- desc->callback = imx_uart_dma_tx_callback;
- desc->callback_param = sport;
+ if (!desc) {
+ dma_unmap_sg(dev, sgl, sport->dma_tx_nents,
+ DMA_TO_DEVICE);
+ dev_err(dev, "We cannot prepare for the TX slave dma!\n");
+ return;
+ }
+ desc->callback = imx_uart_dma_tx_callback;
+ desc->callback_param = sport;
- dev_dbg(dev, "TX: prepare to send %lu bytes by DMA.\n",
- uart_circ_chars_pending(xmit));
+ dev_dbg(dev, "TX: prepare to send %lu bytes by DMA.\n",
+ uart_circ_chars_pending(xmit));
- ucr1 = imx_uart_readl(sport, UCR1);
- ucr1 |= UCR1_TXDMAEN;
- imx_uart_writel(sport, ucr1, UCR1);
+ ucr1 = imx_uart_readl(sport, UCR1);
+ ucr1 |= UCR1_TXDMAEN;
+ imx_uart_writel(sport, ucr1, UCR1);
- /* fire it */
- sport->dma_is_txing = 1;
- dmaengine_submit(desc);
- dma_async_issue_pending(chan);
- return;
+ /* fire it */
+ sport->dma_is_txing = 1;
+ dmaengine_submit(desc);
+ dma_async_issue_pending(chan);
+ return;
+ }
+work_out:
+ spin_unlock_irqrestore(&sport->port.lock, flags);
}
/* called with port.lock taken and irqs off */
@@ -696,7 +703,7 @@ static void imx_uart_start_tx(struct uart_port *port)
if (!uart_circ_empty(&port->state->xmit) &&
!uart_tx_stopped(port))
- imx_uart_dma_tx(sport);
+ schedule_work(&sport->tsk_dma_tx);
return;
}
}
@@ -1405,7 +1412,9 @@ static int imx_uart_startup(struct uart_port *port)
*/
imx_uart_enable_ms(&sport->port);
+ spin_unlock_irqrestore(&sport->port.lock, flags);
if (dma_is_inited) {
+ INIT_WORK(&sport->tsk_dma_tx, dma_tx_work);
imx_uart_enable_dma(sport);
imx_uart_start_rx_dma(sport);
} else {
@@ -1418,8 +1427,6 @@ static int imx_uart_startup(struct uart_port *port)
imx_uart_writel(sport, ucr2, UCR2);
}
- spin_unlock_irqrestore(&sport->port.lock, flags);
-
return 0;
}
@@ -1435,6 +1442,8 @@ static void imx_uart_shutdown(struct uart_port *port)
dmaengine_terminate_sync(sport->dma_chan_tx);
dmaengine_terminate_sync(sport->dma_chan_rx);
+ cancel_work_sync(&sport->tsk_dma_tx);
+
spin_lock_irqsave(&sport->port.lock, flags);
imx_uart_stop_tx(port);
imx_uart_stop_rx(port);
--
2.7.4
^ permalink raw reply related
* [PATCH 4.17 25/45] serial: sh-sci: Stop using printk format %pCr
From: Greg Kroah-Hartman @ 2018-06-14 14:04 UTC (permalink / raw)
To: linux-kernel, Jia-Ju Bai, Jonathan Corbet, Michael Turquette,
Stephen Boyd, Zhang Rui, Eduardo Valentin, Eric Anholt,
Stefan Wahren
Cc: Greg Kroah-Hartman, stable, Sergey Senozhatsky, Petr Mladek,
Linus Torvalds, Steven Rostedt, linux-doc, linux-clk, linux-pm,
linux-serial, linux-arm-kernel, linux-renesas-soc,
Geert Uytterhoeven
In-Reply-To: <20180614132126.797006529@linuxfoundation.org>
4.17-stable review patch. If anyone has any objections, please let me know.
------------------
From: Geert Uytterhoeven <geert+renesas@glider.be>
commit d63c16f8e1ab761775275adcf54f4bef7c330295 upstream.
Printk format "%pCr" will be removed soon, as clk_get_rate() must not be
called in atomic context.
Replace it by open-coding the operation. This is safe here, as the code
runs in task context.
Link: http://lkml.kernel.org/r/1527845302-12159-4-git-send-email-geert+renesas@glider.be
To: Jia-Ju Bai <baijiaju1990@gmail.com>
To: Jonathan Corbet <corbet@lwn.net>
To: Michael Turquette <mturquette@baylibre.com>
To: Stephen Boyd <sboyd@kernel.org>
To: Zhang Rui <rui.zhang@intel.com>
To: Eduardo Valentin <edubezval@gmail.com>
To: Eric Anholt <eric@anholt.net>
To: Stefan Wahren <stefan.wahren@i2se.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-doc@vger.kernel.org
Cc: linux-clk@vger.kernel.org
Cc: linux-pm@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: stable@vger.kernel.org # 4.5+
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/tty/serial/sh-sci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2704,8 +2704,8 @@ found:
dev_dbg(dev, "failed to get %s (%ld)\n", clk_names[i],
PTR_ERR(clk));
else
- dev_dbg(dev, "clk %s is %pC rate %pCr\n", clk_names[i],
- clk, clk);
+ dev_dbg(dev, "clk %s is %pC rate %lu\n", clk_names[i],
+ clk, clk_get_rate(clk));
sci_port->clks[i] = IS_ERR(clk) ? NULL : clk;
}
return 0;
^ permalink raw reply
* [PATCH 4.16 25/43] serial: sh-sci: Stop using printk format %pCr
From: Greg Kroah-Hartman @ 2018-06-14 14:04 UTC (permalink / raw)
To: linux-kernel, Jia-Ju Bai, Jonathan Corbet, Michael Turquette,
Stephen Boyd, Zhang Rui, Eduardo Valentin, Eric Anholt,
Stefan Wahren
Cc: Petr Mladek, Sergey Senozhatsky, Geert Uytterhoeven, linux-doc,
Greg Kroah-Hartman, linux-pm, Steven Rostedt, linux-renesas-soc,
stable, linux-serial, Linus Torvalds, linux-clk, linux-arm-kernel
In-Reply-To: <20180614132135.111973468@linuxfoundation.org>
4.16-stable review patch. If anyone has any objections, please let me know.
------------------
From: Geert Uytterhoeven <geert+renesas@glider.be>
commit d63c16f8e1ab761775275adcf54f4bef7c330295 upstream.
Printk format "%pCr" will be removed soon, as clk_get_rate() must not be
called in atomic context.
Replace it by open-coding the operation. This is safe here, as the code
runs in task context.
Link: http://lkml.kernel.org/r/1527845302-12159-4-git-send-email-geert+renesas@glider.be
To: Jia-Ju Bai <baijiaju1990@gmail.com>
To: Jonathan Corbet <corbet@lwn.net>
To: Michael Turquette <mturquette@baylibre.com>
To: Stephen Boyd <sboyd@kernel.org>
To: Zhang Rui <rui.zhang@intel.com>
To: Eduardo Valentin <edubezval@gmail.com>
To: Eric Anholt <eric@anholt.net>
To: Stefan Wahren <stefan.wahren@i2se.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-doc@vger.kernel.org
Cc: linux-clk@vger.kernel.org
Cc: linux-pm@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: stable@vger.kernel.org # 4.5+
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/tty/serial/sh-sci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2691,8 +2691,8 @@ found:
dev_dbg(dev, "failed to get %s (%ld)\n", clk_names[i],
PTR_ERR(clk));
else
- dev_dbg(dev, "clk %s is %pC rate %pCr\n", clk_names[i],
- clk, clk);
+ dev_dbg(dev, "clk %s is %pC rate %lu\n", clk_names[i],
+ clk, clk_get_rate(clk));
sci_port->clks[i] = IS_ERR(clk) ? NULL : clk;
}
return 0;
^ permalink raw reply
* [PATCH 4.14 18/36] serial: sh-sci: Stop using printk format %pCr
From: Greg Kroah-Hartman @ 2018-06-14 14:04 UTC (permalink / raw)
To: linux-kernel, Jia-Ju Bai, Jonathan Corbet, Michael Turquette,
Stephen Boyd, Zhang Rui, Eduardo Valentin, Eric Anholt,
Stefan Wahren
Cc: Petr Mladek, Sergey Senozhatsky, Geert Uytterhoeven, linux-doc,
Greg Kroah-Hartman, linux-pm, Steven Rostedt, linux-renesas-soc,
stable, linux-serial, Linus Torvalds, linux-clk, linux-arm-kernel
In-Reply-To: <20180614132157.333004166@linuxfoundation.org>
4.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Geert Uytterhoeven <geert+renesas@glider.be>
commit d63c16f8e1ab761775275adcf54f4bef7c330295 upstream.
Printk format "%pCr" will be removed soon, as clk_get_rate() must not be
called in atomic context.
Replace it by open-coding the operation. This is safe here, as the code
runs in task context.
Link: http://lkml.kernel.org/r/1527845302-12159-4-git-send-email-geert+renesas@glider.be
To: Jia-Ju Bai <baijiaju1990@gmail.com>
To: Jonathan Corbet <corbet@lwn.net>
To: Michael Turquette <mturquette@baylibre.com>
To: Stephen Boyd <sboyd@kernel.org>
To: Zhang Rui <rui.zhang@intel.com>
To: Eduardo Valentin <edubezval@gmail.com>
To: Eric Anholt <eric@anholt.net>
To: Stefan Wahren <stefan.wahren@i2se.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-doc@vger.kernel.org
Cc: linux-clk@vger.kernel.org
Cc: linux-pm@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: stable@vger.kernel.org # 4.5+
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/tty/serial/sh-sci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2669,8 +2669,8 @@ found:
dev_dbg(dev, "failed to get %s (%ld)\n", clk_names[i],
PTR_ERR(clk));
else
- dev_dbg(dev, "clk %s is %pC rate %pCr\n", clk_names[i],
- clk, clk);
+ dev_dbg(dev, "clk %s is %pC rate %lu\n", clk_names[i],
+ clk, clk_get_rate(clk));
sci_port->clks[i] = IS_ERR(clk) ? NULL : clk;
}
return 0;
^ permalink raw reply
* [PATCH 4.9 19/30] serial: sh-sci: Stop using printk format %pCr
From: Greg Kroah-Hartman @ 2018-06-14 14:05 UTC (permalink / raw)
To: linux-kernel, Jia-Ju Bai, Jonathan Corbet, Michael Turquette,
Stephen Boyd, Zhang Rui, Eduardo Valentin, Eric Anholt,
Stefan Wahren
Cc: Greg Kroah-Hartman, stable, Sergey Senozhatsky, Petr Mladek,
Linus Torvalds, Steven Rostedt, linux-doc, linux-clk, linux-pm,
linux-serial, linux-arm-kernel, linux-renesas-soc,
Geert Uytterhoeven
In-Reply-To: <20180614132600.255515394@linuxfoundation.org>
4.9-stable review patch. If anyone has any objections, please let me know.
------------------
From: Geert Uytterhoeven <geert+renesas@glider.be>
commit d63c16f8e1ab761775275adcf54f4bef7c330295 upstream.
Printk format "%pCr" will be removed soon, as clk_get_rate() must not be
called in atomic context.
Replace it by open-coding the operation. This is safe here, as the code
runs in task context.
Link: http://lkml.kernel.org/r/1527845302-12159-4-git-send-email-geert+renesas@glider.be
To: Jia-Ju Bai <baijiaju1990@gmail.com>
To: Jonathan Corbet <corbet@lwn.net>
To: Michael Turquette <mturquette@baylibre.com>
To: Stephen Boyd <sboyd@kernel.org>
To: Zhang Rui <rui.zhang@intel.com>
To: Eduardo Valentin <edubezval@gmail.com>
To: Eric Anholt <eric@anholt.net>
To: Stefan Wahren <stefan.wahren@i2se.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-doc@vger.kernel.org
Cc: linux-clk@vger.kernel.org
Cc: linux-pm@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: stable@vger.kernel.org # 4.5+
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/tty/serial/sh-sci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2626,8 +2626,8 @@ found:
dev_dbg(dev, "failed to get %s (%ld)\n", clk_names[i],
PTR_ERR(clk));
else
- dev_dbg(dev, "clk %s is %pC rate %pCr\n", clk_names[i],
- clk, clk);
+ dev_dbg(dev, "clk %s is %pC rate %lu\n", clk_names[i],
+ clk, clk_get_rate(clk));
sci_port->clks[i] = IS_ERR(clk) ? NULL : clk;
}
return 0;
^ permalink raw reply
* Re: [PATCH v2 00/19] Dynamically load/remove serdev devices via sysfs*
From: Ricardo Ribalda Delgado @ 2018-06-14 14:06 UTC (permalink / raw)
To: Johan Hovold; +Cc: LKML, open list:SERIAL DRIVERS, Rob Herring, Andy Shevchenko
In-Reply-To: <20180614133341.GD32411@localhost>
Hi Johan,
On Thu, Jun 14, 2018 at 3:34 PM Johan Hovold <johan@kernel.org> wrote:
>
> On Thu, Jun 14, 2018 at 01:06:59PM +0200, Ricardo Ribalda Delgado wrote:
> > Hi Johan
> > On Thu, Jun 14, 2018 at 12:48 PM Johan Hovold <johan@kernel.org> wrote:
> > >
> > > Hi Ricardo,
> > >
> > > On Mon, Jun 11, 2018 at 01:52:16PM +0200, Ricardo Ribalda Delgado wrote:
> > > > There are some situations where it is interesting to load/remove serdev
> > > > devices dynamically, like during board bring-up or when we are
> > > > developing a new driver or for devices that are neither described via
> > > > ACPI or device tree.
> > >
> > > First of all, this would be more appropriately labeled an RFC as this is
> > > far from being in a mergeable state. Besides some implementation issues,
> > > we need to determine if this approach is at all viable.
> >
> > From previous conversations with Greg it seemed that RFC labels was
> > something to avoid, but I do not mind reseding it as RFC on v3.
> >
> > http://lists.kernelnewbies.org/pipermail/kernelnewbies/2018-March/018844.html
>
> Yeah, Greg uses that to triage his insane workload, but RFCs still have
> their use (and are still mentioned in submitting-patches.rst).
>
> > > Second, I wonder how you tested this given that this series breaks RX
> > > (and write-wakeup signalling) for serial ports (patch 19/24)?
> >
> > I have a serdev device (led ctrls) that is dynamically enumerated with
> > something very similar to:
> >
> > https://github.com/ribalda/linux/commit/415bb3f0076c2b846ebe5409589b8e1e3004f55a
> >
> > and then I have a script that does adds and removes. For standard
> > serial port I was not testing the data path, just that the ttyS* was
> > enumerated fine.
>
> Well there's more to serial ports than just enumeration, you typically
> want to send and receive data as well. ;)
>
> > But yesterday I believe that we found the bug that you mentioned and
> > we have fixed it (check end of mail). I will patch the series and
> > resend after I get more feedback and also implement what Marcel
> > suggested.
> >
> > WIP is at
> > https://github.com/ribalda/linux/tree/serdev3
> >
> > Besides this bug, we have used the new driver for over a week now with
> > no issues.
>
> Hmm... No issues when not testing the main functionality of serial
> ports, you mean?
>
> And there are more issues with the series which are less apparent than
> the rx (and partial tx) regression.
Any hints about this? What else should I change on the series?
>
> > > Third, and as Marcel already suggested, you need to limit your scope
> > > here. Aim at ten patches or so, and use a representative serdev driver
> > > as an example of the kind of driver updates that would be needed. It
> > > also looks like some patches should be squashed (e.g. the ones
> > > introducing new fields and the first one actually using them).
> >
> > >
> > > > This implementation allows the creation of serdev devices via sysfs,
> > > > in a similar way as the i2c bus allows sysfs instantiation [1].
> > >
> > > Note that this is a legacy interface and not necessarily something that
> > > new interfaces should be modelled after.
> >
> > I would not consider it legacy, it is the only way to use an i2c
> > module without writing your own driver and/or modifying ACPI/DT table.
>
> It's legacy as in old, and to be used for one-off hacks and such. But
> sure, that is also what this series aims at even if that doesn't mean
> you *have to* copy the interface.
It is not only one-off hack. It is the ONLY way to use i2c devices
that are not enumerated.
The same way as today we do not have any way of using serdev on non
enumerated devices.
>
> > Author: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> > Date: Thu Jun 14 11:30:27 2018 +0200
> >
> > serdev-ttydev: Restore/change ttyport client ops
>
> Yep, you found the source of the broken serial port rx/tx.
>
> Johan
--
Ricardo Ribalda
^ permalink raw reply
* Re: [PATCH 2/7] clk: intel: Add clock driver for GRX500 SoC
From: Rob Herring @ 2018-06-14 14:09 UTC (permalink / raw)
To: yixin zhu
Cc: Songjun Wu, hua.ma, chuanhua.lei, Linux-MIPS, qi-ming.wu,
linux-clk, open list:SERIAL DRIVERS, devicetree,
Michael Turquette, Stephen Boyd, linux-kernel@vger.kernel.org,
Mark Rutland
In-Reply-To: <41163f48-ce5c-efae-2b6d-b93d75e422e5@linux.intel.com>
On Thu, Jun 14, 2018 at 2:40 AM, yixin zhu <yixin.zhu@linux.intel.com> wrote:
>
>
> On 6/13/2018 6:37 AM, Rob Herring wrote:
>>
>> On Tue, Jun 12, 2018 at 01:40:29PM +0800, Songjun Wu wrote:
>>>
>>> From: Yixin Zhu <yixin.zhu@linux.intel.com>
>>>
>>> PLL of GRX500 provide clock to DDR, CPU, and peripherals as show below
[...]
>>> +Example:
>>> + clkgate0: clkgate0 {
>>> + #clock-cells = <1>;
>>> + compatible = "intel,grx500-gate0-clk";
>>> + reg = <0x114>;
>>> + clock-output-names = "gate_xbar0", "gate_xbar1",
>>> "gate_xbar2",
>>> + "gate_xbar3", "gate_xbar6", "gate_xbar7";
>>> + };
>>
>>
>> We generally don't do a clock node per clock or few clocks but rather 1
>> clock node per clock controller block. See any recent clock bindings.
>>
>> Rob
>
> Do you mean only one example is needed per clock controller block?
> cpuclk is not needed in the document?
No, I mean generally we have 1 DT node for the h/w block with all the
clock control registers rather than nodes with a single register and 1
or a couple of clocks. Sometimes the clock registers are mixed with
other functions which complicates things a bit. But I can't tell that
here because you haven't documented what's in the rest of the register
space.
Rob
^ permalink raw reply
* Re: [PATCH v2 00/19] Dynamically load/remove serdev devices via sysfs*
From: Johan Hovold @ 2018-06-14 14:55 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: Johan Hovold, LKML, open list:SERIAL DRIVERS, Rob Herring,
Andy Shevchenko
In-Reply-To: <CAPybu_3=KhPB_ymtRNSZfcyP57LPD2SbLUPoFO0YdjkzAsdaTA@mail.gmail.com>
On Thu, Jun 14, 2018 at 04:06:18PM +0200, Ricardo Ribalda Delgado wrote:
> Hi Johan,
> On Thu, Jun 14, 2018 at 3:34 PM Johan Hovold <johan@kernel.org> wrote:
> > And there are more issues with the series which are less apparent than
> > the rx (and partial tx) regression.
>
> Any hints about this? What else should I change on the series?
There are implementation issues and there's the more fundamental
question about whether your approach to this is the right one.
Like Rob, I'm not sure we want to have the device topology depend on a
kernel config symbol (serdev and your ttydev driver). We may need to
explore Rob's sibling-device idea further.
I also want to make sure that this can be used for discoverable buses
(e.g. the USB CEC device the I've used as an example before).
As for the current implementation there are both larger and smaller
issues, like for example:
- the fact that your sysfs and lookup interface does not use any
locking whatsoever and thus is susceptible to races
- your ttyport driver currently breaks the sysfs interface for all
serial (core) devices by ignoring the attribute groups
- the ttyport driver is arguably a hack with layering issues (which
admittedly may be hard to avoid given the retrofitting of serdev into
the tty layer)
Again, I suggest you submit a subset of your series (aim at 10 patches
or so) as an RFC which can be used as a basis for further discussion. No
point in discussing every implementation detail if the underlying
approach needs to be revised.
> > It's legacy as in old, and to be used for one-off hacks and such. But
> > sure, that is also what this series aims at even if that doesn't mean
> > you *have to* copy the interface.
>
> It is not only one-off hack. It is the ONLY way to use i2c devices
> that are not enumerated.
>
> The same way as today we do not have any way of using serdev on non
> enumerated devices.
You're missing the point: none of that means you have to copy the
interface.
Johan
^ permalink raw reply
* Re: [PATCH v2 00/19] Dynamically load/remove serdev devices via sysfs*
From: Ricardo Ribalda Delgado @ 2018-06-14 15:20 UTC (permalink / raw)
To: Johan Hovold; +Cc: LKML, open list:SERIAL DRIVERS, Rob Herring, Andy Shevchenko
In-Reply-To: <20180614145531.GE32411@localhost>
Hi Johan,
On Thu, Jun 14, 2018 at 4:55 PM Johan Hovold <johan@kernel.org> wrote:
>
> On Thu, Jun 14, 2018 at 04:06:18PM +0200, Ricardo Ribalda Delgado wrote:
> > Hi Johan,
> > On Thu, Jun 14, 2018 at 3:34 PM Johan Hovold <johan@kernel.org> wrote:
>
> > > And there are more issues with the series which are less apparent than
> > > the rx (and partial tx) regression.
> >
> > Any hints about this? What else should I change on the series?
>
> There are implementation issues and there's the more fundamental
> question about whether your approach to this is the right one.
>
> Like Rob, I'm not sure we want to have the device topology depend on a
> kernel config symbol (serdev and your ttydev driver). We may need to
> explore Rob's sibling-device idea further.
>From my point of view, if the user enables serdev, then everything has
to be a serdev, because serdev does not provide the same functionality
as a core tty device I had to implement, serdev-ttydev.c. Which is
nothing more than a wrapper.
It is very hacky, but allows replacing the core tty device with another serdev.
>
> I also want to make sure that this can be used for discoverable buses
> (e.g. the USB CEC device the I've used as an example before).
>
I have tried your patch:
https://github.com/ribalda/linux/commit/5cb30b4ce6477132a23492c674d8b3dc81ecff86
the only issue is that the serdev device sometimes explotes (OOPS)
when the usb is unplugged :S.
And that might be quite tricy to solve
> As for the current implementation there are both larger and smaller
> issues, like for example:
>
> - the fact that your sysfs and lookup interface does not use any
> locking whatsoever and thus is susceptible to races
I thought that sysfs access where serialised. If that is not the case
yes, we need a lock.
>
> - your ttyport driver currently breaks the sysfs interface for all
> serial (core) devices by ignoring the attribute groups
Yep, you are right, I screwed up that one :).
>
> - the ttyport driver is arguably a hack with layering issues (which
> admittedly may be hard to avoid given the retrofitting of serdev into
> the tty layer)
>
> Again, I suggest you submit a subset of your series (aim at 10 patches
> or so) as an RFC which can be used as a basis for further discussion. No
> point in discussing every implementation detail if the underlying
> approach needs to be revised.
Will do. Give me some time to give it a hand of paint.
Thanks for time reviewing my little moster
>
> > > It's legacy as in old, and to be used for one-off hacks and such. But
> > > sure, that is also what this series aims at even if that doesn't mean
> > > you *have to* copy the interface.
> >
> > It is not only one-off hack. It is the ONLY way to use i2c devices
> > that are not enumerated.
> >
> > The same way as today we do not have any way of using serdev on non
> > enumerated devices.
>
> You're missing the point: none of that means you have to copy the
> interface.
>
> Johan
--
Ricardo Ribalda
^ permalink raw reply
* Re: [PATCH v2 00/19] Dynamically load/remove serdev devices via sysfs*
From: Johan Hovold @ 2018-06-14 15:47 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: Johan Hovold, LKML, open list:SERIAL DRIVERS, Rob Herring,
Andy Shevchenko
In-Reply-To: <CAPybu_1ps+tEr5W_uvMSeH4_gwbjgKHnz0MoSEJO=Ut0a4sx6Q@mail.gmail.com>
On Thu, Jun 14, 2018 at 05:20:40PM +0200, Ricardo Ribalda Delgado wrote:
> Hi Johan,
> On Thu, Jun 14, 2018 at 4:55 PM Johan Hovold <johan@kernel.org> wrote:
> >
> > On Thu, Jun 14, 2018 at 04:06:18PM +0200, Ricardo Ribalda Delgado wrote:
> > > Hi Johan,
> > > On Thu, Jun 14, 2018 at 3:34 PM Johan Hovold <johan@kernel.org> wrote:
> >
> > > > And there are more issues with the series which are less apparent than
> > > > the rx (and partial tx) regression.
> > >
> > > Any hints about this? What else should I change on the series?
> >
> > There are implementation issues and there's the more fundamental
> > question about whether your approach to this is the right one.
> >
> > Like Rob, I'm not sure we want to have the device topology depend on a
> > kernel config symbol (serdev and your ttydev driver). We may need to
> > explore Rob's sibling-device idea further.
>
> From my point of view, if the user enables serdev, then everything has
> to be a serdev, because serdev does not provide the same functionality
> as a core tty device I had to implement, serdev-ttydev.c. Which is
> nothing more than a wrapper.
>
> It is very hacky, but allows replacing the core tty device with
> another serdev.
Yes, and I'm a bit surprised (and impressed) that you got it to work
(mostly) so easily.
My point was that we probably don't want the tty devices to move around
in the device hierarchy depending on if serdev (and your ttydev driver)
is enabled or not.
> > I also want to make sure that this can be used for discoverable buses
> > (e.g. the USB CEC device the I've used as an example before).
> >
>
> I have tried your patch:
>
> https://github.com/ribalda/linux/commit/5cb30b4ce6477132a23492c674d8b3dc81ecff86
>
> the only issue is that the serdev device sometimes explotes (OOPS)
> when the usb is unplugged :S.
>
> And that might be quite tricy to solve
Yes, we all know serdev doesn't support hotplug, that's why it's not
enabled for usbserial.
But when/if we get that sorted, we may want to be able to reuse some of
the matching infrastructure that a sysfs interface would use also for
discoverable buses (e.g. passing a compatible string to serdev core).
Also note that the interface you're proposing suffers from similar
problems as hotplug in that serdev drivers must be prepared to handle
devices going away at anytime; be it through your delete_device
interface or from a sysfs driver unbind (i.e. already an issue today!).
This would be were the oops comes from.
> > As for the current implementation there are both larger and smaller
> > issues, like for example:
> >
> > - the fact that your sysfs and lookup interface does not use any
> > locking whatsoever and thus is susceptible to races
>
> I thought that sysfs access where serialised. If that is not the case
> yes, we need a lock.
It's only serialised per attribute.
> > - your ttyport driver currently breaks the sysfs interface for all
> > serial (core) devices by ignoring the attribute groups
>
> Yep, you are right, I screwed up that one :).
Easy to miss.
> > - the ttyport driver is arguably a hack with layering issues (which
> > admittedly may be hard to avoid given the retrofitting of serdev into
> > the tty layer)
> >
> > Again, I suggest you submit a subset of your series (aim at 10 patches
> > or so) as an RFC which can be used as a basis for further discussion. No
> > point in discussing every implementation detail if the underlying
> > approach needs to be revised.
>
> Will do. Give me some time to give it a hand of paint.
>
> Thanks for time reviewing my little moster
You're welcome.
Johan
^ permalink raw reply
* [PATCH v3 23/27] devicetree: fix name of pinctrl-bindings.txt
From: Mauro Carvalho Chehab @ 2018-06-14 16:09 UTC (permalink / raw)
To: Linux Doc Mailing List
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, Rob Herring, Mark Rutland, Lee Jones,
Ulf Hansson, Linus Walleij, Greg Kroah-Hartman, Mark Brown,
devicetree, linux-mmc, linux-gpio, linux-serial, linux-spi
In-Reply-To: <cover.1528990947.git.mchehab+samsung@kernel.org>
Rename:
pinctrl-binding.txt -> pinctrl-bindings.txt
In order to match the current name of this file.
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 2 +-
Documentation/devicetree/bindings/mfd/as3722.txt | 2 +-
.../devicetree/bindings/mmc/microchip,sdhci-pic32.txt | 2 +-
Documentation/devicetree/bindings/mmc/sdhci-st.txt | 2 +-
.../devicetree/bindings/pinctrl/pinctrl-max77620.txt | 4 ++--
.../devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt | 4 ++--
Documentation/devicetree/bindings/pinctrl/pinctrl-rk805.txt | 4 ++--
.../devicetree/bindings/serial/microchip,pic32-uart.txt | 2 +-
Documentation/devicetree/bindings/spi/spi-st-ssc.txt | 2 +-
9 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
index c7888d6f6408..880d4d70c9fd 100644
--- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
+++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
@@ -28,7 +28,7 @@ See: Documentation/devicetree/bindings/clock/clock-bindings.txt
- pinctrl-names : a pinctrl state named tsin%d-serial or tsin%d-parallel (where %d is tsin-num)
must be defined for each tsin child node.
- pinctrl-0 : phandle referencing pin configuration for this tsin configuration
-See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
Required properties (tsin (child) node):
diff --git a/Documentation/devicetree/bindings/mfd/as3722.txt b/Documentation/devicetree/bindings/mfd/as3722.txt
index 0b2a6099aa20..5297b2210704 100644
--- a/Documentation/devicetree/bindings/mfd/as3722.txt
+++ b/Documentation/devicetree/bindings/mfd/as3722.txt
@@ -46,7 +46,7 @@ is required:
Following properties are require if pin control setting is required
at boot.
- pinctrl-names: A pinctrl state named "default" be defined, using the
- bindings in pinctrl/pinctrl-binding.txt.
+ bindings in pinctrl/pinctrl-bindings.txt.
- pinctrl[0...n]: Properties to contain the phandle that refer to
different nodes of pin control settings. These nodes represents
the pin control setting of state 0 to state n. Each of these
diff --git a/Documentation/devicetree/bindings/mmc/microchip,sdhci-pic32.txt b/Documentation/devicetree/bindings/mmc/microchip,sdhci-pic32.txt
index 3149297b3933..f064528effed 100644
--- a/Documentation/devicetree/bindings/mmc/microchip,sdhci-pic32.txt
+++ b/Documentation/devicetree/bindings/mmc/microchip,sdhci-pic32.txt
@@ -12,7 +12,7 @@ Required properties:
See: Documentation/devicetree/bindings/clock/clock-bindings.txt
- pinctrl-names: A pinctrl state names "default" must be defined.
- pinctrl-0: Phandle referencing pin configuration of the SDHCI controller.
- See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+ See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
Example:
diff --git a/Documentation/devicetree/bindings/mmc/sdhci-st.txt b/Documentation/devicetree/bindings/mmc/sdhci-st.txt
index 6b3d40ca395e..ccf82b4ee838 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-st.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-st.txt
@@ -20,7 +20,7 @@ Required properties:
- pinctrl-names: A pinctrl state names "default" must be defined.
- pinctrl-0: Phandle referencing pin configuration of the sd/emmc controller.
- See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+ See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
- reg: This must provide the host controller base address and it can also
contain the FlashSS Top register for TX/RX delay used by the driver
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-max77620.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-max77620.txt
index ad4fce3552bb..511fc234558b 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-max77620.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-max77620.txt
@@ -11,9 +11,9 @@ Optional Pinmux properties:
--------------------------
Following properties are required if default setting of pins are required
at boot.
-- pinctrl-names: A pinctrl state named per <pinctrl-binding.txt>.
+- pinctrl-names: A pinctrl state named per <pinctrl-bindings.txt>.
- pinctrl[0...n]: Properties to contain the phandle for pinctrl states per
- <pinctrl-binding.txt>.
+ <pinctrl-bindings.txt>.
The pin configurations are defined as child of the pinctrl states node. Each
sub-node have following properties:
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt
index a677145ae6d1..625a22e2f211 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt
@@ -101,9 +101,9 @@ Optional Pinmux properties:
--------------------------
Following properties are required if default setting of pins are required
at boot.
-- pinctrl-names: A pinctrl state named per <pinctrl-binding.txt>.
+- pinctrl-names: A pinctrl state named per <pinctrl-bindings.txt>.
- pinctrl[0...n]: Properties to contain the phandle for pinctrl states per
- <pinctrl-binding.txt>.
+ <pinctrl-bindings.txt>.
The pin configurations are defined as child of the pinctrl states node. Each
sub-node have following properties:
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-rk805.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-rk805.txt
index eee3dc260934..cbcbd31e3ce8 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-rk805.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-rk805.txt
@@ -10,9 +10,9 @@ Optional Pinmux properties:
--------------------------
Following properties are required if default setting of pins are required
at boot.
-- pinctrl-names: A pinctrl state named per <pinctrl-binding.txt>.
+- pinctrl-names: A pinctrl state named per <pinctrl-bindings.txt>.
- pinctrl[0...n]: Properties to contain the phandle for pinctrl states per
- <pinctrl-binding.txt>.
+ <pinctrl-bindings.txt>.
The pin configurations are defined as child of the pinctrl states node. Each
sub-node have following properties:
diff --git a/Documentation/devicetree/bindings/serial/microchip,pic32-uart.txt b/Documentation/devicetree/bindings/serial/microchip,pic32-uart.txt
index 7a34345d0ca3..c8dd440e9747 100644
--- a/Documentation/devicetree/bindings/serial/microchip,pic32-uart.txt
+++ b/Documentation/devicetree/bindings/serial/microchip,pic32-uart.txt
@@ -8,7 +8,7 @@ Required properties:
See: Documentation/devicetree/bindings/clock/clock-bindings.txt
- pinctrl-names: A pinctrl state names "default" must be defined.
- pinctrl-0: Phandle referencing pin configuration of the UART peripheral.
- See: Documentation/devicetree/bindings/pinctrl/pinctrl-binding.txt
+ See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
Optional properties:
- cts-gpios: CTS pin for UART
diff --git a/Documentation/devicetree/bindings/spi/spi-st-ssc.txt b/Documentation/devicetree/bindings/spi/spi-st-ssc.txt
index fe54959ec957..1bdc4709e474 100644
--- a/Documentation/devicetree/bindings/spi/spi-st-ssc.txt
+++ b/Documentation/devicetree/bindings/spi/spi-st-ssc.txt
@@ -9,7 +9,7 @@ Required properties:
- clocks : Must contain an entry for each name in clock-names
See ../clk/*
- pinctrl-names : Uses "default", can use "sleep" if provided
- See ../pinctrl/pinctrl-binding.txt
+ See ../pinctrl/pinctrl-bindings.txt
Optional properties:
- cs-gpios : List of GPIO chip selects
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v3 2/7] serdev: add dev_pm_domain_attach|detach()
From: Sean Wang @ 2018-06-15 2:30 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rob Herring, Mark Rutland, Marcel Holtmann, Johan Hedberg,
devicetree, linux-bluetooth, Linux ARM, linux-mediatek,
Linux Kernel Mailing List, Rob Herring, Greg Kroah-Hartman,
Jiri Slaby, linux-serial
In-Reply-To: <CAPDyKFqVpfhJf3R3jbXtjGgXf+0pg=9dgALBnxM_bSyBQrQQDQ@mail.gmail.com>
On Thu, 2018-06-14 at 10:58 +0200, Ulf Hansson wrote:
> On Thu, 14 Jun 2018 at 09:14, <sean.wang@mediatek.com> wrote:
> >
> > From: Sean Wang <sean.wang@mediatek.com>
> >
> > In order to open up the required power gate before any operation can be
> > effectively performed over the serial bus between CPU and serdev, it's
> > clearly essential to add common attach functions for PM domains to serdev
> > at the probe phase.
> >
> > Similarly, the relevant dettach function for the PM domains should be
> > properly and reversely added at the remove phase.
> >
> > Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Jiri Slaby <jslaby@suse.com>
> > Cc: linux-serial@vger.kernel.org
> > ---
> > drivers/tty/serdev/core.c | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> > index df93b72..c93d8ee 100644
> > --- a/drivers/tty/serdev/core.c
> > +++ b/drivers/tty/serdev/core.c
> > @@ -13,6 +13,7 @@
> > #include <linux/module.h>
> > #include <linux/of.h>
> > #include <linux/of_device.h>
> > +#include <linux/pm_domain.h>
> > #include <linux/serdev.h>
> > #include <linux/slab.h>
> >
> > @@ -330,8 +331,16 @@ EXPORT_SYMBOL_GPL(serdev_device_set_tiocm);
> > static int serdev_drv_probe(struct device *dev)
> > {
> > const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
> > + int ret;
> > +
> > + ret = dev_pm_domain_attach(dev, true);
> > + if (ret != -EPROBE_DEFER) {
>
> From 4.18 rc1 via commit 919b7308fcc4, dev_pm_domain_attach() will
> return better error codes.
>
> I suggest to change the above error path to:
> if (ret)
> return ret;
>
> Then continue with the probing below.
Thanks for sharing me the information. I'll happily respin using the
patch because it makes the most sense.
>
> > + ret = sdrv->probe(to_serdev_device(dev));
> > + if (ret)
> > + dev_pm_domain_detach(dev, true);
> > + }
> >
> > - return sdrv->probe(to_serdev_device(dev));
> > + return ret;
> > }
> >
> > static int serdev_drv_remove(struct device *dev)
> > @@ -339,6 +348,9 @@ static int serdev_drv_remove(struct device *dev)
> > const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
> > if (sdrv->remove)
> > sdrv->remove(to_serdev_device(dev));
> > +
> > + dev_pm_domain_detach(dev, true);
> > +
> > return 0;
> > }
> >
> > --
> > 2.7.4
> >
>
> Otherwise, this makes sense to me!
>
really thanks for your review!
> Kind regards
> Uffe
^ permalink raw reply
* Re: [RFC PATCH 5/6] arm64: dts: ti: Add Support for AM654 SoC
From: Tony Lindgren @ 2018-06-15 5:01 UTC (permalink / raw)
To: Nishanth Menon
Cc: Mark Rutland, devicetree, Sudeep Holla, Vignesh R,
Catalin Marinas, Santosh Shilimkar, Will Deacon,
linux-kernel@vger.kernel.org, Russell King, Tero Kristo,
Rob Herring, open list:SERIAL DRIVERS, Greg Kroah-Hartman,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20180614130435.j2bmzam6corrjylx@kahuna>
* Nishanth Menon <nm@ti.com> [180614 13:07]:
> On 12:38-20180614, Tony Lindgren wrote:
> > Some comments on the ranges below.
>
> Thanks for reviewing in detail (I understand we are in the middle of
> merge window, so thanks for the extra effort).
>
> >
> > * Nishanth Menon <nm@ti.com> [180607 16:41]:
> > > + soc0: soc0 {
> > > + compatible = "simple-bus";
> > > + #address-cells = <2>;
> > > + #size-cells = <2>;
> > > + ranges;
> >
> > I suggest you leave out the soc0, that's not real. Just make
>
> Why is that so, on a more complex board representation with multiple
> SoCs, this is a clear node indicating what the main SoC is in the final
> dtb representation.
It does not have a real reg or range.
> > the cbass@0 the top level interconnect. It can then provide
> > ranges to mcu interconnect which can provide ranges to the wkup
> > interconnect. So just model it after what's in the hardware :)
>
> That might blow up things quite a bit - it is like the comment in:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/dra7.dtsi#n141
That comment at the link above not true I've found. What we have
there as "ocp" should be just "l3" and then the "l4" instances are
children of "l3". The direct ports from some "l4" devices are just
ranges at the parent "l3". And this will get changed slowly over
next few merge cycles.
> The trees are pretty deep with many interconnections (example main does
> have direct connections to wkup as well, which is simplified off in
> top level diagram) - basically it is not a direct one dimensional
> relationship. But then, the same is the case for other SoCs..
In the above example the connection from main to wkup is just a
range provided by main so not a problem.
> we can represent NAVSS as a bus segment as well.
Well ideally each module on the interconnects would be set up
separately to prevent drivers trying to ioremap ranges from
multiple modules. This is important as flushing posted write to
one module will not flush it for the other module.
> > I found the following ranges based on a quick look at the TRM,
> > they could be split further if needed for power domains for
> > genpd for example.
>
> genpd is not really an issue, since it is handled in system firmware and
> OSes dont have a visibility into the permitted ranges that the OS is
> allowed to use.
There are other reasons beyond genpd too. Flushing posted writes
to modules is one. Getting rid of pointless deferred probe is
another one. Preventing device drivers trying to ioremap multiple
module is yet another one..
> I think it is just how accurate a representation is it worth.
The dts really is intended to describe the hardware :) So
let's not repeat the same mistake again with imaginary ranges.
> >
> > main covers
> > 0x0000000000 - 0x5402000000
> >
> > main provides at least the following ranges for mcu
> > 0x0028380000 - 0x002bc00000
> > 0x0040080000 - 0x0041c80000
> > 0x0045100000 - 0x0045180000
> > 0x0045600000 - 0x0045640000
> > 0x0045810000 - 0x0045860000
> > 0x0045950000 - 0x0045950400
> > 0x0045a50000 - 0x0045a50400
> > 0x0045b04000 - 0x0045b06400
> > 0x0045d10000 - 0x0045d24000
> > 0x0046000000 - 0x0060000000
> > 0x0400000000 - 0x0800000000
> > 0x4c3c020000 - 0x4c3c030000
> > 0x4c3e000000 - 0x4c3e040000
> > 0x5400000000 - 0x5402000000
> >
> > then mcu provides the following ranges for wkup
> > 0x0042000000 - 0x0044410020
> > 0x0045000000 - 0x0045030000
> > 0x0045080000 - 0x00450a0000
> > 0x0045808000 - 0x0045808800
> > 0x0045b00000 - 0x0045b02400
> >
> > This based on looking at "figure 1-1. device top-level
> > block diagram" and the memory map in TRM.
>
> Thanks for researching. I did debate something like:
>
> From A53 view, a more accurate view might be - from an interconnect
> view of the world (still simplified - i have ignored the sub bus
> segments in the representations below):
>
> msmc {
> navss_main {
> cbass_main{
> cbass_mcu {
> navss_mcu {
> };
> cbass_wkup{
> };
> };
> };
> };
> };
>
> From R5 view, the view will be very different ofcourse:
> view of the world (still simplified):
>
> cbass_mcu {
> navss_mcu {
> };
> cbass_wkup{
> };
> cbass_main{
> navss_main {
> msmc {
> };
> };
> };
> };
Well if we follow the hardware representation of the interconnects,
it should not matter from which processor view you're looking at things.
There are just different ranges provided.
> Do we really need this level of representation, I am not sure I had seen
> this detailed a representation in other aarch64 SoCs (I am sure they are
> as complex as TI SoCs as well).
Based on my experience yes. See also the reasons I listed above.
> I am trying to understand the direction and logic why we'd want to have
> such a detailed representation.
>
> A more flatter representation of just the main segments allow for dts
> reuse between r5 and a53 as well (but that is minor).
Just model it based on the hardware, then there's no need to
debate it or rework it later on :)
Regards,
Tony
^ permalink raw reply
* Re: possible deadlock in console_unlock
From: Sergey Senozhatsky @ 2018-06-15 8:38 UTC (permalink / raw)
To: Petr Mladek
Cc: Sergey Senozhatsky, Sergey Senozhatsky, syzbot, linux-kernel,
rostedt, syzkaller-bugs, Greg Kroah-Hartman, Jiri Slaby,
linux-serial, Andrew Morton
In-Reply-To: <20180608081855.ctqakvwmg27aydby@pathway.suse.cz>
On (06/08/18 10:18), Petr Mladek wrote:
[..]
> > Could be.
> > The good thing about printk_safe is that printk_safe sections can nest.
> > I suspect there might be locks/printk_safe sections nesting at some
> > point. In any case, switching to a new flavor of printk_safe will be
> > pretty easy - just replace printk_safe_enter() with printk_foo_enter()
> > and the same for printk_save_exit().
>
> We could allow nesting. It is just a matter of how many bits we
> reserve for it in printk_context variable.
[..]
> In each case, I would like to keep the printk_safe context usage
> at minimum. It has its own problems caused by limited per-cpu buffers
> and the need to flush them.
May be. Every new printk_safe flavour comes with increasing memory
usage. We already have a bunch of pages pinned [and mostly unused]
to every CPU for printk_nmi and printk_safe. I'm a little unsure if
we have enough reasons to pin yet another bunch of pages to every
CPU. After all printk_safe is not used very commonly, so all in all
I think we should be fine with printk_safe buffers for the time being.
We always can introduce new printk_safe mode later.
> It is basically needed only to prevent deadlocks related to logbuf_lock.
I wouldn't say that we need printk_safe for logbuf_lock only.
printk_safe helps us to avoid deadlocks on:
- logbuf_lock spin_lock
- console_sem ->lock spin_lock
- console_owner spin_lock
- scheduler ->pi_lock spin_lock
- and probably something else.
-ss
^ permalink raw reply
* [RFC][PATCH 0/6] Use printk_safe context for TTY and UART port locks
From: Sergey Senozhatsky @ 2018-06-15 9:39 UTC (permalink / raw)
To: Petr Mladek, Steven Rostedt, Greg Kroah-Hartman, Jiri Slaby
Cc: Linus Torvalds, Peter Zijlstra, Andrew Morton, Dmitry Vyukov,
linux-kernel, linux-serial, Sergey Senozhatsky,
Sergey Senozhatsky
Hello,
RFC
printk_safe buffers help us to avoid printk() deadlocks that are
caused by recursive printk() calls, e.g.
printk()
vprintk_emit()
spin_lock(&logbuf_lock)
vsnprintf()
format_decode()
warn_slowpath_fmt()
vprintk_emit()
spin_lock(&logbuf_lock) << deadlock
It doesn't come as a surprise that recursive printk() calls are not the
only way for us to deadlock in printk() and we still have a whole bunch
of other printk() deadlock scenarios. For instance, those that involve
TTY port->lock spin_lock and UART port->lock spin_lock.
syzbot recently hit one of such scenarios [1]:
CPU0 CPU1
tty_ioctl()
spin_lock(&tty_port->lock) IRQ
kmalloc() foo_console_handle_IRQ()
printk() spin_lock(&uart_port->lock)
call_console_drivers() tty_wakeup()
foo_console_driver()
spin_lock(&uart_port->lock) spin_lock(&tty_port->lock)
So the idea of this patch set is to take tty_port->lock and
uart_port->lock from printk_safe context and to eliminate some
of non-recursive printk() deadlocks - the ones that don't start
in printk(), but involve console related locks and thus eventually
deadlock us in printk(). For this purpose the patch set introduces
several helper macros:
- uart_port_lock_irq() / uart_port_unlock_irq()
uart_port_lock_irqsave() / uart_port_unlock_irqrestore()
To lock/unlock uart_port->lock spin_lock from printk_safe
context.
And
- tty_port_lock_irq() / tty_port_unlock_irq()
tty_port_lock_irqsave() / tty_port_unlock_irqrestore()
To lock/unlock tty_port->lock spin_lock from printk_safe
context.
I converted tty/pty/serial_core to use those helper macros.
Now, the boring part is that all serial console drivers must be converted
to use uart_port_lock macros. In this patch set I only modified the 8250
serial console [since this is RFC patch set], but if the patch set will
not see a lot of opposition I can do so for the rest of serial consoles
[or ask the maintainers to help me out]. The conversion is pretty
simple.
It would be great if we could "forbid" direct tty_port->lock and
uart_port->lock manipulation [I don't know, rename them to ->__lock]
and enforce uart_port_lock/tty_port_lock macros usage.
There are some other cases [2] that we can handle with this
patch set. For instance:
IRQ
spin_lock(&uart_port->lock)
tty_wakeup()
tty_port_default_wakeup()
tty_port_tty_get()
refcount_inc()
WARN_ONCE()
printk()
call_console_drivers()
foo_console_driver()
spin_lock(&uart_port->lock) << deadlock
Of course, TTY and UART port spin_locks are not the only locks that
we can deadlock on. So this patch set does not address all deadlock
scenarios, it just makes a small step forward.
Any opinions?
[1]: lkml.kernel.org/r/00000000000087008b056df8fbb3@google.com
[2]: lkml.kernel.org/r/20180607051019.GA10406@jagdpanzerIV
Sergey Senozhatsky (6):
printk: move printk_safe macros to printk header
tty: add tty_port locking helpers
tty/pty: switch to tty_port helpers
serial: add uart port locking helpers
serial: switch to uart_port locking helpers
tty: 8250: switch to uart_port locking helpers
drivers/tty/pty.c | 4 +-
drivers/tty/serial/8250/8250_core.c | 8 +-
drivers/tty/serial/8250/8250_dma.c | 4 +-
drivers/tty/serial/8250/8250_port.c | 74 +++++++++----------
drivers/tty/serial/serial_core.c | 109 ++++++++++++++--------------
drivers/tty/tty_port.c | 38 +++++-----
include/linux/printk.h | 40 ++++++++++
include/linux/serial_core.h | 35 +++++++++
include/linux/tty.h | 24 ++++++
kernel/printk/internal.h | 37 ----------
10 files changed, 218 insertions(+), 155 deletions(-)
--
2.17.1
^ permalink raw reply
* [RFC][PATCH 1/6] printk: move printk_safe macros to printk header
From: Sergey Senozhatsky @ 2018-06-15 9:39 UTC (permalink / raw)
To: Petr Mladek, Steven Rostedt, Greg Kroah-Hartman, Jiri Slaby
Cc: Linus Torvalds, Peter Zijlstra, Andrew Morton, Dmitry Vyukov,
linux-kernel, linux-serial, Sergey Senozhatsky,
Sergey Senozhatsky
In-Reply-To: <20180615093919.559-1-sergey.senozhatsky@gmail.com>
Make printk_safe_enter_irqsave()/etc macros available to the
rest of the kernel, so we can use them in TTY and UART code.
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
include/linux/printk.h | 40 ++++++++++++++++++++++++++++++++++++++++
kernel/printk/internal.h | 37 -------------------------------------
2 files changed, 40 insertions(+), 37 deletions(-)
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 6d7e800affd8..8b4e1e667919 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -153,6 +153,46 @@ static inline void printk_nmi_enter(void) { }
static inline void printk_nmi_exit(void) { }
#endif /* PRINTK_NMI */
+#ifdef CONFIG_PRINTK
+extern void __printk_safe_enter(void);
+extern void __printk_safe_exit(void);
+
+#define printk_safe_enter_irqsave(flags) \
+ do { \
+ local_irq_save(flags); \
+ __printk_safe_enter(); \
+ } while (0)
+
+#define printk_safe_exit_irqrestore(flags) \
+ do { \
+ __printk_safe_exit(); \
+ local_irq_restore(flags); \
+ } while (0)
+
+#define printk_safe_enter_irq() \
+ do { \
+ local_irq_disable(); \
+ __printk_safe_enter(); \
+ } while (0)
+
+#define printk_safe_exit_irq() \
+ do { \
+ __printk_safe_exit(); \
+ local_irq_enable(); \
+ } while (0)
+#else
+/*
+ * On !PRINTK builds we still export console output related locks
+ * and some functions (console_unlock()/tty/etc.), so printk-safe
+ * must preserve the existing local IRQ guarantees.
+ */
+#define printk_safe_enter_irqsave(flags) local_irq_save(flags)
+#define printk_safe_exit_irqrestore(flags) local_irq_restore(flags)
+
+#define printk_safe_enter_irq() local_irq_disable()
+#define printk_safe_exit_irq() local_irq_enable()
+#endif
+
#ifdef CONFIG_PRINTK
asmlinkage __printf(5, 0)
int vprintk_emit(int facility, int level,
diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
index 2a7d04049af4..cf4f85e53cc2 100644
--- a/kernel/printk/internal.h
+++ b/kernel/printk/internal.h
@@ -27,46 +27,9 @@ extern raw_spinlock_t logbuf_lock;
__printf(1, 0) int vprintk_default(const char *fmt, va_list args);
__printf(1, 0) int vprintk_deferred(const char *fmt, va_list args);
__printf(1, 0) int vprintk_func(const char *fmt, va_list args);
-void __printk_safe_enter(void);
-void __printk_safe_exit(void);
-
-#define printk_safe_enter_irqsave(flags) \
- do { \
- local_irq_save(flags); \
- __printk_safe_enter(); \
- } while (0)
-
-#define printk_safe_exit_irqrestore(flags) \
- do { \
- __printk_safe_exit(); \
- local_irq_restore(flags); \
- } while (0)
-
-#define printk_safe_enter_irq() \
- do { \
- local_irq_disable(); \
- __printk_safe_enter(); \
- } while (0)
-
-#define printk_safe_exit_irq() \
- do { \
- __printk_safe_exit(); \
- local_irq_enable(); \
- } while (0)
#else
__printf(1, 0) int vprintk_func(const char *fmt, va_list args) { return 0; }
-/*
- * In !PRINTK builds we still export logbuf_lock spin_lock, console_sem
- * semaphore and some of console functions (console_unlock()/etc.), so
- * printk-safe must preserve the existing local IRQ guarantees.
- */
-#define printk_safe_enter_irqsave(flags) local_irq_save(flags)
-#define printk_safe_exit_irqrestore(flags) local_irq_restore(flags)
-
-#define printk_safe_enter_irq() local_irq_disable()
-#define printk_safe_exit_irq() local_irq_enable()
-
#endif /* CONFIG_PRINTK */
--
2.17.1
^ permalink raw reply related
* [RFC][PATCH 2/6] tty: add tty_port locking helpers
From: Sergey Senozhatsky @ 2018-06-15 9:39 UTC (permalink / raw)
To: Petr Mladek, Steven Rostedt, Greg Kroah-Hartman, Jiri Slaby
Cc: Linus Torvalds, Peter Zijlstra, Andrew Morton, Dmitry Vyukov,
linux-kernel, linux-serial, Sergey Senozhatsky,
Sergey Senozhatsky
In-Reply-To: <20180615093919.559-1-sergey.senozhatsky@gmail.com>
TTY port spin_lock is one of the locks that can cause a
printk-recursion deadlock, thus we need to lock it from
printk_safe context. E.g.
ioctl()
tty_ioctl()
spin_lock(&tty_port->lock)
printk()
call_console_drivers()
foo_console_write()
spin_lock(&uart_port->lock)
tty_wakeup()
spin_lock(&tty_port->lock) << deadlock
This patch adds tty_port->lock helper macros which take care
of printk_safe context and redirect potentially unsafe printk()
calls to per-CPU buffers, which we flush later from safe a
context. The helper macros will turn the above mentioned deadlock
scenario into:
ioctl()
tty_ioctl()
printk_safe_enter()
spin_lock(&tty_port->lock)
printk()
printk_safe_log_store()
irq_work_queue()
spin_unlock(&tty_port->lock)
printk_safe_exit()
IRQ
printk_safe_flush_buffer()
vprintk_deferred()
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
include/linux/tty.h | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/include/linux/tty.h b/include/linux/tty.h
index c56e3978b00f..828f91ab680b 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -252,6 +252,30 @@ struct tty_port {
void *client_data;
};
+#define tty_port_lock_irq(lock) \
+ do { \
+ printk_safe_enter_irq(); \
+ spin_lock(lock); \
+ } while (0)
+
+#define tty_port_unlock_irq(lock) \
+ do { \
+ spin_unlock(lock); \
+ printk_safe_exit_irq(); \
+ } while (0)
+
+#define tty_port_lock_irqsave(lock, flags) \
+ do { \
+ printk_safe_enter_irqsave(flags); \
+ spin_lock(lock); \
+ } while (0)
+
+#define tty_port_unlock_irqrestore(lock, flags) \
+ do { \
+ spin_unlock(lock); \
+ printk_safe_exit_irqrestore(flags); \
+ } while (0)
+
/* tty_port::iflags bits -- use atomic bit ops */
#define TTY_PORT_INITIALIZED 0 /* device is initialized */
#define TTY_PORT_SUSPENDED 1 /* device is suspended */
--
2.17.1
^ permalink raw reply related
* [RFC][PATCH 3/6] tty/pty: switch to tty_port helpers
From: Sergey Senozhatsky @ 2018-06-15 9:39 UTC (permalink / raw)
To: Petr Mladek, Steven Rostedt, Greg Kroah-Hartman, Jiri Slaby
Cc: Linus Torvalds, Peter Zijlstra, Andrew Morton, Dmitry Vyukov,
linux-kernel, linux-serial, Sergey Senozhatsky,
Sergey Senozhatsky
In-Reply-To: <20180615093919.559-1-sergey.senozhatsky@gmail.com>
Do not directly lock/unlock tty_port->lock, but use tty_port_lock
helper macros.
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
drivers/tty/pty.c | 4 ++--
drivers/tty/serial/serial_core.c | 8 +++----
drivers/tty/tty_port.c | 38 ++++++++++++++++----------------
3 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index b0e2c4847a5d..dddeecc2289e 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -116,13 +116,13 @@ static int pty_write(struct tty_struct *tty, const unsigned char *buf, int c)
return 0;
if (c > 0) {
- spin_lock_irqsave(&to->port->lock, flags);
+ tty_port_lock_irqsave(&to->port->lock, flags);
/* Stuff the data into the input queue of the other end */
c = tty_insert_flip_string(to->port, buf, c);
/* And shovel */
if (c)
tty_flip_buffer_push(to->port);
- spin_unlock_irqrestore(&to->port->lock, flags);
+ tty_port_unlock_irqrestore(&to->port->lock, flags);
}
return c;
}
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 9c14a453f73c..48b3377d7f77 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1480,9 +1480,9 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
state = drv->state + tty->index;
port = &state->port;
- spin_lock_irq(&port->lock);
+ tty_port_lock_irq(&port->lock);
--port->count;
- spin_unlock_irq(&port->lock);
+ tty_port_unlock_irq(&port->lock);
return;
}
@@ -1603,9 +1603,9 @@ static void uart_hangup(struct tty_struct *tty)
if (tty_port_active(port)) {
uart_flush_buffer(tty);
uart_shutdown(tty, state);
- spin_lock_irqsave(&port->lock, flags);
+ tty_port_lock_irqsave(&port->lock, flags);
port->count = 0;
- spin_unlock_irqrestore(&port->lock, flags);
+ tty_port_unlock_irqrestore(&port->lock, flags);
tty_port_set_active(port, 0);
tty_port_tty_set(port, NULL);
if (uport && !uart_console(uport))
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 25d736880013..4237d282f89f 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -285,9 +285,9 @@ struct tty_struct *tty_port_tty_get(struct tty_port *port)
unsigned long flags;
struct tty_struct *tty;
- spin_lock_irqsave(&port->lock, flags);
+ tty_port_lock_irqsave(&port->lock, flags);
tty = tty_kref_get(port->tty);
- spin_unlock_irqrestore(&port->lock, flags);
+ tty_port_unlock_irqrestore(&port->lock, flags);
return tty;
}
EXPORT_SYMBOL(tty_port_tty_get);
@@ -305,10 +305,10 @@ void tty_port_tty_set(struct tty_port *port, struct tty_struct *tty)
{
unsigned long flags;
- spin_lock_irqsave(&port->lock, flags);
+ tty_port_lock_irqsave(&port->lock, flags);
tty_kref_put(port->tty);
port->tty = tty_kref_get(tty);
- spin_unlock_irqrestore(&port->lock, flags);
+ tty_port_unlock_irqrestore(&port->lock, flags);
}
EXPORT_SYMBOL(tty_port_tty_set);
@@ -349,13 +349,13 @@ void tty_port_hangup(struct tty_port *port)
struct tty_struct *tty;
unsigned long flags;
- spin_lock_irqsave(&port->lock, flags);
+ tty_port_lock_irqsave(&port->lock, flags);
port->count = 0;
tty = port->tty;
if (tty)
set_bit(TTY_IO_ERROR, &tty->flags);
port->tty = NULL;
- spin_unlock_irqrestore(&port->lock, flags);
+ tty_port_unlock_irqrestore(&port->lock, flags);
tty_port_set_active(port, 0);
tty_port_shutdown(port, tty);
tty_kref_put(tty);
@@ -496,10 +496,10 @@ int tty_port_block_til_ready(struct tty_port *port,
retval = 0;
/* The port lock protects the port counts */
- spin_lock_irqsave(&port->lock, flags);
+ tty_port_lock_irqsave(&port->lock, flags);
port->count--;
port->blocked_open++;
- spin_unlock_irqrestore(&port->lock, flags);
+ tty_port_unlock_irqrestore(&port->lock, flags);
while (1) {
/* Indicate we are open */
@@ -536,11 +536,11 @@ int tty_port_block_til_ready(struct tty_port *port,
/* Update counts. A parallel hangup will have set count to zero and
we must not mess that up further */
- spin_lock_irqsave(&port->lock, flags);
+ tty_port_lock_irqsave(&port->lock, flags);
if (!tty_hung_up_p(filp))
port->count++;
port->blocked_open--;
- spin_unlock_irqrestore(&port->lock, flags);
+ tty_port_unlock_irqrestore(&port->lock, flags);
if (retval == 0)
tty_port_set_active(port, 1);
return retval;
@@ -570,7 +570,7 @@ int tty_port_close_start(struct tty_port *port,
if (tty_hung_up_p(filp))
return 0;
- spin_lock_irqsave(&port->lock, flags);
+ tty_port_lock_irqsave(&port->lock, flags);
if (tty->count == 1 && port->count != 1) {
tty_warn(tty, "%s: tty->count = 1 port count = %d\n", __func__,
port->count);
@@ -583,10 +583,10 @@ int tty_port_close_start(struct tty_port *port,
}
if (port->count) {
- spin_unlock_irqrestore(&port->lock, flags);
+ tty_port_unlock_irqrestore(&port->lock, flags);
return 0;
}
- spin_unlock_irqrestore(&port->lock, flags);
+ tty_port_unlock_irqrestore(&port->lock, flags);
tty->closing = 1;
@@ -615,16 +615,16 @@ void tty_port_close_end(struct tty_port *port, struct tty_struct *tty)
tty_ldisc_flush(tty);
tty->closing = 0;
- spin_lock_irqsave(&port->lock, flags);
+ tty_port_lock_irqsave(&port->lock, flags);
if (port->blocked_open) {
- spin_unlock_irqrestore(&port->lock, flags);
+ tty_port_unlock_irqrestore(&port->lock, flags);
if (port->close_delay)
msleep_interruptible(jiffies_to_msecs(port->close_delay));
- spin_lock_irqsave(&port->lock, flags);
+ tty_port_lock_irqsave(&port->lock, flags);
wake_up_interruptible(&port->open_wait);
}
- spin_unlock_irqrestore(&port->lock, flags);
+ tty_port_unlock_irqrestore(&port->lock, flags);
tty_port_set_active(port, 0);
}
EXPORT_SYMBOL(tty_port_close_end);
@@ -675,9 +675,9 @@ EXPORT_SYMBOL_GPL(tty_port_install);
int tty_port_open(struct tty_port *port, struct tty_struct *tty,
struct file *filp)
{
- spin_lock_irq(&port->lock);
+ tty_port_lock_irq(&port->lock);
++port->count;
- spin_unlock_irq(&port->lock);
+ tty_port_unlock_irq(&port->lock);
tty_port_tty_set(port, tty);
/*
--
2.17.1
^ permalink raw reply related
* [RFC][PATCH 4/6] serial: add uart port locking helpers
From: Sergey Senozhatsky @ 2018-06-15 9:39 UTC (permalink / raw)
To: Petr Mladek, Steven Rostedt, Greg Kroah-Hartman, Jiri Slaby
Cc: Linus Torvalds, Peter Zijlstra, Andrew Morton, Dmitry Vyukov,
linux-kernel, linux-serial, Sergey Senozhatsky,
Sergey Senozhatsky
In-Reply-To: <20180615093919.559-1-sergey.senozhatsky@gmail.com>
UART port spin_lock is one of the locks that can cause a
printk-recursion deadlock, thus we need to lock it from
printk_safe context. An example of a possible deadlock:
IRQ
foo_console_handle_IRQ()
spin_lock(&uart_port->lock)
tty_wakeup()
printk()
call_console_drivers()
foo_console_write()
spin_lock(&uart_port->lock) << deadlock
This patch adds uart_port->lock helper macros which take care
of printk_safe context and redirect potentially unsafe printk()
calls to per-CPU buffers, which we flush later from safe a
context. The helper macros will turn the above mentioned deadlock
scenario into:
IRQ
foo_console_handle_IRQ()
printk_safe_enter()
spin_lock(&uart_port->lock)
tty_wakeup()
printk()
printk_safe_log_store()
irq_work_queue()
spin_unlock(&uart_port->lock)
printk_safe_exit()
iret
IRQ
printk_safe_flush_buffer()
vprintk_deferred()
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
include/linux/serial_core.h | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 06ea4eeb09ab..deb464520946 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -256,6 +256,41 @@ struct uart_port {
void *private_data; /* generic platform data pointer */
};
+#define uart_port_lock_irq(lock) \
+ do { \
+ printk_safe_enter_irq(); \
+ spin_lock(lock); \
+ } while (0)
+
+#define uart_port_unlock_irq(lock) \
+ do { \
+ spin_unlock(lock); \
+ printk_safe_exit_irq(); \
+ } while (0)
+
+#define uart_port_lock_irqsave(lock, flags) \
+ do { \
+ printk_safe_enter_irqsave(flags); \
+ spin_lock(lock); \
+ } while (0)
+
+#define uart_port_trylock_irqsave(lock, flags) \
+ ({ \
+ int locked; \
+ \
+ printk_safe_enter_irqsave(flags); \
+ locked = spin_trylock(lock); \
+ if (!locked) \
+ printk_safe_exit_irqrestore(flags); \
+ locked; \
+ })
+
+#define uart_port_unlock_irqrestore(lock, flags) \
+ do { \
+ spin_unlock(lock); \
+ printk_safe_exit_irqrestore(flags); \
+ } while (0)
+
static inline int serial_port_in(struct uart_port *up, int offset)
{
return up->serial_in(up, offset);
--
2.17.1
^ permalink raw reply related
* [RFC][PATCH 5/6] serial: switch to uart_port locking helpers
From: Sergey Senozhatsky @ 2018-06-15 9:39 UTC (permalink / raw)
To: Petr Mladek, Steven Rostedt, Greg Kroah-Hartman, Jiri Slaby
Cc: Linus Torvalds, Peter Zijlstra, Andrew Morton, Dmitry Vyukov,
linux-kernel, linux-serial, Sergey Senozhatsky,
Sergey Senozhatsky
In-Reply-To: <20180615093919.559-1-sergey.senozhatsky@gmail.com>
Do not directly lock/unlock uart_port->lock, but use uart_port_lock
helper macros.
The patch also renames serial_core internal macros uart_port_lock()
and uart_port_unlock() to uart_port_ref_lock() and to
uart_port_unlock_deref() correspondingly.
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
drivers/tty/serial/serial_core.c | 101 ++++++++++++++++---------------
1 file changed, 51 insertions(+), 50 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 48b3377d7f77..48624ca485e5 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -65,19 +65,20 @@ static inline void uart_port_deref(struct uart_port *uport)
wake_up(&uport->state->remove_wait);
}
-#define uart_port_lock(state, flags) \
+#define uart_port_ref_lock(state, flags) \
({ \
struct uart_port *__uport = uart_port_ref(state); \
if (__uport) \
- spin_lock_irqsave(&__uport->lock, flags); \
+ uart_port_lock_irqsave(&__uport->lock, flags); \
__uport; \
})
-#define uart_port_unlock(uport, flags) \
+#define uart_port_unlock_deref(uport, flags) \
({ \
struct uart_port *__uport = uport; \
if (__uport) { \
- spin_unlock_irqrestore(&__uport->lock, flags); \
+ uart_port_unlock_irqrestore(&__uport->lock, \
+ flags); \
uart_port_deref(__uport); \
} \
})
@@ -109,10 +110,10 @@ static void uart_stop(struct tty_struct *tty)
struct uart_port *port;
unsigned long flags;
- port = uart_port_lock(state, flags);
+ port = uart_port_ref_lock(state, flags);
if (port)
port->ops->stop_tx(port);
- uart_port_unlock(port, flags);
+ uart_port_unlock_deref(port, flags);
}
static void __uart_start(struct tty_struct *tty)
@@ -130,9 +131,9 @@ static void uart_start(struct tty_struct *tty)
struct uart_port *port;
unsigned long flags;
- port = uart_port_lock(state, flags);
+ port = uart_port_ref_lock(state, flags);
__uart_start(tty);
- uart_port_unlock(port, flags);
+ uart_port_unlock_deref(port, flags);
}
static void
@@ -141,12 +142,12 @@ uart_update_mctrl(struct uart_port *port, unsigned int set, unsigned int clear)
unsigned long flags;
unsigned int old;
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(&port->lock, flags);
old = port->mctrl;
port->mctrl = (old & ~clear) | set;
if (old != port->mctrl)
port->ops->set_mctrl(port, port->mctrl);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
}
#define uart_set_mctrl(port, set) uart_update_mctrl(port, set, 0)
@@ -499,7 +500,7 @@ static void uart_change_speed(struct tty_struct *tty, struct uart_state *state,
/*
* Set modem status enables based on termios cflag
*/
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(&uport->lock);
if (termios->c_cflag & CRTSCTS)
uport->status |= UPSTAT_CTS_ENABLE;
else
@@ -521,7 +522,7 @@ static void uart_change_speed(struct tty_struct *tty, struct uart_state *state,
if (hw_stopped)
__uart_start(tty);
}
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(&uport->lock);
}
static int uart_put_char(struct tty_struct *tty, unsigned char c)
@@ -536,13 +537,13 @@ static int uart_put_char(struct tty_struct *tty, unsigned char c)
if (!circ->buf)
return 0;
- port = uart_port_lock(state, flags);
+ port = uart_port_ref_lock(state, flags);
if (port && uart_circ_chars_free(circ) != 0) {
circ->buf[circ->head] = c;
circ->head = (circ->head + 1) & (UART_XMIT_SIZE - 1);
ret = 1;
}
- uart_port_unlock(port, flags);
+ uart_port_unlock_deref(port, flags);
return ret;
}
@@ -573,7 +574,7 @@ static int uart_write(struct tty_struct *tty,
if (!circ->buf)
return 0;
- port = uart_port_lock(state, flags);
+ port = uart_port_ref_lock(state, flags);
while (port) {
c = CIRC_SPACE_TO_END(circ->head, circ->tail, UART_XMIT_SIZE);
if (count < c)
@@ -588,7 +589,7 @@ static int uart_write(struct tty_struct *tty,
}
__uart_start(tty);
- uart_port_unlock(port, flags);
+ uart_port_unlock_deref(port, flags);
return ret;
}
@@ -599,9 +600,9 @@ static int uart_write_room(struct tty_struct *tty)
unsigned long flags;
int ret;
- port = uart_port_lock(state, flags);
+ port = uart_port_ref_lock(state, flags);
ret = uart_circ_chars_free(&state->xmit);
- uart_port_unlock(port, flags);
+ uart_port_unlock_deref(port, flags);
return ret;
}
@@ -612,9 +613,9 @@ static int uart_chars_in_buffer(struct tty_struct *tty)
unsigned long flags;
int ret;
- port = uart_port_lock(state, flags);
+ port = uart_port_ref_lock(state, flags);
ret = uart_circ_chars_pending(&state->xmit);
- uart_port_unlock(port, flags);
+ uart_port_unlock_deref(port, flags);
return ret;
}
@@ -635,13 +636,13 @@ static void uart_flush_buffer(struct tty_struct *tty)
pr_debug("uart_flush_buffer(%d) called\n", tty->index);
- port = uart_port_lock(state, flags);
+ port = uart_port_ref_lock(state, flags);
if (!port)
return;
uart_circ_clear(&state->xmit);
if (port->ops->flush_buffer)
port->ops->flush_buffer(port);
- uart_port_unlock(port, flags);
+ uart_port_unlock_deref(port, flags);
tty_port_tty_wakeup(&state->port);
}
@@ -662,11 +663,11 @@ static void uart_send_xchar(struct tty_struct *tty, char ch)
if (port->ops->send_xchar)
port->ops->send_xchar(port, ch);
else {
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(&port->lock, flags);
port->x_char = ch;
if (ch)
port->ops->start_tx(port);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
}
uart_port_deref(port);
}
@@ -1048,9 +1049,9 @@ static int uart_tiocmget(struct tty_struct *tty)
if (!tty_io_error(tty)) {
result = uport->mctrl;
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(&uport->lock);
result |= uport->ops->get_mctrl(uport);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(&uport->lock);
}
out:
mutex_unlock(&port->mutex);
@@ -1186,16 +1187,16 @@ static int uart_wait_modem_status(struct uart_state *state, unsigned long arg)
uport = uart_port_ref(state);
if (!uport)
return -EIO;
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(&uport->lock);
memcpy(&cprev, &uport->icount, sizeof(struct uart_icount));
uart_enable_ms(uport);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(&uport->lock);
add_wait_queue(&port->delta_msr_wait, &wait);
for (;;) {
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(&uport->lock);
memcpy(&cnow, &uport->icount, sizeof(struct uart_icount));
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(&uport->lock);
set_current_state(TASK_INTERRUPTIBLE);
@@ -1240,9 +1241,9 @@ static int uart_get_icount(struct tty_struct *tty,
uport = uart_port_ref(state);
if (!uport)
return -EIO;
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(&uport->lock);
memcpy(&cnow, &uport->icount, sizeof(struct uart_icount));
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(&uport->lock);
uart_port_deref(uport);
icount->cts = cnow.cts;
@@ -1266,9 +1267,9 @@ static int uart_get_rs485_config(struct uart_port *port,
unsigned long flags;
struct serial_rs485 aux;
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(&port->lock, flags);
aux = port->rs485;
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
if (copy_to_user(rs485, &aux, sizeof(aux)))
return -EFAULT;
@@ -1289,9 +1290,9 @@ static int uart_set_rs485_config(struct uart_port *port,
if (copy_from_user(&rs485, rs485_user, sizeof(*rs485_user)))
return -EFAULT;
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(&port->lock, flags);
ret = port->rs485_config(port, &rs485);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
if (ret)
return ret;
@@ -1503,9 +1504,9 @@ static void uart_tty_port_shutdown(struct tty_port *port)
if (WARN(!uport, "detached port still initialized!\n"))
return;
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(&uport->lock);
uport->ops->stop_rx(uport);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(&uport->lock);
uart_port_shutdown(port);
@@ -1659,10 +1660,10 @@ static int uart_carrier_raised(struct tty_port *port)
*/
if (WARN_ON(!uport))
return 1;
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(&uport->lock);
uart_enable_ms(uport);
mctrl = uport->ops->get_mctrl(uport);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(&uport->lock);
uart_port_deref(uport);
if (mctrl & TIOCM_CAR)
return 1;
@@ -1770,9 +1771,9 @@ static void uart_line_info(struct seq_file *m, struct uart_driver *drv, int i)
pm_state = state->pm_state;
if (pm_state != UART_PM_STATE_ON)
uart_change_pm(state, UART_PM_STATE_ON);
- spin_lock_irq(&uport->lock);
+ uart_port_unlock_irq(&uport->lock);
status = uport->ops->get_mctrl(uport);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(&uport->lock);
if (pm_state != UART_PM_STATE_ON)
uart_change_pm(state, pm_state);
@@ -2099,11 +2100,11 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
tty_port_set_suspended(port, 1);
tty_port_set_initialized(port, 0);
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(&uport->lock);
ops->stop_tx(uport);
ops->set_mctrl(uport, 0);
ops->stop_rx(uport);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(&uport->lock);
/*
* Wait for the transmitter to empty.
@@ -2179,9 +2180,9 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
int ret;
uart_change_pm(state, UART_PM_STATE_ON);
- spin_lock_irq(&uport->lock);
+ uart_port_lock_irq(&uport->lock);
ops->set_mctrl(uport, 0);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(&uport->lock);
if (console_suspend_enabled || !uart_console(uport)) {
/* Protected by port mutex for now */
struct tty_struct *tty = port->tty;
@@ -2189,10 +2190,10 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
if (ret == 0) {
if (tty)
uart_change_speed(tty, state, NULL);
- spin_lock_irq(&uport->lock);
+ uart_port_unlock_irq(&uport->lock);
ops->set_mctrl(uport, uport->mctrl);
ops->start_tx(uport);
- spin_unlock_irq(&uport->lock);
+ uart_port_unlock_irq(&uport->lock);
tty_port_set_initialized(port, 1);
} else {
/*
@@ -2286,9 +2287,9 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
* keep the DTR setting that is set in uart_set_options()
* We probably don't need a spinlock around this, but
*/
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(&port->lock, flags);
port->ops->set_mctrl(port, port->mctrl & TIOCM_DTR);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
/*
* If this driver supports console, and it hasn't been
--
2.17.1
^ permalink raw reply related
* [RFC][PATCH 6/6] tty: 8250: switch to uart_port locking helpers
From: Sergey Senozhatsky @ 2018-06-15 9:39 UTC (permalink / raw)
To: Petr Mladek, Steven Rostedt, Greg Kroah-Hartman, Jiri Slaby
Cc: Linus Torvalds, Peter Zijlstra, Andrew Morton, Dmitry Vyukov,
linux-kernel, linux-serial, Sergey Senozhatsky,
Sergey Senozhatsky
In-Reply-To: <20180615093919.559-1-sergey.senozhatsky@gmail.com>
Do not directly lock/unlock uart_port->lock, but use uart_port_lock
helper macros.
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
drivers/tty/serial/8250/8250_core.c | 8 ++--
drivers/tty/serial/8250/8250_dma.c | 4 +-
drivers/tty/serial/8250/8250_port.c | 74 ++++++++++++++---------------
3 files changed, 43 insertions(+), 43 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 9342fc2ee7df..ce7aa601c809 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -272,7 +272,7 @@ static void serial8250_backup_timeout(struct timer_list *t)
unsigned int iir, ier = 0, lsr;
unsigned long flags;
- spin_lock_irqsave(&up->port.lock, flags);
+ uart_port_lock_irqsave(&up->port.lock, flags);
/*
* Must disable interrupts or else we risk racing with the interrupt
@@ -306,7 +306,7 @@ static void serial8250_backup_timeout(struct timer_list *t)
if (up->port.irq)
serial_out(up, UART_IER, ier);
- spin_unlock_irqrestore(&up->port.lock, flags);
+ uart_port_unlock_irqrestore(&up->port.lock, flags);
/* Standard timer interval plus 0.2s to keep the port running */
mod_timer(&up->timer,
@@ -1078,9 +1078,9 @@ void serial8250_unregister_port(int line)
if (uart->em485) {
unsigned long flags;
- spin_lock_irqsave(&uart->port.lock, flags);
+ uart_port_lock_irqsave(&uart->port.lock, flags);
serial8250_em485_destroy(uart);
- spin_unlock_irqrestore(&uart->port.lock, flags);
+ uart_port_unlock_irqrestore(&uart->port.lock, flags);
}
uart_remove_one_port(&serial8250_reg, &uart->port);
diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index bfa1a857f3ff..ec601779c32f 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -22,7 +22,7 @@ static void __dma_tx_complete(void *param)
dma_sync_single_for_cpu(dma->txchan->device->dev, dma->tx_addr,
UART_XMIT_SIZE, DMA_TO_DEVICE);
- spin_lock_irqsave(&p->port.lock, flags);
+ uart_port_lock_irqsave(&p->port.lock, flags);
dma->tx_running = 0;
@@ -39,7 +39,7 @@ static void __dma_tx_complete(void *param)
serial_port_out(&p->port, UART_IER, p->ier);
}
- spin_unlock_irqrestore(&p->port.lock, flags);
+ uart_port_unlock_irqrestore(&p->port.lock, flags);
}
static void __dma_rx_complete(void *param)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index cf541aab2bd0..a1ee17870ebc 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -772,9 +772,9 @@ static void enable_rsa(struct uart_8250_port *up)
{
if (up->port.type == PORT_RSA) {
if (up->port.uartclk != SERIAL_RSA_BAUD_BASE * 16) {
- spin_lock_irq(&up->port.lock);
+ uart_port_lock_irq(&up->port.lock);
__enable_rsa(up);
- spin_unlock_irq(&up->port.lock);
+ uart_port_unlock_irq(&up->port.lock);
}
if (up->port.uartclk == SERIAL_RSA_BAUD_BASE * 16)
serial_out(up, UART_RSA_FRR, 0);
@@ -794,7 +794,7 @@ static void disable_rsa(struct uart_8250_port *up)
if (up->port.type == PORT_RSA &&
up->port.uartclk == SERIAL_RSA_BAUD_BASE * 16) {
- spin_lock_irq(&up->port.lock);
+ uart_port_lock_irq(&up->port.lock);
mode = serial_in(up, UART_RSA_MSR);
result = !(mode & UART_RSA_MSR_FIFO);
@@ -807,7 +807,7 @@ static void disable_rsa(struct uart_8250_port *up)
if (result)
up->port.uartclk = SERIAL_RSA_BAUD_BASE_LO * 16;
- spin_unlock_irq(&up->port.lock);
+ uart_port_unlock_irq(&up->port.lock);
}
}
#endif /* CONFIG_SERIAL_8250_RSA */
@@ -1218,7 +1218,7 @@ static void autoconfig(struct uart_8250_port *up)
* We really do need global IRQs disabled here - we're going to
* be frobbing the chips IRQ enable register to see if it exists.
*/
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(&port->lock, flags);
up->capabilities = 0;
up->bugs = 0;
@@ -1257,7 +1257,7 @@ static void autoconfig(struct uart_8250_port *up)
/*
* We failed; there's nothing here
*/
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
DEBUG_AUTOCONF("IER test failed (%02x, %02x) ",
scratch2, scratch3);
goto out;
@@ -1281,7 +1281,7 @@ static void autoconfig(struct uart_8250_port *up)
status1 = serial_in(up, UART_MSR) & 0xF0;
serial8250_out_MCR(up, save_mcr);
if (status1 != 0x90) {
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
DEBUG_AUTOCONF("LOOP test failed (%02x) ",
status1);
goto out;
@@ -1354,7 +1354,7 @@ static void autoconfig(struct uart_8250_port *up)
serial_out(up, UART_IER, 0);
out_lock:
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
/*
* Check if the device is a Fintek F81216A
@@ -1466,12 +1466,12 @@ static enum hrtimer_restart serial8250_em485_handle_stop_tx(struct hrtimer *t)
p = em485->port;
serial8250_rpm_get(p);
- spin_lock_irqsave(&p->port.lock, flags);
+ uart_port_lock_irqsave(&p->port.lock, flags);
if (em485->active_timer == &em485->stop_tx_timer) {
__do_stop_tx_rs485(p);
em485->active_timer = NULL;
}
- spin_unlock_irqrestore(&p->port.lock, flags);
+ uart_port_unlock_irqrestore(&p->port.lock, flags);
serial8250_rpm_put(p);
return HRTIMER_NORESTART;
}
@@ -1620,12 +1620,12 @@ static enum hrtimer_restart serial8250_em485_handle_start_tx(struct hrtimer *t)
em485 = container_of(t, struct uart_8250_em485, start_tx_timer);
p = em485->port;
- spin_lock_irqsave(&p->port.lock, flags);
+ uart_port_lock_irqsave(&p->port.lock, flags);
if (em485->active_timer == &em485->start_tx_timer) {
__start_tx(&p->port);
em485->active_timer = NULL;
}
- spin_unlock_irqrestore(&p->port.lock, flags);
+ uart_port_unlock_irqrestore(&p->port.lock, flags);
return HRTIMER_NORESTART;
}
@@ -1867,7 +1867,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
if (iir & UART_IIR_NO_INT)
return 0;
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(&port->lock, flags);
status = serial_port_in(port, UART_LSR);
@@ -1880,7 +1880,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
if ((!up->dma || up->dma->tx_err) && (status & UART_LSR_THRE))
serial8250_tx_chars(up);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
return 1;
}
EXPORT_SYMBOL_GPL(serial8250_handle_irq);
@@ -1915,9 +1915,9 @@ static int serial8250_tx_threshold_handle_irq(struct uart_port *port)
if ((iir & UART_IIR_ID) == UART_IIR_THRI) {
struct uart_8250_port *up = up_to_u8250p(port);
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(&port->lock, flags);
serial8250_tx_chars(up);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
}
iir = serial_port_in(port, UART_IIR);
@@ -1932,10 +1932,10 @@ static unsigned int serial8250_tx_empty(struct uart_port *port)
serial8250_rpm_get(up);
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(&port->lock, flags);
lsr = serial_port_in(port, UART_LSR);
up->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
serial8250_rpm_put(up);
@@ -2008,13 +2008,13 @@ static void serial8250_break_ctl(struct uart_port *port, int break_state)
unsigned long flags;
serial8250_rpm_get(up);
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(&port->lock, flags);
if (break_state == -1)
up->lcr |= UART_LCR_SBC;
else
up->lcr &= ~UART_LCR_SBC;
serial_port_out(port, UART_LCR, up->lcr);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
serial8250_rpm_put(up);
}
@@ -2266,7 +2266,7 @@ int serial8250_do_startup(struct uart_port *port)
* the interrupt is enabled. Delays are necessary to
* allow register changes to become visible.
*/
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(&port->lock, flags);
if (up->port.irqflags & IRQF_SHARED)
disable_irq_nosync(port->irq);
@@ -2282,7 +2282,7 @@ int serial8250_do_startup(struct uart_port *port)
if (port->irqflags & IRQF_SHARED)
enable_irq(port->irq);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
/*
* If the interrupt is not reasserted, or we otherwise
@@ -2304,7 +2304,7 @@ int serial8250_do_startup(struct uart_port *port)
*/
serial_port_out(port, UART_LCR, UART_LCR_WLEN8);
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(&port->lock, flags);
if (up->port.flags & UPF_FOURPORT) {
if (!up->port.irq)
up->port.mctrl |= TIOCM_OUT1;
@@ -2351,7 +2351,7 @@ int serial8250_do_startup(struct uart_port *port)
}
dont_test_tx_en:
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
/*
* Clear the interrupt registers again for luck, and clear the
@@ -2418,17 +2418,17 @@ void serial8250_do_shutdown(struct uart_port *port)
/*
* Disable interrupts from this port
*/
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(&port->lock, flags);
up->ier = 0;
serial_port_out(port, UART_IER, 0);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
synchronize_irq(port->irq);
if (up->dma)
serial8250_release_dma(up);
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(&port->lock, flags);
if (port->flags & UPF_FOURPORT) {
/* reset interrupts on the AST Fourport board */
inb((port->iobase & 0xfe0) | 0x1f);
@@ -2437,7 +2437,7 @@ void serial8250_do_shutdown(struct uart_port *port)
port->mctrl &= ~TIOCM_OUT2;
serial8250_set_mctrl(port, port->mctrl);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
/*
* Disable break condition and FIFOs
@@ -2643,7 +2643,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
* interrupts disabled.
*/
serial8250_rpm_get(up);
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(&port->lock, flags);
up->lcr = cval; /* Save computed LCR */
@@ -2747,7 +2747,7 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
serial_port_out(port, UART_FCR, up->fcr); /* set fcr */
}
serial8250_set_mctrl(port, port->mctrl);
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
serial8250_rpm_put(up);
/* Don't rewrite B0 */
@@ -2770,15 +2770,15 @@ void serial8250_do_set_ldisc(struct uart_port *port, struct ktermios *termios)
{
if (termios->c_line == N_PPS) {
port->flags |= UPF_HARDPPS_CD;
- spin_lock_irq(&port->lock);
+ uart_port_lock_irq(&port->lock);
serial8250_enable_ms(port);
- spin_unlock_irq(&port->lock);
+ uart_port_unlock_irq(&port->lock);
} else {
port->flags &= ~UPF_HARDPPS_CD;
if (!UART_ENABLE_MS(port, termios->c_cflag)) {
- spin_lock_irq(&port->lock);
+ uart_port_lock_irq(&port->lock);
serial8250_disable_ms(port);
- spin_unlock_irq(&port->lock);
+ uart_port_unlock_irq(&port->lock);
}
}
}
@@ -3225,9 +3225,9 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s,
if (port->sysrq)
locked = 0;
else if (oops_in_progress)
- locked = spin_trylock_irqsave(&port->lock, flags);
+ locked = uart_port_trylock_irqsave(&port->lock, flags);
else
- spin_lock_irqsave(&port->lock, flags);
+ uart_port_lock_irqsave(&port->lock, flags);
/*
* First save the IER then disable the interrupts
@@ -3265,7 +3265,7 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s,
serial8250_modem_status(up);
if (locked)
- spin_unlock_irqrestore(&port->lock, flags);
+ uart_port_unlock_irqrestore(&port->lock, flags);
serial8250_rpm_put(up);
}
--
2.17.1
^ permalink raw reply related
* Re: [RFC PATCH 5/6] arm64: dts: ti: Add Support for AM654 SoC
From: Sekhar Nori @ 2018-06-15 13:38 UTC (permalink / raw)
To: Tony Lindgren, Nishanth Menon
Cc: Mark Rutland, devicetree, Sudeep Holla, Vignesh R,
Catalin Marinas, Santosh Shilimkar, Will Deacon,
linux-kernel@vger.kernel.org, Russell King, Tero Kristo,
Rob Herring, open list:SERIAL DRIVERS, Greg Kroah-Hartman,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20180615050108.GG112168@atomide.com>
Hi Tony,
On Friday 15 June 2018 10:31 AM, Tony Lindgren wrote:
> * Nishanth Menon <nm@ti.com> [180614 13:07]:
>> On 12:38-20180614, Tony Lindgren wrote:
>> From A53 view, a more accurate view might be - from an interconnect
>> view of the world (still simplified - i have ignored the sub bus
>> segments in the representations below):
>>
>> msmc {
>> navss_main {
>> cbass_main{
>> cbass_mcu {
>> navss_mcu {
>> };
>> cbass_wkup{
>> };
>> };
>> };
>> };
>> };
>>
>> From R5 view, the view will be very different ofcourse:
>> view of the world (still simplified):
>>
>> cbass_mcu {
>> navss_mcu {
>> };
>> cbass_wkup{
>> };
>> cbass_main{
>> navss_main {
>> msmc {
>> };
>> };
>> };
>> };
>
> Well if we follow the hardware representation of the interconnects,
> it should not matter from which processor view you're looking at things.
> There are just different ranges provided.
AFAIK, the root node needs to have the CPU which is using the DT. So,
the hierarchy will change based on CPU view (if we describe it fully).
How well we can reuse individual interconnect segments is something I
have to think about / experiment. Will have to be wary of any "short
paths" or "cross connections".
Thanks,
Sekhar
^ 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