Messages in this thread | | | From | Andy Shevchenko <> | Date | Tue, 21 Jul 2020 22:28:49 +0300 | Subject | Re: [RFC PATCH] one-bit-adc-dac: Add initial version of one bit ADC, DAC |
| |
On Thu, Jul 16, 2020 at 10:27 AM Cristian Pop <cristian.pop@analog.com> wrote: > > Implementation for 1-bit ADC (comparator) and a 1-bit DAC (switch)
...
> +struct one_bit_adc_dac_state {
> + struct platform_device *pdev;
Wouldn't 'struct device *dev;' be enough?
> + struct gpio_descs *in_gpio_descs; > + struct gpio_descs *out_gpio_descs; > +};
...
> +{ > + struct one_bit_adc_dac_state *st = iio_priv(indio_dev); > + int in_num_ch = 0, out_num_ch = 0; > + int channel = chan->channel;
> + if (st->in_gpio_descs) > + in_num_ch = st->in_gpio_descs->ndescs; > + > + if (st->out_gpio_descs) > + out_num_ch = st->out_gpio_descs->ndescs;
I'm wondering if you need these conditionals. Is it even possible you have ndescs > 0 and no descriptors?
> + switch (info) { > + case IIO_CHAN_INFO_RAW: > + if (channel < in_num_ch) {
> + *val = gpiod_get_value_cansleep( > + st->in_gpio_descs->desc[channel]);
Strange indentation...
> + } else { > + channel -= in_num_ch;
> + *val = gpiod_get_value_cansleep( > + st->out_gpio_descs->desc[channel]);
Ditto.
> + } > + return IIO_VAL_INT; > + default: > + return -EINVAL; > + } > +} > + > +static int one_bit_adc_dac_write_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int val, > + int val2, > + long info) > +{
Same comments as per previous function above.
> +}
...
> + ret = device_property_read_string_array(&st->pdev->dev, > + propname, > + gpio_names, > + num_ch);
> + if (ret < 0)
if (ret)
> + return ret;
...
> + channels = devm_kcalloc(indio_dev->dev.parent, (in_num_ch + out_num_ch),
Extra parentheses.
> + sizeof(struct iio_chan_spec), GFP_KERNEL);
sizeof(*channels) ?
> + if (!channels) > + return -ENOMEM;
...
> +static const struct of_device_id one_bit_adc_dac_dt_match[] = { > + { .compatible = "adi,one-bit-adc-dac" },
> + {},
No comma here!
> +};
> +
Unnecessary blank line.
> +MODULE_DEVICE_TABLE(of, one_bit_adc_dac_dt_match);
...
> +
Ditto.
> +module_platform_driver(one_bit_adc_dac_driver);
-- With Best Regards, Andy Shevchenko
| |