Linux-Serial Archive on lore.kernel.org
 help / color / Atom feed
  • * [PATCH v2] serial: 8250_dw: Fix common clocks usage race condition
           [not found] <20200306130231.05BBC8030795@mail.baikalelectronics.ru>
           [not found] ` <20200306134049.86F8180307C2@mail.baikalelectronics.ru>
    @ 2020-03-23  2:46 ` Sergey.Semin
      2020-03-23  9:20   ` Andy Shevchenko
                         ` (2 more replies)
      1 sibling, 3 replies; 29+ messages in thread
    From: Sergey.Semin @ 2020-03-23  2:46 UTC (permalink / raw)
      To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby
      Cc: Serge Semin, Serge Semin, Alexey Malahov, Maxim Kaurkin,
    	Pavel Parkhomenko, Ramil Zaripov, Ekaterina Skachko,
    	Vadim Vlasov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
    	Maxime Ripard, Chen-Yu Tsai, Ray Jui, Scott Branden,
    	Florian Fainelli, Wei Xu, Jason Cooper, Andrew Lunn,
    	Gregory Clement, Sebastian Hesselbarth, Jisheng Zhang,
    	Heiko Stuebner, Catalin Marinas, Will Deacon, Russell King,
    	linux-arm-kernel, Michael Turquette, Stephen Boyd, linux-clk,
    	Heikki Krogerus, Kefeng Wang, linux-serial, linux-kernel
    
    From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
    
    There are races possible in the dw8250_set_termios() callback method
    and while the device is in PM suspend state. A race condition may
    happen if the baudrate clock source device is shared with some other
    device (in our machine it's another DW UART port). In this case if that
    device changes the clock rate while serial console is using it the
    DW 8250 UART port might not only end up with an invalid uartclk value
    saved, but may also experience a distorted output data since baud-clock
    could have been changed. In order to fix this lets enable an exclusive
    reference clock rate access in case if "baudclk" device is specified.
    
    So if some other device also acquires the rate exclusivity during the
    time of a DW UART 8250 port being opened, then DW UART 8250 driver
    won't be able to alter the baud-clock. It shall just use the available
    clock rate. Similarly another device also won't manage to change the
    rate at that time. If nothing else have the exclusive rate access
    acquired except DW UART 8250 driver, then the driver will be able to
    alter the rate as much as it needs to in accordance with the currently
    implemented logic.
    
    Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
    Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
    Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
    Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
    Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
    Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
    Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
    Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
    Cc: Paul Burton <paulburton@kernel.org>
    Cc: Ralf Baechle <ralf@linux-mips.org>
    Cc: Maxime Ripard <mripard@kernel.org>
    Cc: Chen-Yu Tsai <wens@csie.org>
    CC: Ray Jui <rjui@broadcom.com>
    Cc: Scott Branden <sbranden@broadcom.com>
    Cc: Florian Fainelli <f.fainelli@gmail.com>
    Cc: Wei Xu <xuwei5@hisilicon.com>
    Cc: Jason Cooper <jason@lakedaemon.net>
    Cc: Andrew Lunn <andrew@lunn.ch>
    Cc: Gregory Clement <gregory.clement@bootlin.com>
    Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
    Cc: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
    Cc: Heiko Stuebner <heiko@sntech.de>
    Cc: Catalin Marinas <catalin.marinas@arm.com>
    Cc: Will Deacon <will@kernel.org>
    Cc: Russell King <linux@armlinux.org.uk>
    Cc: linux-arm-kernel@lists.infradead.org
    Cc: Michael Turquette <mturquette@baylibre.com>
    Cc: Stephen Boyd <sboyd@kernel.org>
    Cc: linux-clk@vger.kernel.org
    
    ---
    
    Changelog v2:
    - Move exclusive ref clock lock/unlock precudures to the 8250 port
      startup/shutdown methods.
    - The changelog message has also been slightly modified due to the
      alteration.
    - Remove Alexey' SoB tag.
    - Cc someone from ARM who might be concerned regarding this change.
    - Cc someone from Clocks Framework to get their comments on this patch.
    ---
     drivers/tty/serial/8250/8250_dw.c | 36 +++++++++++++++++++++++++++++++
     1 file changed, 36 insertions(+)
    
    diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
    index aab3cccc6789..08f3f745ed54 100644
    --- a/drivers/tty/serial/8250/8250_dw.c
    +++ b/drivers/tty/serial/8250/8250_dw.c
    @@ -319,6 +319,40 @@ 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);
    +
    +	/*
    +	 * 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 nothing will change the rate behind our back and second
    +	 * the tty/serial subsystem knows the actual reference clock rate of
    +	 * the port.
    +	 */
    +	if (clk_rate_exclusive_get(d->clk)) {
    +		dev_warn(p->dev, "Couldn't lock the clock rate\n");
    +	} else if (d->clk) {
    +		p->uartclk = clk_get_rate(d->clk);
    +		if (!p->uartclk) {
    +			clk_rate_exclusive_put(d->clk);
    +			dev_err(p->dev, "Clock rate not defined\n");
    +			return -EINVAL;
    +		}
    +	}
    +
    +	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);
    +
    +	clk_rate_exclusive_put(d->clk);
    +}
    +
     /*
      * 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
    @@ -414,6 +448,8 @@ 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)
    -- 
    2.25.1
    
    
    ^ permalink raw reply	[flat|nested] 29+ messages in thread

  • end of thread, back to index
    
    Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
    -- links below jump to the message on this page --
         [not found] <20200306130231.05BBC8030795@mail.baikalelectronics.ru>
         [not found] ` <20200306134049.86F8180307C2@mail.baikalelectronics.ru>
         [not found]   ` <20200310001444.B0F50803087C@mail.baikalelectronics.ru>
    2020-03-10 14:17     ` [PATCH] serial: 8250_dw: Fix common clocks usage race condition Andy Shevchenko
         [not found]     ` <20200310141715.7063280307CA@mail.baikalelectronics.ru>
    2020-03-12 18:47       ` Sergey Semin
    2020-03-18 15:19         ` Andy Shevchenko
    2020-03-23  2:46 ` [PATCH v2] " Sergey.Semin
    2020-03-23  9:20   ` Andy Shevchenko
    2020-03-23 11:11     ` Sergey Semin
    2020-03-23 11:52       ` Andy Shevchenko
    2020-03-23 17:07         ` Sergey Semin
    2020-03-23 10:01   ` Maxime Ripard
    2020-03-23 13:50     ` Sergey Semin
    2020-03-24  9:41       ` Maxime Ripard
    2020-03-24 10:12       ` Andy Shevchenko
    2020-03-25 17:11         ` Sergey Semin
    2020-03-26  1:44           ` Stephen Boyd
    2020-03-27  9:12             ` Sergey Semin
    2020-05-06 23:31   ` [PATCH v3 0/4] serial: 8250_dw: Fix ref clock usage Serge Semin
    2020-05-06 23:31     ` [PATCH v3 1/4] serial: 8250: Fix max baud limit in generic 8250 port Serge Semin
    2020-05-15 13:48       ` Andy Shevchenko
    2020-05-06 23:31     ` [PATCH v3 2/4] serial: 8250: Add 8250 port clock update method Serge Semin
    2020-05-15 12:45       ` Greg Kroah-Hartman
    2020-05-15 14:32         ` Serge Semin
    2020-05-06 23:31     ` [PATCH v3 3/4] serial: 8250_dw: Simplify the ref clock rate setting procedure Serge Semin
    2020-05-15 14:05       ` Andy Shevchenko
    2020-05-15 14:50         ` Serge Semin
    2020-05-15 15:05           ` Andy Shevchenko
    2020-05-06 23:31     ` [PATCH v3 4/4] serial: 8250_dw: Fix common clocks usage race condition Serge Semin
    2020-05-15 14:10       ` Andy Shevchenko
    2020-05-15 15:19         ` Serge Semin
    2020-05-07 18:29     ` [PATCH v3 0/4] serial: 8250_dw: Fix ref clock usage Andy Shevchenko
    

    Linux-Serial Archive on lore.kernel.org
    
    Archives are clonable:
    	git clone --mirror https://lore.kernel.org/linux-serial/0 linux-serial/git/0.git
    
    	# If you have public-inbox 1.1+ installed, you may
    	# initialize and index your mirror using the following commands:
    	public-inbox-init -V2 linux-serial linux-serial/ https://lore.kernel.org/linux-serial \
    		linux-serial@vger.kernel.org
    	public-inbox-index linux-serial
    
    Example config snippet for mirrors
    
    Newsgroup available over NNTP:
    	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-serial
    
    
    AGPL code for this site: git clone https://public-inbox.org/public-inbox.git