Hi,
to properly test this commit on my hardware, the following
prerequisite commit is required:
commit b4a778303ea0 ("serial: sc16is7xx: add missing support for rs485
devicetree properties")
As an added benefit, by applying commit b4a778303ea0 before, the
following rework can stay as in the original upstream commit:
> @@ -1302,6 +1305,8 @@ static int sc16is7xx_probe(struct device
> goto out_ports;
> }
>
> + mutex_init(&s->p[i].efr_lock);
> +
If it is not possible to add prerequisite commit b4a778303ea0 before
in the queue, I will simply wait and re-submit the prerequisite commit
with a slight rework after this current commit (...change
EFR lock...) lands in the stable/linux-5.15.y branch.
Hugo.
On Thu, 12 Dec 2024 13:23:37 +0100
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> This is a note to let you know that I've just added the patch titled
>
> serial: sc16is7xx: change EFR lock to operate on each channels
>
> to the 5.15-stable tree which can be found at:
> http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>
> The filename of the patch is:
> serial-sc16is7xx-change-efr-lock-to-operate-on-each-channels.patch
> and it can be found in the queue-5.15 subdirectory.
>
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@xxxxxxxxxxxxxxx> know about it.
>
>
> From 4409df5866b7ff7686ba27e449ca97a92ee063c9 Mon Sep 17 00:00:00 2001
> From: Hugo Villeneuve <hvilleneuve@xxxxxxxxxxxx>
> Date: Mon, 11 Dec 2023 12:13:51 -0500
> Subject: serial: sc16is7xx: change EFR lock to operate on each channels
>
> From: Hugo Villeneuve <hvilleneuve@xxxxxxxxxxxx>
>
> 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.
>
> Fixes: 3837a0379533 ("serial: sc16is7xx: improve regmap debugfs by using one regmap per port")
> Cc: <stable@xxxxxxxxxxxxxxx> # 6.1.x: 3837a03 serial: sc16is7xx: improve regmap debugfs by using one regmap per port
> Signed-off-by: Hugo Villeneuve <hvilleneuve@xxxxxxxxxxxx>
> Link: https://lore.kernel.org/r/20231211171353.2901416-5-hugo@xxxxxxxxxxx
> [Hui: fixed some conflict when backporting to 5.15.y]
> Signed-off-by: Hui Wang <hui.wang@xxxxxxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
> drivers/tty/serial/sc16is7xx.c | 41 +++++++++++++++++++++++------------------
> 1 file changed, 23 insertions(+), 18 deletions(-)
>
> --- 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(struc
> */
> 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 uar
> * 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 uar
> /* 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 u
>
> 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 sc
> 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
> 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
> 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
> 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
> /* 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
>
> 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
> 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 */
>
>
> Patches currently in stable-queue which might be from hvilleneuve@xxxxxxxxxxxx are
>
> queue-5.15/serial-sc16is7xx-remove-unused-line-structure-member.patch
> queue-5.15/serial-sc16is7xx-improve-regmap-debugfs-by-using-one-regmap-per-port.patch
> queue-5.15/serial-sc16is7xx-change-efr-lock-to-operate-on-each-channels.patch
> queue-5.15/serial-sc16is7xx-remove-global-regmap-from-struct-sc16is7xx_port.patch
> queue-5.15/serial-sc16is7xx-remove-wasteful-static-buffer-in-sc16is7xx_regmap_name.patch
--
Hugo Villeneuve
