linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] staging: iio: adc: ad7192: add device-tree support to driver
@ 2018-01-05 15:21 alexandru.ardelean
  2018-01-05 15:21 ` [PATCH 2/2] staging:iio:docs: add ad7291 doc to detail dt usage alexandru.ardelean
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: alexandru.ardelean @ 2018-01-05 15:21 UTC (permalink / raw)
  To: linux-iio; +Cc: Alexandru Ardelean

From: Alexandru Ardelean <alexandru.ardelean@analog.com>

Admittedly, this will always use up to
sizeof(struct ad7192_platform_data) bytes (~12 bytes at most)
on the stack, during the probing of the device.

But this will maintain most of the other
init code unchanged.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/staging/iio/adc/ad7192.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index cadfb96734ed..fca988330588 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -610,18 +610,41 @@ static const struct iio_chan_spec ad7193_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(14),
 };
 
+static void ad7192_parse_dt(struct device_node *np,
+			    struct ad7192_platform_data *pdata)
+{
+	of_property_read_u16(np, "adi,reference-voltage-mv", &pdata->vref_mv);
+	of_property_read_u8(np, "adi,clock-source-select", &pdata->clock_source_sel);
+	of_property_read_u32(np, "adi,external-clock-Hz", &pdata->ext_clk_hz);
+	pdata->refin2_en = of_property_read_bool(np, "adi,refin2-pins-enable");
+	pdata->rej60_en = of_property_read_bool(np, "adi,rejection-60-Hz-enable");
+	pdata->sinc3_en = of_property_read_bool(np, "adi,sinc3-filter-enable");
+	pdata->chop_en = of_property_read_bool(np, "adi,chop-enable");
+	pdata->buf_en = of_property_read_bool(np, "adi,buffer-enable");
+	pdata->unipolar_en = of_property_read_bool(np, "adi,unipolar-enable");
+	pdata->burnout_curr_en = of_property_read_bool(np, "adi,burnout-currents-enable");
+}
+
 static int ad7192_probe(struct spi_device *spi)
 {
 	const struct ad7192_platform_data *pdata = dev_get_platdata(&spi->dev);
+	const struct device_node *of_node = dev_of_node(&spi->dev);
+	struct ad7192_platform_data lpdata;
 	struct ad7192_state *st;
 	struct iio_dev *indio_dev;
 	int ret, voltage_uv = 0;
 
-	if (!pdata) {
-		dev_err(&spi->dev, "no platform data?\n");
+	if (!pdata && !of_node) {
+		dev_err(&spi->dev, "no platform data or device tree configuration?\n");
 		return -ENODEV;
 	}
 
+	if (!pdata) {
+		memset(&lpdata, 0, sizeof(lpdata));
+		pdata = &lpdata;
+		ad7192_parse_dt(of_node, pdata);
+	}
+
 	if (!spi->irq) {
 		dev_err(&spi->dev, "no IRQ?\n");
 		return -ENODEV;
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] staging:iio:docs: add ad7291 doc to detail dt usage
  2018-01-05 15:21 [PATCH 1/2] staging: iio: adc: ad7192: add device-tree support to driver alexandru.ardelean
@ 2018-01-05 15:21 ` alexandru.ardelean
  2018-01-05 15:25   ` Ardelean, Alexandru
  2018-01-05 15:40 ` [PATCH 1/2] staging: iio: adc: ad7192: add device-tree support to driver Ardelean, Alexandru
  2018-01-06 12:58 ` Jonathan Cameron
  2 siblings, 1 reply; 7+ messages in thread
From: alexandru.ardelean @ 2018-01-05 15:21 UTC (permalink / raw)
  To: linux-iio; +Cc: Alexandru Ardelean

From: Alexandru Ardelean <alexandru.ardelean@analog.com>

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/staging/iio/Documentation/adc/ad7192.txt | 60 ++++++++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 drivers/staging/iio/Documentation/adc/ad7192.txt

diff --git a/drivers/staging/iio/Documentation/adc/ad7192.txt b/drivers/staging/iio/Documentation/adc/ad7192.txt
new file mode 100644
index 000000000000..19efc0a9411c
--- /dev/null
+++ b/drivers/staging/iio/Documentation/adc/ad7192.txt
@@ -0,0 +1,60 @@
+Analog Devices AD719x ADC Driver
+
+1. Overview
+
+The driver is intended to work with all AD719x ADC chips
+from Analog Devices (AD7190, AD7192, AD7193, AD7195).
+
+It's based on top of the AD Sigma Delta driver
+
+2. Device Tree Configuration
+
+Example for use on a Raspberry Pi with interrupt line
+connected to PIN 25 on the GPIO:
+
+	adc_vref: fixedregulator@0 {
+		reg = <0>;
+
+		compatible = "regulator-fixed";
+		regulator-name = "fixed-supply";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+	};
+
+	ad7190@0 {
+		compatible = "ad7190";
+		reg = <0>;
+		spi-max-frequency = <1000000>;
+		spi-cpol;
+		spi-cpha;
+
+		#interrupt-cells = <2>;
+		interrupts = <25 0x2>;
+		interrupt-parent = <&gpio>;
+		avdd-supply = <&adc_vref>;
+
+		adi,clock-source-select = /bits/ 8 <0>;
+		adi,reference-voltage-mv = /bits/ 16 <3300>;
+
+		adi,refin2-pins-enable;
+		adi,rejection-60-Hz-enable;
+		adi,chop-enable;
+		adi,buffer-enable;
+		adi,burnout-currents-enable;
+		adi,sinc3-filter-enable;
+		adi,unipolar-enable;
+	};
+
+Notes (about this example):
+* be sure to replace `compatible = "ad7190";` with
+  your actual chip model (ad7190, ad7192, ad7193, ad7195)
+  in order to make sure it works correctly
+* PIN 25 must also be connected also to the DOUT pin
+  of the ADC (or MISO on the host)
+* be sure to add the "/bits/" specifiers ; newer dtc versions
+  specify the property type/size using this field,
+  and the driver may not be able to find the property
+* not all "adi,xxx" parameters are required ;
+  see the datasheet to get an idea of what you need
+
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* RE: [PATCH 2/2] staging:iio:docs: add ad7291 doc to detail dt usage
  2018-01-05 15:21 ` [PATCH 2/2] staging:iio:docs: add ad7291 doc to detail dt usage alexandru.ardelean
