serial: sc16is7xx: refactor EFR lock

Move common code for EFR lock/unlock of mutex into functions for code reuse
and clarity.

With the addition of old_lcr, move irda_mode within struct sc16is7xx_one to
reduce memory usage:
    Before: /* size: 752, cachelines: 12, members: 10 */
    After:  /* size: 744, cachelines: 12, members: 10 */

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Link: https://lore.kernel.org/r/20231221231823.2327894-17-hugo@hugovil.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 15a46c164821827e04658905f9bbdbd2d977ecf4..929206a9a6e1150f9ec9803240840cfe6c11a37a 100644 (file)
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -330,8 +330,9 @@ struct sc16is7xx_one {
        struct kthread_work             reg_work;
        struct kthread_delayed_work     ms_work;
        struct sc16is7xx_one_config     config;
-       bool                            irda_mode;
        unsigned int                    old_mctrl;
+       u8                              old_lcr; /* Value before EFR access. */
+       bool                            irda_mode;
 };
 
 struct sc16is7xx_port {
@@ -412,6 +413,49 @@ static void sc16is7xx_power(struct uart_port *port, int on)
                              on ? 0 : SC16IS7XX_IER_SLEEP_BIT);
 }
 
+/*
+ * In an amazing feat of design, the Enhanced Features Register (EFR)
+ * shares the address of the Interrupt Identification Register (IIR).
+ * Access to EFR is switched on by writing a magic value (0xbf) to the
+ * Line Control Register (LCR). Any interrupt firing during this time will
+ * see the EFR where it expects the IIR to be, leading to
+ * "Unexpected interrupt" messages.
+ *
+ * Prevent this possibility by claiming a mutex while accessing the EFR,
+ * and claiming the same mutex from within the interrupt handler. This is
+ * similar to disabling the interrupt, but that doesn't work because the
+ * bulk of the interrupt processing is run as a workqueue job in thread
+ * context.
+ */
+static void sc16is7xx_efr_lock(struct uart_port *port)
+{
+       struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
+
+       mutex_lock(&one->efr_lock);
+
+       /* Backup content of LCR. */
+       one->old_lcr = sc16is7xx_port_read(port, SC16IS7XX_LCR_REG);
+
+       /* Enable access to Enhanced register set */
+       sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, SC16IS7XX_LCR_CONF_MODE_B);
+
+       /* Disable cache updates when writing to EFR registers */
+       regcache_cache_bypass(one->regmap, true);
+}
+
+static void sc16is7xx_efr_unlock(struct uart_port *port)
+{
+       struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
+
+       /* Re-enable cache updates when writing to normal registers */
+       regcache_cache_bypass(one->regmap, false);
+
+       /* Restore original content of LCR */
+       sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, one->old_lcr);
+
+       mutex_unlock(&one->efr_lock);
+}
+
 static void sc16is7xx_ier_clear(struct uart_port *port, u8 bit)
 {
        struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
@@ -522,45 +566,19 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud)
                div /= 4;
        }
 
-       /* In an amazing feat of design, the Enhanced Features Register shares
-        * the address of the Interrupt Identification Register, and is
-        * switched in by writing a magic value (0xbf) to the Line Control
-        * Register. Any interrupt firing during this time will see the EFR
-        * where it expects the IIR to be, leading to "Unexpected interrupt"
-        * messages.
-        *
-        * Prevent this possibility by claiming a mutex while accessing the
-        * EFR, and claiming the same mutex from within the interrupt handler.
-        * This is similar to disabling the interrupt, but that doesn't work
-        * because the bulk of the interrupt processing is run as a workqueue
-        * job in thread context.
-        */
-       mutex_lock(&one->efr_lock);
-
-       lcr = sc16is7xx_port_read(port, SC16IS7XX_LCR_REG);
-
-       /* Open the LCR divisors for configuration */
-       sc16is7xx_port_write(port, SC16IS7XX_LCR_REG,
-                            SC16IS7XX_LCR_CONF_MODE_B);
-
        /* Enable enhanced features */
-       regcache_cache_bypass(one->regmap, true);
+       sc16is7xx_efr_lock(port);
        sc16is7xx_port_update(port, SC16IS7XX_EFR_REG,
                              SC16IS7XX_EFR_ENABLE_BIT,
                              SC16IS7XX_EFR_ENABLE_BIT);
-
-       regcache_cache_bypass(one->regmap, false);
-
-       /* Put LCR back to the normal mode */
-       sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, lcr);
-
-       mutex_unlock(&one->efr_lock);
+       sc16is7xx_efr_unlock(port);
 
        sc16is7xx_port_update(port, SC16IS7XX_MCR_REG,
                              SC16IS7XX_MCR_CLKSEL_BIT,
                              prescaler);
 
-       /* Open the LCR divisors for configuration */
+       /* Backup LCR and access special register set (DLL/DLH) */
+       lcr = sc16is7xx_port_read(port, SC16IS7XX_LCR_REG);
        sc16is7xx_port_write(port, SC16IS7XX_LCR_REG,
                             SC16IS7XX_LCR_CONF_MODE_A);
 
@@ -570,7 +588,7 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud)
        sc16is7xx_port_write(port, SC16IS7XX_DLL_REG, div % 256);
        regcache_cache_bypass(one->regmap, false);
 
-       /* Put LCR back to the normal mode */
+       /* Restore LCR and access to general register set */
        sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, lcr);
 
        return DIV_ROUND_CLOSEST(clk / 16, div);
@@ -1049,17 +1067,7 @@ static void sc16is7xx_set_termios(struct uart_port *port,
        if (!(termios->c_cflag & CREAD))
                port->ignore_status_mask |= SC16IS7XX_LSR_BRK_ERROR_MASK;
 
-       /* As above, claim the mutex while accessing the EFR. */
-       mutex_lock(&one->efr_lock);
-
-       sc16is7xx_port_write(port, SC16IS7XX_LCR_REG,
-                            SC16IS7XX_LCR_CONF_MODE_B);
-
        /* Configure flow control */
-       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]);
-
        port->status &= ~(UPSTAT_AUTOCTS | UPSTAT_AUTORTS);
        if (termios->c_cflag & CRTSCTS) {
                flow |= SC16IS7XX_EFR_AUTOCTS_BIT |
@@ -1071,16 +1079,16 @@ static void sc16is7xx_set_termios(struct uart_port *port,
        if (termios->c_iflag & IXOFF)
                flow |= SC16IS7XX_EFR_SWFLOW1_BIT;
 
-       sc16is7xx_port_update(port,
-                             SC16IS7XX_EFR_REG,
-                             SC16IS7XX_EFR_FLOWCTRL_BITS,
-                             flow);
-       regcache_cache_bypass(one->regmap, false);
-
        /* Update LCR register */
        sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, lcr);
 
-       mutex_unlock(&one->efr_lock);
+       /* Update EFR registers */
+       sc16is7xx_efr_lock(port);
+       sc16is7xx_port_write(port, SC16IS7XX_XON1_REG, termios->c_cc[VSTART]);
+       sc16is7xx_port_write(port, SC16IS7XX_XOFF1_REG, termios->c_cc[VSTOP]);
+       sc16is7xx_port_update(port, SC16IS7XX_EFR_REG,
+                             SC16IS7XX_EFR_FLOWCTRL_BITS, flow);
+       sc16is7xx_efr_unlock(port);
 
        /* Get baud rate generator configuration */
        baud = uart_get_baud_rate(port, termios, old,