staging: iio: cdc: ad7746: add dt properties for capacitive channel setup
Subsystems (5)
v3 cover staging: iio: cdc: ad7746: add dt properties for capacitive channel setup
This patch series aims to replace the platform_struct for the ad7746 driver in favor of device tree bindings, creating the dt-binding documentation in the process.
Since the header file was only used to define the struct and the excitation level values, it was possible to remove the file entirely.
Changelog v2 -> v3:
- Add application reference note for the inverted EXCX dt bindings
- Replace macro with lookup table for setting the capacitive channel excitation voltage
Lucas Stankus (2):
dt-bindings: staging: iio: cdc: ad7746: add binding documentation for
AD7746
staging: iio: cdc: ad7746: use dt for capacitive channel setup.
.../bindings/iio/cdc/adi,ad7746.yaml | 77 +++++++++++++++++++ drivers/staging/iio/cdc/ad7746.c | 54 ++++++++----- drivers/staging/iio/cdc/ad7746.h | 28 ------- 3 files changed, 110 insertions(+), 49 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/cdc/adi,ad7746.yaml delete mode 100644 drivers/staging/iio/cdc/ad7746.h
Signature · 1 line
2.31.1
PATCH v3 1/2 dt-bindings: staging: iio: cdc: ad7746: add binding documentation for AD7746
Add device tree binding documentation for AD7746 cdc in YAML format.
--- .../bindings/iio/cdc/adi,ad7746.yaml | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/cdc/adi,ad7746.yaml
Documentation/devicetree/bindings/iio/cdc/adi,ad7746.yaml new file +77 -0
@@ -0,0 +1,77 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/cdc/adi,ad7746.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: AD7746 24-Bit Capacitance-to-Digital Converter with Temperature Sensor + +maintainers: + - Michael Hennerich <[email protected]> + +description: | + AD7746 24-Bit Capacitance-to-Digital Converter with Temperature Sensor + + Specifications about the part can be found at: + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7291.pdf + +properties: + compatible: + enum: + - adi,ad7745 + - adi,ad7746 + - adi,ad7747 + + reg: + maxItems: 1 + + adi,excitation-vdd-permille: + description: | + Set VDD per mille to be used as the excitation voltage. + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [125, 250, 375, 500] + + adi,exca-output-en: + description: Enables the EXCA pin as the excitation output. + type: boolean + + adi,exca-output-invert: + description: | + Inverts the excitation output in the EXCA pin. + Normally only one of the EXCX pins would be inverted, check the following + application notes for more details + https://www.analog.com/media/en/technical-documentation/application-notes/AN-1585.pdf + type: boolean + + adi,excb-output-en: + description: Enables the EXCB pin as the excitation output. + type: boolean + + adi,excb-output-invert: + description: Inverts the excitation output in the EXCB pin. + type: boolean + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + ad7746: cdc@48 { + compatible = "adi,ad7746"; + reg = <0x48>; + adi,excitation-vdd-permille = <125>; + + adi,exca-output-en; + adi,exca-output-invert; + adi,excb-output-en; + adi,excb-output-invert; + }; + }; +...
Signature · 1 line
2.31.1
PATCH v3 2/2 staging: iio: cdc: ad7746: use dt for capacitive channel setup.
In reply to Lucas StankusDitch 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.
--- 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
drivers/staging/iio/cdc/ad7746.c +33 -21
@@ -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,
drivers/staging/iio/cdc/ad7746.h deleted +0 -28
@@ -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_ */
Signature · 1 line
2.31.1
Jonathan Cameron
maintainerQuoted 12 lines · On Sat, 1 May 2021 09:32:03 -0300 Lucas Stankus <[email protected]>…
This patch series aims to replace the
platform_structfor the ad7746 driver in favor of device tree bindings, creating the dt-binding documentation in the process.Since the header file was only used to define the struct and the excitation level values, it was possible to remove the file entirely.
Changelog v2 -> v3:
- Add application reference note for the inverted EXCX dt bindings
- Replace macro with lookup table for setting the capacitive channel excitation voltage
Quoted 4 lines
Lucas Stankus (2):
dt-bindings: staging: iio: cdc: ad7746: add binding documentation forAD7746staging: iio: cdc: ad7746: use dt for capacitive channel setup.
Looks good to me. Thanks!
Will leave time for Rob or anyone else to take a look etc before applying.
If I seem to have lost it then feel free to give me a poke in a few weeks (it's happened a few times in the past :(
Thanks,
Jonathan
Quoted 8 lines
.../bindings/iio/cdc/adi,ad7746.yaml | 77 +++++++++++++++++++ drivers/staging/iio/cdc/ad7746.c | 54 ++++++++----- drivers/staging/iio/cdc/ad7746.h | 28 ------- 3 files changed, 110 insertions(+), 49 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/cdc/adi,ad7746.yaml delete mode 100644 drivers/staging/iio/cdc/ad7746.h
Re: [PATCH v3 2/2] staging: iio: cdc: ad7746: use dt for capacitive channel setup.
In reply to Lucas StankusQuoted 55 lines · Lucas Stankus
drivers/staging/iio/cdc/ad7746.c +13
-16@@ -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 ;
drivers/staging/iio/cdc/ad7746.c static int ad7746_probe(struct i2c_client *client, · line 721
Quoted 10 lines
drivers/staging/iio/cdc/ad7746.c +4
-5+ } - 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;
Quoted 19 lines
drivers/staging/iio/cdc/ad7746.c +16
-0+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,
Re: [PATCH v3 2/2] staging: iio: cdc: ad7746: use dt for capacitive channel setup.
Quoted 70 lines · Dan Carpenter
Quoted 55 lines · Lucas Stankus
drivers/staging/iio/cdc/ad7746.c +13
-16@@ -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 ;drivers/staging/iio/cdc/ad7746.c static int ad7746_probe(struct i2c_client *client, · line 721
Quoted 10 lines
drivers/staging/iio/cdc/ad7746.c +4
-5+ } - 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:
Quoted 4 lines
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
Re: [PATCH v3 1/2] dt-bindings: staging: iio: cdc: ad7746: add binding documentation for AD7746
In reply to Lucas StankusQuoted 8 lines · Lucas Stankus
Add device tree binding documentation for AD7746 cdc in YAML format.
Signed-off-by Lucas Stankus <[email protected]>--- .../bindings/iio/cdc/adi,ad7746.yaml | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/cdc/adi,ad7746.yaml
Quoted 23 lines · Jonathan Cameron
Quoted 12 lines · On Sat, 1 May 2021 09:32:03 -0300 Lucas Stankus <[email protected]>…
This patch series aims to replace the
platform_structfor the ad7746 driver in favor of device tree bindings, creating the dt-binding documentation in the process.Since the header file was only used to define the struct and the excitation level values, it was possible to remove the file entirely.
Changelog v2 -> v3:
- Add application reference note for the inverted EXCX dt bindings
- Replace macro with lookup table for setting the capacitive channel excitation voltageQuoted 4 lines
Lucas Stankus (2):
dt-bindings: staging: iio: cdc: ad7746: add binding documentation forAD7746staging: iio: cdc: ad7746: use dt for capacitive channel setup.
Looks good to me. Thanks!
Quoted 17 lines
Will leave time for Rob or anyone else to take a look etc before applying.
If I seem to have lost it then feel free to give me a poke in a few weeks (it's happened a few times in the past :(
Thanks,
Jonathan
Quoted 8 lines
.../bindings/iio/cdc/adi,ad7746.yaml | 77 +++++++++++++++++++ drivers/staging/iio/cdc/ad7746.c | 54 ++++++++----- drivers/staging/iio/cdc/ad7746.h | 28 ------- 3 files changed, 110 insertions(+), 49 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/cdc/adi,ad7746.yaml delete mode 100644 drivers/staging/iio/cdc/ad7746.h
Hey Jonathan,
It has been a week or so without any more feedback, so I'm just a pinging here to know if this can be applied.
Thanks,
Lucas
Jonathan Cameron
maintainerQuoted 42 lines · On Wed, 19 May 2021 17:59:31 -0300 Lucas Stankus <[email protected]>…
Quoted 23 lines · Jonathan Cameron
Quoted 12 lines · On Sat, 1 May 2021 09:32:03 -0300 Lucas Stankus <[email protected]>…
This patch series aims to replace the
platform_structfor the ad7746 driver in favor of device tree bindings, creating the dt-binding documentation in the process.Since the header file was only used to define the struct and the excitation level values, it was possible to remove the file entirely.
Changelog v2 -> v3:
- Add application reference note for the inverted EXCX dt bindings
- Replace macro with lookup table for setting the capacitive channel excitation voltageQuoted 4 lines
Lucas Stankus (2):
dt-bindings: staging: iio: cdc: ad7746: add binding documentation forAD7746staging: iio: cdc: ad7746: use dt for capacitive channel setup.
Looks good to me. Thanks!
Quoted 17 lines
Will leave time for Rob or anyone else to take a look etc before applying.
If I seem to have lost it then feel free to give me a poke in a few weeks (it's happened a few times in the past :(
Thanks,
Jonathan
Quoted 8 lines
.../bindings/iio/cdc/adi,ad7746.yaml | 77 +++++++++++++++++++ drivers/staging/iio/cdc/ad7746.c | 54 ++++++++----- drivers/staging/iio/cdc/ad7746.h | 28 ------- 3 files changed, 110 insertions(+), 49 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/cdc/adi,ad7746.yaml delete mode 100644 drivers/staging/iio/cdc/ad7746.h
Quoted 4 lines
Hey Jonathan,
It has been a week or so without any more feedback, so I'm just a pinging here to know if this can be applied.
oops. Indeed time I picked these up. Somehow I had them in the wrong colour in my horrendous colour based tracking system :(
Now applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to do their worst.
Thanks,
Jonathan
Quoted 3 lines
Thanks,
Lucas