Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1427357 > unrolled thread

[PATCH 0/3] iio: potentiometer: mcp4531: New parts and device tree

Started byFlorian Vaussard <florian.vaussard@gmail.com>
First post2016-06-21 09:00 +0200
Last post2016-06-21 09:00 +0200
Articles 10 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] iio: potentiometer: mcp4531: New parts and device tree Florian Vaussard <florian.vaussard@gmail.com> - 2016-06-21 09:00 +0200
    [PATCH 3/3] iio: potentiometer: mcp4531: Add device tree binding Florian Vaussard <florian.vaussard@gmail.com> - 2016-06-21 09:00 +0200
      Re: [PATCH 3/3] iio: potentiometer: mcp4531: Add device tree binding Florian Vaussard <florian.vaussard@heig-vd.ch> - 2016-06-22 08:30 +0200
        Re: [PATCH 3/3] iio: potentiometer: mcp4531: Add device tree binding Florian Vaussard <florian.vaussard@heig-vd.ch> - 2016-06-23 11:00 +0200
    [PATCH 2/3] iio: potentiometer: mcp4531: Add device tree binding documentation Florian Vaussard <florian.vaussard@gmail.com> - 2016-06-21 09:00 +0200
      Re: [PATCH 2/3] iio: potentiometer: mcp4531: Add device tree binding  documentation Florian Vaussard <florian.vaussard@heig-vd.ch> - 2016-06-21 10:20 +0200
        Re: [PATCH 2/3] iio: potentiometer: mcp4531: Add device tree binding  documentation Rob Herring <robh@kernel.org> - 2016-06-21 23:40 +0200
      Re: [PATCH 2/3] iio: potentiometer: mcp4531: Add device tree binding  documentation Rob Herring <robh@kernel.org> - 2016-06-21 23:50 +0200
        Re: [PATCH 2/3] iio: potentiometer: mcp4531: Add device tree binding  documentation Florian Vaussard <florian.vaussard@heig-vd.ch> - 2016-06-22 08:20 +0200
    [PATCH 1/3] iio: potentiometer: mcp4531: Add support for MCP454x, MCP456x, MCP464x and MCP466x Florian Vaussard <florian.vaussard@gmail.com> - 2016-06-21 09:00 +0200

#1427357 — [PATCH 0/3] iio: potentiometer: mcp4531: New parts and device tree

FromFlorian Vaussard <florian.vaussard@gmail.com>
Date2016-06-21 09:00 +0200
Subject[PATCH 0/3] iio: potentiometer: mcp4531: New parts and device tree
Message-ID<rMnPb-2gH-5@gated-at.bofh.it>
Hello,

This series first adds support for parts missing from mcp4531 driver
(MCP454x, MCP456x, MCP464x and MCP466x). It then introduces the necessary
device tree binding to perform DT boot.

Tested with MCP4561-103 and MCP4561-503 (DT boot).

Best regards,
Florian

Florian Vaussard (3):
  iio: potentiometer: mcp4531: Add support for MCP454x, MCP456x, MCP464x
    and MCP466x
  iio: potentiometer: mcp4531: Add device tree binding documentation
  iio: potentiometer: mcp4531: Add device tree binding

 .../bindings/iio/potentiometer/mcp4531.txt         |  84 +++++++++++
 drivers/iio/potentiometer/Kconfig                  |   6 +-
 drivers/iio/potentiometer/mcp4531.c                | 155 ++++++++++++++++++++-
 3 files changed, 242 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt

-- 
2.5.0

[toc] | [next] | [standalone]

#1427358 — [PATCH 3/3] iio: potentiometer: mcp4531: Add device tree binding

FromFlorian Vaussard <florian.vaussard@gmail.com>
Date2016-06-21 09:00 +0200
Subject[PATCH 3/3] iio: potentiometer: mcp4531: Add device tree binding
Message-ID<rMnPb-2gH-7@gated-at.bofh.it>
In reply to#1427357
This patch adds the necessary device tree binding to allow DT probing of
currently supported parts.

Signed-off-by: Florian Vaussard <florian.vaussard@heig-vd.ch>
---
 drivers/iio/potentiometer/mcp4531.c | 83 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 82 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/potentiometer/mcp4531.c b/drivers/iio/potentiometer/mcp4531.c
index 2251173..41a1e46 100644
--- a/drivers/iio/potentiometer/mcp4531.c
+++ b/drivers/iio/potentiometer/mcp4531.c
@@ -31,6 +31,8 @@
 #include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include <linux/iio/iio.h>
 
@@ -188,12 +190,84 @@ static const struct iio_info mcp4531_info = {
 	.driver_module = THIS_MODULE,
 };
 
