Re: [PATCH v3 2/2] iio: add MCP4728 I2C DAC driver

[Andrea Collamati]

Hi Jonathan,

On 7/20/23 21:13, Jonathan Cameron wrote:
>> +static const char *const mcp4728_vref_modes[] = {
>> +	"vdd_ext",
>> +	"internal",
>> +};
>> +
>> +static int mcp4728_get_vref_mode(struct iio_dev *indio_dev,
>> +				 const struct iio_chan_spec *chan)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +
>> +	return data->channel_data[chan->channel].ref_mode;
>> +}
>> +
>> +static int mcp4728_set_vref_mode(struct iio_dev *indio_dev,
>> +				 const struct iio_chan_spec *chan,
>> +				 unsigned int mode)
>> +{
>> +	struct mcp4728_data *data = iio_priv(indio_dev);
>> +	int ret;
>> +
>> +	data->channel_data[chan->channel].ref_mode = mode;
>> +
>> +	if (mode == MCP4728_VREF_EXTERNAL_VDD &&
>> +	    data->channel_data[chan->channel].g_mode == MCP4728_GAIN_X2) {
>> +		dev_warn(&data->client->dev,
>> +			 "CH%d: Gain x2 not effective when vref is vdd, force to x1",
>> +			 chan->channel);
> Even better if you don't present the option at all and wrap it up in the
> standard ABI of _scale
>
I think that the solution could be:

- Removing custom ABI (vref/gain)

- Initialize them at device tree level using two 4-elements arrays.

- Finally using the same approach of https://github.com/torvalds/linux/blob/c2782531397f5cb19ca3f8f9c17727f1cdf5bee8/drivers/iio/dac/mcp4725.c#L462 where after having  synced current parameters stored in EEPROM they are updated with the ones specified in dts.


Best regards

                       Andrea