[stable-kernel][5.15.y][PATCH 0/5] Fix a regression on sc16is7xx

[stable-kernel][5.15.y][PATCH 0/5] Fix a regression on sc16is7xx

[Hui Wang]

Recently we found the fifo_read() and fifo_write() are broken in our
5.15 kernel after rebase to the latest 5.15.y, the 5.15.y integrated
the commit e635f652696e ("serial: sc16is7xx: convert from _raw_ to
_noinc_ regmap functions for FIFO"), but it forgot to integrate a
prerequisite commit 3837a0379533 ("serial: sc16is7xx: improve regmap
debugfs by using one regmap per port").

And about the prerequisite commit, there are also 4 commits to fix it,
So in total, I backported 5 patches to 5.15.y to fix this regression.

0002-xxx and 0004-xxx could be cleanly applied to 5.15.y, the remaining
3 patches need to resolve some conflict.

Hugo Villeneuve (5):
  serial: sc16is7xx: improve regmap debugfs by using one regmap per port
  serial: sc16is7xx: remove wasteful static buffer in
    sc16is7xx_regmap_name()
  serial: sc16is7xx: remove global regmap from struct sc16is7xx_port
  serial: sc16is7xx: remove unused line structure member
  serial: sc16is7xx: change EFR lock to operate on each channels

 drivers/tty/serial/sc16is7xx.c | 185 +++++++++++++++++++--------------
 1 file changed, 107 insertions(+), 78 deletions(-)

-- 
2.34.1

[stable-kernel][5.15.y][PATCH 1/5] serial: sc16is7xx: improve regmap debugfs by using one regmap per port

[Hui Wang]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

commit 3837a0379533aabb9e4483677077479f7c6aa910 upstream.

With this current driver regmap implementation, it is hard to make sense
of the register addresses displayed using the regmap debugfs interface,
because they do not correspond to the actual register addresses documented
in the datasheet. For example, register 1 is displayed as registers 04 thru
07:

$ cat /sys/kernel/debug/regmap/spi0.0/registers
  04: 10 -> Port 0, register offset 1
  05: 10 -> Port 1, register offset 1
  06: 00 -> Port 2, register offset 1 -> invalid
  07: 00 -> port 3, register offset 1 -> invalid
  ...

The reason is that bits 0 and 1 of the register address correspond to the
channel (port) bits, so the register address itself starts at bit 2, and we
must 'mentally' shift each register address by 2 bits to get its real
address/offset.

Also, only channels 0 and 1 are supported by the chip, so channel mask
combinations of 10b and 11b are invalid, and the display of these
registers is useless.

This patch adds a separate regmap configuration for each port, similar to
what is done in the max310x driver, so that register addresses displayed
match the register addresses in the chip datasheet. Also, each port now has
its own debugfs entry.

Example with new regmap implementation:

$ cat /sys/kernel/debug/regmap/spi0.0-port0/registers
1: 10
2: 01
3: 00
...

$ cat /sys/kernel/debug/regmap/spi0.0-port1/registers
1: 10
2: 01
3: 00

As an added bonus, this also simplifies some operations (read/write/modify)
because it is no longer necessary to manually shift register addresses.

[Hui: Fixed some conflict when backporting to 5.15.y]

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Link: https://lore.kernel.org/r/20231030211447.974779-1-hugo@hugovil.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 drivers/tty/serial/sc16is7xx.c | 136 +++++++++++++++++++--------------
 1 file changed, 79 insertions(+), 57 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index d274a847c6ab..88adbd9d5002 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -292,8 +292,9 @@
 						  */
 
 /* Misc definitions */
+#define SC16IS7XX_SPI_READ_BIT		BIT(7)
 #define SC16IS7XX_FIFO_SIZE		(64)
-#define SC16IS7XX_REG_SHIFT		2
+#define SC16IS7XX_GPIOS_PER_BANK	4
 
 struct sc16is7xx_devtype {
 	char	name[10];
@@ -313,6 +314,7 @@ struct sc16is7xx_one_config {
 struct sc16is7xx_one {
 	struct uart_port		port;
 	u8				line;
+	struct regmap			*regmap;
 	struct kthread_work		tx_work;
 	struct kthread_work		reg_work;
 	struct sc16is7xx_one_config	config;
@@ -344,46 +346,35 @@ static struct uart_driver sc16is7xx_uart = {
 #define to_sc16is7xx_port(p,e)	((container_of((p), struct sc16is7xx_port, e)))
 #define to_sc16is7xx_one(p,e)	((container_of((p), struct sc16is7xx_one, e)))
 
-static int sc16is7xx_line(struct uart_port *port)
-{
-	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
-
-	return one->line;
-}
-
 static u8 sc16is7xx_port_read(struct uart_port *port, u8 reg)
 {
-	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
+	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
 	unsigned int val = 0;
-	const u8 line = sc16is7xx_line(port);
 
-	regmap_read(s->regmap, (reg << SC16IS7XX_REG_SHIFT) | line, &val);
+	regmap_read(one->regmap, reg, &val);
 
 	return val;
 }
 
 static void sc16is7xx_port_write(struct uart_port *port, u8 reg, u8 val)
 {
-	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
-	const u8 line = sc16is7xx_line(port);
+	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
 
-	regmap_write(s->regmap, (reg << SC16IS7XX_REG_SHIFT) | line, val);
+	regmap_write(one->regmap, reg, val);
 }
 
 static void sc16is7xx_fifo_read(struct uart_port *port, unsigned int rxlen)
 {
 	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
-	const u8 line = sc16is7xx_line(port);
-	u8 addr = (SC16IS7XX_RHR_REG << SC16IS7XX_REG_SHIFT) | line;
+	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
 
-	regmap_noinc_read(s->regmap, addr, s->buf, rxlen);
+	regmap_noinc_read(one->regmap, SC16IS7XX_RHR_REG, s->buf, rxlen);
 }
 
 static void sc16is7xx_fifo_write(struct uart_port *port, u8 to_send)
 {
 	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
-	const u8 line = sc16is7xx_line(port);
-	u8 addr = (SC16IS7XX_THR_REG << SC16IS7XX_REG_SHIFT) | line;
+	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
 
 	/*
 	 * Don't send zero-length data, at least on SPI it confuses the chip
@@ -392,17 +383,15 @@ static void sc16is7xx_fifo_write(struct uart_port *port, u8 to_send)
 	if (unlikely(!to_send))
 		return;
 
-	regmap_noinc_write(s->regmap, addr, s->buf, to_send);
+	regmap_noinc_write(one->regmap, SC16IS7XX_THR_REG, s->buf, to_send);
 }
 
 static void sc16is7xx_port_update(struct uart_port *port, u8 reg,
 				  u8 mask, u8 val)
 {
-	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
-	const u8 line = sc16is7xx_line(port);
+	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
 
-	regmap_update_bits(s->regmap, (reg << SC16IS7XX_REG_SHIFT) | line,
-			   mask, val);
+	regmap_update_bits(one->regmap, reg, mask, val);
 }
 
 static int sc16is7xx_alloc_line(void)
@@ -457,7 +446,7 @@ static const struct sc16is7xx_devtype sc16is762_devtype = {
 
 static bool sc16is7xx_regmap_volatile(struct device *dev, unsigned int reg)
 {
-	switch (reg >> SC16IS7XX_REG_SHIFT) {
+	switch (reg) {
 	case SC16IS7XX_RHR_REG:
 	case SC16IS7XX_IIR_REG:
 	case SC16IS7XX_LSR_REG:
@@ -475,7 +464,7 @@ static bool sc16is7xx_regmap_volatile(struct device *dev, unsigned int reg)
 
 static bool sc16is7xx_regmap_precious(struct device *dev, unsigned int reg)
 {
-	switch (reg >> SC16IS7XX_REG_SHIFT) {
+	switch (reg) {
 	case SC16IS7XX_RHR_REG:
 		return true;
 	default:
@@ -505,6 +494,7 @@ static bool sc16is7xx_regmap_noinc(struct device *dev, unsigned int reg)
 static int sc16is7xx_set_baud(struct uart_port *port, int baud)
 {
 	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
+	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
 	u8 lcr;
 	unsigned int prescaler = 1;
 	unsigned long clk = port->uartclk, div = clk / 16 / baud;
@@ -536,10 +526,10 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud)
 			     SC16IS7XX_LCR_CONF_MODE_B);
 
 	/* Enable enhanced features */
-	regcache_cache_bypass(s->regmap, true);
+	regcache_cache_bypass(one->regmap, true);
 	sc16is7xx_port_write(port, SC16IS7XX_EFR_REG,
 			     SC16IS7XX_EFR_ENABLE_BIT);
-	regcache_cache_bypass(s->regmap, false);
+	regcache_cache_bypass(one->regmap, false);
 
 	/* Put LCR back to the normal mode */
 	sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, lcr);
@@ -556,10 +546,10 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud)
 			     SC16IS7XX_LCR_CONF_MODE_A);
 
 	/* Write the new divisor */
-	regcache_cache_bypass(s->regmap, true);
+	regcache_cache_bypass(one->regmap, true);
 	sc16is7xx_port_write(port, SC16IS7XX_DLH_REG, div / 256);
 	sc16is7xx_port_write(port, SC16IS7XX_DLL_REG, div % 256);
-	regcache_cache_bypass(s->regmap, false);
+	regcache_cache_bypass(one->regmap, false);
 
 	/* Put LCR back to the normal mode */
 	sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, lcr);
@@ -891,6 +881,7 @@ static void sc16is7xx_set_termios(struct uart_port *port,
 				  struct ktermios *old)
 {
 	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
+	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
 	unsigned int lcr, flow = 0;
 	int baud;
 
@@ -951,7 +942,7 @@ static void sc16is7xx_set_termios(struct uart_port *port,
 			     SC16IS7XX_LCR_CONF_MODE_B);
 
 	/* Configure flow control */
-	regcache_cache_bypass(s->regmap, true);
+	regcache_cache_bypass(one->regmap, true);
 	sc16is7xx_port_write(port, SC16IS7XX_XON1_REG, termios->c_cc[VSTART]);
 	sc16is7xx_port_write(port, SC16IS7XX_XOFF1_REG, termios->c_cc[VSTOP]);
 	if (termios->c_cflag & CRTSCTS)
@@ -963,7 +954,7 @@ static void sc16is7xx_set_termios(struct uart_port *port,
 		flow |= SC16IS7XX_EFR_SWFLOW1_BIT;
 
 	sc16is7xx_port_write(port, SC16IS7XX_EFR_REG, flow);
-	regcache_cache_bypass(s->regmap, false);
+	regcache_cache_bypass(one->regmap, false);
 
 	/* Update LCR register */
 	sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, lcr);
@@ -1018,7 +1009,6 @@ static int sc16is7xx_config_rs485(struct uart_port *port,
 static int sc16is7xx_startup(struct uart_port *port)
 {
 	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
-	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
 	unsigned int val;
 
 	sc16is7xx_power(port, 1);
@@ -1034,7 +1024,7 @@ static int sc16is7xx_startup(struct uart_port *port)
 	sc16is7xx_port_write(port, SC16IS7XX_LCR_REG,
 			     SC16IS7XX_LCR_CONF_MODE_B);
 
-	regcache_cache_bypass(s->regmap, true);
+	regcache_cache_bypass(one->regmap, true);
 
 	/* Enable write access to enhanced features and internal clock div */
 	sc16is7xx_port_write(port, SC16IS7XX_EFR_REG,
@@ -1051,7 +1041,7 @@ static int sc16is7xx_startup(struct uart_port *port)
 			     SC16IS7XX_TCR_RX_RESUME(24) |
 			     SC16IS7XX_TCR_RX_HALT(48));
 
-	regcache_cache_bypass(s->regmap, false);
+	regcache_cache_bypass(one->regmap, false);
 
 	/* Now, initialize the UART */
 	sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, SC16IS7XX_LCR_WORD_LEN_8);
@@ -1216,7 +1206,7 @@ static int sc16is7xx_gpio_direction_output(struct gpio_chip *chip,
 
 static int sc16is7xx_probe(struct device *dev,
 			   const struct sc16is7xx_devtype *devtype,
-			   struct regmap *regmap, int irq)
+			   struct regmap *regmaps[], int irq)
 {
 	unsigned long freq = 0, *pfreq = dev_get_platdata(dev);
 	unsigned int val;
@@ -1224,16 +1214,16 @@ static int sc16is7xx_probe(struct device *dev,
 	int i, ret;
 	struct sc16is7xx_port *s;
 
-	if (IS_ERR(regmap))
-		return PTR_ERR(regmap);
+	for (i = 0; i < devtype->nr_uart; i++)
+		if (IS_ERR(regmaps[i]))
+			return PTR_ERR(regmaps[i]);
 
 	/*
 	 * This device does not have an identification register that would
 	 * tell us if we are really connected to the correct device.
 	 * The best we can do is to check if communication is at all possible.
 	 */
-	ret = regmap_read(regmap,
-			  SC16IS7XX_LSR_REG << SC16IS7XX_REG_SHIFT, &val);
+	ret = regmap_read(regmaps[0], SC16IS7XX_LSR_REG, &val);
 	if (ret < 0)
 		return -EPROBE_DEFER;
 
@@ -1267,7 +1257,7 @@ static int sc16is7xx_probe(struct device *dev,
 			return -EINVAL;
 	}
 
-	s->regmap = regmap;
+	s->regmap = regmaps[0];
 	s->devtype = devtype;
 	dev_set_drvdata(dev, s);
 	mutex_init(&s->efr_lock);
@@ -1282,8 +1272,8 @@ static int sc16is7xx_probe(struct device *dev,
 	sched_set_fifo(s->kworker_task);
 
 	/* reset device, purging any pending irq / data */
-	regmap_write(s->regmap, SC16IS7XX_IOCONTROL_REG << SC16IS7XX_REG_SHIFT,
-			SC16IS7XX_IOCONTROL_SRESET_BIT);
+	regmap_write(s->regmap, SC16IS7XX_IOCONTROL_REG,
+		     SC16IS7XX_IOCONTROL_SRESET_BIT);
 
 	for (i = 0; i < devtype->nr_uart; ++i) {
 		s->p[i].line		= i;
@@ -1305,6 +1295,8 @@ static int sc16is7xx_probe(struct device *dev,
 		s->p[i].port.rs485_config = sc16is7xx_config_rs485;
 		s->p[i].port.ops	= &sc16is7xx_ops;
 		s->p[i].port.line	= sc16is7xx_alloc_line();
+		s->p[i].regmap		= regmaps[i];
+
 		if (s->p[i].port.line >= SC16IS7XX_MAX_DEVS) {
 			ret = -ENOMEM;
 			goto out_ports;
@@ -1326,13 +1318,13 @@ static int sc16is7xx_probe(struct device *dev,
 		sc16is7xx_port_write(&s->p[i].port, SC16IS7XX_LCR_REG,
 				     SC16IS7XX_LCR_CONF_MODE_B);
 
-		regcache_cache_bypass(s->regmap, true);
+		regcache_cache_bypass(regmaps[i], true);
 
 		/* Enable write access to enhanced features */
 		sc16is7xx_port_write(&s->p[i].port, SC16IS7XX_EFR_REG,
 				     SC16IS7XX_EFR_ENABLE_BIT);
 
-		regcache_cache_bypass(s->regmap, false);
+		regcache_cache_bypass(regmaps[i], false);
 
 		/* Restore access to general registers */
 		sc16is7xx_port_write(&s->p[i].port, SC16IS7XX_LCR_REG, 0x00);
@@ -1447,21 +1439,38 @@ static const struct of_device_id __maybe_unused sc16is7xx_dt_ids[] = {
 MODULE_DEVICE_TABLE(of, sc16is7xx_dt_ids);
 
 static struct regmap_config regcfg = {
-	.reg_bits = 7,
-	.pad_bits = 1,
+	.reg_bits = 5,
+	.pad_bits = 3,
 	.val_bits = 8,
 	.cache_type = REGCACHE_RBTREE,
 	.volatile_reg = sc16is7xx_regmap_volatile,
 	.precious_reg = sc16is7xx_regmap_precious,
 	.writeable_noinc_reg = sc16is7xx_regmap_noinc,
 	.readable_noinc_reg = sc16is7xx_regmap_noinc,
+	.max_register = SC16IS7XX_EFCR_REG,
 };
 
+static const char *sc16is7xx_regmap_name(unsigned int port_id)
+{
+	static char buf[6];
+
+	snprintf(buf, sizeof(buf), "port%d", port_id);
+
+	return buf;
+}
+
+static unsigned int sc16is7xx_regmap_port_mask(unsigned int port_id)
+{
+	/* CH1,CH0 are at bits 2:1. */
+	return port_id << 1;
+}
+
 #ifdef CONFIG_SERIAL_SC16IS7XX_SPI
 static int sc16is7xx_spi_probe(struct spi_device *spi)
 {
 	const struct sc16is7xx_devtype *devtype;
-	struct regmap *regmap;
+	struct regmap *regmaps[2];
+	unsigned int i;
 	int ret;
 
 	/* Setup SPI bus */
@@ -1486,11 +1495,20 @@ static int sc16is7xx_spi_probe(struct spi_device *spi)
 		devtype = (struct sc16is7xx_devtype *)id_entry->driver_data;
 	}
 
-	regcfg.max_register = (0xf << SC16IS7XX_REG_SHIFT) |
-			      (devtype->nr_uart - 1);
-	regmap = devm_regmap_init_spi(spi, &regcfg);
+	for (i = 0; i < devtype->nr_uart; i++) {
+		regcfg.name = sc16is7xx_regmap_name(i);
+		/*
+		 * If read_flag_mask is 0, the regmap code sets it to a default
+		 * of 0x80. Since we specify our own mask, we must add the READ
+		 * bit ourselves:
+		 */
+		regcfg.read_flag_mask = sc16is7xx_regmap_port_mask(i) |
+			SC16IS7XX_SPI_READ_BIT;
+		regcfg.write_flag_mask = sc16is7xx_regmap_port_mask(i);
+		regmaps[i] = devm_regmap_init_spi(spi, &regcfg);
+	}
 
-	return sc16is7xx_probe(&spi->dev, devtype, regmap, spi->irq);
+	return sc16is7xx_probe(&spi->dev, devtype, regmaps, spi->irq);
 }
 
 static int sc16is7xx_spi_remove(struct spi_device *spi)
@@ -1529,7 +1547,8 @@ static int sc16is7xx_i2c_probe(struct i2c_client *i2c,
 			       const struct i2c_device_id *id)
 {
 	const struct sc16is7xx_devtype *devtype;
-	struct regmap *regmap;
+	struct regmap *regmaps[2];
+	unsigned int i;
 
 	if (i2c->dev.of_node) {
 		devtype = device_get_match_data(&i2c->dev);
@@ -1539,11 +1558,14 @@ static int sc16is7xx_i2c_probe(struct i2c_client *i2c,
 		devtype = (struct sc16is7xx_devtype *)id->driver_data;
 	}
 
-	regcfg.max_register = (0xf << SC16IS7XX_REG_SHIFT) |
-			      (devtype->nr_uart - 1);
-	regmap = devm_regmap_init_i2c(i2c, &regcfg);
+	for (i = 0; i < devtype->nr_uart; i++) {
+		regcfg.name = sc16is7xx_regmap_name(i);
+		regcfg.read_flag_mask = sc16is7xx_regmap_port_mask(i);
+		regcfg.write_flag_mask = sc16is7xx_regmap_port_mask(i);
+		regmaps[i] = devm_regmap_init_i2c(i2c, &regcfg);
+	}
 
-	return sc16is7xx_probe(&i2c->dev, devtype, regmap, i2c->irq);
+	return sc16is7xx_probe(&i2c->dev, devtype, regmaps, i2c->irq);
 }
 
 static int sc16is7xx_i2c_remove(struct i2c_client *client)
-- 
2.34.1

Re: [stable-kernel][5.15.y][PATCH 1/5] serial: sc16is7xx: improve regmap debugfs by using one regmap per port

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: 3837a0379533aabb9e4483677077479f7c6aa910

WARNING: Author mismatch between patch and upstream commit:
Backport author: Hui Wang <hui.wang@canonical.com>
Commit author: Hugo Villeneuve <hvilleneuve@dimonoff.com>


Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.6.y | Present (different SHA1: 452ed2b218b1)
6.1.y | Present (different SHA1: 45ec1b7accd5)
5.15.y | Not found

Note: The patch differs from the upstream commit:
---
1:  3837a0379533a ! 1:  cffffd769d922 serial: sc16is7xx: improve regmap debugfs by using one regmap per port
    @@ Metadata
      ## Commit message ##
         serial: sc16is7xx: improve regmap debugfs by using one regmap per port
     
    +    commit 3837a0379533aabb9e4483677077479f7c6aa910 upstream.
    +
         With this current driver regmap implementation, it is hard to make sense
         of the register addresses displayed using the regmap debugfs interface,
         because they do not correspond to the actual register addresses documented
    @@ Commit message
         As an added bonus, this also simplifies some operations (read/write/modify)
         because it is no longer necessary to manually shift register addresses.
     
    +    [Hui: Fixed some conflict when backporting to 5.15.y]
    +
         Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
         Link: https://lore.kernel.org/r/20231030211447.974779-1-hugo@hugovil.com
         Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    +    Signed-off-by: Hui Wang <hui.wang@canonical.com>
     
      ## drivers/tty/serial/sc16is7xx.c ##
     @@
    - 
    + 						  */
      
      /* Misc definitions */
     +#define SC16IS7XX_SPI_READ_BIT		BIT(7)
      #define SC16IS7XX_FIFO_SIZE		(64)
     -#define SC16IS7XX_REG_SHIFT		2
    - #define SC16IS7XX_GPIOS_PER_BANK	4
    ++#define SC16IS7XX_GPIOS_PER_BANK	4
      
      struct sc16is7xx_devtype {
    + 	char	name[10];
     @@ drivers/tty/serial/sc16is7xx.c: struct sc16is7xx_one_config {
      struct sc16is7xx_one {
      	struct uart_port		port;
    @@ drivers/tty/serial/sc16is7xx.c: struct sc16is7xx_one_config {
     +	struct regmap			*regmap;
      	struct kthread_work		tx_work;
      	struct kthread_work		reg_work;
    - 	struct kthread_delayed_work	ms_work;
    -@@ drivers/tty/serial/sc16is7xx.c: static void sc16is7xx_stop_tx(struct uart_port *port);
    - 
    + 	struct sc16is7xx_one_config	config;
    +@@ drivers/tty/serial/sc16is7xx.c: static struct uart_driver sc16is7xx_uart = {
    + #define to_sc16is7xx_port(p,e)	((container_of((p), struct sc16is7xx_port, e)))
      #define to_sc16is7xx_one(p,e)	((container_of((p), struct sc16is7xx_one, e)))
      
     -static int sc16is7xx_line(struct uart_port *port)
    @@ drivers/tty/serial/sc16is7xx.c: static void sc16is7xx_stop_tx(struct uart_port *
     -	u8 addr = (SC16IS7XX_RHR_REG << SC16IS7XX_REG_SHIFT) | line;
     +	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
      
    --	regcache_cache_bypass(s->regmap, true);
    --	regmap_raw_read(s->regmap, addr, s->buf, rxlen);
    --	regcache_cache_bypass(s->regmap, false);
    -+	regcache_cache_bypass(one->regmap, true);
    -+	regmap_raw_read(one->regmap, SC16IS7XX_RHR_REG, s->buf, rxlen);
    -+	regcache_cache_bypass(one->regmap, false);
    +-	regmap_noinc_read(s->regmap, addr, s->buf, rxlen);
    ++	regmap_noinc_read(one->regmap, SC16IS7XX_RHR_REG, s->buf, rxlen);
      }
      
      static void sc16is7xx_fifo_write(struct uart_port *port, u8 to_send)
    @@ drivers/tty/serial/sc16is7xx.c: static void sc16is7xx_fifo_write(struct uart_por
      	if (unlikely(!to_send))
      		return;
      
    --	regcache_cache_bypass(s->regmap, true);
    --	regmap_raw_write(s->regmap, addr, s->buf, to_send);
    --	regcache_cache_bypass(s->regmap, false);
    -+	regcache_cache_bypass(one->regmap, true);
    -+	regmap_raw_write(one->regmap, SC16IS7XX_THR_REG, s->buf, to_send);
    -+	regcache_cache_bypass(one->regmap, false);
    +-	regmap_noinc_write(s->regmap, addr, s->buf, to_send);
    ++	regmap_noinc_write(one->regmap, SC16IS7XX_THR_REG, s->buf, to_send);
      }
      
      static void sc16is7xx_port_update(struct uart_port *port, u8 reg,
    @@ drivers/tty/serial/sc16is7xx.c: static bool sc16is7xx_regmap_volatile(struct dev
      	case SC16IS7XX_RHR_REG:
      		return true;
      	default:
    -@@ drivers/tty/serial/sc16is7xx.c: static bool sc16is7xx_regmap_precious(struct device *dev, unsigned int reg)
    +@@ drivers/tty/serial/sc16is7xx.c: static bool sc16is7xx_regmap_noinc(struct device *dev, unsigned int reg)
      static int sc16is7xx_set_baud(struct uart_port *port, int baud)
      {
      	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
     +	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
      	u8 lcr;
    - 	u8 prescaler = 0;
    + 	unsigned int prescaler = 1;
      	unsigned long clk = port->uartclk, div = clk / 16 / baud;
     @@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_set_baud(struct uart_port *port, int baud)
      			     SC16IS7XX_LCR_CONF_MODE_B);
    @@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_set_baud(struct uart_port *
      	/* Enable enhanced features */
     -	regcache_cache_bypass(s->regmap, true);
     +	regcache_cache_bypass(one->regmap, true);
    - 	sc16is7xx_port_update(port, SC16IS7XX_EFR_REG,
    - 			      SC16IS7XX_EFR_ENABLE_BIT,
    - 			      SC16IS7XX_EFR_ENABLE_BIT);
    - 
    + 	sc16is7xx_port_write(port, SC16IS7XX_EFR_REG,
    + 			     SC16IS7XX_EFR_ENABLE_BIT);
     -	regcache_cache_bypass(s->regmap, false);
     +	regcache_cache_bypass(one->regmap, false);
      
    @@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_set_baud(struct uart_port *
      
      	/* Put LCR back to the normal mode */
      	sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, lcr);
    +@@ drivers/tty/serial/sc16is7xx.c: static void sc16is7xx_set_termios(struct uart_port *port,
    + 				  struct ktermios *old)
    + {
    + 	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
    ++	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
    + 	unsigned int lcr, flow = 0;
    + 	int baud;
    + 
     @@ drivers/tty/serial/sc16is7xx.c: static void sc16is7xx_set_termios(struct uart_port *port,
      			     SC16IS7XX_LCR_CONF_MODE_B);
      
    @@ drivers/tty/serial/sc16is7xx.c: static void sc16is7xx_set_termios(struct uart_po
     +	regcache_cache_bypass(one->regmap, true);
      	sc16is7xx_port_write(port, SC16IS7XX_XON1_REG, termios->c_cc[VSTART]);
      	sc16is7xx_port_write(port, SC16IS7XX_XOFF1_REG, termios->c_cc[VSTOP]);
    - 
    + 	if (termios->c_cflag & CRTSCTS)
     @@ drivers/tty/serial/sc16is7xx.c: static void sc16is7xx_set_termios(struct uart_port *port,
    - 			      SC16IS7XX_EFR_REG,
    - 			      SC16IS7XX_EFR_FLOWCTRL_BITS,
    - 			      flow);
    + 		flow |= SC16IS7XX_EFR_SWFLOW1_BIT;
    + 
    + 	sc16is7xx_port_write(port, SC16IS7XX_EFR_REG, flow);
     -	regcache_cache_bypass(s->regmap, false);
     +	regcache_cache_bypass(one->regmap, false);
      
      	/* Update LCR register */
      	sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, lcr);
    -@@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_config_rs485(struct uart_port *port, struct ktermios *termi
    +@@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_config_rs485(struct uart_port *port,
      static int sc16is7xx_startup(struct uart_port *port)
      {
      	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
     -	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
      	unsigned int val;
    - 	unsigned long flags;
      
    + 	sc16is7xx_power(port, 1);
     @@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_startup(struct uart_port *port)
      	sc16is7xx_port_write(port, SC16IS7XX_LCR_REG,
      			     SC16IS7XX_LCR_CONF_MODE_B);
    @@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_startup(struct uart_port *p
     +	regcache_cache_bypass(one->regmap, true);
      
      	/* Enable write access to enhanced features and internal clock div */
    - 	sc16is7xx_port_update(port, SC16IS7XX_EFR_REG,
    + 	sc16is7xx_port_write(port, SC16IS7XX_EFR_REG,
     @@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_startup(struct uart_port *port)
      			     SC16IS7XX_TCR_RX_RESUME(24) |
      			     SC16IS7XX_TCR_RX_HALT(48));
    @@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_startup(struct uart_port *p
      
      	/* Now, initialize the UART */
      	sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, SC16IS7XX_LCR_WORD_LEN_8);
    -@@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_setup_mctrl_ports(struct sc16is7xx_port *s)
    - 	if (s->mctrl_mask)
    - 		regmap_update_bits(
    - 			s->regmap,
    --			SC16IS7XX_IOCONTROL_REG << SC16IS7XX_REG_SHIFT,
    -+			SC16IS7XX_IOCONTROL_REG,
    - 			SC16IS7XX_IOCONTROL_MODEM_A_BIT |
    - 			SC16IS7XX_IOCONTROL_MODEM_B_BIT, s->mctrl_mask);
    - 
    -@@ drivers/tty/serial/sc16is7xx.c: static const struct serial_rs485 sc16is7xx_rs485_supported = {
    +@@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_gpio_direction_output(struct gpio_chip *chip,
      
      static int sc16is7xx_probe(struct device *dev,
      			   const struct sc16is7xx_devtype *devtype,
    @@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_probe(struct device *dev,
      	for (i = 0; i < devtype->nr_uart; ++i) {
      		s->p[i].line		= i;
     @@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_probe(struct device *dev,
    + 		s->p[i].port.rs485_config = sc16is7xx_config_rs485;
      		s->p[i].port.ops	= &sc16is7xx_ops;
    - 		s->p[i].old_mctrl	= 0;
      		s->p[i].port.line	= sc16is7xx_alloc_line();
     +		s->p[i].regmap		= regmaps[i];
    - 
    ++
      		if (s->p[i].port.line >= SC16IS7XX_MAX_DEVS) {
      			ret = -ENOMEM;
    + 			goto out_ports;
     @@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_probe(struct device *dev,
      		sc16is7xx_port_write(&s->p[i].port, SC16IS7XX_LCR_REG,
      				     SC16IS7XX_LCR_CONF_MODE_B);
    @@ drivers/tty/serial/sc16is7xx.c: static const struct of_device_id __maybe_unused
      	.cache_type = REGCACHE_RBTREE,
      	.volatile_reg = sc16is7xx_regmap_volatile,
      	.precious_reg = sc16is7xx_regmap_precious,
    + 	.writeable_noinc_reg = sc16is7xx_regmap_noinc,
    + 	.readable_noinc_reg = sc16is7xx_regmap_noinc,
     +	.max_register = SC16IS7XX_EFCR_REG,
      };
      
    @@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_spi_probe(struct spi_device
     +	return sc16is7xx_probe(&spi->dev, devtype, regmaps, spi->irq);
      }
      
    - static void sc16is7xx_spi_remove(struct spi_device *spi)
    -@@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_i2c_probe(struct i2c_client *i2c)
    + static int sc16is7xx_spi_remove(struct spi_device *spi)
    +@@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_i2c_probe(struct i2c_client *i2c,
    + 			       const struct i2c_device_id *id)
      {
    - 	const struct i2c_device_id *id = i2c_client_get_device_id(i2c);
      	const struct sc16is7xx_devtype *devtype;
     -	struct regmap *regmap;
     +	struct regmap *regmaps[2];
    @@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_i2c_probe(struct i2c_client
      
      	if (i2c->dev.of_node) {
      		devtype = device_get_match_data(&i2c->dev);
    -@@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_i2c_probe(struct i2c_client *i2c)
    +@@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_i2c_probe(struct i2c_client *i2c,
      		devtype = (struct sc16is7xx_devtype *)id->driver_data;
      	}
      
    @@ drivers/tty/serial/sc16is7xx.c: static int sc16is7xx_i2c_probe(struct i2c_client
     +	return sc16is7xx_probe(&i2c->dev, devtype, regmaps, i2c->irq);
      }
      
    - static void sc16is7xx_i2c_remove(struct i2c_client *client)
    + static int sc16is7xx_i2c_remove(struct i2c_client *client)
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.15.y       |  Success    |  Success   |

[stable-kernel][5.15.y][PATCH 2/5] serial: sc16is7xx: remove wasteful static buffer in sc16is7xx_regmap_name()

[Hui Wang]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

commit 6bcab3c8acc88e265c570dea969fd04f137c8a4c upstream.

Using a static buffer inside sc16is7xx_regmap_name() was a convenient and
simple way to set the regmap name without having to allocate and free a
buffer each time it is called. The drawback is that the static buffer
wastes memory for nothing once regmap is fully initialized.

Remove static buffer and use constant strings instead.

This also avoids a truncation warning when using "%d" or "%u" in snprintf
which was flagged by kernel test robot.

Fixes: 3837a0379533 ("serial: sc16is7xx: improve regmap debugfs by using one regmap per port")
Cc:  <stable@vger.kernel.org> # 6.1.x: 3837a03 serial: sc16is7xx: improve regmap debugfs by using one regmap per port
Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Link: https://lore.kernel.org/r/20231211171353.2901416-2-hugo@hugovil.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 drivers/tty/serial/sc16is7xx.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 88adbd9d5002..35001cc7ec90 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1450,13 +1450,15 @@ static struct regmap_config regcfg = {
 	.max_register = SC16IS7XX_EFCR_REG,
 };
 
-static const char *sc16is7xx_regmap_name(unsigned int port_id)
+static const char *sc16is7xx_regmap_name(u8 port_id)
 {
-	static char buf[6];
-
-	snprintf(buf, sizeof(buf), "port%d", port_id);
-
-	return buf;
+	switch (port_id) {
+	case 0:	return "port0";
+	case 1:	return "port1";
+	default:
+		WARN_ON(true);
+		return NULL;
+	}
 }
 
 static unsigned int sc16is7xx_regmap_port_mask(unsigned int port_id)
-- 
2.34.1

Re: [stable-kernel][5.15.y][PATCH 2/5] serial: sc16is7xx: remove wasteful static buffer in sc16is7xx_regmap_name()

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: 6bcab3c8acc88e265c570dea969fd04f137c8a4c

WARNING: Author mismatch between patch and upstream commit:
Backport author: Hui Wang <hui.wang@canonical.com>
Commit author: Hugo Villeneuve <hvilleneuve@dimonoff.com>


Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.6.y | Present (different SHA1: f769407d0194)
6.1.y | Present (different SHA1: 9bcb019aee47)
5.15.y | Not found

Note: The patch differs from the upstream commit:
---
Failed to apply patch cleanly, falling back to interdiff...
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.15.y       |  Success    |  Success   |

[stable-kernel][5.15.y][PATCH 3/5] serial: sc16is7xx: remove global regmap from struct sc16is7xx_port

[Hui Wang]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

commit f6959c5217bd799bcb770b95d3c09b3244e175c6 upstream.

Remove global struct regmap so that it is more obvious that this
regmap is to be used only in the probe function.

Also add a comment to that effect in probe function.

[Hui: fixed some conflict when backporting to 5.15.y]

Fixes: 3837a0379533 ("serial: sc16is7xx: improve regmap debugfs by using one regmap per port")
Cc:  <stable@vger.kernel.org>
Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Link: https://lore.kernel.org/r/20231211171353.2901416-3-hugo@hugovil.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 drivers/tty/serial/sc16is7xx.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 35001cc7ec90..90b39ddec082 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -323,7 +323,6 @@ struct sc16is7xx_one {
 
 struct sc16is7xx_port {
 	const struct sc16is7xx_devtype	*devtype;
-	struct regmap			*regmap;
 	struct clk			*clk;
 #ifdef CONFIG_GPIOLIB
 	struct gpio_chip		gpio;
@@ -1222,6 +1221,10 @@ static int sc16is7xx_probe(struct device *dev,
 	 * This device does not have an identification register that would
 	 * tell us if we are really connected to the correct device.
 	 * The best we can do is to check if communication is at all possible.
+	 *
+	 * Note: regmap[0] is used in the probe function to access registers
+	 * common to all channels/ports, as it is guaranteed to be present on
+	 * all variants.
 	 */
 	ret = regmap_read(regmaps[0], SC16IS7XX_LSR_REG, &val);
 	if (ret < 0)
@@ -1257,7 +1260,6 @@ static int sc16is7xx_probe(struct device *dev,
 			return -EINVAL;
 	}
 
-	s->regmap = regmaps[0];
 	s->devtype = devtype;
 	dev_set_drvdata(dev, s);
 	mutex_init(&s->efr_lock);
@@ -1272,7 +1274,7 @@ static int sc16is7xx_probe(struct device *dev,
 	sched_set_fifo(s->kworker_task);
 
 	/* reset device, purging any pending irq / data */
-	regmap_write(s->regmap, SC16IS7XX_IOCONTROL_REG,
+	regmap_write(regmaps[0], SC16IS7XX_IOCONTROL_REG,
 		     SC16IS7XX_IOCONTROL_SRESET_BIT);
 
 	for (i = 0; i < devtype->nr_uart; ++i) {
-- 
2.34.1

Re: [stable-kernel][5.15.y][PATCH 3/5] serial: sc16is7xx: remove global regmap from struct sc16is7xx_port

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: f6959c5217bd799bcb770b95d3c09b3244e175c6

WARNING: Author mismatch between patch and upstream commit:
Backport author: Hui Wang <hui.wang@canonical.com>
Commit author: Hugo Villeneuve <hvilleneuve@dimonoff.com>


Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.6.y | Present (different SHA1: fc3de570cb30)
6.1.y | Present (different SHA1: 6dca71e6e14a)
5.15.y | Not found

Note: The patch differs from the upstream commit:
---
Failed to apply patch cleanly, falling back to interdiff...
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.15.y       |  Success    |  Success   |

[stable-kernel][5.15.y][PATCH 4/5] serial: sc16is7xx: remove unused line structure member

[Hui Wang]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

commit 41a308cbedb2a68a6831f0f2e992e296c4b8aff0 upstream.

Now that the driver has been converted to use one regmap per port, the line
structure member is no longer used, so remove it.

Fixes: 3837a0379533 ("serial: sc16is7xx: improve regmap debugfs by using one regmap per port")
Cc:  <stable@vger.kernel.org>
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Link: https://lore.kernel.org/r/20231211171353.2901416-4-hugo@hugovil.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 drivers/tty/serial/sc16is7xx.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 90b39ddec082..a2d05dd5a339 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -313,7 +313,6 @@ struct sc16is7xx_one_config {
 
 struct sc16is7xx_one {
 	struct uart_port		port;
-	u8				line;
 	struct regmap			*regmap;
 	struct kthread_work		tx_work;
 	struct kthread_work		reg_work;
@@ -1278,7 +1277,6 @@ static int sc16is7xx_probe(struct device *dev,
 		     SC16IS7XX_IOCONTROL_SRESET_BIT);
 
 	for (i = 0; i < devtype->nr_uart; ++i) {
-		s->p[i].line		= i;
 		/* Initialize port data */
 		s->p[i].port.dev	= dev;
 		s->p[i].port.irq	= irq;
-- 
2.34.1

Re: [stable-kernel][5.15.y][PATCH 4/5] serial: sc16is7xx: remove unused line structure member

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: 41a308cbedb2a68a6831f0f2e992e296c4b8aff0

WARNING: Author mismatch between patch and upstream commit:
Backport author: Hui Wang <hui.wang@canonical.com>
Commit author: Hugo Villeneuve <hvilleneuve@dimonoff.com>


Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.6.y | Present (different SHA1: 2f6ae16a5874)
6.1.y | Present (different SHA1: f6c58552a8d9)
5.15.y | Not found

Note: The patch differs from the upstream commit:
---
Failed to apply patch cleanly, falling back to interdiff...
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.15.y       |  Success    |  Success   |

[stable-kernel][5.15.y][PATCH 5/5] serial: sc16is7xx: change EFR lock to operate on each channels

[Hui Wang]

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

commit 4409df5866b7ff7686ba27e449ca97a92ee063c9 upstream.

Now that the driver has been converted to use one regmap per port, change
efr locking to operate on a channel basis instead of on the whole IC.

[Hui: fixed some conflict when backporting to 5.15.y]

Fixes: 3837a0379533 ("serial: sc16is7xx: improve regmap debugfs by using one regmap per port")
Cc:  <stable@vger.kernel.org> # 6.1.x: 3837a03 serial: sc16is7xx: improve regmap debugfs by using one regmap per port
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Link: https://lore.kernel.org/r/20231211171353.2901416-5-hugo@hugovil.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 drivers/tty/serial/sc16is7xx.c | 41 +++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index a2d05dd5a339..692c14d7f7d1 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -314,6 +314,7 @@ struct sc16is7xx_one_config {
 struct sc16is7xx_one {
 	struct uart_port		port;
 	struct regmap			*regmap;
+	struct mutex			efr_lock; /* EFR registers access */
 	struct kthread_work		tx_work;
 	struct kthread_work		reg_work;
 	struct sc16is7xx_one_config	config;
@@ -329,7 +330,6 @@ struct sc16is7xx_port {
 	unsigned char			buf[SC16IS7XX_FIFO_SIZE];
 	struct kthread_worker		kworker;
 	struct task_struct		*kworker_task;
-	struct mutex			efr_lock;
 	struct sc16is7xx_one		p[];
 };
 
@@ -491,7 +491,6 @@ static bool sc16is7xx_regmap_noinc(struct device *dev, unsigned int reg)
  */
 static int sc16is7xx_set_baud(struct uart_port *port, int baud)
 {
-	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
 	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
 	u8 lcr;
 	unsigned int prescaler = 1;
@@ -515,7 +514,7 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud)
 	 * because the bulk of the interrupt processing is run as a workqueue
 	 * job in thread context.
 	 */
-	mutex_lock(&s->efr_lock);
+	mutex_lock(&one->efr_lock);
 
 	lcr = sc16is7xx_port_read(port, SC16IS7XX_LCR_REG);
 
@@ -532,7 +531,7 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud)
 	/* Put LCR back to the normal mode */
 	sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, lcr);
 
-	mutex_unlock(&s->efr_lock);
+	mutex_unlock(&one->efr_lock);
 
 	/* If bit MCR_CLKSEL is set, the divide by 4 prescaler is activated. */
 	sc16is7xx_port_update(port, SC16IS7XX_MCR_REG,
@@ -680,14 +679,20 @@ static void sc16is7xx_handle_tx(struct uart_port *port)
 
 static bool sc16is7xx_port_irq(struct sc16is7xx_port *s, int portno)
 {
+	bool rc = true;
 	struct uart_port *port = &s->p[portno].port;
+	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
+
+	mutex_lock(&one->efr_lock);
 
 	do {
 		unsigned int iir, rxlen;
 
 		iir = sc16is7xx_port_read(port, SC16IS7XX_IIR_REG);
-		if (iir & SC16IS7XX_IIR_NO_INT_BIT)
-			return false;
+		if (iir & SC16IS7XX_IIR_NO_INT_BIT) {
+			rc = false;
+			goto out_port_irq;
+		}
 
 		iir &= SC16IS7XX_IIR_ID_MASK;
 
@@ -722,15 +727,17 @@ static bool sc16is7xx_port_irq(struct sc16is7xx_port *s, int portno)
 			break;
 		}
 	} while (0);
-	return true;
+
+out_port_irq:
+	mutex_unlock(&one->efr_lock);
+
+	return rc;
 }
 
 static irqreturn_t sc16is7xx_irq(int irq, void *dev_id)
 {
 	struct sc16is7xx_port *s = (struct sc16is7xx_port *)dev_id;
 
-	mutex_lock(&s->efr_lock);
-
 	while (1) {
 		bool keep_polling = false;
 		int i;
@@ -741,23 +748,21 @@ static irqreturn_t sc16is7xx_irq(int irq, void *dev_id)
 			break;
 	}
 
-	mutex_unlock(&s->efr_lock);
-
 	return IRQ_HANDLED;
 }
 
 static void sc16is7xx_tx_proc(struct kthread_work *ws)
 {
 	struct uart_port *port = &(to_sc16is7xx_one(ws, tx_work)->port);
-	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
+	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
 
 	if ((port->rs485.flags & SER_RS485_ENABLED) &&
 	    (port->rs485.delay_rts_before_send > 0))
 		msleep(port->rs485.delay_rts_before_send);
 
-	mutex_lock(&s->efr_lock);
+	mutex_lock(&one->efr_lock);
 	sc16is7xx_handle_tx(port);
-	mutex_unlock(&s->efr_lock);
+	mutex_unlock(&one->efr_lock);
 }
 
 static void sc16is7xx_reconf_rs485(struct uart_port *port)
@@ -878,7 +883,6 @@ static void sc16is7xx_set_termios(struct uart_port *port,
 				  struct ktermios *termios,
 				  struct ktermios *old)
 {
-	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
 	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
 	unsigned int lcr, flow = 0;
 	int baud;
@@ -934,7 +938,7 @@ static void sc16is7xx_set_termios(struct uart_port *port,
 		port->ignore_status_mask |= SC16IS7XX_LSR_BRK_ERROR_MASK;
 
 	/* As above, claim the mutex while accessing the EFR. */
-	mutex_lock(&s->efr_lock);
+	mutex_lock(&one->efr_lock);
 
 	sc16is7xx_port_write(port, SC16IS7XX_LCR_REG,
 			     SC16IS7XX_LCR_CONF_MODE_B);
@@ -957,7 +961,7 @@ static void sc16is7xx_set_termios(struct uart_port *port,
 	/* Update LCR register */
 	sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, lcr);
 
-	mutex_unlock(&s->efr_lock);
+	mutex_unlock(&one->efr_lock);
 
 	/* Get baud rate generator configuration */
 	baud = uart_get_baud_rate(port, termios, old,
@@ -1261,7 +1265,6 @@ static int sc16is7xx_probe(struct device *dev,
 
 	s->devtype = devtype;
 	dev_set_drvdata(dev, s);
-	mutex_init(&s->efr_lock);
 
 	kthread_init_worker(&s->kworker);
 	s->kworker_task = kthread_run(kthread_worker_fn, &s->kworker,
@@ -1302,6 +1305,8 @@ static int sc16is7xx_probe(struct device *dev,
 			goto out_ports;
 		}
 
+		mutex_init(&s->p[i].efr_lock);
+
 		/* Disable all interrupts */
 		sc16is7xx_port_write(&s->p[i].port, SC16IS7XX_IER_REG, 0);
 		/* Disable TX/RX */
-- 
2.34.1

Re: [stable-kernel][5.15.y][PATCH 5/5] serial: sc16is7xx: change EFR lock to operate on each channels

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: 4409df5866b7ff7686ba27e449ca97a92ee063c9

WARNING: Author mismatch between patch and upstream commit:
Backport author: Hui Wang <hui.wang@canonical.com>
Commit author: Hugo Villeneuve <hvilleneuve@dimonoff.com>


Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.6.y | Present (different SHA1: 9879e1bec3c0)
6.1.y | Present (different SHA1: 4b068e55bf5e)
5.15.y | Not found

Note: The patch differs from the upstream commit:
---
Failed to apply patch cleanly, falling back to interdiff...
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.15.y       |  Success    |  Success   |

Re: [stable-kernel][5.15.y][PATCH 0/5] Fix a regression on sc16is7xx

[Greg KH]

On Wed, Dec 11, 2024 at 12:25:39PM +0800, Hui Wang wrote:
> Recently we found the fifo_read() and fifo_write() are broken in our
> 5.15 kernel after rebase to the latest 5.15.y, the 5.15.y integrated
> the commit e635f652696e ("serial: sc16is7xx: convert from _raw_ to
> _noinc_ regmap functions for FIFO"), but it forgot to integrate a
> prerequisite commit 3837a0379533 ("serial: sc16is7xx: improve regmap
> debugfs by using one regmap per port").
> 
> And about the prerequisite commit, there are also 4 commits to fix it,
> So in total, I backported 5 patches to 5.15.y to fix this regression.
> 
> 0002-xxx and 0004-xxx could be cleanly applied to 5.15.y, the remaining
> 3 patches need to resolve some conflict.
> 
> Hugo Villeneuve (5):
>   serial: sc16is7xx: improve regmap debugfs by using one regmap per port
>   serial: sc16is7xx: remove wasteful static buffer in
>     sc16is7xx_regmap_name()
>   serial: sc16is7xx: remove global regmap from struct sc16is7xx_port
>   serial: sc16is7xx: remove unused line structure member
>   serial: sc16is7xx: change EFR lock to operate on each channels
> 
>  drivers/tty/serial/sc16is7xx.c | 185 +++++++++++++++++++--------------
>  1 file changed, 107 insertions(+), 78 deletions(-)

How well did you test this series?  It seems you forgot about commit
133f4c00b8b2 ("serial: sc16is7xx: fix TX fifo corruption"), right?

Please do better testing and resend a working set of patches.

thanks,

greg k-h

Re: [stable-kernel][5.15.y][PATCH 0/5] Fix a regression on sc16is7xx

[Hui Wang]

On 12/12/24 21:44, Greg KH wrote:
> On Wed, Dec 11, 2024 at 12:25:39PM +0800, Hui Wang wrote:
>> Recently we found the fifo_read() and fifo_write() are broken in our
>> 5.15 kernel after rebase to the latest 5.15.y, the 5.15.y integrated
>> the commit e635f652696e ("serial: sc16is7xx: convert from _raw_ to
>> _noinc_ regmap functions for FIFO"), but it forgot to integrate a
>> prerequisite commit 3837a0379533 ("serial: sc16is7xx: improve regmap
>> debugfs by using one regmap per port").
>>
>> And about the prerequisite commit, there are also 4 commits to fix it,
>> So in total, I backported 5 patches to 5.15.y to fix this regression.
>>
>> 0002-xxx and 0004-xxx could be cleanly applied to 5.15.y, the remaining
>> 3 patches need to resolve some conflict.
>>
>> Hugo Villeneuve (5):
>>    serial: sc16is7xx: improve regmap debugfs by using one regmap per port
>>    serial: sc16is7xx: remove wasteful static buffer in
>>      sc16is7xx_regmap_name()
>>    serial: sc16is7xx: remove global regmap from struct sc16is7xx_port
>>    serial: sc16is7xx: remove unused line structure member
>>    serial: sc16is7xx: change EFR lock to operate on each channels
>>
>>   drivers/tty/serial/sc16is7xx.c | 185 +++++++++++++++++++--------------
>>   1 file changed, 107 insertions(+), 78 deletions(-)
> How well did you test this series?  It seems you forgot about commit
> 133f4c00b8b2 ("serial: sc16is7xx: fix TX fifo corruption"), right?
>
> Please do better testing and resend a working set of patches.

Okay, got it.

Thanks.

>
> thanks,
>
> greg k-h

Re: [stable-kernel][5.15.y][PATCH 0/5] Fix a regression on sc16is7xx

[Hugo Villeneuve]

On Thu, 12 Dec 2024 22:00:00 +0800
Hui Wang <hui.wang@canonical.com> wrote:

> 
> On 12/12/24 21:44, Greg KH wrote:
> > On Wed, Dec 11, 2024 at 12:25:39PM +0800, Hui Wang wrote:
> >> Recently we found the fifo_read() and fifo_write() are broken in our
> >> 5.15 kernel after rebase to the latest 5.15.y, the 5.15.y integrated
> >> the commit e635f652696e ("serial: sc16is7xx: convert from _raw_ to
> >> _noinc_ regmap functions for FIFO"), but it forgot to integrate a
> >> prerequisite commit 3837a0379533 ("serial: sc16is7xx: improve regmap
> >> debugfs by using one regmap per port").
> >>
> >> And about the prerequisite commit, there are also 4 commits to fix it,
> >> So in total, I backported 5 patches to 5.15.y to fix this regression.
> >>
> >> 0002-xxx and 0004-xxx could be cleanly applied to 5.15.y, the remaining
> >> 3 patches need to resolve some conflict.
> >>
> >> Hugo Villeneuve (5):
> >>    serial: sc16is7xx: improve regmap debugfs by using one regmap per port
> >>    serial: sc16is7xx: remove wasteful static buffer in
> >>      sc16is7xx_regmap_name()
> >>    serial: sc16is7xx: remove global regmap from struct sc16is7xx_port
> >>    serial: sc16is7xx: remove unused line structure member
> >>    serial: sc16is7xx: change EFR lock to operate on each channels
> >>
> >>   drivers/tty/serial/sc16is7xx.c | 185 +++++++++++++++++++--------------
> >>   1 file changed, 107 insertions(+), 78 deletions(-)
> > How well did you test this series?  It seems you forgot about commit
> > 133f4c00b8b2 ("serial: sc16is7xx: fix TX fifo corruption"), right?
> >
> > Please do better testing and resend a working set of patches.
> 
> Okay, got it.

Hi Hui / Greg,
I am testing these changes on my RS-485 board, and I
found out that this patch is required:

commit b4a778303ea0 ("serial: sc16is7xx: add missing support for rs485
devicetree properties")

With it, it now works (basic loopback test) on 5.15 branch and with my
hardware.

As per Greg's suggestion, I have also tested (and reworked) commit
133f4c00b8b2 ("serial: sc16is7xx: fix TX fifo corruption"), with a
prerequisite patch for it to apply more easily: 53a8c50802745 ("serial:
sc16is7xx: refactor FIFO access functions to increase commonality").

And finally I have added commit c41698d1a04cb ("serial: sc16is7xx: fix
invalid FIFO access with special register set").

I will submit these 4 patches to stable soon.

-- 
Hugo Villeneuve

Re: [stable-kernel][5.15.y][PATCH 0/5] Fix a regression on sc16is7xx

[Hui Wang]

On 12/17/24 02:36, Hugo Villeneuve wrote:
> On Thu, 12 Dec 2024 22:00:00 +0800
> Hui Wang <hui.wang@canonical.com> wrote:
>
>> On 12/12/24 21:44, Greg KH wrote:
>>> On Wed, Dec 11, 2024 at 12:25:39PM +0800, Hui Wang wrote:
>>>> Recently we found the fifo_read() and fifo_write() are broken in our
>>>> 5.15 kernel after rebase to the latest 5.15.y, the 5.15.y integrated
>>>> the commit e635f652696e ("serial: sc16is7xx: convert from _raw_ to
>>>> _noinc_ regmap functions for FIFO"), but it forgot to integrate a
>>>> prerequisite commit 3837a0379533 ("serial: sc16is7xx: improve regmap
>>>> debugfs by using one regmap per port").
>>>>
>>>> And about the prerequisite commit, there are also 4 commits to fix it,
>>>> So in total, I backported 5 patches to 5.15.y to fix this regression.
>>>>
>>>> 0002-xxx and 0004-xxx could be cleanly applied to 5.15.y, the remaining
>>>> 3 patches need to resolve some conflict.
>>>>
>>>> Hugo Villeneuve (5):
>>>>     serial: sc16is7xx: improve regmap debugfs by using one regmap per port
>>>>     serial: sc16is7xx: remove wasteful static buffer in
>>>>       sc16is7xx_regmap_name()
>>>>     serial: sc16is7xx: remove global regmap from struct sc16is7xx_port
>>>>     serial: sc16is7xx: remove unused line structure member
>>>>     serial: sc16is7xx: change EFR lock to operate on each channels
>>>>
>>>>    drivers/tty/serial/sc16is7xx.c | 185 +++++++++++++++++++--------------
>>>>    1 file changed, 107 insertions(+), 78 deletions(-)
>>> How well did you test this series?  It seems you forgot about commit
>>> 133f4c00b8b2 ("serial: sc16is7xx: fix TX fifo corruption"), right?
>>>
>>> Please do better testing and resend a working set of patches.
>> Okay, got it.
> Hi Hui / Greg,
> I am testing these changes on my RS-485 board, and I
> found out that this patch is required:
>
> commit b4a778303ea0 ("serial: sc16is7xx: add missing support for rs485
> devicetree properties")
>
> With it, it now works (basic loopback test) on 5.15 branch and with my
> hardware.
>
> As per Greg's suggestion, I have also tested (and reworked) commit
> 133f4c00b8b2 ("serial: sc16is7xx: fix TX fifo corruption"), with a
> prerequisite patch for it to apply more easily: 53a8c50802745 ("serial:
> sc16is7xx: refactor FIFO access functions to increase commonality").
>
> And finally I have added commit c41698d1a04cb ("serial: sc16is7xx: fix
> invalid FIFO access with special register set").
>
> I will submit these 4 patches to stable soon.

Hi Hugo,

Thanks for your help. I planned to do it, but It is blocked by 
backporting the commit 133f4c00b8b2 ("serial: sc16is7xx: fix TX fifo 
corruption"), it requires kfifo_out_linear_ptr() but this function is 
not in 5.15.y. It is great you could help.

Thanks,

Hui.

>