2022-05-31 18:22:55

by Cosmin Tanislav

[permalink] [raw]
Subject: [PATCH 4/4] serial: max310x: implement I2C support

From: Cosmin Tanislav <[email protected]>

I2C implementation on this chip has a few key differences
compared to SPI, as described in previous patches.
* extended register space access needs no extra logic
* slave address is used to select which UART to communicate
with

To accommodate these differences, add an I2C interface config,
set the RevID register address and implement an empty method
for setting the GlobalCommand register, since no special handling
is needed for the extended register space.

To handle the port-specific slave address, create an I2C dummy
device for each port, except the base one (UART0), which is
expected to be the one specified in firmware, and create a
regmap for each I2C device.
Add minimum and maximum slave addresses to each devtype for
sanity checking.

Also, use a separate regmap config with no write_flag_mask,
since I2C has a R/W bit in its slave address, and set the
max register to the address of the RevID register, since the
extended register space needs no extra logic.

Finally, add the I2C driver.

Signed-off-by: Cosmin Tanislav <[email protected]>
---
drivers/tty/serial/Kconfig | 1 +
drivers/tty/serial/max310x.c | 131 ++++++++++++++++++++++++++++++++++-
2 files changed, 131 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 80321b9c4465..a397c8607647 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -324,6 +324,7 @@ config SERIAL_MAX310X
depends on SPI_MASTER
select SERIAL_CORE
select REGMAP_SPI if SPI_MASTER
+ select REGMAP_I2C if I2C
help
This selects support for an advanced UART from Maxim (Dallas).
Supported ICs are MAX3107, MAX3108, MAX3109, MAX14830.
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 0cbe7cb1ad26..0ebb723dd241 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -14,6 +14,7 @@
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/gpio/driver.h>
+#include <linux/i2c.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
#include <linux/property.h>
@@ -73,6 +74,7 @@

/* Extended registers */
#define MAX310X_SPI_REVID_EXTREG MAX310X_REG_05 /* Revision ID */
+#define MAX310X_I2C_REVID_EXTREG (0x25) /* Revision ID */

/* IRQ register bits */
#define MAX310X_IRQ_LSR_BIT (1 << 0) /* LSR interrupt */
@@ -252,6 +254,10 @@ struct max310x_if_cfg {
};

struct max310x_devtype {
+ struct {
+ unsigned short min;
+ unsigned short max;
+ } slave_addr;
char name[9];
int nr;
u8 mode1;
@@ -365,6 +371,11 @@ static int max310x_spi_set_ext_reg_en(struct device *dev, bool enable)
enable ? MAX310X_EXTREG_ENBL : MAX310X_EXTREG_DSBL);
}

+static int max310x_i2c_set_ext_reg_en(struct device *dev, bool enable)
+{
+ return 0;
+}
+
static int max3109_detect(struct device *dev)
{
struct max310x_port *s = dev_get_drvdata(dev);
@@ -431,6 +442,10 @@ static const struct max310x_devtype max3107_devtype = {
.mode1 = MAX310X_MODE1_AUTOSLEEP_BIT | MAX310X_MODE1_IRQSEL_BIT,
.detect = max3107_detect,
.power = max310x_power,
+ .slave_addr = {
+ .min = 0x2c,
+ .max = 0x2f,
+ },
};

static const struct max310x_devtype max3108_devtype = {
@@ -439,6 +454,10 @@ static const struct max310x_devtype max3108_devtype = {
.mode1 = MAX310X_MODE1_AUTOSLEEP_BIT,
.detect = max3108_detect,
.power = max310x_power,
+ .slave_addr = {
+ .min = 0x60,
+ .max = 0x6f,
+ },
};

static const struct max310x_devtype max3109_devtype = {
@@ -447,6 +466,10 @@ static const struct max310x_devtype max3109_devtype = {
.mode1 = MAX310X_MODE1_AUTOSLEEP_BIT,
.detect = max3109_detect,
.power = max310x_power,
+ .slave_addr = {
+ .min = 0x60,
+ .max = 0x6f,
+ },
};

static const struct max310x_devtype max14830_devtype = {
@@ -455,6 +478,10 @@ static const struct max310x_devtype max14830_devtype = {
.mode1 = MAX310X_MODE1_IRQSEL_BIT,
.detect = max14830_detect,
.power = max14830_power,
+ .slave_addr = {
+ .min = 0x60,
+ .max = 0x6f,
+ },
};

static bool max310x_reg_writeable(struct device *dev, unsigned int reg)
@@ -1517,6 +1544,90 @@ static struct spi_driver max310x_spi_driver = {
};
#endif

+static struct regmap_config regcfg_i2c = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .cache_type = REGCACHE_RBTREE,
+ .writeable_reg = max310x_reg_writeable,
+ .volatile_reg = max310x_reg_volatile,
+ .precious_reg = max310x_reg_precious,
+ .max_register = MAX310X_I2C_REVID_EXTREG,
+};
+
+static const struct max310x_if_cfg max310x_i2c_if_cfg = {
+ .set_ext_reg_en = max310x_i2c_set_ext_reg_en,
+ .rev_id_reg = MAX310X_I2C_REVID_EXTREG,
+};
+
+static unsigned short max310x_i2c_slave_addr(unsigned short addr,
+ unsigned int nr)
+{
+ /*
+ * 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
+ */
+
+ addr -= nr * 0x10;
+
+ if (nr >= 2)
+ addr -= 0x20;
+
+ return addr;
+}
+
+static int max310x_i2c_probe(struct i2c_client *client)
+{
+ const struct max310x_devtype *devtype =
+ device_get_match_data(&client->dev);
+ struct i2c_client *port_client;
+ struct regmap *regmaps[4];
+ unsigned int i;
+ u8 port_addr;
+
+ if (client->addr < devtype->slave_addr.min ||
+ 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,
+ devtype->slave_addr.max);
+
+ regmaps[0] = devm_regmap_init_i2c(client, &regcfg_i2c);
+
+ for (i = 1; i < devtype->nr; i++) {
+ port_addr = max310x_i2c_slave_addr(client->addr, i);
+ port_client = devm_i2c_new_dummy_device(&client->dev,
+ client->adapter,
+ port_addr);
+
+ regmaps[i] = devm_regmap_init_i2c(port_client, &regcfg_i2c);
+ }
+
+ return max310x_probe(&client->dev, devtype, &max310x_i2c_if_cfg,
+ regmaps, client->irq);
+}
+
+static int max310x_i2c_remove(struct i2c_client *client)
+{
+ max310x_remove(&client->dev);
+
+ return 0;
+}
+
+static struct i2c_driver __maybe_unused max310x_i2c_driver = {
+ .driver = {
+ .name = MAX310X_NAME,
+ .of_match_table = max310x_dt_ids,
+ .pm = &max310x_pm_ops,
+ },
+ .probe_new = max310x_i2c_probe,
+ .remove = max310x_i2c_remove,
+};
+
static int __init max310x_uart_init(void)
{
int ret;
@@ -1530,15 +1641,33 @@ static int __init max310x_uart_init(void)
#ifdef CONFIG_SPI_MASTER
ret = spi_register_driver(&max310x_spi_driver);
if (ret)
- uart_unregister_driver(&max310x_uart);
+ goto err_spi_register;
#endif

+#ifdef CONFIG_I2C
+ ret = i2c_add_driver(&max310x_i2c_driver);
+ if (ret)
+ goto err_i2c_register;
+#endif
+
+ return 0;
+
+err_spi_register:
+ spi_unregister_driver(&max310x_spi_driver);
+
+err_i2c_register:
+ uart_unregister_driver(&max310x_uart);
+
return ret;
}
module_init(max310x_uart_init);

static void __exit max310x_uart_exit(void)
{
+#ifdef CONFIG_I2C
+ i2c_del_driver(&max310x_i2c_driver);
+#endif
+
#ifdef CONFIG_SPI_MASTER
spi_unregister_driver(&max310x_spi_driver);
#endif
--
2.36.1



2022-06-01 11:22:35

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 4/4] serial: max310x: implement I2C support

Hi Cosmin,

I love your patch! Yet something to improve:

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on usb/usb-testing v5.18 next-20220531]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/intel-lab-lkp/linux/commits/Cosmin-Tanislav/serial-max310x-use-regmap-methods-for-SPI-batch-operations/20220531-061619
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: openrisc-randconfig-c023-20220531 (https://download.01.org/0day-ci/archive/20220531/[email protected]/config)
compiler: or1k-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/6c293b95fc5654df5353ba273a9bbd08f1cd3f3a
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Cosmin-Tanislav/serial-max310x-use-regmap-methods-for-SPI-batch-operations/20220531-061619
git checkout 6c293b95fc5654df5353ba273a9bbd08f1cd3f3a
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=openrisc SHELL=/bin/bash drivers/tty/serial/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All error/warnings (new ones prefixed by >>):

drivers/tty/serial/max310x.c: In function 'max310x_i2c_probe':
>> drivers/tty/serial/max310x.c:1603:31: error: implicit declaration of function 'devm_i2c_new_dummy_device' [-Werror=implicit-function-declaration]
1603 | port_client = devm_i2c_new_dummy_device(&client->dev,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/tty/serial/max310x.c:1603:29: warning: assignment to 'struct i2c_client *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
1603 | port_client = devm_i2c_new_dummy_device(&client->dev,
| ^
drivers/tty/serial/max310x.c: In function 'max310x_uart_init':
drivers/tty/serial/max310x.c:1658:1: warning: label 'err_i2c_register' defined but not used [-Wunused-label]
1658 | err_i2c_register:
| ^~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors


vim +/devm_i2c_new_dummy_device +1603 drivers/tty/serial/max310x.c

1582
1583 static int max310x_i2c_probe(struct i2c_client *client)
1584 {
1585 const struct max310x_devtype *devtype =
1586 device_get_match_data(&client->dev);
1587 struct i2c_client *port_client;
1588 struct regmap *regmaps[4];
1589 unsigned int i;
1590 u8 port_addr;
1591
1592 if (client->addr < devtype->slave_addr.min ||
1593 client->addr > devtype->slave_addr.max)
1594 return dev_err_probe(&client->dev, -EINVAL,
1595 "Slave addr 0x%x outside of range [0x%x, 0x%x]\n",
1596 client->addr, devtype->slave_addr.min,
1597 devtype->slave_addr.max);
1598
1599 regmaps[0] = devm_regmap_init_i2c(client, &regcfg_i2c);
1600
1601 for (i = 1; i < devtype->nr; i++) {
1602 port_addr = max310x_i2c_slave_addr(client->addr, i);
> 1603 port_client = devm_i2c_new_dummy_device(&client->dev,
1604 client->adapter,
1605 port_addr);
1606
1607 regmaps[i] = devm_regmap_init_i2c(port_client, &regcfg_i2c);
1608 }
1609
1610 return max310x_probe(&client->dev, devtype, &max310x_i2c_if_cfg,
1611 regmaps, client->irq);
1612 }
1613

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-06-01 20:19:15

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 4/4] serial: max310x: implement I2C support

Hi Cosmin,

I love your patch! Perhaps something to improve:

[auto build test WARNING on tty/tty-testing]
[also build test WARNING on usb/usb-testing v5.18 next-20220531]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/intel-lab-lkp/linux/commits/Cosmin-Tanislav/serial-max310x-use-regmap-methods-for-SPI-batch-operations/20220531-061619
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20220601/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project c825abd6b0198fb088d9752f556a70705bc99dfd)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/6c293b95fc5654df5353ba273a9bbd08f1cd3f3a
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Cosmin-Tanislav/serial-max310x-use-regmap-methods-for-SPI-batch-operations/20220531-061619
git checkout 6c293b95fc5654df5353ba273a9bbd08f1cd3f3a
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/tty/serial/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> drivers/tty/serial/max310x.c:1658:1: warning: unused label 'err_i2c_register' [-Wunused-label]
err_i2c_register:
^~~~~~~~~~~~~~~~~
1 warning generated.


vim +/err_i2c_register +1658 drivers/tty/serial/max310x.c

1652
1653 return 0;
1654
1655 err_spi_register:
1656 spi_unregister_driver(&max310x_spi_driver);
1657
> 1658 err_i2c_register:
1659 uart_unregister_driver(&max310x_uart);
1660
1661 return ret;
1662 }
1663 module_init(max310x_uart_init);
1664

--
0-DAY CI Kernel Test Service
https://01.org/lkp