+#ifdef CONFIG_OF
+static const struct of_device_id mcp45xx_of_match[] = {
+	{ .compatible = "microchip,mcp4531-502", .data = (void *)MCP453x_502 },
+	{ .compatible = "microchip,mcp4531-103", .data = (void *)MCP453x_103 },
+	{ .compatible = "microchip,mcp4531-503", .data = (void *)MCP453x_503 },
+	{ .compatible = "microchip,mcp4531-104", .data = (void *)MCP453x_104 },
+	{ .compatible = "microchip,mcp4532-502", .data = (void *)MCP453x_502 },
+	{ .compatible = "microchip,mcp4532-103", .data = (void *)MCP453x_103 },
+	{ .compatible = "microchip,mcp4532-503", .data = (void *)MCP453x_503 },
+	{ .compatible = "microchip,mcp4532-104", .data = (void *)MCP453x_104 },
+	{ .compatible = "microchip,mcp4541-502", .data = (void *)MCP454x_502 },
+	{ .compatible = "microchip,mcp4541-103", .data = (void *)MCP454x_103 },
+	{ .compatible = "microchip,mcp4541-503", .data = (void *)MCP454x_503 },
+	{ .compatible = "microchip,mcp4541-104", .data = (void *)MCP454x_104 },
+	{ .compatible = "microchip,mcp4542-502", .data = (void *)MCP454x_502 },
+	{ .compatible = "microchip,mcp4542-103", .data = (void *)MCP454x_103 },
+	{ .compatible = "microchip,mcp4542-503", .data = (void *)MCP454x_503 },
+	{ .compatible = "microchip,mcp4542-104", .data = (void *)MCP454x_104 },
+	{ .compatible = "microchip,mcp4551-502", .data = (void *)MCP455x_502 },
+	{ .compatible = "microchip,mcp4551-103", .data = (void *)MCP455x_103 },
+	{ .compatible = "microchip,mcp4551-503", .data = (void *)MCP455x_503 },
+	{ .compatible = "microchip,mcp4551-104", .data = (void *)MCP455x_104 },
+	{ .compatible = "microchip,mcp4552-502", .data = (void *)MCP455x_502 },
+	{ .compatible = "microchip,mcp4552-103", .data = (void *)MCP455x_103 },
+	{ .compatible = "microchip,mcp4552-503", .data = (void *)MCP455x_503 },
+	{ .compatible = "microchip,mcp4552-104", .data = (void *)MCP455x_104 },
+	{ .compatible = "microchip,mcp4561-502", .data = (void *)MCP456x_502 },
+	{ .compatible = "microchip,mcp4561-103", .data = (void *)MCP456x_103 },
+	{ .compatible = "microchip,mcp4561-503", .data = (void *)MCP456x_503 },
+	{ .compatible = "microchip,mcp4561-104", .data = (void *)MCP456x_104 },
+	{ .compatible = "microchip,mcp4562-502", .data = (void *)MCP456x_502 },
+	{ .compatible = "microchip,mcp4562-103", .data = (void *)MCP456x_103 },
+	{ .compatible = "microchip,mcp4562-503", .data = (void *)MCP456x_503 },
+	{ .compatible = "microchip,mcp4562-104", .data = (void *)MCP456x_104 },
+	{ .compatible = "microchip,mcp4631-502", .data = (void *)MCP463x_502 },
+	{ .compatible = "microchip,mcp4631-103", .data = (void *)MCP463x_103 },
+	{ .compatible = "microchip,mcp4631-503", .data = (void *)MCP463x_503 },
+	{ .compatible = "microchip,mcp4631-104", .data = (void *)MCP463x_104 },
+	{ .compatible = "microchip,mcp4632-502", .data = (void *)MCP463x_502 },
+	{ .compatible = "microchip,mcp4632-103", .data = (void *)MCP463x_103 },
+	{ .compatible = "microchip,mcp4632-503", .data = (void *)MCP463x_503 },
+	{ .compatible = "microchip,mcp4632-104", .data = (void *)MCP463x_104 },
+	{ .compatible = "microchip,mcp4641-502", .data = (void *)MCP464x_502 },
+	{ .compatible = "microchip,mcp4641-103", .data = (void *)MCP464x_103 },
+	{ .compatible = "microchip,mcp4641-503", .data = (void *)MCP464x_503 },
+	{ .compatible = "microchip,mcp4641-104", .data = (void *)MCP464x_104 },
+	{ .compatible = "microchip,mcp4642-502", .data = (void *)MCP464x_502 },
+	{ .compatible = "microchip,mcp4642-103", .data = (void *)MCP464x_103 },
+	{ .compatible = "microchip,mcp4642-503", .data = (void *)MCP464x_503 },
+	{ .compatible = "microchip,mcp4642-104", .data = (void *)MCP464x_104 },
+	{ .compatible = "microchip,mcp4651-502", .data = (void *)MCP465x_502 },
+	{ .compatible = "microchip,mcp4651-103", .data = (void *)MCP465x_103 },
+	{ .compatible = "microchip,mcp4651-503", .data = (void *)MCP465x_503 },
+	{ .compatible = "microchip,mcp4651-104", .data = (void *)MCP465x_104 },
+	{ .compatible = "microchip,mcp4652-502", .data = (void *)MCP465x_502 },
+	{ .compatible = "microchip,mcp4652-103", .data = (void *)MCP465x_103 },
+	{ .compatible = "microchip,mcp4652-503", .data = (void *)MCP465x_503 },
+	{ .compatible = "microchip,mcp4652-104", .data = (void *)MCP465x_104 },
+	{ .compatible = "microchip,mcp4661-502", .data = (void *)MCP466x_502 },
+	{ .compatible = "microchip,mcp4661-103", .data = (void *)MCP466x_103 },
+	{ .compatible = "microchip,mcp4661-503", .data = (void *)MCP466x_503 },
+	{ .compatible = "microchip,mcp4661-104", .data = (void *)MCP466x_104 },
+	{ .compatible = "microchip,mcp4662-502", .data = (void *)MCP466x_502 },
+	{ .compatible = "microchip,mcp4662-103", .data = (void *)MCP466x_103 },
+	{ .compatible = "microchip,mcp4662-503", .data = (void *)MCP466x_503 },
+	{ .compatible = "microchip,mcp4662-104", .data = (void *)MCP466x_104 },
+	{ /*sentinel*/ }
+};
+#endif
+
 static int mcp4531_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct device *dev = &client->dev;
+	unsigned long devid;
 	struct mcp4531_data *data;
 	struct iio_dev *indio_dev;
+	const struct of_device_id *match;
 
 	if (!i2c_check_functionality(client->adapter,
 				     I2C_FUNC_SMBUS_WORD_DATA)) {
@@ -201,13 +275,19 @@ static int mcp4531_probe(struct i2c_client *client,
 		return -EOPNOTSUPP;
 	}
 
+	match = of_match_device(of_match_ptr(mcp45xx_of_match), dev);
+	if (match)
+		devid = (int)of_device_get_match_data(dev);
+	else
+		devid = id->driver_data;
+
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
 	if (!indio_dev)
 		return -ENOMEM;
 	data = iio_priv(indio_dev);
 	i2c_set_clientdata(client, indio_dev);
 	data->client = client;
-	data->cfg = &mcp4531_cfg[id->driver_data];
+	data->cfg = &mcp4531_cfg[devid];
 
 	indio_dev->dev.parent = dev;
 	indio_dev->info = &mcp4531_info;
@@ -290,6 +370,7 @@ MODULE_DEVICE_TABLE(i2c, mcp4531_id);
 static struct i2c_driver mcp4531_driver = {
 	.driver = {
 		.name	= "mcp4531",
+		.of_match_table = of_match_ptr(mcp45xx_of_match),
 	},
 	.probe		= mcp4531_probe,
 	.id_table	= mcp4531_id,
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1428456 — Re: [PATCH 3/3] iio: potentiometer: mcp4531: Add device tree binding

FromFlorian Vaussard <florian.vaussard@heig-vd.ch>
Date2016-06-22 08:30 +0200
SubjectRe: [PATCH 3/3] iio: potentiometer: mcp4531: Add device tree binding
Message-ID<rMJPH-8fv-15@gated-at.bofh.it>
In reply to#1427358
Hello Peter,

Le 21. 06. 16 à 09:51, Peter Rosin a écrit :
> On 2016-06-21 08:55, Florian Vaussard wrote:
>> This patch adds the necessary device tree binding to allow DT probing of
>> currently supported parts.
>>
>> Signed-off-by: Florian Vaussard <florian.vaussard@heig-vd.ch>
>> ---
>>  drivers/iio/potentiometer/mcp4531.c | 83 ++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 82 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/potentiometer/mcp4531.c b/drivers/iio/potentiometer/mcp4531.c
>> index 2251173..41a1e46 100644
>> --- a/drivers/iio/potentiometer/mcp4531.c
>> +++ b/drivers/iio/potentiometer/mcp4531.c
>> @@ -31,6 +31,8 @@
>>  #include <linux/module.h>
>>  #include <linux/i2c.h>
>>  #include <linux/err.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>>  
>>  #include <linux/iio/iio.h>
>>  
>> @@ -188,12 +190,84 @@ static const struct iio_info mcp4531_info = {
>>  	.driver_module = THIS_MODULE,
>>  };
>>  
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id mcp45xx_of_match[] = {
> 
> Should be named mcp4531_of_match, also the casting to void and
> then back to an integer type is ugly. Would it work to store a
> pointer directly into the mcp4531_cfg array instead? I.e.
> 
> 	{ .compatible = "microchip,mcp4531-502", .data = &mcp4531_cfg[MCP453x_502] },
> etc
> 
> and then adjust accordingly in mcp4531_probe()?
> 

Yes the casting is pretty ugly. And kbuild agrees with you :) I took this from
another driver (lame excuse). Your suggestion is sensible, I will give it a try.

> That is, if you need this patch at all, see my reply to 2/3...
> 

This seems necessary in order to have the vendor ID in the compatible string.

Thanks for your review.

Florian

[toc] | [prev] | [next] | [standalone]

#1429610 — Re: [PATCH 3/3] iio: potentiometer: mcp4531: Add device tree binding

FromFlorian Vaussard <florian.vaussard@heig-vd.ch>
Date2016-06-23 11:00 +0200
SubjectRe: [PATCH 3/3] iio: potentiometer: mcp4531: Add device tree binding
Message-ID<rN8Eq-7sH-33@gated-at.bofh.it>
In reply to#1428456
Hello Peter,

On 06/22/2016 09:06 AM, Peter Rosin wrote:
> On 2016-06-22 08:22, Florian Vaussard wrote:
>> Hello Peter,
>>
>> Le 21. 06. 16 à 09:51, Peter Rosin a écrit :
>>> That is, if you need this patch at all, see my reply to 2/3...
>>>
>>
>> This seems necessary in order to have the vendor ID in the compatible string.
> 
> Hmm, I don't think so. The way I read the response from Rob was that
> *my* device tree snippet should not assume that the i2c subsystem
> ignores the vendor. So, I think that even w/o this patch a DT entry
> like
> 
> 		mcp4651-104@28 {
> 			compatible = "microchip,mcp4651-104";
> 			reg = <0x28>;
> 		};
> 
> will work, precisely since i2c ignores the microchip, part (and thus
> allows you to omit/misspell it). I.e. I think that Rob is concerned
> with how the DT is documented/defined, and not so much about how it
> is then implemented in Linux.
> 

The way that I read Rob's reply is reverse: you should not rely on the matching
done by i2c subsystem and provide a proper of_device_id table instead.

I think that ignoring the vendor when matching the compatible, like what is
currently done by the i2c subsystem, is bad. What happens if you have two chips
from two companies with the same name (yes it happens)? You may need different
data to address the small specificities of each chip, or even worse a different
driver at all. This implies that we should explicitly match the vendor as well,
otherwise we would be unable to cope with such cases.

Moreover, if we look at the current i2c drivers, a whole bunch of them already
have a of_device_id table:

(on v4.6)

$ git grep -l i2c_device_id | wc -l
636
$ git grep -l i2c_device_id | xargs grep -l 'of_device_id' | wc -l
197

Currently 30% of all i2c drivers explicitly declare DT compatible strings. And I
am sure that this number will keep increasing as drivers are converted to DT.

Regards,
Florian

[toc] | [prev] | [next] | [standalone]

#1427362 — [PATCH 2/3] iio: potentiometer: mcp4531: Add device tree binding documentation

FromFlorian Vaussard <florian.vaussard@gmail.com>
Date2016-06-21 09:00 +0200
Subject[PATCH 2/3] iio: potentiometer: mcp4531: Add device tree binding documentation
Message-ID<rMnPb-2gH-15@gated-at.bofh.it>
In reply to#1427357
Add the device tree documentation for all the supported parts. Apart the
compatible string and standard I2C binding, no other binding is currently
needed.

Signed-off-by: Florian Vaussard <florian.vaussard@heig-vd.ch>
---
 .../bindings/iio/potentiometer/mcp4531.txt         | 84 ++++++++++++++++++++++
 1 file changed, 84 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt

diff --git a/Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt b/Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt
new file mode 100644
index 0000000..b052299
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt
@@ -0,0 +1,84 @@
+* Microchip MCP453X/454X/455X/456X/463X/464X/465X/466X Digital Potentiometer
+  driver
+
+The node for this driver must be a child node of a I2C controller, hence
+all mandatory properties for your controller must be specified. See directory:
+
+        Documentation/devicetree/bindings/i2c
+
+for more details.
+
+Required properties:
+	- compatible:  	Must be one of the following, depending on the
+			model:
+			"microchip,mcp4531-502"
+			"microchip,mcp4531-103"
+			"microchip,mcp4531-503"
+			"microchip,mcp4531-104"
+			"microchip,mcp4532-502"
+			"microchip,mcp4532-103"
+			"microchip,mcp4532-503"
+			"microchip,mcp4532-104"
+			"microchip,mcp4541-502"
+			"microchip,mcp4541-103"
+			"microchip,mcp4541-503"
+			"microchip,mcp4541-104"
+			"microchip,mcp4542-502"
+			"microchip,mcp4542-103"
+			"microchip,mcp4542-503"
+			"microchip,mcp4542-104"
+			"microchip,mcp4551-502"
+			"microchip,mcp4551-103"
+			"microchip,mcp4551-503"
+			"microchip,mcp4551-104"
+			"microchip,mcp4552-502"
+			"microchip,mcp4552-103"
+			"microchip,mcp4552-503"
+			"microchip,mcp4552-104"
+			"microchip,mcp4561-502"
+			"microchip,mcp4561-103"
+			"microchip,mcp4561-503"
+			"microchip,mcp4561-104"
+			"microchip,mcp4562-502"
+			"microchip,mcp4562-103"
+			"microchip,mcp4562-503"
+			"microchip,mcp4562-104"
+			"microchip,mcp4631-502"
+			"microchip,mcp4631-103"
+			"microchip,mcp4631-503"
+			"microchip,mcp4631-104"
+			"microchip,mcp4632-502"
+			"microchip,mcp4632-103"
+			"microchip,mcp4632-503"
+			"microchip,mcp4632-104"
+			"microchip,mcp4641-502"
+			"microchip,mcp4641-103"
+			"microchip,mcp4641-503"
+			"microchip,mcp4641-104"
+			"microchip,mcp4642-502"
+			"microchip,mcp4642-103"
+			"microchip,mcp4642-503"
+			"microchip,mcp4642-104"
+			"microchip,mcp4651-502"
+			"microchip,mcp4651-103"
+			"microchip,mcp4651-503"
+			"microchip,mcp4651-104"
+			"microchip,mcp4652-502"
+			"microchip,mcp4652-103"
+			"microchip,mcp4652-503"
+			"microchip,mcp4652-104"
+			"microchip,mcp4661-502"
+			"microchip,mcp4661-103"
+			"microchip,mcp4661-503"
+			"microchip,mcp4661-104"
+			"microchip,mcp4662-502"
+			"microchip,mcp4662-103"
+			"microchip,mcp4662-503"
+			"microchip,mcp4662-104"
+	-reg: Slave I2C address of the potentiometer
+
+Example:
+mcp4561: mcp4561@2e {
+	compatible = "microchip,mcp4561-103";
+	reg = <0x2e>;
+};
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1427440 — Re: [PATCH 2/3] iio: potentiometer: mcp4531: Add device tree binding documentation

FromFlorian Vaussard <florian.vaussard@heig-vd.ch>
Date2016-06-21 10:20 +0200
SubjectRe: [PATCH 2/3] iio: potentiometer: mcp4531: Add device tree binding documentation
Message-ID<rMp4B-3gc-1@gated-at.bofh.it>
In reply to#1427362
Hello,

On 06/21/2016 09:38 AM, Peter Rosin wrote:
> On 2016-06-21 08:55, Florian Vaussard wrote:
>> Add the device tree documentation for all the supported parts. Apart the
>> compatible string and standard I2C binding, no other binding is currently
>> needed.
>>
>> Signed-off-by: Florian Vaussard <florian.vaussard@heig-vd.ch>
>> ---
>>  .../bindings/iio/potentiometer/mcp4531.txt         | 84 ++++++++++++++++++++++
>>  1 file changed, 84 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt b/Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt
>> new file mode 100644
>> index 0000000..b052299
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt
>> @@ -0,0 +1,84 @@
>> +* Microchip MCP453X/454X/455X/456X/463X/464X/465X/466X Digital Potentiometer
>> +  driver
>> +
>> +The node for this driver must be a child node of a I2C controller, hence
>> +all mandatory properties for your controller must be specified. See directory:
>> +
>> +        Documentation/devicetree/bindings/i2c
>> +
>> +for more details.
>> +
>> +Required properties:
>> +	- compatible:  	Must be one of the following, depending on the
>> +			model:
>> +			"microchip,mcp4531-502"
>> +			"microchip,mcp4531-103"
>> +			"microchip,mcp4531-503"
> 
> *snip*
> 
> I'm not directly opposed, but I have used the following and DT booting
> works like a charm here.
> 
> 		mcp4651-104@28 {
> 			compatible = "mcp4651-104";
> 			reg = <0x28>;
> 		};
> 

I was not aware that the i2c subsystem had a facility to match the compatible
string against i2c_device_id. Good to know.

> But, I suppose some DT documentation is not bad, and my understanding of
> the device instantiation process and the i2c/dt interactions are not
> complete, so my DT snippet might be an abomination? I'll leave the
> decision if this is needed to someone with more experience on how other
> drivers handle this.
> 

Your compatible string is missing the vendor ID (microchip). This is the only
objection that I can see to your proposition. Let's wait to hear from a DT
maintainer.

Thanks,
Florian

[toc] | [prev] | [next] | [standalone]

#1428194 — Re: [PATCH 2/3] iio: potentiometer: mcp4531: Add device tree binding documentation

FromRob Herring <robh@kernel.org>
Date2016-06-21 23:40 +0200
SubjectRe: [PATCH 2/3] iio: potentiometer: mcp4531: Add device tree binding documentation
Message-ID<rMByN-2NM-3@gated-at.bofh.it>
In reply to#1427440
On Tue, Jun 21, 2016 at 09:49:20AM +0200, Florian Vaussard wrote:
> Hello,
> 
> On 06/21/2016 09:38 AM, Peter Rosin wrote:
> > On 2016-06-21 08:55, Florian Vaussard wrote:
> >> Add the device tree documentation for all the supported parts. Apart the
> >> compatible string and standard I2C binding, no other binding is currently
> >> needed.
> >>
> >> Signed-off-by: Florian Vaussard <florian.vaussard@heig-vd.ch>
> >> ---
> >>  .../bindings/iio/potentiometer/mcp4531.txt         | 84 ++++++++++++++++++++++
> >>  1 file changed, 84 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt b/Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt
> >> new file mode 100644
> >> index 0000000..b052299
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt
> >> @@ -0,0 +1,84 @@
> >> +* Microchip MCP453X/454X/455X/456X/463X/464X/465X/466X Digital Potentiometer
> >> +  driver
> >> +
> >> +The node for this driver must be a child node of a I2C controller, hence
> >> +all mandatory properties for your controller must be specified. See directory:
> >> +
> >> +        Documentation/devicetree/bindings/i2c
> >> +
> >> +for more details.
> >> +
> >> +Required properties:
> >> +	- compatible:  	Must be one of the following, depending on the
> >> +			model:
> >> +			"microchip,mcp4531-502"
> >> +			"microchip,mcp4531-103"
> >> +			"microchip,mcp4531-503"
> > 
> > *snip*
> > 
> > I'm not directly opposed, but I have used the following and DT booting
> > works like a charm here.
> > 
> > 		mcp4651-104@28 {
> > 			compatible = "mcp4651-104";
> > 			reg = <0x28>;
> > 		};
> > 
> 
> I was not aware that the i2c subsystem had a facility to match the compatible
> string against i2c_device_id. Good to know.
> 
> > But, I suppose some DT documentation is not bad, and my understanding of
> > the device instantiation process and the i2c/dt interactions are not
> > complete, so my DT snippet might be an abomination? I'll leave the
> > decision if this is needed to someone with more experience on how other
> > drivers handle this.
> > 
> 
> Your compatible string is missing the vendor ID (microchip). This is the only
> objection that I can see to your proposition. Let's wait to hear from a DT
> maintainer.

I2C subsystem happens to ignore the vendor prefix in matching. You 
should not rely on that and provide proper compatible strings.

Rob

[toc] | [prev] | [next] | [standalone]

#1428211 — Re: [PATCH 2/3] iio: potentiometer: mcp4531: Add device tree binding documentation

FromRob Herring <robh@kernel.org>
Date2016-06-21 23:50 +0200
SubjectRe: [PATCH 2/3] iio: potentiometer: mcp4531: Add device tree binding documentation
Message-ID<rMBIu-2Rm-15@gated-at.bofh.it>
In reply to#1427362
On Tue, Jun 21, 2016 at 08:55:36AM +0200, Florian Vaussard wrote:
> Add the device tree documentation for all the supported parts. Apart the
> compatible string and standard I2C binding, no other binding is currently
> needed.
> 
> Signed-off-by: Florian Vaussard <florian.vaussard@heig-vd.ch>
> ---
>  .../bindings/iio/potentiometer/mcp4531.txt         | 84 ++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt b/Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt
> new file mode 100644
> index 0000000..b052299
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt
> @@ -0,0 +1,84 @@
> +* Microchip MCP453X/454X/455X/456X/463X/464X/465X/466X Digital Potentiometer
> +  driver
> +
> +The node for this driver must be a child node of a I2C controller, hence
> +all mandatory properties for your controller must be specified. See directory:
> +
> +        Documentation/devicetree/bindings/i2c
> +
> +for more details.
> +
> +Required properties:
> +	- compatible:  	Must be one of the following, depending on the
> +			model:
> +			"microchip,mcp4531-502"

These can go in trivial-devices.txt

> +			"microchip,mcp4531-103"
> +			"microchip,mcp4531-503"
> +			"microchip,mcp4531-104"
> +			"microchip,mcp4532-502"
> +			"microchip,mcp4532-103"
> +			"microchip,mcp4532-503"
> +			"microchip,mcp4532-104"
> +			"microchip,mcp4541-502"
> +			"microchip,mcp4541-103"
> +			"microchip,mcp4541-503"
> +			"microchip,mcp4541-104"
> +			"microchip,mcp4542-502"
> +			"microchip,mcp4542-103"
> +			"microchip,mcp4542-503"
> +			"microchip,mcp4542-104"
> +			"microchip,mcp4551-502"
> +			"microchip,mcp4551-103"
> +			"microchip,mcp4551-503"
> +			"microchip,mcp4551-104"
> +			"microchip,mcp4552-502"
> +			"microchip,mcp4552-103"
> +			"microchip,mcp4552-503"
> +			"microchip,mcp4552-104"
> +			"microchip,mcp4561-502"
> +			"microchip,mcp4561-103"
> +			"microchip,mcp4561-503"
> +			"microchip,mcp4561-104"
> +			"microchip,mcp4562-502"
> +			"microchip,mcp4562-103"
> +			"microchip,mcp4562-503"
> +			"microchip,mcp4562-104"
> +			"microchip,mcp4631-502"
> +			"microchip,mcp4631-103"
> +			"microchip,mcp4631-503"
> +			"microchip,mcp4631-104"
> +			"microchip,mcp4632-502"
> +			"microchip,mcp4632-103"
> +			"microchip,mcp4632-503"
> +			"microchip,mcp4632-104"
> +			"microchip,mcp4641-502"
> +			"microchip,mcp4641-103"
> +			"microchip,mcp4641-503"
> +			"microchip,mcp4641-104"
> +			"microchip,mcp4642-502"
> +			"microchip,mcp4642-103"
> +			"microchip,mcp4642-503"
> +			"microchip,mcp4642-104"
> +			"microchip,mcp4651-502"
> +			"microchip,mcp4651-103"
> +			"microchip,mcp4651-503"
> +			"microchip,mcp4651-104"
> +			"microchip,mcp4652-502"
> +			"microchip,mcp4652-103"
> +			"microchip,mcp4652-503"
> +			"microchip,mcp4652-104"
> +			"microchip,mcp4661-502"
> +			"microchip,mcp4661-103"
> +			"microchip,mcp4661-503"
> +			"microchip,mcp4661-104"
> +			"microchip,mcp4662-502"
> +			"microchip,mcp4662-103"
> +			"microchip,mcp4662-503"
> +			"microchip,mcp4662-104"
> +	-reg: Slave I2C address of the potentiometer
> +
> +Example:
> +mcp4561: mcp4561@2e {
> +	compatible = "microchip,mcp4561-103";
> +	reg = <0x2e>;
> +};
> -- 
> 2.5.0
>

[toc] | [prev] | [next] | [standalone]

#1428452 — Re: [PATCH 2/3] iio: potentiometer: mcp4531: Add device tree binding documentation

FromFlorian Vaussard <florian.vaussard@heig-vd.ch>
Date2016-06-22 08:20 +0200
SubjectRe: [PATCH 2/3] iio: potentiometer: mcp4531: Add device tree binding documentation
Message-ID<rMJG1-8cc-17@gated-at.bofh.it>
In reply to#1428211
Hi Rob,

Le 21. 06. 16 à 23:43, Rob Herring a écrit :
> On Tue, Jun 21, 2016 at 08:55:36AM +0200, Florian Vaussard wrote:
>> Add the device tree documentation for all the supported parts. Apart the
>> compatible string and standard I2C binding, no other binding is currently
>> needed.
>>
>> Signed-off-by: Florian Vaussard <florian.vaussard@heig-vd.ch>
>> ---
>>  .../bindings/iio/potentiometer/mcp4531.txt         | 84 ++++++++++++++++++++++
>>  1 file changed, 84 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt b/Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt
>> new file mode 100644
>> index 0000000..b052299
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/potentiometer/mcp4531.txt
>> @@ -0,0 +1,84 @@
>> +* Microchip MCP453X/454X/455X/456X/463X/464X/465X/466X Digital Potentiometer
>> +  driver
>> +
>> +The node for this driver must be a child node of a I2C controller, hence
>> +all mandatory properties for your controller must be specified. See directory:
>> +
>> +        Documentation/devicetree/bindings/i2c
>> +
>> +for more details.
>> +
>> +Required properties:
>> +	- compatible:  	Must be one of the following, depending on the
>> +			model:
>> +			"microchip,mcp4531-502"
> 
> These can go in trivial-devices.txt
> 

Yes indeed. I will fix.

Thanks for the review.

Florian

>> +			"microchip,mcp4531-103"
>> +			"microchip,mcp4531-503"
>> +			"microchip,mcp4531-104"
>> +			"microchip,mcp4532-502"
>> +			"microchip,mcp4532-103"
>> +			"microchip,mcp4532-503"
>> +			"microchip,mcp4532-104"
>> +			"microchip,mcp4541-502"
>> +			"microchip,mcp4541-103"
>> +			"microchip,mcp4541-503"
>> +			"microchip,mcp4541-104"
>> +			"microchip,mcp4542-502"
>> +			"microchip,mcp4542-103"
>> +			"microchip,mcp4542-503"
>> +			"microchip,mcp4542-104"
>> +			"microchip,mcp4551-502"
>> +			"microchip,mcp4551-103"
>> +			"microchip,mcp4551-503"
>> +			"microchip,mcp4551-104"
>> +			"microchip,mcp4552-502"
>> +			"microchip,mcp4552-103"
>> +			"microchip,mcp4552-503"
>> +			"microchip,mcp4552-104"
>> +			"microchip,mcp4561-502"
>> +			"microchip,mcp4561-103"
>> +			"microchip,mcp4561-503"
>> +			"microchip,mcp4561-104"
>> +			"microchip,mcp4562-502"
>> +			"microchip,mcp4562-103"
>> +			"microchip,mcp4562-503"
>> +			"microchip,mcp4562-104"
>> +			"microchip,mcp4631-502"
>> +			"microchip,mcp4631-103"
>> +			"microchip,mcp4631-503"
>> +			"microchip,mcp4631-104"
>> +			"microchip,mcp4632-502"
>> +			"microchip,mcp4632-103"
>> +			"microchip,mcp4632-503"
>> +			"microchip,mcp4632-104"
>> +			"microchip,mcp4641-502"
>> +			"microchip,mcp4641-103"
>> +			"microchip,mcp4641-503"
>> +			"microchip,mcp4641-104"
>> +			"microchip,mcp4642-502"
>> +			"microchip,mcp4642-103"
>> +			"microchip,mcp4642-503"
>> +			"microchip,mcp4642-104"
>> +			"microchip,mcp4651-502"
>> +			"microchip,mcp4651-103"
>> +			"microchip,mcp4651-503"
>> +			"microchip,mcp4651-104"
>> +			"microchip,mcp4652-502"
>> +			"microchip,mcp4652-103"
>> +			"microchip,mcp4652-503"
>> +			"microchip,mcp4652-104"
>> +			"microchip,mcp4661-502"
>> +			"microchip,mcp4661-103"
>> +			"microchip,mcp4661-503"
>> +			"microchip,mcp4661-104"
>> +			"microchip,mcp4662-502"
>> +			"microchip,mcp4662-103"
>> +			"microchip,mcp4662-503"
>> +			"microchip,mcp4662-104"
>> +	-reg: Slave I2C address of the potentiometer
>> +
>> +Example:
>> +mcp4561: mcp4561@2e {
>> +	compatible = "microchip,mcp4561-103";
>> +	reg = <0x2e>;
>> +};
>> -- 
>> 2.5.0
>>

[toc] | [prev] | [next] | [standalone]

#1427363 — [PATCH 1/3] iio: potentiometer: mcp4531: Add support for MCP454x, MCP456x, MCP464x and MCP466x

FromFlorian Vaussard <florian.vaussard@gmail.com>
Date2016-06-21 09:00 +0200
Subject[PATCH 1/3] iio: potentiometer: mcp4531: Add support for MCP454x, MCP456x, MCP464x and MCP466x
Message-ID<rMnPb-2gH-17@gated-at.bofh.it>
In reply to#1427357
This patch adds support for MCP454x, MCP456x, MCP464x and MCP466x parts.
The main difference with currently supported parts (MCP453x and alike) is
the addition of a non-volatile memory in order to recall the wiper setting
at power-on. This feature is currently not supported and only the
volatile memory is used to set the wiper.

Signed-off-by: Florian Vaussard <florian.vaussard@heig-vd.ch>
---
 drivers/iio/potentiometer/Kconfig   |  6 ++--
 drivers/iio/potentiometer/mcp4531.c | 72 +++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/potentiometer/Kconfig b/drivers/iio/potentiometer/Kconfig
index 0941c8d4..55c2414 100644
--- a/drivers/iio/potentiometer/Kconfig
+++ b/drivers/iio/potentiometer/Kconfig
@@ -49,8 +49,10 @@ config MCP4531
 	depends on I2C
 	help
 	  Say yes here to build support for the Microchip
-	  MCP4531, MCP4532, MCP4551, MCP4552,
-	  MCP4631, MCP4632, MCP4651, MCP4652
+	  MCP4531, MCP4532, MCP4541, MCP4542,
+	  MCP4551, MCP4552, MCP4561, MCP4562,
+	  MCP4631, MCP4632, MCP4641, MCP4642,
+	  MCP4651, MCP4652, MCP4661, MCP4662
 	  digital potentiomenter chips.
 
 	  To compile this driver as a module, choose M here: the
diff --git a/drivers/iio/potentiometer/mcp4531.c b/drivers/iio/potentiometer/mcp4531.c
index 3b72e1a..2251173 100644
--- a/drivers/iio/potentiometer/mcp4531.c
+++ b/drivers/iio/potentiometer/mcp4531.c
@@ -8,12 +8,20 @@
  * DEVID	#Wipers	#Positions	Resistor Opts (kOhm)	i2c address
  * mcp4531	1	129		5, 10, 50, 100          010111x
  * mcp4532	1	129		5, 10, 50, 100          01011xx
+ * mcp4541	1	129		5, 10, 50, 100          010111x
+ * mcp4542	1	129		5, 10, 50, 100          01011xx
  * mcp4551	1	257		5, 10, 50, 100          010111x
  * mcp4552	1	257		5, 10, 50, 100          01011xx
+ * mcp4561	1	257		5, 10, 50, 100          010111x
+ * mcp4562	1	257		5, 10, 50, 100          01011xx
  * mcp4631	2	129		5, 10, 50, 100          0101xxx
  * mcp4632	2	129		5, 10, 50, 100          01011xx
+ * mcp4641	2	129		5, 10, 50, 100          0101xxx
+ * mcp4642	2	129		5, 10, 50, 100          01011xx
  * mcp4651	2	257		5, 10, 50, 100          0101xxx
  * mcp4652	2	257		5, 10, 50, 100          01011xx
+ * mcp4661	2	257		5, 10, 50, 100          0101xxx
+ * mcp4662	2	257		5, 10, 50, 100          01011xx
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License version 2 as published by
@@ -37,18 +45,34 @@ enum mcp4531_type {
 	MCP453x_103,
 	MCP453x_503,
 	MCP453x_104,
+	MCP454x_502,
+	MCP454x_103,
+	MCP454x_503,
+	MCP454x_104,
 	MCP455x_502,
 	MCP455x_103,
 	MCP455x_503,
 	MCP455x_104,
+	MCP456x_502,
+	MCP456x_103,
+	MCP456x_503,
+	MCP456x_104,
 	MCP463x_502,
 	MCP463x_103,
 	MCP463x_503,
 	MCP463x_104,
+	MCP464x_502,
+	MCP464x_103,
+	MCP464x_503,
+	MCP464x_104,
 	MCP465x_502,
 	MCP465x_103,
 	MCP465x_503,
 	MCP465x_104,
+	MCP466x_502,
+	MCP466x_103,
+	MCP466x_503,
+	MCP466x_104,
 };
 
 static const struct mcp4531_cfg mcp4531_cfg[] = {
@@ -56,18 +80,34 @@ static const struct mcp4531_cfg mcp4531_cfg[] = {
 	[MCP453x_103] = { .wipers = 1, .max_pos = 128, .kohms =  10, },
 	[MCP453x_503] = { .wipers = 1, .max_pos = 128, .kohms =  50, },
 	[MCP453x_104] = { .wipers = 1, .max_pos = 128, .kohms = 100, },
+	[MCP454x_502] = { .wipers = 1, .max_pos = 128, .kohms =   5, },
+	[MCP454x_103] = { .wipers = 1, .max_pos = 128, .kohms =  10, },
+	[MCP454x_503] = { .wipers = 1, .max_pos = 128, .kohms =  50, },
+	[MCP454x_104] = { .wipers = 1, .max_pos = 128, .kohms = 100, },
 	[MCP455x_502] = { .wipers = 1, .max_pos = 256, .kohms =   5, },
 	[MCP455x_103] = { .wipers = 1, .max_pos = 256, .kohms =  10, },
 	[MCP455x_503] = { .wipers = 1, .max_pos = 256, .kohms =  50, },
 	[MCP455x_104] = { .wipers = 1, .max_pos = 256, .kohms = 100, },
+	[MCP456x_502] = { .wipers = 1, .max_pos = 256, .kohms =   5, },
+	[MCP456x_103] = { .wipers = 1, .max_pos = 256, .kohms =  10, },
+	[MCP456x_503] = { .wipers = 1, .max_pos = 256, .kohms =  50, },
+	[MCP456x_104] = { .wipers = 1, .max_pos = 256, .kohms = 100, },
 	[MCP463x_502] = { .wipers = 2, .max_pos = 128, .kohms =   5, },
 	[MCP463x_103] = { .wipers = 2, .max_pos = 128, .kohms =  10, },
 	[MCP463x_503] = { .wipers = 2, .max_pos = 128, .kohms =  50, },
 	[MCP463x_104] = { .wipers = 2, .max_pos = 128, .kohms = 100, },
+	[MCP464x_502] = { .wipers = 2, .max_pos = 128, .kohms =   5, },
+	[MCP464x_103] = { .wipers = 2, .max_pos = 128, .kohms =  10, },
+	[MCP464x_503] = { .wipers = 2, .max_pos = 128, .kohms =  50, },
+	[MCP464x_104] = { .wipers = 2, .max_pos = 128, .kohms = 100, },
 	[MCP465x_502] = { .wipers = 2, .max_pos = 256, .kohms =   5, },
 	[MCP465x_103] = { .wipers = 2, .max_pos = 256, .kohms =  10, },
 	[MCP465x_503] = { .wipers = 2, .max_pos = 256, .kohms =  50, },
 	[MCP465x_104] = { .wipers = 2, .max_pos = 256, .kohms = 100, },
+	[MCP466x_502] = { .wipers = 2, .max_pos = 256, .kohms =   5, },
+	[MCP466x_103] = { .wipers = 2, .max_pos = 256, .kohms =  10, },
+	[MCP466x_503] = { .wipers = 2, .max_pos = 256, .kohms =  50, },
+	[MCP466x_104] = { .wipers = 2, .max_pos = 256, .kohms = 100, },
 };
 
 #define MCP4531_WRITE (0 << 2)
@@ -187,6 +227,14 @@ static const struct i2c_device_id mcp4531_id[] = {
 	{ "mcp4532-103", MCP453x_103 },
 	{ "mcp4532-503", MCP453x_503 },
 	{ "mcp4532-104", MCP453x_104 },
+	{ "mcp4541-502", MCP454x_502 },
+	{ "mcp4541-103", MCP454x_103 },
+	{ "mcp4541-503", MCP454x_503 },
+	{ "mcp4541-104", MCP454x_104 },
+	{ "mcp4542-502", MCP454x_502 },
+	{ "mcp4542-103", MCP454x_103 },
+	{ "mcp4542-503", MCP454x_503 },
+	{ "mcp4542-104", MCP454x_104 },
 	{ "mcp4551-502", MCP455x_502 },
 	{ "mcp4551-103", MCP455x_103 },
 	{ "mcp4551-503", MCP455x_503 },
@@ -195,6 +243,14 @@ static const struct i2c_device_id mcp4531_id[] = {
 	{ "mcp4552-103", MCP455x_103 },
 	{ "mcp4552-503", MCP455x_503 },
 	{ "mcp4552-104", MCP455x_104 },
+	{ "mcp4561-502", MCP456x_502 },
+	{ "mcp4561-103", MCP456x_103 },
+	{ "mcp4561-503", MCP456x_503 },
+	{ "mcp4561-104", MCP456x_104 },
+	{ "mcp4562-502", MCP456x_502 },
+	{ "mcp4562-103", MCP456x_103 },
+	{ "mcp4562-503", MCP456x_503 },
+	{ "mcp4562-104", MCP456x_104 },
 	{ "mcp4631-502", MCP463x_502 },
 	{ "mcp4631-103", MCP463x_103 },
 	{ "mcp4631-503", MCP463x_503 },
@@ -203,6 +259,14 @@ static const struct i2c_device_id mcp4531_id[] = {
 	{ "mcp4632-103", MCP463x_103 },
 	{ "mcp4632-503", MCP463x_503 },
 	{ "mcp4632-104", MCP463x_104 },
+	{ "mcp4641-502", MCP464x_502 },
+	{ "mcp4641-103", MCP464x_103 },
+	{ "mcp4641-503", MCP464x_503 },
+	{ "mcp4641-104", MCP464x_104 },
+	{ "mcp4642-502", MCP464x_502 },
+	{ "mcp4642-103", MCP464x_103 },
+	{ "mcp4642-503", MCP464x_503 },
+	{ "mcp4642-104", MCP464x_104 },
 	{ "mcp4651-502", MCP465x_502 },
 	{ "mcp4651-103", MCP465x_103 },
 	{ "mcp4651-503", MCP465x_503 },
@@ -211,6 +275,14 @@ static const struct i2c_device_id mcp4531_id[] = {
 	{ "mcp4652-103", MCP465x_103 },
 	{ "mcp4652-503", MCP465x_503 },
 	{ "mcp4652-104", MCP465x_104 },
+	{ "mcp4661-502", MCP466x_502 },
+	{ "mcp4661-103", MCP466x_103 },
+	{ "mcp4661-503", MCP466x_503 },
+	{ "mcp4661-104", MCP466x_104 },
+	{ "mcp4662-502", MCP466x_502 },
+	{ "mcp4662-103", MCP466x_103 },
+	{ "mcp4662-503", MCP466x_503 },
+	{ "mcp4662-104", MCP466x_104 },
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, mcp4531_id);
-- 
2.5.0

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web