@ 2018-01-05 15:25   ` Ardelean, Alexandru
  0 siblings, 0 replies; 7+ messages in thread
From: Ardelean, Alexandru @ 2018-01-05 15:25 UTC (permalink / raw)
  To: linux-iio@vger.kernel.org

From: alexandru.ardelean@analog.com [alexandru.ardelean@analog.com]=0A=
Sent: Friday, January 05, 2018 5:21 PM=0A=
To: linux-iio@vger.kernel.org=0A=
Cc: Ardelean, Alexandru=0A=
Subject: [PATCH 2/2] staging:iio:docs: add ad7291 doc to detail dt usage=0A=
=0A=
urgs... there's a typo in the title=0A=
=0A=
From: Alexandru Ardelean <alexandru.ardelean@analog.com>=0A=
=0A=
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>=0A=
---=0A=
 drivers/staging/iio/Documentation/adc/ad7192.txt | 60 ++++++++++++++++++++=
++++=0A=
 1 file changed, 60 insertions(+)=0A=
 create mode 100644 drivers/staging/iio/Documentation/adc/ad7192.txt=0A=
=0A=
diff --git a/drivers/staging/iio/Documentation/adc/ad7192.txt b/drivers/sta=
ging/iio/Documentation/adc/ad7192.txt=0A=
new file mode 100644=0A=
index 000000000000..19efc0a9411c=0A=
--- /dev/null=0A=
+++ b/drivers/staging/iio/Documentation/adc/ad7192.txt=0A=
@@ -0,0 +1,60 @@=0A=
+Analog Devices AD719x ADC Driver=0A=
+=0A=
+1. Overview=0A=
+=0A=
+The driver is intended to work with all AD719x ADC chips=0A=
+from Analog Devices (AD7190, AD7192, AD7193, AD7195).=0A=
+=0A=
+It's based on top of the AD Sigma Delta driver=0A=
+=0A=
+2. Device Tree Configuration=0A=
+=0A=
+Example for use on a Raspberry Pi with interrupt line=0A=
+connected to PIN 25 on the GPIO:=0A=
+=0A=
+       adc_vref: fixedregulator@0 {=0A=
+               reg =3D <0>;=0A=
+=0A=
+               compatible =3D "regulator-fixed";=0A=
+               regulator-name =3D "fixed-supply";=0A=
+               regulator-min-microvolt =3D <3300000>;=0A=
+               regulator-max-microvolt =3D <3300000>;=0A=
+               regulator-boot-on;=0A=
+       };=0A=
+=0A=
+       ad7190@0 {=0A=
+               compatible =3D "ad7190";=0A=
+               reg =3D <0>;=0A=
+               spi-max-frequency =3D <1000000>;=0A=
+               spi-cpol;=0A=
+               spi-cpha;=0A=
+=0A=
+               #interrupt-cells =3D <2>;=0A=
+               interrupts =3D <25 0x2>;=0A=
+               interrupt-parent =3D <&gpio>;=0A=
+               avdd-supply =3D <&adc_vref>;=0A=
+=0A=
+               adi,clock-source-select =3D /bits/ 8 <0>;=0A=
+               adi,reference-voltage-mv =3D /bits/ 16 <3300>;=0A=
+=0A=
+               adi,refin2-pins-enable;=0A=
+               adi,rejection-60-Hz-enable;=0A=
+               adi,chop-enable;=0A=
+               adi,buffer-enable;=0A=
+               adi,burnout-currents-enable;=0A=
+               adi,sinc3-filter-enable;=0A=
+               adi,unipolar-enable;=0A=
+       };=0A=
+=0A=
+Notes (about this example):=0A=
+* be sure to replace `compatible =3D "ad7190";` with=0A=
+  your actual chip model (ad7190, ad7192, ad7193, ad7195)=0A=
+  in order to make sure it works correctly=0A=
+* PIN 25 must also be connected also to the DOUT pin=0A=
+  of the ADC (or MISO on the host)=0A=
+* be sure to add the "/bits/" specifiers ; newer dtc versions=0A=
+  specify the property type/size using this field,=0A=
+  and the driver may not be able to find the property=0A=
+* not all "adi,xxx" parameters are required ;=0A=
+  see the datasheet to get an idea of what you need=0A=
+=0A=
--=0A=
2.14.1=0A=
=0A=

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 1/2] staging: iio: adc: ad7192: add device-tree support to driver
  2018-01-05 15:21 [PATCH 1/2] staging: iio: adc: ad7192: add device-tree support to driver alexandru.ardelean
  2018-01-05 15:21 ` [PATCH 2/2] staging:iio:docs: add ad7291 doc to detail dt usage alexandru.ardelean
