Re: [PATCH v3 4/4] iio: dac: ad3530r: add support for AD5710R/AD5711R

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Read-only `powerdown_mode` enum is exposed as writable in sysfs, returning -EINVAL on write.
--

--- Patch [4]: [PATCH v3 4/4] iio: dac: ad3530r: add support for AD5710R/AD5711R ---
commit 4d59cde2891949f47bb58dbf8c77908db02a7285
Author: Kim Seer Paller <[email protected]>

iio: dac: ad3530r: add support for AD5710R/AD5711R

Adds support for the AD5710R/AD5711R 8-channel configurable IDAC/VDAC parts.
Each channel is configured as voltage or current output from its DT node,
with the powerdown mode derived from the channel's configured type.

> diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
> --- a/drivers/iio/dac/ad3530r.c
> +++ b/drivers/iio/dac/ad3530r.c
[ ... ]
> @@ -419,6 +511,17 @@ static const struct iio_chan_spec_ext_info ad3532r_ext_info[] = {
>  	{ }
>  };
>  
> +static const struct iio_chan_spec_ext_info ad5710r_ext_info[] = {
> +	{
> +		.name = "powerdown",
> +		.shared = IIO_SEPARATE,
> +		.read = ad5710r_get_dac_powerdown,
> +		.write = ad5710r_set_dac_powerdown,
> +	},
> +	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &ad5710r_powerdown_mode_enum),

[Severity: Low]
The commit message mentions that the powerdown mode is read-only. Does using
the IIO_ENUM macro here inadvertently make the sysfs attribute writable?

The IIO_ENUM macro automatically populates both the .read and .write
callbacks (iio_enum_read and iio_enum_write). Since
ad5710r_powerdown_mode_enum lacks a .set callback, any writes from userspace
will safely fail and return -EINVAL.

Should this be manually defined as an iio_chan_spec_ext_info struct without
a .write callback to ensure the sysfs attribute is correctly exposed as
read-only?

> +	{ }
> +};
> +
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4