Re: [PATCH v4 7/9] iio: dac: mcp47feb02: refactor MCP47FEB02 I2C driver into two modules

On Wed, 19 Aug 2026 15:41:11 +0300
Ariana Lazar <[email protected]> wrote:

> Prepare the driver for the bus-specific code by refactoring into separate
> files. The renamed file will contain the common DAC functionality shared by
> the MCP47FxBy1/2/4/8 I2C and MCP48FxBy1/2/4/8 SPI drivers. The MCP47FEB02
> driver was refactored into two modules: mcp47feb02-core.c and
> mcp47feb02-i2c.c in order to prepare the support for SPI MCP48FxBy1/2/4/8
> DAC family on top of the current implementation.
> 
> Signed-off-by: Ariana Lazar <[email protected]>
A few trivial things inline.

Thanks,

Jonathan

> ---
>  MAINTAINERS                                        |   4 +-
>  drivers/iio/dac/Kconfig                            |  10 +-
>  drivers/iio/dac/Makefile                           |   3 +-
>  .../iio/dac/{mcp47feb02.c => mcp47feb02-core.c}    | 327 ++-------------------
>  drivers/iio/dac/mcp47feb02-i2c.c                   | 145 +++++++++
>  drivers/iio/dac/mcp47feb02.h                       |  46 +++
>  6 files changed, 227 insertions(+), 308 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6b4560681b51cfabf0b0e98fd89b69d8c527228b..fc87609865f26a9f484edc42058f7df659a0fa4f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16118,7 +16118,9 @@ M:	Ariana Lazar <[email protected]>
>  L:	[email protected]
>  S:	Supported
>  F:	Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml
> -F:	drivers/iio/dac/mcp47feb02.c
> +F:	drivers/iio/dac/mcp47feb02-core.c
> +F:	drivers/iio/dac/mcp47feb02-i2c.c
> +F:	drivers/iio/dac/mcp47feb02.h
>  

Can use wild cards. We should probably do this more often!

F:   drivers/iio/dac/mcp47feb02* 

....

> diff --git a/drivers/iio/dac/mcp47feb02.h b/drivers/iio/dac/mcp47feb02.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..64620c865871254f5db373f951ffad3b9c273f29
> --- /dev/null
> +++ b/drivers/iio/dac/mcp47feb02.h
> @@ -0,0 +1,46 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +#ifndef __DRIVERS_IIO_DAC_MCP47FEB02_H__
> +#define __DRIVERS_IIO_DAC_MCP47FEB02_H__
> +
> +#include <linux/pm.h>
> +#include <linux/types.h>
> +
> +extern const char * const mcp47feb02_powerdown_modes[];
> +
> +/* Macro used for generating chip features structures */

That seems kind of obvious. I'd drop the comment!

> +#define MCP47FEB02_CHIP_INFO(_name, _channels, _res, _vref1, _eeprom) \
> +static const struct mcp47feb02_features _name##_chip_features = { \
> +	.name = #_name, \
> +	.phys_channels = _channels, \
> +	.resolution = _res, \
> +	.have_ext_vref1 = _vref1, \
> +	.have_eeprom = _eeprom, \
> +}
> +/* Properties shared by I2C and SPI families */

I wouldn't call the following function a property.  Probably just drop this comment
as not that useful.

> +int mcp47feb02_common_probe(const struct mcp47feb02_features *chip_features,
> +			    struct regmap *regmap);
> +
> +extern const struct dev_pm_ops mcp47feb02_pm_ops;
> +
> +#endif /* __DRIVERS_IIO_DAC_MCP47FEB02_H__ */
> +

Seems to be an extra trailing blank line. Delete it.