[PATCH 0/2] iio: potentiometer: tpl0102: improve driver to report steps/range

[PATCH 0/2] iio: potentiometer: tpl0102: improve driver to report steps/range

[Matt Ranostay]

This patchset enables userspace to know how many steps a respective
potentiometer can support.

Matt Ranostay (2):
  iio: potentiometer: tpl0102: switch to using pointer to chip config
  iio: potentiometer: tpl0102: add IIO_AVAIL_RANGE support

 drivers/iio/potentiometer/tpl0102.c | 41 +++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 11 deletions(-)

-- 
2.17.1

[PATCH 1/2] iio: potentiometer: tpl0102: switch to using pointer to chip config

[Matt Ranostay]

More concise to have a pointer to tpl0102_cfg struct in the iio_priv
data than an integer to an index of an array.

Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
---
 drivers/iio/potentiometer/tpl0102.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/potentiometer/tpl0102.c b/drivers/iio/potentiometer/tpl0102.c
index ca1cce58fe20..8e8adabe672b 100644
--- a/drivers/iio/potentiometer/tpl0102.c
+++ b/drivers/iio/potentiometer/tpl0102.c
@@ -37,7 +37,7 @@ static const struct tpl0102_cfg tpl0102_cfg[] = {
 
 struct tpl0102_data {
 	struct regmap *regmap;
-	unsigned long devid;
+	const struct tpl0102_cfg *cfg;
 };
 
 static const struct regmap_config tpl0102_regmap_config = {
@@ -72,8 +72,8 @@ static int tpl0102_read_raw(struct iio_dev *indio_dev,
 		return ret ? ret : IIO_VAL_INT;
 	}
 	case IIO_CHAN_INFO_SCALE:
-		*val = 1000 * tpl0102_cfg[data->devid].kohms;
-		*val2 = tpl0102_cfg[data->devid].max_pos;
+		*val = 1000 * data->cfg->kohms;
+		*val2 = data->cfg->max_pos;
 		return IIO_VAL_FRACTIONAL;
 	}
 
@@ -89,7 +89,7 @@ static int tpl0102_write_raw(struct iio_dev *indio_dev,
 	if (mask != IIO_CHAN_INFO_RAW)
 		return -EINVAL;
 
-	if (val >= tpl0102_cfg[data->devid].max_pos || val < 0)
+	if (val >= data->cfg->max_pos || val < 0)
 		return -EINVAL;
 
 	return regmap_write(data->regmap, chan->channel, val);
@@ -113,7 +113,7 @@ static int tpl0102_probe(struct i2c_client *client,
 	data = iio_priv(indio_dev);
 	i2c_set_clientdata(client, indio_dev);
 
-	data->devid = id->driver_data;
+	data->cfg = &tpl0102_cfg[id->driver_data];
 	data->regmap = devm_regmap_init_i2c(client, &tpl0102_regmap_config);
 	if (IS_ERR(data->regmap)) {
 		dev_err(dev, "regmap initialization failed\n");
@@ -123,7 +123,7 @@ static int tpl0102_probe(struct i2c_client *client,
 	indio_dev->dev.parent = dev;
 	indio_dev->info = &tpl0102_info;
 	indio_dev->channels = tpl0102_channels;
-	indio_dev->num_channels = tpl0102_cfg[data->devid].wipers;
+	indio_dev->num_channels = data->cfg->wipers;
 	indio_dev->name = client->name;
 
 	return devm_iio_device_register(dev, indio_dev);
-- 
2.17.1

[PATCH 2/2] iio: potentiometer: tpl0102: add IIO_AVAIL_RANGE support

[Matt Ranostay]

Report back the step range of the respective potentiometers that are
possible back to userspace.

Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
---
 drivers/iio/potentiometer/tpl0102.c | 33 +++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/potentiometer/tpl0102.c b/drivers/iio/potentiometer/tpl0102.c
index 8e8adabe672b..d5aedf358fc2 100644
--- a/drivers/iio/potentiometer/tpl0102.c
+++ b/drivers/iio/potentiometer/tpl0102.c
@@ -15,7 +15,7 @@
 
 struct tpl0102_cfg {
 	int wipers;
-	int max_pos;
+	int avail[3];
 	int kohms;
 };
 
@@ -28,11 +28,11 @@ enum tpl0102_type {
 
 static const struct tpl0102_cfg tpl0102_cfg[] = {
 	/* on-semiconductor parts */
-	[CAT5140_503] = { .wipers = 1, .max_pos = 256, .kohms = 50, },
-	[CAT5140_104] = { .wipers = 1, .max_pos = 256, .kohms = 100, },
+	[CAT5140_503] = { .wipers = 1, .avail = { 0, 1, 255 }, .kohms = 50, },
+	[CAT5140_104] = { .wipers = 1, .avail = { 0, 1, 255 }, .kohms = 100, },
 	/* ti parts */
-	[TPL0102_104] = { .wipers = 2, .max_pos = 256, .kohms = 100 },
-	[TPL0401_103] = { .wipers = 1, .max_pos = 128, .kohms = 10, },
+	[TPL0102_104] = { .wipers = 2, .avail = { 0, 1, 255 }, .kohms = 100 },
+	[TPL0401_103] = { .wipers = 1, .avail = { 0, 1, 127 }, .kohms = 10, },
 };
 
 struct tpl0102_data {
@@ -73,13 +73,31 @@ static int tpl0102_read_raw(struct iio_dev *indio_dev,
 	}
 	case IIO_CHAN_INFO_SCALE:
 		*val = 1000 * data->cfg->kohms;
-		*val2 = data->cfg->max_pos;
+		*val2 = data->cfg->avail[2] + 1;
 		return IIO_VAL_FRACTIONAL;
 	}
 
 	return -EINVAL;
 }
 
+static int tpl0102_read_avail(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      const int **vals, int *type, int *length,
+			      long mask)
+{
+	struct tpl0102_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		*length = ARRAY_SIZE(data->cfg->avail);
+		*vals = data->cfg->avail;
+		*type = IIO_VAL_INT;
+		return IIO_AVAIL_RANGE;
+	}
+
+	return -EINVAL;
+}
+
 static int tpl0102_write_raw(struct iio_dev *indio_dev,
 			     struct iio_chan_spec const *chan,
 			     int val, int val2, long mask)
@@ -89,7 +107,7 @@ static int tpl0102_write_raw(struct iio_dev *indio_dev,
 	if (mask != IIO_CHAN_INFO_RAW)
 		return -EINVAL;
 
-	if (val >= data->cfg->max_pos || val < 0)
+	if (val > data->cfg->avail[2] || val < 0)
 		return -EINVAL;
 
 	return regmap_write(data->regmap, chan->channel, val);
@@ -97,6 +115,7 @@ static int tpl0102_write_raw(struct iio_dev *indio_dev,
 
 static const struct iio_info tpl0102_info = {
 	.read_raw = tpl0102_read_raw,
+	.read_avail = tpl0102_read_avail,
 	.write_raw = tpl0102_write_raw,
 };
 
-- 
2.17.1

Re: [PATCH 1/2] iio: potentiometer: tpl0102: switch to using pointer to chip config

[Slawomir Stepien]

On paź 24, 2018 11:38, Matt Ranostay wrote:
> More concise to have a pointer to tpl0102_cfg struct in the iio_priv
> data than an integer to an index of an array.
> 
> Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
> ---
>  drivers/iio/potentiometer/tpl0102.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

Looks good to me.

Reviewed-by: Slawomir Stepien <sst@poczta.fm>

-- 
Slawomir Stepien

Re: [PATCH 2/2] iio: potentiometer: tpl0102: add IIO_AVAIL_RANGE support

[Slawomir Stepien]

On paź 24, 2018 11:38, Matt Ranostay wrote:
> Report back the step range of the respective potentiometers that are
> possible back to userspace.

The many "back" in that sentence don't you think? ;)

Maybe: "Report the possible step range of the respective potentiometers back to
userspace."

> Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
> ---
>  drivers/iio/potentiometer/tpl0102.c | 33 +++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 7 deletions(-)

The rest looks good to me

Reviewed-by: Slawomir Stepien <sst@poczta.fm>

-- 
Slawomir Stepien

Re: [PATCH 1/2] iio: potentiometer: tpl0102: switch to using pointer to chip config

[Jonathan Cameron]

On Wed, 24 Oct 2018 19:30:12 +0200
Slawomir Stepien <sst@poczta.fm> wrote:

> On pa=C5=BA 24, 2018 11:38, Matt Ranostay wrote:
> > More concise to have a pointer to tpl0102_cfg struct in the iio_priv
> > data than an integer to an index of an array.
> >=20
> > Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
> > ---
> >  drivers/iio/potentiometer/tpl0102.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-) =20
>=20
> Looks good to me.
>=20
> Reviewed-by: Slawomir Stepien <sst@poczta.fm>
>=20
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

Re: [PATCH 2/2] iio: potentiometer: tpl0102: add IIO_AVAIL_RANGE support

[Jonathan Cameron]

On Wed, 24 Oct 2018 11:38:58 +0100
Matt Ranostay <matt.ranostay@konsulko.com> wrote:

> Report back the step range of the respective potentiometers that are
> possible back to userspace.
> 
> Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
Hi Matt,

Hmm. I had to actually look at the code to remember how we register
the available stuff.  The early plan was to just do it for all provided
info_mask elements but in the end we decided that was too much to
require and went with individual masks.

So to actually have these appear to userspace I think you need an
entry in 
info_mask_separate_available which is missing here.

Jonathan

> ---
>  drivers/iio/potentiometer/tpl0102.c | 33 +++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/potentiometer/tpl0102.c b/drivers/iio/potentiometer/tpl0102.c
> index 8e8adabe672b..d5aedf358fc2 100644
> --- a/drivers/iio/potentiometer/tpl0102.c
> +++ b/drivers/iio/potentiometer/tpl0102.c
> @@ -15,7 +15,7 @@
>  
>  struct tpl0102_cfg {
>  	int wipers;
> -	int max_pos;
> +	int avail[3];
>  	int kohms;
>  };
>  
> @@ -28,11 +28,11 @@ enum tpl0102_type {
>  
>  static const struct tpl0102_cfg tpl0102_cfg[] = {
>  	/* on-semiconductor parts */
> -	[CAT5140_503] = { .wipers = 1, .max_pos = 256, .kohms = 50, },
> -	[CAT5140_104] = { .wipers = 1, .max_pos = 256, .kohms = 100, },
> +	[CAT5140_503] = { .wipers = 1, .avail = { 0, 1, 255 }, .kohms = 50, },
> +	[CAT5140_104] = { .wipers = 1, .avail = { 0, 1, 255 }, .kohms = 100, },
>  	/* ti parts */
> -	[TPL0102_104] = { .wipers = 2, .max_pos = 256, .kohms = 100 },
> -	[TPL0401_103] = { .wipers = 1, .max_pos = 128, .kohms = 10, },
> +	[TPL0102_104] = { .wipers = 2, .avail = { 0, 1, 255 }, .kohms = 100 },
> +	[TPL0401_103] = { .wipers = 1, .avail = { 0, 1, 127 }, .kohms = 10, },
>  };
>  
>  struct tpl0102_data {
> @@ -73,13 +73,31 @@ static int tpl0102_read_raw(struct iio_dev *indio_dev,
>  	}
>  	case IIO_CHAN_INFO_SCALE:
>  		*val = 1000 * data->cfg->kohms;
> -		*val2 = data->cfg->max_pos;
> +		*val2 = data->cfg->avail[2] + 1;
>  		return IIO_VAL_FRACTIONAL;
>  	}
>  
>  	return -EINVAL;
>  }
>  
> +static int tpl0102_read_avail(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      const int **vals, int *type, int *length,
> +			      long mask)
> +{
> +	struct tpl0102_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		*length = ARRAY_SIZE(data->cfg->avail);
> +		*vals = data->cfg->avail;
> +		*type = IIO_VAL_INT;
> +		return IIO_AVAIL_RANGE;
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  static int tpl0102_write_raw(struct iio_dev *indio_dev,
>  			     struct iio_chan_spec const *chan,
>  			     int val, int val2, long mask)
> @@ -89,7 +107,7 @@ static int tpl0102_write_raw(struct iio_dev *indio_dev,
>  	if (mask != IIO_CHAN_INFO_RAW)
>  		return -EINVAL;
>  
> -	if (val >= data->cfg->max_pos || val < 0)
> +	if (val > data->cfg->avail[2] || val < 0)
>  		return -EINVAL;
>  
>  	return regmap_write(data->regmap, chan->channel, val);
> @@ -97,6 +115,7 @@ static int tpl0102_write_raw(struct iio_dev *indio_dev,
>  
>  static const struct iio_info tpl0102_info = {
>  	.read_raw = tpl0102_read_raw,
> +	.read_avail = tpl0102_read_avail,
>  	.write_raw = tpl0102_write_raw,
>  };
>