On Sun, 2024-02-18 at 17:27 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>
> Using the generic firmware data access functions from property.h
> provides a number of advantages:
> 1) Works with different firmware types.
> 2) Doesn't provide a 'bad' example for new IIO drivers.
> 3) Lets us use the new _scoped() loops with automatic reference count
> cleanup for fwnode_handle
>
> Cc: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Cc: Michael Hennerich <Michael.Hennerich@xxxxxxxxxx>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> ---
Just one minor nit from me... Still, fell free to add:
Reviewed-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
> drivers/iio/adc/ad7124.c | 55 +++++++++++++++++-----------------------
> 1 file changed, 23 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
> index b9b206fcd748..e7b1d517d3de 100644
> --- a/drivers/iio/adc/ad7124.c
> +++ b/drivers/iio/adc/ad7124.c
> @@ -14,7 +14,8 @@
> #include <linux/kernel.h>
> #include <linux/kfifo.h>
> #include <linux/module.h>
> -#include <linux/of.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/property.h>
> #include <linux/regulator/consumer.h>
> #include <linux/spi/spi.h>
>
> @@ -807,22 +808,19 @@ static int ad7124_check_chip_id(struct ad7124_state *st)
> return 0;
> }
>
> -static int ad7124_of_parse_channel_config(struct iio_dev *indio_dev,
> - struct device_node *np)
> +static int ad7124_parse_channel_config(struct iio_dev *indio_dev,
> + struct device *dev)
> {
> struct ad7124_state *st = iio_priv(indio_dev);
> struct ad7124_channel_config *cfg;
> struct ad7124_channel *channels;
> - struct device_node *child;
> struct iio_chan_spec *chan;
> unsigned int ain[2], channel = 0, tmp;
> int ret;
>
> - st->num_channels = of_get_available_child_count(np);
> - if (!st->num_channels) {
> - dev_err(indio_dev->dev.parent, "no channel children\n");
> - return -ENODEV;
> - }
> + st->num_channels = device_get_child_node_count(dev);
> + if (!st->num_channels)
> + return dev_err_probe(dev, -ENODEV, "no channel children\n");
>
> chan = devm_kcalloc(indio_dev->dev.parent, st->num_channels,
> sizeof(*chan), GFP_KERNEL);
> @@ -838,39 +836,38 @@ static int ad7124_of_parse_channel_config(struct iio_dev
> *indio_dev,
> indio_dev->num_channels = st->num_channels;
> st->channels = channels;
>
> - for_each_available_child_of_node(np, child) {
> + device_for_each_child_node_scoped(dev, child) {
> cfg = &st->channels[channel].cfg;
>
> - ret = of_property_read_u32(child, "reg", &channel);
> + ret = fwnode_property_read_u32(child, "reg", &channel);
> if (ret)
> - goto err;
> + return ret;
>
> - if (channel >= indio_dev->num_channels) {
> - dev_err(indio_dev->dev.parent,
> + if (channel >= indio_dev->num_channels)
> + return dev_err_probe(dev, -EINVAL,
> "Channel index >= number of channels\n");
> - ret = -EINVAL;
> - goto err;
> - }
>
> - ret = of_property_read_u32_array(child, "diff-channels",
> - ain, 2);
> + ret = fwnode_property_read_u32_array(child, "diff-channels",
> + ain, 2);
> if (ret)
> - goto err;
> + return ret;
>
> st->channels[channel].nr = channel;
> st->channels[channel].ain = AD7124_CHANNEL_AINP(ain[0]) |
>
> AD7124_CHANNEL_AINM(ain[1]);
>
> - cfg->bipolar = of_property_read_bool(child, "bipolar");
> + cfg->bipolar = fwnode_property_read_bool(child, "bipolar");
>
> - ret = of_property_read_u32(child, "adi,reference-select",
> &tmp);
> + ret = fwnode_property_read_u32(child, "adi,reference-select",
> &tmp);
> if (ret)
> cfg->refsel = AD7124_INT_REF;
> else
> cfg->refsel = tmp;
>
> - cfg->buf_positive = of_property_read_bool(child,
> "adi,buffered-positive");
> - cfg->buf_negative = of_property_read_bool(child,
> "adi,buffered-negative");
> + cfg->buf_positive =
> + fwnode_property_read_bool(child, "adi,buffered-
> positive");
> + cfg->buf_negative =
> + fwnode_property_read_bool(child, "adi,buffered-
> negative");
I think this is one of those cases where 80col limit hurts readability...
- Nuno Sá
