Messages in this thread | | | From | Andy Shevchenko <> | Date | Wed, 12 Jan 2022 17:23:31 +0200 | Subject | Re: [PATCH v2 2/2] one-bit-adc-dac: Add initial version of one bit ADC-DAC |
| |
On Wed, Jan 12, 2022 at 11:50 AM Cristian Pop <cristian.pop@analog.com> wrote: > > This allows remote reading and writing of the GPIOs. This is useful in > application that run on another PC, at system level, where multiple iio > devices and GPIO devices are integrated together.
Should it be called GPIO-IIO proxy or something like that?
...
> +/*
> + * one-bit-adc-dac > + *
These two lines make no sense.
> + * Copyright 2022 Analog Devices Inc. > + */
...
> +enum ch_direction { > + CH_IN, > + CH_OUT, > +};
How is it different from the corresponding GPIO flag?
...
> +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)
Can be compressed to a fewer LOCs.
...
> + return sprintf(label, "%s\n", st->labels[ch]);
In a few releases the sysfs_emit() is present and must be used.
...
> + fwnode = dev_fwnode(device);
No need. See below.
...
> + child_num = device_get_child_node_count(device);
Error checks?
...
> + st->labels = devm_kzalloc(device, sizeof(*st->labels) * child_num, GFP_KERNEL);
You should use devm_kcalloc() instead, it does slightly more than simple multiplication.
> + if (!st->labels) > + return -ENOMEM;
...
> + fwnode_for_each_child_node(fwnode, child) {
device_for_each_...
> + if (fwnode_property_read_u32(child, "reg", &crt_ch)) > + continue; > + > + if (crt_ch >= num_channels) > + continue; > + > + if (fwnode_property_read_string(child, "label", &label)) > + continue; > + > + chan = &channels[crt_ch]; > + st->labels[--i] = label; > + }
...
> +MODULE_LICENSE("Dual BSD/GPL"); > \ No newline at end of file
Aiaiai.
-- With Best Regards, Andy Shevchenko
| |