On Sun, May 28, 2023 at 12:48:54AM +0300, George Stark wrote:
> Patch adds two sysfs nodes: chan7_mux to set mux state
> and chan7_mux_available to show available mux states.
> Mux can be used to debug and calibrate adc by
> switching and measuring well-known inputs like GND, Vdd etc.
Thank you for an update, my comments below.
...
> ---
Missing changelog, what has been done in v2, how it's different to v1.
> drivers/iio/adc/meson_saradc.c | 65 ++++++++++++++++++++++++++++++++++
> 1 file changed, 65 insertions(+)
...
> +static const char * const chan7_vol[] = {
> + "gnd",
> + "vdd/4",
> + "vdd/2",
> + "vdd*3/4",
> + "vdd",
> + "ch7_input",
> +};
> +
> +static ssize_t chan7_mux_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
> + unsigned int index = priv->chan7_mux_sel;
> +
> + if (index >= ARRAY_SIZE(chan7_vol))
> + index = ARRAY_SIZE(chan7_vol) - 1;
I think this is incorrect and prone to error in the future in case this array
will be extended. What I would expect is to return something like "unknown".
> + return sysfs_emit(buf, "%s\n", chan7_vol[index]);
> +}
> +
> +static ssize_t chan7_mux_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + int i;
> +
> + i = sysfs_match_string(chan7_vol, buf);
> + if (i < 0)
> + return -EINVAL;
Do not shadow the error code if it's not justified.
return i;
> + meson_sar_adc_set_chan7_mux(indio_dev, i);
> + return count;
> +}
> +
Redundant blank line.
> +static IIO_DEVICE_ATTR_RW(chan7_mux, -1);
> +
> +static ssize_t chan7_mux_available_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + int i, len = 0;
> +
> + for (i = 0; i < ARRAY_SIZE(chan7_vol); i++)
> + len += sysfs_emit_at(buf, len, "%s ", chan7_vol[i]);
> +
> + return len;
> +}
> +
Ditto.
> +static IIO_DEVICE_ATTR_RO(chan7_mux_available, -1);
--
With Best Regards,
Andy Shevchenko
