diff mbox series

[v3,2/2] staging: iio: cdc: ad7746: use dt for capacitive channel setup.

Message ID 3e7f2a0a8960cece185f518ff2b7ceb87891edcd.1619841953.git.lucas.p.stankus@gmail.com (mailing list archive)
State New, archived
Headers show
Series staging: iio: cdc: ad7746: add dt properties for capacitive channel setup | expand

Commit Message

Lucas Stankus May 1, 2021, 12:32 p.m. UTC 
Ditch platform_data fields in favor of device tree properties for
configuring EXCA and EXCB output pins and setting the capacitive channel
excitation level.

As this covers all current use cases for the platform_data struct, remove
ad7746.h header file since it's no longer needed.

Signed-off-by: Lucas Stankus <lucas.p.stankus@gmail.com>
---
 drivers/staging/iio/cdc/ad7746.c | 54 +++++++++++++++++++-------------
 drivers/staging/iio/cdc/ad7746.h | 28 -----------------
 2 files changed, 33 insertions(+), 49 deletions(-)
 delete mode 100644 drivers/staging/iio/cdc/ad7746.h

Comments

Dan Carpenter May 3, 2021, 10:07 a.m. UTC | #1 
On Sat, May 01, 2021 at 09:32:53AM -0300, Lucas Stankus wrote:
> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
> index dfd71e99e872..531f1b96dea2 100644
> --- a/drivers/staging/iio/cdc/ad7746.c
> +++ b/drivers/staging/iio/cdc/ad7746.c
> @@ -18,8 +18,6 @@
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  
> -#include "ad7746.h"
> -
>  /*
>   * AD7746 Register Definition
>   */
> @@ -676,10 +674,11 @@ static const struct iio_info ad7746_info = {
>  static int ad7746_probe(struct i2c_client *client,
>  			const struct i2c_device_id *id)
>  {
> -	struct ad7746_platform_data *pdata = client->dev.platform_data;
> +	struct device *dev = &client->dev;
>  	struct ad7746_chip_info *chip;
>  	struct iio_dev *indio_dev;
>  	unsigned char regval = 0;
> +	unsigned int vdd_permille;
>  	int ret = 0;
>  
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> @@ -703,26 +702,39 @@ static int ad7746_probe(struct i2c_client *client,
>  	indio_dev->num_channels = ARRAY_SIZE(ad7746_channels);
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
> -	if (pdata) {
> -		if (pdata->exca_en) {
> -			if (pdata->exca_inv_en)
> -				regval |= AD7746_EXCSETUP_NEXCA;
> -			else
> -				regval |= AD7746_EXCSETUP_EXCA;
> -		}
> +	if (device_property_read_bool(dev, "adi,exca-output-en")) {
> +		if (device_property_read_bool(dev, "adi,exca-output-invert"))
> +			regval |= AD7746_EXCSETUP_NEXCA;
> +		else
> +			regval |= AD7746_EXCSETUP_EXCA;
> +	}
>  
> -		if (pdata->excb_en) {
> -			if (pdata->excb_inv_en)
> -				regval |= AD7746_EXCSETUP_NEXCB;
> -			else
> -				regval |= AD7746_EXCSETUP_EXCB;
> -		}
> +	if (device_property_read_bool(dev, "adi,excb-output-en")) {
> +		if (device_property_read_bool(dev, "adi,excb-output-invert"))
> +			regval |= AD7746_EXCSETUP_NEXCB;
> +		else
> +			regval |= AD7746_EXCSETUP_EXCB;
> +	}
>  
> -		regval |= AD7746_EXCSETUP_EXCLVL(pdata->exclvl);
> -	} else {
> -		dev_warn(&client->dev, "No platform data? using default\n");
> -		regval = AD7746_EXCSETUP_EXCA | AD7746_EXCSETUP_EXCB |
> -			AD7746_EXCSETUP_EXCLVL(3);
> +	ret = device_property_read_u32(dev, "adi,excitation-vdd-permille",
> +				       &vdd_permille);
> +	if (!ret) {

This test is reversed.  I wonder if the static checkers can catch the
uninitialized variable bug...  It's probably better to write it as:

	if (device_property_read_u32(dev, "adi,excitation-vdd-permille",
				     &vdd_permille) {

So it matches the others.

regards,
dan carpenter;

> +		switch (vdd_permille) {
> +		case 125:
> +			regval |= AD7746_EXCSETUP_EXCLVL(0);
> +			break;
> +		case 250:
> +			regval |= AD7746_EXCSETUP_EXCLVL(1);
> +			break;
> +		case 375:
> +			regval |= AD7746_EXCSETUP_EXCLVL(2);
> +			break;
> +		case 500:
> +			regval |= AD7746_EXCSETUP_EXCLVL(3);
> +			break;
> +		default:
> +			break;
> +		}
>  	}
>  
>  	ret = i2c_smbus_write_byte_data(chip->client,
Dan Carpenter May 3, 2021, 2:44 p.m. UTC | #2 
On Mon, May 03, 2021 at 01:07:20PM +0300, Dan Carpenter wrote:
> On Sat, May 01, 2021 at 09:32:53AM -0300, Lucas Stankus wrote:
> > diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
> > index dfd71e99e872..531f1b96dea2 100644
> > --- a/drivers/staging/iio/cdc/ad7746.c
> > +++ b/drivers/staging/iio/cdc/ad7746.c
> > @@ -18,8 +18,6 @@
> >  #include <linux/iio/iio.h>
> >  #include <linux/iio/sysfs.h>
> >  
> > -#include "ad7746.h"
> > -
> >  /*
> >   * AD7746 Register Definition
> >   */
> > @@ -676,10 +674,11 @@ static const struct iio_info ad7746_info = {
> >  static int ad7746_probe(struct i2c_client *client,
> >  			const struct i2c_device_id *id)
> >  {
> > -	struct ad7746_platform_data *pdata = client->dev.platform_data;
> > +	struct device *dev = &client->dev;
> >  	struct ad7746_chip_info *chip;
> >  	struct iio_dev *indio_dev;
> >  	unsigned char regval = 0;
> > +	unsigned int vdd_permille;
> >  	int ret = 0;
> >  
> >  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> > @@ -703,26 +702,39 @@ static int ad7746_probe(struct i2c_client *client,
> >  	indio_dev->num_channels = ARRAY_SIZE(ad7746_channels);
> >  	indio_dev->modes = INDIO_DIRECT_MODE;
> >  
> > -	if (pdata) {
> > -		if (pdata->exca_en) {
> > -			if (pdata->exca_inv_en)
> > -				regval |= AD7746_EXCSETUP_NEXCA;
> > -			else
> > -				regval |= AD7746_EXCSETUP_EXCA;
> > -		}
> > +	if (device_property_read_bool(dev, "adi,exca-output-en")) {
> > +		if (device_property_read_bool(dev, "adi,exca-output-invert"))
> > +			regval |= AD7746_EXCSETUP_NEXCA;
> > +		else
> > +			regval |= AD7746_EXCSETUP_EXCA;
> > +	}
> >  
> > -		if (pdata->excb_en) {
> > -			if (pdata->excb_inv_en)
> > -				regval |= AD7746_EXCSETUP_NEXCB;
> > -			else
> > -				regval |= AD7746_EXCSETUP_EXCB;
> > -		}
> > +	if (device_property_read_bool(dev, "adi,excb-output-en")) {
> > +		if (device_property_read_bool(dev, "adi,excb-output-invert"))
> > +			regval |= AD7746_EXCSETUP_NEXCB;
> > +		else
> > +			regval |= AD7746_EXCSETUP_EXCB;
> > +	}
> >  
> > -		regval |= AD7746_EXCSETUP_EXCLVL(pdata->exclvl);
> > -	} else {
> > -		dev_warn(&client->dev, "No platform data? using default\n");
> > -		regval = AD7746_EXCSETUP_EXCA | AD7746_EXCSETUP_EXCB |
> > -			AD7746_EXCSETUP_EXCLVL(3);
> > +	ret = device_property_read_u32(dev, "adi,excitation-vdd-permille",
> > +				       &vdd_permille);
> > +	if (!ret) {
> 
> This test is reversed.  I wonder if the static checkers can catch the
> uninitialized variable bug...  It's probably better to write it as:
> 
> 	if (device_property_read_u32(dev, "adi,excitation-vdd-permille",
> 				     &vdd_permille) {
> 
> So it matches the others.

Oops.  Sorry for the noise.  I was wrong on this.  I looked at the
device_property_read_bool() code instead of the device_property_read_u32().

It's disappointing that the returns are reversed.

regards,
dan carpenter
diff mbox series

Patch

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index dfd71e99e872..531f1b96dea2 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -18,8 +18,6 @@ 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-#include "ad7746.h"
-
 /*
  * AD7746 Register Definition
  */
@@ -676,10 +674,11 @@  static const struct iio_info ad7746_info = {
 static int ad7746_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
-	struct ad7746_platform_data *pdata = client->dev.platform_data;
+	struct device *dev = &client->dev;
 	struct ad7746_chip_info *chip;
 	struct iio_dev *indio_dev;
 	unsigned char regval = 0;
+	unsigned int vdd_permille;
 	int ret = 0;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
@@ -703,26 +702,39 @@  static int ad7746_probe(struct i2c_client *client,
 	indio_dev->num_channels = ARRAY_SIZE(ad7746_channels);
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	if (pdata) {
-		if (pdata->exca_en) {
-			if (pdata->exca_inv_en)
-				regval |= AD7746_EXCSETUP_NEXCA;
-			else
-				regval |= AD7746_EXCSETUP_EXCA;
-		}
+	if (device_property_read_bool(dev, "adi,exca-output-en")) {
+		if (device_property_read_bool(dev, "adi,exca-output-invert"))
+			regval |= AD7746_EXCSETUP_NEXCA;
+		else
+			regval |= AD7746_EXCSETUP_EXCA;
+	}
 
-		if (pdata->excb_en) {
-			if (pdata->excb_inv_en)
-				regval |= AD7746_EXCSETUP_NEXCB;
-			else
-				regval |= AD7746_EXCSETUP_EXCB;
-		}
+	if (device_property_read_bool(dev, "adi,excb-output-en")) {
+		if (device_property_read_bool(dev, "adi,excb-output-invert"))
+			regval |= AD7746_EXCSETUP_NEXCB;
+		else
+			regval |= AD7746_EXCSETUP_EXCB;
+	}
 
-		regval |= AD7746_EXCSETUP_EXCLVL(pdata->exclvl);
-	} else {
-		dev_warn(&client->dev, "No platform data? using default\n");
-		regval = AD7746_EXCSETUP_EXCA | AD7746_EXCSETUP_EXCB |
-			AD7746_EXCSETUP_EXCLVL(3);
+	ret = device_property_read_u32(dev, "adi,excitation-vdd-permille",
+				       &vdd_permille);
+	if (!ret) {
+		switch (vdd_permille) {
+		case 125:
+			regval |= AD7746_EXCSETUP_EXCLVL(0);
+			break;
+		case 250:
+			regval |= AD7746_EXCSETUP_EXCLVL(1);
+			break;
+		case 375:
+			regval |= AD7746_EXCSETUP_EXCLVL(2);
+			break;
+		case 500:
+			regval |= AD7746_EXCSETUP_EXCLVL(3);
+			break;
+		default:
+			break;
+		}
 	}
 
 	ret = i2c_smbus_write_byte_data(chip->client,
diff --git a/drivers/staging/iio/cdc/ad7746.h b/drivers/staging/iio/cdc/ad7746.h
deleted file mode 100644
index 8bdbd732dbbd..000000000000
--- a/drivers/staging/iio/cdc/ad7746.h
+++ /dev/null
@@ -1,28 +0,0 @@ 
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * AD7746 capacitive sensor driver supporting AD7745, AD7746 and AD7747
- *
- * Copyright 2011 Analog Devices Inc.
- */
-
-#ifndef IIO_CDC_AD7746_H_
-#define IIO_CDC_AD7746_H_
-
-/*
- * TODO: struct ad7746_platform_data needs to go into include/linux/iio
- */
-
-#define AD7466_EXCLVL_0		0 /* +-VDD/8 */
-#define AD7466_EXCLVL_1		1 /* +-VDD/4 */
-#define AD7466_EXCLVL_2		2 /* +-VDD * 3/8 */
-#define AD7466_EXCLVL_3		3 /* +-VDD/2 */
-
-struct ad7746_platform_data {
-	unsigned char exclvl;	/*Excitation Voltage Level */
-	bool exca_en;		/* enables EXCA pin as the excitation output */
-	bool exca_inv_en;	/* enables /EXCA pin as the excitation output */
-	bool excb_en;		/* enables EXCB pin as the excitation output */
-	bool excb_inv_en;	/* enables /EXCB pin as the excitation output */
-};
-
-#endif /* IIO_CDC_AD7746_H_ */