@ 2018-01-05 15:40 ` Ardelean, Alexandru
  2018-01-06 12:58 ` Jonathan Cameron
  2 siblings, 0 replies; 7+ messages in thread
From: Ardelean, Alexandru @ 2018-01-05 15:40 UTC (permalink / raw)
  To: linux-iio@vger.kernel.org

From: alexandru.ardelean@analog.com [alexandru.ardelean@analog.com]=0A=
Sent: Friday, January 05, 2018 5:21 PM=0A=
To: linux-iio@vger.kernel.org=0A=
Cc: Ardelean, Alexandru=0A=
Subject: [PATCH 1/2] staging: iio: adc: ad7192: add device-tree support to =
driver=0A=
=0A=
From: Alexandru Ardelean <alexandru.ardelean@analog.com>=0A=
=0A=
Admittedly, this will always use up to=0A=
sizeof(struct ad7192_platform_data) bytes (~12 bytes at most)=0A=
on the stack, during the probing of the device.=0A=
=0A=
But this will maintain most of the other=0A=
init code unchanged.=0A=
=0A=
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>=0A=
---=0A=
 drivers/staging/iio/adc/ad7192.c | 27 +++++++++++++++++++++++++--=0A=
 1 file changed, 25 insertions(+), 2 deletions(-)=0A=
=0A=
diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7=
192.c=0A=
index cadfb96734ed..fca988330588 100644=0A=
--- a/drivers/staging/iio/adc/ad7192.c=0A=
+++ b/drivers/staging/iio/adc/ad7192.c=0A=
@@ -610,18 +610,41 @@ static const struct iio_chan_spec ad7193_channels[] =
=3D {=0A=
        IIO_CHAN_SOFT_TIMESTAMP(14),=0A=
 };=0A=
