Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220223163525.13399-1-jagathjog1996@gmail.com> <20220223163525.13399-6-jagathjog1996@gmail.com>
In-Reply-To: <20220223163525.13399-6-jagathjog1996@gmail.com>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Thu, 24 Feb 2022 02:46:34 +0200
Message-ID: <CAHp75VcKvXi6oUm2Ysx_F+Myu6Dy9-Vxp9MwTcrT5si0Fe6c7w@mail.gmail.com>
Subject: Re: [PATCH v5 5/7] iio: potentiometer: Add device specific read_raw function
To:     Jagath Jog J <jagathjog1996@gmail.com>
Cc:     Jonathan Cameron <jic23@kernel.org>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Slawomir Stepien <sst@poczta.fm>,
        Rob Herring <robh+dt@kernel.org>,
        linux-iio <linux-iio@vger.kernel.org>,
        devicetree <devicetree@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Feb 23, 2022 at 6:35 PM Jagath Jog J <jagathjog1996@gmail.com> wrote:
>
> Added function pointer in the device specific structure to
> call the appropriate device read_raw function, so that the
> other type of devices with different read method can be
> handled.
>
> Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
> ---
>  drivers/iio/potentiometer/ds1803.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/potentiometer/ds1803.c b/drivers/iio/potentiometer/ds1803.c
> index aeb74ee46fbc..878188351f8f 100644
> --- a/drivers/iio/potentiometer/ds1803.c
> +++ b/drivers/iio/potentiometer/ds1803.c
> @@ -32,6 +32,8 @@ struct ds1803_cfg {
>         int kohms;
>         const struct iio_chan_spec *channels;
>         u8 num_channels;
> +       int (*read)(struct iio_dev *indio_dev,
> +                   struct iio_chan_spec const *chan, int *val);
>  };
>
>  struct ds1803_data {
> @@ -55,6 +57,22 @@ static const struct iio_chan_spec ds1803_channels[] = {
>         DS1803_CHANNEL(1, DS1803_WIPER_1),
>  };
>
> +static int ds1803_read(struct iio_dev *indio_dev,
> +                      struct iio_chan_spec const *chan,
> +                      int *val)
> +{
> +       struct ds1803_data *data = iio_priv(indio_dev);
> +       int ret;
> +       u8 result[ARRAY_SIZE(ds1803_channels)];
> +
> +       ret = i2c_master_recv(data->client, result, indio_dev->num_channels);
> +       if (ret < 0)
> +               return ret;
> +
> +       *val = result[chan->channel];
> +       return ret;
> +}
> +
>  static const struct ds1803_cfg ds1803_cfg[] = {
>         [DS1803_010] = {
>           .wipers = 2,
> @@ -62,6 +80,7 @@ static const struct ds1803_cfg ds1803_cfg[] = {
>           .kohms =  10,
>           .channels = ds1803_channels,
>           .num_channels = ARRAY_SIZE(ds1803_channels),
> +         .read = ds1803_read,
>         },
>         [DS1803_050] = {
>           .wipers = 2,
> @@ -69,6 +88,7 @@ static const struct ds1803_cfg ds1803_cfg[] = {
>           .kohms =  50,
>           .channels = ds1803_channels,
>           .num_channels = ARRAY_SIZE(ds1803_channels),
> +         .read = ds1803_read,
>         },
>         [DS1803_100] = {
>           .wipers = 2,
> @@ -76,6 +96,7 @@ static const struct ds1803_cfg ds1803_cfg[] = {
>           .kohms = 100,
>           .channels = ds1803_channels,
>           .num_channels = ARRAY_SIZE(ds1803_channels),
> +         .read = ds1803_read,
>         },
>  };
>
> @@ -84,20 +105,15 @@ static int ds1803_read_raw(struct iio_dev *indio_dev,
>                            int *val, int *val2, long mask)
>  {
>         struct ds1803_data *data = iio_priv(indio_dev);
> -       int pot = chan->channel;
>         int ret;
> -       u8 result[ARRAY_SIZE(ds1803_channels)];
>
>         switch (mask) {
>         case IIO_CHAN_INFO_RAW:
> -               ret = i2c_master_recv(data->client, result,
> -                                     indio_dev->num_channels);
> +               ret = data->cfg->read(indio_dev, chan, val);
>                 if (ret < 0)
>                         return ret;
>
> -               *val = result[pot];
>                 return IIO_VAL_INT;

> -

Seems like a stray change. Up to Jonathan to decide what to do (no
need for resend b/c of this).

>         case IIO_CHAN_INFO_SCALE:
>                 *val = 1000 * data->cfg->kohms;
>                 *val2 = data->cfg->avail[2]; /* Max wiper position */
> --
> 2.17.1
>


-- 
With Best Regards,
Andy Shevchenko