From: Hugo Villeneuve <[email protected]>
Hello,
this patch series brings a few clean-ups and improvements to the max310x
driver.
Some of these changes are based on suggestions for the sc16is7xx driver by
Andy Shevchenko following this discussion:
Link: https://lore.kernel.org/all/CAHp75VebCZckUrNraYQj9k=Mrn2kbYs1Lx26f5-8rKJ3RXeh-w@mail.gmail.com/
The changes have been tested on a custom board using a max14830 in SPI
mode, with an external oscillator (not crystal). Tests included a simple
communication test with a GPS connected to UART0.
They also have been tested by using i2c-stub to simulate the four ports on a
virtual I2C max14830 device, but with obvious limitations as this cannot
simulate all the hardware functions.
Thank you.
Link: [v1] https://lore.kernel.org/all/[email protected]
Changes for V2:
- Drop patch "serial: max310x: replace ENOTSUPP with preferred EOPNOTSUPP (checkpatch)"
- Fix cover letter typo
- Add Andy Shevchenko Suggested-by and Reviewed-by tags for selected patches
Hugo Villeneuve (17):
serial: max310x: fix NULL pointer dereference in I2C instantiation
serial: max310x: add I2C device table for instantiation from userspace
serial: max310x: use i2c_get_match_data()
serial: max310x: use spi_get_device_match_data()
serial: max310x: fix syntax error in IRQ error message
serial: max310x: remove holes in struct max310x_devtype
serial: max310x: add macro for max number of ports
serial: max310x: use separate regmap name for each port
serial: max310x: simplify probe() and remove() error handling
serial: max310x: add explicit return for some switch default cases
serial: max310x: use dev_err_probe() instead of dev_err()
serial: max310x: replace hardcoded masks with preferred GENMASK()
serial: max310x: use common detect function for all variants
serial: max310x: use common power function for all variants
serial: max310x: replace bare use of 'unsigned' with 'unsigned int'
(checkpatch)
serial: max310x: reformat and improve comments
serial: max310x: fix indentation
drivers/tty/serial/max310x.c | 327 ++++++++++++++++++-----------------
1 file changed, 164 insertions(+), 163 deletions(-)
base-commit: 0c84bea0cabc4e2b98a3de88eeb4ff798931f056
--
2.39.2
From: Hugo Villeneuve <[email protected]>
This allows to instantiate a max14830 I2C device from userspace.
Helpful when testing driver with i2c-stub.
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/max310x.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 4a33fd950ed2..053cf2458264 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1639,6 +1639,15 @@ static void max310x_i2c_remove(struct i2c_client *client)
max310x_remove(&client->dev);
}
+static const struct i2c_device_id max310x_i2c_id_table[] = {
+ { "max3107", (kernel_ulong_t)&max3107_devtype, },
+ { "max3108", (kernel_ulong_t)&max3108_devtype, },
+ { "max3109", (kernel_ulong_t)&max3109_devtype, },
+ { "max14830", (kernel_ulong_t)&max14830_devtype, },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, max310x_i2c_id_table);
+
static struct i2c_driver max310x_i2c_driver = {
.driver = {
.name = MAX310X_NAME,
@@ -1647,6 +1656,7 @@ static struct i2c_driver max310x_i2c_driver = {
},
.probe = max310x_i2c_probe,
.remove = max310x_i2c_remove,
+ .id_table = max310x_i2c_id_table,
};
#endif
--
2.39.2
From: Hugo Villeneuve <[email protected]>
When trying to instantiate a max14830 device from userspace:
echo max14830 0x60 > /sys/bus/i2c/devices/i2c-2/new_device
we get the following error:
Unable to handle kernel NULL pointer dereference at virtual address...
...
Call trace:
max310x_i2c_probe+0x48/0x170 [max310x]
i2c_device_probe+0x150/0x2a0
...
Add check for validity of devtype to prevent the error, and abort probe
with a meaningful error message.
Fixes: 2e1f2d9a9bdb ("serial: max310x: implement I2C support")
Cc: <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/max310x.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index f3a99daebdaa..4a33fd950ed2 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1602,13 +1602,16 @@ static unsigned short max310x_i2c_slave_addr(unsigned short addr,
static int max310x_i2c_probe(struct i2c_client *client)
{
- const struct max310x_devtype *devtype =
- device_get_match_data(&client->dev);
+ const struct max310x_devtype *devtype;
struct i2c_client *port_client;
struct regmap *regmaps[4];
unsigned int i;
u8 port_addr;
+ devtype = device_get_match_data(&client->dev);
+ if (!devtype)
+ return dev_err_probe(&client->dev, -ENODEV, "Failed to match device\n");
+
if (client->addr < devtype->slave_addr.min ||
client->addr > devtype->slave_addr.max)
return dev_err_probe(&client->dev, -EINVAL,
--
2.39.2
From: Hugo Villeneuve <[email protected]>
Use preferred i2c_get_match_data() instead of device_get_match_data()
to get the driver match data.
Suggested-by: Andy Shevchenko <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/max310x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 053cf2458264..a051bc773c4b 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1608,7 +1608,7 @@ static int max310x_i2c_probe(struct i2c_client *client)
unsigned int i;
u8 port_addr;
- devtype = device_get_match_data(&client->dev);
+ devtype = i2c_get_match_data(client);
if (!devtype)
return dev_err_probe(&client->dev, -ENODEV, "Failed to match device\n");
--
2.39.2
From: Hugo Villeneuve <[email protected]>
Use preferred spi_get_device_match_data() instead of
device_get_match_data() and spi_get_device_id() to get the driver match
data.
Suggested-by: Andy Shevchenko <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/max310x.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index a051bc773c4b..2314ec2afd3f 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1514,9 +1514,9 @@ static int max310x_spi_probe(struct spi_device *spi)
if (ret)
return ret;
- devtype = device_get_match_data(&spi->dev);
+ devtype = spi_get_device_match_data(spi);
if (!devtype)
- devtype = (struct max310x_devtype *)spi_get_device_id(spi)->driver_data;
+ return dev_err_probe(&spi->dev, -ENODEV, "Failed to match device\n");
for (i = 0; i < devtype->nr; i++) {
u8 port_mask = i * 0x20;
--
2.39.2
From: Hugo Villeneuve <[email protected]>
Replace g with q.
Helpful when grepping thru source code or logs for
"request" keyword.
Fixes: f65444187a66 ("serial: New serial driver MAX310X")
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/max310x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 2314ec2afd3f..27c8ec956691 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1428,7 +1428,7 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
if (!ret)
return 0;
- dev_err(dev, "Unable to reguest IRQ %i\n", irq);
+ dev_err(dev, "Unable to request IRQ %i\n", irq);
out_uart:
for (i = 0; i < devtype->nr; i++) {
--
2.39.2
From: Hugo Villeneuve <[email protected]>
Running pahole shows that there are some holes within the
max310x_devtype structure.
Remove holes and optimize alignment by reorganizing structure members.
This can also lead to data structure size reduction for some CPUs.
On 64-bit CPU (arm64):
Before:
/* size: 40, cachelines: 1, members: 6 */
/* sum members: 34, holes: 2, sum holes: 6 */
/* last cacheline: 40 bytes */
After:
/* size: 40, cachelines: 1, members: 6 */
/* padding: 6 */
/* last cacheline: 40 bytes */
On 32-bit CPU (i386):
Before:
/* size: 32, cachelines: 1, members: 6 */
/* sum members: 26, holes: 2, sum holes: 6 */
/* last cacheline: 32 bytes */
After:
/* size: 24, cachelines: 1, members: 8 */
/* padding: 2 */
/* last cacheline: 24 bytes */
Suggested-by: Andy Shevchenko <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/max310x.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 27c8ec956691..21f2fa3a91e5 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -258,11 +258,11 @@ struct max310x_devtype {
unsigned short min;
unsigned short max;
} slave_addr;
- char name[9];
int nr;
- u8 mode1;
int (*detect)(struct device *);
void (*power)(struct uart_port *, int);
+ char name[9];
+ u8 mode1;
};
struct max310x_one {
--
2.39.2
From: Hugo Villeneuve <[email protected]>
Add macro to hold the maximum number of UART ports per IC/device.
Reviewed-by: Andy Shevchenko <[email protected]>
Reviewed-by: Jan Kundrát <[email protected]>
Tested-by: Jan Kundrát <[email protected]>
Link: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/max310x.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 21f2fa3a91e5..6549eee4f6a6 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -30,6 +30,7 @@
#define MAX310X_MAJOR 204
#define MAX310X_MINOR 209
#define MAX310X_UART_NRMAX 16
+#define MAX310X_MAX_PORTS 4 /* Maximum number of UART ports per IC. */
/* MAX310X register definitions */
#define MAX310X_RHR_REG (0x00) /* RX FIFO */
@@ -1502,7 +1503,7 @@ static const struct max310x_if_cfg __maybe_unused max310x_spi_if_cfg = {
static int max310x_spi_probe(struct spi_device *spi)
{
const struct max310x_devtype *devtype;
- struct regmap *regmaps[4];
+ struct regmap *regmaps[MAX310X_MAX_PORTS];
unsigned int i;
int ret;
@@ -1604,7 +1605,7 @@ static int max310x_i2c_probe(struct i2c_client *client)
{
const struct max310x_devtype *devtype;
struct i2c_client *port_client;
- struct regmap *regmaps[4];
+ struct regmap *regmaps[MAX310X_MAX_PORTS];
unsigned int i;
u8 port_addr;
--
2.39.2
From: Hugo Villeneuve <[email protected]>
Use a separate regmap name for each port so they can each have their own
debugfs entry, allowing to access each port registers independently.
For example, a four channels/ports device like the MAX14830 will have four
entries in its regmap debugfs:
$ find /sys/kernel/debug/regmap -type d | grep spi0.0
/sys/kernel/debug/regmap/spi0.0-port0
/sys/kernel/debug/regmap/spi0.0-port1
/sys/kernel/debug/regmap/spi0.0-port2
/sys/kernel/debug/regmap/spi0.0-port3
Cc: Jan Kundrát <[email protected]>
Link: https://lore.kernel.org/all/[email protected]/
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
---
I didn't put again Jan Kundrát's Reviewed-by and Tested-by tags since this
version has modifications compared to the original one.
---
drivers/tty/serial/max310x.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 6549eee4f6a6..d6219077d23c 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1486,6 +1486,19 @@ static struct regmap_config regcfg = {
.max_raw_write = MAX310X_FIFO_SIZE,
};
+static const char *max310x_regmap_name(u8 port_id)
+{
+ switch (port_id) {
+ case 0: return "port0";
+ case 1: return "port1";
+ case 2: return "port2";
+ case 3: return "port3";
+ default:
+ WARN_ON(true);
+ return NULL;
+ }
+}
+
#ifdef CONFIG_SPI_MASTER
static int max310x_spi_extended_reg_enable(struct device *dev, bool enable)
{
@@ -1521,6 +1534,8 @@ static int max310x_spi_probe(struct spi_device *spi)
for (i = 0; i < devtype->nr; i++) {
u8 port_mask = i * 0x20;
+
+ regcfg.name = max310x_regmap_name(i);
regcfg.read_flag_mask = port_mask;
regcfg.write_flag_mask = port_mask | MAX310X_WRITE_BIT;
regmaps[i] = devm_regmap_init_spi(spi, ®cfg);
@@ -1620,6 +1635,7 @@ static int max310x_i2c_probe(struct i2c_client *client)
client->addr, devtype->slave_addr.min,
devtype->slave_addr.max);
+ regcfg_i2c.name = max310x_regmap_name(0);
regmaps[0] = devm_regmap_init_i2c(client, ®cfg_i2c);
for (i = 1; i < devtype->nr; i++) {
@@ -1628,6 +1644,7 @@ static int max310x_i2c_probe(struct i2c_client *client)
client->adapter,
port_addr);
+ regcfg_i2c.name = max310x_regmap_name(i);
regmaps[i] = devm_regmap_init_i2c(port_client, ®cfg_i2c);
}
--
2.39.2
From: Hugo Villeneuve <[email protected]>
Simplify error handling and only call uart_remove_one_port() if line bit
is set, instead of having to manually set s->p[i].port.dev to NULL.
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/max310x.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index d6219077d23c..9ef146f09d5b 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1395,10 +1395,9 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
/* Register port */
ret = uart_add_one_port(&max310x_uart, &s->p[i].port);
- if (ret) {
- s->p[i].port.dev = NULL;
+ if (ret)
goto out_uart;
- }
+
set_bit(line, max310x_lines);
/* Go to suspend mode */
@@ -1433,10 +1432,8 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
out_uart:
for (i = 0; i < devtype->nr; i++) {
- if (s->p[i].port.dev) {
+ if (test_and_clear_bit(s->p[i].port.line, max310x_lines))
uart_remove_one_port(&max310x_uart, &s->p[i].port);
- clear_bit(s->p[i].port.line, max310x_lines);
- }
}
out_clk:
@@ -1454,8 +1451,10 @@ static void max310x_remove(struct device *dev)
cancel_work_sync(&s->p[i].tx_work);
cancel_work_sync(&s->p[i].md_work);
cancel_work_sync(&s->p[i].rs_work);
- uart_remove_one_port(&max310x_uart, &s->p[i].port);
- clear_bit(s->p[i].port.line, max310x_lines);
+
+ if (test_and_clear_bit(s->p[i].port.line, max310x_lines))
+ uart_remove_one_port(&max310x_uart, &s->p[i].port);
+
s->devtype->power(&s->p[i].port, 0);
}
--
2.39.2
From: Hugo Villeneuve <[email protected]>
GENMASK() is preferred when defining bitmasks.
Of all the masks changed, only MAX310x_REV_MASK is actually used.
No functional change.
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/max310x.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 701bf54e4084..c93b326faf89 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -161,14 +161,14 @@
#define MAX310X_IRDA_SIR_BIT (1 << 1) /* SIR mode enable */
/* Flow control trigger level register masks */
-#define MAX310X_FLOWLVL_HALT_MASK (0x000f) /* Flow control halt level */
-#define MAX310X_FLOWLVL_RES_MASK (0x00f0) /* Flow control resume level */
+#define MAX310X_FLOWLVL_HALT_MASK GENMASK(3, 0) /* Flow control halt level */
+#define MAX310X_FLOWLVL_RES_MASK GENMASK(7, 4) /* Flow control resume level */
#define MAX310X_FLOWLVL_HALT(words) ((words / 8) & 0x0f)
#define MAX310X_FLOWLVL_RES(words) (((words / 8) & 0x0f) << 4)
/* FIFO interrupt trigger level register masks */
-#define MAX310X_FIFOTRIGLVL_TX_MASK (0x0f) /* TX FIFO trigger level */
-#define MAX310X_FIFOTRIGLVL_RX_MASK (0xf0) /* RX FIFO trigger level */
+#define MAX310X_FIFOTRIGLVL_TX_MASK GENMASK(3, 0) /* TX FIFO trigger level */
+#define MAX310X_FIFOTRIGLVL_RX_MASK GENMASK(7, 4) /* RX FIFO trigger level */
#define MAX310X_FIFOTRIGLVL_TX(words) ((words / 8) & 0x0f)
#define MAX310X_FIFOTRIGLVL_RX(words) (((words / 8) & 0x0f) << 4)
@@ -215,8 +215,8 @@
*/
/* PLL configuration register masks */
-#define MAX310X_PLLCFG_PREDIV_MASK (0x3f) /* PLL predivision value */
-#define MAX310X_PLLCFG_PLLFACTOR_MASK (0xc0) /* PLL multiplication factor */
+#define MAX310X_PLLCFG_PREDIV_MASK GENMASK(5, 0) /* PLL predivision value */
+#define MAX310X_PLLCFG_PLLFACTOR_MASK GENMASK(7, 6) /* PLL multiplication factor */
/* Baud rate generator configuration register bits */
#define MAX310X_BRGCFG_2XMODE_BIT (1 << 4) /* Double baud rate */
@@ -235,7 +235,7 @@
/* Misc definitions */
#define MAX310X_FIFO_SIZE (128)
-#define MAX310x_REV_MASK (0xf8)
+#define MAX310x_REV_MASK GENMASK(7, 3)
#define MAX310X_WRITE_BIT 0x80
/* MAX3107 specific */
--
2.39.2
From: Hugo Villeneuve <[email protected]>
Simplify driver by defining a common function to handle the detection
of all variants.
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/max310x.c | 134 ++++++++++++++---------------------
1 file changed, 54 insertions(+), 80 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index c93b326faf89..83beaab3a0c5 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -67,6 +67,7 @@
#define MAX310X_BRGDIVMSB_REG (0x1d) /* Baud rate divisor MSB */
#define MAX310X_CLKSRC_REG (0x1e) /* Clock source */
#define MAX310X_REG_1F (0x1f)
+#define MAX310X_EXTREG_START (0x20) /* Only relevant in SPI mode. */
#define MAX310X_REVID_REG MAX310X_REG_1F /* Revision ID */
@@ -74,9 +75,9 @@
#define MAX310X_GLOBALCMD_REG MAX310X_REG_1F /* Global Command (WO) */
/* Extended registers */
-#define MAX310X_SPI_REVID_EXTREG MAX310X_REG_05 /* Revision ID */
-#define MAX310X_I2C_REVID_EXTREG (0x25) /* Revision ID */
-
+#define MAX310X_REVID_EXTREG (0x25) /* Revision ID
+ * (extended addressing space)
+ */
/* IRQ register bits */
#define MAX310X_IRQ_LSR_BIT (1 << 0) /* LSR interrupt */
#define MAX310X_IRQ_SPCHR_BIT (1 << 1) /* Special char interrupt */
@@ -250,8 +251,7 @@
struct max310x_if_cfg {
int (*extended_reg_enable)(struct device *dev, bool enable);
-
- unsigned int rev_id_reg;
+ u8 rev_id_offset;
};
struct max310x_devtype {
@@ -260,10 +260,11 @@ struct max310x_devtype {
unsigned short max;
} slave_addr;
int nr;
- int (*detect)(struct device *);
void (*power)(struct uart_port *, int);
char name[9];
u8 mode1;
+ u8 rev_id_val;
+ u8 rev_id_reg; /* Relevant only if rev_id_val is defined. */
};
struct max310x_one {
@@ -324,62 +325,52 @@ static void max310x_port_update(struct uart_port *port, u8 reg, u8 mask, u8 val)
regmap_update_bits(one->regmap, reg, mask, val);
}
-static int max3107_detect(struct device *dev)
+static int max310x_detect(struct device *dev)
{
struct max310x_port *s = dev_get_drvdata(dev);
unsigned int val = 0;
int ret;
- ret = regmap_read(s->regmap, MAX310X_REVID_REG, &val);
- if (ret)
- return ret;
+ /* Check if variant supports REV ID register: */
+ if (s->devtype->rev_id_val) {
+ u8 rev_id_reg = s->devtype->rev_id_reg;
- if (((val & MAX310x_REV_MASK) != MAX3107_REV_ID)) {
- dev_err(dev,
- "%s ID 0x%02x does not match\n", s->devtype->name, val);
- return -ENODEV;
- }
+ /* Check if REV ID is in extended addressing space: */
+ if (s->devtype->rev_id_reg >= MAX310X_EXTREG_START) {
+ ret = s->if_cfg->extended_reg_enable(dev, true);
+ if (ret)
+ return ret;
- return 0;
-}
+ /* Adjust REV ID extended addressing space address: */
+ if (s->if_cfg->rev_id_offset)
+ rev_id_reg -= s->if_cfg->rev_id_offset;
+ }
-static int max3108_detect(struct device *dev)
-{
- struct max310x_port *s = dev_get_drvdata(dev);
- unsigned int val = 0;
- int ret;
+ regmap_read(s->regmap, rev_id_reg, &val);
- /* MAX3108 have not REV ID register, we just check default value
- * from clocksource register to make sure everything works.
- */
- ret = regmap_read(s->regmap, MAX310X_CLKSRC_REG, &val);
- if (ret)
- return ret;
+ if (s->devtype->rev_id_reg >= MAX310X_EXTREG_START) {
+ ret = s->if_cfg->extended_reg_enable(dev, false);
+ if (ret)
+ return ret;
+ }
- if (val != (MAX310X_CLKSRC_EXTCLK_BIT | MAX310X_CLKSRC_PLLBYP_BIT)) {
- dev_err(dev, "%s not present\n", s->devtype->name);
- return -ENODEV;
- }
+ if (((val & MAX310x_REV_MASK) != s->devtype->rev_id_val))
+ return dev_err_probe(dev, -ENODEV,
+ "%s ID 0x%02x does not match\n",
+ s->devtype->name, val);
+ } else {
+ /*
+ * For variant without REV ID register, just check default value
+ * from clocksource register to make sure everything works.
+ */
+ ret = regmap_read(s->regmap, MAX310X_CLKSRC_REG, &val);
+ if (ret)
+ return ret;
- return 0;
-}
-
-static int max3109_detect(struct device *dev)
-{
- struct max310x_port *s = dev_get_drvdata(dev);
- unsigned int val = 0;
- int ret;
-
- ret = s->if_cfg->extended_reg_enable(dev, true);
- if (ret)
- return ret;
-
- regmap_read(s->regmap, s->if_cfg->rev_id_reg, &val);
- s->if_cfg->extended_reg_enable(dev, false);
- if (((val & MAX310x_REV_MASK) != MAX3109_REV_ID)) {
- dev_err(dev,
- "%s ID 0x%02x does not match\n", s->devtype->name, val);
- return -ENODEV;
+ if (val != (MAX310X_CLKSRC_EXTCLK_BIT | MAX310X_CLKSRC_PLLBYP_BIT))
+ return dev_err_probe(dev, -ENODEV,
+ "%s not present\n",
+ s->devtype->name);
}
return 0;
@@ -394,27 +385,6 @@ static void max310x_power(struct uart_port *port, int on)
msleep(50);
}
-static int max14830_detect(struct device *dev)
-{
- struct max310x_port *s = dev_get_drvdata(dev);
- unsigned int val = 0;
- int ret;
-
- ret = s->if_cfg->extended_reg_enable(dev, true);
- if (ret)
- return ret;
-
- regmap_read(s->regmap, s->if_cfg->rev_id_reg, &val);
- s->if_cfg->extended_reg_enable(dev, false);
- if (((val & MAX310x_REV_MASK) != MAX14830_REV_ID)) {
- dev_err(dev,
- "%s ID 0x%02x does not match\n", s->devtype->name, val);
- return -ENODEV;
- }
-
- return 0;
-}
-
static void max14830_power(struct uart_port *port, int on)
{
max310x_port_update(port, MAX310X_BRGCFG_REG,
@@ -428,7 +398,8 @@ static const struct max310x_devtype max3107_devtype = {
.name = "MAX3107",
.nr = 1,
.mode1 = MAX310X_MODE1_AUTOSLEEP_BIT | MAX310X_MODE1_IRQSEL_BIT,
- .detect = max3107_detect,
+ .rev_id_val = MAX3107_REV_ID,
+ .rev_id_reg = MAX310X_REVID_REG,
.power = max310x_power,
.slave_addr = {
.min = 0x2c,
@@ -440,7 +411,8 @@ static const struct max310x_devtype max3108_devtype = {
.name = "MAX3108",
.nr = 1,
.mode1 = MAX310X_MODE1_AUTOSLEEP_BIT,
- .detect = max3108_detect,
+ .rev_id_val = 0, /* Unsupported. */
+ .rev_id_reg = 0, /* Irrelevant when rev_id_val is not defined. */
.power = max310x_power,
.slave_addr = {
.min = 0x60,
@@ -452,7 +424,8 @@ static const struct max310x_devtype max3109_devtype = {
.name = "MAX3109",
.nr = 2,
.mode1 = MAX310X_MODE1_AUTOSLEEP_BIT,
- .detect = max3109_detect,
+ .rev_id_val = MAX3109_REV_ID,
+ .rev_id_reg = MAX310X_REVID_EXTREG,
.power = max310x_power,
.slave_addr = {
.min = 0x60,
@@ -464,7 +437,8 @@ static const struct max310x_devtype max14830_devtype = {
.name = "MAX14830",
.nr = 4,
.mode1 = MAX310X_MODE1_IRQSEL_BIT,
- .detect = max14830_detect,
+ .rev_id_val = MAX14830_REV_ID,
+ .rev_id_reg = MAX310X_REVID_EXTREG,
.power = max14830_power,
.slave_addr = {
.min = 0x60,
@@ -1322,7 +1296,7 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
dev_set_drvdata(dev, s);
/* Check device to ensure we are talking to what we expect */
- ret = devtype->detect(dev);
+ ret = max310x_detect(dev);
if (ret)
goto out_clk;
@@ -1501,7 +1475,7 @@ static int max310x_spi_extended_reg_enable(struct device *dev, bool enable)
static const struct max310x_if_cfg __maybe_unused max310x_spi_if_cfg = {
.extended_reg_enable = max310x_spi_extended_reg_enable,
- .rev_id_reg = MAX310X_SPI_REVID_EXTREG,
+ .rev_id_offset = MAX310X_EXTREG_START,
};
static int max310x_spi_probe(struct spi_device *spi)
@@ -1574,7 +1548,7 @@ static struct regmap_config regcfg_i2c = {
.writeable_reg = max310x_reg_writeable,
.volatile_reg = max310x_reg_volatile,
.precious_reg = max310x_reg_precious,
- .max_register = MAX310X_I2C_REVID_EXTREG,
+ .max_register = MAX310X_REVID_EXTREG,
.writeable_noinc_reg = max310x_reg_noinc,
.readable_noinc_reg = max310x_reg_noinc,
.max_raw_read = MAX310X_FIFO_SIZE,
@@ -1583,7 +1557,7 @@ static struct regmap_config regcfg_i2c = {
static const struct max310x_if_cfg max310x_i2c_if_cfg = {
.extended_reg_enable = max310x_i2c_extended_reg_enable,
- .rev_id_reg = MAX310X_I2C_REVID_EXTREG,
+ .rev_id_offset = 0, /* No offset in I2C mode. */
};
static unsigned short max310x_i2c_slave_addr(unsigned short addr,
--
2.39.2
From: Hugo Villeneuve <[email protected]>
Fixes the following checkpatch warnings:
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
With this change, the affected functions now match the prototypes in
struct gpio_chip.
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/max310x.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index e39d8ea51e4e..9faea1224a58 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1156,7 +1156,7 @@ static int __maybe_unused max310x_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(max310x_pm_ops, max310x_suspend, max310x_resume);
#ifdef CONFIG_GPIOLIB
-static int max310x_gpio_get(struct gpio_chip *chip, unsigned offset)
+static int max310x_gpio_get(struct gpio_chip *chip, unsigned int offset)
{
unsigned int val;
struct max310x_port *s = gpiochip_get_data(chip);
@@ -1167,7 +1167,7 @@ static int max310x_gpio_get(struct gpio_chip *chip, unsigned offset)
return !!((val >> 4) & (1 << (offset % 4)));
}
-static void max310x_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+static void max310x_gpio_set(struct gpio_chip *chip, unsigned int offset, int value)
{
struct max310x_port *s = gpiochip_get_data(chip);
struct uart_port *port = &s->p[offset / 4].port;
@@ -1176,7 +1176,7 @@ static void max310x_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
value ? 1 << (offset % 4) : 0);
}
-static int max310x_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+static int max310x_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
{
struct max310x_port *s = gpiochip_get_data(chip);
struct uart_port *port = &s->p[offset / 4].port;
@@ -1187,7 +1187,7 @@ static int max310x_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
}
static int max310x_gpio_direction_output(struct gpio_chip *chip,
- unsigned offset, int value)
+ unsigned int offset, int value)
{
struct max310x_port *s = gpiochip_get_data(chip);
struct uart_port *port = &s->p[offset / 4].port;
--
2.39.2
From: Hugo Villeneuve <[email protected]>
Add comments about I2C slave address structure, and reformat to
improve readability.
Also reformat some comments according to kernel coding style.
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/max310x.c | 40 ++++++++++++++++++++++--------------
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 9faea1224a58..37007b25fbee 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -179,7 +179,8 @@
#define MAX310X_FLOWCTRL_GPIADDR_BIT (1 << 2) /* Enables that GPIO inputs
* are used in conjunction with
* XOFF2 for definition of
- * special character */
+ * special character
+ */
#define MAX310X_FLOWCTRL_SWFLOWEN_BIT (1 << 3) /* Auto SW flow ctrl enable */
#define MAX310X_FLOWCTRL_SWFLOW0_BIT (1 << 4) /* SWFLOW bit 0 */
#define MAX310X_FLOWCTRL_SWFLOW1_BIT (1 << 5) /* SWFLOW bit 1
@@ -258,7 +259,7 @@ struct max310x_devtype {
struct {
unsigned short min;
unsigned short max;
- } slave_addr;
+ } slave_addr; /* Relevant only in I2C mode. */
int nr;
char name[9];
u8 mode1;
@@ -639,7 +640,8 @@ static void max310x_handle_rx(struct uart_port *port, unsigned int rxlen)
u8 ch, flag;
if (port->read_status_mask == MAX310X_LSR_RXOVR_BIT) {
- /* We are just reading, happily ignoring any error conditions.
+ /*
+ * We are just reading, happily ignoring any error conditions.
* Break condition, parity checking, framing errors -- they
* are all ignored. That means that we can do a batch-read.
*
@@ -648,7 +650,7 @@ static void max310x_handle_rx(struct uart_port *port, unsigned int rxlen)
* that the LSR register applies to the "current" character.
* That's also the reason why we cannot do batched reads when
* asked to check the individual statuses.
- * */
+ */
sts = max310x_port_read(port, MAX310X_LSR_IRQSTS_REG);
max310x_batch_read(port, one->rx_buf, rxlen);
@@ -752,8 +754,10 @@ static void max310x_handle_tx(struct uart_port *port)
to_send = (to_send > txlen) ? txlen : to_send;
if (until_end < to_send) {
- /* It's a circ buffer -- wrap around.
- * We could do that in one SPI transaction, but meh. */
+ /*
+ * It's a circ buffer -- wrap around.
+ * We could do that in one SPI transaction, but meh.
+ */
max310x_batch_write(port, xmit->buf + xmit->tail, until_end);
max310x_batch_write(port, xmit->buf, to_send - until_end);
} else {
@@ -842,7 +846,8 @@ static unsigned int max310x_tx_empty(struct uart_port *port)
static unsigned int max310x_get_mctrl(struct uart_port *port)
{
- /* DCD and DSR are not wired and CTS/RTS is handled automatically
+ /*
+ * DCD and DSR are not wired and CTS/RTS is handled automatically
* so just indicate DSR and CAR asserted
*/
return TIOCM_DSR | TIOCM_CAR;
@@ -934,7 +939,8 @@ static void max310x_set_termios(struct uart_port *port,
max310x_port_write(port, MAX310X_XON1_REG, termios->c_cc[VSTART]);
max310x_port_write(port, MAX310X_XOFF1_REG, termios->c_cc[VSTOP]);
- /* Disable transmitter before enabling AutoCTS or auto transmitter
+ /*
+ * Disable transmitter before enabling AutoCTS or auto transmitter
* flow control
*/
if (termios->c_cflag & CRTSCTS || termios->c_iflag & IXOFF) {
@@ -961,7 +967,8 @@ static void max310x_set_termios(struct uart_port *port,
}
max310x_port_write(port, MAX310X_FLOWCTRL_REG, flow);
- /* Enable transmitter after disabling AutoCTS and auto transmitter
+ /*
+ * Enable transmitter after disabling AutoCTS and auto transmitter
* flow control
*/
if (!(termios->c_cflag & CRTSCTS) && !(termios->c_iflag & IXOFF)) {
@@ -1052,8 +1059,11 @@ static int max310x_startup(struct uart_port *port)
MAX310X_MODE2_ECHOSUPR_BIT);
}
- /* Configure flow control levels */
- /* Flow control halt level 96, resume level 48 */
+ /*
+ * Configure flow control levels:
+ * resume: 48
+ * halt: 96
+ */
max310x_port_write(port, MAX310X_FLOWLVL_REG,
MAX310X_FLOWLVL_RES(48) | MAX310X_FLOWLVL_HALT(96));
@@ -1561,10 +1571,10 @@ static unsigned short max310x_i2c_slave_addr(unsigned short addr,
* For MAX14830 and MAX3109, the slave address depends on what the
* A0 and A1 pins are tied to.
* See Table I2C Address Map of the datasheet.
- * Based on that table, the following formulas were determined.
- * UART1 - UART0 = 0x10
- * UART2 - UART1 = 0x20 + 0x10
- * UART3 - UART2 = 0x10
+ * Based on that table, the following formulas were determined:
+ * UART1 - UART0 = 0x10
+ * UART2 - UART1 = 0x20 + 0x10
+ * UART3 - UART2 = 0x10
*/
addr -= nr * 0x10;
--
2.39.2
From: Hugo Villeneuve <[email protected]>
Simplify driver by defining a common function to handle the power
control of all variants.
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/max310x.c | 44 ++++++++++++++++--------------------
1 file changed, 19 insertions(+), 25 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 83beaab3a0c5..e39d8ea51e4e 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -260,11 +260,12 @@ struct max310x_devtype {
unsigned short max;
} slave_addr;
int nr;
- void (*power)(struct uart_port *, int);
char name[9];
u8 mode1;
u8 rev_id_val;
u8 rev_id_reg; /* Relevant only if rev_id_val is defined. */
+ u8 power_reg; /* Register address for power/sleep control. */
+ u8 power_bit; /* Bit for sleep or power-off mode (active high). */
};
struct max310x_one {
@@ -378,18 +379,10 @@ static int max310x_detect(struct device *dev)
static void max310x_power(struct uart_port *port, int on)
{
- max310x_port_update(port, MAX310X_MODE1_REG,
- MAX310X_MODE1_FORCESLEEP_BIT,
- on ? 0 : MAX310X_MODE1_FORCESLEEP_BIT);
- if (on)
- msleep(50);
-}
+ struct max310x_port *s = dev_get_drvdata(port->dev);
-static void max14830_power(struct uart_port *port, int on)
-{
- max310x_port_update(port, MAX310X_BRGCFG_REG,
- MAX14830_BRGCFG_CLKDIS_BIT,
- on ? 0 : MAX14830_BRGCFG_CLKDIS_BIT);
+ max310x_port_update(port, s->devtype->power_reg, s->devtype->power_bit,
+ on ? 0 : s->devtype->power_bit);
if (on)
msleep(50);
}
@@ -400,7 +393,8 @@ static const struct max310x_devtype max3107_devtype = {
.mode1 = MAX310X_MODE1_AUTOSLEEP_BIT | MAX310X_MODE1_IRQSEL_BIT,
.rev_id_val = MAX3107_REV_ID,
.rev_id_reg = MAX310X_REVID_REG,
- .power = max310x_power,
+ .power_reg = MAX310X_MODE1_REG,
+ .power_bit = MAX310X_MODE1_FORCESLEEP_BIT,
.slave_addr = {
.min = 0x2c,
.max = 0x2f,
@@ -413,7 +407,8 @@ static const struct max310x_devtype max3108_devtype = {
.mode1 = MAX310X_MODE1_AUTOSLEEP_BIT,
.rev_id_val = 0, /* Unsupported. */
.rev_id_reg = 0, /* Irrelevant when rev_id_val is not defined. */
- .power = max310x_power,
+ .power_reg = MAX310X_MODE1_REG,
+ .power_bit = MAX310X_MODE1_FORCESLEEP_BIT,
.slave_addr = {
.min = 0x60,
.max = 0x6f,
@@ -426,7 +421,8 @@ static const struct max310x_devtype max3109_devtype = {
.mode1 = MAX310X_MODE1_AUTOSLEEP_BIT,
.rev_id_val = MAX3109_REV_ID,
.rev_id_reg = MAX310X_REVID_EXTREG,
- .power = max310x_power,
+ .power_reg = MAX310X_MODE1_REG,
+ .power_bit = MAX310X_MODE1_FORCESLEEP_BIT,
.slave_addr = {
.min = 0x60,
.max = 0x6f,
@@ -439,7 +435,8 @@ static const struct max310x_devtype max14830_devtype = {
.mode1 = MAX310X_MODE1_IRQSEL_BIT,
.rev_id_val = MAX14830_REV_ID,
.rev_id_reg = MAX310X_REVID_EXTREG,
- .power = max14830_power,
+ .power_reg = MAX310X_BRGCFG_REG,
+ .power_bit = MAX14830_BRGCFG_CLKDIS_BIT,
.slave_addr = {
.min = 0x60,
.max = 0x6f,
@@ -1025,10 +1022,9 @@ static int max310x_rs485_config(struct uart_port *port, struct ktermios *termios
static int max310x_startup(struct uart_port *port)
{
- struct max310x_port *s = dev_get_drvdata(port->dev);
unsigned int val;
- s->devtype->power(port, 1);
+ max310x_power(port, 1);
/* Configure MODE1 register */
max310x_port_update(port, MAX310X_MODE1_REG,
@@ -1073,12 +1069,10 @@ static int max310x_startup(struct uart_port *port)
static void max310x_shutdown(struct uart_port *port)
{
- struct max310x_port *s = dev_get_drvdata(port->dev);
-
/* Disable all interrupts */
max310x_port_write(port, MAX310X_IRQEN_REG, 0);
- s->devtype->power(port, 0);
+ max310x_power(port, 0);
}
static const char *max310x_type(struct uart_port *port)
@@ -1140,7 +1134,7 @@ static int __maybe_unused max310x_suspend(struct device *dev)
for (i = 0; i < s->devtype->nr; i++) {
uart_suspend_port(&max310x_uart, &s->p[i].port);
- s->devtype->power(&s->p[i].port, 0);
+ max310x_power(&s->p[i].port, 0);
}
return 0;
@@ -1152,7 +1146,7 @@ static int __maybe_unused max310x_resume(struct device *dev)
int i;
for (i = 0; i < s->devtype->nr; i++) {
- s->devtype->power(&s->p[i].port, 1);
+ max310x_power(&s->p[i].port, 1);
uart_resume_port(&max310x_uart, &s->p[i].port);
}
@@ -1367,7 +1361,7 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
set_bit(line, max310x_lines);
/* Go to suspend mode */
- devtype->power(&s->p[i].port, 0);
+ max310x_power(&s->p[i].port, 0);
}
#ifdef CONFIG_GPIOLIB
@@ -1421,7 +1415,7 @@ static void max310x_remove(struct device *dev)
if (test_and_clear_bit(s->p[i].port.line, max310x_lines))
uart_remove_one_port(&max310x_uart, &s->p[i].port);
- s->devtype->power(&s->p[i].port, 0);
+ max310x_power(&s->p[i].port, 0);
}
clk_disable_unprepare(s->clk);
--
2.39.2
From: Hugo Villeneuve <[email protected]>
Allows to simplify code by removing the break statement in the default
switch/case in some functions.
Suggested-by: Andy Shevchenko <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/max310x.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 9ef146f09d5b..048ae432ba48 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -483,10 +483,8 @@ static bool max310x_reg_writeable(struct device *dev, unsigned int reg)
case MAX310X_RXFIFOLVL_REG:
return false;
default:
- break;
+ return true;
}
-
- return true;
}
static bool max310x_reg_volatile(struct device *dev, unsigned int reg)
@@ -505,10 +503,8 @@ static bool max310x_reg_volatile(struct device *dev, unsigned int reg)
case MAX310X_REG_1F:
return true;
default:
- break;
+ return false;
}
-
- return false;
}
static bool max310x_reg_precious(struct device *dev, unsigned int reg)
@@ -520,10 +516,8 @@ static bool max310x_reg_precious(struct device *dev, unsigned int reg)
case MAX310X_STS_IRQSTS_REG:
return true;
default:
- break;
+ return false;
}
-
- return false;
}
static bool max310x_reg_noinc(struct device *dev, unsigned int reg)
--
2.39.2
From: Hugo Villeneuve <[email protected]>
Replace dev_err() with dev_err_probe().
This helps in simplifing code and standardizing the error output.
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/max310x.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 048ae432ba48..701bf54e4084 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1275,10 +1275,9 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
/* Alloc port structure */
s = devm_kzalloc(dev, struct_size(s, p, devtype->nr), GFP_KERNEL);
- if (!s) {
- dev_err(dev, "Error allocating port structure\n");
- return -ENOMEM;
- }
+ if (!s)
+ return dev_err_probe(dev, -ENOMEM,
+ "Error allocating port structure\n");
/* Always ask for fixed clock rate from a property. */
device_property_read_u32(dev, "clock-frequency", &uartclk);
@@ -1299,8 +1298,7 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
if (freq == 0)
freq = uartclk;
if (freq == 0) {
- dev_err(dev, "Cannot get clock rate\n");
- ret = -EINVAL;
+ ret = dev_err_probe(dev, -EINVAL, "Cannot get clock rate\n");
goto out_clk;
}
--
2.39.2
On Thu, Jan 18, 2024 at 5:22 PM Hugo Villeneuve <[email protected]> wrote:
>
> From: Hugo Villeneuve <[email protected]>
>
> Use a separate regmap name for each port so they can each have their own
> debugfs entry, allowing to access each port registers independently.
>
> For example, a four channels/ports device like the MAX14830 will have four
> entries in its regmap debugfs:
>
> $ find /sys/kernel/debug/regmap -type d | grep spi0.0
Just a side note for the future. The above is an example of "Useless
use of grep".
`find` has a `-name` parameter for such, using `find ...-name
'spi0.0*'` makes the grep unnecessary.
> /sys/kernel/debug/regmap/spi0.0-port0
> /sys/kernel/debug/regmap/spi0.0-port1
> /sys/kernel/debug/regmap/spi0.0-port2
> /sys/kernel/debug/regmap/spi0.0-port3
--
With Best Regards,
Andy Shevchenko
From: Hugo Villeneuve <[email protected]>
Fix indentation and add line after do/while() block.
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/max310x.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 37007b25fbee..0065e17572be 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -802,6 +802,7 @@ static irqreturn_t max310x_port_irq(struct max310x_port *s, int portno)
if (ists & MAX310X_IRQ_TXEMPTY_BIT)
max310x_start_tx(port);
} while (1);
+
return res;
}
@@ -1598,7 +1599,7 @@ static int max310x_i2c_probe(struct i2c_client *client)
return dev_err_probe(&client->dev, -ENODEV, "Failed to match device\n");
if (client->addr < devtype->slave_addr.min ||
- client->addr > devtype->slave_addr.max)
+ client->addr > devtype->slave_addr.max)
return dev_err_probe(&client->dev, -EINVAL,
"Slave addr 0x%x outside of range [0x%x, 0x%x]\n",
client->addr, devtype->slave_addr.min,
--
2.39.2
On Thu, 18 Jan 2024 17:30:16 +0200
Andy Shevchenko <[email protected]> wrote:
> On Thu, Jan 18, 2024 at 5:22 PM Hugo Villeneuve <[email protected]> wrote:
> >
> > From: Hugo Villeneuve <[email protected]>
> >
> > Use a separate regmap name for each port so they can each have their own
> > debugfs entry, allowing to access each port registers independently.
> >
> > For example, a four channels/ports device like the MAX14830 will have four
> > entries in its regmap debugfs:
> >
> > $ find /sys/kernel/debug/regmap -type d | grep spi0.0
>
> Just a side note for the future. The above is an example of "Useless
> use of grep".
> `find` has a `-name` parameter for such, using `find ...-name
> 'spi0.0*'` makes the grep unnecessary.
Hi,
yes, I fixed a similar one in the sc16is7xx serie, but forgot to update
this one :)
Will fix it if sending V3.
Hugo Villeneuve
>
> > /sys/kernel/debug/regmap/spi0.0-port0
> > /sys/kernel/debug/regmap/spi0.0-port1
> > /sys/kernel/debug/regmap/spi0.0-port2
> > /sys/kernel/debug/regmap/spi0.0-port3
>
> --
> With Best Regards,
> Andy Shevchenko