diff mbox series

[3/5] iio: amplifiers: ad8366: rework driver to allow other chips

Message ID 20190530131812.3476-3-alexandru.ardelean@analog.com (mailing list archive)
State New, archived
Headers show
Series [1/5] iio: amplifiers: update license information | expand

Commit Message

Alexandru Ardelean May 30, 2019, 1:18 p.m. UTC
The SPI gain amplifiers are simple devices, with 1 or 2 channels, to which
are read-from/written-to.

The gain computation in ad8366_write_raw() has been updated to compute gain
in dB for negative values.

This driver will be extended to support other chips as well.
To do that, this rework handles the AD8366 device as a special-case (via
switch-statements). This will make things easier when adding new chips.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/amplifiers/ad8366.c | 81 +++++++++++++++++++++++++--------
 1 file changed, 62 insertions(+), 19 deletions(-)

Comments

Jonathan Cameron June 8, 2019, 1:57 p.m. UTC | #1
On Thu, 30 May 2019 16:18:10 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> The SPI gain amplifiers are simple devices, with 1 or 2 channels, to which
> are read-from/written-to.
> 
> The gain computation in ad8366_write_raw() has been updated to compute gain
> in dB for negative values.
> 
> This driver will be extended to support other chips as well.
> To do that, this rework handles the AD8366 device as a special-case (via
> switch-statements). This will make things easier when adding new chips.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Looks good to me.

Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

thanks,

Jonathan

