lkml.org 
[lkml]   [2016]   [Jan]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 1a/1] serial: sh-sci: Remove cpufreq notifier to fix crash/deadlock
    Date
    The BSP team noticed that there is spin/mutex lock issue on sh-sci when
    CPUFREQ is used. The issue is that the notifier function may call
    mutex_lock() while the spinlock is held, which can lead to a BUG().
    This may happen if CPUFREQ is changed while another CPU calls
    clk_get_rate().

    Taking the spinlock was added to the notifier function in commit
    e552de2413edad1a ("sh-sci: add platform device private data"), to
    protect the list of serial ports against modification during traversal.
    At that time the Common Clock Framework didn't exist yet, and
    clk_get_rate() just returned clk->rate without taking a mutex.
    Note that since commit d535a2305facf9b4 ("serial: sh-sci: Require a
    device per port mapping."), there's no longer a list of serial ports to
    traverse, and taking the spinlock became superfluous.

    To fix the issue, just remove the cpufreq notifier:
    1. The notifier doesn't work correctly: all it does is update the
    stored clock rate; it does not update the divider in the hardware.
    The divider will only be updated when calling sci_set_termios().
    I believe this was broken back in 2004, when the old
    drivers/char/sh-sci.c driver (where the notifier did update the
    divider) was replaced by drivers/serial/sh-sci.c (where the
    notifier just updated port->uartclk).
    Cfr. full-history-linux commits 6f8deaef2e9675d9 ("[PATCH] sh: port
    sh-sci driver to the new API") and 3f73fe878dc9210a ("[PATCH]
    Remove old sh-sci driver").
    2. On modern SoCs, the sh-sci parent clock rate is no longer related
    to the CPU clock rate anyway, so using a cpufreq notifier is
    futile.

    Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
    ---
    This version applies against tty/tty-next and v4.4-rc1..v4.4-rc8.
    ---
    drivers/tty/serial/sh-sci.c | 39 ---------------------------------------
    1 file changed, 39 deletions(-)

    diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
    index 51c7507b0444957b..63a06ab6ba038ad5 100644
    --- a/drivers/tty/serial/sh-sci.c
    +++ b/drivers/tty/serial/sh-sci.c
    @@ -38,7 +38,6 @@
    #include <linux/major.h>
    #include <linux/module.h>
    #include <linux/mm.h>
    -#include <linux/notifier.h>
    #include <linux/of.h>
    #include <linux/platform_device.h>
    #include <linux/pm_runtime.h>
    @@ -116,8 +115,6 @@ struct sci_port {
    struct timer_list rx_timer;
    unsigned int rx_timeout;
    #endif
    -
    - struct notifier_block freq_transition;
    };

    #define SCI_NPORTS CONFIG_SERIAL_SH_SCI_NR_UARTS
    @@ -1606,29 +1603,6 @@ static irqreturn_t sci_mpxed_interrupt(int irq, void *ptr)
    return ret;
    }

    -/*
    - * Here we define a transition notifier so that we can update all of our
    - * ports' baud rate when the peripheral clock changes.
    - */
    -static int sci_notifier(struct notifier_block *self,
    - unsigned long phase, void *p)
    -{
    - struct sci_port *sci_port;
    - unsigned long flags;
    -
    - sci_port = container_of(self, struct sci_port, freq_transition);
    -
    - if (phase == CPUFREQ_POSTCHANGE) {
    - struct uart_port *port = &sci_port->port;
    -
    - spin_lock_irqsave(&port->lock, flags);
    - port->uartclk = clk_get_rate(sci_port->iclk);
    - spin_unlock_irqrestore(&port->lock, flags);
    - }
    -
    - return NOTIFY_OK;
    -}
    -
    static const struct sci_irq_desc {
    const char *desc;
    irq_handler_t handler;
    @@ -2559,9 +2533,6 @@ static int sci_remove(struct platform_device *dev)
    {
    struct sci_port *port = platform_get_drvdata(dev);

    - cpufreq_unregister_notifier(&port->freq_transition,
    - CPUFREQ_TRANSITION_NOTIFIER);
    -
    uart_remove_one_port(&sci_uart_driver, &port->port);

    sci_cleanup_single(port);
    @@ -2714,16 +2685,6 @@ static int sci_probe(struct platform_device *dev)
    if (ret)
    return ret;

    - sp->freq_transition.notifier_call = sci_notifier;
    -
    - ret = cpufreq_register_notifier(&sp->freq_transition,
    - CPUFREQ_TRANSITION_NOTIFIER);
    - if (unlikely(ret < 0)) {
    - uart_remove_one_port(&sci_uart_driver, &sp->port);
    - sci_cleanup_single(sp);
    - return ret;
    - }
    -
    #ifdef CONFIG_SH_STANDARD_BIOS
    sh_bios_gdb_detach();
    #endif
    --
    1.9.1


    \
     
     \ /
      Last update: 2016-01-05 20:21    [W:4.309 / U:0.568 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site