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 for Android: free password hash cracker in your pocket

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

Message-ID: <420dbd79-c981-4278-b888-5dd9f5baa4c6@baylibre.com>
Date: Thu, 10 Apr 2025 09:11:46 -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/10/25 3:39 AM, Paller, Kim Seer wrote:
> 
> 
>> -----Original Message-----
>> From: David Lechner <dlechner@...libre.com>
>> Sent: Tuesday, April 8, 2025 10:06 PM
>> 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-kernel@...r.kernel.org;
>> devicetree@...r.kernel.org
>> Subject: Re: [PATCH v3 3/3] iio: dac: ad3530r: Add driver for AD3530R and
>> AD3531R
>>
>> [External]
>>
>> 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.
> 
> I can confirm that the adi,range-double property is already present in the adi,ad3530r.yaml
> file as a boolean type, and consistent with its usage in adi,ad7923.yaml.

Sorry, I don't know why I didn't see it the first 2 times I looked. :-(
It is there.

> 
>>
>> 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.
> 
> The apps engineer for this chip indicated that the boolean type format aligns better with
> how users are expected to configure this feature, especially for users who may not be
> familiar with specific range values or enum specifiers.
> 

Very good.