> ---
>  drivers/iio/amplifiers/ad8366.c | 81 +++++++++++++++++++++++++--------
>  1 file changed, 62 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c
> index 24ff5475d9f2..1beda6409301 100644
> --- a/drivers/iio/amplifiers/ad8366.c
> +++ b/drivers/iio/amplifiers/ad8366.c
> @@ -18,11 +18,22 @@
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  
> +enum ad8366_type {
> +	ID_AD8366,
> +};
> +
> +struct ad8366_info {
> +	int gain_min;
> +	int gain_max;
> +};
> +
>  struct ad8366_state {
>  	struct spi_device	*spi;
>  	struct regulator	*reg;
>  	struct mutex            lock; /* protect sensor state */
>  	unsigned char		ch[2];
> +	enum ad8366_type	type;
> +	struct ad8366_info	*info;
>  	/*
>  	 * DMA (thus cache coherency maintenance) requires the
>  	 * transfer buffers to live in their own cache lines.
> @@ -30,19 +41,30 @@ struct ad8366_state {
>  	unsigned char		data[2] ____cacheline_aligned;
>  };
>  
> +static struct ad8366_info ad8366_infos[] = {
> +	[ID_AD8366] = {
> +		.gain_min = 4500,
> +		.gain_max = 20500,
> +	},
> +};
> +
>  static int ad8366_write(struct iio_dev *indio_dev,
>  			unsigned char ch_a, unsigned char ch_b)
>  {
>  	struct ad8366_state *st = iio_priv(indio_dev);
>  	int ret;
>  
> -	ch_a = bitrev8(ch_a & 0x3F);
> -	ch_b = bitrev8(ch_b & 0x3F);
> +	switch (st->type) {
> +	case ID_AD8366:
> +		ch_a = bitrev8(ch_a & 0x3F);
> +		ch_b = bitrev8(ch_b & 0x3F);
>  
> -	st->data[0] = ch_b >> 4;
> -	st->data[1] = (ch_b << 4) | (ch_a >> 2);
> +		st->data[0] = ch_b >> 4;
> +		st->data[1] = (ch_b << 4) | (ch_a >> 2);
> +		break;
> +	}
>  
> -	ret = spi_write(st->spi, st->data, ARRAY_SIZE(st->data));
> +	ret = spi_write(st->spi, st->data, indio_dev->num_channels);
>  	if (ret < 0)
>  		dev_err(&indio_dev->dev, "write failed (%d)", ret);
>  
> @@ -57,17 +79,22 @@ static int ad8366_read_raw(struct iio_dev *indio_dev,
>  {
>  	struct ad8366_state *st = iio_priv(indio_dev);
>  	int ret;
> -	unsigned code;
> +	int code, gain = 0;
>  
>  	mutex_lock(&st->lock);
>  	switch (m) {
>  	case IIO_CHAN_INFO_HARDWAREGAIN:
>  		code = st->ch[chan->channel];
>  
> +		switch (st->type) {
> +		case ID_AD8366:
> +			gain = code * 253 + 4500;
> +			break;
> +		}
> +
>  		/* Values in dB */
> -		code = code * 253 + 4500;
> -		*val = code / 1000;
> -		*val2 = (code % 1000) * 1000;
> +		*val = gain / 1000;
> +		*val2 = (gain % 1000) * 1000;
>  
>  		ret = IIO_VAL_INT_PLUS_MICRO_DB;
>  		break;
> @@ -86,19 +113,24 @@ static int ad8366_write_raw(struct iio_dev *indio_dev,
>  			    long mask)
>  {
>  	struct ad8366_state *st = iio_priv(indio_dev);
> -	unsigned code;
> +	struct ad8366_info *inf = st->info;
> +	int code = 0, gain;
>  	int ret;
>  
> -	if (val < 0 || val2 < 0)
> -		return -EINVAL;
> -
>  	/* Values in dB */
> -	code = (((u8)val * 1000) + ((u32)val2 / 1000));
> +	if (val < 0)
> +		gain = (val * 1000) - (val2 / 1000);
> +	else
> +		gain = (val * 1000) + (val2 / 1000);
>  
> -	if (code > 20500 || code < 4500)
> +	if (gain > inf->gain_max || gain < inf->gain_min)
>  		return -EINVAL;
>  
> -	code = (code - 4500) / 253;
> +	switch (st->type) {
> +	case ID_AD8366:
> +		code = (gain - 4500) / 253;
> +		break;
> +	}
>  
>  	mutex_lock(&st->lock);
>  	switch (mask) {
> @@ -154,13 +186,24 @@ static int ad8366_probe(struct spi_device *spi)
>  	spi_set_drvdata(spi, indio_dev);
>  	mutex_init(&st->lock);
>  	st->spi = spi;
> +	st->type = spi_get_device_id(spi)->driver_data;
> +
> +	switch (st->type) {
> +	case ID_AD8366:
> +		indio_dev->channels = ad8366_channels;
> +		indio_dev->num_channels = ARRAY_SIZE(ad8366_channels);
> +		break;
> +	default:
> +		dev_err(&spi->dev, "Invalid device ID\n");
> +		ret = -EINVAL;
> +		goto error_disable_reg;
> +	}
>  
> +	st->info = &ad8366_infos[st->type];
>  	indio_dev->dev.parent = &spi->dev;
>  	indio_dev->name = spi_get_device_id(spi)->name;
>  	indio_dev->info = &ad8366_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> -	indio_dev->channels = ad8366_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(ad8366_channels);
>  
>  	ret = ad8366_write(indio_dev, 0 , 0);
>  	if (ret < 0)
> @@ -194,7 +237,7 @@ static int ad8366_remove(struct spi_device *spi)
>  }
>  
>  static const struct spi_device_id ad8366_id[] = {
> -	{"ad8366", 0},
> +	{"ad8366",  ID_AD8366},
>  	{}
>  };
>  MODULE_DEVICE_TABLE(spi, ad8366_id);
diff mbox series

Patch

diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c
index 24ff5475d9f2..1beda6409301 100644
--- a/drivers/iio/amplifiers/ad8366.c
+++ b/drivers/iio/amplifiers/ad8366.c
@@ -18,11 +18,22 @@ 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
+enum ad8366_type {
+	ID_AD8366,
+};
+
+struct ad8366_info {
+	int gain_min;
+	int gain_max;
+};
+
 struct ad8366_state {
 	struct spi_device	*spi;
 	struct regulator	*reg;
 	struct mutex            lock; /* protect sensor state */
 	unsigned char		ch[2];
+	enum ad8366_type	type;
+	struct ad8366_info	*info;
 	/*
 	 * DMA (thus cache coherency maintenance) requires the
 	 * transfer buffers to live in their own cache lines.
@@ -30,19 +41,30 @@  struct ad8366_state {
 	unsigned char		data[2] ____cacheline_aligned;
 };
 
+static struct ad8366_info ad8366_infos[] = {
+	[ID_AD8366] = {
+		.gain_min = 4500,
+		.gain_max = 20500,
+	},
+};
+
 static int ad8366_write(struct iio_dev *indio_dev,
 			unsigned char ch_a, unsigned char ch_b)
 {
 	struct ad8366_state *st = iio_priv(indio_dev);
 	int ret;
 
-	ch_a = bitrev8(ch_a & 0x3F);
-	ch_b = bitrev8(ch_b & 0x3F);
+	switch (st->type) {
+	case ID_AD8366:
+		ch_a = bitrev8(ch_a & 0x3F);
+		ch_b = bitrev8(ch_b & 0x3F);
 
-	st->data[0] = ch_b >> 4;
-	st->data[1] = (ch_b << 4) | (ch_a >> 2);
+		st->data[0] = ch_b >> 4;
+		st->data[1] = (ch_b << 4) | (ch_a >> 2);
+		break;
+	}
 
-	ret = spi_write(st->spi, st->data, ARRAY_SIZE(st->data));
+	ret = spi_write(st->spi, st->data, indio_dev->num_channels);
 	if (ret < 0)
 		dev_err(&indio_dev->dev, "write failed (%d)", ret);
 
@@ -57,17 +79,22 @@  static int ad8366_read_raw(struct iio_dev *indio_dev,
 {
 	struct ad8366_state *st = iio_priv(indio_dev);
 	int ret;
-	unsigned code;
+	int code, gain = 0;
 
 	mutex_lock(&st->lock);
 	switch (m) {
 	case IIO_CHAN_INFO_HARDWAREGAIN:
 		code = st->ch[chan->channel];
 
+		switch (st->type) {
+		case ID_AD8366:
+			gain = code * 253 + 4500;
+			break;
+		}
+
 		/* Values in dB */
-		code = code * 253 + 4500;
-		*val = code / 1000;
-		*val2 = (code % 1000) * 1000;
+		*val = gain / 1000;
+		*val2 = (gain % 1000) * 1000;
 
 		ret = IIO_VAL_INT_PLUS_MICRO_DB;
 		break;
@@ -86,19 +113,24 @@  static int ad8366_write_raw(struct iio_dev *indio_dev,
 			    long mask)
 {
 	struct ad8366_state *st = iio_priv(indio_dev);
-	unsigned code;
+	struct ad8366_info *inf = st->info;
+	int code = 0, gain;
 	int ret;
 
-	if (val < 0 || val2 < 0)
-		return -EINVAL;
-
 	/* Values in dB */
-	code = (((u8)val * 1000) + ((u32)val2 / 1000));
+	if (val < 0)
+		gain = (val * 1000) - (val2 / 1000);
+	else
+		gain = (val * 1000) + (val2 / 1000);
 
-	if (code > 20500 || code < 4500)
+	if (gain > inf->gain_max || gain < inf->gain_min)
 		return -EINVAL;
 
-	code = (code - 4500) / 253;
+	switch (st->type) {
+	case ID_AD8366:
+		code = (gain - 4500) / 253;
+		break;
+	}
 
 	mutex_lock(&st->lock);
 	switch (mask) {
@@ -154,13 +186,24 @@  static int ad8366_probe(struct spi_device *spi)
 	spi_set_drvdata(spi, indio_dev);
 	mutex_init(&st->lock);
 	st->spi = spi;
+	st->type = spi_get_device_id(spi)->driver_data;
+
+	switch (st->type) {
+	case ID_AD8366:
+		indio_dev->channels = ad8366_channels;
+		indio_dev->num_channels = ARRAY_SIZE(ad8366_channels);
+		break;
+	default:
+		dev_err(&spi->dev, "Invalid device ID\n");
+		ret = -EINVAL;
+		goto error_disable_reg;
+	}
 
+	st->info = &ad8366_infos[st->type];
 	indio_dev->dev.parent = &spi->dev;
 	indio_dev->name = spi_get_device_id(spi)->name;
 	indio_dev->info = &ad8366_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->channels = ad8366_channels;
-	indio_dev->num_channels = ARRAY_SIZE(ad8366_channels);
 
 	ret = ad8366_write(indio_dev, 0 , 0);
 	if (ret < 0)
@@ -194,7 +237,7 @@  static int ad8366_remove(struct spi_device *spi)
 }
 
 static const struct spi_device_id ad8366_id[] = {
-	{"ad8366", 0},
+	{"ad8366",  ID_AD8366},
 	{}
 };
 MODULE_DEVICE_TABLE(spi, ad8366_id);