serial: max310x: fix I2C-only build

[PATCH] serial: max310x: fix I2C-only build

Posted by Arnd Bergmann 5 days, 3 hours ago

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

Re: [PATCH] serial: max310x: fix I2C-only build

Posted by Hugo Villeneuve 5 days, 3 hours ago

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

Re: [PATCH] serial: max310x: fix I2C-only build

Posted by Arnd Bergmann 4 days, 17 hours ago

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

Re: [PATCH] serial: max310x: fix I2C-only build

Posted by Hugo Villeneuve 4 days, 10 hours ago

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

Re: [PATCH] serial: max310x: fix I2C-only build

Posted by Arnd Bergmann 4 days, 9 hours ago

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