[PATCH 01/11] tty: serial: add missing braces

[PATCH 01/11] tty: serial: add missing braces

[Felipe Balbi]

per CodingStyle we should have those braces, no
functional changes.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/tty/serial/omap-serial.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 77f0351..73abba8 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1677,8 +1677,10 @@ static int serial_omap_probe(struct platform_device *pdev)
 	    omap_up_info->DTR_present) {
 		up->DTR_gpio = omap_up_info->DTR_gpio;
 		up->DTR_inverted = omap_up_info->DTR_inverted;
-	} else
+	} else {
 		up->DTR_gpio = -EINVAL;
+	}
+
 	up->DTR_active = 0;
 
 	up->dev = &pdev->dev;
@@ -1740,6 +1742,7 @@ static int serial_omap_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, up);
 	if (omap_up_info->autosuspend_timeout == 0)
 		omap_up_info->autosuspend_timeout = -1;
+
 	device_init_wakeup(up->dev, true);
 	pm_runtime_use_autosuspend(&pdev->dev);
 	pm_runtime_set_autosuspend_delay(&pdev->dev,
-- 
1.9.1.286.g5172cb3

[PATCH 02/11] tty: serial: omap: switch over to devm_request_gpio

[Felipe Balbi]

this will make sure gpio gets freed automatically
when this device is destroyed.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/tty/serial/omap-serial.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 73abba8..db5d90b 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1594,7 +1594,7 @@ static int serial_omap_probe_rs485(struct uart_omap_port *up,
 	/* check for tx enable gpio */
 	up->rts_gpio = of_get_named_gpio_flags(np, "rts-gpio", 0, &flags);
 	if (gpio_is_valid(up->rts_gpio)) {
-		ret = gpio_request(up->rts_gpio, "omap-serial");
+		ret = devm_gpio_request(up->dev, up->rts_gpio, "omap-serial");
 		if (ret < 0)
 			return ret;
 		ret = gpio_direction_output(up->rts_gpio,
@@ -1660,7 +1660,8 @@ static int serial_omap_probe(struct platform_device *pdev)
 
 	if (gpio_is_valid(omap_up_info->DTR_gpio) &&
 	    omap_up_info->DTR_present) {
-		ret = gpio_request(omap_up_info->DTR_gpio, "omap-serial");
+		ret = devm_gpio_request(&pdev->dev, omap_up_info->DTR_gpio,
+				"omap-serial");
 		if (ret < 0)
 			return ret;
 		ret = gpio_direction_output(omap_up_info->DTR_gpio,
-- 
1.9.1.286.g5172cb3

[PATCH 03/11] tty: serial: omap: cleanup variable declarations

[Felipe Balbi]

cleanup only, no functional changes.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/tty/serial/omap-serial.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index db5d90b..6831425 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1624,10 +1624,13 @@ static int serial_omap_probe_rs485(struct uart_omap_port *up,
 
 static int serial_omap_probe(struct platform_device *pdev)
 {
-	struct uart_omap_port	*up;
-	struct resource		*mem, *irq;
 	struct omap_uart_port_info *omap_up_info = dev_get_platdata(&pdev->dev);
-	int ret, uartirq = 0, wakeirq = 0;
+	struct uart_omap_port *up;
+	struct resource *mem;
+	struct resource *irq;
+	int uartirq = 0;
+	int wakeirq = 0;
+	int ret;
 
 	/* The optional wakeirq may be specified in the board dts file */
 	if (pdev->dev.of_node) {
-- 
1.9.1.286.g5172cb3

[PATCH 04/11] tty: serial: omap: switch over to platform_get_resource

[Felipe Balbi]

this way we can remove one pointer declaration.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/tty/serial/omap-serial.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 6831425..d041060 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1627,7 +1627,6 @@ static int serial_omap_probe(struct platform_device *pdev)
 	struct omap_uart_port_info *omap_up_info = dev_get_platdata(&pdev->dev);
 	struct uart_omap_port *up;
 	struct resource *mem;
-	struct resource *irq;
 	int uartirq = 0;
 	int wakeirq = 0;
 	int ret;
@@ -1641,12 +1640,9 @@ static int serial_omap_probe(struct platform_device *pdev)
 		omap_up_info = of_get_uart_port_info(&pdev->dev);
 		pdev->dev.platform_data = omap_up_info;
 	} else {
-		irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-		if (!irq) {
-			dev_err(&pdev->dev, "no irq resource?\n");
-			return -ENODEV;
-		}
-		uartirq = irq->start;
+		uartirq = platform_get_irq(pdev, 0);
+		if (uartirq < 0)
+			return -EPROBE_DEFER;
 	}
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-- 
1.9.1.286.g5172cb3

[PATCH 05/11] tty: serial: omap: switch over to devm_ioremap_resource

[Felipe Balbi]

just using helper function to remove some duplicated
code a bit. While at that, also move allocation of
struct uart_omap_port higher in the code so that
we return much earlier in case of no memory.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/tty/serial/omap-serial.c | 32 +++++++++-----------------------
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index d041060..d785327 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1627,6 +1627,7 @@ static int serial_omap_probe(struct platform_device *pdev)
 	struct omap_uart_port_info *omap_up_info = dev_get_platdata(&pdev->dev);
 	struct uart_omap_port *up;
 	struct resource *mem;
+	void __iomem *base;
 	int uartirq = 0;
 	int wakeirq = 0;
 	int ret;
@@ -1645,17 +1646,14 @@ static int serial_omap_probe(struct platform_device *pdev)
 			return -EPROBE_DEFER;
 	}
 
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!mem) {
-		dev_err(&pdev->dev, "no mem resource?\n");
-		return -ENODEV;
-	}
+	up = devm_kzalloc(&pdev->dev, sizeof(*up), GFP_KERNEL);
+	if (!up)
+		return -ENOMEM;
 
-	if (!devm_request_mem_region(&pdev->dev, mem->start, resource_size(mem),
-				pdev->dev.driver->name)) {
-		dev_err(&pdev->dev, "memory region already claimed\n");
-		return -EBUSY;
-	}
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, mem);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
 
 	if (gpio_is_valid(omap_up_info->DTR_gpio) &&
 	    omap_up_info->DTR_present) {
@@ -1669,10 +1667,6 @@ static int serial_omap_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	up = devm_kzalloc(&pdev->dev, sizeof(*up), GFP_KERNEL);
-	if (!up)
-		return -ENOMEM;
-
 	if (gpio_is_valid(omap_up_info->DTR_gpio) &&
 	    omap_up_info->DTR_present) {
 		up->DTR_gpio = omap_up_info->DTR_gpio;
@@ -1715,14 +1709,7 @@ static int serial_omap_probe(struct platform_device *pdev)
 
 	sprintf(up->name, "OMAP UART%d", up->port.line);
 	up->port.mapbase = mem->start;
-	up->port.membase = devm_ioremap(&pdev->dev, mem->start,
-						resource_size(mem));
-	if (!up->port.membase) {
-		dev_err(&pdev->dev, "can't ioremap UART\n");
-		ret = -ENOMEM;
-		goto err_ioremap;
-	}
-
+	up->port.membase = base;
 	up->port.flags = omap_up_info->flags;
 	up->port.uartclk = omap_up_info->uartclk;
 	if (!up->port.uartclk) {
@@ -1769,7 +1756,6 @@ static int serial_omap_probe(struct platform_device *pdev)
 err_add_port:
 	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-err_ioremap:
 err_rs485:
 err_port_line:
 	dev_err(&pdev->dev, "[UART%d]: failure [%s]: %d\n",
-- 
1.9.1.286.g5172cb3

[PATCH 06/11] tty: serial: omap: remove some dead code

[Felipe Balbi]

nobody passes a DTR_gpio to this driver, so
this code is not necessary.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/tty/serial/omap-serial.c | 39 ---------------------------------------
 1 file changed, 39 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index d785327..c79d3e6 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -163,10 +163,6 @@ struct uart_omap_port {
 	u8			wakeups_enabled;
 	u32			features;
 
-	int			DTR_gpio;
-	int			DTR_inverted;
-	int			DTR_active;
-
 	struct serial_rs485	rs485;
 	int			rts_gpio;
 
@@ -685,16 +681,6 @@ static void serial_omap_set_mctrl(struct uart_port *port, unsigned int mctrl)
 	serial_out(up, UART_MCR, up->mcr);
 	pm_runtime_mark_last_busy(up->dev);
 	pm_runtime_put_autosuspend(up->dev);
-
-	if (gpio_is_valid(up->DTR_gpio) &&
-	    !!(mctrl & TIOCM_DTR) != up->DTR_active) {
-		up->DTR_active = !up->DTR_active;
-		if (gpio_cansleep(up->DTR_gpio))
-			schedule_work(&up->qos_work);
-		else
-			gpio_set_value(up->DTR_gpio,
-				       up->DTR_active != up->DTR_inverted);
-	}
 }
 
 static void serial_omap_break_ctl(struct uart_port *port, int break_state)
@@ -838,9 +824,6 @@ static void serial_omap_uart_qos_work(struct work_struct *work)
 						qos_work);
 
 	pm_qos_update_request(&up->pm_qos_request, up->latency);
-	if (gpio_is_valid(up->DTR_gpio))
-		gpio_set_value_cansleep(up->DTR_gpio,
-					up->DTR_active != up->DTR_inverted);
 }
 
 static void
@@ -1655,28 +1638,6 @@ static int serial_omap_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-	if (gpio_is_valid(omap_up_info->DTR_gpio) &&
-	    omap_up_info->DTR_present) {
-		ret = devm_gpio_request(&pdev->dev, omap_up_info->DTR_gpio,
-				"omap-serial");
-		if (ret < 0)
-			return ret;
-		ret = gpio_direction_output(omap_up_info->DTR_gpio,
-					    omap_up_info->DTR_inverted);
-		if (ret < 0)
-			return ret;
-	}
-
-	if (gpio_is_valid(omap_up_info->DTR_gpio) &&
-	    omap_up_info->DTR_present) {
-		up->DTR_gpio = omap_up_info->DTR_gpio;
-		up->DTR_inverted = omap_up_info->DTR_inverted;
-	} else {
-		up->DTR_gpio = -EINVAL;
-	}
-
-	up->DTR_active = 0;
-
 	up->dev = &pdev->dev;
 	up->port.dev = &pdev->dev;
 	up->port.type = PORT_OMAP;
-- 
1.9.1.286.g5172cb3

[PATCH 07/11] tty: serial: omap: remove unneeded singlethread workqueue

[Felipe Balbi]

it wasn't used by anything, just remove it.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/tty/serial/omap-serial.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index c79d3e6..b092d3d 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -180,8 +180,6 @@ static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];
 /* Forward declaration of functions */
 static void serial_omap_mdr1_errataset(struct uart_omap_port *up, u8 mdr1);
 
-static struct workqueue_struct *serial_omap_uart_wq;
-
 static inline unsigned int serial_in(struct uart_omap_port *up, int offset)
 {
 	offset <<= up->port.regshift;
@@ -1684,7 +1682,6 @@ static int serial_omap_probe(struct platform_device *pdev)
 	up->calc_latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE;
 	pm_qos_add_request(&up->pm_qos_request,
 		PM_QOS_CPU_DMA_LATENCY, up->latency);
-	serial_omap_uart_wq = create_singlethread_workqueue(up->name);
 	INIT_WORK(&up->qos_work, serial_omap_uart_qos_work);
 
 	platform_set_drvdata(pdev, up);
-- 
1.9.1.286.g5172cb3

[PATCH 08/11] In the uart_handle_cts_change(), uart_write_wakeup() is called after we call @uart_port->ops->start_tx().

[Felipe Balbi]

From: Huang Shijie <b32955@freescale.com>

The Documentation/serial/driver tells us:
-----------------------------------------------
  start_tx(port)
	Start transmitting characters.

	Locking: port->lock taken.
	Interrupts: locally disabled.
-----------------------------------------------

So when the uart_write_wakeup() is called, the port->lock is taken by
the upper. See the following callstack:

	|_ uart_write_wakeup
	   |_ tty_wakeup
	      |_ ld->ops->write_wakeup

With the port->lock held, we call the @write_wakeup. Some implemetation of
the @write_wakeup does not notice that the port->lock is held, and it still
tries to send data with uart_write() which will try to grab the prot->lock.
A dead lock occurs, see the following log caught in the Bluetooth by uart:

--------------------------------------------------------------------
BUG: spinlock lockup suspected on CPU#0, swapper/0/0
 lock: 0xdc3f4410, .magic: dead4ead, .owner: swapper/0/0, .owner_cpu: 0
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W    3.10.17-16839-ge4a1bef #1320
[<80014cbc>] (unwind_backtrace+0x0/0x138) from [<8001251c>] (show_stack+0x10/0x14)
[<8001251c>] (show_stack+0x10/0x14) from [<802816ac>] (do_raw_spin_lock+0x108/0x184)
[<802816ac>] (do_raw_spin_lock+0x108/0x184) from [<806a22b0>] (_raw_spin_lock_irqsave+0x54/0x60)
[<806a22b0>] (_raw_spin_lock_irqsave+0x54/0x60) from [<802f5754>] (uart_write+0x38/0xe0)
[<802f5754>] (uart_write+0x38/0xe0) from [<80455270>] (hci_uart_tx_wakeup+0xa4/0x168)
[<80455270>] (hci_uart_tx_wakeup+0xa4/0x168) from [<802dab18>] (tty_wakeup+0x50/0x5c)
[<802dab18>] (tty_wakeup+0x50/0x5c) from [<802f81a4>] (imx_rtsint+0x50/0x80)
[<802f81a4>] (imx_rtsint+0x50/0x80) from [<802f88f4>] (imx_int+0x158/0x17c)
[<802f88f4>] (imx_int+0x158/0x17c) from [<8007abe0>] (handle_irq_event_percpu+0x50/0x194)
[<8007abe0>] (handle_irq_event_percpu+0x50/0x194) from [<8007ad60>] (handle_irq_event+0x3c/0x5c)
--------------------------------------------------------------------

This patch adds more limits to the @write_wakeup, the one who wants to
implemet the @write_wakeup should follow the limits which avoid the deadlock.

Signed-off-by: Huang Shijie <b32955@freescale.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 include/linux/tty_ldisc.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index b8347c2..8946e31 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -92,7 +92,10 @@
  *	This function is called by the low-level tty driver to signal
  *	that line discpline should try to send more characters to the
  *	low-level driver for transmission.  If the line discpline does
- *	not have any more data to send, it can just return.
+ *	not have any more data to send, it can just return. If the line
+ *	discipline does have some data to send, please arise a tasklet
+ *	or workqueue to do the real data transfer. Do not send data in
+ *	this hook, it may leads to a deadlock.
  *
  * int (*hangup)(struct tty_struct *)
  *
-- 
1.9.1.286.g5172cb3

Re: [PATCH 08/11] In the uart_handle_cts_change(), uart_write_wakeup() is called after we call @uart_port->ops->start_tx().

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2545 bytes --]

Hi,


On Thu, Mar 20, 2014 at 02:30:04PM -0500, Felipe Balbi wrote:
> From: Huang Shijie <b32955@freescale.com>
> 
> The Documentation/serial/driver tells us:
> -----------------------------------------------
>   start_tx(port)
> 	Start transmitting characters.
> 
> 	Locking: port->lock taken.
> 	Interrupts: locally disabled.
> -----------------------------------------------
> 
> So when the uart_write_wakeup() is called, the port->lock is taken by
> the upper. See the following callstack:
> 
> 	|_ uart_write_wakeup
> 	   |_ tty_wakeup
> 	      |_ ld->ops->write_wakeup
> 
> With the port->lock held, we call the @write_wakeup. Some implemetation of
> the @write_wakeup does not notice that the port->lock is held, and it still
> tries to send data with uart_write() which will try to grab the prot->lock.
> A dead lock occurs, see the following log caught in the Bluetooth by uart:
> 
> --------------------------------------------------------------------
> BUG: spinlock lockup suspected on CPU#0, swapper/0/0
>  lock: 0xdc3f4410, .magic: dead4ead, .owner: swapper/0/0, .owner_cpu: 0
> CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W    3.10.17-16839-ge4a1bef #1320
> [<80014cbc>] (unwind_backtrace+0x0/0x138) from [<8001251c>] (show_stack+0x10/0x14)
> [<8001251c>] (show_stack+0x10/0x14) from [<802816ac>] (do_raw_spin_lock+0x108/0x184)
> [<802816ac>] (do_raw_spin_lock+0x108/0x184) from [<806a22b0>] (_raw_spin_lock_irqsave+0x54/0x60)
> [<806a22b0>] (_raw_spin_lock_irqsave+0x54/0x60) from [<802f5754>] (uart_write+0x38/0xe0)
> [<802f5754>] (uart_write+0x38/0xe0) from [<80455270>] (hci_uart_tx_wakeup+0xa4/0x168)
> [<80455270>] (hci_uart_tx_wakeup+0xa4/0x168) from [<802dab18>] (tty_wakeup+0x50/0x5c)
> [<802dab18>] (tty_wakeup+0x50/0x5c) from [<802f81a4>] (imx_rtsint+0x50/0x80)
> [<802f81a4>] (imx_rtsint+0x50/0x80) from [<802f88f4>] (imx_int+0x158/0x17c)
> [<802f88f4>] (imx_int+0x158/0x17c) from [<8007abe0>] (handle_irq_event_percpu+0x50/0x194)
> [<8007abe0>] (handle_irq_event_percpu+0x50/0x194) from [<8007ad60>] (handle_irq_event+0x3c/0x5c)
> --------------------------------------------------------------------
> 
> This patch adds more limits to the @write_wakeup, the one who wants to
> implemet the @write_wakeup should follow the limits which avoid the deadlock.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>

patch came without a subject, Peter already sent it anyway, so Greg can
ignore this one.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 09/11] bluetooth: hci_ldisc: fix deadlock condition

[Felipe Balbi]

LDISCs shouldn't call tty->ops->write() from within
->write_wakeup().

->write_wakeup() is called with port lock taken and
IRQs disabled, tty->ops->write() will try to acquire
the same port lock and we will deadlock.

Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Reported-by: Huang Shijie <b32955@freescale.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/bluetooth/hci_ldisc.c | 24 +++++++++++++++++++-----
 drivers/bluetooth/hci_uart.h  |  1 +
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 6e06f6f..77af52f 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -118,10 +118,6 @@ static inline struct sk_buff *hci_uart_dequeue(struct hci_uart *hu)
 
 int hci_uart_tx_wakeup(struct hci_uart *hu)
 {
-	struct tty_struct *tty = hu->tty;
-	struct hci_dev *hdev = hu->hdev;
-	struct sk_buff *skb;
-
 	if (test_and_set_bit(HCI_UART_SENDING, &hu->tx_state)) {
 		set_bit(HCI_UART_TX_WAKEUP, &hu->tx_state);
 		return 0;
@@ -129,6 +125,22 @@ int hci_uart_tx_wakeup(struct hci_uart *hu)
 
 	BT_DBG("");
 
+	schedule_work(&hu->write_work);
+
+	return 0;
+}
+
+static void hci_uart_write_work(struct work_struct *work)
+{
+	struct hci_uart *hu = container_of(work, struct hci_uart, write_work);
+	struct tty_struct *tty = hu->tty;
+	struct hci_dev *hdev = hu->hdev;
+	struct sk_buff *skb;
+
+	/* REVISIT: should we cope with bad skbs or ->write() returning
+	 * and error value ?
+	 */
+
 restart:
 	clear_bit(HCI_UART_TX_WAKEUP, &hu->tx_state);
 
@@ -153,7 +165,6 @@ restart:
 		goto restart;
 
 	clear_bit(HCI_UART_SENDING, &hu->tx_state);
-	return 0;
 }
 
 static void hci_uart_init_work(struct work_struct *work)
@@ -281,6 +292,7 @@ static int hci_uart_tty_open(struct tty_struct *tty)
 	tty->receive_room = 65536;
 
 	INIT_WORK(&hu->init_ready, hci_uart_init_work);
+	INIT_WORK(&hu->write_work, hci_uart_write_work);
 
 	spin_lock_init(&hu->rx_lock);
 
@@ -318,6 +330,8 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 	if (hdev)
 		hci_uart_close(hdev);
 
+	cancel_work_sync(&hu->write_work);
+
 	if (test_and_clear_bit(HCI_UART_PROTO_SET, &hu->flags)) {
 		if (hdev) {
 			if (test_bit(HCI_UART_REGISTERED, &hu->flags))
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index fffa61f..12df101 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -68,6 +68,7 @@ struct hci_uart {
 	unsigned long		hdev_flags;
 
 	struct work_struct	init_ready;
+	struct work_struct	write_work;
 
 	struct hci_uart_proto	*proto;
 	void			*priv;
-- 
1.9.1.286.g5172cb3

Re: [PATCH 09/11] bluetooth: hci_ldisc: fix deadlock condition

[Marcel Holtmann]

Hi Felipe,

> LDISCs shouldn't call tty->ops->write() from within
> ->write_wakeup().
> 
> ->write_wakeup() is called with port lock taken and
> IRQs disabled, tty->ops->write() will try to acquire
> the same port lock and we will deadlock.
> 
> Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
> Reported-by: Huang Shijie <b32955@freescale.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
> drivers/bluetooth/hci_ldisc.c | 24 +++++++++++++++++++-----
> drivers/bluetooth/hci_uart.h  |  1 +
> 2 files changed, 20 insertions(+), 5 deletions(-)

I hope these are not causing any conflicts with bluetooth-next / linux-next. If not, then I can let Greg take it through tty-next tree.

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [PATCH 09/11] bluetooth: hci_ldisc: fix deadlock condition

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1079 bytes --]

On Thu, Mar 20, 2014 at 12:40:40PM -0700, Marcel Holtmann wrote:
> Hi Felipe,
> 
> > LDISCs shouldn't call tty->ops->write() from within
> > ->write_wakeup().
> > 
> > ->write_wakeup() is called with port lock taken and
> > IRQs disabled, tty->ops->write() will try to acquire
> > the same port lock and we will deadlock.
> > 
> > Reviewed-by: Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
> > Reported-by: Huang Shijie <b32955-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
> > ---
> > drivers/bluetooth/hci_ldisc.c | 24 +++++++++++++++++++-----
> > drivers/bluetooth/hci_uart.h  |  1 +
> > 2 files changed, 20 insertions(+), 5 deletions(-)
> 
> I hope these are not causing any conflicts with bluetooth-next /
> linux-next. If not, then I can let Greg take it through tty-next tree.
> 
> Acked-by: Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>

tty-next is already closed, i'll rebase (if necessary) once -rc1 is out
;-)

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 09/11] bluetooth: hci_ldisc: fix deadlock condition

[Peter Hurley]

[ +to Marcel Holtmann ]

On 03/20/2014 03:30 PM, Felipe Balbi wrote:
> LDISCs shouldn't call tty->ops->write() from within
> ->write_wakeup().
>
> ->write_wakeup() is called with port lock taken and
> IRQs disabled, tty->ops->write() will try to acquire
> the same port lock and we will deadlock.
>
> Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
> Reported-by: Huang Shijie <b32955@freescale.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>

I just noticed this patch wasn't addressed to Marcel;
seems like this should go through the bluetooth tree (but not
through bluetooth-next because it fixes an oops).

Marcel,

You may want to build on top of this patch split handling;
I noticed some of the protocol drivers are calling
hci_uart_tx_wakeup() from work functions already (so don't
need to schedule another work...)

Regards,
Peter Hurley

> ---
>   drivers/bluetooth/hci_ldisc.c | 24 +++++++++++++++++++-----
>   drivers/bluetooth/hci_uart.h  |  1 +
>   2 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index 6e06f6f..77af52f 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -118,10 +118,6 @@ static inline struct sk_buff *hci_uart_dequeue(struct hci_uart *hu)
>
>   int hci_uart_tx_wakeup(struct hci_uart *hu)
>   {
> -	struct tty_struct *tty = hu->tty;
> -	struct hci_dev *hdev = hu->hdev;
> -	struct sk_buff *skb;
> -
>   	if (test_and_set_bit(HCI_UART_SENDING, &hu->tx_state)) {
>   		set_bit(HCI_UART_TX_WAKEUP, &hu->tx_state);
>   		return 0;
> @@ -129,6 +125,22 @@ int hci_uart_tx_wakeup(struct hci_uart *hu)
>
>   	BT_DBG("");
>
> +	schedule_work(&hu->write_work);
> +
> +	return 0;
> +}
> +
> +static void hci_uart_write_work(struct work_struct *work)
> +{
> +	struct hci_uart *hu = container_of(work, struct hci_uart, write_work);
> +	struct tty_struct *tty = hu->tty;
> +	struct hci_dev *hdev = hu->hdev;
> +	struct sk_buff *skb;
> +
> +	/* REVISIT: should we cope with bad skbs or ->write() returning
> +	 * and error value ?
> +	 */
> +
>   restart:
>   	clear_bit(HCI_UART_TX_WAKEUP, &hu->tx_state);
>
> @@ -153,7 +165,6 @@ restart:
>   		goto restart;
>
>   	clear_bit(HCI_UART_SENDING, &hu->tx_state);
> -	return 0;
>   }
>
>   static void hci_uart_init_work(struct work_struct *work)
> @@ -281,6 +292,7 @@ static int hci_uart_tty_open(struct tty_struct *tty)
>   	tty->receive_room = 65536;
>
>   	INIT_WORK(&hu->init_ready, hci_uart_init_work);
> +	INIT_WORK(&hu->write_work, hci_uart_write_work);
>
>   	spin_lock_init(&hu->rx_lock);
>
> @@ -318,6 +330,8 @@ static void hci_uart_tty_close(struct tty_struct *tty)
>   	if (hdev)
>   		hci_uart_close(hdev);
>
> +	cancel_work_sync(&hu->write_work);
> +
>   	if (test_and_clear_bit(HCI_UART_PROTO_SET, &hu->flags)) {
>   		if (hdev) {
>   			if (test_bit(HCI_UART_REGISTERED, &hu->flags))
> diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
> index fffa61f..12df101 100644
> --- a/drivers/bluetooth/hci_uart.h
> +++ b/drivers/bluetooth/hci_uart.h
> @@ -68,6 +68,7 @@ struct hci_uart {
>   	unsigned long		hdev_flags;
>
>   	struct work_struct	init_ready;
> +	struct work_struct	write_work;
>
>   	struct hci_uart_proto	*proto;
>   	void			*priv;
>

Re: [PATCH 09/11] bluetooth: hci_ldisc: fix deadlock condition

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1117 bytes --]

Hi,

On Wed, Mar 26, 2014 at 08:47:15PM -0400, Peter Hurley wrote:
> [ +to Marcel Holtmann ]
> 
> On 03/20/2014 03:30 PM, Felipe Balbi wrote:
> >LDISCs shouldn't call tty->ops->write() from within
> >->write_wakeup().
> >
> >->write_wakeup() is called with port lock taken and
> >IRQs disabled, tty->ops->write() will try to acquire
> >the same port lock and we will deadlock.
> >
> >Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
> >Reported-by: Huang Shijie <b32955@freescale.com>
> >Signed-off-by: Felipe Balbi <balbi@ti.com>
> 
> I just noticed this patch wasn't addressed to Marcel;
> seems like this should go through the bluetooth tree (but not
> through bluetooth-next because it fixes an oops).

read the archives:

http://marc.info/?l=linux-bluetooth&m=139534449409583&w=2

> Marcel,
> 
> You may want to build on top of this patch split handling;
> I noticed some of the protocol drivers are calling
> hci_uart_tx_wakeup() from work functions already (so don't
> need to schedule another work...)

I don't think that should be part of $subject, though.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 09/11] bluetooth: hci_ldisc: fix deadlock condition

[Peter Hurley]

On 03/26/2014 10:09 PM, Felipe Balbi wrote:
>> I just noticed this patch wasn't addressed to Marcel;
>> seems like this should go through the bluetooth tree (but not
>> through bluetooth-next because it fixes an oops).
>
> read the archives:
>
> http://marc.info/?l=linux-bluetooth&m=139534449409583&w=2

Sorry. I did actually get Marcel's reply but Thunderbird
didn't parent the reply properly in my inbox and I forgot about it.


>> Marcel,
>>
>> You may want to build on top of this patch split handling;
>> I noticed some of the protocol drivers are calling
>> hci_uart_tx_wakeup() from work functions already (so don't
>> need to schedule another work...)
>
> I don't think that should be part of $subject, though.

I don't understand what you mean here.

Regards,
Peter Hurley

Re: [PATCH 09/11] bluetooth: hci_ldisc: fix deadlock condition

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 606 bytes --]

Hi,

On Wed, Mar 26, 2014 at 10:20:15PM -0400, Peter Hurley wrote:
> >>You may want to build on top of this patch split handling;
> >>I noticed some of the protocol drivers are calling
> >>hci_uart_tx_wakeup() from work functions already (so don't
> >>need to schedule another work...)
> >
> >I don't think that should be part of $subject, though.
> 
> I don't understand what you mean here.

it seemed, at first, like you suggested to redo this patch modifying the
protocol drivers to avoid two workqueues. But now that I read it again
you _did_ write "on top of this patch".

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 09/11] bluetooth: hci_ldisc: fix deadlock condition

[Dean Jenkins]

On 20/03/14 19:30, Felipe Balbi wrote:
> LDISCs shouldn't call tty->ops->write() from within
> ->write_wakeup().
>
> ->write_wakeup() is called with port lock taken and
> IRQs disabled, tty->ops->write() will try to acquire
> the same port lock and we will deadlock.
>
I think the work queue should be placed into hci_uart_tty_wakeup() and 
not hci_uart_tx_wakeup() as added by this patch.

The reason is that the kernel thread hci_uart_send_frame() calls 
hci_uart_tx_wakeup() and this patch unnecessarily introduces a work 
queue in the program flow of that kernel thread.

In other words, I think this patch has undesirable side-effects such as 
adding latency and increased processor loading for hci_uart_send_frame().

Regards,
Dean

> Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
> Reported-by: Huang Shijie <b32955@freescale.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>   drivers/bluetooth/hci_ldisc.c | 24 +++++++++++++++++++-----
>   drivers/bluetooth/hci_uart.h  |  1 +
>   2 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index 6e06f6f..77af52f 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -118,10 +118,6 @@ static inline struct sk_buff *hci_uart_dequeue(struct hci_uart *hu)
>   
>   int hci_uart_tx_wakeup(struct hci_uart *hu)
>   {
> -	struct tty_struct *tty = hu->tty;
> -	struct hci_dev *hdev = hu->hdev;
> -	struct sk_buff *skb;
> -
>   	if (test_and_set_bit(HCI_UART_SENDING, &hu->tx_state)) {
>   		set_bit(HCI_UART_TX_WAKEUP, &hu->tx_state);
>   		return 0;
> @@ -129,6 +125,22 @@ int hci_uart_tx_wakeup(struct hci_uart *hu)
>   
>   	BT_DBG("");
>   
> +	schedule_work(&hu->write_work);
> +
> +	return 0;
> +}
> +
> +static void hci_uart_write_work(struct work_struct *work)
> +{
> +	struct hci_uart *hu = container_of(work, struct hci_uart, write_work);
> +	struct tty_struct *tty = hu->tty;
> +	struct hci_dev *hdev = hu->hdev;
> +	struct sk_buff *skb;
> +
> +	/* REVISIT: should we cope with bad skbs or ->write() returning
> +	 * and error value ?
> +	 */
> +
>   restart:
>   	clear_bit(HCI_UART_TX_WAKEUP, &hu->tx_state);
>   
> @@ -153,7 +165,6 @@ restart:
>   		goto restart;
>   
>   	clear_bit(HCI_UART_SENDING, &hu->tx_state);
> -	return 0;
>   }
>   
>   static void hci_uart_init_work(struct work_struct *work)
> @@ -281,6 +292,7 @@ static int hci_uart_tty_open(struct tty_struct *tty)
>   	tty->receive_room = 65536;
>   
>   	INIT_WORK(&hu->init_ready, hci_uart_init_work);
> +	INIT_WORK(&hu->write_work, hci_uart_write_work);
>   
>   	spin_lock_init(&hu->rx_lock);
>   
> @@ -318,6 +330,8 @@ static void hci_uart_tty_close(struct tty_struct *tty)
>   	if (hdev)
>   		hci_uart_close(hdev);
>   
> +	cancel_work_sync(&hu->write_work);
> +
>   	if (test_and_clear_bit(HCI_UART_PROTO_SET, &hu->flags)) {
>   		if (hdev) {
>   			if (test_bit(HCI_UART_REGISTERED, &hu->flags))
> diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
> index fffa61f..12df101 100644
> --- a/drivers/bluetooth/hci_uart.h
> +++ b/drivers/bluetooth/hci_uart.h
> @@ -68,6 +68,7 @@ struct hci_uart {
>   	unsigned long		hdev_flags;
>   
>   	struct work_struct	init_ready;
> +	struct work_struct	write_work;
>   
>   	struct hci_uart_proto	*proto;
>   	void			*priv;

[PATCH 10/11] Revert "serial: omap: unlock the port lock"

[Felipe Balbi]

This reverts commit 0324a821029e1f54e7a7f8fed48693cfce42dc0e.

That commit tried to fix a deadlock problem when using
hci_ldisc, but it turns out the bug was in hci_ldsic
all along where it was calling ->write() from within
->write_wakeup() callback.

The problem is that ->write_wakeup() was called with
port lock held and ->write() tried to grab the same
port lock.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/tty/serial/omap-serial.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index b092d3d..be10e7e 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -380,11 +380,8 @@ static void transmit_chars(struct uart_omap_port *up, unsigned int lsr)
 			break;
 	} while (--count > 0);
 
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS) {
-		spin_unlock(&up->port.lock);
+	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
 		uart_write_wakeup(&up->port);
-		spin_lock(&up->port.lock);
-	}
 
 	if (uart_circ_empty(xmit))
 		serial_omap_stop_tx(&up->port);
-- 
1.9.1.286.g5172cb3

Re: [PATCH 10/11] Revert "serial: omap: unlock the port lock"

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [140320 12:39]:
> This reverts commit 0324a821029e1f54e7a7f8fed48693cfce42dc0e.
> 
> That commit tried to fix a deadlock problem when using
> hci_ldisc, but it turns out the bug was in hci_ldsic
> all along where it was calling ->write() from within
> ->write_wakeup() callback.
> 
> The problem is that ->write_wakeup() was called with
> port lock held and ->write() tried to grab the same
> port lock.

Should this and the next patch be earlier in the series
as a fix for the v3.15-rc cycle? Should they be cc: stable
as well?

Regards,

Tony

Re: [PATCH 10/11] Revert "serial: omap: unlock the port lock"

[Peter Hurley]

On 03/25/2014 02:28 PM, Tony Lindgren wrote:
> * Felipe Balbi <balbi@ti.com> [140320 12:39]:
>> This reverts commit 0324a821029e1f54e7a7f8fed48693cfce42dc0e.
>>
>> That commit tried to fix a deadlock problem when using
>> hci_ldisc, but it turns out the bug was in hci_ldsic
>> all along where it was calling ->write() from within
>> ->write_wakeup() callback.
>>
>> The problem is that ->write_wakeup() was called with
>> port lock held and ->write() tried to grab the same
>> port lock.
>
> Should this and the next patch be earlier in the series
> as a fix for the v3.15-rc cycle? Should they be cc: stable
> as well?

Well, right now the other fix has had _zero_ testing
so not really a -stable candidate just yet.

Re: [PATCH 10/11] Revert "serial: omap: unlock the port lock"

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1073 bytes --]

Hi,

On Wed, Mar 26, 2014 at 08:39:11PM -0400, Peter Hurley wrote:
> On 03/25/2014 02:28 PM, Tony Lindgren wrote:
> >* Felipe Balbi <balbi@ti.com> [140320 12:39]:
> >>This reverts commit 0324a821029e1f54e7a7f8fed48693cfce42dc0e.
> >>
> >>That commit tried to fix a deadlock problem when using
> >>hci_ldisc, but it turns out the bug was in hci_ldsic
> >>all along where it was calling ->write() from within
> >>->write_wakeup() callback.
> >>
> >>The problem is that ->write_wakeup() was called with
> >>port lock held and ->write() tried to grab the same
> >>port lock.
> >
> >Should this and the next patch be earlier in the series
> >as a fix for the v3.15-rc cycle? Should they be cc: stable
> >as well?
> 
> Well, right now the other fix has had _zero_ testing
> so not really a -stable candidate just yet.

how can you even say that ? Unless you work for some 3 letter acronym
organizations, you have no clue about the fact that this was tested on a
keystone 2 platform. How else would we have found the issue to start
with ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 10/11] Revert "serial: omap: unlock the port lock"

[Peter Hurley]

On 03/26/2014 10:10 PM, Felipe Balbi wrote:
> Hi,
>
> On Wed, Mar 26, 2014 at 08:39:11PM -0400, Peter Hurley wrote:
>> On 03/25/2014 02:28 PM, Tony Lindgren wrote:
>>> * Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> [140320 12:39]:
>>>> This reverts commit 0324a821029e1f54e7a7f8fed48693cfce42dc0e.
>>>>
>>>> That commit tried to fix a deadlock problem when using
>>>> hci_ldisc, but it turns out the bug was in hci_ldsic
>>>> all along where it was calling ->write() from within
>>>> ->write_wakeup() callback.
>>>>
>>>> The problem is that ->write_wakeup() was called with
>>>> port lock held and ->write() tried to grab the same
>>>> port lock.
>>>
>>> Should this and the next patch be earlier in the series
>>> as a fix for the v3.15-rc cycle? Should they be cc: stable
>>> as well?
>>
>> Well, right now the other fix has had _zero_ testing
>> so not really a -stable candidate just yet.
>
> how can you even say that ?

I misunderstood when you wrote:

On 03/20/2014 02:11 PM, Felipe Balbi wrote:
 > here's a build-tested only patch which is waiting for testing from other
 > colleagues who've got a platform to reproduce the problem:

and then the version I reviewed had no Tested-by: tags.

> Unless you work for some 3 letter acronym
> organizations, you have no clue about the fact that this was tested on a
> keystone 2 platform.

Ok.

> How else would we have found the issue to start with ?

Bug report?

Regards,
Peter Hurley

Re: [PATCH 10/11] Revert "serial: omap: unlock the port lock"

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1542 bytes --]

Hi,

On Wed, Mar 26, 2014 at 10:27:13PM -0400, Peter Hurley wrote:
> On 03/26/2014 10:10 PM, Felipe Balbi wrote:
> >Hi,
> >
> >On Wed, Mar 26, 2014 at 08:39:11PM -0400, Peter Hurley wrote:
> >>On 03/25/2014 02:28 PM, Tony Lindgren wrote:
> >>>* Felipe Balbi <balbi@ti.com> [140320 12:39]:
> >>>>This reverts commit 0324a821029e1f54e7a7f8fed48693cfce42dc0e.
> >>>>
> >>>>That commit tried to fix a deadlock problem when using
> >>>>hci_ldisc, but it turns out the bug was in hci_ldsic
> >>>>all along where it was calling ->write() from within
> >>>>->write_wakeup() callback.
> >>>>
> >>>>The problem is that ->write_wakeup() was called with
> >>>>port lock held and ->write() tried to grab the same
> >>>>port lock.
> >>>
> >>>Should this and the next patch be earlier in the series
> >>>as a fix for the v3.15-rc cycle? Should they be cc: stable
> >>>as well?
> >>
> >>Well, right now the other fix has had _zero_ testing
> >>so not really a -stable candidate just yet.
> >
> >how can you even say that ?
> 
> I misunderstood when you wrote:
> 
> On 03/20/2014 02:11 PM, Felipe Balbi wrote:
> > here's a build-tested only patch which is waiting for testing from other
> > colleagues who've got a platform to reproduce the problem:
> 
> and then the version I reviewed had no Tested-by: tags.

I wouldn't add that tag myself, but Murali (in Cc) did help testing
together with other colleagues.

> >How else would we have found the issue to start with ?
> 
> Bug report?

touchè :-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH 10/11] Revert "serial: omap: unlock the port lock"

[Karicheri, Muralidharan]

>-----Original Message-----
>From: Balbi, Felipe
>Sent: Wednesday, March 26, 2014 11:37 PM
>To: Peter Hurley
>Cc: Balbi, Felipe; Tony Lindgren; Greg KH; linux-serial@vger.kernel.org; linux-
>bluetooth@vger.kernel.org; Karicheri, Muralidharan; b32955@freescale.com; Linux OMAP
>Mailing List; Linux Kernel Mailing List
>Subject: Re: [PATCH 10/11] Revert "serial: omap: unlock the port lock"
>
>Hi,
>
>On Wed, Mar 26, 2014 at 10:27:13PM -0400, Peter Hurley wrote:
>> On 03/26/2014 10:10 PM, Felipe Balbi wrote:
>> >Hi,
>> >
>> >On Wed, Mar 26, 2014 at 08:39:11PM -0400, Peter Hurley wrote:
>> >>On 03/25/2014 02:28 PM, Tony Lindgren wrote:
>> >>>* Felipe Balbi <balbi@ti.com> [140320 12:39]:
>> >>>>This reverts commit 0324a821029e1f54e7a7f8fed48693cfce42dc0e.
>> >>>>
>> >>>>That commit tried to fix a deadlock problem when using hci_ldisc,
>> >>>>but it turns out the bug was in hci_ldsic all along where it was
>> >>>>calling ->write() from within
>> >>>>->write_wakeup() callback.
>> >>>>
>> >>>>The problem is that ->write_wakeup() was called with port lock
>> >>>>held and ->write() tried to grab the same port lock.
>> >>>
>> >>>Should this and the next patch be earlier in the series as a fix
>> >>>for the v3.15-rc cycle? Should they be cc: stable as well?
>> >>
>> >>Well, right now the other fix has had _zero_ testing so not really a
>> >>-stable candidate just yet.
>> >
>> >how can you even say that ?
>>
>> I misunderstood when you wrote:
>>
>> On 03/20/2014 02:11 PM, Felipe Balbi wrote:
>> > here's a build-tested only patch which is waiting for testing from
>> > other colleagues who've got a platform to reproduce the problem:
>>
>> and then the version I reviewed had no Tested-by: tags.
>
>I wouldn't add that tag myself, but Murali (in Cc) did help testing together with other

Yes. One of our customer did the test as the problem was reported by the
customer. The deadlock fix patch from Balbi (hci_ldsic) fixed the issue 
and customer confirmed that the issue is fixed.

Murali

>colleagues.
>
>> >How else would we have found the issue to start with ?
>>
>> Bug report?
>
>touchè :-)
>
>--
>balbi

[PATCH 11/11] serial: fix UART_IIR_ID

[Felipe Balbi]

UART IRQ Identification bitfield is 3
bits long (bits 3:1) but current mask only
masks 2 bits. Fix it.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 include/uapi/linux/serial_reg.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
index e632260..99b4705 100644
--- a/include/uapi/linux/serial_reg.h
+++ b/include/uapi/linux/serial_reg.h
@@ -32,7 +32,7 @@
 
 #define UART_IIR	2	/* In:  Interrupt ID Register */
 #define UART_IIR_NO_INT		0x01 /* No interrupts pending */
-#define UART_IIR_ID		0x06 /* Mask for the interrupt ID */
+#define UART_IIR_ID		0x0e /* Mask for the interrupt ID */
 #define UART_IIR_MSI		0x00 /* Modem status interrupt */
 #define UART_IIR_THRI		0x02 /* Transmitter holding register empty */
 #define UART_IIR_RDI		0x04 /* Receiver data interrupt */
-- 
1.9.1.286.g5172cb3

Re: [PATCH 01/11] tty: serial: add missing braces

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 268 bytes --]

Hi,

On Thu, Mar 20, 2014 at 02:29:57PM -0500, Felipe Balbi wrote:
> per CodingStyle we should have those braces, no
> functional changes.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>

Greg, do you want me to refresh and resend this series ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 01/11] tty: serial: add missing braces

[Greg KH]

On Tue, Apr 22, 2014 at 09:22:56AM -0500, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Mar 20, 2014 at 02:29:57PM -0500, Felipe Balbi wrote:
> > per CodingStyle we should have those braces, no
> > functional changes.
> > 
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> 
> Greg, do you want me to refresh and resend this series ?

If there are conflicts with my current tree, sure, I'd love to have
that.  I should be trying to dig out from my patch backlog for 3.16-rc1
by the end of this week...

thanks,

greg k-h

Re: [PATCH 01/11] tty: serial: add missing braces

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 705 bytes --]

On Tue, Apr 22, 2014 at 05:28:43PM -0700, Greg KH wrote:
> On Tue, Apr 22, 2014 at 09:22:56AM -0500, Felipe Balbi wrote:
> > Hi,
> > 
> > On Thu, Mar 20, 2014 at 02:29:57PM -0500, Felipe Balbi wrote:
> > > per CodingStyle we should have those braces, no
> > > functional changes.
> > > 
> > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > 
> > Greg, do you want me to refresh and resend this series ?
> 
> If there are conflicts with my current tree, sure, I'd love to have
> that.  I should be trying to dig out from my patch backlog for 3.16-rc1
> by the end of this week...

then I'll resend rebased on -rc2, should make your life a little easier,
hopefully.

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]