=0A=
+static void ad7192_parse_dt(struct device_node *np,=0A=
+                           struct ad7192_platform_data *pdata)=0A=
+{=0A=
+       of_property_read_u16(np, "adi,reference-voltage-mv", &pdata->vref_m=
v);=0A=
+       of_property_read_u8(np, "adi,clock-source-select", &pdata->clock_so=
urce_sel);=0A=
+       of_property_read_u32(np, "adi,external-clock-Hz", &pdata->ext_clk_h=
z);=0A=
+       pdata->refin2_en =3D of_property_read_bool(np, "adi,refin2-pins-ena=
ble");=0A=
+       pdata->rej60_en =3D of_property_read_bool(np, "adi,rejection-60-Hz-=
enable");=0A=
+       pdata->sinc3_en =3D of_property_read_bool(np, "adi,sinc3-filter-ena=
ble");=0A=
+       pdata->chop_en =3D of_property_read_bool(np, "adi,chop-enable");=0A=
+       pdata->buf_en =3D of_property_read_bool(np, "adi,buffer-enable");=
=0A=
+       pdata->unipolar_en =3D of_property_read_bool(np, "adi,unipolar-enab=
le");=0A=
+       pdata->burnout_curr_en =3D of_property_read_bool(np, "adi,burnout-c=
urrents-enable");=0A=
+}=0A=
+=0A=
 static int ad7192_probe(struct spi_device *spi)=0A=
 {=0A=
        const struct ad7192_platform_data *pdata =3D dev_get_platdata(&spi-=
>dev);=0A=
+       const struct device_node *of_node =3D dev_of_node(&spi->dev);=0A=
+       struct ad7192_platform_data lpdata;=0A=
        struct ad7192_state *st;=0A=
        struct iio_dev *indio_dev;=0A=
        int ret, voltage_uv =3D 0;=0A=
=0A=
-       if (!pdata) {=0A=
-               dev_err(&spi->dev, "no platform data?\n");=0A=
+       if (!pdata && !of_node) {=0A=
+               dev_err(&spi->dev, "no platform data or device tree configu=
ration?\n");=0A=
                return -ENODEV;=0A=
        }=0A=
=0A=
+       if (!pdata) {=0A=
+               memset(&lpdata, 0, sizeof(lpdata));=0A=
+               pdata =3D &lpdata;=0A=
+               ad7192_parse_dt(of_node, pdata);=0A=
=0A=
weird how my compiler did not catch this const qualifier disregard....=0A=
though, this did work on an ARM64 [Raspberry  Pi]=0A=
i guess i'll have to re-spin this as well=0A=
=0A=
+       }=0A=
+=0A=
        if (!spi->irq) {=0A=
                dev_err(&spi->dev, "no IRQ?\n");=0A=
                return -ENODEV;=0A=
--=0A=
2.14.1=0A=
=0A=

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] staging: iio: adc: ad7192: add device-tree support to driver
  2018-01-05 15:21 [PATCH 1/2] staging: iio: adc: ad7192: add device-tree support to driver alexandru.ardelean
  2018-01-05 15:21 ` [PATCH 2/2] staging:iio:docs: add ad7291 doc to detail dt usage alexandru.ardelean
  2018-01-05 15:40 ` [PATCH 1/2] staging: iio: adc: ad7192: add device-tree support to driver Ardelean, Alexandru
@ 2018-01-06 12:58 ` Jonathan Cameron
  2018-01-08  7:51   ` Ardelean, Alexandru
  2 siblings, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2018-01-06 12:58 UTC (permalink / raw)
  To: alexandru.ardelean; +Cc: linux-iio

On Fri, 5 Jan 2018 17:21:32 +0200
<alexandru.ardelean@analog.com> wrote:

> From: Alexandru Ardelean <alexandru.ardelean@analog.com>
> 
Say what the patch does before going into details of why
you have done it the way you have.

> Admittedly, this will always use up to
> sizeof(struct ad7192_platform_data) bytes (~12 bytes at most)
> on the stack, during the probing of the device.

> 
> But this will maintain most of the other
> init code unchanged.
> 

I'm hopeful that you are going to also do any remaining work necessary
to get this out of staging once and for all?

Anyhow, comments inline.
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
>  drivers/staging/iio/adc/ad7192.c | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> index cadfb96734ed..fca988330588 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -610,18 +610,41 @@ static const struct iio_chan_spec ad7193_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(14),
>  };
>  
> +static void ad7192_parse_dt(struct device_node *np,
> +			    struct ad7192_platform_data *pdata)
> +{
> +	of_property_read_u16(np, "adi,reference-voltage-mv", &pdata->vref_mv);
> +	of_property_read_u8(np, "adi,clock-source-select", &pdata->clock_source_sel);
> +	of_property_read_u32(np, "adi,external-clock-Hz", &pdata->ext_clk_hz);
> +	pdata->refin2_en = of_property_read_bool(np, "adi,refin2-pins-enable");
> +	pdata->rej60_en = of_property_read_bool(np, "adi,rejection-60-Hz-enable");
> +	pdata->sinc3_en = of_property_read_bool(np, "adi,sinc3-filter-enable");
> +	pdata->chop_en = of_property_read_bool(np, "adi,chop-enable");
> +	pdata->buf_en = of_property_read_bool(np, "adi,buffer-enable");
> +	pdata->unipolar_en = of_property_read_bool(np, "adi,unipolar-enable");
> +	pdata->burnout_curr_en = of_property_read_bool(np, "adi,burnout-currents-enable");

Error checking?  No particular reason a devicetree must be right and have all of
these.

> +}
> +
>  static int ad7192_probe(struct spi_device *spi)
>  {
>  	const struct ad7192_platform_data *pdata = dev_get_platdata(&spi->dev);
> +	const struct device_node *of_node = dev_of_node(&spi->dev);
> +	struct ad7192_platform_data lpdata;
>  	struct ad7192_state *st;
>  	struct iio_dev *indio_dev;
>  	int ret, voltage_uv = 0;
>  
> -	if (!pdata) {
> -		dev_err(&spi->dev, "no platform data?\n");
> +	if (!pdata && !of_node) {
> +		dev_err(&spi->dev, "no platform data or device tree configuration?\n");
>  		return -ENODEV;
>  	}
>  
> +	if (!pdata) {
> +		memset(&lpdata, 0, sizeof(lpdata));
> +		pdata = &lpdata;
> +		ad7192_parse_dt(of_node, pdata);
> +	}
> +
>  	if (!spi->irq) {
>  		dev_err(&spi->dev, "no IRQ?\n");
>  		return -ENODEV;


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 1/2] staging: iio: adc: ad7192: add device-tree support to driver
  2018-01-06 12:58 ` Jonathan Cameron
@ 2018-01-08  7:51   ` Ardelean, Alexandru
  2018-01-14 10:53     ` Jonathan Cameron
  0 siblings, 1 reply; 7+ messages in thread
From: Ardelean, Alexandru @ 2018-01-08  7:51 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio@vger.kernel.org

=0A=
________________________________________=0A=
From: Jonathan Cameron [jic23@jic23.retrosnub.co.uk]=0A=
Sent: Saturday, January 06, 2018 2:58 PM=0A=
To: Ardelean, Alexandru=0A=
Cc: linux-iio@vger.kernel.org=0A=
Subject: Re: [PATCH 1/2] staging: iio: adc: ad7192: add device-tree support=
 to driver=0A=
=0A=
On Fri, 5 Jan 2018 17:21:32 +0200=0A=
<alexandru.ardelean@analog.com> wrote:=0A=
=0A=
> From: Alexandru Ardelean <alexandru.ardelean@analog.com>=0A=
>=0A=
Say what the patch does before going into details of why=0A=
you have done it the way you have.=0A=
=0A=
Ack=0A=
=0A=
> Admittedly, this will always use up to=0A=
> sizeof(struct ad7192_platform_data) bytes (~12 bytes at most)=0A=
> on the stack, during the probing of the device.=0A=
=0A=
>=0A=
> But this will maintain most of the other=0A=
> init code unchanged.=0A=
>=0A=
=0A=
I'm hopeful that you are going to also do any remaining work necessary=0A=
to get this out of staging once and for all?=0A=
=0A=
Yep.=0A=
Once I get the gist of sending patches to the kernel.=0A=
So far, I've only sent to other smaller projects ; I'll make an effort to l=
earn the rules here.=0A=
=0A=
Anyhow, comments inline.=0A=
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>=0A=
> ---=0A=
>  drivers/staging/iio/adc/ad7192.c | 27 +++++++++++++++++++++++++--=0A=
>  1 file changed, 25 insertions(+), 2 deletions(-)=0A=
>=0A=
> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/a=
d7192.c=0A=
> index cadfb96734ed..fca988330588 100644=0A=
> --- a/drivers/staging/iio/adc/ad7192.c=0A=
> +++ b/drivers/staging/iio/adc/ad7192.c=0A=
> @@ -610,18 +610,41 @@ static const struct iio_chan_spec ad7193_channels[]=
 =3D {=0A=
>       IIO_CHAN_SOFT_TIMESTAMP(14),=0A=
>  };=0A=
>=0A=
> +static void ad7192_parse_dt(struct device_node *np,=0A=
> +                         struct ad7192_platform_data *pdata)=0A=
> +{=0A=
> +     of_property_read_u16(np, "adi,reference-voltage-mv", &pdata->vref_m=
v);=0A=
> +     of_property_read_u8(np, "adi,clock-source-select", &pdata->clock_so=
urce_sel);=0A=
> +     of_property_read_u32(np, "adi,external-clock-Hz", &pdata->ext_clk_h=
z);=0A=
> +     pdata->refin2_en =3D of_property_read_bool(np, "adi,refin2-pins-ena=
ble");=0A=
> +     pdata->rej60_en =3D of_property_read_bool(np, "adi,rejection-60-Hz-=
enable");=0A=
> +     pdata->sinc3_en =3D of_property_read_bool(np, "adi,sinc3-filter-ena=
ble");=0A=
> +     pdata->chop_en =3D of_property_read_bool(np, "adi,chop-enable");=0A=
> +     pdata->buf_en =3D of_property_read_bool(np, "adi,buffer-enable");=
=0A=
> +     pdata->unipolar_en =3D of_property_read_bool(np, "adi,unipolar-enab=
le");=0A=
> +     pdata->burnout_curr_en =3D of_property_read_bool(np, "adi,burnout-c=
urrents-enable");=0A=
=0A=
Error checking?  No particular reason a devicetree must be right and have a=
ll of=0A=
these.=0A=
=0A=
Ack.=0A=
=0A=
> +}=0A=
> +=0A=
>  static int ad7192_probe(struct spi_device *spi)=0A=
>  {=0A=
>       const struct ad7192_platform_data *pdata =3D dev_get_platdata(&spi-=
>dev);=0A=
> +     const struct device_node *of_node =3D dev_of_node(&spi->dev);=0A=
> +     struct ad7192_platform_data lpdata;=0A=
>       struct ad7192_state *st;=0A=
>       struct iio_dev *indio_dev;=0A=
>       int ret, voltage_uv =3D 0;=0A=
>=0A=
> -     if (!pdata) {=0A=
> -             dev_err(&spi->dev, "no platform data?\n");=0A=
> +     if (!pdata && !of_node) {=0A=
> +             dev_err(&spi->dev, "no platform data or device tree configu=
ration?\n");=0A=
>               return -ENODEV;=0A=
>       }=0A=
>=0A=
> +     if (!pdata) {=0A=
> +             memset(&lpdata, 0, sizeof(lpdata));=0A=
> +             pdata =3D &lpdata;=0A=
> +             ad7192_parse_dt(of_node, pdata);=0A=
> +     }=0A=
> +=0A=
>       if (!spi->irq) {=0A=
>               dev_err(&spi->dev, "no IRQ?\n");=0A=
>               return -ENODEV;=0A=
=0A=
=0A=

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] staging: iio: adc: ad7192: add device-tree support to driver
  2018-01-08  7:51   ` Ardelean, Alexandru
@ 2018-01-14 10:53     ` Jonathan Cameron
  0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2018-01-14 10:53 UTC (permalink / raw)
  To: Ardelean, Alexandru; +Cc: linux-iio@vger.kernel.org

On Mon, 8 Jan 2018 07:51:08 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:

> ________________________________________
> From: Jonathan Cameron [jic23@jic23.retrosnub.co.uk]
> Sent: Saturday, January 06, 2018 2:58 PM
> To: Ardelean, Alexandru
> Cc: linux-iio@vger.kernel.org
> Subject: Re: [PATCH 1/2] staging: iio: adc: ad7192: add device-tree support to driver
> 
> On Fri, 5 Jan 2018 17:21:32 +0200
> <alexandru.ardelean@analog.com> wrote:
> 
> > From: Alexandru Ardelean <alexandru.ardelean@analog.com>
> >  
> Say what the patch does before going into details of why
> you have done it the way you have.
> 
> Ack
One more thing, if you can find an email client (or configure current one)
to keep to indenting quotes with >

Makes it easier to tell what you wrote!

Jonathan
> 
> > Admittedly, this will always use up to
> > sizeof(struct ad7192_platform_data) bytes (~12 bytes at most)
> > on the stack, during the probing of the device.  
> 
> >
> > But this will maintain most of the other
> > init code unchanged.
> >  
> 
> I'm hopeful that you are going to also do any remaining work necessary
> to get this out of staging once and for all?
> 
> Yep.
> Once I get the gist of sending patches to the kernel.
> So far, I've only sent to other smaller projects ; I'll make an effort to learn the rules here.
> 
> Anyhow, comments inline.
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> >  drivers/staging/iio/adc/ad7192.c | 27 +++++++++++++++++++++++++--
> >  1 file changed, 25 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> > index cadfb96734ed..fca988330588 100644
> > --- a/drivers/staging/iio/adc/ad7192.c
> > +++ b/drivers/staging/iio/adc/ad7192.c
> > @@ -610,18 +610,41 @@ static const struct iio_chan_spec ad7193_channels[] = {
> >       IIO_CHAN_SOFT_TIMESTAMP(14),
> >  };
> >
> > +static void ad7192_parse_dt(struct device_node *np,
> > +                         struct ad7192_platform_data *pdata)
> > +{
> > +     of_property_read_u16(np, "adi,reference-voltage-mv", &pdata->vref_mv);
> > +     of_property_read_u8(np, "adi,clock-source-select", &pdata->clock_source_sel);
> > +     of_property_read_u32(np, "adi,external-clock-Hz", &pdata->ext_clk_hz);
> > +     pdata->refin2_en = of_property_read_bool(np, "adi,refin2-pins-enable");
> > +     pdata->rej60_en = of_property_read_bool(np, "adi,rejection-60-Hz-enable");
> > +     pdata->sinc3_en = of_property_read_bool(np, "adi,sinc3-filter-enable");
> > +     pdata->chop_en = of_property_read_bool(np, "adi,chop-enable");
> > +     pdata->buf_en = of_property_read_bool(np, "adi,buffer-enable");
> > +     pdata->unipolar_en = of_property_read_bool(np, "adi,unipolar-enable");
> > +     pdata->burnout_curr_en = of_property_read_bool(np, "adi,burnout-currents-enable");  
> 
> Error checking?  No particular reason a devicetree must be right and have all of
> these.
> 
> Ack.
> 
> > +}
> > +
> >  static int ad7192_probe(struct spi_device *spi)
> >  {
> >       const struct ad7192_platform_data *pdata = dev_get_platdata(&spi->dev);
> > +     const struct device_node *of_node = dev_of_node(&spi->dev);
> > +     struct ad7192_platform_data lpdata;
> >       struct ad7192_state *st;
> >       struct iio_dev *indio_dev;
> >       int ret, voltage_uv = 0;
> >
> > -     if (!pdata) {
> > -             dev_err(&spi->dev, "no platform data?\n");
> > +     if (!pdata && !of_node) {
> > +             dev_err(&spi->dev, "no platform data or device tree configuration?\n");
> >               return -ENODEV;
> >       }
> >
> > +     if (!pdata) {
> > +             memset(&lpdata, 0, sizeof(lpdata));
> > +             pdata = &lpdata;
> > +             ad7192_parse_dt(of_node, pdata);
> > +     }
> > +
> >       if (!spi->irq) {
> >               dev_err(&spi->dev, "no IRQ?\n");
> >               return -ENODEV;  
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-01-14 10:53 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-05 15:21 [PATCH 1/2] staging: iio: adc: ad7192: add device-tree support to driver alexandru.ardelean
2018-01-05 15:21 ` [PATCH 2/2] staging:iio:docs: add ad7291 doc to detail dt usage alexandru.ardelean
2018-01-05 15:25   ` Ardelean, Alexandru
2018-01-05 15:40 ` [PATCH 1/2] staging: iio: adc: ad7192: add device-tree support to driver Ardelean, Alexandru
2018-01-06 12:58 ` Jonathan Cameron
2018-01-08  7:51   ` Ardelean, Alexandru
2018-01-14 10:53     ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).