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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <739ea3c7-7181-183c-799d-a3f9403c7f66@gmail.com>
Date:   Sun, 23 Jul 2023 07:09:11 +0200
From:   Andrea Collamati <andrea.collamati@...il.com>
To:     Jonathan Cameron <jic23@...nel.org>
Cc:     Lars-Peter Clausen <lars@...afoo.de>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>, linux-iio@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/2] iio: add MCP4728 I2C DAC driver

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