Re: [PATCH 2/2] iio: dac: mcp47feb02: add MCP48FEB02 SPI driver to MCP47FEB02 I2C driver

[David Lechner]

On 4/14/26 10:28 AM, Ariana.Lazar@xxxxxxxxxxxxx wrote:
> Hi David,
> 
>>> --- a/drivers/iio/dac/Makefile
>>> +++ b/drivers/iio/dac/Makefile
>>> @@ -54,6 +54,9 @@ obj-$(CONFIG_MAX5821) += max5821.o
>>>  obj-$(CONFIG_MCP4725) += mcp4725.o
>>>  obj-$(CONFIG_MCP4728) += mcp4728.o
>>>  obj-$(CONFIG_MCP47FEB02) += mcp47feb02.o
>>
>> Shouldn't we be removing this old file?
>>
>> The patch series would be eaiser to understand if it was split into
>> one commit to split the existing driver into two files and then
>> another commit to add support for the new parts.
>>
>>
>>> +mcp47feb02-objs := mcp47feb02-core.o

Ah, I missed this line.

>>> +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
>>
> 
> Thank you for the review.
> 
> I kept that line so the core module would compile as 'mcp47feb02.ko'.
> If you prefer, I can rename the core file to mcp47feb02.c and add just
> the lines for the SPI and I2C modules in the Makefile.

It looks like there is only a few other driver that implement
it the way you have. And only a few others that don't have "core"
in the C file name.

> 
> Another option would be to keep the core module named mcp47feb02-core.c
> and compile it as 'mcp47feb02-core.ko'.

It looks like this is the option most other drivers go with. So unless
there is a good reason to do otherwise, I would tend to go with the
most common pattern.

> 
> Best regards,
> Ariana