* [PATCH] serial: max310x: fix I2C-only build
@ 2026-05-19 20:34 Arnd Bergmann
2026-05-19 21:14 ` Hugo Villeneuve
0 siblings, 1 reply; 5+ messages in thread
From: Arnd Bergmann @ 2026-05-19 20:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Hugo Villeneuve
Cc: Arnd Bergmann, Randy Dunlap, Geert Uytterhoeven, linux-kernel,
linux-serial
From: Arnd Bergmann <arnd@arndb.de>
Allowing the driver to be built when CONFIG_SPI_MASTER is turned off
now causes build failures:
drivers/tty/serial/max310x.c: In function 'max310x_uart_init':
drivers/tty/serial/max310x.c:1737:32: error: 'max310x_spi_driver' undeclared (first use in this function); did you mean 'max310x_i2c_driver'?
1737 | spi_unregister_driver(&max310x_spi_driver);
| ^~~~~~~~~~~~~~~~~~
| max310x_i2c_driver
drivers/tty/serial/max310x.c:1737:32: note: each undeclared identifier is reported only once for each function it appears in
drivers/tty/serial/max310x.c:1740:1: error: label 'err_spi_register' defined but not used [-Werror=unused-label]
1740 | err_spi_register:
| ^~~~~~~~~~~~~~~~
drivers/tty/serial/max310x.c: At top level:
drivers/tty/serial/max310x.c:1502:29: error: 'regcfg' defined but not used [-Werror=unused-variable]
1502 | static struct regmap_config regcfg = {
| ^~~~~~
While fixing this, I ran into another problem with I2C=m, which
previously would just not use I2C support.
Fix the #ifdef checks to handle all combinations correctly, and
change the Kconfig dependency to ensure that the driver cannot
be built-in when I2C=m.
Fixes: 20ffe4b3330a ("serial: max310x: allow driver to be built with SPI or I2C")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/tty/serial/Kconfig | 1 +
drivers/tty/serial/max310x.c | 40 +++++++++++++++++++-----------------
2 files changed, 22 insertions(+), 19 deletions(-)
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 811250bbbd39..b9b40e80ea81 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -323,6 +323,7 @@ config SERIAL_MAX3100
config SERIAL_MAX310X
tristate "MAX310X support"
depends on SPI_MASTER || I2C
+ depends on I2C || !I2C
select SERIAL_CORE
select REGMAP_SPI if SPI_MASTER
select REGMAP_I2C if I2C
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 5168490a1cbb..042ae962c1f6 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1499,6 +1499,20 @@ static const struct of_device_id __maybe_unused max310x_dt_ids[] = {
};
MODULE_DEVICE_TABLE(of, max310x_dt_ids);
+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 struct regmap_config regcfg = {
.reg_bits = 8,
.val_bits = 8,
@@ -1514,20 +1528,6 @@ 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)
{
struct max310x_port *s = dev_get_drvdata(dev);
@@ -1598,7 +1598,7 @@ static struct spi_driver max310x_spi_driver = {
};
#endif
-#ifdef CONFIG_I2C
+#if IS_ENABLED(CONFIG_I2C)
static int max310x_i2c_extended_reg_enable(struct device *dev, bool enable)
{
return 0;
@@ -1724,7 +1724,7 @@ static int __init max310x_uart_init(void)
goto err_spi_register;
#endif
-#ifdef CONFIG_I2C
+#if IS_ENABLED(CONFIG_I2C)
ret = i2c_add_driver(&max310x_i2c_driver);
if (ret)
goto err_i2c_register;
@@ -1732,12 +1732,14 @@ static int __init max310x_uart_init(void)
return 0;
-#ifdef CONFIG_I2C
+#if IS_ENABLED(CONFIG_I2C)
err_i2c_register:
+#endif
+#ifdef CONFIG_SPI_MASTER
spi_unregister_driver(&max310x_spi_driver);
+err_spi_register:
#endif
-err_spi_register:
uart_unregister_driver(&max310x_uart);
return ret;
@@ -1746,7 +1748,7 @@ module_init(max310x_uart_init);
static void __exit max310x_uart_exit(void)
{
-#ifdef CONFIG_I2C
+#if IS_ENABLED(CONFIG_I2C)
i2c_del_driver(&max310x_i2c_driver);
#endif
--
2.39.5
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] serial: max310x: fix I2C-only build
2026-05-19 20:34 [PATCH] serial: max310x: fix I2C-only build Arnd Bergmann
@ 2026-05-19 21:14 ` Hugo Villeneuve
2026-05-20 6:47 ` Arnd Bergmann
0 siblings, 1 reply; 5+ messages in thread
From: Hugo Villeneuve @ 2026-05-19 21:14 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Greg Kroah-Hartman, Jiri Slaby, Hugo Villeneuve, Arnd Bergmann,
Randy Dunlap, Geert Uytterhoeven, linux-kernel, linux-serial
Hi Anrnd,
On Tue, 19 May 2026 22:34:25 +0200
Arnd Bergmann <arnd@kernel.org> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Allowing the driver to be built when CONFIG_SPI_MASTER is turned off
> now causes build failures:
A few days ago i sent v2 of a patch to fix this problem:
https://lore.kernel.org/all/20260515183217.3554037-1-hugo@hugovil.com/raw
Did you saw it?
>
> drivers/tty/serial/max310x.c: In function 'max310x_uart_init':
> drivers/tty/serial/max310x.c:1737:32: error: 'max310x_spi_driver' undeclared (first use in this function); did you mean 'max310x_i2c_driver'?
> 1737 | spi_unregister_driver(&max310x_spi_driver);
> | ^~~~~~~~~~~~~~~~~~
> | max310x_i2c_driver
> drivers/tty/serial/max310x.c:1737:32: note: each undeclared identifier is reported only once for each function it appears in
> drivers/tty/serial/max310x.c:1740:1: error: label 'err_spi_register' defined but not used [-Werror=unused-label]
> 1740 | err_spi_register:
> | ^~~~~~~~~~~~~~~~
> drivers/tty/serial/max310x.c: At top level:
> drivers/tty/serial/max310x.c:1502:29: error: 'regcfg' defined but not used [-Werror=unused-variable]
> 1502 | static struct regmap_config regcfg = {
> | ^~~~~~
>
> While fixing this, I ran into another problem with I2C=m, which
> previously would just not use I2C support.
>
> Fix the #ifdef checks to handle all combinations correctly, and
> change the Kconfig dependency to ensure that the driver cannot
> be built-in when I2C=m.
>
> Fixes: 20ffe4b3330a ("serial: max310x: allow driver to be built with SPI or I2C")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/tty/serial/Kconfig | 1 +
> drivers/tty/serial/max310x.c | 40 +++++++++++++++++++-----------------
> 2 files changed, 22 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 811250bbbd39..b9b40e80ea81 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -323,6 +323,7 @@ config SERIAL_MAX3100
> config SERIAL_MAX310X
> tristate "MAX310X support"
> depends on SPI_MASTER || I2C
> + depends on I2C || !I2C
> select SERIAL_CORE
> select REGMAP_SPI if SPI_MASTER
> select REGMAP_I2C if I2C
> diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
> index 5168490a1cbb..042ae962c1f6 100644
> --- a/drivers/tty/serial/max310x.c
> +++ b/drivers/tty/serial/max310x.c
> @@ -1499,6 +1499,20 @@ static const struct of_device_id __maybe_unused max310x_dt_ids[] = {
> };
> MODULE_DEVICE_TABLE(of, max310x_dt_ids);
>
> +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 struct regmap_config regcfg = {
> .reg_bits = 8,
> .val_bits = 8,
> @@ -1514,20 +1528,6 @@ 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)
> {
> struct max310x_port *s = dev_get_drvdata(dev);
> @@ -1598,7 +1598,7 @@ static struct spi_driver max310x_spi_driver = {
> };
> #endif
>
> -#ifdef CONFIG_I2C
> +#if IS_ENABLED(CONFIG_I2C)
> static int max310x_i2c_extended_reg_enable(struct device *dev, bool enable)
> {
> return 0;
> @@ -1724,7 +1724,7 @@ static int __init max310x_uart_init(void)
> goto err_spi_register;
> #endif
>
> -#ifdef CONFIG_I2C
> +#if IS_ENABLED(CONFIG_I2C)
> ret = i2c_add_driver(&max310x_i2c_driver);
> if (ret)
> goto err_i2c_register;
> @@ -1732,12 +1732,14 @@ static int __init max310x_uart_init(void)
>
> return 0;
>
> -#ifdef CONFIG_I2C
> +#if IS_ENABLED(CONFIG_I2C)
> err_i2c_register:
> +#endif
> +#ifdef CONFIG_SPI_MASTER
> spi_unregister_driver(&max310x_spi_driver);
> +err_spi_register:
> #endif
>
> -err_spi_register:
> uart_unregister_driver(&max310x_uart);
>
> return ret;
> @@ -1746,7 +1748,7 @@ module_init(max310x_uart_init);
>
> static void __exit max310x_uart_exit(void)
> {
> -#ifdef CONFIG_I2C
> +#if IS_ENABLED(CONFIG_I2C)
> i2c_del_driver(&max310x_i2c_driver);
> #endif
>
> --
> 2.39.5
>
>
>
--
Hugo Villeneuve
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] serial: max310x: fix I2C-only build
2026-05-19 21:14 ` Hugo Villeneuve
@ 2026-05-20 6:47 ` Arnd Bergmann
2026-05-20 14:04 ` Hugo Villeneuve
0 siblings, 1 reply; 5+ messages in thread
From: Arnd Bergmann @ 2026-05-20 6:47 UTC (permalink / raw)
To: Hugo Villeneuve, Arnd Bergmann
Cc: Greg Kroah-Hartman, Jiri Slaby, Hugo Villeneuve, Randy Dunlap,
Geert Uytterhoeven, linux-kernel, linux-serial
On Tue, May 19, 2026, at 23:14, Hugo Villeneuve wrote:
>> now causes build failures:
>
> A few days ago i sent v2 of a patch to fix this problem:
>
> https://lore.kernel.org/all/20260515183217.3554037-1-hugo@hugovil.com/raw
>
> Did you saw it?
Sorry I hadn't checked for other patches. I verified that your
changes are functionally the same as mine.
The one change I have that you didn't include is
>> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
>> index 811250bbbd39..b9b40e80ea81 100644
>> --- a/drivers/tty/serial/Kconfig
>> +++ b/drivers/tty/serial/Kconfig
>> @@ -323,6 +323,7 @@ config SERIAL_MAX3100
>> config SERIAL_MAX310X
>> tristate "MAX310X support"
>> depends on SPI_MASTER || I2C
>> + depends on I2C || !I2C
>> select SERIAL_CORE
>> select REGMAP_SPI if SPI_MASTER
>> select REGMAP_I2C if I2C
This is still required to avoid a link failure with
SERIAL_MAX310X=y and I2C=m. Do you want to include this in
a v3 of your patch, or should I send this separately?
Arnd
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] serial: max310x: fix I2C-only build
2026-05-20 6:47 ` Arnd Bergmann
@ 2026-05-20 14:04 ` Hugo Villeneuve
2026-05-20 14:34 ` Arnd Bergmann
0 siblings, 1 reply; 5+ messages in thread
From: Hugo Villeneuve @ 2026-05-20 14:04 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Arnd Bergmann, Greg Kroah-Hartman, Jiri Slaby, Hugo Villeneuve,
Randy Dunlap, Geert Uytterhoeven, linux-kernel, linux-serial
Hi Arnd,
On Wed, 20 May 2026 08:47:05 +0200
"Arnd Bergmann" <arnd@arndb.de> wrote:
> [You don't often get email from arnd@arndb.de. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Tue, May 19, 2026, at 23:14, Hugo Villeneuve wrote:
> >> now causes build failures:
> >
> > A few days ago i sent v2 of a patch to fix this problem:
> >
> > https://lore.kernel.org/all/20260515183217.3554037-1-hugo@hugovil.com/raw
> >
> > Did you saw it?
>
> Sorry I hadn't checked for other patches. I verified that your
> changes are functionally the same as mine.
>
> The one change I have that you didn't include is
>
> >> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> >> index 811250bbbd39..b9b40e80ea81 100644
> >> --- a/drivers/tty/serial/Kconfig
> >> +++ b/drivers/tty/serial/Kconfig
> >> @@ -323,6 +323,7 @@ config SERIAL_MAX3100
> >> config SERIAL_MAX310X
> >> tristate "MAX310X support"
> >> depends on SPI_MASTER || I2C
> >> + depends on I2C || !I2C
> >> select SERIAL_CORE
> >> select REGMAP_SPI if SPI_MASTER
> >> select REGMAP_I2C if I2C
>
> This is still required to avoid a link failure with
> SERIAL_MAX310X=y and I2C=m. Do you want to include this in
> a v3 of your patch, or should I send this separately?
If I2C=m, i cannot select SERIAL_MAX310X=y, it is automatically set (or
reset) to "m", even if I manually force it to "y", do you have the same
behavior?
Before i converted the sc16is7xx driver to split i2c/spi, it was done
like this:
depends on (SPI_MASTER && !I2C) || I2C
Based on what we agree on, I will include it in V3...
--
Hugo Villeneuve
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] serial: max310x: fix I2C-only build
2026-05-20 14:04 ` Hugo Villeneuve
@ 2026-05-20 14:34 ` Arnd Bergmann
0 siblings, 0 replies; 5+ messages in thread
From: Arnd Bergmann @ 2026-05-20 14:34 UTC (permalink / raw)
To: Hugo Villeneuve
Cc: Arnd Bergmann, Greg Kroah-Hartman, Jiri Slaby, Hugo Villeneuve,
Randy Dunlap, Geert Uytterhoeven, linux-kernel, linux-serial
On Wed, May 20, 2026, at 16:04, Hugo Villeneuve wrote:
> On Wed, 20 May 2026 08:47:05 +0200
> "Arnd Bergmann" <arnd@arndb.de> wrote:
>> >> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
>> >> index 811250bbbd39..b9b40e80ea81 100644
>> >> --- a/drivers/tty/serial/Kconfig
>> >> +++ b/drivers/tty/serial/Kconfig
>> >> @@ -323,6 +323,7 @@ config SERIAL_MAX3100
>> >> config SERIAL_MAX310X
>> >> tristate "MAX310X support"
>> >> depends on SPI_MASTER || I2C
>> >> + depends on I2C || !I2C
>> >> select SERIAL_CORE
>> >> select REGMAP_SPI if SPI_MASTER
>> >> select REGMAP_I2C if I2C
>>
>> This is still required to avoid a link failure with
>> SERIAL_MAX310X=y and I2C=m. Do you want to include this in
>> a v3 of your patch, or should I send this separately?
>
> If I2C=m, i cannot select SERIAL_MAX310X=y, it is automatically set (or
> reset) to "m", even if I manually force it to "y", do you have the same
> behavior?
The problem happens only when SPI_MASTER=y, in which case the
current logic does allow SERIAL_MAX310X=y. When SPI_MASTER is
disabled, it works correctly as you describe.
> Before i converted the sc16is7xx driver to split i2c/spi, it was done
> like this:
>
> depends on (SPI_MASTER && !I2C) || I2C
>
> Based on what we agree on, I will include it in V3...
Right, the '(SPI_MASTER && !I2C) || I2C' expression is equivalent
to what I suggested. I picked the syntax that I find easier to
understand, but I'm fine with that as well.
Arnd
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-20 14:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-19 20:34 [PATCH] serial: max310x: fix I2C-only build Arnd Bergmann
2026-05-19 21:14 ` Hugo Villeneuve
2026-05-20 6:47 ` Arnd Bergmann
2026-05-20 14:04 ` Hugo Villeneuve
2026-05-20 14:34 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox