From: Hugo Villeneuve <[email protected]>
Split the common code from sc16is7xx driver and move the I2C and SPI bus
parts into interface-specific source files.
sc16is7xx becomes the core functions which can support multiple bus
interfaces like I2C and SPI.
No functional changes intended.
Also simplify and improve Kconfig entries.
- Capitalize SPI keyword for consistency
- Display list of supported ICs each on a separate line (and aligned) to
facilitate locating a specific IC model
- Add Manufacturer name at start of description string
- Add UART keyword to description string
Suggested-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/tty/serial/Kconfig | 47 +++---
drivers/tty/serial/Makefile | 2 +
drivers/tty/serial/sc16is7xx.c | 225 +++++------------------------
drivers/tty/serial/sc16is7xx.h | 41 ++++++
drivers/tty/serial/sc16is7xx_i2c.c | 64 ++++++++
drivers/tty/serial/sc16is7xx_spi.c | 87 +++++++++++
6 files changed, 248 insertions(+), 218 deletions(-)
create mode 100644 drivers/tty/serial/sc16is7xx.h
create mode 100644 drivers/tty/serial/sc16is7xx_i2c.c
create mode 100644 drivers/tty/serial/sc16is7xx_spi.c
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index ffcf4882b25f9..9b5a65ea8ede3 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1021,41 +1021,30 @@ config SERIAL_SCCNXP_CONSOLE
Support for console on SCCNXP serial ports.
config SERIAL_SC16IS7XX_CORE
- tristate
-
-config SERIAL_SC16IS7XX
- tristate "SC16IS7xx serial support"
+ tristate "NXP SC16IS7xx UART support"
select SERIAL_CORE
- depends on (SPI_MASTER && !I2C) || I2C
+ select SERIAL_SC16IS7XX_SPI if SPI_MASTER
+ select SERIAL_SC16IS7XX_I2C if I2C
help
- This selects support for SC16IS7xx serial ports.
- Supported ICs are SC16IS740, SC16IS741, SC16IS750, SC16IS752,
- SC16IS760 and SC16IS762. Select supported buses using options below.
+ Core driver for NXP SC16IS7xx UARTs.
+ Supported ICs are:
+
+ SC16IS740
+ SC16IS741
+ SC16IS750
+ SC16IS752
+ SC16IS760
+ SC16IS762
+
+ The driver supports both I2C and SPI interfaces.
config SERIAL_SC16IS7XX_I2C
- bool "SC16IS7xx for I2C interface"
- depends on SERIAL_SC16IS7XX
- depends on I2C
- select SERIAL_SC16IS7XX_CORE if SERIAL_SC16IS7XX
- select REGMAP_I2C if I2C
- default y
- help
- Enable SC16IS7xx driver on I2C bus,
- If required say y, and say n to i2c if not required,
- Enabled by default to support oldconfig.
- You must select at least one bus for the driver to be built.
+ tristate
+ select REGMAP_I2C
config SERIAL_SC16IS7XX_SPI
- bool "SC16IS7xx for spi interface"
- depends on SERIAL_SC16IS7XX
- depends on SPI_MASTER
- select SERIAL_SC16IS7XX_CORE if SERIAL_SC16IS7XX
- select REGMAP_SPI if SPI_MASTER
- help
- Enable SC16IS7xx driver on SPI bus,
- If required say y, and say n to spi if not required,
- This is additional support to existing driver.
- You must select at least one bus for the driver to be built.
+ tristate
+ select REGMAP_SPI
config SERIAL_TIMBERDALE
tristate "Support for timberdale UART"
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index b25e9b54a6609..faa45f2b8bb05 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -76,6 +76,8 @@ obj-$(CONFIG_SERIAL_SAMSUNG) += samsung_tty.o
obj-$(CONFIG_SERIAL_SB1250_DUART) += sb1250-duart.o
obj-$(CONFIG_SERIAL_SCCNXP) += sccnxp.o
obj-$(CONFIG_SERIAL_SC16IS7XX_CORE) += sc16is7xx.o
+obj-$(CONFIG_SERIAL_SC16IS7XX_SPI) += sc16is7xx_spi.o
+obj-$(CONFIG_SERIAL_SC16IS7XX_I2C) += sc16is7xx_i2c.o
obj-$(CONFIG_SERIAL_SH_SCI) += sh-sci.o
obj-$(CONFIG_SERIAL_SIFIVE) += sifive.o
obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index b971fd2a9a77e..51fb1c95d3826 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1,19 +1,22 @@
// SPDX-License-Identifier: GPL-2.0+
/*
- * SC16IS7xx tty serial driver - Copyright (C) 2014 GridPoint
- * Author: Jon Ringle <[email protected]>
+ * SC16IS7xx tty serial driver - common code
*
- * Based on max310x.c, by Alexander Shiyan <[email protected]>
+ * Copyright (C) 2014 GridPoint
+ * Author: Jon Ringle <[email protected]>
+ * Based on max310x.c, by Alexander Shiyan <[email protected]>
*/
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#undef DEFAULT_SYMBOL_NAMESPACE
+#define DEFAULT_SYMBOL_NAMESPACE SERIAL_NXP_SC16IS7XX
#include <linux/bitops.h>
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/device.h>
+#include <linux/export.h>
#include <linux/gpio/driver.h>
-#include <linux/i2c.h>
+#include <linux/kthread.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/property.h>
@@ -21,15 +24,15 @@
#include <linux/sched.h>
#include <linux/serial_core.h>
#include <linux/serial.h>
+#include <linux/string.h>
#include <linux/tty.h>
#include <linux/tty_flip.h>
-#include <linux/spi/spi.h>
#include <linux/uaccess.h>
#include <linux/units.h>
-#define SC16IS7XX_NAME "sc16is7xx"
+#include "sc16is7xx.h"
+
#define SC16IS7XX_MAX_DEVS 8
-#define SC16IS7XX_MAX_PORTS 2 /* Maximum number of UART ports per IC. */
/* SC16IS7XX register definitions */
#define SC16IS7XX_RHR_REG (0x00) /* RX FIFO */
@@ -302,16 +305,9 @@
/* Misc definitions */
-#define SC16IS7XX_SPI_READ_BIT BIT(7)
#define SC16IS7XX_FIFO_SIZE (64)
#define SC16IS7XX_GPIOS_PER_BANK 4
-struct sc16is7xx_devtype {
- char name[10];
- int nr_gpio;
- int nr_uart;
-};
-
#define SC16IS7XX_RECONF_MD (1 << 0)
#define SC16IS7XX_RECONF_IER (1 << 1)
#define SC16IS7XX_RECONF_RS485 (1 << 2)
@@ -492,35 +488,40 @@ static void sc16is7xx_stop_rx(struct uart_port *port)
sc16is7xx_ier_clear(port, SC16IS7XX_IER_RDI_BIT);
}
-static const struct sc16is7xx_devtype sc16is74x_devtype = {
+const struct sc16is7xx_devtype sc16is74x_devtype = {
.name = "SC16IS74X",
.nr_gpio = 0,
.nr_uart = 1,
};
+EXPORT_SYMBOL_GPL(sc16is74x_devtype);
-static const struct sc16is7xx_devtype sc16is750_devtype = {
+const struct sc16is7xx_devtype sc16is750_devtype = {
.name = "SC16IS750",
.nr_gpio = 8,
.nr_uart = 1,
};
+EXPORT_SYMBOL_GPL(sc16is750_devtype);
-static const struct sc16is7xx_devtype sc16is752_devtype = {
+const struct sc16is7xx_devtype sc16is752_devtype = {
.name = "SC16IS752",
.nr_gpio = 8,
.nr_uart = 2,
};
+EXPORT_SYMBOL_GPL(sc16is752_devtype);
-static const struct sc16is7xx_devtype sc16is760_devtype = {
+const struct sc16is7xx_devtype sc16is760_devtype = {
.name = "SC16IS760",
.nr_gpio = 8,
.nr_uart = 1,
};
+EXPORT_SYMBOL_GPL(sc16is760_devtype);
-static const struct sc16is7xx_devtype sc16is762_devtype = {
+const struct sc16is7xx_devtype sc16is762_devtype = {
.name = "SC16IS762",
.nr_gpio = 8,
.nr_uart = 2,
};
+EXPORT_SYMBOL_GPL(sc16is762_devtype);
static bool sc16is7xx_regmap_volatile(struct device *dev, unsigned int reg)
{
@@ -1463,9 +1464,8 @@ static const struct serial_rs485 sc16is7xx_rs485_supported = {
.delay_rts_after_send = 1, /* Not supported but keep returning -EINVAL */
};
-static int sc16is7xx_probe(struct device *dev,
- const struct sc16is7xx_devtype *devtype,
- struct regmap *regmaps[], int irq)
+int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
+ struct regmap *regmaps[], int irq)
{
unsigned long freq = 0, *pfreq = dev_get_platdata(dev);
unsigned int val;
@@ -1657,8 +1657,9 @@ static int sc16is7xx_probe(struct device *dev,
return ret;
}
+EXPORT_SYMBOL_GPL(sc16is7xx_probe);
-static void sc16is7xx_remove(struct device *dev)
+void sc16is7xx_remove(struct device *dev)
{
struct sc16is7xx_port *s = dev_get_drvdata(dev);
int i;
@@ -1680,8 +1681,9 @@ static void sc16is7xx_remove(struct device *dev)
clk_disable_unprepare(s->clk);
}
+EXPORT_SYMBOL_GPL(sc16is7xx_remove);
-static const struct of_device_id __maybe_unused sc16is7xx_dt_ids[] = {
+const struct of_device_id __maybe_unused sc16is7xx_dt_ids[] = {
{ .compatible = "nxp,sc16is740", .data = &sc16is74x_devtype, },
{ .compatible = "nxp,sc16is741", .data = &sc16is74x_devtype, },
{ .compatible = "nxp,sc16is750", .data = &sc16is750_devtype, },
@@ -1690,9 +1692,10 @@ static const struct of_device_id __maybe_unused sc16is7xx_dt_ids[] = {
{ .compatible = "nxp,sc16is762", .data = &sc16is762_devtype, },
{ }
};
+EXPORT_SYMBOL_GPL(sc16is7xx_dt_ids);
MODULE_DEVICE_TABLE(of, sc16is7xx_dt_ids);
-static struct regmap_config regcfg = {
+struct regmap_config sc16is7xx_regcfg = {
.reg_bits = 5,
.pad_bits = 3,
.val_bits = 8,
@@ -1705,8 +1708,9 @@ static struct regmap_config regcfg = {
.max_raw_write = SC16IS7XX_FIFO_SIZE,
.max_register = SC16IS7XX_EFCR_REG,
};
+EXPORT_SYMBOL_GPL(sc16is7xx_regcfg);
-static const char *sc16is7xx_regmap_name(u8 port_id)
+const char *sc16is7xx_regmap_name(u8 port_id)
{
switch (port_id) {
case 0: return "port0";
@@ -1716,184 +1720,27 @@ static const char *sc16is7xx_regmap_name(u8 port_id)
return NULL;
}
}
+EXPORT_SYMBOL_GPL(sc16is7xx_regmap_name);
-static unsigned int sc16is7xx_regmap_port_mask(unsigned int port_id)
+unsigned int sc16is7xx_regmap_port_mask(unsigned int port_id)
{
/* CH1,CH0 are at bits 2:1. */
return port_id << 1;
}
-
-#ifdef CONFIG_SERIAL_SC16IS7XX_SPI
-static int sc16is7xx_spi_probe(struct spi_device *spi)
-{
- const struct sc16is7xx_devtype *devtype;
- struct regmap *regmaps[SC16IS7XX_MAX_PORTS];
- unsigned int i;
- int ret;
-
- /* Setup SPI bus */
- spi->bits_per_word = 8;
- /* For all variants, only mode 0 is supported */
- if ((spi->mode & SPI_MODE_X_MASK) != SPI_MODE_0)
- return dev_err_probe(&spi->dev, -EINVAL, "Unsupported SPI mode\n");
-
- spi->mode = spi->mode ? : SPI_MODE_0;
- spi->max_speed_hz = spi->max_speed_hz ? : 4 * HZ_PER_MHZ;
- ret = spi_setup(spi);
- if (ret)
- return ret;
-
- devtype = spi_get_device_match_data(spi);
- if (!devtype)
- return dev_err_probe(&spi->dev, -ENODEV, "Failed to match device\n");
-
- for (i = 0; i < devtype->nr_uart; i++) {
- regcfg.name = sc16is7xx_regmap_name(i);
- /*
- * If read_flag_mask is 0, the regmap code sets it to a default
- * of 0x80. Since we specify our own mask, we must add the READ
- * bit ourselves:
- */
- regcfg.read_flag_mask = sc16is7xx_regmap_port_mask(i) |
- SC16IS7XX_SPI_READ_BIT;
- regcfg.write_flag_mask = sc16is7xx_regmap_port_mask(i);
- regmaps[i] = devm_regmap_init_spi(spi, ®cfg);
- }
-
- return sc16is7xx_probe(&spi->dev, devtype, regmaps, spi->irq);
-}
-
-static void sc16is7xx_spi_remove(struct spi_device *spi)
-{
- sc16is7xx_remove(&spi->dev);
-}
-
-static const struct spi_device_id sc16is7xx_spi_id_table[] = {
- { "sc16is74x", (kernel_ulong_t)&sc16is74x_devtype, },
- { "sc16is740", (kernel_ulong_t)&sc16is74x_devtype, },
- { "sc16is741", (kernel_ulong_t)&sc16is74x_devtype, },
- { "sc16is750", (kernel_ulong_t)&sc16is750_devtype, },
- { "sc16is752", (kernel_ulong_t)&sc16is752_devtype, },
- { "sc16is760", (kernel_ulong_t)&sc16is760_devtype, },
- { "sc16is762", (kernel_ulong_t)&sc16is762_devtype, },
- { }
-};
-
-MODULE_DEVICE_TABLE(spi, sc16is7xx_spi_id_table);
-
-static struct spi_driver sc16is7xx_spi_uart_driver = {
- .driver = {
- .name = SC16IS7XX_NAME,
- .of_match_table = sc16is7xx_dt_ids,
- },
- .probe = sc16is7xx_spi_probe,
- .remove = sc16is7xx_spi_remove,
- .id_table = sc16is7xx_spi_id_table,
-};
-#endif
-
-#ifdef CONFIG_SERIAL_SC16IS7XX_I2C
-static int sc16is7xx_i2c_probe(struct i2c_client *i2c)
-{
- const struct sc16is7xx_devtype *devtype;
- struct regmap *regmaps[SC16IS7XX_MAX_PORTS];
- unsigned int i;
-
- devtype = i2c_get_match_data(i2c);
- if (!devtype)
- return dev_err_probe(&i2c->dev, -ENODEV, "Failed to match device\n");
-
- for (i = 0; i < devtype->nr_uart; i++) {
- regcfg.name = sc16is7xx_regmap_name(i);
- regcfg.read_flag_mask = sc16is7xx_regmap_port_mask(i);
- regcfg.write_flag_mask = sc16is7xx_regmap_port_mask(i);
- regmaps[i] = devm_regmap_init_i2c(i2c, ®cfg);
- }
-
- return sc16is7xx_probe(&i2c->dev, devtype, regmaps, i2c->irq);
-}
-
-static void sc16is7xx_i2c_remove(struct i2c_client *client)
-{
- sc16is7xx_remove(&client->dev);
-}
-
-static const struct i2c_device_id sc16is7xx_i2c_id_table[] = {
- { "sc16is74x", (kernel_ulong_t)&sc16is74x_devtype, },
- { "sc16is740", (kernel_ulong_t)&sc16is74x_devtype, },
- { "sc16is741", (kernel_ulong_t)&sc16is74x_devtype, },
- { "sc16is750", (kernel_ulong_t)&sc16is750_devtype, },
- { "sc16is752", (kernel_ulong_t)&sc16is752_devtype, },
- { "sc16is760", (kernel_ulong_t)&sc16is760_devtype, },
- { "sc16is762", (kernel_ulong_t)&sc16is762_devtype, },
- { }
-};
-MODULE_DEVICE_TABLE(i2c, sc16is7xx_i2c_id_table);
-
-static struct i2c_driver sc16is7xx_i2c_uart_driver = {
- .driver = {
- .name = SC16IS7XX_NAME,
- .of_match_table = sc16is7xx_dt_ids,
- },
- .probe = sc16is7xx_i2c_probe,
- .remove = sc16is7xx_i2c_remove,
- .id_table = sc16is7xx_i2c_id_table,
-};
-
-#endif
+EXPORT_SYMBOL_GPL(sc16is7xx_regmap_port_mask);
static int __init sc16is7xx_init(void)
{
- int ret;
-
- ret = uart_register_driver(&sc16is7xx_uart);
- if (ret) {
- pr_err("Registering UART driver failed\n");
- return ret;
- }
-
-#ifdef CONFIG_SERIAL_SC16IS7XX_I2C
- ret = i2c_add_driver(&sc16is7xx_i2c_uart_driver);
- if (ret < 0) {
- pr_err("failed to init sc16is7xx i2c --> %d\n", ret);
- goto err_i2c;
- }
-#endif
-
-#ifdef CONFIG_SERIAL_SC16IS7XX_SPI
- ret = spi_register_driver(&sc16is7xx_spi_uart_driver);
- if (ret < 0) {
- pr_err("failed to init sc16is7xx spi --> %d\n", ret);
- goto err_spi;
- }
-#endif
- return ret;
-
-#ifdef CONFIG_SERIAL_SC16IS7XX_SPI
-err_spi:
-#endif
-#ifdef CONFIG_SERIAL_SC16IS7XX_I2C
- i2c_del_driver(&sc16is7xx_i2c_uart_driver);
-err_i2c:
-#endif
- uart_unregister_driver(&sc16is7xx_uart);
- return ret;
+ return uart_register_driver(&sc16is7xx_uart);
}
module_init(sc16is7xx_init);
static void __exit sc16is7xx_exit(void)
{
-#ifdef CONFIG_SERIAL_SC16IS7XX_I2C
- i2c_del_driver(&sc16is7xx_i2c_uart_driver);
-#endif
-
-#ifdef CONFIG_SERIAL_SC16IS7XX_SPI
- spi_unregister_driver(&sc16is7xx_spi_uart_driver);
-#endif
uart_unregister_driver(&sc16is7xx_uart);
}
module_exit(sc16is7xx_exit);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Jon Ringle <[email protected]>");
-MODULE_DESCRIPTION("SC16IS7XX serial driver");
+MODULE_DESCRIPTION("SC16IS7xx tty serial core driver");
diff --git a/drivers/tty/serial/sc16is7xx.h b/drivers/tty/serial/sc16is7xx.h
new file mode 100644
index 0000000000000..2ee3ce83d95a4
--- /dev/null
+++ b/drivers/tty/serial/sc16is7xx.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* SC16IS7xx SPI/I2C tty serial driver */
+
+#ifndef _SC16IS7XX_H_
+#define _SC16IS7XX_H_
+
+#include <linux/mod_devicetable.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+#define SC16IS7XX_NAME "sc16is7xx"
+#define SC16IS7XX_MAX_PORTS 2 /* Maximum number of UART ports per IC. */
+
+struct device;
+
+struct sc16is7xx_devtype {
+ char name[10];
+ int nr_gpio;
+ int nr_uart;
+};
+
+extern struct regmap_config sc16is7xx_regcfg;
+
+extern const struct of_device_id sc16is7xx_dt_ids[];
+
+extern const struct sc16is7xx_devtype sc16is74x_devtype;
+extern const struct sc16is7xx_devtype sc16is750_devtype;
+extern const struct sc16is7xx_devtype sc16is752_devtype;
+extern const struct sc16is7xx_devtype sc16is760_devtype;
+extern const struct sc16is7xx_devtype sc16is762_devtype;
+
+const char *sc16is7xx_regmap_name(u8 port_id);
+
+unsigned int sc16is7xx_regmap_port_mask(unsigned int port_id);
+
+int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
+ struct regmap *regmaps[], int irq);
+
+void sc16is7xx_remove(struct device *dev);
+
+#endif /* _SC16IS7XX_H_ */
diff --git a/drivers/tty/serial/sc16is7xx_i2c.c b/drivers/tty/serial/sc16is7xx_i2c.c
new file mode 100644
index 0000000000000..de51d1675abfd
--- /dev/null
+++ b/drivers/tty/serial/sc16is7xx_i2c.c
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* SC16IS7xx I2C interface driver */
+
+#include <linux/dev_printk.h>
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/string.h>
+
+#include "sc16is7xx.h"
+
+static int sc16is7xx_i2c_probe(struct i2c_client *i2c)
+{
+ const struct sc16is7xx_devtype *devtype;
+ struct regmap *regmaps[SC16IS7XX_MAX_PORTS];
+ unsigned int i;
+
+ devtype = i2c_get_match_data(i2c);
+ if (!devtype)
+ return dev_err_probe(&i2c->dev, -ENODEV, "Failed to match device\n");
+
+ for (i = 0; i < devtype->nr_uart; i++) {
+ sc16is7xx_regcfg.name = sc16is7xx_regmap_name(i);
+ sc16is7xx_regcfg.read_flag_mask = sc16is7xx_regmap_port_mask(i);
+ sc16is7xx_regcfg.write_flag_mask = sc16is7xx_regmap_port_mask(i);
+ regmaps[i] = devm_regmap_init_i2c(i2c, &sc16is7xx_regcfg);
+ }
+
+ return sc16is7xx_probe(&i2c->dev, devtype, regmaps, i2c->irq);
+}
+
+static void sc16is7xx_i2c_remove(struct i2c_client *client)
+{
+ sc16is7xx_remove(&client->dev);
+}
+
+static const struct i2c_device_id sc16is7xx_i2c_id_table[] = {
+ { "sc16is74x", (kernel_ulong_t)&sc16is74x_devtype, },
+ { "sc16is740", (kernel_ulong_t)&sc16is74x_devtype, },
+ { "sc16is741", (kernel_ulong_t)&sc16is74x_devtype, },
+ { "sc16is750", (kernel_ulong_t)&sc16is750_devtype, },
+ { "sc16is752", (kernel_ulong_t)&sc16is752_devtype, },
+ { "sc16is760", (kernel_ulong_t)&sc16is760_devtype, },
+ { "sc16is762", (kernel_ulong_t)&sc16is762_devtype, },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sc16is7xx_i2c_id_table);
+
+static struct i2c_driver sc16is7xx_i2c_driver = {
+ .driver = {
+ .name = SC16IS7XX_NAME,
+ .of_match_table = sc16is7xx_dt_ids,
+ },
+ .probe = sc16is7xx_i2c_probe,
+ .remove = sc16is7xx_i2c_remove,
+ .id_table = sc16is7xx_i2c_id_table,
+};
+
+module_i2c_driver(sc16is7xx_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("SC16IS7xx I2C interface driver");
+MODULE_IMPORT_NS(SERIAL_NXP_SC16IS7XX);
diff --git a/drivers/tty/serial/sc16is7xx_spi.c b/drivers/tty/serial/sc16is7xx_spi.c
new file mode 100644
index 0000000000000..f110c4e6dce63
--- /dev/null
+++ b/drivers/tty/serial/sc16is7xx_spi.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* SC16IS7xx SPI interface driver */
+
+#include <linux/dev_printk.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+#include <linux/string.h>
+#include <linux/units.h>
+
+#include "sc16is7xx.h"
+
+/* SPI definitions */
+#define SC16IS7XX_SPI_READ_BIT BIT(7)
+
+static int sc16is7xx_spi_probe(struct spi_device *spi)
+{
+ const struct sc16is7xx_devtype *devtype;
+ struct regmap *regmaps[SC16IS7XX_MAX_PORTS];
+ unsigned int i;
+ int ret;
+
+ /* Setup SPI bus */
+ spi->bits_per_word = 8;
+ /* For all variants, only mode 0 is supported */
+ if ((spi->mode & SPI_MODE_X_MASK) != SPI_MODE_0)
+ return dev_err_probe(&spi->dev, -EINVAL, "Unsupported SPI mode\n");
+
+ spi->mode = spi->mode ? : SPI_MODE_0;
+ spi->max_speed_hz = spi->max_speed_hz ? : 4 * HZ_PER_MHZ;
+ ret = spi_setup(spi);
+ if (ret)
+ return ret;
+
+ devtype = spi_get_device_match_data(spi);
+ if (!devtype)
+ return dev_err_probe(&spi->dev, -ENODEV, "Failed to match device\n");
+
+ for (i = 0; i < devtype->nr_uart; i++) {
+ sc16is7xx_regcfg.name = sc16is7xx_regmap_name(i);
+ /*
+ * If read_flag_mask is 0, the regmap code sets it to a default
+ * of 0x80. Since we specify our own mask, we must add the READ
+ * bit ourselves:
+ */
+ sc16is7xx_regcfg.read_flag_mask = sc16is7xx_regmap_port_mask(i) |
+ SC16IS7XX_SPI_READ_BIT;
+ sc16is7xx_regcfg.write_flag_mask = sc16is7xx_regmap_port_mask(i);
+ regmaps[i] = devm_regmap_init_spi(spi, &sc16is7xx_regcfg);
+ }
+
+ return sc16is7xx_probe(&spi->dev, devtype, regmaps, spi->irq);
+}
+
+static void sc16is7xx_spi_remove(struct spi_device *spi)
+{
+ sc16is7xx_remove(&spi->dev);
+}
+
+static const struct spi_device_id sc16is7xx_spi_id_table[] = {
+ { "sc16is74x", (kernel_ulong_t)&sc16is74x_devtype, },
+ { "sc16is740", (kernel_ulong_t)&sc16is74x_devtype, },
+ { "sc16is741", (kernel_ulong_t)&sc16is74x_devtype, },
+ { "sc16is750", (kernel_ulong_t)&sc16is750_devtype, },
+ { "sc16is752", (kernel_ulong_t)&sc16is752_devtype, },
+ { "sc16is760", (kernel_ulong_t)&sc16is760_devtype, },
+ { "sc16is762", (kernel_ulong_t)&sc16is762_devtype, },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, sc16is7xx_spi_id_table);
+
+static struct spi_driver sc16is7xx_spi_driver = {
+ .driver = {
+ .name = SC16IS7XX_NAME,
+ .of_match_table = sc16is7xx_dt_ids,
+ },
+ .probe = sc16is7xx_spi_probe,
+ .remove = sc16is7xx_spi_remove,
+ .id_table = sc16is7xx_spi_id_table,
+};
+
+module_spi_driver(sc16is7xx_spi_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("SC16IS7xx SPI interface driver");
+MODULE_IMPORT_NS(SERIAL_NXP_SC16IS7XX);
--
2.39.2
Hi Hugo,
On Tue, Apr 9, 2024 at 5:48 PM Hugo Villeneuve <[email protected]> wrote:
> From: Hugo Villeneuve <[email protected]>
>
> Split the common code from sc16is7xx driver and move the I2C and SPI bus
> parts into interface-specific source files.
>
> sc16is7xx becomes the core functions which can support multiple bus
> interfaces like I2C and SPI.
>
> No functional changes intended.
>
> Also simplify and improve Kconfig entries.
> - Capitalize SPI keyword for consistency
> - Display list of supported ICs each on a separate line (and aligned) to
> facilitate locating a specific IC model
> - Add Manufacturer name at start of description string
> - Add UART keyword to description string
>
> Suggested-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Hugo Villeneuve <[email protected]>
Thanks for your patch, which is now commit d49216438139bca0
("serial: sc16is7xx: split into core and I2C/SPI parts (core)")
in tty-next (next-20240415 and later).
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1021,41 +1021,30 @@ config SERIAL_SCCNXP_CONSOLE
> Support for console on SCCNXP serial ports.
>
> config SERIAL_SC16IS7XX_CORE
> - tristate
> -
> -config SERIAL_SC16IS7XX
> - tristate "SC16IS7xx serial support"
> + tristate "NXP SC16IS7xx UART support"
Hence this replaces SERIAL_SC16IS7XX_CORE by SERIAL_SC16IS7XX,
so arch/mips/configs/cu1??0-neo_defconfig needs to updated.
> select SERIAL_CORE
> - depends on (SPI_MASTER && !I2C) || I2C
> + select SERIAL_SC16IS7XX_SPI if SPI_MASTER
> + select SERIAL_SC16IS7XX_I2C if I2C
So if SPI_MASTER or I2C is enabled, the corresponding SERIAL_SC16IS7XX_*
subdriver can no longer be disabled? According to
https://lore.kernel.org/all/20240403123501.8ef5c99f65a40ca2c10f635a@hugovilcom/
you did want to support that?
> help
> - This selects support for SC16IS7xx serial ports.
> - Supported ICs are SC16IS740, SC16IS741, SC16IS750, SC16IS752,
> - SC16IS760 and SC16IS762. Select supported buses using options below.
> + Core driver for NXP SC16IS7xx UARTs.
> + Supported ICs are:
> +
> + SC16IS740
> + SC16IS741
> + SC16IS750
> + SC16IS752
> + SC16IS760
> + SC16IS762
> +
> + The driver supports both I2C and SPI interfaces.
>
> config SERIAL_SC16IS7XX_I2C
> - bool "SC16IS7xx for I2C interface"
> - depends on SERIAL_SC16IS7XX
> - depends on I2C
> - select SERIAL_SC16IS7XX_CORE if SERIAL_SC16IS7XX
> - select REGMAP_I2C if I2C
> - default y
> - help
> - Enable SC16IS7xx driver on I2C bus,
> - If required say y, and say n to i2c if not required,
> - Enabled by default to support oldconfig.
> - You must select at least one bus for the driver to be built.
> + tristate
> + select REGMAP_I2C
>
> config SERIAL_SC16IS7XX_SPI
> - bool "SC16IS7xx for spi interface"
> - depends on SERIAL_SC16IS7XX
> - depends on SPI_MASTER
> - select SERIAL_SC16IS7XX_CORE if SERIAL_SC16IS7XX
> - select REGMAP_SPI if SPI_MASTER
> - help
> - Enable SC16IS7xx driver on SPI bus,
> - If required say y, and say n to spi if not required,
> - This is additional support to existing driver.
> - You must select at least one bus for the driver to be built.
> + tristate
> + select REGMAP_SPI
>
> config SERIAL_TIMBERDALE
> tristate "Support for timberdale UART"
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Tue, Apr 23, 2024 at 12:01 PM Geert Uytterhoeven
<[email protected]> wrote:
> On Tue, Apr 9, 2024 at 5:48 PM Hugo Villeneuve <[email protected]> wrote:
> > From: Hugo Villeneuve <[email protected]>
> >
> > Split the common code from sc16is7xx driver and move the I2C and SPI bus
> > parts into interface-specific source files.
> >
> > sc16is7xx becomes the core functions which can support multiple bus
> > interfaces like I2C and SPI.
> >
> > No functional changes intended.
> >
> > Also simplify and improve Kconfig entries.
> > - Capitalize SPI keyword for consistency
> > - Display list of supported ICs each on a separate line (and aligned) to
> > facilitate locating a specific IC model
> > - Add Manufacturer name at start of description string
> > - Add UART keyword to description string
> >
> > Suggested-by: Andy Shevchenko <[email protected]>
> > Signed-off-by: Hugo Villeneuve <[email protected]>
>
> Thanks for your patch, which is now commit d49216438139bca0
> ("serial: sc16is7xx: split into core and I2C/SPI parts (core)")
> in tty-next (next-20240415 and later).
>
> > --- a/drivers/tty/serial/Kconfig
> > +++ b/drivers/tty/serial/Kconfig
> > @@ -1021,41 +1021,30 @@ config SERIAL_SCCNXP_CONSOLE
> > Support for console on SCCNXP serial ports.
> >
> > config SERIAL_SC16IS7XX_CORE
> > - tristate
> > -
> > -config SERIAL_SC16IS7XX
> > - tristate "SC16IS7xx serial support"
> > + tristate "NXP SC16IS7xx UART support"
>
> Hence this replaces SERIAL_SC16IS7XX_CORE by SERIAL_SC16IS7XX,
> so arch/mips/configs/cu1??0-neo_defconfig needs to updated.
Or just rename SERIAL_SC16IS7XX_CORE back to SERIAL_SC16IS7XX.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Tue, Apr 23, 2024 at 1:01 PM Geert Uytterhoeven <[email protected]> wrote:
> On Tue, Apr 9, 2024 at 5:48 PM Hugo Villeneuve <[email protected]> wrote:
..
> So if SPI_MASTER or I2C is enabled, the corresponding SERIAL_SC16IS7XX_*
> subdriver can no longer be disabled? According to
> https://lore.kernel.org/all/[email protected]/
> you did want to support that?
I believe it has been taken from one of the IIO drivers as an example.
--
With Best Regards,
Andy Shevchenko
On Tue, Apr 23, 2024 at 1:03 PM Geert Uytterhoeven <[email protected]> wrote:
> On Tue, Apr 23, 2024 at 12:01 PM Geert Uytterhoeven
> <[email protected]> wrote:
> > On Tue, Apr 9, 2024 at 5:48 PM Hugo Villeneuve <[email protected]> wrote:
..
> Or just rename SERIAL_SC16IS7XX_CORE back to SERIAL_SC16IS7XX.
Since we do not know how many configurations elsewhere rely on this
and we have a real use case this suggestion seems plausible to me.
--
With Best Regards,
Andy Shevchenko
Hi Andy,
On Tue, Apr 23, 2024 at 12:37 PM Andy Shevchenko
<[email protected]> wrote:
> On Tue, Apr 23, 2024 at 1:01 PM Geert Uytterhoeven <[email protected]> wrote:
> > On Tue, Apr 9, 2024 at 5:48 PM Hugo Villeneuve <[email protected]> wrote:
> > > -config SERIAL_SC16IS7XX
> > > - tristate "SC16IS7xx serial support"
> > > + tristate "NXP SC16IS7xx UART support"
> >
> > Hence this replaces SERIAL_SC16IS7XX_CORE by SERIAL_SC16IS7XX,
> > so arch/mips/configs/cu1??0-neo_defconfig needs to updated.
>
> select SERIAL_CORE
> - depends on (SPI_MASTER && !I2C) || I2C
> + select SERIAL_SC16IS7XX_SPI if SPI_MASTER
> + select SERIAL_SC16IS7XX_I2C if I2C
>
> > So if SPI_MASTER or I2C is enabled, the corresponding SERIAL_SC16IS7XX_*
> > subdriver can no longer be disabled? According to
> > https://lore.kernel.org/all/[email protected]/
> > you did want to support that?
>
> I believe it has been taken from one of the IIO drivers as an example.
Looks like a bad example to follow:
1. The driver question now pops up if both I2C and SPI_MASTER
are disabled,
2. What if SERIAL_SC16IS7XX_CORE is builtin, but I2C and/or
SPI_MASTER are modular?
I believe the only way to fix that is by letting the sub-drivers select the
core driver, like before.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Tue, 23 Apr 2024 12:01:23 +0200
Geert Uytterhoeven <[email protected]> wrote:
> Hi Hugo,
>
> On Tue, Apr 9, 2024 at 5:48 PM Hugo Villeneuve <[email protected]> wrote:
> > From: Hugo Villeneuve <[email protected]>
> >
> > Split the common code from sc16is7xx driver and move the I2C and SPI bus
> > parts into interface-specific source files.
> >
> > sc16is7xx becomes the core functions which can support multiple bus
> > interfaces like I2C and SPI.
> >
> > No functional changes intended.
> >
> > Also simplify and improve Kconfig entries.
> > - Capitalize SPI keyword for consistency
> > - Display list of supported ICs each on a separate line (and aligned) to
> > facilitate locating a specific IC model
> > - Add Manufacturer name at start of description string
> > - Add UART keyword to description string
> >
> > Suggested-by: Andy Shevchenko <[email protected]>
> > Signed-off-by: Hugo Villeneuve <[email protected]>
>
> Thanks for your patch, which is now commit d49216438139bca0
> ("serial: sc16is7xx: split into core and I2C/SPI parts (core)")
> in tty-next (next-20240415 and later).
>
> > --- a/drivers/tty/serial/Kconfig
> > +++ b/drivers/tty/serial/Kconfig
> > @@ -1021,41 +1021,30 @@ config SERIAL_SCCNXP_CONSOLE
> > Support for console on SCCNXP serial ports.
> >
> > config SERIAL_SC16IS7XX_CORE
> > - tristate
> > -
> > -config SERIAL_SC16IS7XX
> > - tristate "SC16IS7xx serial support"
> > + tristate "NXP SC16IS7xx UART support"
>
> Hence this replaces SERIAL_SC16IS7XX_CORE by SERIAL_SC16IS7XX,
> so arch/mips/configs/cu1??0-neo_defconfig needs to updated.
Hi Geert,
yes you are right, sorry I missed that.
>
> > select SERIAL_CORE
> > - depends on (SPI_MASTER && !I2C) || I2C
> > + select SERIAL_SC16IS7XX_SPI if SPI_MASTER
> > + select SERIAL_SC16IS7XX_I2C if I2C
>
> So if SPI_MASTER or I2C is enabled, the corresponding SERIAL_SC16IS7XX_*
> subdriver can no longer be disabled? According to
> https://lore.kernel.org/all/[email protected]/
> you did want to support that?
Just to clarify, SPI subdriver will be disabled if
SPI_MASTER is disabled, even if I2C is enabled, and vice versa.
It was not my original intention, V1 of the patch offered the
possibility to enable/disable each subdriver individually, but Andy
pointed out that was maybe not the standard/usual/recommended way of
defining it, and to look into what other subsystems were doing,
especially iio. After some research, I settled on this approach as a
compromise.
Hugo.
>
> > help
> > - This selects support for SC16IS7xx serial ports.
> > - Supported ICs are SC16IS740, SC16IS741, SC16IS750, SC16IS752,
> > - SC16IS760 and SC16IS762. Select supported buses using options below.
> > + Core driver for NXP SC16IS7xx UARTs.
> > + Supported ICs are:
> > +
> > + SC16IS740
> > + SC16IS741
> > + SC16IS750
> > + SC16IS752
> > + SC16IS760
> > + SC16IS762
> > +
> > + The driver supports both I2C and SPI interfaces.
> >
> > config SERIAL_SC16IS7XX_I2C
> > - bool "SC16IS7xx for I2C interface"
> > - depends on SERIAL_SC16IS7XX
> > - depends on I2C
> > - select SERIAL_SC16IS7XX_CORE if SERIAL_SC16IS7XX
> > - select REGMAP_I2C if I2C
> > - default y
> > - help
> > - Enable SC16IS7xx driver on I2C bus,
> > - If required say y, and say n to i2c if not required,
> > - Enabled by default to support oldconfig.
> > - You must select at least one bus for the driver to be built.
> > + tristate
> > + select REGMAP_I2C
> >
> > config SERIAL_SC16IS7XX_SPI
> > - bool "SC16IS7xx for spi interface"
> > - depends on SERIAL_SC16IS7XX
> > - depends on SPI_MASTER
> > - select SERIAL_SC16IS7XX_CORE if SERIAL_SC16IS7XX
> > - select REGMAP_SPI if SPI_MASTER
> > - help
> > - Enable SC16IS7xx driver on SPI bus,
> > - If required say y, and say n to spi if not required,
> > - This is additional support to existing driver.
> > - You must select at least one bus for the driver to be built.
> > + tristate
> > + select REGMAP_SPI
> >
> > config SERIAL_TIMBERDALE
> > tristate "Support for timberdale UART"
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
On Tue, 23 Apr 2024 13:37:37 +0300
Andy Shevchenko <[email protected]> wrote:
> On Tue, Apr 23, 2024 at 1:03 PM Geert Uytterhoeven <[email protected]> wrote:
> > On Tue, Apr 23, 2024 at 12:01 PM Geert Uytterhoeven
> > <[email protected]> wrote:
> > > On Tue, Apr 9, 2024 at 5:48 PM Hugo Villeneuve <[email protected]> wrote:
>
> ...
>
> > Or just rename SERIAL_SC16IS7XX_CORE back to SERIAL_SC16IS7XX.
>
> Since we do not know how many configurations elsewhere rely on this
> and we have a real use case this suggestion seems plausible to me.
Agreed.
I will look into it.
Hugo.
On Tue, 23 Apr 2024 15:11:12 +0200
Geert Uytterhoeven <[email protected]> wrote:
Hi Geert,
> Hi Andy,
>
> On Tue, Apr 23, 2024 at 12:37 PM Andy Shevchenko
> <[email protected]> wrote:
> > On Tue, Apr 23, 2024 at 1:01 PM Geert Uytterhoeven <[email protected]> wrote:
> > > On Tue, Apr 9, 2024 at 5:48 PM Hugo Villeneuve <[email protected]> wrote:
>
> > > > -config SERIAL_SC16IS7XX
> > > > - tristate "SC16IS7xx serial support"
> > > > + tristate "NXP SC16IS7xx UART support"
> > >
> > > Hence this replaces SERIAL_SC16IS7XX_CORE by SERIAL_SC16IS7XX,
> > > so arch/mips/configs/cu1??0-neo_defconfig needs to updated.
> >
> > select SERIAL_CORE
> > - depends on (SPI_MASTER && !I2C) || I2C
> > + select SERIAL_SC16IS7XX_SPI if SPI_MASTER
> > + select SERIAL_SC16IS7XX_I2C if I2C
> >
> > > So if SPI_MASTER or I2C is enabled, the corresponding SERIAL_SC16IS7XX_*
> > > subdriver can no longer be disabled? According to
> > > https://lore.kernel.org/all/[email protected]/
> > > you did want to support that?
> >
> > I believe it has been taken from one of the IIO drivers as an example.
>
> Looks like a bad example to follow:
> 1. The driver question now pops up if both I2C and SPI_MASTER
> are disabled,
True.
V3 originally had this:
> config SERIAL_SC16IS7XX
> tristate "SC16IS7xx serial support"
> select SERIAL_CORE
> - depends on (SPI_MASTER && !I2C) || I2C
> + depends on SPI_MASTER || I2C
And Andy commented "Is it?", which I probably misinterpreted as I should
not list them as dependencies.
Reintroducing "depends on SPI_MASTER || I2C" fixes this issue.
> 2. What if SERIAL_SC16IS7XX_CORE is builtin, but I2C and/or
> SPI_MASTER are modular?
a) SERIAL_SC16IS7XX builtin and I2C modular:
CONFIG_SERIAL_SC16IS7XX=y
CONFIG_SERIAL_SC16IS7XX_I2C=m
CONFIG_SERIAL_SC16IS7XX_SPI=y
SPI_MASTER is only boolean and cannot be modular.
Hugo.
>
> I believe the only way to fix that is by letting the sub-drivers select the
> core driver, like before.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
--
Hugo Villeneuve
On Tue, 23 Apr 2024 09:14:10 -0400
Hugo Villeneuve <[email protected]> wrote:
> On Tue, 23 Apr 2024 12:01:23 +0200
> Geert Uytterhoeven <[email protected]> wrote:
>
> > Hi Hugo,
> >
> > On Tue, Apr 9, 2024 at 5:48 PM Hugo Villeneuve <[email protected]> wrote:
> > > From: Hugo Villeneuve <[email protected]>
> > >
> > > Split the common code from sc16is7xx driver and move the I2C and SPI bus
> > > parts into interface-specific source files.
> > >
> > > sc16is7xx becomes the core functions which can support multiple bus
> > > interfaces like I2C and SPI.
> > >
> > > No functional changes intended.
> > >
> > > Also simplify and improve Kconfig entries.
> > > - Capitalize SPI keyword for consistency
> > > - Display list of supported ICs each on a separate line (and aligned) to
> > > facilitate locating a specific IC model
> > > - Add Manufacturer name at start of description string
> > > - Add UART keyword to description string
> > >
> > > Suggested-by: Andy Shevchenko <[email protected]>
> > > Signed-off-by: Hugo Villeneuve <[email protected]>
> >
> > Thanks for your patch, which is now commit d49216438139bca0
> > ("serial: sc16is7xx: split into core and I2C/SPI parts (core)")
> > in tty-next (next-20240415 and later).
> >
> > > --- a/drivers/tty/serial/Kconfig
> > > +++ b/drivers/tty/serial/Kconfig
> > > @@ -1021,41 +1021,30 @@ config SERIAL_SCCNXP_CONSOLE
> > > Support for console on SCCNXP serial ports.
> > >
> > > config SERIAL_SC16IS7XX_CORE
> > > - tristate
> > > -
> > > -config SERIAL_SC16IS7XX
> > > - tristate "SC16IS7xx serial support"
> > > + tristate "NXP SC16IS7xx UART support"
> >
> > Hence this replaces SERIAL_SC16IS7XX_CORE by SERIAL_SC16IS7XX,
> > so arch/mips/configs/cu1??0-neo_defconfig needs to updated.
>
> Hi Geert,
> yes you are right, sorry I missed that.
>
> >
> > > select SERIAL_CORE
> > > - depends on (SPI_MASTER && !I2C) || I2C
> > > + select SERIAL_SC16IS7XX_SPI if SPI_MASTER
> > > + select SERIAL_SC16IS7XX_I2C if I2C
> >
> > So if SPI_MASTER or I2C is enabled, the corresponding SERIAL_SC16IS7XX_*
> > subdriver can no longer be disabled? According to
> > https://lore.kernel.org/all/[email protected]/
> > you did want to support that?
>
> Just to clarify, SPI subdriver will be disabled if
> SPI_MASTER is disabled, even if I2C is enabled, and vice versa.
>
> It was not my original intention, V1 of the patch offered the
> possibility to enable/disable each subdriver individually, but Andy
> pointed out that was maybe not the standard/usual/recommended way of
> defining it, and to look into what other subsystems were doing,
> especially iio. After some research, I settled on this approach as a
> compromise.
Hi Andy and Geert,
if you are ok with what I explained, I will send a patch.
Hugo.
>
>
> Hugo.
>
> >
> > > help
> > > - This selects support for SC16IS7xx serial ports.
> > > - Supported ICs are SC16IS740, SC16IS741, SC16IS750, SC16IS752,
> > > - SC16IS760 and SC16IS762. Select supported buses using options below.
> > > + Core driver for NXP SC16IS7xx UARTs.
> > > + Supported ICs are:
> > > +
> > > + SC16IS740
> > > + SC16IS741
> > > + SC16IS750
> > > + SC16IS752
> > > + SC16IS760
> > > + SC16IS762
> > > +
> > > + The driver supports both I2C and SPI interfaces.
> > >
> > > config SERIAL_SC16IS7XX_I2C
> > > - bool "SC16IS7xx for I2C interface"
> > > - depends on SERIAL_SC16IS7XX
> > > - depends on I2C
> > > - select SERIAL_SC16IS7XX_CORE if SERIAL_SC16IS7XX
> > > - select REGMAP_I2C if I2C
> > > - default y
> > > - help
> > > - Enable SC16IS7xx driver on I2C bus,
> > > - If required say y, and say n to i2c if not required,
> > > - Enabled by default to support oldconfig.
> > > - You must select at least one bus for the driver to be built.
> > > + tristate
> > > + select REGMAP_I2C
> > >
> > > config SERIAL_SC16IS7XX_SPI
> > > - bool "SC16IS7xx for spi interface"
> > > - depends on SERIAL_SC16IS7XX
> > > - depends on SPI_MASTER
> > > - select SERIAL_SC16IS7XX_CORE if SERIAL_SC16IS7XX
> > > - select REGMAP_SPI if SPI_MASTER
> > > - help
> > > - Enable SC16IS7xx driver on SPI bus,
> > > - If required say y, and say n to spi if not required,
> > > - This is additional support to existing driver.
> > > - You must select at least one bus for the driver to be built.
> > > + tristate
> > > + select REGMAP_SPI
> > >
> > > config SERIAL_TIMBERDALE
> > > tristate "Support for timberdale UART"
> >
> > Gr{oetje,eeting}s,
> >
> > Geert
> >
> > --
> > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
> >
> > In personal conversations with technical people, I call myself a hacker. But
> > when I'm talking to journalists I just say "programmer" or something like that.
> > -- Linus Torvalds
> >
>
>
--
Hugo Villeneuve
On Tue, Apr 23, 2024 at 3:11 PM Geert Uytterhoeven <[email protected]> wrote:
> On Tue, Apr 23, 2024 at 12:37 PM Andy Shevchenko
> <[email protected]> wrote:
> > On Tue, Apr 23, 2024 at 1:01 PM Geert Uytterhoeven <[email protected]> wrote:
> > > On Tue, Apr 9, 2024 at 5:48 PM Hugo Villeneuve <[email protected]> wrote:
>
> > > > -config SERIAL_SC16IS7XX
> > > > - tristate "SC16IS7xx serial support"
> > > > + tristate "NXP SC16IS7xx UART support"
> > >
> > > Hence this replaces SERIAL_SC16IS7XX_CORE by SERIAL_SC16IS7XX,
> > > so arch/mips/configs/cu1??0-neo_defconfig needs to updated.
> >
> > select SERIAL_CORE
> > - depends on (SPI_MASTER && !I2C) || I2C
> > + select SERIAL_SC16IS7XX_SPI if SPI_MASTER
> > + select SERIAL_SC16IS7XX_I2C if I2C
> >
> > > So if SPI_MASTER or I2C is enabled, the corresponding SERIAL_SC16IS7XX_*
> > > subdriver can no longer be disabled? According to
> > > https://lore.kernel.org/all/[email protected]/
> > > you did want to support that?
> >
> > I believe it has been taken from one of the IIO drivers as an example.
>
> Looks like a bad example to follow:
> 1. The driver question now pops up if both I2C and SPI_MASTER
> are disabled,
> 2. What if SERIAL_SC16IS7XX_CORE is builtin, but I2C and/or
> SPI_MASTER are modular?
>
> I believe the only way to fix that is by letting the sub-drivers select the
> core driver, like before.
FTR, this issue is now upstream.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Thu, 23 May 2024 09:33:36 +0200
Geert Uytterhoeven <[email protected]> wrote:
> On Tue, Apr 23, 2024 at 3:11 PM Geert Uytterhoeven <[email protected]> wrote:
> > On Tue, Apr 23, 2024 at 12:37 PM Andy Shevchenko
> > <[email protected]> wrote:
> > > On Tue, Apr 23, 2024 at 1:01 PM Geert Uytterhoeven <[email protected]> wrote:
> > > > On Tue, Apr 9, 2024 at 5:48 PM Hugo Villeneuve <[email protected]> wrote:
> >
> > > > > -config SERIAL_SC16IS7XX
> > > > > - tristate "SC16IS7xx serial support"
> > > > > + tristate "NXP SC16IS7xx UART support"
> > > >
> > > > Hence this replaces SERIAL_SC16IS7XX_CORE by SERIAL_SC16IS7XX,
> > > > so arch/mips/configs/cu1??0-neo_defconfig needs to updated.
> > >
> > > select SERIAL_CORE
> > > - depends on (SPI_MASTER && !I2C) || I2C
> > > + select SERIAL_SC16IS7XX_SPI if SPI_MASTER
> > > + select SERIAL_SC16IS7XX_I2C if I2C
> > >
> > > > So if SPI_MASTER or I2C is enabled, the corresponding SERIAL_SC16IS7XX_*
> > > > subdriver can no longer be disabled? According to
> > > > https://lore.kernel.org/all/[email protected]/
> > > > you did want to support that?
> > >
> > > I believe it has been taken from one of the IIO drivers as an example.
> >
> > Looks like a bad example to follow:
> > 1. The driver question now pops up if both I2C and SPI_MASTER
> > are disabled,
> > 2. What if SERIAL_SC16IS7XX_CORE is builtin, but I2C and/or
> > SPI_MASTER are modular?
> >
> > I believe the only way to fix that is by letting the sub-drivers select the
> > core driver, like before.
>
> FTR, this issue is now upstream.
Hi Geert,
I replied to you and Andy a few weeks ago about this (multiple emails with suggestions/explanations), and I even asked if you were satisfied with what I proposed, but never got anything from you, so I am still waiting on feedback to send a patch to fix this:
https://lore.kernel.org/all/[email protected]/
Hugo.
Hi Hugo,
On Fri, May 24, 2024 at 1:56 AM Hugo Villeneuve <[email protected]> wrote:
> On Thu, 23 May 2024 09:33:36 +0200
> Geert Uytterhoeven <[email protected]> wrote:
> > On Tue, Apr 23, 2024 at 3:11 PM Geert Uytterhoeven <[email protected]> wrote:
> > > On Tue, Apr 23, 2024 at 12:37 PM Andy Shevchenko
> > > <[email protected]> wrote:
> > > > On Tue, Apr 23, 2024 at 1:01 PM Geert Uytterhoeven <[email protected]> wrote:
> > > > > On Tue, Apr 9, 2024 at 5:48 PM Hugo Villeneuve <[email protected]> wrote:
> > >
> > > > > > -config SERIAL_SC16IS7XX
> > > > > > - tristate "SC16IS7xx serial support"
> > > > > > + tristate "NXP SC16IS7xx UART support"
> > > > >
> > > > > Hence this replaces SERIAL_SC16IS7XX_CORE by SERIAL_SC16IS7XX,
> > > > > so arch/mips/configs/cu1??0-neo_defconfig needs to updated.
> > > >
> > > > select SERIAL_CORE
> > > > - depends on (SPI_MASTER && !I2C) || I2C
> > > > + select SERIAL_SC16IS7XX_SPI if SPI_MASTER
> > > > + select SERIAL_SC16IS7XX_I2C if I2C
> > > >
> > > > > So if SPI_MASTER or I2C is enabled, the corresponding SERIAL_SC16IS7XX_*
> > > > > subdriver can no longer be disabled? According to
> > > > > https://lore.kernel.org/all/[email protected]/
> > > > > you did want to support that?
> > > >
> > > > I believe it has been taken from one of the IIO drivers as an example.
> > >
> > > Looks like a bad example to follow:
> > > 1. The driver question now pops up if both I2C and SPI_MASTER
> > > are disabled,
> > > 2. What if SERIAL_SC16IS7XX_CORE is builtin, but I2C and/or
> > > SPI_MASTER are modular?
> > >
> > > I believe the only way to fix that is by letting the sub-drivers select the
> > > core driver, like before.
> >
> > FTR, this issue is now upstream.
> I replied to you and Andy a few weeks ago about this (multiple emails with suggestions/explanations), and I even asked if you were satisfied with what I proposed, but never got anything from you, so I am still waiting on feedback to send a patch to fix this:
>
> https://lore.kernel.org/all/[email protected]/
Sorry, I indeed forgot to reply there.
Please use two visible symbols (for I2C and SPI), which select
the invisible SERIAL_SC16IS7XX_CORE.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds