[PATCH 0/3] ti,ads7950 device tree bindings

[PATCH 0/3] ti,ads7950 device tree bindings

[David Lechner]

This series adds device tree bindings for the TI ADS7950 family of A/DC chips.
The series includes the bindings documentation and some fixes to the iio driver
to make it work with the device tree bindings.

FYI, the ads7950 driver has not made it into mainline yet, so no worries about
breaking anyone with these changes.

David Lechner (3):
  DT/bindings: Add bindings for TI ADS7950 A/DC chips
  iio: adc: ti-ads7950: Drop "ti-" prefix from module name
  iio: adc: ti-ads7950: Change regulator matching string to "vref"

 .../devicetree/bindings/iio/adc/ti-ads7950.txt     | 23 ++++++++++++++++
 drivers/iio/adc/ti-ads7950.c                       | 32 +++++++++++-----------
 2 files changed, 39 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt

-- 
2.7.4

[PATCH 1/3] DT/bindings: Add bindings for TI ADS7950 A/DC chips

[David Lechner]

This adds device tree bindings for the TI ADS7950 family of A/DC chips.

Signed-off-by: David Lechner <david@lechnology.com>
---
 .../devicetree/bindings/iio/adc/ti-ads7950.txt     | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt

diff --git a/Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt b/Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt
new file mode 100644
index 0000000..e77a6f7
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt
@@ -0,0 +1,23 @@
+* Texas Instruments ADS7950 family of A/DC chips
+
+Required properties:
+ - compatible: Must be one of "ti,ads7950", "ti,ads7951", "ti,ads7952",
+   "ti,ads7953", "ti,ads7954", "ti,ads7955", "ti,ads7956", "ti,ads7957",
+   "ti,ads7958", "ti,ads7959", "ti,ads7960", or "ti,ads7961"
+ - reg: SPI chip select number for the device
+ - #io-channel-cells: Must be 1 as per ../iio-bindings.txt
+ - vref-supply: phandle to a regulator node that supplies the 2.5V or 5V
+   reference voltage
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+		Documentation/devicetree/bindings/spi/spi-bus.txt
+
+Example:
+adc@0 {
+	compatible = "ti,ads7957";
+	reg = <0>;
+	#io-channel-cells = <1>;
+	vref-supply = <&refin_supply>;
+	spi-max-frequency = <10000000>;
+};
-- 
2.7.4

[PATCH 2/3] iio: adc: ti-ads7950: Drop "ti-" prefix from module name

[David Lechner]

This drops the "ti-" prefix from the module name. It makes the module name
consistent with other iio ti-ads* drivers and it makes the driver work
with device tree (the spi subsystem drops the "ti," prefix when matching
compatible strings from device tree).

Tested working on LEGO MINDSTORMS EV3 with the following device tree node:

	adc@3 {
		compatible = "ti,ads7957";
		reg = <3>;
		#io-channel-cells = <1>;
		spi-max-frequency = <10000000>;
		vref-supply = <&adc_ref>;
	};

Signed-off-by: David Lechner <david@lechnology.com>
---
 drivers/iio/adc/ti-ads7950.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
index 0330361..b587fa6 100644
--- a/drivers/iio/adc/ti-ads7950.c
+++ b/drivers/iio/adc/ti-ads7950.c
@@ -459,25 +459,25 @@ static int ti_ads7950_remove(struct spi_device *spi)
 }
 
 static const struct spi_device_id ti_ads7950_id[] = {
-	{"ti-ads7950", TI_ADS7950},
-	{"ti-ads7951", TI_ADS7951},
-	{"ti-ads7952", TI_ADS7952},
-	{"ti-ads7953", TI_ADS7953},
-	{"ti-ads7954", TI_ADS7954},
-	{"ti-ads7955", TI_ADS7955},
-	{"ti-ads7956", TI_ADS7956},
-	{"ti-ads7957", TI_ADS7957},
-	{"ti-ads7958", TI_ADS7958},
-	{"ti-ads7959", TI_ADS7959},
-	{"ti-ads7960", TI_ADS7960},
-	{"ti-ads7961", TI_ADS7961},
+	{ "ads7950", TI_ADS7950 },
+	{ "ads7951", TI_ADS7951 },
+	{ "ads7952", TI_ADS7952 },
+	{ "ads7953", TI_ADS7953 },
+	{ "ads7954", TI_ADS7954 },
+	{ "ads7955", TI_ADS7955 },
+	{ "ads7956", TI_ADS7956 },
+	{ "ads7957", TI_ADS7957 },
+	{ "ads7958", TI_ADS7958 },
+	{ "ads7959", TI_ADS7959 },
+	{ "ads7960", TI_ADS7960 },
+	{ "ads7961", TI_ADS7961 },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, ti_ads7950_id);
 
 static struct spi_driver ti_ads7950_driver = {
 	.driver = {
-		.name	= "ti-ads7950",
+		.name	= "ads7950",
 	},
 	.probe		= ti_ads7950_probe,
 	.remove		= ti_ads7950_remove,
-- 
2.7.4

[PATCH 3/3] iio: adc: ti-ads7950: Change regulator matching string to "vref"

[David Lechner]

This changes the reference voltage regulator matching string from "refin"
to "vref". This is to be consistent with other A/DC chips that also use
"vref-supply" in their device tree bindings.

Signed-off-by: David Lechner <david@lechnology.com>
---
 drivers/iio/adc/ti-ads7950.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
index b587fa6..16a0663 100644
--- a/drivers/iio/adc/ti-ads7950.c
+++ b/drivers/iio/adc/ti-ads7950.c
@@ -411,15 +411,15 @@ static int ti_ads7950_probe(struct spi_device *spi)
 	spi_message_init_with_transfers(&st->scan_single_msg,
 					st->scan_single_xfer, 3);
 
-	st->reg = devm_regulator_get(&spi->dev, "refin");
+	st->reg = devm_regulator_get(&spi->dev, "vref");
 	if (IS_ERR(st->reg)) {
-		dev_err(&spi->dev, "Failed get get regulator \"refin\"\n");
+		dev_err(&spi->dev, "Failed get get regulator \"vref\"\n");
 		return PTR_ERR(st->reg);
 	}
 
 	ret = regulator_enable(st->reg);
 	if (ret) {
-		dev_err(&spi->dev, "Failed to enable regulator \"refin\"\n");
+		dev_err(&spi->dev, "Failed to enable regulator \"vref\"\n");
 		return ret;
 	}
 
-- 
2.7.4

Re: [PATCH 2/3] iio: adc: ti-ads7950: Drop "ti-" prefix from module name

[Jonathan Cameron]

On 11/01/17 17:52, David Lechner wrote:
> This drops the "ti-" prefix from the module name. It makes the module name
> consistent with other iio ti-ads* drivers and it makes the driver work
> with device tree (the spi subsystem drops the "ti," prefix when matching
> compatible strings from device tree).
> 
> Tested working on LEGO MINDSTORMS EV3 with the following device tree node:
> 
> 	adc@3 {
> 		compatible = "ti,ads7957";
> 		reg = <3>;
> 		#io-channel-cells = <1>;
> 		spi-max-frequency = <10000000>;
> 		vref-supply = <&adc_ref>;
> 	};
> 
> Signed-off-by: David Lechner <david@lechnology.com>
What worries me here is that we might break existing setups.  I agree
we should have gotten this 'right' in the first place, but can we fix
it now.  Not so sure. We'd be better off perhaps adding an of_device_id
table with the write entries for device tree.
> ---
>  drivers/iio/adc/ti-ads7950.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
> index 0330361..b587fa6 100644
> --- a/drivers/iio/adc/ti-ads7950.c
> +++ b/drivers/iio/adc/ti-ads7950.c
> @@ -459,25 +459,25 @@ static int ti_ads7950_remove(struct spi_device *spi)
>  }
>  
>  static const struct spi_device_id ti_ads7950_id[] = {
> -	{"ti-ads7950", TI_ADS7950},
> -	{"ti-ads7951", TI_ADS7951},
> -	{"ti-ads7952", TI_ADS7952},
> -	{"ti-ads7953", TI_ADS7953},
> -	{"ti-ads7954", TI_ADS7954},
> -	{"ti-ads7955", TI_ADS7955},
> -	{"ti-ads7956", TI_ADS7956},
> -	{"ti-ads7957", TI_ADS7957},
> -	{"ti-ads7958", TI_ADS7958},
> -	{"ti-ads7959", TI_ADS7959},
> -	{"ti-ads7960", TI_ADS7960},
> -	{"ti-ads7961", TI_ADS7961},
> +	{ "ads7950", TI_ADS7950 },
> +	{ "ads7951", TI_ADS7951 },
> +	{ "ads7952", TI_ADS7952 },
> +	{ "ads7953", TI_ADS7953 },
> +	{ "ads7954", TI_ADS7954 },
> +	{ "ads7955", TI_ADS7955 },
> +	{ "ads7956", TI_ADS7956 },
> +	{ "ads7957", TI_ADS7957 },
> +	{ "ads7958", TI_ADS7958 },
> +	{ "ads7959", TI_ADS7959 },
> +	{ "ads7960", TI_ADS7960 },
> +	{ "ads7961", TI_ADS7961 },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(spi, ti_ads7950_id);
>  
>  static struct spi_driver ti_ads7950_driver = {
>  	.driver = {
> -		.name	= "ti-ads7950",
> +		.name	= "ads7950",
>  	},
>  	.probe		= ti_ads7950_probe,
>  	.remove		= ti_ads7950_remove,
> 

Re: [PATCH 3/3] iio: adc: ti-ads7950: Change regulator matching string to "vref"

[Jonathan Cameron]

On 11/01/17 17:52, David Lechner wrote:
> This changes the reference voltage regulator matching string from "refin"
> to "vref". This is to be consistent with other A/DC chips that also use
> "vref-supply" in their device tree bindings.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>  drivers/iio/adc/ti-ads7950.c | 6 +++---
Again, we missed this before and it would have been nice to have
had it as vref (which is matches the datasheet).  The question
becomes how do we handle this going forward with no risk of breaking
existing device trees.  We may have to do an optional get on one
name and then a non optional on the second. It's ugly, but
would fix this up in a 'safe' way.

Jonathan
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
> index b587fa6..16a0663 100644
> --- a/drivers/iio/adc/ti-ads7950.c
> +++ b/drivers/iio/adc/ti-ads7950.c
> @@ -411,15 +411,15 @@ static int ti_ads7950_probe(struct spi_device *spi)
>  	spi_message_init_with_transfers(&st->scan_single_msg,
>  					st->scan_single_xfer, 3);
>  
> -	st->reg = devm_regulator_get(&spi->dev, "refin");
> +	st->reg = devm_regulator_get(&spi->dev, "vref");
>  	if (IS_ERR(st->reg)) {
> -		dev_err(&spi->dev, "Failed get get regulator \"refin\"\n");
> +		dev_err(&spi->dev, "Failed get get regulator \"vref\"\n");
>  		return PTR_ERR(st->reg);
>  	}
>  
>  	ret = regulator_enable(st->reg);
>  	if (ret) {
> -		dev_err(&spi->dev, "Failed to enable regulator \"refin\"\n");
> +		dev_err(&spi->dev, "Failed to enable regulator \"vref\"\n");
>  		return ret;
>  	}
>  
> 

Re: [PATCH 1/3] DT/bindings: Add bindings for TI ADS7950 A/DC chips

[Jonathan Cameron]

On 11/01/17 17:52, David Lechner wrote:
> This adds device tree bindings for the TI ADS7950 family of A/DC chips.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
This is in of itself good, but we may need to have some deprecated
elements to continue supporting what was implicitly happening with
the missnaming so as to avoid accidentally breaking someone's device
tree.

Jonathan
> ---
>  .../devicetree/bindings/iio/adc/ti-ads7950.txt     | 23 ++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt b/Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt
> new file mode 100644
> index 0000000..e77a6f7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt
> @@ -0,0 +1,23 @@
> +* Texas Instruments ADS7950 family of A/DC chips
> +
> +Required properties:
> + - compatible: Must be one of "ti,ads7950", "ti,ads7951", "ti,ads7952",
> +   "ti,ads7953", "ti,ads7954", "ti,ads7955", "ti,ads7956", "ti,ads7957",
> +   "ti,ads7958", "ti,ads7959", "ti,ads7960", or "ti,ads7961"
> + - reg: SPI chip select number for the device
> + - #io-channel-cells: Must be 1 as per ../iio-bindings.txt
> + - vref-supply: phandle to a regulator node that supplies the 2.5V or 5V
> +   reference voltage
> +
> +Recommended properties:
> + - spi-max-frequency: Definition as per
> +		Documentation/devicetree/bindings/spi/spi-bus.txt
> +
> +Example:
> +adc@0 {
> +	compatible = "ti,ads7957";
> +	reg = <0>;
> +	#io-channel-cells = <1>;
> +	vref-supply = <&refin_supply>;
> +	spi-max-frequency = <10000000>;
> +};
> 

Re: [PATCH 1/3] DT/bindings: Add bindings for TI ADS7950 A/DC chips

[David Lechner]

On 01/14/2017 06:53 AM, Jonathan Cameron wrote:
> On 11/01/17 17:52, David Lechner wrote:
>> This adds device tree bindings for the TI ADS7950 family of A/DC chips.
>>
>> Signed-off-by: David Lechner <david@lechnology.com>
> This is in of itself good, but we may need to have some deprecated
> elements to continue supporting what was implicitly happening with
> the missnaming so as to avoid accidentally breaking someone's device
> tree.
>

As I mentioned in my cover letter, this driver, as far as I can tell, 
only exists in your testing branch, so I find it highly unlikely that we 
would be breaking anyone. It is not in mainline and it is not even in 
linux-next.

> Jonathan
>> ---
>>  .../devicetree/bindings/iio/adc/ti-ads7950.txt     | 23 ++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt b/Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt
>> new file mode 100644
>> index 0000000..e77a6f7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt
>> @@ -0,0 +1,23 @@
>> +* Texas Instruments ADS7950 family of A/DC chips
>> +
>> +Required properties:
>> + - compatible: Must be one of "ti,ads7950", "ti,ads7951", "ti,ads7952",
>> +   "ti,ads7953", "ti,ads7954", "ti,ads7955", "ti,ads7956", "ti,ads7957",
>> +   "ti,ads7958", "ti,ads7959", "ti,ads7960", or "ti,ads7961"
>> + - reg: SPI chip select number for the device
>> + - #io-channel-cells: Must be 1 as per ../iio-bindings.txt
>> + - vref-supply: phandle to a regulator node that supplies the 2.5V or 5V
>> +   reference voltage
>> +
>> +Recommended properties:
>> + - spi-max-frequency: Definition as per
>> +		Documentation/devicetree/bindings/spi/spi-bus.txt
>> +
>> +Example:
>> +adc@0 {
>> +	compatible = "ti,ads7957";
>> +	reg = <0>;
>> +	#io-channel-cells = <1>;
>> +	vref-supply = <&refin_supply>;
>> +	spi-max-frequency = <10000000>;
>> +};
>>
>

Re: [PATCH 2/3] iio: adc: ti-ads7950: Drop "ti-" prefix from module name

[David Lechner]

On 01/14/2017 06:49 AM, Jonathan Cameron wrote:
> On 11/01/17 17:52, David Lechner wrote:
>> This drops the "ti-" prefix from the module name. It makes the module name
>> consistent with other iio ti-ads* drivers and it makes the driver work
>> with device tree (the spi subsystem drops the "ti," prefix when matching
>> compatible strings from device tree).
>>
>> Tested working on LEGO MINDSTORMS EV3 with the following device tree node:
>>
>> 	adc@3 {
>> 		compatible = "ti,ads7957";
>> 		reg = <3>;
>> 		#io-channel-cells = <1>;
>> 		spi-max-frequency = <10000000>;
>> 		vref-supply = <&adc_ref>;
>> 	};
>>
>> Signed-off-by: David Lechner <david@lechnology.com>
> What worries me here is that we might break existing setups.  I agree
> we should have gotten this 'right' in the first place, but can we fix
> it now.  Not so sure. We'd be better off perhaps adding an of_device_id
> table with the write entries for device tree.

As far as I can tell, this driver only exists in your testing branch. 
Does that really mean that it is too late to get it right?

>> ---
>>  drivers/iio/adc/ti-ads7950.c | 26 +++++++++++++-------------
>>  1 file changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
>> index 0330361..b587fa6 100644
>> --- a/drivers/iio/adc/ti-ads7950.c
>> +++ b/drivers/iio/adc/ti-ads7950.c
>> @@ -459,25 +459,25 @@ static int ti_ads7950_remove(struct spi_device *spi)
>>  }
>>
>>  static const struct spi_device_id ti_ads7950_id[] = {
>> -	{"ti-ads7950", TI_ADS7950},
>> -	{"ti-ads7951", TI_ADS7951},
>> -	{"ti-ads7952", TI_ADS7952},
>> -	{"ti-ads7953", TI_ADS7953},
>> -	{"ti-ads7954", TI_ADS7954},
>> -	{"ti-ads7955", TI_ADS7955},
>> -	{"ti-ads7956", TI_ADS7956},
>> -	{"ti-ads7957", TI_ADS7957},
>> -	{"ti-ads7958", TI_ADS7958},
>> -	{"ti-ads7959", TI_ADS7959},
>> -	{"ti-ads7960", TI_ADS7960},
>> -	{"ti-ads7961", TI_ADS7961},
>> +	{ "ads7950", TI_ADS7950 },
>> +	{ "ads7951", TI_ADS7951 },
>> +	{ "ads7952", TI_ADS7952 },
>> +	{ "ads7953", TI_ADS7953 },
>> +	{ "ads7954", TI_ADS7954 },
>> +	{ "ads7955", TI_ADS7955 },
>> +	{ "ads7956", TI_ADS7956 },
>> +	{ "ads7957", TI_ADS7957 },
>> +	{ "ads7958", TI_ADS7958 },
>> +	{ "ads7959", TI_ADS7959 },
>> +	{ "ads7960", TI_ADS7960 },
>> +	{ "ads7961", TI_ADS7961 },
>>  	{ }
>>  };
>>  MODULE_DEVICE_TABLE(spi, ti_ads7950_id);
>>
>>  static struct spi_driver ti_ads7950_driver = {
>>  	.driver = {
>> -		.name	= "ti-ads7950",
>> +		.name	= "ads7950",
>>  	},
>>  	.probe		= ti_ads7950_probe,
>>  	.remove		= ti_ads7950_remove,
>>
>

Re: [PATCH 3/3] iio: adc: ti-ads7950: Change regulator matching string to "vref"

[David Lechner]

On 01/14/2017 06:52 AM, Jonathan Cameron wrote:
> On 11/01/17 17:52, David Lechner wrote:
>> This changes the reference voltage regulator matching string from "refin"
>> to "vref". This is to be consistent with other A/DC chips that also use
>> "vref-supply" in their device tree bindings.
>>
>> Signed-off-by: David Lechner <david@lechnology.com>
>> ---
>>  drivers/iio/adc/ti-ads7950.c | 6 +++---
> Again, we missed this before and it would have been nice to have
> had it as vref (which is matches the datasheet).  The question
> becomes how do we handle this going forward with no risk of breaking
> existing device trees.  We may have to do an optional get on one
> name and then a non optional on the second. It's ugly, but
> would fix this up in a 'safe' way.
>

Again, I don't think it is too late since this driver only exists in the 
iio/testing branch.

> Jonathan
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
>> index b587fa6..16a0663 100644
>> --- a/drivers/iio/adc/ti-ads7950.c
>> +++ b/drivers/iio/adc/ti-ads7950.c
>> @@ -411,15 +411,15 @@ static int ti_ads7950_probe(struct spi_device *spi)
>>  	spi_message_init_with_transfers(&st->scan_single_msg,
>>  					st->scan_single_xfer, 3);
>>
>> -	st->reg = devm_regulator_get(&spi->dev, "refin");
>> +	st->reg = devm_regulator_get(&spi->dev, "vref");
>>  	if (IS_ERR(st->reg)) {
>> -		dev_err(&spi->dev, "Failed get get regulator \"refin\"\n");
>> +		dev_err(&spi->dev, "Failed get get regulator \"vref\"\n");
>>  		return PTR_ERR(st->reg);
>>  	}
>>
>>  	ret = regulator_enable(st->reg);
>>  	if (ret) {
>> -		dev_err(&spi->dev, "Failed to enable regulator \"refin\"\n");
>> +		dev_err(&spi->dev, "Failed to enable regulator \"vref\"\n");
>>  		return ret;
>>  	}
>>
>>
>

Re: [PATCH 1/3] DT/bindings: Add bindings for TI ADS7950 A/DC chips

[Jonathan Cameron]

On 14/01/17 18:00, David Lechner wrote:
> On 01/14/2017 06:53 AM, Jonathan Cameron wrote:
>> On 11/01/17 17:52, David Lechner wrote:
>>> This adds device tree bindings for the TI ADS7950 family of A/DC chips.
>>>
>>> Signed-off-by: David Lechner <david@lechnology.com>
>> This is in of itself good, but we may need to have some deprecated
>> elements to continue supporting what was implicitly happening with
>> the missnaming so as to avoid accidentally breaking someone's device
>> tree.
>>
> 
> As I mentioned in my cover letter, this driver, as far as I can tell,
> only exists in your testing branch, so I find it highly unlikely that
> we would be breaking anyone. It is not in mainline and it is not even
> in linux-next.

Oops, I not only have a memory like a goldfish, I clearly didn't read your
cover letter either.

Applied to the togreg branch of iio.git and pushed out as testing.

The driver will hit next before this does but will get into mainline
in the same ultimate pull request unless something weird happens so that's
all fine.

Jonathan
>> Jonathan
>>> ---
>>>  .../devicetree/bindings/iio/adc/ti-ads7950.txt     | 23 ++++++++++++++++++++++
>>>  1 file changed, 23 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt b/Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt
>>> new file mode 100644
>>> index 0000000..e77a6f7
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt
>>> @@ -0,0 +1,23 @@
>>> +* Texas Instruments ADS7950 family of A/DC chips
>>> +
>>> +Required properties:
>>> + - compatible: Must be one of "ti,ads7950", "ti,ads7951", "ti,ads7952",
>>> +   "ti,ads7953", "ti,ads7954", "ti,ads7955", "ti,ads7956", "ti,ads7957",
>>> +   "ti,ads7958", "ti,ads7959", "ti,ads7960", or "ti,ads7961"
>>> + - reg: SPI chip select number for the device
>>> + - #io-channel-cells: Must be 1 as per ../iio-bindings.txt
>>> + - vref-supply: phandle to a regulator node that supplies the 2.5V or 5V
>>> +   reference voltage
>>> +
>>> +Recommended properties:
>>> + - spi-max-frequency: Definition as per
>>> +        Documentation/devicetree/bindings/spi/spi-bus.txt
>>> +
>>> +Example:
>>> +adc@0 {
>>> +    compatible = "ti,ads7957";
>>> +    reg = <0>;
>>> +    #io-channel-cells = <1>;
>>> +    vref-supply = <&refin_supply>;
>>> +    spi-max-frequency = <10000000>;
>>> +};
>>>
>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] iio: adc: ti-ads7950: Drop "ti-" prefix from module name

[Jonathan Cameron]

On 14/01/17 18:07, David Lechner wrote:
> On 01/14/2017 06:49 AM, Jonathan Cameron wrote:
>> On 11/01/17 17:52, David Lechner wrote:
>>> This drops the "ti-" prefix from the module name. It makes the module name
>>> consistent with other iio ti-ads* drivers and it makes the driver work
>>> with device tree (the spi subsystem drops the "ti," prefix when matching
>>> compatible strings from device tree).
>>>
>>> Tested working on LEGO MINDSTORMS EV3 with the following device tree node:
>>>
>>>     adc@3 {
>>>         compatible = "ti,ads7957";
>>>         reg = <3>;
>>>         #io-channel-cells = <1>;
>>>         spi-max-frequency = <10000000>;
>>>         vref-supply = <&adc_ref>;
>>>     };
>>>
>>> Signed-off-by: David Lechner <david@lechnology.com>
>> What worries me here is that we might break existing setups.  I agree
>> we should have gotten this 'right' in the first place, but can we fix
>> it now.  Not so sure. We'd be better off perhaps adding an of_device_id
>> table with the write entries for device tree.
> 

> As far as I can tell, this driver only exists in your testing branch.
> Does that really mean that it is too late to get it right?
Gah!  I have a memory like a goldfish.

Excellent point - we can and should do this asap.
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan


>>> ---
>>>  drivers/iio/adc/ti-ads7950.c | 26 +++++++++++++-------------
>>>  1 file changed, 13 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
>>> index 0330361..b587fa6 100644
>>> --- a/drivers/iio/adc/ti-ads7950.c
>>> +++ b/drivers/iio/adc/ti-ads7950.c
>>> @@ -459,25 +459,25 @@ static int ti_ads7950_remove(struct spi_device *spi)
>>>  }
>>>
>>>  static const struct spi_device_id ti_ads7950_id[] = {
>>> -    {"ti-ads7950", TI_ADS7950},
>>> -    {"ti-ads7951", TI_ADS7951},
>>> -    {"ti-ads7952", TI_ADS7952},
>>> -    {"ti-ads7953", TI_ADS7953},
>>> -    {"ti-ads7954", TI_ADS7954},
>>> -    {"ti-ads7955", TI_ADS7955},
>>> -    {"ti-ads7956", TI_ADS7956},
>>> -    {"ti-ads7957", TI_ADS7957},
>>> -    {"ti-ads7958", TI_ADS7958},
>>> -    {"ti-ads7959", TI_ADS7959},
>>> -    {"ti-ads7960", TI_ADS7960},
>>> -    {"ti-ads7961", TI_ADS7961},
>>> +    { "ads7950", TI_ADS7950 },
>>> +    { "ads7951", TI_ADS7951 },
>>> +    { "ads7952", TI_ADS7952 },
>>> +    { "ads7953", TI_ADS7953 },
>>> +    { "ads7954", TI_ADS7954 },
>>> +    { "ads7955", TI_ADS7955 },
>>> +    { "ads7956", TI_ADS7956 },
>>> +    { "ads7957", TI_ADS7957 },
>>> +    { "ads7958", TI_ADS7958 },
>>> +    { "ads7959", TI_ADS7959 },
>>> +    { "ads7960", TI_ADS7960 },
>>> +    { "ads7961", TI_ADS7961 },
>>>      { }
>>>  };
>>>  MODULE_DEVICE_TABLE(spi, ti_ads7950_id);
>>>
>>>  static struct spi_driver ti_ads7950_driver = {
>>>      .driver = {
>>> -        .name    = "ti-ads7950",
>>> +        .name    = "ads7950",
>>>      },
>>>      .probe        = ti_ads7950_probe,
>>>      .remove        = ti_ads7950_remove,
>>>
>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] iio: adc: ti-ads7950: Change regulator matching string to "vref"

[Jonathan Cameron]

On 14/01/17 18:09, David Lechner wrote:
> On 01/14/2017 06:52 AM, Jonathan Cameron wrote:
>> On 11/01/17 17:52, David Lechner wrote:
>>> This changes the reference voltage regulator matching string from "refin"
>>> to "vref". This is to be consistent with other A/DC chips that also use
>>> "vref-supply" in their device tree bindings.
>>>
>>> Signed-off-by: David Lechner <david@lechnology.com>
>>> ---
>>>  drivers/iio/adc/ti-ads7950.c | 6 +++---
>> Again, we missed this before and it would have been nice to have
>> had it as vref (which is matches the datasheet).  The question
>> becomes how do we handle this going forward with no risk of breaking
>> existing device trees.  We may have to do an optional get on one
>> name and then a non optional on the second. It's ugly, but
>> would fix this up in a 'safe' way.
>>
> 
> Again, I don't think it is too late since this driver only exists in the iio/testing branch.
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan
> 
>> Jonathan
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
>>> index b587fa6..16a0663 100644
>>> --- a/drivers/iio/adc/ti-ads7950.c
>>> +++ b/drivers/iio/adc/ti-ads7950.c
>>> @@ -411,15 +411,15 @@ static int ti_ads7950_probe(struct spi_device *spi)
>>>      spi_message_init_with_transfers(&st->scan_single_msg,
>>>                      st->scan_single_xfer, 3);
>>>
>>> -    st->reg = devm_regulator_get(&spi->dev, "refin");
>>> +    st->reg = devm_regulator_get(&spi->dev, "vref");
>>>      if (IS_ERR(st->reg)) {
>>> -        dev_err(&spi->dev, "Failed get get regulator \"refin\"\n");
>>> +        dev_err(&spi->dev, "Failed get get regulator \"vref\"\n");
>>>          return PTR_ERR(st->reg);
>>>      }
>>>
>>>      ret = regulator_enable(st->reg);
>>>      if (ret) {
>>> -        dev_err(&spi->dev, "Failed to enable regulator \"refin\"\n");
>>> +        dev_err(&spi->dev, "Failed to enable regulator \"vref\"\n");
>>>          return ret;
>>>      }
>>>
>>>
>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] ti,ads7950 device tree bindings

[Jonathan Cameron]

On 11/01/17 17:52, David Lechner wrote:
> This series adds device tree bindings for the TI ADS7950 family of A/DC chips.
> The series includes the bindings documentation and some fixes to the iio driver
> to make it work with the device tree bindings.
> 
> FYI, the ads7950 driver has not made it into mainline yet, so no worries about
> breaking anyone with these changes.
> 
And here's the bit I failed to read!

As an extra point, could you confirm what /sys/bus/iio/iio\:deviceX/name for this
one reads?  I have a feeling this is another case of what Lars has been pointing
out in other drivers this morning.  That name should be the device part number..

Thanks,

Jonathan
> David Lechner (3):
>   DT/bindings: Add bindings for TI ADS7950 A/DC chips
>   iio: adc: ti-ads7950: Drop "ti-" prefix from module name
>   iio: adc: ti-ads7950: Change regulator matching string to "vref"
> 
>  .../devicetree/bindings/iio/adc/ti-ads7950.txt     | 23 ++++++++++++++++
>  drivers/iio/adc/ti-ads7950.c                       | 32 +++++++++++-----------
>  2 files changed, 39 insertions(+), 16 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt
> 

Re: [PATCH 0/3] ti,ads7950 device tree bindings

[David Lechner]

On 01/15/2017 07:58 AM, Jonathan Cameron wrote:
> On 11/01/17 17:52, David Lechner wrote:
>> This series adds device tree bindings for the TI ADS7950 family of A/DC chips.
>> The series includes the bindings documentation and some fixes to the iio driver
>> to make it work with the device tree bindings.
>>
>> FYI, the ads7950 driver has not made it into mainline yet, so no worries about
>> breaking anyone with these changes.
>>
> And here's the bit I failed to read!
>
> As an extra point, could you confirm what /sys/bus/iio/iio\:deviceX/name for this
> one reads?  I have a feeling this is another case of what Lars has been pointing
> out in other drivers this morning.  That name should be the device part number..
>

cat /sys/bus/iio/devices/iio\:device0/name
ads7957

This is the part number for the specific chip I am using. So, I am 
guessing that it is correct unless it is supposed to be upper case or 
something like that.



> Thanks,
>
> Jonathan
>> David Lechner (3):
>>   DT/bindings: Add bindings for TI ADS7950 A/DC chips
>>   iio: adc: ti-ads7950: Drop "ti-" prefix from module name
>>   iio: adc: ti-ads7950: Change regulator matching string to "vref"
>>
>>  .../devicetree/bindings/iio/adc/ti-ads7950.txt     | 23 ++++++++++++++++
>>  drivers/iio/adc/ti-ads7950.c                       | 32 +++++++++++-----------
>>  2 files changed, 39 insertions(+), 16 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt
>>
>

Re: [PATCH 1/3] DT/bindings: Add bindings for TI ADS7950 A/DC chips

[Rob Herring]

On Wed, Jan 11, 2017 at 11:52:49AM -0600, David Lechner wrote:
> This adds device tree bindings for the TI ADS7950 family of A/DC chips.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>  .../devicetree/bindings/iio/adc/ti-ads7950.txt     | 23 ++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 0/3] ti,ads7950 device tree bindings

[Jonathan Cameron]

On 15/01/17 23:44, David Lechner wrote:
> On 01/15/2017 07:58 AM, Jonathan Cameron wrote:
>> On 11/01/17 17:52, David Lechner wrote:
>>> This series adds device tree bindings for the TI ADS7950 family of A/DC chips.
>>> The series includes the bindings documentation and some fixes to the iio driver
>>> to make it work with the device tree bindings.
>>>
>>> FYI, the ads7950 driver has not made it into mainline yet, so no worries about
>>> breaking anyone with these changes.
>>>
>> And here's the bit I failed to read!
>>
>> As an extra point, could you confirm what /sys/bus/iio/iio\:deviceX/name for this
>> one reads?  I have a feeling this is another case of what Lars has been pointing
>> out in other drivers this morning.  That name should be the device part number..
>>
> 
> cat /sys/bus/iio/devices/iio\:device0/name
> ads7957
> 
> This is the part number for the specific chip I am using. So, I am guessing that it is correct unless it is supposed to be upper case or something like that.
> 
> 
Cool. Spot on.
> 
>> Thanks,
>>
>> Jonathan
>>> David Lechner (3):
>>>   DT/bindings: Add bindings for TI ADS7950 A/DC chips
>>>   iio: adc: ti-ads7950: Drop "ti-" prefix from module name
>>>   iio: adc: ti-ads7950: Change regulator matching string to "vref"
>>>
>>>  .../devicetree/bindings/iio/adc/ti-ads7950.txt     | 23 ++++++++++++++++
>>>  drivers/iio/adc/ti-ads7950.c                       | 32 +++++++++++-----------
>>>  2 files changed, 39 insertions(+), 16 deletions(-)
>>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-ads7950.txt
>>>
>>
>