After upgrading from 5.16 to 6.1, our board with a MAX14830 started producing lots of garbage data over UART. Bisection pointed out commit 285e76fc049c as the culprit. That patch tried to replace hand-written code which I added in 2b4bac48c1084 ("serial: max310x: Use batched reads when reasonably safe") with the generic regmap infrastructure for batched operations.
Unfortunately, the `regmap_raw_read` and `regmap_raw_write` which were used are actually functions which perform IO over *multiple* registers. That's not what is needed for accessing these Tx/Rx FIFOs; the appropriate functions are the `_noinc_` versions, not the `_raw_` ones.
Fix this regression by using `regmap_noinc_read()` and `regmap_noinc_write()` along with the necessary `regmap_config` setup; with this patch in place, our board communicates happily again. Since our board uses SPI for talking to this chip, the I2C part is completely untested.
Fixes: 285e76fc049c serial: max310x: use regmap methods for SPI batch operations Signed-off-by: Jan Kundrát jan.kundrat@cesnet.cz Cc: stable@vger.kernel.org --- drivers/tty/serial/max310x.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c index c82391c928cb..47520d4a381f 100644 --- a/drivers/tty/serial/max310x.c +++ b/drivers/tty/serial/max310x.c @@ -529,6 +529,11 @@ static bool max310x_reg_precious(struct device *dev, unsigned int reg) return false; }
+static bool max310x_reg_noinc(struct device *dev, unsigned int reg) +{ + return reg == MAX310X_RHR_REG; +} + static int max310x_set_baud(struct uart_port *port, int baud) { unsigned int mode = 0, div = 0, frac = 0, c = 0, F = 0; @@ -658,14 +663,14 @@ static void max310x_batch_write(struct uart_port *port, u8 *txbuf, unsigned int { struct max310x_one *one = to_max310x_port(port);
- regmap_raw_write(one->regmap, MAX310X_THR_REG, txbuf, len); + regmap_noinc_write(one->regmap, MAX310X_THR_REG, txbuf, len); }
static void max310x_batch_read(struct uart_port *port, u8 *rxbuf, unsigned int len) { struct max310x_one *one = to_max310x_port(port);
- regmap_raw_read(one->regmap, MAX310X_RHR_REG, rxbuf, len); + regmap_noinc_read(one->regmap, MAX310X_RHR_REG, rxbuf, len); }
static void max310x_handle_rx(struct uart_port *port, unsigned int rxlen) @@ -1480,6 +1485,10 @@ static struct regmap_config regcfg = { .writeable_reg = max310x_reg_writeable, .volatile_reg = max310x_reg_volatile, .precious_reg = max310x_reg_precious, + .writeable_noinc_reg = max310x_reg_noinc, + .readable_noinc_reg = max310x_reg_noinc, + .max_raw_read = MAX310X_FIFO_SIZE, + .max_raw_write = MAX310X_FIFO_SIZE, };
#ifdef CONFIG_SPI_MASTER @@ -1565,6 +1574,10 @@ static struct regmap_config regcfg_i2c = { .volatile_reg = max310x_reg_volatile, .precious_reg = max310x_reg_precious, .max_register = MAX310X_I2C_REVID_EXTREG, + .writeable_noinc_reg = max310x_reg_noinc, + .readable_noinc_reg = max310x_reg_noinc, + .max_raw_read = MAX310X_FIFO_SIZE, + .max_raw_write = MAX310X_FIFO_SIZE, };
static const struct max310x_if_cfg max310x_i2c_if_cfg = {
On Thu, Apr 6, 2023 at 1:08 AM Jan Kundrát jan.kundrat@cesnet.cz wrote:
After upgrading from 5.16 to 6.1, our board with a MAX14830 started producing lots of garbage data over UART. Bisection pointed out commit 285e76fc049c as the culprit. That patch tried to replace hand-written code which I added in 2b4bac48c1084 ("serial: max310x: Use batched reads when reasonably safe") with the generic regmap infrastructure for batched operations.
Unfortunately, the `regmap_raw_read` and `regmap_raw_write` which were used are actually functions which perform IO over *multiple* registers. That's not what is needed for accessing these Tx/Rx FIFOs; the appropriate functions are the `_noinc_` versions, not the `_raw_` ones.
Fix this regression by using `regmap_noinc_read()` and `regmap_noinc_write()` along with the necessary `regmap_config` setup; with this patch in place, our board communicates happily again. Since our board uses SPI for talking to this chip, the I2C part is completely untested.
Make sense, thanks for fixing this!
Reviewed-by: Andy Shevchenko andy.shevchenko@gmail.com
Fixes: 285e76fc049c serial: max310x: use regmap methods for SPI batch operations Signed-off-by: Jan Kundrát jan.kundrat@cesnet.cz Cc: stable@vger.kernel.org
drivers/tty/serial/max310x.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c index c82391c928cb..47520d4a381f 100644 --- a/drivers/tty/serial/max310x.c +++ b/drivers/tty/serial/max310x.c @@ -529,6 +529,11 @@ static bool max310x_reg_precious(struct device *dev, unsigned int reg) return false; }
+static bool max310x_reg_noinc(struct device *dev, unsigned int reg) +{
return reg == MAX310X_RHR_REG;+}
static int max310x_set_baud(struct uart_port *port, int baud) { unsigned int mode = 0, div = 0, frac = 0, c = 0, F = 0; @@ -658,14 +663,14 @@ static void max310x_batch_write(struct uart_port *port, u8 *txbuf, unsigned int { struct max310x_one *one = to_max310x_port(port);
regmap_raw_write(one->regmap, MAX310X_THR_REG, txbuf, len);
regmap_noinc_write(one->regmap, MAX310X_THR_REG, txbuf, len);}
static void max310x_batch_read(struct uart_port *port, u8 *rxbuf, unsigned int len) { struct max310x_one *one = to_max310x_port(port);
regmap_raw_read(one->regmap, MAX310X_RHR_REG, rxbuf, len);
regmap_noinc_read(one->regmap, MAX310X_RHR_REG, rxbuf, len);}
static void max310x_handle_rx(struct uart_port *port, unsigned int rxlen) @@ -1480,6 +1485,10 @@ static struct regmap_config regcfg = { .writeable_reg = max310x_reg_writeable, .volatile_reg = max310x_reg_volatile, .precious_reg = max310x_reg_precious,
.writeable_noinc_reg = max310x_reg_noinc,.readable_noinc_reg = max310x_reg_noinc,.max_raw_read = MAX310X_FIFO_SIZE,.max_raw_write = MAX310X_FIFO_SIZE,};
#ifdef CONFIG_SPI_MASTER @@ -1565,6 +1574,10 @@ static struct regmap_config regcfg_i2c = { .volatile_reg = max310x_reg_volatile, .precious_reg = max310x_reg_precious, .max_register = MAX310X_I2C_REVID_EXTREG,
.writeable_noinc_reg = max310x_reg_noinc,.readable_noinc_reg = max310x_reg_noinc,.max_raw_read = MAX310X_FIFO_SIZE,.max_raw_write = MAX310X_FIFO_SIZE,};
static const struct max310x_if_cfg max310x_i2c_if_cfg = {
2.39.2
On Wed, 5 Apr 2023, Jan Kundrát wrote:
After upgrading from 5.16 to 6.1, our board with a MAX14830 started producing lots of garbage data over UART. Bisection pointed out commit 285e76fc049c as the culprit. That patch tried to replace hand-written code which I added in 2b4bac48c1084 ("serial: max310x: Use batched reads when reasonably safe") with the generic regmap infrastructure for batched operations.
Unfortunately, the `regmap_raw_read` and `regmap_raw_write` which were used are actually functions which perform IO over *multiple* registers. That's not what is needed for accessing these Tx/Rx FIFOs; the appropriate functions are the `_noinc_` versions, not the `_raw_` ones.
Fix this regression by using `regmap_noinc_read()` and `regmap_noinc_write()` along with the necessary `regmap_config` setup; with this patch in place, our board communicates happily again. Since our board uses SPI for talking to this chip, the I2C part is completely untested.
Fixes: 285e76fc049c serial: max310x: use regmap methods for SPI batch operations Signed-off-by: Jan Kundrát jan.kundrat@cesnet.cz Cc: stable@vger.kernel.org
drivers/tty/serial/max310x.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c index c82391c928cb..47520d4a381f 100644 --- a/drivers/tty/serial/max310x.c +++ b/drivers/tty/serial/max310x.c @@ -529,6 +529,11 @@ static bool max310x_reg_precious(struct device *dev, unsigned int reg) return false; } +static bool max310x_reg_noinc(struct device *dev, unsigned int reg) +{
- return reg == MAX310X_RHR_REG;
I'd either add a comment or just do || reg == MAX310X_THR_REG (which probably gets optimized away by the compiler) to make it look less magic for the Tx side.
On Wed, Apr 05, 2023 at 10:14:23PM +0200, Jan Kundrát wrote:
After upgrading from 5.16 to 6.1, our board with a MAX14830 started producing lots of garbage data over UART. Bisection pointed out commit 285e76fc049c as the culprit. That patch tried to replace hand-written code which I added in 2b4bac48c1084 ("serial: max310x: Use batched reads when reasonably safe") with the generic regmap infrastructure for batched operations.
Unfortunately, the `regmap_raw_read` and `regmap_raw_write` which were used are actually functions which perform IO over *multiple* registers. That's not what is needed for accessing these Tx/Rx FIFOs; the appropriate functions are the `_noinc_` versions, not the `_raw_` ones.
Fix this regression by using `regmap_noinc_read()` and `regmap_noinc_write()` along with the necessary `regmap_config` setup; with this patch in place, our board communicates happily again. Since our board uses SPI for talking to this chip, the I2C part is completely untested.
Fixes: 285e76fc049c serial: max310x: use regmap methods for SPI batch operations
Nit, please use the style that the documentation asks for here, which should look like:
Fixes: 285e76fc049c ("serial: max310x: use regmap methods for SPI batch operations")
otherwise our tools complain :( I'll go fix this up by hand...
greg k-h
linux-stable-mirror@lists.linaro.org
-
Andy Shevchenko -
Greg Kroah-Hartman -
Ilpo Järvinen -
Jan Kundrát