Received-SPF: pass (google.com: domain of linux-kernel+bounces-30265-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: Hugo Villeneuve <hugo@hugovil.com>
To: gregkh@linuxfoundation.org,
	jirislaby@kernel.org,
	cosmin.tanislav@analog.com,
	andy.shevchenko@gmail.com,
	shc_work@mail.ru
Cc: linux-kernel@vger.kernel.org,
	linux-serial@vger.kernel.org,
	hugo@hugovil.com,
	Hugo Villeneuve <hvilleneuve@dimonoff.com>
Date: Thu, 18 Jan 2024 10:22:09 -0500
Message-Id: <20240118152213.2644269-14-hugo@hugovil.com>
In-Reply-To: <20240118152213.2644269-1-hugo@hugovil.com>
References: <20240118152213.2644269-1-hugo@hugovil.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: [PATCH v2 13/17] serial: max310x: use common detect function for all variants

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Simplify driver by defining a common function to handle the detection
of all variants.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 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