linux-kernel - Re: [PATCH v3 3/3] iio: dac: ad3530r: Add driver for AD3530R and AD3531R

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: <a66220e9-f690-4f95-9a14-d2b663d63490@baylibre.com>
Date: Tue, 8 Apr 2025 09:06:14 -0500
From: David Lechner <dlechner@...libre.com>
To: "Paller, Kim Seer" <KimSeer.Paller@...log.com>,
 Jonathan Cameron <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
 "Hennerich, Michael" <Michael.Hennerich@...log.com>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>
Cc: "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH v3 3/3] iio: dac: ad3530r: Add driver for AD3530R and
 AD3531R

On 4/7/25 3:01 AM, Paller, Kim Seer wrote:
>>> +	for (i = 0; i < chip_info->num_channels; i++)
>>> +		st->chan[i].powerdown_mode =
>> AD3530R_32KOHM_POWERDOWN_MODE;
>>> +
>>> +	st->ldac_gpio = devm_gpiod_get_optional(dev, "ldac",
>> GPIOD_OUT_HIGH);
>>> +	if (IS_ERR(st->ldac_gpio))
>>> +		return dev_err_probe(dev, PTR_ERR(st->ldac_gpio),
>>> +				     "Failed to get ldac GPIO\n");
>>> +
>>> +	if (device_property_present(dev, "adi,double-output-range")) {
>>
>> This is not documented in the DT bindings.
>>
>> This could instead be implemented by making the out_voltage_scale attribute
>> writeable.
>>
>> If we really do need it in the devicetree because we want to protect against
>> someone accidentally setting it too high, then the property should be the actual
>> multipler value with an enum specifier of 1, 2 and a default of 1 rather than a
>> flag (e.g. adi,output-multipler).
> 
> Thank you for the feedback. This should be adi,range-double, which is already
> documented in the DT bindings and is also used as a boolean type in other drivers.
> I just forgot to update it here. This is a one-bit configuration that doubles the
> output range (multiplier of 2).  Should I proceed with the suggested approach?
> 

I see adi,range-double in
Documentation/devicetree/bindings/iio/adc/adi,ad7923.yaml

We would need to add the same in the new .yaml file added in this
series along with a justification in the commit message of why this
needs to be set in the devicetree rather than at runtime.

Ideally, we would ask the applications engineer for this chip to
find out how real users would want to use this feature to make sure
setting it in the devicetree aligns with that and is not too
restrictive.