Hi!
Sorry for the late reply.
On 2022-02-02 21:44, Andy Shevchenko wrote:
> Convert the module to be property provider agnostic and allow
> it to be used on non-OF platforms.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
> v2: dropped depends on OF
> drivers/iio/multiplexer/Kconfig | 1 -
> drivers/iio/multiplexer/iio-mux.c | 48 ++++++++++++++-----------------
> 2 files changed, 22 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/iio/multiplexer/Kconfig b/drivers/iio/multiplexer/Kconfig
> index a1e1332d1206..928f424a1ed3 100644
> --- a/drivers/iio/multiplexer/Kconfig
> +++ b/drivers/iio/multiplexer/Kconfig
> @@ -9,7 +9,6 @@ menu "Multiplexers"
> config IIO_MUX
> tristate "IIO multiplexer driver"
> select MULTIPLEXER
> - depends on OF || COMPILE_TEST
> help
> Say yes here to build support for the IIO multiplexer.
>
> diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c
> index f422d44377df..e6f0fef96494 100644
> --- a/drivers/iio/multiplexer/iio-mux.c
> +++ b/drivers/iio/multiplexer/iio-mux.c
> @@ -10,11 +10,12 @@
> #include <linux/err.h>
> #include <linux/iio/consumer.h>
> #include <linux/iio/iio.h>
> +#include <linux/mod_devicetable.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> #include <linux/mux/consumer.h>
> -#include <linux/of.h>
> #include <linux/platform_device.h>
> +#include <linux/property.h>
>
> struct mux_ext_info_cache {
> char *data;
> @@ -324,37 +325,20 @@ static int mux_configure_channel(struct device *dev, struct mux *mux,
> return 0;
> }
>
> -/*
> - * Same as of_property_for_each_string(), but also keeps track of the
> - * index of each string.
> - */
> -#define of_property_for_each_string_index(np, propname, prop, s, i) \
> - for (prop = of_find_property(np, propname, NULL), \
> - s = of_prop_next_string(prop, NULL), \
> - i = 0; \
> - s; \
> - s = of_prop_next_string(prop, s), \
> - i++)
> -
> static int mux_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> - struct device_node *np = pdev->dev.of_node;
> struct iio_dev *indio_dev;
> struct iio_channel *parent;
> struct mux *mux;
> - struct property *prop;
> - const char *label;
> + const char **labels;
> u32 state;
> + int children, all_children;
I dislike that the pattern with one declaration per line is broken here.
There are perhaps a few too many variables, but introducing one instance
of two declarations on one line is disturbing for me.
So, with that fixed,
Acked-by: Peter Rosin <peda@xxxxxxxxxx>
> int sizeof_ext_info;
> - int children;
> int sizeof_priv;
> int i;
> int ret;
>
> - if (!np)
> - return -ENODEV;
> -
> parent = devm_iio_channel_get(dev, "parent");
> if (IS_ERR(parent))
> return dev_err_probe(dev, PTR_ERR(parent),
> @@ -366,9 +350,21 @@ static int mux_probe(struct platform_device *pdev)
> sizeof_ext_info *= sizeof(*mux->ext_info);
> }
>
> + all_children = device_property_count_string_array(dev, "channels");
> + if (all_children < 0)
> + return all_children;
> +
> + labels = devm_kmalloc_array(dev, all_children, sizeof(char *), GFP_KERNEL);
I'm a bit sad to see the copy of the labels array. No big deal,
but it seems inelegant. But the of_property_for_each_string_index
macro was also misplaced and therefore inelegant. My guess is
that the device properties should be fairly static and that the
copy is a waste?
So, anyway, I'm just whining here, and the labels copy is ok.
Cheers,
Peter
> + if (!labels)
> + return -ENOMEM;
> +
> + ret = device_property_read_string_array(dev, "channels", labels, all_children);
> + if (ret < 0)
> + return ret;
> +
> children = 0;
> - of_property_for_each_string(np, "channels", prop, label) {
> - if (*label)
> + for (state = 0; state < all_children; state++) {
> + if (*labels[state])
> children++;
> }
> if (children <= 0) {
> @@ -395,7 +391,7 @@ static int mux_probe(struct platform_device *pdev)
> mux->cached_state = -1;
>
> mux->delay_us = 0;
> - of_property_read_u32(np, "settle-time-us", &mux->delay_us);
> + device_property_read_u32(dev, "settle-time-us", &mux->delay_us);
>
> indio_dev->name = dev_name(dev);
> indio_dev->info = &mux_info;
> @@ -426,11 +422,11 @@ static int mux_probe(struct platform_device *pdev)
> }
>
> i = 0;
> - of_property_for_each_string_index(np, "channels", prop, label, state) {
> - if (!*label)
> + for (state = 0; state < all_children; state++) {
> + if (!*labels[state])
> continue;
>
> - ret = mux_configure_channel(dev, mux, state, label, i++);
> + ret = mux_configure_channel(dev, mux, state, labels[state], i++);
> if (ret < 0)
> return ret;
> }
