On 1/27/25 4:57 AM, Antoniu Miclaus wrote:
> Add support for the AD485X a fully buffered, 8-channel simultaneous
> sampling, 16/20-bit, 1 MSPS data acquisition system (DAS) with
> differential, wide common-mode range inputs.
>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx>
> ---
I think we have the important bits sorted now (i.e. userspace-facing stuff).
Just noticed a few minor things in the latest revision.
> +static int ad4851_setup(struct ad4851_state *st)
> +{
> + unsigned int product_id;
> + int ret;
> +
> + if (st->pd_gpio) {
> + /* To initiate a global reset, bring the PD pin high twice */
> + gpiod_set_value(st->pd_gpio, 1);
> + fsleep(1);
> + gpiod_set_value(st->pd_gpio, 0);
> + fsleep(1);
> + gpiod_set_value(st->pd_gpio, 1);
> + fsleep(1);
> + gpiod_set_value(st->pd_gpio, 0);
> + fsleep(1000);
> + } else {
> + ret = regmap_set_bits(st->regmap, AD4851_REG_INTERFACE_CONFIG_A,
> + AD4851_SW_RESET);
> + if (ret)
> + return ret;
Do we also need fsleep() after software reset?
> + }
> +
...
> +static int ad4851_parse_channels(struct iio_dev *indio_dev,
> + const struct iio_chan_spec ad4851_chan)
> +{
> + struct ad4851_state *st = iio_priv(indio_dev);
> + struct device *dev = &st->spi->dev;
> + struct iio_chan_spec *channels;
> + unsigned int num_channels, reg;
> + unsigned int index = 0;
> + int ret;
> +
> + num_channels = device_get_child_node_count(dev);
> + if (num_channels > AD4851_MAX_CH_NR)
> + return dev_err_probe(dev, -EINVAL, "Too many channels: %u\n",
> + num_channels);
> +
> + channels = devm_kcalloc(dev, num_channels, sizeof(*channels), GFP_KERNEL);
> + if (!channels)
> + return -ENOMEM;
> +
> + indio_dev->channels = channels;
> + indio_dev->num_channels = num_channels;
> +
> + device_for_each_child_node_scoped(dev, child) {
> + ret = fwnode_property_read_u32(child, "reg", ®);
> + if (reg >= AD4851_MAX_CH_NR)
> + return dev_err_probe(dev, ret,
> + "Invalid channel number\n");
Need to check ret first, otherwise reg may be unintialized.
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Missing channel number\n");
> + *channels = ad4851_chan;
> + channels->scan_index = index++;
> + channels->channel = reg;
> +
> + if (fwnode_property_present(child, "diff-channels")) {
> + channels->channel2 = reg + st->info->max_channels;
> + channels->differential = 1;
> + }
> +
> + channels++;
> +
> + st->bipolar_ch[reg] = fwnode_property_read_bool(child, "bipolar");
> +
> + if (st->bipolar_ch[reg]) {
> + channels->scan_type.sign = 's';
> + } else {
> + ret = regmap_write(st->regmap, AD4851_REG_CHX_SOFTSPAN(reg),
> + AD4851_SOFTSPAN_0V_40V);
> + if (ret)
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int ad4857_parse_channels(struct iio_dev *indio_dev)
> +{
> + const struct iio_chan_spec ad4851_chan = AD4857_IIO_CHANNEL;
> +
> + return ad4851_parse_channels(indio_dev, ad4851_chan);
> +}
> +
> +static int ad4858_parse_channels(struct iio_dev *indio_dev)
> +{
> + struct ad4851_state *st = iio_priv(indio_dev);
> + struct device *dev = &st->spi->dev;
> + struct iio_chan_spec *ad4851_channels;
> + const struct iio_chan_spec ad4851_chan = AD4858_IIO_CHANNEL;
> + int ret;
> +
> + ad4851_channels = (struct iio_chan_spec *)indio_dev->channels;
> +
> + ret = ad4851_parse_channels(indio_dev, ad4851_chan);
> + if (ret)
> + return ret;
> +
> + device_for_each_child_node_scoped(dev, child) {
> + ad4851_channels->has_ext_scan_type = 1;
> + if (fwnode_property_present(child, "bipolar")) {
fwnode_property_read_bool()
(to be consistent with same check in ad4851_parse_channels())
> + ad4851_channels->ext_scan_type = ad4851_scan_type_20_b;
> + ad4851_channels->num_ext_scan_type = ARRAY_SIZE(ad4851_scan_type_20_b);
> +
> + } else {
> + ad4851_channels->ext_scan_type = ad4851_scan_type_20_u;
> + ad4851_channels->num_ext_scan_type = ARRAY_SIZE(ad4851_scan_type_20_u);
> + }
> + ad4851_channels++;
> + }
> +
> + return 0;
> +}
> +
