Commit cf9c3753 authored by Hugo Villeneuve's avatar Hugo Villeneuve Committed by Greg Kroah-Hartman
Browse files

serial: sc16is7xx: split into core and I2C/SPI parts (sc16is7xx_lines) 

Before, sc16is7xx_lines was checked for a free (zero) bit first, and then
later it was set only if UART port registration succeeded. Now that
sc16is7xx_lines is shared for the I2C and SPI drivers, there is a
possibility that the two drivers can simultaneously try to reserve the same
line bit at the same time.

To prevent this, make sure line allocation is reserved atomically, and use
a new variable to hold the status of UART port registration.

Now that we no longer need to search if a bit is set, it is now possible
to simplify sc16is7xx_lines allocation by using the IDA framework.

Link: https://lore.kernel.org/all/20231212150302.a9ec5d085a4ba65e89ca41af@hugovil.com/
Link: https://lore.kernel.org/all/CAHp75VebCZckUrNraYQj9k=Mrn2kbYs1Lx26f5-8rKJ3RXeh-w@mail.gmail.com/


Suggested-by: default avatarAndy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: default avatarHugo Villeneuve <hvilleneuve@dimonoff.com>
Link: https://lore.kernel.org/r/20240409154253.3043822-5-hugo@hugovil.com


Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent d4921643

drivers/tty/serial/sc16is7xx.c

+21 −11
Original line number Diff line number Diff line
@@ -10,12 +10,12 @@ 
#undef DEFAULT_SYMBOL_NAMESPACE

#define DEFAULT_SYMBOL_NAMESPACE SERIAL_NXP_SC16IS7XX



#include <linux/bitops.h>

#include <linux/clk.h>

#include <linux/delay.h>

#include <linux/device.h>

#include <linux/export.h>

#include <linux/gpio/driver.h>

#include <linux/idr.h>

#include <linux/kthread.h>

#include <linux/mod_devicetable.h>

#include <linux/module.h>
@@ -345,7 +345,7 @@ struct sc16is7xx_port { 
	struct sc16is7xx_one		p[];

};



static DECLARE_BITMAP(sc16is7xx_lines, SC16IS7XX_MAX_DEVS);

static DEFINE_IDA(sc16is7xx_lines);



static struct uart_driver sc16is7xx_uart = {

	.owner		= THIS_MODULE,
@@ -1462,6 +1462,7 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype, 
	u32 uartclk = 0;

	int i, ret;

	struct sc16is7xx_port *s;

	bool port_registered[SC16IS7XX_MAX_PORTS];



	for (i = 0; i < devtype->nr_uart; i++)

		if (IS_ERR(regmaps[i]))
@@ -1526,14 +1527,20 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype, 
	regmap_write(regmaps[0], SC16IS7XX_IOCONTROL_REG,

		     SC16IS7XX_IOCONTROL_SRESET_BIT);



	/* Mark each port line and status as uninitialised. */

	for (i = 0; i < devtype->nr_uart; ++i) {

		s->p[i].port.line = find_first_zero_bit(sc16is7xx_lines,

							SC16IS7XX_MAX_DEVS);

		if (s->p[i].port.line >= SC16IS7XX_MAX_DEVS) {

			ret = -ERANGE;

			goto out_ports;

		s->p[i].port.line = SC16IS7XX_MAX_DEVS;

		port_registered[i] = false;

	}



	for (i = 0; i < devtype->nr_uart; ++i) {

		ret = ida_alloc_max(&sc16is7xx_lines,

				    SC16IS7XX_MAX_DEVS - 1, GFP_KERNEL);

		if (ret < 0)

			goto out_ports;



		s->p[i].port.line = ret;



		/* Initialize port data */

		s->p[i].port.dev	= dev;

		s->p[i].port.irq	= irq;
@@ -1578,7 +1585,7 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype, 
		if (ret)

			goto out_ports;



		set_bit(s->p[i].port.line, sc16is7xx_lines);

		port_registered[i] = true;



		/* Enable EFR */

		sc16is7xx_port_write(&s->p[i].port, SC16IS7XX_LCR_REG,
@@ -1636,9 +1643,12 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype, 
#endif



out_ports:

	for (i = 0; i < devtype->nr_uart; i++)

		if (test_and_clear_bit(s->p[i].port.line, sc16is7xx_lines))

	for (i = 0; i < devtype->nr_uart; i++) {

		if (s->p[i].port.line < SC16IS7XX_MAX_DEVS)

			ida_free(&sc16is7xx_lines, s->p[i].port.line);

		if (port_registered[i])

			uart_remove_one_port(&sc16is7xx_uart, &s->p[i].port);

	}



	kthread_stop(s->kworker_task);
@@ -1661,7 +1671,7 @@ void sc16is7xx_remove(struct device *dev) 


	for (i = 0; i < s->devtype->nr_uart; i++) {

		kthread_cancel_delayed_work_sync(&s->p[i].ms_work);

		clear_bit(s->p[i].port.line, sc16is7xx_lines);

		ida_free(&sc16is7xx_lines, s->p[i].port.line);

		uart_remove_one_port(&sc16is7xx_uart, &s->p[i].port);

		sc16is7xx_power(&s->p[i].port, 0);

	}