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

[David Lechner]

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.