lkml.org 
[lkml]   [2020]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.9 354/757] serial: 8250_dw: Fix clk-notifier/port suspend deadlock
    Date
    From: Serge Semin <Sergey.Semin@baikalelectronics.ru>

    [ Upstream commit 85985a3dcd7415dd849cf62ec14f931cd905099a ]

    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/f1cd5c75-9cda-6896-a4e2-42c5bfc3f5c3@redhat.com

    Fixes: cc816969d7b5 ("serial: 8250_dw: Fix common clocks usage race condition")
    Reported-by: Hans de Goede <hdegoede@redhat.com>
    Tested-by: Hans de Goede <hdegoede@redhat.com>
    Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
    Link: https://lore.kernel.org/r/20200923161950.6237-4-Sergey.Semin@baikalelectronics.ru
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    drivers/tty/serial/8250/8250_dw.c | 54 +++++++++++--------------------
    1 file changed, 19 insertions(+), 35 deletions(-)

    diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
    index 87f450b7c1779..9e204f9b799a1 100644
    --- a/drivers/tty/serial/8250/8250_dw.c
    +++ b/drivers/tty/serial/8250/8250_dw.c
    @@ -373,39 +373,6 @@ static void dw8250_set_ldisc(struct uart_port *p, struct ktermios *termios)
    serial8250_do_set_ldisc(p, termios);
    }

    -static int dw8250_startup(struct uart_port *p)
    -{
    - struct dw8250_data *d = to_dw8250_data(p->private_data);
    - int ret;
    -
    - /*
    - * Some platforms may provide a reference clock shared between several
    - * devices. In this case before using the serial port first we have to
    - * make sure that any clock state change is known to the UART port at
    - * least post factum.
    - */
    - if (d->clk) {
    - ret = clk_notifier_register(d->clk, &d->clk_notifier);
    - if (ret)
    - dev_warn(p->dev, "Failed to set the clock notifier\n");
    - }
    -
    - return serial8250_do_startup(p);
    -}
    -
    -static void dw8250_shutdown(struct uart_port *p)
    -{
    - struct dw8250_data *d = to_dw8250_data(p->private_data);
    -
    - serial8250_do_shutdown(p);
    -
    - if (d->clk) {
    - clk_notifier_unregister(d->clk, &d->clk_notifier);
    -
    - flush_work(&d->clk_work);
    - }
    -}
    -
    /*
    * dw8250_fallback_dma_filter will prevent the UART from getting just any free
    * 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)
    p->serial_out = dw8250_serial_out;
    p->set_ldisc = dw8250_set_ldisc;
    p->set_termios = dw8250_set_termios;
    - p->startup = dw8250_startup;
    - p->shutdown = dw8250_shutdown;

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

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

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

    pm_runtime_get_sync(dev);

    + if (data->clk) {
    + clk_notifier_unregister(data->clk, &data->clk_notifier);
    +
    + flush_work(&data->clk_work);
    + }
    +
    serial8250_unregister_port(data->data.line);

    reset_control_assert(data->rst);
    --
    2.25.1


    \
     
     \ /
      Last update: 2020-10-27 17:10    [W:2.698 / U:0.108 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site