| From ff1cab374ad98f4b9f408525ca9c08992b4ed784 Mon Sep 17 00:00:00 2001 |
| From: Geert Uytterhoeven <geert+renesas@glider.be> |
| Date: Tue, 5 Jan 2016 19:36:37 +0100 |
| Subject: serial: sh-sci: Remove cpufreq notifier to fix crash/deadlock |
| |
| From: Geert Uytterhoeven <geert+renesas@glider.be> |
| |
| commit ff1cab374ad98f4b9f408525ca9c08992b4ed784 upstream. |
| |
| 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 stored |
| clock rates; 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> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| |
| |
| --- |
| drivers/tty/serial/sh-sci.c | 39 --------------------------------------- |
| 1 file changed, 39 deletions(-) |
| |
| --- 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> |
| @@ -118,8 +117,6 @@ struct sci_port { |
| struct timer_list rx_timer; |
| unsigned int rx_timeout; |
| #endif |
| - |
| - struct notifier_block freq_transition; |
| }; |
| |
| /* Function prototypes */ |
| @@ -1029,30 +1026,6 @@ static irqreturn_t sci_mpxed_interrupt(i |
| 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) || |
| - (phase == CPUFREQ_RESUMECHANGE)) { |
| - 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 struct sci_irq_desc { |
| const char *desc; |
| irq_handler_t handler; |
| @@ -2406,9 +2379,6 @@ static int sci_remove(struct platform_de |
| { |
| 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); |
| @@ -2559,15 +2529,6 @@ static int sci_probe(struct platform_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)) { |
| - sci_cleanup_single(sp); |
| - return ret; |
| - } |
| - |
| #ifdef CONFIG_SH_STANDARD_BIOS |
| sh_bios_gdb_detach(); |
| #endif |