On Tue, Jul 25, 2023 at 10:23:33AM -0400, Hugo Villeneuve wrote:
From: Hugo Villeneuve hvilleneuve@dimonoff.com
The sc16is7xx_config_rs485() function is called only for the second port (index 1, channel B), causing initialization problems for the first port.
For the sc16is7xx driver, port->membase and port->mapbase are not set, and their default values are 0. And we set port->iobase to the device index. This means that when the first device is registered using the uart_add_one_port() function, the following values will be in the port structure: port->membase = 0 port->mapbase = 0 port->iobase = 0
Therefore, the function uart_configure_port() in serial_core.c will exit early because of the following check: /* * If there isn't a port here, don't do anything further. */ if (!port->iobase && !port->mapbase && !port->membase) return;
Typically, I2C and SPI drivers do not set port->membase and port->mapbase.
The max310x driver sets port->membase to ~0 (all ones). By implementing the same change in this driver, uart_configure_port() is now correctly executed for all ports.
Fixes: dfeae619d781 ("serial: sc16is7xx")
That commit is in a very old 3.x release.
Cc: stable@vger.kernel.org # 6.1.x
But you say this should only go to 6.1.y? Why? What is wrong with the older kernels?
Signed-off-by: Hugo Villeneuve hvilleneuve@dimonoff.com Reviewed-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Reviewed-by: Lech Perczak lech.perczak@camlingroup.com Tested-by: Lech Perczak lech.perczak@camlingroup.com
drivers/tty/serial/sc16is7xx.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index 2e7e7c409cf2..8ae2afc76a9b 100644 --- a/drivers/tty/serial/sc16is7xx.c +++ b/drivers/tty/serial/sc16is7xx.c @@ -1436,6 +1436,7 @@ static int sc16is7xx_probe(struct device *dev, s->p[i].port.fifosize = SC16IS7XX_FIFO_SIZE; s->p[i].port.flags = UPF_FIXED_TYPE | UPF_LOW_LATENCY; s->p[i].port.iobase = i;
s->p[i].port.membase = (void __iomem *)~0;
That's a magic value, some comment should be added here to explain why setting all bits is ok. Why does this work exactly? You only say that the max310x driver does this, but not why it does this at all.
confused,
greg k-h