Re: [PATCH v7 09/11] iio: dac: mcp47feb02: refactor MCP47FEB02 I2C driver into two modules

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] kbuild: Kconfig backward compatibility breakage during module split
- [Low] iio: dac: mcp47feb02: PM operations exported outside driver namespace
--

commit 8ebb6f8ca4b9fbd374359f689f69be2c855ffca3
Author: Ariana Lazar <ariana.lazar@microchip.com>

iio: dac: mcp47feb02: refactor MCP47FEB02 I2C driver into two modules

This commit prepares the MCP47FEB02 driver for bus-specific code by
refactoring it into separate core and I2C files. It creates a shared
core module to support upcoming SPI parts and renames Kconfig and file
structures accordingly.

> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 3b3c77d8b3e1b..c99c32b229fb6 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -593,9 +593,13 @@ config MCP47A1
>  	  will be called mcp47a1.
>  
>  config MCP47FEB02
> -	tristate "MCP47F(E/V)B01/02/04/08/11/12/14/18/21/22/24/28 DAC driver"
> +	tristate
> +
> +config MCP47FEB02_I2C
> +	tristate "MCP47F(E/V)B01/02/04/08/11/12/14/18/21/22/24/28 I2C DAC driver"

[Severity: Medium]
Does renaming the user-visible Kconfig prompt break backward compatibility
during kernel upgrades?

When users upgrade their kernels using make oldconfig or make olddefconfig,
their existing CONFIG_MCP47FEB02 setting will be discarded because it lacks
a prompt. The new CONFIG_MCP47FEB02_I2C defaults to 'n', resulting in the
driver being silently disabled on existing configurations.

Is it possible to keep the original config name for the user-visible option
and create a new hidden config (e.g., CONFIG_MCP47FEB02_CORE) for the
shared code to ensure seamless upgrades?

[ ... ]

> diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02-core.c
> similarity index 73%
> rename from drivers/iio/dac/mcp47feb02.c
> rename to drivers/iio/dac/mcp47feb02-core.c
> index 7312c2c96141e..91fad42df2168 100644
> --- a/drivers/iio/dac/mcp47feb02.c
> +++ b/drivers/iio/dac/mcp47feb02-core.c

[ ... ]

> @@ -668,7 +461,7 @@ static ssize_t mcp47feb02_write_powerdown(struct iio_dev *indio_dev, uintptr_t p
>  	return len;
>  }
>  
> -static DEFINE_SIMPLE_DEV_PM_OPS(mcp47feb02_pm_ops, mcp47feb02_suspend, mcp47feb02_resume);
> +EXPORT_SIMPLE_DEV_PM_OPS(mcp47feb02_pm_ops, mcp47feb02_suspend, mcp47feb02_resume);

[Severity: Low]
Does this macro export the PM ops symbol without the intended IIO_MCP47FEB02
namespace?

The driver establishes a dedicated namespace IIO_MCP47FEB02 for its core
exported symbols, but mcp47feb02_pm_ops is exported using
EXPORT_SIMPLE_DEV_PM_OPS, which places it into the global namespace.

Should this use EXPORT_NS_SIMPLE_DEV_PM_OPS(mcp47feb02_pm_ops,
mcp47feb02_suspend, mcp47feb02_resume, IIO_MCP47FEB02) instead to
maintain namespace encapsulation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918-mcp47feb02_refactor-v7-0-82ca794eafe2@microchip.com?part=9