* [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver
@ 2026-09-09 14:18 Ariana Lazar
2026-09-09 14:18 ` [PATCH v5 01/10] iio: dac: mcp47feb02: initialize dac_data field in channel data struct at probe Ariana Lazar
` (10 more replies)
0 siblings, 11 replies; 27+ messages in thread
From: Ariana Lazar @ 2026-09-09 14:18 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-kernel, linux-iio, devicetree, Ariana Lazar
Refactor I2C driver implementation into separate modules in order to add
support for SPI MCP48FxBy1/2/4/8 DAC family on top of the I2C implementation.
The I2C and SPI devices have the same memory map and supported
functionalities.
Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com>
---
Changes in v5:
- improve commit messages by justifying the changes made
- increase polling time for EEPROM writes
- delete unnecessary comments and extra blank line from header file
- Link to v4: https://lore.kernel.org/r/20260819-mcp47feb02_refactor-v4-0-beb2aa3bfd09@microchip.com
Changes in v4:
- remove unused headers
- add missing header types.h in protocol files
- create 6 precursor commits before the refactor patch for handling:
- comment typo
- renaming command mask define
- for an invalid label use a debug message instead of probe error
- correct return value in store_eeprom_store() for false state case
- correct channel gain formula
- read DAC value register to initialize channel data struct field at
probe
- Link to v3: https://lore.kernel.org/r/20260804-mcp47feb02_refactor-v3-0-97f8b81f3628@microchip.com
Changes in v3:
- renumbering the patch version to include the first patch submision
- fix review comments device tree binding:
- change 'additionalProperties' to unevaluatedProperties
- add else branch with spi-max-frequency in the condition which checks
if a device has SPI interface
- correct SPI address example
- add clock polarity and clock phase properties for SPI devices
- enumerate I2C devices in lexicographic order from the description section
- specify supported SPI modes using dependencies
- fix review comments driver:
- remove unneccesarry libraries and add the missing ones
- correct comment typo
- return -ENODEV using dev_err_probe() in protocol probe functions
- use named initializers
- move defines for registers and enums from the header to the core file
- add dev_set_drvdata() in probe()
- move mutex aquiring from mcp47feb02_write_to_eeprom() to
store_eeprom_store()
- write correct value into Vref register in resume()
- Link to v2: https://lore.kernel.org/r/20260723-mcp47feb02_refactor-v1-0-ee59e63672bc@microchip.com
v2:
- include in the same patch series the refactoring of the driver and the
support for SPI devices, but in different patches
- remove changes regarding review comments received not related to the
code refactoring
- first version of the combined refactoring I2C and adding support for SPI
driver
- Link to v1: https://lore.kernel.org/all/20260403-mcp47feb02-fix2-v1-1-da60c773550e@microchip.com
v1:
- first version which did not separate the refactoring and adding support for
SPI devices (into different patches from the same series)
---
Ariana Lazar (10):
iio: dac: mcp47feb02: initialize dac_data field in channel data struct at probe
iio: dac: mcp47feb02: Fix gain field initialization for active channels
iio: dac: mcp47feb02: Return len when disabling EEPROM store
iio: dac: mcp47feb02: Increase EEPROM Programming Write Cycle Time
iio: dac: mcp47feb02: Avoid unjustified probe failure on missing label
iio: dac: mcp47feb02: correct typo from a comment
iio: dac: mcp47feb02: rename command mask define
iio: dac: mcp47feb02: refactor MCP47FEB02 I2C driver into two modules
dt-bindings: iio: dac: add support for MCP48FEB02 SPI
iio: dac: add support for Microchip MCP48FEB02
.../bindings/iio/dac/microchip,mcp47feb02.yaml | 221 ++++++++++---
MAINTAINERS | 2 +-
drivers/iio/dac/Kconfig | 33 +-
drivers/iio/dac/Makefile | 4 +-
.../iio/dac/{mcp47feb02.c => mcp47feb02-core.c} | 355 +++------------------
drivers/iio/dac/mcp47feb02-i2c.c | 145 +++++++++
drivers/iio/dac/mcp47feb02-spi.c | 145 +++++++++
drivers/iio/dac/mcp47feb02.h | 43 +++
8 files changed, 580 insertions(+), 368 deletions(-)
---
base-commit: 9c6acc6e6555d34198e45cb0d309569dfbadf929
change-id: 20260625-mcp47feb02_refactor-fa8420ae3282
Best regards,
--
Ariana Lazar <ariana.lazar@microchip.com>
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v5 01/10] iio: dac: mcp47feb02: initialize dac_data field in channel data struct at probe 2026-09-09 14:18 [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver Ariana Lazar @ 2026-09-09 14:18 ` Ariana Lazar 2026-09-09 14:32 ` sashiko-bot 2026-09-09 14:18 ` [PATCH v5 02/10] iio: dac: mcp47feb02: Fix gain field initialization for active channels Ariana Lazar ` (9 subsequent siblings) 10 siblings, 1 reply; 27+ messages in thread From: Ariana Lazar @ 2026-09-09 14:18 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-kernel, linux-iio, devicetree, Ariana Lazar Prevent operations which use cache from overwriting default hardware values with zeros by reading active DAC registers during probe to correctly initialize the channel struct DAC values. Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> --- drivers/iio/dac/mcp47feb02.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c index a823c2a673a26d70e5829cb587034da435af0451..7502959d98eab27941d58b6b961e6e3dee4222e6 100644 --- a/drivers/iio/dac/mcp47feb02.c +++ b/drivers/iio/dac/mcp47feb02.c @@ -1020,7 +1020,12 @@ static int mcp47feb02_init_ctrl_regs(struct mcp47feb02_data *data) gain_ch = gain_ch & MCP47FEB02_GAIN_BITS_MASK; for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { struct device *dev = regmap_get_device(data->regmap); - unsigned int pd_tmp; + unsigned int pd_tmp, dac_val; + + ret = regmap_read(data->regmap, REG_ADDR(i), &dac_val); + if (ret) + return ret; + data->chdata[i].dac_data = dac_val; data->chdata[i].ref_mode = (vref_ch >> (2 * i)) & MCP47FEB02_DAC_CTRL_MASK; data->chdata[i].use_2x_gain = (gain_ch >> i) & MCP47FEB02_GAIN_BIT_MASK; -- 2.43.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v5 01/10] iio: dac: mcp47feb02: initialize dac_data field in channel data struct at probe 2026-09-09 14:18 ` [PATCH v5 01/10] iio: dac: mcp47feb02: initialize dac_data field in channel data struct at probe Ariana Lazar @ 2026-09-09 14:32 ` sashiko-bot 0 siblings, 0 replies; 27+ messages in thread From: sashiko-bot @ 2026-09-09 14:32 UTC (permalink / raw) To: Ariana Lazar; +Cc: conor+dt, devicetree, robh Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Pre-existing issues: - [High] Incorrect powerdown state evaluation at probe time causes the driver to misinterpret hardware powerdown mode 1 as normal operation. - [High] System resume unconditionally forces channels out of powerdown, losing user configuration. -- commit f99608deaab75112b697e5e0e28a4014f58de232 Author: Ariana Lazar <ariana.lazar@microchip.com> iio: dac: mcp47feb02: initialize dac_data field in channel data struct at probe Prevent operations which use cache from overwriting default hardware values with zeros by reading active DAC registers during probe to correctly initialize the channel struct DAC values. > diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c > index a823c2a673a26..7502959d98eab 100644 > --- a/drivers/iio/dac/mcp47feb02.c > +++ b/drivers/iio/dac/mcp47feb02.c > @@ -1020,7 +1020,12 @@ static int mcp47feb02_init_ctrl_regs(struct mcp47feb02_data *data) > gain_ch = gain_ch & MCP47FEB02_GAIN_BITS_MASK; > for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { > struct device *dev = regmap_get_device(data->regmap); > - unsigned int pd_tmp; > + unsigned int pd_tmp, dac_val; > + > + ret = regmap_read(data->regmap, REG_ADDR(i), &dac_val); > + if (ret) > + return ret; > + data->chdata[i].dac_data = dac_val; > > data->chdata[i].ref_mode = (vref_ch >> (2 * i)) & MCP47FEB02_DAC_CTRL_MASK; > data->chdata[i].use_2x_gain = (gain_ch >> i) & MCP47FEB02_GAIN_BIT_MASK; [Severity: High] This is a pre-existing issue, but does the probe logic misinterpret hardware powerdown mode 1 as normal operation? If the probe phase reads the powerdown state register and it returns 1, pd_tmp is 1. This correctly maps powerdown_mode to index 0. However, the boolean powerdown evaluation becomes !!0, which is false: mcp47feb02_init_ctrl_regs() { [ ... ] pd_tmp = (pd_ch >> (2 * i)) & MCP47FEB02_DAC_CTRL_MASK; data->chdata[i].powerdown_mode = pd_tmp ? (pd_tmp - 1) : pd_tmp; data->chdata[i].powerdown = !!(data->chdata[i].powerdown_mode); [ ... ] } Because of this, the channel is flagged as actively operating when the hardware is physically in powerdown mode 1 (1kOhm to GND), creating a mismatch between the software state and the physical hardware state. [Severity: High] This is another pre-existing issue, but do the suspend and resume routines discard user configuration and unconditionally force channels out of powerdown? During suspend, the driver overrides the software powerdown state to true: mcp47feb02_suspend() { [ ... ] data->chdata[ch].powerdown = true; pd_mode = data->chdata[ch].powerdown_mode + 1; ret = regmap_update_bits(data->regmap, MCP47FEB02_POWER_DOWN_REG_ADDR, DAC_CTRL_MASK(ch), DAC_CTRL_VAL(ch, pd_mode)); [ ... ] } During resume, it blindly forces all active channels to false and writes MCP47FEB02_NORMAL_OPERATION to the hardware: mcp47feb02_resume() { [ ... ] data->chdata[ch].powerdown = false; ret = regmap_write(data->regmap, REG_ADDR(ch), data->chdata[ch].dac_data); [ ... ] ret = regmap_update_bits(data->regmap, MCP47FEB02_POWER_DOWN_REG_ADDR, DAC_CTRL_MASK(ch), DAC_CTRL_VAL(ch, MCP47FEB02_NORMAL_OPERATION)); [ ... ] } If a user intentionally powered down a channel, it will unexpectedly wake up and output voltage upon system resume. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260909-mcp47feb02_refactor-v5-0-8b67bcab93d1@microchip.com?part=1 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 02/10] iio: dac: mcp47feb02: Fix gain field initialization for active channels 2026-09-09 14:18 [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver Ariana Lazar 2026-09-09 14:18 ` [PATCH v5 01/10] iio: dac: mcp47feb02: initialize dac_data field in channel data struct at probe Ariana Lazar @ 2026-09-09 14:18 ` Ariana Lazar 2026-09-13 21:43 ` Jonathan Cameron 2026-09-09 14:18 ` [PATCH v5 03/10] iio: dac: mcp47feb02: Return len when disabling EEPROM store Ariana Lazar ` (8 subsequent siblings) 10 siblings, 1 reply; 27+ messages in thread From: Ariana Lazar @ 2026-09-09 14:18 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-kernel, linux-iio, devicetree, Ariana Lazar As per MCP47FXBX48 Datasheet, in the format of the Gain Control and System Status Register each DAC channel has one bit to control its gain, starting at bit 8, while bits 0-7 contain status or unimplemented bits. The previous formula didn't initialize correctly all channels, being replaced by the already defined macro used in write operations where needed in the rest of the driver implementation. DAC_GAIN_MASK(i) extracts the gain control bit for each active channel correctly ine one step. Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> --- drivers/iio/dac/mcp47feb02.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c index 7502959d98eab27941d58b6b961e6e3dee4222e6..bf78618ac2c896b94e494a1ce76ef5b3e520f482 100644 --- a/drivers/iio/dac/mcp47feb02.c +++ b/drivers/iio/dac/mcp47feb02.c @@ -1017,7 +1017,6 @@ static int mcp47feb02_init_ctrl_regs(struct mcp47feb02_data *data) if (ret) return ret; - gain_ch = gain_ch & MCP47FEB02_GAIN_BITS_MASK; for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { struct device *dev = regmap_get_device(data->regmap); unsigned int pd_tmp, dac_val; @@ -1028,7 +1027,7 @@ static int mcp47feb02_init_ctrl_regs(struct mcp47feb02_data *data) data->chdata[i].dac_data = dac_val; data->chdata[i].ref_mode = (vref_ch >> (2 * i)) & MCP47FEB02_DAC_CTRL_MASK; - data->chdata[i].use_2x_gain = (gain_ch >> i) & MCP47FEB02_GAIN_BIT_MASK; + data->chdata[i].use_2x_gain = (gain_ch & DAC_GAIN_MASK(i)) ? 1 : 0; /* * Inform the user that the current voltage reference read from the volatile -- 2.43.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v5 02/10] iio: dac: mcp47feb02: Fix gain field initialization for active channels 2026-09-09 14:18 ` [PATCH v5 02/10] iio: dac: mcp47feb02: Fix gain field initialization for active channels Ariana Lazar @ 2026-09-13 21:43 ` Jonathan Cameron 0 siblings, 0 replies; 27+ messages in thread From: Jonathan Cameron @ 2026-09-13 21:43 UTC (permalink / raw) To: Ariana Lazar Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-iio, devicetree On Wed, 9 Sep 2026 17:18:45 +0300 Ariana Lazar <ariana.lazar@microchip.com> wrote: > As per MCP47FXBX48 Datasheet, in the format of the Gain Control and System > Status Register each DAC channel has one bit to control its gain, > starting at bit 8, while bits 0-7 contain status or unimplemented bits. > > The previous formula didn't initialize correctly all channels, being > replaced by the already defined macro used in write operations where needed > in the rest of the driver implementation. DAC_GAIN_MASK(i) extracts the > gain control bit for each active channel correctly ine one step. > > Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> > --- > drivers/iio/dac/mcp47feb02.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c > index 7502959d98eab27941d58b6b961e6e3dee4222e6..bf78618ac2c896b94e494a1ce76ef5b3e520f482 100644 > --- a/drivers/iio/dac/mcp47feb02.c > +++ b/drivers/iio/dac/mcp47feb02.c > @@ -1017,7 +1017,6 @@ static int mcp47feb02_init_ctrl_regs(struct mcp47feb02_data *data) > if (ret) > return ret; > > - gain_ch = gain_ch & MCP47FEB02_GAIN_BITS_MASK; > for_each_set_bit(i, &data->active_channels_mask, data->phys_channels) { > struct device *dev = regmap_get_device(data->regmap); > unsigned int pd_tmp, dac_val; > @@ -1028,7 +1027,7 @@ static int mcp47feb02_init_ctrl_regs(struct mcp47feb02_data *data) > data->chdata[i].dac_data = dac_val; > > data->chdata[i].ref_mode = (vref_ch >> (2 * i)) & MCP47FEB02_DAC_CTRL_MASK; > - data->chdata[i].use_2x_gain = (gain_ch >> i) & MCP47FEB02_GAIN_BIT_MASK; > + data->chdata[i].use_2x_gain = (gain_ch & DAC_GAIN_MASK(i)) ? 1 : 0; It's not a performance path (field_get() is a bit heavyweight!) so data->chdata[i].use_2x_gain = field_get(DAC_GAIN_MASK(i), gain_ch); is perhaps a little more readable. I don't mind that much either way. Jonathan > > /* > * Inform the user that the current voltage reference read from the volatile > ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 03/10] iio: dac: mcp47feb02: Return len when disabling EEPROM store 2026-09-09 14:18 [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver Ariana Lazar 2026-09-09 14:18 ` [PATCH v5 01/10] iio: dac: mcp47feb02: initialize dac_data field in channel data struct at probe Ariana Lazar 2026-09-09 14:18 ` [PATCH v5 02/10] iio: dac: mcp47feb02: Fix gain field initialization for active channels Ariana Lazar @ 2026-09-09 14:18 ` Ariana Lazar 2026-09-09 14:33 ` sashiko-bot 2026-09-09 14:18 ` [PATCH v5 04/10] iio: dac: mcp47feb02: Increase EEPROM Programming Write Cycle Time Ariana Lazar ` (7 subsequent siblings) 10 siblings, 1 reply; 27+ messages in thread From: Ariana Lazar @ 2026-09-09 14:18 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-kernel, linux-iio, devicetree, Ariana Lazar When state is false, return the number of bytes (len) the buffer has instead of 0 to properly signal that the input buffer was fully consumed, preventing unnecessary write retries. Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> --- drivers/iio/dac/mcp47feb02.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c index bf78618ac2c896b94e494a1ce76ef5b3e520f482..2ee981393aa1a2dc0bf65d25ab937132fe2e7bf1 100644 --- a/drivers/iio/dac/mcp47feb02.c +++ b/drivers/iio/dac/mcp47feb02.c @@ -469,7 +469,7 @@ static ssize_t store_eeprom_store(struct device *dev, struct device_attribute *a return ret; if (!state) - return 0; + return len; /* * Verify DAC Wiper and DAC Configuration are unlocked. If both are disabled, -- 2.43.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v5 03/10] iio: dac: mcp47feb02: Return len when disabling EEPROM store 2026-09-09 14:18 ` [PATCH v5 03/10] iio: dac: mcp47feb02: Return len when disabling EEPROM store Ariana Lazar @ 2026-09-09 14:33 ` sashiko-bot 0 siblings, 0 replies; 27+ messages in thread From: sashiko-bot @ 2026-09-09 14:33 UTC (permalink / raw) To: Ariana Lazar; +Cc: robh, devicetree, conor+dt Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] This problem wasn't introduced by this patch, but suspend and resume operations clobber the user-configured channel powerdown state. -- commit 835e4538da01c3026ccf6237cb4357a864fa680c Author: Ariana Lazar <ariana.lazar@microchip.com> iio: dac: mcp47feb02: Return len when disabling EEPROM store This commit modifies store_eeprom_store() to return the input buffer length instead of 0 when state is false. This properly signals that the buffer was fully consumed, preventing unnecessary write retries. > diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c > index bf78618ac2c89..2ee981393aa1a 100644 > --- a/drivers/iio/dac/mcp47feb02.c > +++ b/drivers/iio/dac/mcp47feb02.c > @@ -469,7 +469,7 @@ static ssize_t store_eeprom_store(struct device *dev, struct device_attribute *a > return ret; > > if (!state) > - return 0; > + return len; > [Severity: High] This is a pre-existing issue, but suspend and resume operations clobber the user-configured channel powerdown state. Looking at mcp47feb02_suspend(): drivers/iio/dac/mcp47feb02.c:mcp47feb02_suspend() { ... for_each_set_bit(ch, &data->active_channels_mask, data->phys_channels) { u8 pd_mode; data->chdata[ch].powerdown = true; ... } Does this unconditionally overwrite the channel's powerdown state during suspend, causing the user's prior configuration to be permanently lost? Similarly, looking at mcp47feb02_resume(): drivers/iio/dac/mcp47feb02.c:mcp47feb02_resume() { ... for_each_set_bit(ch, &data->active_channels_mask, data->phys_channels) { u8 pd_mode; int ret; data->chdata[ch].powerdown = false; ... ret = regmap_update_bits(data->regmap, MCP47FEB02_POWER_DOWN_REG_ADDR, DAC_CTRL_MASK(ch), DAC_CTRL_VAL(ch, MCP47FEB02_NORMAL_OPERATION)); ... } Could this force all channels out of powerdown during resume, ignoring any user-configured disabled state? Waking up a DAC channel that was explicitly powered down by the user via sysfs could drive unintended voltage to attached hardware upon system resume. > /* > * Verify DAC Wiper and DAC Configuration are unlocked. If both are disabled, > * writing to EEPROM is available. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260909-mcp47feb02_refactor-v5-0-8b67bcab93d1@microchip.com?part=3 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 04/10] iio: dac: mcp47feb02: Increase EEPROM Programming Write Cycle Time 2026-09-09 14:18 [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver Ariana Lazar ` (2 preceding siblings ...) 2026-09-09 14:18 ` [PATCH v5 03/10] iio: dac: mcp47feb02: Return len when disabling EEPROM store Ariana Lazar @ 2026-09-09 14:18 ` Ariana Lazar 2026-09-09 14:31 ` sashiko-bot 2026-09-10 6:11 ` Andy Shevchenko 2026-09-09 14:18 ` [PATCH v5 05/10] iio: dac: mcp47feb02: Avoid unjustified probe failure on missing label Ariana Lazar ` (6 subsequent siblings) 10 siblings, 2 replies; 27+ messages in thread From: Ariana Lazar @ 2026-09-09 14:18 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-kernel, linux-iio, devicetree, Ariana Lazar Increase the EEPROM write polling timeout to prevent -ETIMEDOUT errors and incomplete nonvolatile write cycle, according to the time specified in the datasheets (11-16 ms). Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> --- drivers/iio/dac/mcp47feb02.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c index 2ee981393aa1a2dc0bf65d25ab937132fe2e7bf1..53a7bf4e71c46e75dff8bb5b16c343e7552f6901 100644 --- a/drivers/iio/dac/mcp47feb02.c +++ b/drivers/iio/dac/mcp47feb02.c @@ -449,7 +449,7 @@ static int mcp47feb02_write_to_eeprom(struct mcp47feb02_data *data, unsigned int ret = regmap_read_poll_timeout(data->regmap, MCP47FEB02_GAIN_CTRL_STATUS_REG_ADDR, eewa_val, !(eewa_val & MCP47FEB02_GAIN_BIT_STATUS_EEWA_MASK), - USEC_PER_MSEC, USEC_PER_MSEC * 5); + USEC_PER_MSEC, USEC_PER_MSEC * 1500); if (ret) return ret; @@ -509,7 +509,7 @@ static ssize_t store_eeprom_store(struct device *dev, struct device_attribute *a ret = regmap_read_poll_timeout(data->regmap, MCP47FEB02_GAIN_CTRL_STATUS_REG_ADDR, eewa_val, !(eewa_val & MCP47FEB02_GAIN_BIT_STATUS_EEWA_MASK), - USEC_PER_MSEC, USEC_PER_MSEC * 5); + USEC_PER_MSEC, USEC_PER_MSEC * 1500); if (ret) return ret; -- 2.43.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v5 04/10] iio: dac: mcp47feb02: Increase EEPROM Programming Write Cycle Time 2026-09-09 14:18 ` [PATCH v5 04/10] iio: dac: mcp47feb02: Increase EEPROM Programming Write Cycle Time Ariana Lazar @ 2026-09-09 14:31 ` sashiko-bot 2026-09-10 6:11 ` Andy Shevchenko 1 sibling, 0 replies; 27+ messages in thread From: sashiko-bot @ 2026-09-09 14:31 UTC (permalink / raw) To: Ariana Lazar; +Cc: devicetree, robh, conor+dt Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Timeout value is incorrectly set to 1500 ms (1.5 seconds) instead of the intended ~15 ms. -- commit a4c6e4bb178f8d633d5c95e35f6b2ffcf7e7e711 Author: Ariana Lazar <ariana.lazar@microchip.com> iio: dac: mcp47feb02: Increase EEPROM Programming Write Cycle Time This commit increases the EEPROM write polling timeout for the mcp47feb02 driver. The goal is to match the datasheet specifications of 11-16 ms to prevent -ETIMEDOUT errors and incomplete nonvolatile write cycles. > diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c > index 2ee981393aa1a..53a7bf4e71c46 100644 > --- a/drivers/iio/dac/mcp47feb02.c > +++ b/drivers/iio/dac/mcp47feb02.c > @@ -449,7 +449,7 @@ static int mcp47feb02_write_to_eeprom(struct mcp47feb02_data *data, unsigned int > ret = regmap_read_poll_timeout(data->regmap, MCP47FEB02_GAIN_CTRL_STATUS_REG_ADDR, > eewa_val, > !(eewa_val & MCP47FEB02_GAIN_BIT_STATUS_EEWA_MASK), > - USEC_PER_MSEC, USEC_PER_MSEC * 5); > + USEC_PER_MSEC, USEC_PER_MSEC * 1500); [Severity: Medium] Does this timeout value correctly match the commit message's stated 11-16 ms? Using USEC_PER_MSEC * 1500 sets the timeout to 1,500,000 microseconds (1.5 seconds) rather than the intended 15 milliseconds. In the event of a hardware failure where the EEPROM write never completes, this would block the caller for 1.5 seconds instead of 15 milliseconds. [ ... ] > @@ -509,7 +509,7 @@ static ssize_t store_eeprom_store(struct device *dev, struct device_attribute *a > > ret = regmap_read_poll_timeout(data->regmap, MCP47FEB02_GAIN_CTRL_STATUS_REG_ADDR, eewa_val, > !(eewa_val & MCP47FEB02_GAIN_BIT_STATUS_EEWA_MASK), > - USEC_PER_MSEC, USEC_PER_MSEC * 5); > + USEC_PER_MSEC, USEC_PER_MSEC * 1500); [Severity: Medium] Does this scaling error also compound the potential delay here? Since store_eeprom_store() can perform up to 11 sequential writes (8 channels + VREF + POWER_DOWN + I2C_SLAVE), this extended timeout could delay the sysfs write thread for over 16 seconds on a hardware failure where the EEWA bit fails to clear. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260909-mcp47feb02_refactor-v5-0-8b67bcab93d1@microchip.com?part=4 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5 04/10] iio: dac: mcp47feb02: Increase EEPROM Programming Write Cycle Time 2026-09-09 14:18 ` [PATCH v5 04/10] iio: dac: mcp47feb02: Increase EEPROM Programming Write Cycle Time Ariana Lazar 2026-09-09 14:31 ` sashiko-bot @ 2026-09-10 6:11 ` Andy Shevchenko 1 sibling, 0 replies; 27+ messages in thread From: Andy Shevchenko @ 2026-09-10 6:11 UTC (permalink / raw) To: Ariana Lazar Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-iio, devicetree On Wed, Sep 09, 2026 at 05:18:47PM +0300, Ariana Lazar wrote: > Increase the EEPROM write polling timeout to prevent -ETIMEDOUT errors and > incomplete nonvolatile write cycle, according to the time specified in the > datasheets (11-16 ms). ... > ret = regmap_read_poll_timeout(data->regmap, MCP47FEB02_GAIN_CTRL_STATUS_REG_ADDR, > eewa_val, > !(eewa_val & MCP47FEB02_GAIN_BIT_STATUS_EEWA_MASK), > - USEC_PER_MSEC, USEC_PER_MSEC * 5); > + USEC_PER_MSEC, USEC_PER_MSEC * 1500); While at it, make it clearer with 1 * USEC_PER_MSEC, 1500 * USEC_PER_MSEC); (note that some editors marks numeric constants with distinguishable colour). > if (ret) > return ret; ... > ret = regmap_read_poll_timeout(data->regmap, MCP47FEB02_GAIN_CTRL_STATUS_REG_ADDR, eewa_val, > !(eewa_val & MCP47FEB02_GAIN_BIT_STATUS_EEWA_MASK), > - USEC_PER_MSEC, USEC_PER_MSEC * 5); > + USEC_PER_MSEC, USEC_PER_MSEC * 1500); Ditto. > if (ret) > return ret; -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 05/10] iio: dac: mcp47feb02: Avoid unjustified probe failure on missing label 2026-09-09 14:18 [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver Ariana Lazar ` (3 preceding siblings ...) 2026-09-09 14:18 ` [PATCH v5 04/10] iio: dac: mcp47feb02: Increase EEPROM Programming Write Cycle Time Ariana Lazar @ 2026-09-09 14:18 ` Ariana Lazar 2026-09-09 14:40 ` sashiko-bot 2026-09-10 6:12 ` Andy Shevchenko 2026-09-09 14:18 ` [PATCH v5 06/10] iio: dac: mcp47feb02: correct typo from a comment Ariana Lazar ` (5 subsequent siblings) 10 siblings, 2 replies; 27+ messages in thread From: Ariana Lazar @ 2026-09-09 14:18 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-kernel, linux-iio, devicetree, Ariana Lazar Fix unjustified probe failure on missing optional 'label' property by replacing dev_err_probe() with dev_err(). Correct %pfw usage by passing the child fwnode handle directly in the error message. Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> --- drivers/iio/dac/mcp47feb02.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c index 53a7bf4e71c46e75dff8bb5b16c343e7552f6901..6353aabfe252e322a4f0d0efe20673f7e733a7f2 100644 --- a/drivers/iio/dac/mcp47feb02.c +++ b/drivers/iio/dac/mcp47feb02.c @@ -978,8 +978,7 @@ static int mcp47feb02_parse_fw(struct iio_dev *indio_dev, ret = fwnode_property_read_string(child, "label", &data->labels[reg]); if (ret) - return dev_err_probe(dev, ret, "%pfw: invalid label\n", - fwnode_get_name(child)); + dev_err(dev, "%pfw: invalid label\n", child); chanspec.address = reg; chanspec.channel = reg; -- 2.43.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v5 05/10] iio: dac: mcp47feb02: Avoid unjustified probe failure on missing label 2026-09-09 14:18 ` [PATCH v5 05/10] iio: dac: mcp47feb02: Avoid unjustified probe failure on missing label Ariana Lazar @ 2026-09-09 14:40 ` sashiko-bot 2026-09-10 6:12 ` Andy Shevchenko 1 sibling, 0 replies; 27+ messages in thread From: sashiko-bot @ 2026-09-09 14:40 UTC (permalink / raw) To: Ariana Lazar; +Cc: devicetree, conor+dt, robh Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Unconditional error logging when an optional 'label' property is missing. - [Medium] Reading the channel label via sysfs will return the literal string `"(null)"` if the optional label is missing. -- commit 804d4067a2450b6f94e3efd86db0f3e15dc14145 Author: Ariana Lazar <ariana.lazar@microchip.com> iio: dac: mcp47feb02: Avoid unjustified probe failure on missing label This commit modifies the driver to no longer fail the probe when an optional 'label' property is missing, but instead logs an error and allows the probe to continue. > diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c > index 53a7bf4e71c46..6353aabfe252e 100644 > --- a/drivers/iio/dac/mcp47feb02.c > +++ b/drivers/iio/dac/mcp47feb02.c > @@ -978,8 +978,7 @@ static int mcp47feb02_parse_fw(struct iio_dev *indio_dev, > > ret = fwnode_property_read_string(child, "label", &data->labels[reg]); > if (ret) > - return dev_err_probe(dev, ret, "%pfw: invalid label\n", > - fwnode_get_name(child)); > + dev_err(dev, "%pfw: invalid label\n", child); [Severity: Medium] Will this unconditionally log an error when the optional label property is missing? When the optional property is correctly omitted in the device tree, fwnode_property_read_string() returns -EINVAL. This triggers the if (ret) condition and pollutes dmesg with false positive error logs. [Severity: Medium] If the label is omitted and probe continues, data->labels[reg] remains NULL. If a userspace application reads the label sysfs attribute, does this cause sysfs_emit() to output the literal string "(null)\n"? drivers/iio/dac/mcp47feb02.c:mcp47feb02_read_label() { ... return sysfs_emit(label, "%s\n", data->labels[ch->address]); } This exposes kernel-internal formatting details and violates API expectations for the sysfs interface. > chanspec.address = reg; > chanspec.channel = reg; -- Sashiko AI review · https://sashiko.dev/#/patchset/20260909-mcp47feb02_refactor-v5-0-8b67bcab93d1@microchip.com?part=5 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5 05/10] iio: dac: mcp47feb02: Avoid unjustified probe failure on missing label 2026-09-09 14:18 ` [PATCH v5 05/10] iio: dac: mcp47feb02: Avoid unjustified probe failure on missing label Ariana Lazar 2026-09-09 14:40 ` sashiko-bot @ 2026-09-10 6:12 ` Andy Shevchenko 1 sibling, 0 replies; 27+ messages in thread From: Andy Shevchenko @ 2026-09-10 6:12 UTC (permalink / raw) To: Ariana Lazar Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-iio, devicetree On Wed, Sep 09, 2026 at 05:18:48PM +0300, Ariana Lazar wrote: > Fix unjustified probe failure on missing optional 'label' property by > replacing dev_err_probe() with dev_err(). Correct %pfw usage by passing the > child fwnode handle directly in the error message. ... > ret = fwnode_property_read_string(child, "label", &data->labels[reg]); > if (ret) > - return dev_err_probe(dev, ret, "%pfw: invalid label\n", > - fwnode_get_name(child)); > + dev_err(dev, "%pfw: invalid label\n", child); Why not leaving dev_err_probe()? And why error in such a case? Perhaps you want dev_warn_probe()? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 06/10] iio: dac: mcp47feb02: correct typo from a comment 2026-09-09 14:18 [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver Ariana Lazar ` (4 preceding siblings ...) 2026-09-09 14:18 ` [PATCH v5 05/10] iio: dac: mcp47feb02: Avoid unjustified probe failure on missing label Ariana Lazar @ 2026-09-09 14:18 ` Ariana Lazar 2026-09-09 14:18 ` [PATCH v5 07/10] iio: dac: mcp47feb02: rename command mask define Ariana Lazar ` (4 subsequent siblings) 10 siblings, 0 replies; 27+ messages in thread From: Ariana Lazar @ 2026-09-09 14:18 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-kernel, linux-iio, devicetree, Ariana Lazar Typo corrected to 'Register' in the comment regarding defined masks for the gain register. Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> --- drivers/iio/dac/mcp47feb02.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c index 6353aabfe252e322a4f0d0efe20673f7e733a7f2..c054711a55ca01be9e0aba57a2ddbf192406fd47 100644 --- a/drivers/iio/dac/mcp47feb02.c +++ b/drivers/iio/dac/mcp47feb02.c @@ -53,7 +53,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 */ #define DAC_GAIN_MASK(ch) (BIT(0) << (8 + (ch))) #define DAC_GAIN_VAL(ch, val) ((val) << (8 + (ch))) -- 2.43.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v5 07/10] iio: dac: mcp47feb02: rename command mask define 2026-09-09 14:18 [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver Ariana Lazar ` (5 preceding siblings ...) 2026-09-09 14:18 ` [PATCH v5 06/10] iio: dac: mcp47feb02: correct typo from a comment Ariana Lazar @ 2026-09-09 14:18 ` Ariana Lazar 2026-09-09 14:39 ` sashiko-bot 2026-09-09 14:18 ` [PATCH v5 08/10] iio: dac: mcp47feb02: refactor MCP47FEB02 I2C driver into two modules Ariana Lazar ` (3 subsequent siblings) 10 siblings, 1 reply; 27+ messages in thread From: Ariana Lazar @ 2026-09-09 14:18 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-kernel, linux-iio, devicetree, Ariana Lazar Use 2 new defines to select the corresponding bits for each channel in VREF and Power Down registers instead of the initial define without a meaningful name. Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> --- drivers/iio/dac/mcp47feb02.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c index c054711a55ca01be9e0aba57a2ddbf192406fd47..fb66bd2664a9e6dd938eacc90547403041e258a4 100644 --- a/drivers/iio/dac/mcp47feb02.c +++ b/drivers/iio/dac/mcp47feb02.c @@ -33,8 +33,9 @@ /* 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_GET_VREF_MODE(reg, ch) (((reg) >> (2 * (ch))) & GENMASK(1, 0)) #define MCP47FEB02_POWER_DOWN_REG_ADDR 0x48 -#define MCP47FEB02_DAC_CTRL_MASK GENMASK(1, 0) +#define MCP47FEB02_GET_POWER_DOWN_MODE(reg, ch) (((reg) >> (2 * (ch))) & GENMASK(1, 0)) #define MCP47FEB02_GAIN_CTRL_STATUS_REG_ADDR 0x50 #define MCP47FEB02_GAIN_BIT_MASK BIT(0) @@ -1025,7 +1026,7 @@ static int mcp47feb02_init_ctrl_regs(struct mcp47feb02_data *data) return ret; data->chdata[i].dac_data = dac_val; - data->chdata[i].ref_mode = (vref_ch >> (2 * i)) & MCP47FEB02_DAC_CTRL_MASK; + data->chdata[i].ref_mode = MCP47FEB02_GET_VREF_MODE(vref_ch, i); data->chdata[i].use_2x_gain = (gain_ch & DAC_GAIN_MASK(i)) ? 1 : 0; /* @@ -1069,7 +1070,7 @@ static int mcp47feb02_init_ctrl_regs(struct mcp47feb02_data *data) break; } - pd_tmp = (pd_ch >> (2 * i)) & MCP47FEB02_DAC_CTRL_MASK; + pd_tmp = MCP47FEB02_GET_POWER_DOWN_MODE(pd_ch, i); data->chdata[i].powerdown_mode = pd_tmp ? (pd_tmp - 1) : pd_tmp; data->chdata[i].powerdown = !!(data->chdata[i].powerdown_mode); } -- 2.43.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v5 07/10] iio: dac: mcp47feb02: rename command mask define 2026-09-09 14:18 ` [PATCH v5 07/10] iio: dac: mcp47feb02: rename command mask define Ariana Lazar @ 2026-09-09 14:39 ` sashiko-bot 0 siblings, 0 replies; 27+ messages in thread From: sashiko-bot @ 2026-09-09 14:39 UTC (permalink / raw) To: Ariana Lazar; +Cc: conor+dt, devicetree, robh Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Pre-existing issues: - [High] The driver fails to correctly parse the powerdown state during initialization, causing it to lose the distinction between 'Normal Operation' and '1 kOhm to GND' powerdown mode. - [High] The driver's suspend/resume PM callbacks permanently clobber the user-configured logical powerdown state. -- commit 0d81d552cabc3f0d9820a2ed59353c19dc69d754 Author: Ariana Lazar <ariana.lazar@microchip.com> iio: dac: mcp47feb02: rename command mask define This commit replaces the initial mask define with two new macros. These new macros explicitly select the corresponding bits for each channel in the VREF and Power Down registers. > diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c > index c054711a55ca0..fb66bd2664a9e 100644 > --- a/drivers/iio/dac/mcp47feb02.c > +++ b/drivers/iio/dac/mcp47feb02.c [ ... ] > @@ -1069,7 +1070,7 @@ static int mcp47feb02_init_ctrl_regs(struct mcp47feb02_data *data) > break; > } > > - pd_tmp = (pd_ch >> (2 * i)) & MCP47FEB02_DAC_CTRL_MASK; > + pd_tmp = MCP47FEB02_GET_POWER_DOWN_MODE(pd_ch, i); > data->chdata[i].powerdown_mode = pd_tmp ? (pd_tmp - 1) : pd_tmp; > data->chdata[i].powerdown = !!(data->chdata[i].powerdown_mode); [Severity: High] This isn't a bug introduced by this patch, but is it possible this logic incorrectly parses the powerdown state during initialization? When pd_tmp is 1 (1 kOhm to GND), powerdown_mode is calculated as 1 - 1 = 0. When pd_tmp is 0 (Normal Operation), powerdown_mode is also 0. Since the driver then sets powerdown to !!powerdown_mode (which evaluates to false in both cases), it seems the driver loses the hardware's true powerdown state if it booted in the '1 kOhm to GND' mode, misreporting it as normal operation. > } [Severity: High] This is a pre-existing issue, but I wanted to ask about the suspend and resume behavior in this driver. drivers/iio/dac/mcp47feb02.c:mcp47feb02_suspend() { ... for_each_set_bit(ch, &data->active_channels_mask, data->phys_channels) { u8 pd_mode; data->chdata[ch].powerdown = true; ... } drivers/iio/dac/mcp47feb02.c:mcp47feb02_resume() { ... for_each_set_bit(ch, &data->active_channels_mask, data->phys_channels) { u8 pd_mode; int ret; data->chdata[ch].powerdown = false; ... } If a DAC channel is explicitly placed in powerdown by the user via sysfs, would it unexpectedly wake up fully powered on after a system resume because the user's requested powerdown configuration is permanently overwritten? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260909-mcp47feb02_refactor-v5-0-8b67bcab93d1@microchip.com?part=7 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 08/10] iio: dac: mcp47feb02: refactor MCP47FEB02 I2C driver into two modules 2026-09-09 14:18 [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver Ariana Lazar ` (6 preceding siblings ...) 2026-09-09 14:18 ` [PATCH v5 07/10] iio: dac: mcp47feb02: rename command mask define Ariana Lazar @ 2026-09-09 14:18 ` Ariana Lazar 2026-09-09 14:47 ` sashiko-bot ` (2 more replies) 2026-09-09 14:18 ` [PATCH v5 09/10] dt-bindings: iio: dac: add support for MCP48FEB02 SPI Ariana Lazar ` (2 subsequent siblings) 10 siblings, 3 replies; 27+ messages in thread From: Ariana Lazar @ 2026-09-09 14:18 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-kernel, linux-iio, devicetree, Ariana Lazar 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 <ariana.lazar@microchip.com> --- MAINTAINERS | 2 +- 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 | 43 +++ 6 files changed, 222 insertions(+), 308 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 6b4560681b51cfabf0b0e98fd89b69d8c527228b..3c8c1876c7871c060223e4f0823bc7f724a77e39 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -16118,7 +16118,7 @@ M: Ariana Lazar <ariana.lazar@microchip.com> L: linux-iio@vger.kernel.org S: Supported F: Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml -F: drivers/iio/dac/mcp47feb02.c +F: drivers/iio/dac/mcp47feb02* MCP4821 DAC DRIVER M: Anshul Dalal <anshulusr@gmail.com> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig index 657c68e75542e47ad5603165bcbc3660e90c6450..cc10d43ffd1e89bbf945ef89ea5af0abe1a91084 100644 --- a/drivers/iio/dac/Kconfig +++ b/drivers/iio/dac/Kconfig @@ -550,8 +550,13 @@ config MCP4728 will be called mcp4728. 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" depends on I2C + select REGMAP_I2C + select MCP47FEB02 help Say yes here if you want to build the driver for the Microchip: - 8-bit DAC: @@ -567,7 +572,8 @@ config MCP47FEB02 (DAC) with I2C interface. To compile this driver as a module, choose M here: the module - will be called mcp47feb02. + will be called mcp47feb02_i2c and you will also get + mcp47feb02_core for the core module. config MCP4821 tristate "MCP4801/02/11/12/21/22 DAC driver" diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile index 0034317984985035f7987a744899924bfd4612e3..28c8646a987cff1ac4d693fe86898d66c901d5bd 100644 --- a/drivers/iio/dac/Makefile +++ b/drivers/iio/dac/Makefile @@ -54,7 +54,8 @@ obj-$(CONFIG_MAX5522) += max5522.o obj-$(CONFIG_MAX5821) += max5821.o obj-$(CONFIG_MCP4725) += mcp4725.o obj-$(CONFIG_MCP4728) += mcp4728.o -obj-$(CONFIG_MCP47FEB02) += mcp47feb02.o +obj-$(CONFIG_MCP47FEB02) += mcp47feb02-core.o +obj-$(CONFIG_MCP47FEB02_I2C) += mcp47feb02-i2c.o obj-$(CONFIG_MCP4821) += mcp4821.o obj-$(CONFIG_MCP4922) += mcp4922.o obj-$(CONFIG_STM32_DAC_CORE) += stm32-dac-core.o 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 fb66bd2664a9e6dd938eacc90547403041e258a4..4ef64a3408033501c724c7c52fd20686cc53722a 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 <ariana.lazar@microchip.com> * @@ -16,12 +16,10 @@ #include <linux/bitfield.h> #include <linux/delay.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,6 +28,8 @@ #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 @@ -87,220 +87,12 @@ enum mcp47feb02_gain_bit_mode { MCP47FEB02_GAIN_BIT_X2 = 1, }; -static const char * const mcp47feb02_powerdown_modes[] = { +const char * const mcp47feb02_powerdown_modes[] = { "1kohm_to_gnd", "100kohm_to_gnd", "open_circuit", }; -/** - * struct mcp47feb02_features - chip specific data - * @name: device name - * @phys_channels: number of hardware channels - * @resolution: DAC resolution - * @have_ext_vref1: does the hardware have an the second external voltage reference? - * @have_eeprom: does the hardware have an internal eeprom? - */ -struct mcp47feb02_features { - const char *name; - unsigned int phys_channels; - unsigned int resolution; - bool have_ext_vref1; - bool have_eeprom; -}; - -static const struct mcp47feb02_features mcp47feb01_chip_features = { - .name = "mcp47feb01", - .phys_channels = 1, - .resolution = 8, - .have_ext_vref1 = false, - .have_eeprom = true, -}; - -static const struct mcp47feb02_features mcp47feb02_chip_features = { - .name = "mcp47feb02", - .phys_channels = 2, - .resolution = 8, - .have_ext_vref1 = false, - .have_eeprom = true, -}; - -static const struct mcp47feb02_features mcp47feb04_chip_features = { - .name = "mcp47feb04", - .phys_channels = 4, - .resolution = 8, - .have_ext_vref1 = true, - .have_eeprom = true, -}; - -static const struct mcp47feb02_features mcp47feb08_chip_features = { - .name = "mcp47feb08", - .phys_channels = 8, - .resolution = 8, - .have_ext_vref1 = true, - .have_eeprom = true, -}; - -static const struct mcp47feb02_features mcp47feb11_chip_features = { - .name = "mcp47feb11", - .phys_channels = 1, - .resolution = 10, - .have_ext_vref1 = false, - .have_eeprom = true, -}; - -static const struct mcp47feb02_features mcp47feb12_chip_features = { - .name = "mcp47feb12", - .phys_channels = 2, - .resolution = 10, - .have_ext_vref1 = false, - .have_eeprom = true, -}; - -static const struct mcp47feb02_features mcp47feb14_chip_features = { - .name = "mcp47feb14", - .phys_channels = 4, - .resolution = 10, - .have_ext_vref1 = true, - .have_eeprom = true, -}; - -static const struct mcp47feb02_features mcp47feb18_chip_features = { - .name = "mcp47feb18", - .phys_channels = 8, - .resolution = 10, - .have_ext_vref1 = true, - .have_eeprom = true, -}; - -static const struct mcp47feb02_features mcp47feb21_chip_features = { - .name = "mcp47feb21", - .phys_channels = 1, - .resolution = 12, - .have_ext_vref1 = false, - .have_eeprom = true, -}; - -static const struct mcp47feb02_features mcp47feb22_chip_features = { - .name = "mcp47feb22", - .phys_channels = 2, - .resolution = 12, - .have_ext_vref1 = false, - .have_eeprom = true, -}; - -static const struct mcp47feb02_features mcp47feb24_chip_features = { - .name = "mcp47feb24", - .phys_channels = 4, - .resolution = 12, - .have_ext_vref1 = true, - .have_eeprom = true, -}; - -static const struct mcp47feb02_features mcp47feb28_chip_features = { - .name = "mcp47feb28", - .phys_channels = 8, - .resolution = 12, - .have_ext_vref1 = true, - .have_eeprom = true, -}; - -static const struct mcp47feb02_features mcp47fvb01_chip_features = { - .name = "mcp47fvb01", - .phys_channels = 1, - .resolution = 8, - .have_ext_vref1 = false, - .have_eeprom = false, -}; - -static const struct mcp47feb02_features mcp47fvb02_chip_features = { - .name = "mcp47fvb02", - .phys_channels = 2, - .resolution = 8, - .have_ext_vref1 = false, - .have_eeprom = false, -}; - -static const struct mcp47feb02_features mcp47fvb04_chip_features = { - .name = "mcp47fvb04", - .phys_channels = 4, - .resolution = 8, - .have_ext_vref1 = true, - .have_eeprom = false, -}; - -static const struct mcp47feb02_features mcp47fvb08_chip_features = { - .name = "mcp47fvb08", - .phys_channels = 8, - .resolution = 8, - .have_ext_vref1 = true, - .have_eeprom = false, -}; - -static const struct mcp47feb02_features mcp47fvb11_chip_features = { - .name = "mcp47fvb11", - .phys_channels = 1, - .resolution = 10, - .have_ext_vref1 = false, - .have_eeprom = false, -}; - -static const struct mcp47feb02_features mcp47fvb12_chip_features = { - .name = "mcp47fvb12", - .phys_channels = 2, - .resolution = 10, - .have_ext_vref1 = false, - .have_eeprom = false, -}; - -static const struct mcp47feb02_features mcp47fvb14_chip_features = { - .name = "mcp47fvb14", - .phys_channels = 4, - .resolution = 10, - .have_ext_vref1 = true, - .have_eeprom = false, -}; - -static const struct mcp47feb02_features mcp47fvb18_chip_features = { - .name = "mcp47fvb18", - .phys_channels = 8, - .resolution = 10, - .have_ext_vref1 = true, - .have_eeprom = false, -}; - -static const struct mcp47feb02_features mcp47fvb21_chip_features = { - .name = "mcp47fvb21", - .phys_channels = 1, - .resolution = 12, - .have_ext_vref1 = false, - .have_eeprom = false, -}; - -static const struct mcp47feb02_features mcp47fvb22_chip_features = { - .name = "mcp47fvb22", - .phys_channels = 2, - .resolution = 12, - .have_ext_vref1 = false, - .have_eeprom = false, -}; - -static const struct mcp47feb02_features mcp47fvb24_chip_features = { - .name = "mcp47fvb24", - .phys_channels = 4, - .resolution = 12, - .have_ext_vref1 = true, - .have_eeprom = false, -}; - -static const struct mcp47feb02_features mcp47fvb28_chip_features = { - .name = "mcp47fvb28", - .phys_channels = 8, - .resolution = 12, - .have_ext_vref1 = true, - .have_eeprom = false, -}; - /** * struct mcp47feb02_channel_data - channel configuration * @ref_mode: chosen voltage for reference @@ -381,7 +173,7 @@ static const struct regmap_access_table mcp47feb02_volatile_table = { .n_yes_ranges = ARRAY_SIZE(mcp47feb02_volatile_ranges), }; -static const struct regmap_config mcp47feb02_regmap_config = { +const struct regmap_config mcp47feb02_regmap_config = { .name = "mcp47feb02_regmap", .reg_bits = 8, .val_bits = 16, @@ -393,6 +185,7 @@ static const struct regmap_config mcp47feb02_regmap_config = { .cache_type = REGCACHE_MAPLE, .val_format_endian = REGMAP_ENDIAN_BIG, }; +EXPORT_SYMBOL_NS_GPL(mcp47feb02_regmap_config, "IIO_MCP47FEB02"); /* For devices that doesn't have nonvolatile memory */ static const struct regmap_range mcp47fvb02_readable_ranges[] = { @@ -423,7 +216,7 @@ static const struct regmap_access_table mcp47fvb02_volatile_table = { .n_yes_ranges = ARRAY_SIZE(mcp47fvb02_volatile_ranges), }; -static const struct regmap_config mcp47fvb02_regmap_config = { +const struct regmap_config mcp47fvb02_regmap_config = { .name = "mcp47fvb02_regmap", .reg_bits = 8, .val_bits = 16, @@ -435,6 +228,7 @@ static const struct regmap_config mcp47fvb02_regmap_config = { .cache_type = REGCACHE_MAPLE, .val_format_endian = REGMAP_ENDIAN_BIG, }; +EXPORT_SYMBOL_NS_GPL(mcp47fvb02_regmap_config, "IIO_MCP47FEB02"); static int mcp47feb02_write_to_eeprom(struct mcp47feb02_data *data, unsigned int reg, unsigned int val) @@ -445,8 +239,6 @@ static int mcp47feb02_write_to_eeprom(struct mcp47feb02_data *data, unsigned int * Wait until the currently occurring EEPROM Write Cycle is completed. * Only serial commands to the volatile memory are allowed. */ - guard(mutex)(&data->lock); - ret = regmap_read_poll_timeout(data->regmap, MCP47FEB02_GAIN_CTRL_STATUS_REG_ADDR, eewa_val, !(eewa_val & MCP47FEB02_GAIN_BIT_STATUS_EEWA_MASK), @@ -472,6 +264,8 @@ static ssize_t store_eeprom_store(struct device *dev, struct device_attribute *a if (!state) return len; + guard(mutex)(&data->lock); + /* * Verify DAC Wiper and DAC Configuration are unlocked. If both are disabled, * writing to EEPROM is available. @@ -578,18 +372,17 @@ static int mcp47feb02_resume(struct device *dev) guard(mutex)(&data->lock); for_each_set_bit(ch, &data->active_channels_mask, data->phys_channels) { - u8 pd_mode; int ret; data->chdata[ch].powerdown = false; - pd_mode = data->chdata[ch].powerdown_mode + 1; ret = regmap_write(data->regmap, REG_ADDR(ch), data->chdata[ch].dac_data); if (ret) return ret; ret = regmap_update_bits(data->regmap, MCP47FEB02_VREF_REG_ADDR, - DAC_CTRL_MASK(ch), DAC_CTRL_VAL(ch, pd_mode)); + DAC_CTRL_MASK(ch), + DAC_CTRL_VAL(ch, data->chdata[ch].ref_mode)); if (ret) return ret; @@ -667,7 +460,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); static const struct iio_enum mcp47febxx_powerdown_mode_enum = { .items = mcp47feb02_powerdown_modes, @@ -1095,10 +888,10 @@ static int mcp47feb02_init_ch_scales(struct mcp47feb02_data *data, int vdd_uV, return 0; } -static int mcp47feb02_probe(struct i2c_client *client) +int mcp47feb02_common_probe(const struct mcp47feb02_features *chip_features, + struct regmap *regmap) { - const struct mcp47feb02_features *chip_features; - struct device *dev = &client->dev; + struct device *dev = regmap_get_device(regmap); struct mcp47feb02_data *data; struct iio_dev *indio_dev; int vref1_uV, vref_uV, vdd_uV, ret; @@ -1107,22 +900,16 @@ static int mcp47feb02_probe(struct i2c_client *client) if (!indio_dev) return -ENOMEM; - data = iio_priv(indio_dev); - chip_features = i2c_get_match_data(client); - if (!chip_features) - return -EINVAL; + dev_set_drvdata(dev, indio_dev); + data = iio_priv(indio_dev); data->chip_features = chip_features; + data->regmap = regmap; - if (chip_features->have_eeprom) { - data->regmap = devm_regmap_init_i2c(client, &mcp47feb02_regmap_config); + if (chip_features->have_eeprom) indio_dev->info = &mcp47feb02_info; - } else { - data->regmap = devm_regmap_init_i2c(client, &mcp47fvb02_regmap_config); + else indio_dev->info = &mcp47fvb02_info; - } - if (IS_ERR(data->regmap)) - return dev_err_probe(dev, PTR_ERR(data->regmap), "Error initializing i2c regmap\n"); indio_dev->name = chip_features->name; @@ -1179,75 +966,7 @@ static int mcp47feb02_probe(struct i2c_client *client) return devm_iio_device_register(dev, indio_dev); } - -static const struct i2c_device_id mcp47feb02_id[] = { - { .name = "mcp47feb01", .driver_data = (kernel_ulong_t)&mcp47feb01_chip_features }, - { .name = "mcp47feb02", .driver_data = (kernel_ulong_t)&mcp47feb02_chip_features }, - { .name = "mcp47feb04", .driver_data = (kernel_ulong_t)&mcp47feb04_chip_features }, - { .name = "mcp47feb08", .driver_data = (kernel_ulong_t)&mcp47feb08_chip_features }, - { .name = "mcp47feb11", .driver_data = (kernel_ulong_t)&mcp47feb11_chip_features }, - { .name = "mcp47feb12", .driver_data = (kernel_ulong_t)&mcp47feb12_chip_features }, - { .name = "mcp47feb14", .driver_data = (kernel_ulong_t)&mcp47feb14_chip_features }, - { .name = "mcp47feb18", .driver_data = (kernel_ulong_t)&mcp47feb18_chip_features }, - { .name = "mcp47feb21", .driver_data = (kernel_ulong_t)&mcp47feb21_chip_features }, - { .name = "mcp47feb22", .driver_data = (kernel_ulong_t)&mcp47feb22_chip_features }, - { .name = "mcp47feb24", .driver_data = (kernel_ulong_t)&mcp47feb24_chip_features }, - { .name = "mcp47feb28", .driver_data = (kernel_ulong_t)&mcp47feb28_chip_features }, - { .name = "mcp47fvb01", .driver_data = (kernel_ulong_t)&mcp47fvb01_chip_features }, - { .name = "mcp47fvb02", .driver_data = (kernel_ulong_t)&mcp47fvb02_chip_features }, - { .name = "mcp47fvb04", .driver_data = (kernel_ulong_t)&mcp47fvb04_chip_features }, - { .name = "mcp47fvb08", .driver_data = (kernel_ulong_t)&mcp47fvb08_chip_features }, - { .name = "mcp47fvb11", .driver_data = (kernel_ulong_t)&mcp47fvb11_chip_features }, - { .name = "mcp47fvb12", .driver_data = (kernel_ulong_t)&mcp47fvb12_chip_features }, - { .name = "mcp47fvb14", .driver_data = (kernel_ulong_t)&mcp47fvb14_chip_features }, - { .name = "mcp47fvb18", .driver_data = (kernel_ulong_t)&mcp47fvb18_chip_features }, - { .name = "mcp47fvb21", .driver_data = (kernel_ulong_t)&mcp47fvb21_chip_features }, - { .name = "mcp47fvb22", .driver_data = (kernel_ulong_t)&mcp47fvb22_chip_features }, - { .name = "mcp47fvb24", .driver_data = (kernel_ulong_t)&mcp47fvb24_chip_features }, - { .name = "mcp47fvb28", .driver_data = (kernel_ulong_t)&mcp47fvb28_chip_features }, - { } -}; -MODULE_DEVICE_TABLE(i2c, mcp47feb02_id); - -static const struct of_device_id mcp47feb02_of_match[] = { - { .compatible = "microchip,mcp47feb01", .data = &mcp47feb01_chip_features }, - { .compatible = "microchip,mcp47feb02", .data = &mcp47feb02_chip_features }, - { .compatible = "microchip,mcp47feb04", .data = &mcp47feb04_chip_features }, - { .compatible = "microchip,mcp47feb08", .data = &mcp47feb08_chip_features }, - { .compatible = "microchip,mcp47feb11", .data = &mcp47feb11_chip_features }, - { .compatible = "microchip,mcp47feb12", .data = &mcp47feb12_chip_features }, - { .compatible = "microchip,mcp47feb14", .data = &mcp47feb14_chip_features }, - { .compatible = "microchip,mcp47feb18", .data = &mcp47feb18_chip_features }, - { .compatible = "microchip,mcp47feb21", .data = &mcp47feb21_chip_features }, - { .compatible = "microchip,mcp47feb22", .data = &mcp47feb22_chip_features }, - { .compatible = "microchip,mcp47feb24", .data = &mcp47feb24_chip_features }, - { .compatible = "microchip,mcp47feb28", .data = &mcp47feb28_chip_features }, - { .compatible = "microchip,mcp47fvb01", .data = &mcp47fvb01_chip_features }, - { .compatible = "microchip,mcp47fvb02", .data = &mcp47fvb02_chip_features }, - { .compatible = "microchip,mcp47fvb04", .data = &mcp47fvb04_chip_features }, - { .compatible = "microchip,mcp47fvb08", .data = &mcp47fvb08_chip_features }, - { .compatible = "microchip,mcp47fvb11", .data = &mcp47fvb11_chip_features }, - { .compatible = "microchip,mcp47fvb12", .data = &mcp47fvb12_chip_features }, - { .compatible = "microchip,mcp47fvb14", .data = &mcp47fvb14_chip_features }, - { .compatible = "microchip,mcp47fvb18", .data = &mcp47fvb18_chip_features }, - { .compatible = "microchip,mcp47fvb21", .data = &mcp47fvb21_chip_features }, - { .compatible = "microchip,mcp47fvb22", .data = &mcp47fvb22_chip_features }, - { .compatible = "microchip,mcp47fvb24", .data = &mcp47fvb24_chip_features }, - { .compatible = "microchip,mcp47fvb28", .data = &mcp47fvb28_chip_features }, - { } -}; -MODULE_DEVICE_TABLE(of, mcp47feb02_of_match); - -static struct i2c_driver mcp47feb02_driver = { - .driver = { - .name = "mcp47feb02", - .of_match_table = mcp47feb02_of_match, - .pm = pm_sleep_ptr(&mcp47feb02_pm_ops), - }, - .probe = mcp47feb02_probe, - .id_table = mcp47feb02_id, -}; -module_i2c_driver(mcp47feb02_driver); +EXPORT_SYMBOL_NS(mcp47feb02_common_probe, "IIO_MCP47FEB02"); MODULE_AUTHOR("Ariana Lazar <ariana.lazar@microchip.com>"); MODULE_DESCRIPTION("IIO driver for MCP47FEB02 Multi-Channel DAC with I2C interface"); diff --git a/drivers/iio/dac/mcp47feb02-i2c.c b/drivers/iio/dac/mcp47feb02-i2c.c new file mode 100644 index 0000000000000000000000000000000000000000..7e49503e3abc91225b6ede89fdab03f5ca3799f3 --- /dev/null +++ b/drivers/iio/dac/mcp47feb02-i2c.c @@ -0,0 +1,145 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * IIO driver for MCP47FEB02 Multi-Channel DAC with I2C interface + * + * Copyright (C) 2025-2026 Microchip Technology Inc. and its subsidiaries + * + * Author: Ariana Lazar <ariana.lazar@microchip.com> + * + * Datasheet links for devices with I2C interface: + * [MCP47FEBxx] https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005375A.pdf + * [MCP47FVBxx] https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005405A.pdf + * [MCP47FxBx4/8] https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP47FXBX48-Data-Sheet-DS200006368A.pdf + */ +#include <linux/dev_printk.h> +#include <linux/err.h> +#include <linux/i2c.h> +#include <linux/module.h> +#include <linux/pm.h> +#include <linux/regmap.h> +#include <linux/types.h> + +#include "mcp47feb02.h" + +/* Parts with EEPROM memory */ +MCP47FEB02_CHIP_INFO(mcp47feb01, 1, 8, false, true); +MCP47FEB02_CHIP_INFO(mcp47feb02, 2, 8, false, true); +MCP47FEB02_CHIP_INFO(mcp47feb04, 4, 8, true, true); +MCP47FEB02_CHIP_INFO(mcp47feb08, 8, 8, true, true); +MCP47FEB02_CHIP_INFO(mcp47feb11, 1, 10, false, true); +MCP47FEB02_CHIP_INFO(mcp47feb12, 2, 10, false, true); +MCP47FEB02_CHIP_INFO(mcp47feb14, 4, 10, true, true); +MCP47FEB02_CHIP_INFO(mcp47feb18, 8, 10, true, true); +MCP47FEB02_CHIP_INFO(mcp47feb21, 1, 12, false, true); +MCP47FEB02_CHIP_INFO(mcp47feb22, 2, 12, false, true); +MCP47FEB02_CHIP_INFO(mcp47feb24, 4, 12, true, true); +MCP47FEB02_CHIP_INFO(mcp47feb28, 8, 12, true, true); + +/* Parts without EEPROM memory */ +MCP47FEB02_CHIP_INFO(mcp47fvb01, 1, 8, false, false); +MCP47FEB02_CHIP_INFO(mcp47fvb02, 2, 8, false, false); +MCP47FEB02_CHIP_INFO(mcp47fvb04, 4, 8, true, false); +MCP47FEB02_CHIP_INFO(mcp47fvb08, 8, 8, true, false); +MCP47FEB02_CHIP_INFO(mcp47fvb11, 1, 10, false, false); +MCP47FEB02_CHIP_INFO(mcp47fvb12, 2, 10, false, false); +MCP47FEB02_CHIP_INFO(mcp47fvb14, 4, 10, true, false); +MCP47FEB02_CHIP_INFO(mcp47fvb18, 8, 10, true, false); +MCP47FEB02_CHIP_INFO(mcp47fvb21, 1, 12, false, false); +MCP47FEB02_CHIP_INFO(mcp47fvb22, 2, 12, false, false); +MCP47FEB02_CHIP_INFO(mcp47fvb24, 4, 12, true, false); +MCP47FEB02_CHIP_INFO(mcp47fvb28, 8, 12, true, false); + +static int mcp47feb02_i2c_probe(struct i2c_client *client) +{ + const struct mcp47feb02_features *chip_features; + struct device *dev = &client->dev; + struct regmap *regmap; + + chip_features = i2c_get_match_data(client); + if (!chip_features) + return dev_err_probe(dev, -ENODEV, "No I2C device found\n"); + + if (chip_features->have_eeprom) + regmap = devm_regmap_init_i2c(client, &mcp47feb02_regmap_config); + else + regmap = devm_regmap_init_i2c(client, &mcp47fvb02_regmap_config); + + if (IS_ERR(regmap)) + return dev_err_probe(dev, PTR_ERR(regmap), "Error initializing I2C regmap\n"); + + return mcp47feb02_common_probe(chip_features, regmap); +} + +static const struct i2c_device_id mcp47feb02_i2c_id[] = { + { .name = "mcp47feb01", .driver_data = (kernel_ulong_t)&mcp47feb01_chip_features }, + { .name = "mcp47feb02", .driver_data = (kernel_ulong_t)&mcp47feb02_chip_features }, + { .name = "mcp47feb04", .driver_data = (kernel_ulong_t)&mcp47feb04_chip_features }, + { .name = "mcp47feb08", .driver_data = (kernel_ulong_t)&mcp47feb08_chip_features }, + { .name = "mcp47feb11", .driver_data = (kernel_ulong_t)&mcp47feb11_chip_features }, + { .name = "mcp47feb12", .driver_data = (kernel_ulong_t)&mcp47feb12_chip_features }, + { .name = "mcp47feb14", .driver_data = (kernel_ulong_t)&mcp47feb14_chip_features }, + { .name = "mcp47feb18", .driver_data = (kernel_ulong_t)&mcp47feb18_chip_features }, + { .name = "mcp47feb21", .driver_data = (kernel_ulong_t)&mcp47feb21_chip_features }, + { .name = "mcp47feb22", .driver_data = (kernel_ulong_t)&mcp47feb22_chip_features }, + { .name = "mcp47feb24", .driver_data = (kernel_ulong_t)&mcp47feb24_chip_features }, + { .name = "mcp47feb28", .driver_data = (kernel_ulong_t)&mcp47feb28_chip_features }, + { .name = "mcp47fvb01", .driver_data = (kernel_ulong_t)&mcp47fvb01_chip_features }, + { .name = "mcp47fvb02", .driver_data = (kernel_ulong_t)&mcp47fvb02_chip_features }, + { .name = "mcp47fvb04", .driver_data = (kernel_ulong_t)&mcp47fvb04_chip_features }, + { .name = "mcp47fvb08", .driver_data = (kernel_ulong_t)&mcp47fvb08_chip_features }, + { .name = "mcp47fvb11", .driver_data = (kernel_ulong_t)&mcp47fvb11_chip_features }, + { .name = "mcp47fvb12", .driver_data = (kernel_ulong_t)&mcp47fvb12_chip_features }, + { .name = "mcp47fvb14", .driver_data = (kernel_ulong_t)&mcp47fvb14_chip_features }, + { .name = "mcp47fvb18", .driver_data = (kernel_ulong_t)&mcp47fvb18_chip_features }, + { .name = "mcp47fvb21", .driver_data = (kernel_ulong_t)&mcp47fvb21_chip_features }, + { .name = "mcp47fvb22", .driver_data = (kernel_ulong_t)&mcp47fvb22_chip_features }, + { .name = "mcp47fvb24", .driver_data = (kernel_ulong_t)&mcp47fvb24_chip_features }, + { .name = "mcp47fvb28", .driver_data = (kernel_ulong_t)&mcp47fvb28_chip_features }, + { } +}; +MODULE_DEVICE_TABLE(i2c, mcp47feb02_i2c_id); + +static const struct of_device_id mcp47feb02_of_i2c_match[] = { + { .compatible = "microchip,mcp47feb01", .data = &mcp47feb01_chip_features }, + { .compatible = "microchip,mcp47feb02", .data = &mcp47feb02_chip_features }, + { .compatible = "microchip,mcp47feb04", .data = &mcp47feb04_chip_features }, + { .compatible = "microchip,mcp47feb08", .data = &mcp47feb08_chip_features }, + { .compatible = "microchip,mcp47feb11", .data = &mcp47feb11_chip_features }, + { .compatible = "microchip,mcp47feb12", .data = &mcp47feb12_chip_features }, + { .compatible = "microchip,mcp47feb14", .data = &mcp47feb14_chip_features }, + { .compatible = "microchip,mcp47feb18", .data = &mcp47feb18_chip_features }, + { .compatible = "microchip,mcp47feb21", .data = &mcp47feb21_chip_features }, + { .compatible = "microchip,mcp47feb22", .data = &mcp47feb22_chip_features }, + { .compatible = "microchip,mcp47feb24", .data = &mcp47feb24_chip_features }, + { .compatible = "microchip,mcp47feb28", .data = &mcp47feb28_chip_features }, + { .compatible = "microchip,mcp47fvb01", .data = &mcp47fvb01_chip_features }, + { .compatible = "microchip,mcp47fvb02", .data = &mcp47fvb02_chip_features }, + { .compatible = "microchip,mcp47fvb04", .data = &mcp47fvb04_chip_features }, + { .compatible = "microchip,mcp47fvb08", .data = &mcp47fvb08_chip_features }, + { .compatible = "microchip,mcp47fvb11", .data = &mcp47fvb11_chip_features }, + { .compatible = "microchip,mcp47fvb12", .data = &mcp47fvb12_chip_features }, + { .compatible = "microchip,mcp47fvb14", .data = &mcp47fvb14_chip_features }, + { .compatible = "microchip,mcp47fvb18", .data = &mcp47fvb18_chip_features }, + { .compatible = "microchip,mcp47fvb21", .data = &mcp47fvb21_chip_features }, + { .compatible = "microchip,mcp47fvb22", .data = &mcp47fvb22_chip_features }, + { .compatible = "microchip,mcp47fvb24", .data = &mcp47fvb24_chip_features }, + { .compatible = "microchip,mcp47fvb28", .data = &mcp47fvb28_chip_features }, + { } +}; +MODULE_DEVICE_TABLE(of, mcp47feb02_of_i2c_match); + +static struct i2c_driver mcp47feb02_i2c_driver = { + .driver = { + .name = "mcp47feb02", + .of_match_table = mcp47feb02_of_i2c_match, + .pm = pm_sleep_ptr(&mcp47feb02_pm_ops), + }, + .probe = mcp47feb02_i2c_probe, + .id_table = mcp47feb02_i2c_id, +}; +module_i2c_driver(mcp47feb02_i2c_driver); + +MODULE_AUTHOR("Ariana Lazar <ariana.lazar@microchip.com>"); +MODULE_DESCRIPTION("IIO driver for MCP47FEB02 Multi-Channel DAC with I2C interface"); +MODULE_LICENSE("GPL"); +MODULE_IMPORT_NS("IIO_MCP47FEB02"); diff --git a/drivers/iio/dac/mcp47feb02.h b/drivers/iio/dac/mcp47feb02.h new file mode 100644 index 0000000000000000000000000000000000000000..5bcce1f9ae112c4fecc930f9927442eb93832f20 --- /dev/null +++ b/drivers/iio/dac/mcp47feb02.h @@ -0,0 +1,43 @@ +/* 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[]; + +#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, \ +} + +/** + * struct mcp47feb02_features - chip specific data + * @name: device name + * @phys_channels: number of hardware channels + * @resolution: DAC resolution + * @have_ext_vref1: does the hardware have an the second external voltage reference? + * @have_eeprom: does the hardware have an internal eeprom? + */ +struct mcp47feb02_features { + const char *name; + unsigned int phys_channels; + unsigned int resolution; + bool have_ext_vref1; + bool have_eeprom; +}; + +extern const struct regmap_config mcp47feb02_regmap_config; +extern const struct regmap_config mcp47fvb02_regmap_config; + +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__ */ -- 2.43.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v5 08/10] iio: dac: mcp47feb02: refactor MCP47FEB02 I2C driver into two modules 2026-09-09 14:18 ` [PATCH v5 08/10] iio: dac: mcp47feb02: refactor MCP47FEB02 I2C driver into two modules Ariana Lazar @ 2026-09-09 14:47 ` sashiko-bot 2026-09-10 6:16 ` Andy Shevchenko 2026-09-13 21:51 ` Jonathan Cameron 2 siblings, 0 replies; 27+ messages in thread From: sashiko-bot @ 2026-09-09 14:47 UTC (permalink / raw) To: Ariana Lazar; +Cc: conor+dt, robh, devicetree Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Renaming the visible Kconfig symbol breaks existing configurations by silently dropping the driver from user builds. -- commit 528e4c3512ab81e838d441fdb1209303ed961a7f Author: Ariana Lazar <ariana.lazar@microchip.com> iio: dac: mcp47feb02: refactor MCP47FEB02 I2C driver into two modules This patch prepares the MCP47FEB02 driver for bus-specific code by splitting the existing implementation into separate core and I2C modules. It introduces a new I2C-specific Kconfig option to support this separation. > diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig > index 657c68e75542e..cc10d43ffd1e8 100644 > --- a/drivers/iio/dac/Kconfig > +++ b/drivers/iio/dac/Kconfig [ ... ] > @@ -550,8 +550,13 @@ config MCP4728 > will be called mcp4728. > > 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 removing the prompt from CONFIG_MCP47FEB02 break existing configurations? Users upgrading their kernels with an existing .config where CONFIG_MCP47FEB02=y or m will silently lose support for this DAC. When they run make oldconfig, the module will be dropped because the new visible symbol, CONFIG_MCP47FEB02_I2C, defaults to 'n' and nothing automatically selects it. Could a transition path be provided to prevent this regression, perhaps by defaulting the new I2C driver to the value of the old configuration, or by using a configuration scheme that automatically enables the I2C driver if the old symbol was previously selected? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260909-mcp47feb02_refactor-v5-0-8b67bcab93d1@microchip.com?part=8 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5 08/10] iio: dac: mcp47feb02: refactor MCP47FEB02 I2C driver into two modules 2026-09-09 14:18 ` [PATCH v5 08/10] iio: dac: mcp47feb02: refactor MCP47FEB02 I2C driver into two modules Ariana Lazar 2026-09-09 14:47 ` sashiko-bot @ 2026-09-10 6:16 ` Andy Shevchenko 2026-09-13 21:51 ` Jonathan Cameron 2 siblings, 0 replies; 27+ messages in thread From: Andy Shevchenko @ 2026-09-10 6:16 UTC (permalink / raw) To: Ariana Lazar Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-iio, devicetree On Wed, Sep 09, 2026 at 05:18:51PM +0300, Ariana Lazar 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. ... > +static int mcp47feb02_i2c_probe(struct i2c_client *client) > +{ > + const struct mcp47feb02_features *chip_features; > + struct device *dev = &client->dev; > + struct regmap *regmap; > + > + chip_features = i2c_get_match_data(client); > + if (!chip_features) > + return dev_err_probe(dev, -ENODEV, "No I2C device found\n"); Consider adding a patch that replaces ENODEV with ENODATA as we started doing in the other drivers. This will help to distinguish the source of the error. Also fix the error message accordingly. It's not about device. > + if (chip_features->have_eeprom) > + regmap = devm_regmap_init_i2c(client, &mcp47feb02_regmap_config); > + else > + regmap = devm_regmap_init_i2c(client, &mcp47fvb02_regmap_config); > + > + if (IS_ERR(regmap)) > + return dev_err_probe(dev, PTR_ERR(regmap), "Error initializing I2C regmap\n"); > + > + return mcp47feb02_common_probe(chip_features, regmap); > +} ... > +#ifndef __DRIVERS_IIO_DAC_MCP47FEB02_H__ > +#define __DRIVERS_IIO_DAC_MCP47FEB02_H__ > + > +#include <linux/pm.h> + regmap.h for struct regmap_config. > +#include <linux/types.h> > +#endif /* __DRIVERS_IIO_DAC_MCP47FEB02_H__ */ -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5 08/10] iio: dac: mcp47feb02: refactor MCP47FEB02 I2C driver into two modules 2026-09-09 14:18 ` [PATCH v5 08/10] iio: dac: mcp47feb02: refactor MCP47FEB02 I2C driver into two modules Ariana Lazar 2026-09-09 14:47 ` sashiko-bot 2026-09-10 6:16 ` Andy Shevchenko @ 2026-09-13 21:51 ` Jonathan Cameron 2 siblings, 0 replies; 27+ messages in thread From: Jonathan Cameron @ 2026-09-13 21:51 UTC (permalink / raw) To: Ariana Lazar Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-iio, devicetree On Wed, 9 Sep 2026 17:18:51 +0300 Ariana Lazar <ariana.lazar@microchip.com> 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 <ariana.lazar@microchip.com> One thing inline. > 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 fb66bd2664a9e6dd938eacc90547403041e258a4..4ef64a3408033501c724c7c52fd20686cc53722a 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 <ariana.lazar@microchip.com> > * > @@ -16,12 +16,10 @@ > #include <linux/bitfield.h> > #include <linux/delay.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> This surprised me because I thought this was no longer included from any drivers using i2c. Indeed it is gone since : 995832b2cebe "(Replace <linux/mod_devicetable.h> by more specific <linux/device-id/*.h> (c files)") Hence please rebase on 7.3-rc1. I don't think we have anything more recent that touches this driver and a mainline rc is an easy base to deal with. > #include <linux/mutex.h> > #include <linux/property.h> > #include <linux/regmap.h> > @@ -30,6 +28,8 @@ > #include <linux/types.h> > #include <linux/units.h> > > +#include "mcp47feb02.h" ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 09/10] dt-bindings: iio: dac: add support for MCP48FEB02 SPI 2026-09-09 14:18 [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver Ariana Lazar ` (7 preceding siblings ...) 2026-09-09 14:18 ` [PATCH v5 08/10] iio: dac: mcp47feb02: refactor MCP47FEB02 I2C driver into two modules Ariana Lazar @ 2026-09-09 14:18 ` Ariana Lazar 2026-09-09 14:48 ` sashiko-bot 2026-09-09 14:18 ` [PATCH v5 10/10] iio: dac: add support for Microchip MCP48FEB02 Ariana Lazar 2026-09-10 6:18 ` [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver Andy Shevchenko 10 siblings, 1 reply; 27+ messages in thread From: Ariana Lazar @ 2026-09-09 14:18 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-kernel, linux-iio, devicetree, Ariana Lazar Add the SPI MCP48FxBy1/2/4/8 part numbers, the spi-max-frequency property and a devicetree example for SPI usage to the existing binding. Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> Reviewed-by: Rob Herring (Arm) <robh@kernel.org> --- .../bindings/iio/dac/microchip,mcp47feb02.yaml | 221 ++++++++++++++++----- 1 file changed, 173 insertions(+), 48 deletions(-) diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml index d131f136bd15161666932261bcacd6d124b23ede..a86d094f0d4b54f9d9cc39fd54222f5968714668 100644 --- a/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml @@ -4,58 +4,92 @@ $id: http://devicetree.org/schemas/iio/dac/microchip,mcp47feb02.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: Microchip MCP47F(E/V)B(0/1/2)(1/2/4/8) DAC with I2C Interface Families +title: Microchip MCP4(7/8)F(E/V)B(0/1/2)(1/2/4/8) DAC with I2C/SPI Interface Families maintainers: - Ariana Lazar <ariana.lazar@microchip.com> description: | - Datasheet for MCP47FEB01, MCP47FEB11, MCP47FEB21, MCP47FEB02, MCP47FEB12, - MCP47FEB22 can be found here: - https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005375A.pdf - Datasheet for MCP47FVB01, MCP47FVB11, MCP47FVB21, MCP47FVB02, MCP47FVB12, - MCP47FVB22 can be found here: - https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005405A.pdf - Datasheet for MCP47FEB04, MCP47FEB14, MCP47FEB24, MCP47FEB08, MCP47FEB18, - MCP47FEB28, MCP47FVB04, MCP47FVB14, MCP47FVB24, MCP47FVB08, MCP47FVB18, - MCP47FVB28 can be found here: - https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP47FXBX48-Data-Sheet-DS200006368A.pdf - - +------------+--------------+-------------+-------------+------------+ - | Device | Resolution | Channels | Vref number | Memory | - |------------|--------------|-------------|-------------|------------| - | MCP47FEB01 | 8-bit | 1 | 1 | EEPROM | - | MCP47FEB11 | 10-bit | 1 | 1 | EEPROM | - | MCP47FEB21 | 12-bit | 1 | 1 | EEPROM | - |------------|--------------|-------------|-------------|------------| - | MCP47FEB02 | 8-bit | 2 | 1 | EEPROM | - | MCP47FEB12 | 10-bit | 2 | 1 | EEPROM | - | MCP47FEB22 | 12-bit | 2 | 1 | EEPROM | - |------------|--------------|-------------|-------------|------------| - | MCP47FVB01 | 8-bit | 1 | 1 | RAM | - | MCP47FVB11 | 10-bit | 1 | 1 | RAM | - | MCP47FVB21 | 12-bit | 1 | 1 | RAM | - |------------|--------------|-------------|-------------|------------| - | MCP47FVB02 | 8-bit | 2 | 1 | RAM | - | MCP47FVB12 | 10-bit | 2 | 1 | RAM | - | MCP47FVB22 | 12-bit | 2 | 1 | RAM | - |------------|--------------|-------------|-------------|------------| - | MCP47FVB04 | 8-bit | 4 | 2 | RAM | - | MCP47FVB14 | 10-bit | 4 | 2 | RAM | - | MCP47FVB24 | 12-bit | 4 | 2 | RAM | - |------------|--------------|-------------|-------------|------------| - | MCP47FVB08 | 8-bit | 8 | 2 | RAM | - | MCP47FVB18 | 10-bit | 8 | 2 | RAM | - | MCP47FVB28 | 12-bit | 8 | 2 | RAM | - |------------|--------------|-------------|-------------|------------| - | MCP47FEB04 | 8-bit | 4 | 2 | EEPROM | - | MCP47FEB14 | 10-bit | 4 | 2 | EEPROM | - | MCP47FEB24 | 12-bit | 4 | 2 | EEPROM | - |------------|--------------|-------------|-------------|------------| - | MCP47FEB08 | 8-bit | 8 | 2 | EEPROM | - | MCP47FEB18 | 10-bit | 8 | 2 | EEPROM | - | MCP47FEB28 | 12-bit | 8 | 2 | EEPROM | - +------------+--------------+-------------+-------------+------------+ + Datasheets for MCP47F(E/V)B(0/1/2)(1/2/4/8) DAC with I2C Interface Families: + Datasheet for MCP47FEB01, MCP47FEB02, MCP47FEB11, MCP47FEB12, MCP47FEB21, + MCP47FEB22 can be found here: + https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005375A.pdf + Datasheet for MCP47FVB01, MCP47FVB02, MCP47FVB11, MCP47FVB12, MCP47FVB21, + MCP47FVB22 can be found here: + https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005405A.pdf + Datasheet for MCP47FEB04, MCP47FEB08, MCP47FEB14, MCP47FEB18, MCP47FEB24, + MCP47FEB28, MCP47FVB04, MCP47FVB08, MCP47FVB14, MCP47FVB18, MCP47FVB24, + MCP47FVB28 can be found here: + https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP47FXBX48-Data-Sheet-DS200006368A.pdf + Datasheets for MCP48F(E/V)B(0/1/2)(1/2/4/8) DAC with SPI Interface Families: + Datasheet for MCP48FEB01, MCP48FEB02, MCP48FEB11, MCP48FEB12, MCP48FEB21, + MCP48FEB22 can be found here: + https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005429B.pdf + Datasheet for MCP48FVB01, MCP48FVB02, MCP48FVB11, MCP48FVB12, MCP48FVB21, + MCP48FVB22 can be found here: + https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005466A.pdf + Datasheet for MCP48FEB04, MCP48FEB14, MCP48FEB24, MCP48FEB08, MCP48FEB18, + MCP48FEB28, MCP48FVB04, MCP48FVB14, MCP48FVB24, MCP48FVB08, MCP48FVB18, + MCP48FVB28 can be found here: + https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP48FXBX4-8-Family-Data-Sheet-DS20006362A.pdf + + +------------+--------------+-------------+-------------+------------+------------+ + | Device | Resolution | Channels | Vref number | Memory | Interface | + |------------|--------------|-------------|-------------|------------|------------| + | MCP47FEB01 | | 1 | 1 | EEPROM | I2C | + | MCP47FEB02 | | 2 | 1 | EEPROM | I2C | + | MCP47FEB04 | | 4 | 2 | EEPROM | I2C | + | MCP47FEB08 | 8-bit | 8 | 2 | EEPROM | I2C | + | MCP48FEB01 | | 1 | 1 | EEPROM | SPI | + | MCP48FEB02 | | 2 | 1 | EEPROM | SPI | + | MCP48FEB04 | | 4 | 2 | EEPROM | SPI | + | MCP48FEB08 | | 8 | 2 | EEPROM | SPI | + |------------|--------------|-------------|-------------|------------|------------| + | MCP47FEB11 | | 1 | 1 | EEPROM | I2C | + | MCP47FEB12 | | 2 | 1 | EEPROM | I2C | + | MCP47FEB14 | | 4 | 2 | EEPROM | I2C | + | MCP47FEB18 | 10-bit | 8 | 2 | EEPROM | I2C | + | MCP48FEB11 | | 1 | 1 | EEPROM | SPI | + | MCP48FEB12 | | 2 | 1 | EEPROM | SPI | + | MCP48FEB14 | | 4 | 2 | EEPROM | SPI | + | MCP48FEB18 | | 8 | 2 | EEPROM | SPI | + |------------|--------------|-------------|-------------|------------|------------| + | MCP47FEB21 | | 1 | 1 | EEPROM | I2C | + | MCP47FEB22 | | 2 | 1 | EEPROM | I2C | + | MCP47FEB24 | | 4 | 2 | EEPROM | I2C | + | MCP47FEB28 | 12-bit | 8 | 2 | EEPROM | I2C | + | MCP48FEB21 | | 1 | 1 | EEPROM | SPI | + | MCP48FEB22 | | 2 | 1 | EEPROM | SPI | + | MCP48FEB24 | | 4 | 2 | EEPROM | SPI | + | MCP48FEB28 | | 8 | 2 | EEPROM | SPI | + |------------|--------------|-------------|-------------|------------|------------| + | MCP47FVB01 | | 1 | 1 | RAM | I2C | + | MCP47FVB02 | | 2 | 1 | RAM | I2C | + | MCP47FVB04 | | 4 | 2 | RAM | I2C | + | MCP47FVB08 | 8-bit | 8 | 2 | RAM | I2C | + | MCP48FVB01 | | 1 | 1 | RAM | SPI | + | MCP48FVB02 | | 2 | 1 | RAM | SPI | + | MCP48FVB04 | | 4 | 2 | RAM | SPI | + | MCP48FVB08 | | 8 | 2 | RAM | SPI | + |------------|--------------|-------------|-------------|------------|------------| + | MCP47FVB11 | | 1 | 1 | RAM | I2C | + | MCP47FVB12 | | 2 | 1 | RAM | I2C | + | MCP47FVB14 | | 4 | 2 | RAM | I2C | + | MCP47FVB18 | 10-bit | 8 | 2 | RAM | I2C | + | MCP48FVB11 | | 1 | 1 | RAM | SPI | + | MCP48FVB12 | | 2 | 1 | RAM | SPI | + | MCP48FVB14 | | 4 | 2 | RAM | SPI | + | MCP48FVB18 | | 8 | 2 | RAM | SPI | + |------------|--------------|-------------|-------------|------------|------------| + | MCP47FVB21 | | 1 | 1 | RAM | I2C | + | MCP47FVB22 | | 2 | 1 | RAM | I2C | + | MCP47FVB24 | | 4 | 2 | RAM | I2C | + | MCP47FVB28 | 12-bit | 8 | 2 | RAM | I2C | + | MCP48FVB21 | | 1 | 1 | RAM | SPI | + | MCP48FVB22 | | 2 | 1 | RAM | SPI | + | MCP48FVB24 | | 4 | 2 | RAM | SPI | + | MCP48FVB28 | | 8 | 2 | RAM | SPI | + +------------+--------------+-------------+-------------+------------+------------+ properties: compatible: @@ -84,10 +118,37 @@ properties: - microchip,mcp47feb08 - microchip,mcp47feb18 - microchip,mcp47feb28 + - microchip,mcp48feb01 + - microchip,mcp48feb02 + - microchip,mcp48feb04 + - microchip,mcp48feb08 + - microchip,mcp48feb11 + - microchip,mcp48feb12 + - microchip,mcp48feb14 + - microchip,mcp48feb18 + - microchip,mcp48feb21 + - microchip,mcp48feb22 + - microchip,mcp48feb24 + - microchip,mcp48feb28 + - microchip,mcp48fvb01 + - microchip,mcp48fvb02 + - microchip,mcp48fvb04 + - microchip,mcp48fvb08 + - microchip,mcp48fvb11 + - microchip,mcp48fvb12 + - microchip,mcp48fvb14 + - microchip,mcp48fvb18 + - microchip,mcp48fvb21 + - microchip,mcp48fvb22 + - microchip,mcp48fvb24 + - microchip,mcp48fvb28 reg: maxItems: 1 + spi-max-frequency: + maximum: 10000000 + "#address-cells": const: 1 @@ -178,6 +239,21 @@ required: - vdd-supply allOf: + - if: + properties: + compatible: + contains: + pattern: "^microchip,mcp48f[ev]b[0-2][1248]$" + then: + $ref: /schemas/spi/spi-peripheral-props.yaml# + dependencies: + spi-cpha: [ spi-cpol ] + spi-cpol: [ spi-cpha ] + else: + properties: + spi-max-frequency: false + spi-cpha: false + spi-cpol: false - if: properties: compatible: @@ -189,6 +265,12 @@ allOf: - microchip,mcp47fvb01 - microchip,mcp47fvb11 - microchip,mcp47fvb21 + - microchip,mcp48feb01 + - microchip,mcp48feb11 + - microchip,mcp48feb21 + - microchip,mcp48fvb01 + - microchip,mcp48fvb11 + - microchip,mcp48fvb21 then: properties: lat1-gpios: false @@ -211,6 +293,12 @@ allOf: - microchip,mcp47fvb02 - microchip,mcp47fvb12 - microchip,mcp47fvb22 + - microchip,mcp48feb02 + - microchip,mcp48feb12 + - microchip,mcp48feb22 + - microchip,mcp48fvb02 + - microchip,mcp48fvb12 + - microchip,mcp48fvb22 then: properties: lat1-gpios: false @@ -233,6 +321,12 @@ allOf: - microchip,mcp47feb04 - microchip,mcp47feb14 - microchip,mcp47feb24 + - microchip,mcp48feb04 + - microchip,mcp48feb14 + - microchip,mcp48feb24 + - microchip,mcp48fvb04 + - microchip,mcp48fvb14 + - microchip,mcp48fvb24 then: patternProperties: "^channel@[0-3]$": @@ -251,6 +345,12 @@ allOf: - microchip,mcp47feb08 - microchip,mcp47feb18 - microchip,mcp47feb28 + - microchip,mcp48feb08 + - microchip,mcp48feb18 + - microchip,mcp48feb28 + - microchip,mcp48fvb08 + - microchip,mcp48fvb18 + - microchip,mcp48fvb28 then: patternProperties: "^channel@[0-7]$": @@ -272,7 +372,7 @@ allOf: properties: microchip,vref1-buffered: false -additionalProperties: false +unevaluatedProperties: false examples: - | @@ -299,4 +399,29 @@ examples: }; }; }; + - | + spi { + #address-cells = <1>; + #size-cells = <0>; + + dac@0 { + compatible = "microchip,mcp48feb02"; + reg = <0x0>; + vdd-supply = <&vdac_vdd>; + vref-supply = <&vref_reg>; + spi-max-frequency = <10000000>; + + #address-cells = <1>; + #size-cells = <0>; + channel@0 { + reg = <0>; + label = "Adjustable_voltage_ch0"; + }; + + channel@1 { + reg = <0x1>; + label = "Adjustable_voltage_ch1"; + }; + }; + }; ... -- 2.43.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v5 09/10] dt-bindings: iio: dac: add support for MCP48FEB02 SPI 2026-09-09 14:18 ` [PATCH v5 09/10] dt-bindings: iio: dac: add support for MCP48FEB02 SPI Ariana Lazar @ 2026-09-09 14:48 ` sashiko-bot 0 siblings, 0 replies; 27+ messages in thread From: sashiko-bot @ 2026-09-09 14:48 UTC (permalink / raw) To: Ariana Lazar; +Cc: conor+dt, devicetree, robh Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Missing definitions for `spi-cpha` and `spi-cpol` in the top-level `properties` block causes schema validation to reject valid device trees. -- commit a76968b0d8e0b0cd896879ce5f6827e2fd5d9f18 Author: Ariana Lazar <ariana.lazar@microchip.com> dt-bindings: iio: dac: add support for MCP48FEB02 SPI This commit adds support for the SPI MCP48FxBy1/2/4/8 part numbers, introduces the spi-max-frequency property, adds SPI-specific schema validation conditions, and provides a devicetree example for SPI usage. > diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml > index d131f136bd151..a86d094f0d4b5 100644 > --- a/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml > +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml [ ... ] > allOf: > + - if: > + properties: > + compatible: > + contains: > + pattern: "^microchip,mcp48f[ev]b[0-2][1248]$" > + then: > + $ref: /schemas/spi/spi-peripheral-props.yaml# > + dependencies: > + spi-cpha: [ spi-cpol ] > + spi-cpol: [ spi-cpha ] [Severity: Medium] Will device trees configuring the SPI clock phase and polarity for this device incorrectly fail dt-schema validation? This dependencies block within the allOf conditional intends to allow and enforce co-occurrence of the properties, but spi-cpha and spi-cpol are missing from the top-level properties block. > + else: > + properties: > + spi-max-frequency: false > + spi-cpha: false > + spi-cpol: false [ ... ] > @@ -272,7 +372,7 @@ allOf: > properties: > microchip,vref1-buffered: false > > -additionalProperties: false > +unevaluatedProperties: false [Severity: Medium] Because unevaluatedProperties is strictly false here, and spi-cpha and spi-cpol are omitted from the top-level properties block, won't any valid device tree using these properties trigger a validation error during dtbs_check? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260909-mcp47feb02_refactor-v5-0-8b67bcab93d1@microchip.com?part=9 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 10/10] iio: dac: add support for Microchip MCP48FEB02 2026-09-09 14:18 [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver Ariana Lazar ` (8 preceding siblings ...) 2026-09-09 14:18 ` [PATCH v5 09/10] dt-bindings: iio: dac: add support for MCP48FEB02 SPI Ariana Lazar @ 2026-09-09 14:18 ` Ariana Lazar 2026-09-09 14:51 ` sashiko-bot 2026-09-10 6:17 ` Andy Shevchenko 2026-09-10 6:18 ` [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver Andy Shevchenko 10 siblings, 2 replies; 27+ messages in thread From: Ariana Lazar @ 2026-09-09 14:18 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-kernel, linux-iio, devicetree, Ariana Lazar This is the IIO driver for Microchip MCP48FxBy1/2/4/8 series of buffered voltage output Digital-to-Analog Converters with nonvolatile or volatile memory and an SPI Interface. The families support up to 8 output channels. The devices can be 8-bit, 10-bit and 12-bit. Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> --- drivers/iio/dac/Kconfig | 23 +++++++ drivers/iio/dac/Makefile | 1 + drivers/iio/dac/mcp47feb02-spi.c | 145 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 169 insertions(+) diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig index cc10d43ffd1e89bbf945ef89ea5af0abe1a91084..d941f78f7110b185fee6f025be829bae1e4534da 100644 --- a/drivers/iio/dac/Kconfig +++ b/drivers/iio/dac/Kconfig @@ -575,6 +575,29 @@ config MCP47FEB02_I2C will be called mcp47feb02_i2c and you will also get mcp47feb02_core for the core module. +config MCP47FEB02_SPI + tristate "MCP48F(E/V)B01/02/04/08/11/12/14/18/21/22/24/28 SPI DAC driver" + depends on SPI + select REGMAP_SPI + select MCP47FEB02 + help + Say yes here if you want to build the driver for the Microchip: + - 8-bit DAC: + MCP48FEB01, MCP48FEB02, MCP48FEB04, MCP48FEB08, + MCP48FVB01, MCP48FVB02, MCP48FVB04, MCP48FVB08 + - 10-bit DAC: + MCP48FEB11, MCP48FEB12, MCP48FEB14, MCP48FEB18, + MCP48FVB11, MCP48FVB12, MCP48FVB14, MCP48FVB18 + - 12-bit DAC: + MCP48FEB21, MCP48FEB22, MCP48FEB24, MCP48FEB28, + MCP48FVB21, MCP48FVB22, MCP48FVB24, MCP48FVB28 + having 1 to 8 channels, 8/10/12-bit digital-to-analog converter + (DAC) with SPI interface. + + To compile this driver as a module, choose M here: the module + will be called mcp47feb02_spi and you will also get + mcp47feb02_core for the core module. + config MCP4821 tristate "MCP4801/02/11/12/21/22 DAC driver" depends on SPI diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile index 28c8646a987cff1ac4d693fe86898d66c901d5bd..9903198b934e8b66a4ea6562b662d8073ed9cc59 100644 --- a/drivers/iio/dac/Makefile +++ b/drivers/iio/dac/Makefile @@ -56,6 +56,7 @@ obj-$(CONFIG_MCP4725) += mcp4725.o obj-$(CONFIG_MCP4728) += mcp4728.o obj-$(CONFIG_MCP47FEB02) += mcp47feb02-core.o obj-$(CONFIG_MCP47FEB02_I2C) += mcp47feb02-i2c.o +obj-$(CONFIG_MCP47FEB02_SPI) += mcp47feb02-spi.o obj-$(CONFIG_MCP4821) += mcp4821.o obj-$(CONFIG_MCP4922) += mcp4922.o obj-$(CONFIG_STM32_DAC_CORE) += stm32-dac-core.o diff --git a/drivers/iio/dac/mcp47feb02-spi.c b/drivers/iio/dac/mcp47feb02-spi.c new file mode 100644 index 0000000000000000000000000000000000000000..0c68d7c318cb2269a9d713d3c6ec0854a576c19e --- /dev/null +++ b/drivers/iio/dac/mcp47feb02-spi.c @@ -0,0 +1,145 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * IIO driver for MCP48FEB02 Multi-Channel DAC with SPI interface + * + * Copyright (C) 2025-2026 Microchip Technology Inc. and its subsidiaries + * + * Author: Ariana Lazar <ariana.lazar@microchip.com> + * + * Datasheet links for devices with SPI interface: + * [MCP48FEBxx] https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005429B.pdf + * [MCP48FVBxx] https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/20005466A.pdf + * [MCP48FxBx4/8] https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/MCP48FXBX4-8-Family-Data-Sheet-DS20006362A.pdf + */ +#include <linux/dev_printk.h> +#include <linux/err.h> +#include <linux/module.h> +#include <linux/pm.h> +#include <linux/regmap.h> +#include <linux/spi/spi.h> +#include <linux/types.h> + +#include "mcp47feb02.h" + +/* Parts with EEPROM memory */ +MCP47FEB02_CHIP_INFO(mcp48feb01, 1, 8, false, true); +MCP47FEB02_CHIP_INFO(mcp48feb02, 2, 8, false, true); +MCP47FEB02_CHIP_INFO(mcp48feb04, 4, 8, true, true); +MCP47FEB02_CHIP_INFO(mcp48feb08, 8, 8, true, true); +MCP47FEB02_CHIP_INFO(mcp48feb11, 1, 10, false, true); +MCP47FEB02_CHIP_INFO(mcp48feb12, 2, 10, false, true); +MCP47FEB02_CHIP_INFO(mcp48feb14, 4, 10, true, true); +MCP47FEB02_CHIP_INFO(mcp48feb18, 8, 10, true, true); +MCP47FEB02_CHIP_INFO(mcp48feb21, 1, 12, false, true); +MCP47FEB02_CHIP_INFO(mcp48feb22, 2, 12, false, true); +MCP47FEB02_CHIP_INFO(mcp48feb24, 4, 12, true, true); +MCP47FEB02_CHIP_INFO(mcp48feb28, 8, 12, true, true); + +/* Parts without EEPROM memory */ +MCP47FEB02_CHIP_INFO(mcp48fvb01, 1, 8, false, false); +MCP47FEB02_CHIP_INFO(mcp48fvb02, 2, 8, false, false); +MCP47FEB02_CHIP_INFO(mcp48fvb04, 4, 8, true, false); +MCP47FEB02_CHIP_INFO(mcp48fvb08, 8, 8, true, false); +MCP47FEB02_CHIP_INFO(mcp48fvb11, 1, 10, false, false); +MCP47FEB02_CHIP_INFO(mcp48fvb12, 2, 10, false, false); +MCP47FEB02_CHIP_INFO(mcp48fvb14, 4, 10, true, false); +MCP47FEB02_CHIP_INFO(mcp48fvb18, 8, 10, true, false); +MCP47FEB02_CHIP_INFO(mcp48fvb21, 1, 12, false, false); +MCP47FEB02_CHIP_INFO(mcp48fvb22, 2, 12, false, false); +MCP47FEB02_CHIP_INFO(mcp48fvb24, 4, 12, true, false); +MCP47FEB02_CHIP_INFO(mcp48fvb28, 8, 12, true, false); + +static int mcp47feb02_spi_probe(struct spi_device *spi) +{ + const struct mcp47feb02_features *chip_features; + struct device *dev = &spi->dev; + struct regmap *regmap; + + chip_features = spi_get_device_match_data(spi); + if (!chip_features) + return dev_err_probe(dev, -ENODEV, "No SPI device found\n"); + + if (chip_features->have_eeprom) + regmap = devm_regmap_init_spi(spi, &mcp47feb02_regmap_config); + else + regmap = devm_regmap_init_spi(spi, &mcp47fvb02_regmap_config); + + if (IS_ERR(regmap)) + return dev_err_probe(dev, PTR_ERR(regmap), "Error initializing SPI regmap\n"); + + return mcp47feb02_common_probe(chip_features, regmap); +} + +static const struct spi_device_id mcp47feb02_spi_id[] = { + { .name = "mcp48feb01", .driver_data = (kernel_ulong_t)&mcp48feb01_chip_features }, + { .name = "mcp48feb02", .driver_data = (kernel_ulong_t)&mcp48feb02_chip_features }, + { .name = "mcp48feb04", .driver_data = (kernel_ulong_t)&mcp48feb04_chip_features }, + { .name = "mcp48feb08", .driver_data = (kernel_ulong_t)&mcp48feb08_chip_features }, + { .name = "mcp48feb11", .driver_data = (kernel_ulong_t)&mcp48feb11_chip_features }, + { .name = "mcp48feb12", .driver_data = (kernel_ulong_t)&mcp48feb12_chip_features }, + { .name = "mcp48feb14", .driver_data = (kernel_ulong_t)&mcp48feb14_chip_features }, + { .name = "mcp48feb18", .driver_data = (kernel_ulong_t)&mcp48feb18_chip_features }, + { .name = "mcp48feb21", .driver_data = (kernel_ulong_t)&mcp48feb21_chip_features }, + { .name = "mcp48feb22", .driver_data = (kernel_ulong_t)&mcp48feb22_chip_features }, + { .name = "mcp48feb24", .driver_data = (kernel_ulong_t)&mcp48feb24_chip_features }, + { .name = "mcp48feb28", .driver_data = (kernel_ulong_t)&mcp48feb28_chip_features }, + { .name = "mcp48fvb01", .driver_data = (kernel_ulong_t)&mcp48fvb01_chip_features }, + { .name = "mcp48fvb02", .driver_data = (kernel_ulong_t)&mcp48fvb02_chip_features }, + { .name = "mcp48fvb04", .driver_data = (kernel_ulong_t)&mcp48fvb04_chip_features }, + { .name = "mcp48fvb08", .driver_data = (kernel_ulong_t)&mcp48fvb08_chip_features }, + { .name = "mcp48fvb11", .driver_data = (kernel_ulong_t)&mcp48fvb11_chip_features }, + { .name = "mcp48fvb12", .driver_data = (kernel_ulong_t)&mcp48fvb12_chip_features }, + { .name = "mcp48fvb14", .driver_data = (kernel_ulong_t)&mcp48fvb14_chip_features }, + { .name = "mcp48fvb18", .driver_data = (kernel_ulong_t)&mcp48fvb18_chip_features }, + { .name = "mcp48fvb21", .driver_data = (kernel_ulong_t)&mcp48fvb21_chip_features }, + { .name = "mcp48fvb22", .driver_data = (kernel_ulong_t)&mcp48fvb22_chip_features }, + { .name = "mcp48fvb24", .driver_data = (kernel_ulong_t)&mcp48fvb24_chip_features }, + { .name = "mcp48fvb28", .driver_data = (kernel_ulong_t)&mcp48fvb28_chip_features }, + { } +}; +MODULE_DEVICE_TABLE(spi, mcp47feb02_spi_id); + +static const struct of_device_id mcp47feb02_of_spi_match[] = { + { .compatible = "microchip,mcp48feb01", .data = &mcp48feb01_chip_features }, + { .compatible = "microchip,mcp48feb02", .data = &mcp48feb02_chip_features }, + { .compatible = "microchip,mcp48feb04", .data = &mcp48feb04_chip_features }, + { .compatible = "microchip,mcp48feb08", .data = &mcp48feb08_chip_features }, + { .compatible = "microchip,mcp48feb11", .data = &mcp48feb11_chip_features }, + { .compatible = "microchip,mcp48feb12", .data = &mcp48feb12_chip_features }, + { .compatible = "microchip,mcp48feb14", .data = &mcp48feb14_chip_features }, + { .compatible = "microchip,mcp48feb18", .data = &mcp48feb18_chip_features }, + { .compatible = "microchip,mcp48feb21", .data = &mcp48feb21_chip_features }, + { .compatible = "microchip,mcp48feb22", .data = &mcp48feb22_chip_features }, + { .compatible = "microchip,mcp48feb24", .data = &mcp48feb24_chip_features }, + { .compatible = "microchip,mcp48feb28", .data = &mcp48feb28_chip_features }, + { .compatible = "microchip,mcp48fvb01", .data = &mcp48fvb01_chip_features }, + { .compatible = "microchip,mcp48fvb02", .data = &mcp48fvb02_chip_features }, + { .compatible = "microchip,mcp48fvb04", .data = &mcp48fvb04_chip_features }, + { .compatible = "microchip,mcp48fvb08", .data = &mcp48fvb08_chip_features }, + { .compatible = "microchip,mcp48fvb11", .data = &mcp48fvb11_chip_features }, + { .compatible = "microchip,mcp48fvb12", .data = &mcp48fvb12_chip_features }, + { .compatible = "microchip,mcp48fvb14", .data = &mcp48fvb14_chip_features }, + { .compatible = "microchip,mcp48fvb18", .data = &mcp48fvb18_chip_features }, + { .compatible = "microchip,mcp48fvb21", .data = &mcp48fvb21_chip_features }, + { .compatible = "microchip,mcp48fvb22", .data = &mcp48fvb22_chip_features }, + { .compatible = "microchip,mcp48fvb24", .data = &mcp48fvb24_chip_features }, + { .compatible = "microchip,mcp48fvb28", .data = &mcp48fvb28_chip_features }, + { } +}; +MODULE_DEVICE_TABLE(of, mcp47feb02_of_spi_match); + +static struct spi_driver mcp47feb02_spi_driver = { + .driver = { + .name = "mcp47feb02", + .of_match_table = mcp47feb02_of_spi_match, + .pm = pm_sleep_ptr(&mcp47feb02_pm_ops), + }, + .probe = mcp47feb02_spi_probe, + .id_table = mcp47feb02_spi_id, +}; +module_spi_driver(mcp47feb02_spi_driver); + +MODULE_AUTHOR("Ariana Lazar <ariana.lazar@microchip.com>"); +MODULE_DESCRIPTION("IIO driver for MCP48FEB02 Multi-Channel DAC with SPI interface"); +MODULE_LICENSE("GPL"); +MODULE_IMPORT_NS("IIO_MCP47FEB02"); -- 2.43.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v5 10/10] iio: dac: add support for Microchip MCP48FEB02 2026-09-09 14:18 ` [PATCH v5 10/10] iio: dac: add support for Microchip MCP48FEB02 Ariana Lazar @ 2026-09-09 14:51 ` sashiko-bot 2026-09-10 6:17 ` Andy Shevchenko 1 sibling, 0 replies; 27+ messages in thread From: sashiko-bot @ 2026-09-09 14:51 UTC (permalink / raw) To: Ariana Lazar; +Cc: conor+dt, devicetree, robh Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] Missing POR (Power-On Reset) delay after enabling the VDD regulator in probe. -- commit 3cf6710da958614daaff4a4a7e99d396c3398a95 Author: Ariana Lazar <ariana.lazar@microchip.com> iio: dac: add support for Microchip MCP48FEB02 This commit introduces SPI support for the Microchip MCP48FEB02 series DACs. It adds a new SPI driver file and the corresponding Kconfig and Makefile changes. > diff --git a/drivers/iio/dac/mcp47feb02-spi.c b/drivers/iio/dac/mcp47feb02-spi.c > new file mode 100644 > index 0000000000000..0c68d7c318cb2 > --- /dev/null > +++ b/drivers/iio/dac/mcp47feb02-spi.c [ ... ] > +static int mcp47feb02_spi_probe(struct spi_device *spi) > +{ > + const struct mcp47feb02_features *chip_features; > + struct device *dev = &spi->dev; > + struct regmap *regmap; [ ... ] > + if (IS_ERR(regmap)) > + return dev_err_probe(dev, PTR_ERR(regmap), "Error initializing SPI regmap\n"); > + > + return mcp47feb02_common_probe(chip_features, regmap); > +} [Severity: High] This is a pre-existing issue, but does mcp47feb02_common_probe() miss a Power-On Reset (POR) delay after enabling the VDD regulator? Looking at the shared probe function in drivers/iio/dac/mcp47feb02-core.c: ret = devm_regulator_get_enable_read_voltage(dev, "vdd"); if (ret < 0) return ret; /* ... vref initialization ... */ ret = mcp47feb02_init_ctrl_regs(data); When devm_regulator_get_enable_read_voltage() cold-starts the VDD supply for the first time, the device requires a typical 2ms (max 4ms) POR delay before its internal state machine is ready to accept commands. Since the driver immediately proceeds to mcp47feb02_init_ctrl_regs() and performs a regmap operation without any wait (e.g., usleep_range()), could this cause the device to fail to probe or read garbage data on setups where the driver is responsible for turning on the VDD regulator? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260909-mcp47feb02_refactor-v5-0-8b67bcab93d1@microchip.com?part=10 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5 10/10] iio: dac: add support for Microchip MCP48FEB02 2026-09-09 14:18 ` [PATCH v5 10/10] iio: dac: add support for Microchip MCP48FEB02 Ariana Lazar 2026-09-09 14:51 ` sashiko-bot @ 2026-09-10 6:17 ` Andy Shevchenko 1 sibling, 0 replies; 27+ messages in thread From: Andy Shevchenko @ 2026-09-10 6:17 UTC (permalink / raw) To: Ariana Lazar Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-iio, devicetree On Wed, Sep 09, 2026 at 05:18:53PM +0300, Ariana Lazar wrote: > This is the IIO driver for Microchip MCP48FxBy1/2/4/8 series of buffered > voltage output Digital-to-Analog Converters with nonvolatile or volatile > memory and an SPI Interface. > > The families support up to 8 output channels. > > The devices can be 8-bit, 10-bit and 12-bit. ... > +static int mcp47feb02_spi_probe(struct spi_device *spi) > +{ > + const struct mcp47feb02_features *chip_features; > + struct device *dev = &spi->dev; > + struct regmap *regmap; > + > + chip_features = spi_get_device_match_data(spi); > + if (!chip_features) > + return dev_err_probe(dev, -ENODEV, "No SPI device found\n"); -ENODATA and fix the message accordingly. > + if (chip_features->have_eeprom) > + regmap = devm_regmap_init_spi(spi, &mcp47feb02_regmap_config); > + else > + regmap = devm_regmap_init_spi(spi, &mcp47fvb02_regmap_config); > + > + if (IS_ERR(regmap)) > + return dev_err_probe(dev, PTR_ERR(regmap), "Error initializing SPI regmap\n"); > + > + return mcp47feb02_common_probe(chip_features, regmap); > +} -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver 2026-09-09 14:18 [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver Ariana Lazar ` (9 preceding siblings ...) 2026-09-09 14:18 ` [PATCH v5 10/10] iio: dac: add support for Microchip MCP48FEB02 Ariana Lazar @ 2026-09-10 6:18 ` Andy Shevchenko 2026-09-13 21:55 ` Jonathan Cameron 10 siblings, 1 reply; 27+ messages in thread From: Andy Shevchenko @ 2026-09-10 6:18 UTC (permalink / raw) To: Ariana Lazar Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-iio, devicetree On Wed, Sep 09, 2026 at 05:18:43PM +0300, Ariana Lazar wrote: > Refactor I2C driver implementation into separate modules in order to add > support for SPI MCP48FxBy1/2/4/8 DAC family on top of the I2C implementation. > The I2C and SPI devices have the same memory map and supported > functionalities. > --- > Changes in v5: > - improve commit messages by justifying the changes made > - increase polling time for EEPROM writes > - delete unnecessary comments and extra blank line from header file > - Link to v4: https://lore.kernel.org/r/20260819-mcp47feb02_refactor-v4-0-beb2aa3bfd09@microchip.com I have a felling that the first few patches are actual fixes and they lack of Fixes tag (and possible Cc to stable@). -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver 2026-09-10 6:18 ` [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver Andy Shevchenko @ 2026-09-13 21:55 ` Jonathan Cameron 0 siblings, 0 replies; 27+ messages in thread From: Jonathan Cameron @ 2026-09-13 21:55 UTC (permalink / raw) To: Andy Shevchenko Cc: Ariana Lazar, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-iio, devicetree On Thu, 10 Sep 2026 09:18:37 +0300 Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > On Wed, Sep 09, 2026 at 05:18:43PM +0300, Ariana Lazar wrote: > > Refactor I2C driver implementation into separate modules in order to add > > support for SPI MCP48FxBy1/2/4/8 DAC family on top of the I2C implementation. > > The I2C and SPI devices have the same memory map and supported > > functionalities. > > > --- > > Changes in v5: > > - improve commit messages by justifying the changes made > > - increase polling time for EEPROM writes > > - delete unnecessary comments and extra blank line from header file > > - Link to v4: https://lore.kernel.org/r/20260819-mcp47feb02_refactor-v4-0-beb2aa3bfd09@microchip.com > > I have a felling that the first few patches are actual fixes and they lack of > Fixes tag (and possible Cc to stable@). > Excellent point. I nearly picked up the series (tweaking for stuff Andy raised) but I would like those Fixes tags so Ariana, please send a v6 with them an everything tidied up. Rebase on 7.3-rc1 as well please. The based-on tag in here is something I don't even have. Thanks, Jonathan ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2026-09-13 21:55 UTC | newest] Thread overview: 27+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-09 14:18 [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver Ariana Lazar 2026-09-09 14:18 ` [PATCH v5 01/10] iio: dac: mcp47feb02: initialize dac_data field in channel data struct at probe Ariana Lazar 2026-09-09 14:32 ` sashiko-bot 2026-09-09 14:18 ` [PATCH v5 02/10] iio: dac: mcp47feb02: Fix gain field initialization for active channels Ariana Lazar 2026-09-13 21:43 ` Jonathan Cameron 2026-09-09 14:18 ` [PATCH v5 03/10] iio: dac: mcp47feb02: Return len when disabling EEPROM store Ariana Lazar 2026-09-09 14:33 ` sashiko-bot 2026-09-09 14:18 ` [PATCH v5 04/10] iio: dac: mcp47feb02: Increase EEPROM Programming Write Cycle Time Ariana Lazar 2026-09-09 14:31 ` sashiko-bot 2026-09-10 6:11 ` Andy Shevchenko 2026-09-09 14:18 ` [PATCH v5 05/10] iio: dac: mcp47feb02: Avoid unjustified probe failure on missing label Ariana Lazar 2026-09-09 14:40 ` sashiko-bot 2026-09-10 6:12 ` Andy Shevchenko 2026-09-09 14:18 ` [PATCH v5 06/10] iio: dac: mcp47feb02: correct typo from a comment Ariana Lazar 2026-09-09 14:18 ` [PATCH v5 07/10] iio: dac: mcp47feb02: rename command mask define Ariana Lazar 2026-09-09 14:39 ` sashiko-bot 2026-09-09 14:18 ` [PATCH v5 08/10] iio: dac: mcp47feb02: refactor MCP47FEB02 I2C driver into two modules Ariana Lazar 2026-09-09 14:47 ` sashiko-bot 2026-09-10 6:16 ` Andy Shevchenko 2026-09-13 21:51 ` Jonathan Cameron 2026-09-09 14:18 ` [PATCH v5 09/10] dt-bindings: iio: dac: add support for MCP48FEB02 SPI Ariana Lazar 2026-09-09 14:48 ` sashiko-bot 2026-09-09 14:18 ` [PATCH v5 10/10] iio: dac: add support for Microchip MCP48FEB02 Ariana Lazar 2026-09-09 14:51 ` sashiko-bot 2026-09-10 6:17 ` Andy Shevchenko 2026-09-10 6:18 ` [PATCH v5 00/10] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver Andy Shevchenko 2026-09-13 21:55 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox