Skip to content

Commit 85985a3

Browse files
fancergregkh
authored andcommitted
serial: 8250_dw: Fix clk-notifier/port suspend deadlock
It has been discovered that there is a potential deadlock between the clock-change-notifier thread and the UART port suspending one: CPU0 (suspend CPU/UART) CPU1 (update clock) ---- ---- lock(&port->mutex); lock((work_completion)(&data->clk_work)); lock(&port->mutex); lock((work_completion)(&data->clk_work)); *** DEADLOCK *** The best way to fix this is to eliminate the CPU0 port->mutex/work-completion scenario. So we suggest to register and unregister the clock-notifier during the DW APB UART port probe/remove procedures, instead of doing that at the points of the port startup/shutdown. Link: https://lore.kernel.org/linux-serial/[email protected] Fixes: cc81696 ("serial: 8250_dw: Fix common clocks usage race condition") Reported-by: Hans de Goede <[email protected]> Tested-by: Hans de Goede <[email protected]> Signed-off-by: Serge Semin <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent c8dff3a commit 85985a3

File tree

1 file changed

+19
-35
lines changed

1 file changed

+19
-35
lines changed

drivers/tty/serial/8250/8250_dw.c

Lines changed: 19 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -373,39 +373,6 @@ static void dw8250_set_ldisc(struct uart_port *p, struct ktermios *termios)
373373
serial8250_do_set_ldisc(p, termios);
374374
}
375375

376-
static int dw8250_startup(struct uart_port *p)
377-
{
378-
struct dw8250_data *d = to_dw8250_data(p->private_data);
379-
int ret;
380-
381-
/*
382-
* Some platforms may provide a reference clock shared between several
383-
* devices. In this case before using the serial port first we have to
384-
* make sure that any clock state change is known to the UART port at
385-
* least post factum.
386-
*/
387-
if (d->clk) {
388-
ret = clk_notifier_register(d->clk, &d->clk_notifier);
389-
if (ret)
390-
dev_warn(p->dev, "Failed to set the clock notifier\n");
391-
}
392-
393-
return serial8250_do_startup(p);
394-
}
395-
396-
static void dw8250_shutdown(struct uart_port *p)
397-
{
398-
struct dw8250_data *d = to_dw8250_data(p->private_data);
399-
400-
serial8250_do_shutdown(p);
401-
402-
if (d->clk) {
403-
clk_notifier_unregister(d->clk, &d->clk_notifier);
404-
405-
flush_work(&d->clk_work);
406-
}
407-
}
408-
409376
/*
410377
* dw8250_fallback_dma_filter will prevent the UART from getting just any free
411378
* channel on platforms that have DMA engines, but don't have any channels
@@ -501,8 +468,6 @@ static int dw8250_probe(struct platform_device *pdev)
501468
p->serial_out = dw8250_serial_out;
502469
p->set_ldisc = dw8250_set_ldisc;
503470
p->set_termios = dw8250_set_termios;
504-
p->startup = dw8250_startup;
505-
p->shutdown = dw8250_shutdown;
506471

507472
p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
508473
if (!p->membase)
@@ -622,6 +587,19 @@ static int dw8250_probe(struct platform_device *pdev)
622587
goto err_reset;
623588
}
624589

590+
/*
591+
* Some platforms may provide a reference clock shared between several
592+
* devices. In this case any clock state change must be known to the
593+
* UART port at least post factum.
594+
*/
595+
if (data->clk) {
596+
err = clk_notifier_register(data->clk, &data->clk_notifier);
597+
if (err)
598+
dev_warn(p->dev, "Failed to set the clock notifier\n");
599+
else
600+
queue_work(system_unbound_wq, &data->clk_work);
601+
}
602+
625603
platform_set_drvdata(pdev, data);
626604

627605
pm_runtime_set_active(dev);
@@ -648,6 +626,12 @@ static int dw8250_remove(struct platform_device *pdev)
648626

649627
pm_runtime_get_sync(dev);
650628

629+
if (data->clk) {
630+
clk_notifier_unregister(data->clk, &data->clk_notifier);
631+
632+
flush_work(&data->clk_work);
633+
}
634+
651635
serial8250_unregister_port(data->data.line);
652636

653637
reset_control_assert(data->rst);

0 commit comments

Comments
 (0)