Re: [PATCH v3 1/3] iio: dac: mcp47feb02: refactor MCP47FEB02 I2C driver into two modules

On Tue, 4 Aug 2026 16:29:47 +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]>
Hi Ariana,

There are a couple of things in here unrelated to the code movement etc
and splitting of the drivers. Please pull anything like that out to
precursor patches where we can clearly see what changed.

Jonathan

> diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02-core.c
> similarity index 72%
> rename from drivers/iio/dac/mcp47feb02.c
> rename to drivers/iio/dac/mcp47feb02-core.c
> index a823c2a673a26d70e5829cb587034da435af0451..e686c511d73f2be978f3a77af7a4004194e5d284 100644
> --- a/drivers/iio/dac/mcp47feb02.c
> +++ b/drivers/iio/dac/mcp47feb02-core.c
> @@ -1,8 +1,8 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
> - * IIO driver for MCP47FEB02 Multi-Channel DAC with I2C interface
> + * IIO driver for MCP47FEB02 Multi-Channel DAC with I2C and SPI interface
>   *
> - * Copyright (C) 2025 Microchip Technology Inc. and its subsidiaries
> + * Copyright (C) 2025-2026 Microchip Technology Inc. and its subsidiaries
>   *
>   * Author: Ariana Lazar <[email protected]>
>   *
> @@ -15,13 +15,12 @@
>  #include <linux/bits.h>
>  #include <linux/bitfield.h>
>  #include <linux/delay.h>
> +#include <linux/dev_printk.h>
>  #include <linux/err.h>
> -#include <linux/i2c.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  #include <linux/kstrtox.h>
>  #include <linux/module.h>
> -#include <linux/mod_devicetable.h>
>  #include <linux/mutex.h>
>  #include <linux/property.h>
>  #include <linux/regmap.h>
> @@ -30,11 +29,13 @@
>  #include <linux/types.h>
>  #include <linux/units.h>
>  
> +#include "mcp47feb02.h"
> +
>  /* Register addresses must be left shifted with 3 positions in order to append command mask */
>  #define MCP47FEB02_DAC0_REG_ADDR			0x00
>  #define MCP47FEB02_VREF_REG_ADDR			0x40
>  #define MCP47FEB02_POWER_DOWN_REG_ADDR			0x48
> -#define MCP47FEB02_DAC_CTRL_MASK			GENMASK(1, 0)
> +#define MCP47FEB02_CMD_MASK				GENMASK(1, 0)

These renames don't belong in a patch doing anything else. Please break
them out as a precursor. 

>  
>  #define MCP47FEB02_GAIN_CTRL_STATUS_REG_ADDR		0x50
>  #define MCP47FEB02_GAIN_BIT_MASK			BIT(0)
> @@ -53,7 +54,7 @@
>  #define DAC_CTRL_MASK(ch)				(GENMASK(1, 0) << (2 * (ch)))
>  #define DAC_CTRL_VAL(ch, val)				((val) << (2 * (ch)))
>  
> -/* Gain Control and I2C Slave Address Reguster fields */
> +/* Gain Control and I2C Slave Address Register fields */

This is also an unrelated change. Precursor patch.

>  #define DAC_GAIN_MASK(ch)				(BIT(0) << (8 + (ch)))
>  #define DAC_GAIN_VAL(ch, val)				((val) << (8 + (ch)))