Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 26 Feb 2022 18:40:33 +0000
From:   Jonathan Cameron <jic23@kernel.org>
To:     Jagath Jog J <jagathjog1996@gmail.com>
Cc:     lars@metafoo.de, andy.shevchenko@gmail.com, sst@poczta.fm,
        robh+dt@kernel.org, linux-iio@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 2/7] iio: potentiometer: Add available functionality
Message-ID: <20220226184033.5ec94a17@jic23-huawei>
In-Reply-To: <20220223163525.13399-3-jagathjog1996@gmail.com>
References: <20220223163525.13399-1-jagathjog1996@gmail.com>
        <20220223163525.13399-3-jagathjog1996@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 23 Feb 2022 22:05:20 +0530
Jagath Jog J <jagathjog1996@gmail.com> wrote:

> Adding available functionality for DS1803 driver which
> will show the minimum, step and maximum values that the
> driver can accepts through sysfs entry.
> Now using the max value present in avail array instead of chip
> type specific macro to make the driver flexible to add other
> type of potentiometer with different max position value.
> 
> Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
> ---
>  drivers/iio/potentiometer/ds1803.c | 61 ++++++++++++++++++++++--------
>  1 file changed, 45 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/iio/potentiometer/ds1803.c b/drivers/iio/potentiometer/ds1803.c
> index 3c542a50ece6..9610269bed7f 100644
> --- a/drivers/iio/potentiometer/ds1803.c
> +++ b/drivers/iio/potentiometer/ds1803.c
> @@ -16,7 +16,6 @@
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
>  
> -#define DS1803_MAX_POS		255
>  #define DS1803_WRITE(chan)	(0xa8 | ((chan) + 1))
>  
>  enum ds1803_type {
> @@ -26,27 +25,23 @@ enum ds1803_type {
>  };
>  
>  struct ds1803_cfg {
> +	int avail[3];
>  	int kohms;
>  };
>  
> -static const struct ds1803_cfg ds1803_cfg[] = {
> -	[DS1803_010] = { .kohms =  10, },
> -	[DS1803_050] = { .kohms =  50, },
> -	[DS1803_100] = { .kohms = 100, },
> -};
> -
>  struct ds1803_data {
>  	struct i2c_client *client;
>  	const struct ds1803_cfg *cfg;
>  };
>  
> -#define DS1803_CHANNEL(ch) {					\
> -	.type = IIO_RESISTANCE,					\
> -	.indexed = 1,						\
> -	.output = 1,						\
> -	.channel = (ch),					\
> -	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> -	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
> +#define DS1803_CHANNEL(ch) {						\
> +	.type = IIO_RESISTANCE,						\
> +	.indexed = 1,							\
> +	.output = 1,							\
> +	.channel = (ch),						\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
> +	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_RAW),   \
>  }
>  
>  static const struct iio_chan_spec ds1803_channels[] = {
> @@ -54,6 +49,21 @@ static const struct iio_chan_spec ds1803_channels[] = {
>  	DS1803_CHANNEL(1),
>  };
>  
> +static const struct ds1803_cfg ds1803_cfg[] = {
> +	[DS1803_010] = {
> +	  .avail = { 0, 1, 255 },
Why non standard indenting.  These should be a tab in rather than
a couple of spaces.

I'll fixup whilst applying.

Jonathan

> +	  .kohms =  10,
> +	},
> +	[DS1803_050] = {
> +	  .avail = { 0, 1, 255 },
> +	  .kohms =  50,
> +	},
> +	[DS1803_100] = {
> +	  .avail = { 0, 1, 255 },
> +	  .kohms = 100,
> +	},
> +};
> +
>  static int ds1803_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan,
>  			   int *val, int *val2, long mask)
> @@ -75,7 +85,7 @@ static int ds1803_read_raw(struct iio_dev *indio_dev,
>  
>  	case IIO_CHAN_INFO_SCALE:
>  		*val = 1000 * data->cfg->kohms;
> -		*val2 = DS1803_MAX_POS;
> +		*val2 = data->cfg->avail[2]; /* Max wiper position */
>  		return IIO_VAL_FRACTIONAL;
>  	}
>  
> @@ -88,13 +98,14 @@ static int ds1803_write_raw(struct iio_dev *indio_dev,
>  {
>  	struct ds1803_data *data = iio_priv(indio_dev);
>  	int pot = chan->channel;
> +	int max_pos = data->cfg->avail[2];
>  
>  	if (val2 != 0)
>  		return -EINVAL;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> -		if (val > DS1803_MAX_POS || val < 0)
> +		if (val > max_pos || val < 0)
>  			return -EINVAL;
>  		break;
>  	default:
> @@ -104,9 +115,27 @@ static int ds1803_write_raw(struct iio_dev *indio_dev,
>  	return i2c_smbus_write_byte_data(data->client, DS1803_WRITE(pot), val);
>  }
>  
> +static int ds1803_read_avail(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     const int **vals, int *type,
> +			     int *length, long mask)
> +{
> +	struct ds1803_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		*vals = data->cfg->avail;
> +		*length = ARRAY_SIZE(data->cfg->avail);
> +		*type = IIO_VAL_INT;
> +		return IIO_AVAIL_RANGE;
> +	}
> +	return -EINVAL;
> +}
> +
>  static const struct iio_info ds1803_info = {
>  	.read_raw = ds1803_read_raw,
>  	.write_raw = ds1803_write_raw,
> +	.read_avail = ds1803_read_avail,
>  };
>  
>  static int ds1803_probe(struct i2c_client *client, const struct i2c_device_id *id)