linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/7] iio: potentiometer: Add support for DS3502
@ 2022-02-23 16:35 Jagath Jog J
  2022-02-23 16:35 ` [PATCH v5 1/7] iio: potentiometer: Alignment to match the open parenthesis Jagath Jog J
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Jagath Jog J @ 2022-02-23 16:35 UTC (permalink / raw)
  To: jic23, lars, andy.shevchenko, sst, robh+dt
  Cc: linux-iio, devicetree, linux-kernel

Add dt-bindings and support for Maxim DS3502 into existing ds1803 driver.
DS3502 is a 7 bit Nonvolatile Digital Potentiometer.

Changes since v4:
1. Included property.h header which has device_get_match_data()
   function prototype.
2. Removed blank space in tag block of the commit message.
3. Style changes for ds1803_cfg structure.

Changes since v3:
1. Dropped the chip type switch statement in read_raw function.
2. Added device specific read function pointer in their structure.
3. Added two separate functions to read values from two different types
   of devices.

Changes since v2:
1. Addressed Andy Shevchenko comments.
2. Adding device name in Kconfig file.
3. Spliting up of patch into 3 patches.
4. Adding channel info into ds1803_cfg in separate patch.
5. Dropping the use of enum in firmware data instead using previous
   pointer method for accessing device specific data.
6. Separate patch for using firmware provided data instead of 
   id->driver_data.
7. Adding DS3502 support in separate patch.

Changes since v1:
1. Fixes the alignment to match the open parenthesis in separate patch.
2. Adding available functionality for ds1803 driver in separate patch.
3. Moving maxim_potentiometer members into ds1803_cfg structure.
4. Droping of the INFO_ENABLE channel type.
5. Firmware entry with data is used instead of id->driver_data to
   to retrieve the chip specific data.

Jagath Jog J (7):
  iio: potentiometer: Alignment to match the open parenthesis
  iio: potentiometer: Add available functionality
  iio: potentiometer: Add channel information in device data
  iio: potentiometer: Change to firmware provided data
  iio: potentiometer: Add device specific read_raw function
  iio: potentiometer: Add support for Maxim DS3502
  dt-bindings: iio: potentiometer: Add Maxim DS3502 in trivial-devices

 .../devicetree/bindings/trivial-devices.yaml  |   2 +
 drivers/iio/potentiometer/Kconfig             |   6 +-
 drivers/iio/potentiometer/ds1803.c            | 170 ++++++++++++++----
 3 files changed, 138 insertions(+), 40 deletions(-)

-- 
2.17.1


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

* [PATCH v5 1/7] iio: potentiometer: Alignment to match the open parenthesis
  2022-02-23 16:35 [PATCH v5 0/7] iio: potentiometer: Add support for DS3502 Jagath Jog J
@ 2022-02-23 16:35 ` Jagath Jog J
  2022-02-26 18:35   ` Jonathan Cameron
  2022-02-23 16:35 ` [PATCH v5 2/7] iio: potentiometer: Add available functionality Jagath Jog J
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Jagath Jog J @ 2022-02-23 16:35 UTC (permalink / raw)
  To: jic23, lars, andy.shevchenko, sst, robh+dt
  Cc: linux-iio, devicetree, linux-kernel

Fix following checkpatch.pl check by removing blank space.
CHECK: Alignment should match open parenthesis.

Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
---
 drivers/iio/potentiometer/ds1803.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/potentiometer/ds1803.c b/drivers/iio/potentiometer/ds1803.c
index 20b45407eaac..3c542a50ece6 100644
--- a/drivers/iio/potentiometer/ds1803.c
+++ b/drivers/iio/potentiometer/ds1803.c
@@ -55,8 +55,8 @@ static const struct iio_chan_spec ds1803_channels[] = {
 };
 
 static int ds1803_read_raw(struct iio_dev *indio_dev,
-			    struct iio_chan_spec const *chan,
-			    int *val, int *val2, long mask)
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
 {
 	struct ds1803_data *data = iio_priv(indio_dev);
 	int pot = chan->channel;
@@ -66,7 +66,7 @@ static int ds1803_read_raw(struct iio_dev *indio_dev,
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		ret = i2c_master_recv(data->client, result,
-				indio_dev->num_channels);
+				      indio_dev->num_channels);
 		if (ret < 0)
 			return ret;
 
@@ -83,8 +83,8 @@ static int ds1803_read_raw(struct iio_dev *indio_dev,
 }
 
 static int ds1803_write_raw(struct iio_dev *indio_dev,
-			     struct iio_chan_spec const *chan,
-			     int val, int val2, long mask)
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
 {
 	struct ds1803_data *data = iio_priv(indio_dev);
 	int pot = chan->channel;
@@ -109,8 +109,7 @@ static const struct iio_info ds1803_info = {
 	.write_raw = ds1803_write_raw,
 };
 
-static int ds1803_probe(struct i2c_client *client,
-			 const struct i2c_device_id *id)
+static int ds1803_probe(struct i2c_client *client, const struct i2c_device_id *id)
 {
 	struct device *dev = &client->dev;
 	struct ds1803_data *data;
-- 
2.17.1


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

* [PATCH v5 2/7] iio: potentiometer: Add available functionality
  2022-02-23 16:35 [PATCH v5 0/7] iio: potentiometer: Add support for DS3502 Jagath Jog J
  2022-02-23 16:35 ` [PATCH v5 1/7] iio: potentiometer: Alignment to match the open parenthesis Jagath Jog J
@ 2022-02-23 16:35 ` Jagath Jog J
  2022-02-26 18:40   ` Jonathan Cameron
  2022-02-23 16:35 ` [PATCH v5 3/7] iio: potentiometer: Add channel information in device data Jagath Jog J
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Jagath Jog J @ 2022-02-23 16:35 UTC (permalink / raw)
  To: jic23, lars, andy.shevchenko, sst, robh+dt
  Cc: linux-iio, devicetree, linux-kernel

Adding available functionality for DS1803 driver which
will show the minimum, step and maximum values that the
driver can accepts through sysfs entry.
Now using the max value present in avail array instead of chip
type specific macro to make the driver flexible to add other
type of potentiometer with different max position value.

Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
---
 drivers/iio/potentiometer/ds1803.c | 61 ++++++++++++++++++++++--------
 1 file changed, 45 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/potentiometer/ds1803.c b/drivers/iio/potentiometer/ds1803.c
index 3c542a50ece6..9610269bed7f 100644
--- a/drivers/iio/potentiometer/ds1803.c
+++ b/drivers/iio/potentiometer/ds1803.c
@@ -16,7 +16,6 @@
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
 
-#define DS1803_MAX_POS		255
 #define DS1803_WRITE(chan)	(0xa8 | ((chan) + 1))
 
 enum ds1803_type {
@@ -26,27 +25,23 @@ enum ds1803_type {
 };
 
 struct ds1803_cfg {
+	int avail[3];
 	int kohms;
 };
 
-static const struct ds1803_cfg ds1803_cfg[] = {
-	[DS1803_010] = { .kohms =  10, },
-	[DS1803_050] = { .kohms =  50, },
-	[DS1803_100] = { .kohms = 100, },
-};
-
 struct ds1803_data {
 	struct i2c_client *client;
 	const struct ds1803_cfg *cfg;
 };
 
-#define DS1803_CHANNEL(ch) {					\
-	.type = IIO_RESISTANCE,					\
-	.indexed = 1,						\
-	.output = 1,						\
-	.channel = (ch),					\
-	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
-	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
+#define DS1803_CHANNEL(ch) {						\
+	.type = IIO_RESISTANCE,						\
+	.indexed = 1,							\
+	.output = 1,							\
+	.channel = (ch),						\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
+	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_RAW),   \
 }
 
 static const struct iio_chan_spec ds1803_channels[] = {
@@ -54,6 +49,21 @@ static const struct iio_chan_spec ds1803_channels[] = {
 	DS1803_CHANNEL(1),
 };
 
+static const struct ds1803_cfg ds1803_cfg[] = {
+	[DS1803_010] = {
+	  .avail = { 0, 1, 255 },
+	  .kohms =  10,
+	},
+	[DS1803_050] = {
+	  .avail = { 0, 1, 255 },
+	  .kohms =  50,
+	},
+	[DS1803_100] = {
+	  .avail = { 0, 1, 255 },
+	  .kohms = 100,
+	},
+};
+
 static int ds1803_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val, int *val2, long mask)
@@ -75,7 +85,7 @@ static int ds1803_read_raw(struct iio_dev *indio_dev,
 
 	case IIO_CHAN_INFO_SCALE:
 		*val = 1000 * data->cfg->kohms;
-		*val2 = DS1803_MAX_POS;
+		*val2 = data->cfg->avail[2]; /* Max wiper position */
 		return IIO_VAL_FRACTIONAL;
 	}
 
@@ -88,13 +98,14 @@ static int ds1803_write_raw(struct iio_dev *indio_dev,
 {
 	struct ds1803_data *data = iio_priv(indio_dev);
 	int pot = chan->channel;
+	int max_pos = data->cfg->avail[2];
 
 	if (val2 != 0)
 		return -EINVAL;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		if (val > DS1803_MAX_POS || val < 0)
+		if (val > max_pos || val < 0)
 			return -EINVAL;
 		break;
 	default:
@@ -104,9 +115,27 @@ static int ds1803_write_raw(struct iio_dev *indio_dev,
 	return i2c_smbus_write_byte_data(data->client, DS1803_WRITE(pot), val);
 }
 
+static int ds1803_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type,
+			     int *length, long mask)
+{
+	struct ds1803_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		*vals = data->cfg->avail;
+		*length = ARRAY_SIZE(data->cfg->avail);
+		*type = IIO_VAL_INT;
+		return IIO_AVAIL_RANGE;
+	}
+	return -EINVAL;
+}
+
 static const struct iio_info ds1803_info = {
 	.read_raw = ds1803_read_raw,
 	.write_raw = ds1803_write_raw,
+	.read_avail = ds1803_read_avail,
 };
 
 static int ds1803_probe(struct i2c_client *client, const struct i2c_device_id *id)
-- 
2.17.1


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

* [PATCH v5 3/7] iio: potentiometer: Add channel information in device data
  2022-02-23 16:35 [PATCH v5 0/7] iio: potentiometer: Add support for DS3502 Jagath Jog J
  2022-02-23 16:35 ` [PATCH v5 1/7] iio: potentiometer: Alignment to match the open parenthesis Jagath Jog J
  2022-02-23 16:35 ` [PATCH v5 2/7] iio: potentiometer: Add available functionality Jagath Jog J
@ 2022-02-23 16:35 ` Jagath Jog J
  2022-02-23 16:35 ` [PATCH v5 4/7] iio: potentiometer: Change to firmware provided data Jagath Jog J
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Jagath Jog J @ 2022-02-23 16:35 UTC (permalink / raw)
  To: jic23, lars, andy.shevchenko, sst, robh+dt
  Cc: linux-iio, devicetree, linux-kernel

Adding each device wiper count and channel information into
device private data.
Utilizing addr member of struct iio_chan_spec to get the
wiper register address so that the value can be read or write
to the same address.

Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
---
 drivers/iio/potentiometer/ds1803.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/potentiometer/ds1803.c b/drivers/iio/potentiometer/ds1803.c
index 9610269bed7f..f93cd342e09a 100644
--- a/drivers/iio/potentiometer/ds1803.c
+++ b/drivers/iio/potentiometer/ds1803.c
@@ -16,7 +16,8 @@
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
 
-#define DS1803_WRITE(chan)	(0xa8 | ((chan) + 1))
+#define DS1803_WIPER_0         0xA9
+#define DS1803_WIPER_1         0xAA
 
 enum ds1803_type {
 	DS1803_010,
@@ -25,8 +26,11 @@ enum ds1803_type {
 };
 
 struct ds1803_cfg {
+	int wipers;
 	int avail[3];
 	int kohms;
+	const struct iio_chan_spec *channels;
+	u8 num_channels;
 };
 
 struct ds1803_data {
@@ -34,33 +38,43 @@ struct ds1803_data {
 	const struct ds1803_cfg *cfg;
 };
 
-#define DS1803_CHANNEL(ch) {						\
+#define DS1803_CHANNEL(ch, addr) {					\
 	.type = IIO_RESISTANCE,						\
 	.indexed = 1,							\
 	.output = 1,							\
 	.channel = (ch),						\
+	.address = (addr),						\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
 	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
 	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_RAW),   \
 }
 
 static const struct iio_chan_spec ds1803_channels[] = {
-	DS1803_CHANNEL(0),
-	DS1803_CHANNEL(1),
+	DS1803_CHANNEL(0, DS1803_WIPER_0),
+	DS1803_CHANNEL(1, DS1803_WIPER_1),
 };
 
 static const struct ds1803_cfg ds1803_cfg[] = {
 	[DS1803_010] = {
+	  .wipers = 2,
 	  .avail = { 0, 1, 255 },
 	  .kohms =  10,
+	  .channels = ds1803_channels,
+	  .num_channels = ARRAY_SIZE(ds1803_channels),
 	},
 	[DS1803_050] = {
+	  .wipers = 2,
 	  .avail = { 0, 1, 255 },
 	  .kohms =  50,
+	  .channels = ds1803_channels,
+	  .num_channels = ARRAY_SIZE(ds1803_channels),
 	},
 	[DS1803_100] = {
+	  .wipers = 2,
 	  .avail = { 0, 1, 255 },
 	  .kohms = 100,
+	  .channels = ds1803_channels,
+	  .num_channels = ARRAY_SIZE(ds1803_channels),
 	},
 };
 
@@ -97,7 +111,7 @@ static int ds1803_write_raw(struct iio_dev *indio_dev,
 			    int val, int val2, long mask)
 {
 	struct ds1803_data *data = iio_priv(indio_dev);
-	int pot = chan->channel;
+	u8 addr = chan->address;
 	int max_pos = data->cfg->avail[2];
 
 	if (val2 != 0)
@@ -112,7 +126,7 @@ static int ds1803_write_raw(struct iio_dev *indio_dev,
 		return -EINVAL;
 	}
 
-	return i2c_smbus_write_byte_data(data->client, DS1803_WRITE(pot), val);
+	return i2c_smbus_write_byte_data(data->client, addr, val);
 }
 
 static int ds1803_read_avail(struct iio_dev *indio_dev,
@@ -155,8 +169,8 @@ static int ds1803_probe(struct i2c_client *client, const struct i2c_device_id *i
 	data->cfg = &ds1803_cfg[id->driver_data];
 
 	indio_dev->info = &ds1803_info;
-	indio_dev->channels = ds1803_channels;
-	indio_dev->num_channels = ARRAY_SIZE(ds1803_channels);
+	indio_dev->channels = data->cfg->channels;
+	indio_dev->num_channels = data->cfg->num_channels;
 	indio_dev->name = client->name;
 
 	return devm_iio_device_register(dev, indio_dev);
-- 
2.17.1


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

* [PATCH v5 4/7] iio: potentiometer: Change to firmware provided data
  2022-02-23 16:35 [PATCH v5 0/7] iio: potentiometer: Add support for DS3502 Jagath Jog J
                   ` (2 preceding siblings ...)
  2022-02-23 16:35 ` [PATCH v5 3/7] iio: potentiometer: Add channel information in device data Jagath Jog J
@ 2022-02-23 16:35 ` Jagath Jog J
  2022-02-23 16:35 ` [PATCH v5 5/7] iio: potentiometer: Add device specific read_raw function Jagath Jog J
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Jagath Jog J @ 2022-02-23 16:35 UTC (permalink / raw)
  To: jic23, lars, andy.shevchenko, sst, robh+dt
  Cc: linux-iio, devicetree, linux-kernel

Using firmware provided data to get the device specific
structure if not available fall back to id->driver_data.

Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
---
 drivers/iio/potentiometer/ds1803.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/potentiometer/ds1803.c b/drivers/iio/potentiometer/ds1803.c
index f93cd342e09a..aeb74ee46fbc 100644
--- a/drivers/iio/potentiometer/ds1803.c
+++ b/drivers/iio/potentiometer/ds1803.c
@@ -15,6 +15,7 @@
 #include <linux/iio/iio.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
+#include <linux/property.h>
 
 #define DS1803_WIPER_0         0xA9
 #define DS1803_WIPER_1         0xAA
@@ -166,7 +167,9 @@ static int ds1803_probe(struct i2c_client *client, const struct i2c_device_id *i
 
 	data = iio_priv(indio_dev);
 	data->client = client;
-	data->cfg = &ds1803_cfg[id->driver_data];
+	data->cfg = device_get_match_data(dev);
+	if (!data->cfg)
+		data->cfg = &ds1803_cfg[id->driver_data];
 
 	indio_dev->info = &ds1803_info;
 	indio_dev->channels = data->cfg->channels;
-- 
2.17.1


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

* [PATCH v5 5/7] iio: potentiometer: Add device specific read_raw function
  2022-02-23 16:35 [PATCH v5 0/7] iio: potentiometer: Add support for DS3502 Jagath Jog J
                   ` (3 preceding siblings ...)
  2022-02-23 16:35 ` [PATCH v5 4/7] iio: potentiometer: Change to firmware provided data Jagath Jog J
@ 2022-02-23 16:35 ` Jagath Jog J
  2022-02-24  0:46   ` Andy Shevchenko
  2022-02-23 16:35 ` [PATCH v5 6/7] iio: potentiometer: Add support for Maxim DS3502 Jagath Jog J
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Jagath Jog J @ 2022-02-23 16:35 UTC (permalink / raw)
  To: jic23, lars, andy.shevchenko, sst, robh+dt
  Cc: linux-iio, devicetree, linux-kernel

Added function pointer in the device specific structure to
call the appropriate device read_raw function, so that the
other type of devices with different read method can be
handled.

Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
---
 drivers/iio/potentiometer/ds1803.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/potentiometer/ds1803.c b/drivers/iio/potentiometer/ds1803.c
index aeb74ee46fbc..878188351f8f 100644
--- a/drivers/iio/potentiometer/ds1803.c
+++ b/drivers/iio/potentiometer/ds1803.c
@@ -32,6 +32,8 @@ struct ds1803_cfg {
 	int kohms;
 	const struct iio_chan_spec *channels;
 	u8 num_channels;
+	int (*read)(struct iio_dev *indio_dev,
+		    struct iio_chan_spec const *chan, int *val);
 };
 
 struct ds1803_data {
@@ -55,6 +57,22 @@ static const struct iio_chan_spec ds1803_channels[] = {
 	DS1803_CHANNEL(1, DS1803_WIPER_1),
 };
 
+static int ds1803_read(struct iio_dev *indio_dev,
+		       struct iio_chan_spec const *chan,
+		       int *val)
+{
+	struct ds1803_data *data = iio_priv(indio_dev);
+	int ret;
+	u8 result[ARRAY_SIZE(ds1803_channels)];
+
+	ret = i2c_master_recv(data->client, result, indio_dev->num_channels);
+	if (ret < 0)
+		return ret;
+
+	*val = result[chan->channel];
+	return ret;
+}
+
 static const struct ds1803_cfg ds1803_cfg[] = {
 	[DS1803_010] = {
 	  .wipers = 2,
@@ -62,6 +80,7 @@ static const struct ds1803_cfg ds1803_cfg[] = {
 	  .kohms =  10,
 	  .channels = ds1803_channels,
 	  .num_channels = ARRAY_SIZE(ds1803_channels),
+	  .read = ds1803_read,
 	},
 	[DS1803_050] = {
 	  .wipers = 2,
@@ -69,6 +88,7 @@ static const struct ds1803_cfg ds1803_cfg[] = {
 	  .kohms =  50,
 	  .channels = ds1803_channels,
 	  .num_channels = ARRAY_SIZE(ds1803_channels),
+	  .read = ds1803_read,
 	},
 	[DS1803_100] = {
 	  .wipers = 2,
@@ -76,6 +96,7 @@ static const struct ds1803_cfg ds1803_cfg[] = {
 	  .kohms = 100,
 	  .channels = ds1803_channels,
 	  .num_channels = ARRAY_SIZE(ds1803_channels),
+	  .read = ds1803_read,
 	},
 };
 
@@ -84,20 +105,15 @@ static int ds1803_read_raw(struct iio_dev *indio_dev,
 			   int *val, int *val2, long mask)
 {
 	struct ds1803_data *data = iio_priv(indio_dev);
-	int pot = chan->channel;
 	int ret;
-	u8 result[ARRAY_SIZE(ds1803_channels)];
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		ret = i2c_master_recv(data->client, result,
-				      indio_dev->num_channels);
+		ret = data->cfg->read(indio_dev, chan, val);
 		if (ret < 0)
 			return ret;
 
-		*val = result[pot];
 		return IIO_VAL_INT;
-
 	case IIO_CHAN_INFO_SCALE:
 		*val = 1000 * data->cfg->kohms;
 		*val2 = data->cfg->avail[2]; /* Max wiper position */
-- 
2.17.1


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

* [PATCH v5 6/7] iio: potentiometer: Add support for Maxim DS3502
  2022-02-23 16:35 [PATCH v5 0/7] iio: potentiometer: Add support for DS3502 Jagath Jog J
                   ` (4 preceding siblings ...)
  2022-02-23 16:35 ` [PATCH v5 5/7] iio: potentiometer: Add device specific read_raw function Jagath Jog J
@ 2022-02-23 16:35 ` Jagath Jog J
  2022-02-23 16:35 ` [PATCH v5 7/7] dt-bindings: iio: potentiometer: Add Maxim DS3502 in trivial-devices Jagath Jog J
  2022-02-24  0:47 ` [PATCH v5 0/7] iio: potentiometer: Add support for DS3502 Andy Shevchenko
  7 siblings, 0 replies; 14+ messages in thread
From: Jagath Jog J @ 2022-02-23 16:35 UTC (permalink / raw)
  To: jic23, lars, andy.shevchenko, sst, robh+dt
  Cc: linux-iio, devicetree, linux-kernel

The DS3502 is a 7-bit, nonvolatile digital potentiometer featuring
an output voltage range of up to 15.5V. DS3502 support is added
into existing DS1803 driver.

Datasheet: https://datasheets.maximintegrated.com/en/ds/DS3502.pdf
Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
---
 drivers/iio/potentiometer/Kconfig  |  6 ++---
 drivers/iio/potentiometer/ds1803.c | 37 +++++++++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/potentiometer/Kconfig b/drivers/iio/potentiometer/Kconfig
index 832df8da2bc6..01dd3f858d99 100644
--- a/drivers/iio/potentiometer/Kconfig
+++ b/drivers/iio/potentiometer/Kconfig
@@ -27,11 +27,11 @@ config AD5272
 	  module will be called ad5272.
 
 config DS1803
-	tristate "Maxim Integrated DS1803 Digital Potentiometer driver"
+	tristate "Maxim Integrated DS1803 and similar Digital Potentiometer driver"
 	depends on I2C
 	help
-	  Say yes here to build support for the Maxim Integrated DS1803
-	  digital potentiometer chip.
+	  Say yes here to build support for the Maxim Integrated DS1803 and
+	  DS3502 digital potentiometer chip.
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called ds1803.
diff --git a/drivers/iio/potentiometer/ds1803.c b/drivers/iio/potentiometer/ds1803.c
index 878188351f8f..81456b7d314b 100644
--- a/drivers/iio/potentiometer/ds1803.c
+++ b/drivers/iio/potentiometer/ds1803.c
@@ -1,12 +1,15 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Maxim Integrated DS1803 digital potentiometer driver
+ * Maxim Integrated DS1803 and similar digital potentiometer driver
  * Copyright (c) 2016 Slawomir Stepien
+ * Copyright (c) 2022 Jagath Jog J
  *
  * Datasheet: https://datasheets.maximintegrated.com/en/ds/DS1803.pdf
+ * Datasheet: https://datasheets.maximintegrated.com/en/ds/DS3502.pdf
  *
  * DEVID	#Wipers	#Positions	Resistor Opts (kOhm)	i2c address
  * ds1803	2	256		10, 50, 100		0101xxx
+ * ds3502	1	128		10			01010xx
  */
 
 #include <linux/err.h>
@@ -19,11 +22,13 @@
 
 #define DS1803_WIPER_0         0xA9
 #define DS1803_WIPER_1         0xAA
+#define DS3502_WR_IVR          0x00
 
 enum ds1803_type {
 	DS1803_010,
 	DS1803_050,
 	DS1803_100,
+	DS3502,
 };
 
 struct ds1803_cfg {
@@ -57,6 +62,10 @@ static const struct iio_chan_spec ds1803_channels[] = {
 	DS1803_CHANNEL(1, DS1803_WIPER_1),
 };
 
+static const struct iio_chan_spec ds3502_channels[] = {
+	DS1803_CHANNEL(0, DS3502_WR_IVR),
+};
+
 static int ds1803_read(struct iio_dev *indio_dev,
 		       struct iio_chan_spec const *chan,
 		       int *val)
@@ -73,6 +82,21 @@ static int ds1803_read(struct iio_dev *indio_dev,
 	return ret;
 }
 
+static int ds3502_read(struct iio_dev *indio_dev,
+		       struct iio_chan_spec const *chan,
+		       int *val)
+{
+	struct ds1803_data *data = iio_priv(indio_dev);
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(data->client, chan->address);
+	if (ret < 0)
+		return ret;
+
+	*val = ret;
+	return ret;
+}
+
 static const struct ds1803_cfg ds1803_cfg[] = {
 	[DS1803_010] = {
 	  .wipers = 2,
@@ -98,6 +122,14 @@ static const struct ds1803_cfg ds1803_cfg[] = {
 	  .num_channels = ARRAY_SIZE(ds1803_channels),
 	  .read = ds1803_read,
 	},
+	[DS3502] = {
+	  .wipers = 1,
+	  .avail = { 0, 1, 127 },
+	  .kohms =  10,
+	  .channels = ds3502_channels,
+	  .num_channels = ARRAY_SIZE(ds3502_channels),
+	  .read = ds3502_read,
+	},
 };
 
 static int ds1803_read_raw(struct iio_dev *indio_dev,
@@ -199,6 +231,7 @@ static const struct of_device_id ds1803_dt_ids[] = {
 	{ .compatible = "maxim,ds1803-010", .data = &ds1803_cfg[DS1803_010] },
 	{ .compatible = "maxim,ds1803-050", .data = &ds1803_cfg[DS1803_050] },
 	{ .compatible = "maxim,ds1803-100", .data = &ds1803_cfg[DS1803_100] },
+	{ .compatible = "maxim,ds3502", .data = &ds1803_cfg[DS3502] },
 	{}
 };
 MODULE_DEVICE_TABLE(of, ds1803_dt_ids);
@@ -207,6 +240,7 @@ static const struct i2c_device_id ds1803_id[] = {
 	{ "ds1803-010", DS1803_010 },
 	{ "ds1803-050", DS1803_050 },
 	{ "ds1803-100", DS1803_100 },
+	{ "ds3502", DS3502 },
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, ds1803_id);
@@ -223,5 +257,6 @@ static struct i2c_driver ds1803_driver = {
 module_i2c_driver(ds1803_driver);
 
 MODULE_AUTHOR("Slawomir Stepien <sst@poczta.fm>");
+MODULE_AUTHOR("Jagath Jog J <jagathjog1996@gmail.com>");
 MODULE_DESCRIPTION("DS1803 digital potentiometer");
 MODULE_LICENSE("GPL v2");
-- 
2.17.1


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

* [PATCH v5 7/7] dt-bindings: iio: potentiometer: Add Maxim DS3502 in trivial-devices
  2022-02-23 16:35 [PATCH v5 0/7] iio: potentiometer: Add support for DS3502 Jagath Jog J
                   ` (5 preceding siblings ...)
  2022-02-23 16:35 ` [PATCH v5 6/7] iio: potentiometer: Add support for Maxim DS3502 Jagath Jog J
@ 2022-02-23 16:35 ` Jagath Jog J
  2022-02-24  0:47 ` [PATCH v5 0/7] iio: potentiometer: Add support for DS3502 Andy Shevchenko
  7 siblings, 0 replies; 14+ messages in thread
From: Jagath Jog J @ 2022-02-23 16:35 UTC (permalink / raw)
  To: jic23, lars, andy.shevchenko, sst, robh+dt
  Cc: linux-iio, devicetree, linux-kernel

Maxim DS3502 is a 7 bit nonvolatile digital potentiometer.
Add DS3502 binding into trivial-devices.yaml.

Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 091792ba993e..b6187603317a 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -157,6 +157,8 @@ properties:
           - maxim,ds1803-050
             # 100 kOhm digital potentiometer with I2C interface
           - maxim,ds1803-100
+            # 10 kOhm digital potentiometer with I2C interface
+          - maxim,ds3502
             # Low-Power, 4-/12-Channel, 2-Wire Serial, 12-Bit ADCs
           - maxim,max1237
             # Temperature Sensor, I2C interface
-- 
2.17.1


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

* Re: [PATCH v5 5/7] iio: potentiometer: Add device specific read_raw function
  2022-02-23 16:35 ` [PATCH v5 5/7] iio: potentiometer: Add device specific read_raw function Jagath Jog J
@ 2022-02-24  0:46   ` Andy Shevchenko
  2022-02-26 18:39     ` Jonathan Cameron
  0 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2022-02-24  0:46 UTC (permalink / raw)
  To: Jagath Jog J
  Cc: Jonathan Cameron, Lars-Peter Clausen, Slawomir Stepien,
	Rob Herring, linux-iio, devicetree, Linux Kernel Mailing List

On Wed, Feb 23, 2022 at 6:35 PM Jagath Jog J <jagathjog1996@gmail.com> wrote:
>
> Added function pointer in the device specific structure to
> call the appropriate device read_raw function, so that the
> other type of devices with different read method can be
> handled.
>
> Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
> ---
>  drivers/iio/potentiometer/ds1803.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/potentiometer/ds1803.c b/drivers/iio/potentiometer/ds1803.c
> index aeb74ee46fbc..878188351f8f 100644
> --- a/drivers/iio/potentiometer/ds1803.c
> +++ b/drivers/iio/potentiometer/ds1803.c
> @@ -32,6 +32,8 @@ struct ds1803_cfg {
>         int kohms;
>         const struct iio_chan_spec *channels;
>         u8 num_channels;
> +       int (*read)(struct iio_dev *indio_dev,
> +                   struct iio_chan_spec const *chan, int *val);
>  };
>
>  struct ds1803_data {
> @@ -55,6 +57,22 @@ static const struct iio_chan_spec ds1803_channels[] = {
>         DS1803_CHANNEL(1, DS1803_WIPER_1),
>  };
>
> +static int ds1803_read(struct iio_dev *indio_dev,
> +                      struct iio_chan_spec const *chan,
> +                      int *val)
> +{
> +       struct ds1803_data *data = iio_priv(indio_dev);
> +       int ret;
> +       u8 result[ARRAY_SIZE(ds1803_channels)];
> +
> +       ret = i2c_master_recv(data->client, result, indio_dev->num_channels);
> +       if (ret < 0)
> +               return ret;
> +
> +       *val = result[chan->channel];
> +       return ret;
> +}
> +
>  static const struct ds1803_cfg ds1803_cfg[] = {
>         [DS1803_010] = {
>           .wipers = 2,
> @@ -62,6 +80,7 @@ static const struct ds1803_cfg ds1803_cfg[] = {
>           .kohms =  10,
>           .channels = ds1803_channels,
>           .num_channels = ARRAY_SIZE(ds1803_channels),
> +         .read = ds1803_read,
>         },
>         [DS1803_050] = {
>           .wipers = 2,
> @@ -69,6 +88,7 @@ static const struct ds1803_cfg ds1803_cfg[] = {
>           .kohms =  50,
>           .channels = ds1803_channels,
>           .num_channels = ARRAY_SIZE(ds1803_channels),
> +         .read = ds1803_read,
>         },
>         [DS1803_100] = {
>           .wipers = 2,
> @@ -76,6 +96,7 @@ static const struct ds1803_cfg ds1803_cfg[] = {
>           .kohms = 100,
>           .channels = ds1803_channels,
>           .num_channels = ARRAY_SIZE(ds1803_channels),
> +         .read = ds1803_read,
>         },
>  };
>
> @@ -84,20 +105,15 @@ static int ds1803_read_raw(struct iio_dev *indio_dev,
>                            int *val, int *val2, long mask)
>  {
>         struct ds1803_data *data = iio_priv(indio_dev);
> -       int pot = chan->channel;
>         int ret;
> -       u8 result[ARRAY_SIZE(ds1803_channels)];
>
>         switch (mask) {
>         case IIO_CHAN_INFO_RAW:
> -               ret = i2c_master_recv(data->client, result,
> -                                     indio_dev->num_channels);
> +               ret = data->cfg->read(indio_dev, chan, val);
>                 if (ret < 0)
>                         return ret;
>
> -               *val = result[pot];
>                 return IIO_VAL_INT;

> -

Seems like a stray change. Up to Jonathan to decide what to do (no
need for resend b/c of this).

>         case IIO_CHAN_INFO_SCALE:
>                 *val = 1000 * data->cfg->kohms;
>                 *val2 = data->cfg->avail[2]; /* Max wiper position */
> --
> 2.17.1
>


-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v5 0/7] iio: potentiometer: Add support for DS3502
  2022-02-23 16:35 [PATCH v5 0/7] iio: potentiometer: Add support for DS3502 Jagath Jog J
                   ` (6 preceding siblings ...)
  2022-02-23 16:35 ` [PATCH v5 7/7] dt-bindings: iio: potentiometer: Add Maxim DS3502 in trivial-devices Jagath Jog J
@ 2022-02-24  0:47 ` Andy Shevchenko
  2022-02-26 18:41   ` Jonathan Cameron
  7 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2022-02-24  0:47 UTC (permalink / raw)
  To: Jagath Jog J
  Cc: Jonathan Cameron, Lars-Peter Clausen, Slawomir Stepien,
	Rob Herring, linux-iio, devicetree, Linux Kernel Mailing List

On Wed, Feb 23, 2022 at 6:35 PM Jagath Jog J <jagathjog1996@gmail.com> wrote:
>
> Add dt-bindings and support for Maxim DS3502 into existing ds1803 driver.
> DS3502 is a 7 bit Nonvolatile Digital Potentiometer.

LGTM, thanks!
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
for the entire series,

> Changes since v4:
> 1. Included property.h header which has device_get_match_data()
>    function prototype.
> 2. Removed blank space in tag block of the commit message.
> 3. Style changes for ds1803_cfg structure.
>
> Changes since v3:
> 1. Dropped the chip type switch statement in read_raw function.
> 2. Added device specific read function pointer in their structure.
> 3. Added two separate functions to read values from two different types
>    of devices.
>
> Changes since v2:
> 1. Addressed Andy Shevchenko comments.
> 2. Adding device name in Kconfig file.
> 3. Spliting up of patch into 3 patches.
> 4. Adding channel info into ds1803_cfg in separate patch.
> 5. Dropping the use of enum in firmware data instead using previous
>    pointer method for accessing device specific data.
> 6. Separate patch for using firmware provided data instead of
>    id->driver_data.
> 7. Adding DS3502 support in separate patch.
>
> Changes since v1:
> 1. Fixes the alignment to match the open parenthesis in separate patch.
> 2. Adding available functionality for ds1803 driver in separate patch.
> 3. Moving maxim_potentiometer members into ds1803_cfg structure.
> 4. Droping of the INFO_ENABLE channel type.
> 5. Firmware entry with data is used instead of id->driver_data to
>    to retrieve the chip specific data.
>
> Jagath Jog J (7):
>   iio: potentiometer: Alignment to match the open parenthesis
>   iio: potentiometer: Add available functionality
>   iio: potentiometer: Add channel information in device data
>   iio: potentiometer: Change to firmware provided data
>   iio: potentiometer: Add device specific read_raw function
>   iio: potentiometer: Add support for Maxim DS3502
>   dt-bindings: iio: potentiometer: Add Maxim DS3502 in trivial-devices
>
>  .../devicetree/bindings/trivial-devices.yaml  |   2 +
>  drivers/iio/potentiometer/Kconfig             |   6 +-
>  drivers/iio/potentiometer/ds1803.c            | 170 ++++++++++++++----
>  3 files changed, 138 insertions(+), 40 deletions(-)
>
> --
> 2.17.1
>


-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v5 1/7] iio: potentiometer: Alignment to match the open parenthesis
  2022-02-23 16:35 ` [PATCH v5 1/7] iio: potentiometer: Alignment to match the open parenthesis Jagath Jog J
@ 2022-02-26 18:35   ` Jonathan Cameron
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2022-02-26 18:35 UTC (permalink / raw)
  To: Jagath Jog J
  Cc: lars, andy.shevchenko, sst, robh+dt, linux-iio, devicetree, linux-kernel

On Wed, 23 Feb 2022 22:05:19 +0530
Jagath Jog J <jagathjog1996@gmail.com> wrote:

> Fix following checkpatch.pl check by removing blank space.
> CHECK: Alignment should match open parenthesis.
> 
> Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
I didn't noticed before, but for future please put the driver name
in the patch titles!

iio: potentiometer: ds1803: xxxx

I'll fix up.

> ---
>  drivers/iio/potentiometer/ds1803.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/potentiometer/ds1803.c b/drivers/iio/potentiometer/ds1803.c
> index 20b45407eaac..3c542a50ece6 100644
> --- a/drivers/iio/potentiometer/ds1803.c
> +++ b/drivers/iio/potentiometer/ds1803.c
> @@ -55,8 +55,8 @@ static const struct iio_chan_spec ds1803_channels[] = {
>  };
>  
>  static int ds1803_read_raw(struct iio_dev *indio_dev,
> -			    struct iio_chan_spec const *chan,
> -			    int *val, int *val2, long mask)
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
>  {
>  	struct ds1803_data *data = iio_priv(indio_dev);
>  	int pot = chan->channel;
> @@ -66,7 +66,7 @@ static int ds1803_read_raw(struct iio_dev *indio_dev,
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
>  		ret = i2c_master_recv(data->client, result,
> -				indio_dev->num_channels);
> +				      indio_dev->num_channels);
>  		if (ret < 0)
>  			return ret;
>  
> @@ -83,8 +83,8 @@ static int ds1803_read_raw(struct iio_dev *indio_dev,
>  }
>  
>  static int ds1803_write_raw(struct iio_dev *indio_dev,
> -			     struct iio_chan_spec const *chan,
> -			     int val, int val2, long mask)
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long mask)
>  {
>  	struct ds1803_data *data = iio_priv(indio_dev);
>  	int pot = chan->channel;
> @@ -109,8 +109,7 @@ static const struct iio_info ds1803_info = {
>  	.write_raw = ds1803_write_raw,
>  };
>  
> -static int ds1803_probe(struct i2c_client *client,
> -			 const struct i2c_device_id *id)
> +static int ds1803_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  {
>  	struct device *dev = &client->dev;
>  	struct ds1803_data *data;


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

* Re: [PATCH v5 5/7] iio: potentiometer: Add device specific read_raw function
  2022-02-24  0:46   ` Andy Shevchenko
@ 2022-02-26 18:39     ` Jonathan Cameron
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2022-02-26 18:39 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jagath Jog J, Lars-Peter Clausen, Slawomir Stepien, Rob Herring,
	linux-iio, devicetree, Linux Kernel Mailing List

On Thu, 24 Feb 2022 02:46:34 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Wed, Feb 23, 2022 at 6:35 PM Jagath Jog J <jagathjog1996@gmail.com> wrote:
> >
> > Added function pointer in the device specific structure to
> > call the appropriate device read_raw function, so that the
> > other type of devices with different read method can be
> > handled.
> >
> > Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
> > ---
> >  drivers/iio/potentiometer/ds1803.c | 28 ++++++++++++++++++++++------
> >  1 file changed, 22 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iio/potentiometer/ds1803.c b/drivers/iio/potentiometer/ds1803.c
> > index aeb74ee46fbc..878188351f8f 100644
> > --- a/drivers/iio/potentiometer/ds1803.c
> > +++ b/drivers/iio/potentiometer/ds1803.c
> > @@ -32,6 +32,8 @@ struct ds1803_cfg {
> >         int kohms;
> >         const struct iio_chan_spec *channels;
> >         u8 num_channels;
> > +       int (*read)(struct iio_dev *indio_dev,
> > +                   struct iio_chan_spec const *chan, int *val);
> >  };
> >
> >  struct ds1803_data {
> > @@ -55,6 +57,22 @@ static const struct iio_chan_spec ds1803_channels[] = {
> >         DS1803_CHANNEL(1, DS1803_WIPER_1),
> >  };
> >
> > +static int ds1803_read(struct iio_dev *indio_dev,
> > +                      struct iio_chan_spec const *chan,
> > +                      int *val)
> > +{
> > +       struct ds1803_data *data = iio_priv(indio_dev);
> > +       int ret;
> > +       u8 result[ARRAY_SIZE(ds1803_channels)];
> > +
> > +       ret = i2c_master_recv(data->client, result, indio_dev->num_channels);
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       *val = result[chan->channel];
> > +       return ret;
> > +}
> > +
> >  static const struct ds1803_cfg ds1803_cfg[] = {
> >         [DS1803_010] = {
> >           .wipers = 2,
> > @@ -62,6 +80,7 @@ static const struct ds1803_cfg ds1803_cfg[] = {
> >           .kohms =  10,
> >           .channels = ds1803_channels,
> >           .num_channels = ARRAY_SIZE(ds1803_channels),
> > +         .read = ds1803_read,
> >         },
> >         [DS1803_050] = {
> >           .wipers = 2,
> > @@ -69,6 +88,7 @@ static const struct ds1803_cfg ds1803_cfg[] = {
> >           .kohms =  50,
> >           .channels = ds1803_channels,
> >           .num_channels = ARRAY_SIZE(ds1803_channels),
> > +         .read = ds1803_read,
> >         },
> >         [DS1803_100] = {
> >           .wipers = 2,
> > @@ -76,6 +96,7 @@ static const struct ds1803_cfg ds1803_cfg[] = {
> >           .kohms = 100,
> >           .channels = ds1803_channels,
> >           .num_channels = ARRAY_SIZE(ds1803_channels),
> > +         .read = ds1803_read,
> >         },
> >  };
> >
> > @@ -84,20 +105,15 @@ static int ds1803_read_raw(struct iio_dev *indio_dev,
> >                            int *val, int *val2, long mask)
> >  {
> >         struct ds1803_data *data = iio_priv(indio_dev);
> > -       int pot = chan->channel;
> >         int ret;
> > -       u8 result[ARRAY_SIZE(ds1803_channels)];
> >
> >         switch (mask) {
> >         case IIO_CHAN_INFO_RAW:
> > -               ret = i2c_master_recv(data->client, result,
> > -                                     indio_dev->num_channels);
> > +               ret = data->cfg->read(indio_dev, chan, val);
> >                 if (ret < 0)
> >                         return ret;
> >
> > -               *val = result[pot];
> >                 return IIO_VAL_INT;  
> 
> > -  
> 
> Seems like a stray change. Up to Jonathan to decide what to do (no
> need for resend b/c of this).

Good spot. Given I was tweaking this series a fair bit anyway whilst
applying I tidied this up as well.

Thanks,

Jonathan

> 
> >         case IIO_CHAN_INFO_SCALE:
> >                 *val = 1000 * data->cfg->kohms;
> >                 *val2 = data->cfg->avail[2]; /* Max wiper position */
> > --
> > 2.17.1
> >  
> 
> 


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

* Re: [PATCH v5 2/7] iio: potentiometer: Add available functionality
  2022-02-23 16:35 ` [PATCH v5 2/7] iio: potentiometer: Add available functionality Jagath Jog J
@ 2022-02-26 18:40   ` Jonathan Cameron
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2022-02-26 18:40 UTC (permalink / raw)
  To: Jagath Jog J
  Cc: lars, andy.shevchenko, sst, robh+dt, linux-iio, devicetree, linux-kernel

On Wed, 23 Feb 2022 22:05:20 +0530
Jagath Jog J <jagathjog1996@gmail.com> wrote:

> Adding available functionality for DS1803 driver which
> will show the minimum, step and maximum values that the
> driver can accepts through sysfs entry.
> Now using the max value present in avail array instead of chip
> type specific macro to make the driver flexible to add other
> type of potentiometer with different max position value.
> 
> Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
> ---
>  drivers/iio/potentiometer/ds1803.c | 61 ++++++++++++++++++++++--------
>  1 file changed, 45 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/iio/potentiometer/ds1803.c b/drivers/iio/potentiometer/ds1803.c
> index 3c542a50ece6..9610269bed7f 100644
> --- a/drivers/iio/potentiometer/ds1803.c
> +++ b/drivers/iio/potentiometer/ds1803.c
> @@ -16,7 +16,6 @@
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
>  
> -#define DS1803_MAX_POS		255
>  #define DS1803_WRITE(chan)	(0xa8 | ((chan) + 1))
>  
>  enum ds1803_type {
> @@ -26,27 +25,23 @@ enum ds1803_type {
>  };
>  
>  struct ds1803_cfg {
> +	int avail[3];
>  	int kohms;
>  };
>  
> -static const struct ds1803_cfg ds1803_cfg[] = {
> -	[DS1803_010] = { .kohms =  10, },
> -	[DS1803_050] = { .kohms =  50, },
> -	[DS1803_100] = { .kohms = 100, },
> -};
> -
>  struct ds1803_data {
>  	struct i2c_client *client;
>  	const struct ds1803_cfg *cfg;
>  };
>  
> -#define DS1803_CHANNEL(ch) {					\
> -	.type = IIO_RESISTANCE,					\
> -	.indexed = 1,						\
> -	.output = 1,						\
> -	.channel = (ch),					\
> -	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> -	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
> +#define DS1803_CHANNEL(ch) {						\
> +	.type = IIO_RESISTANCE,						\
> +	.indexed = 1,							\
> +	.output = 1,							\
> +	.channel = (ch),						\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
> +	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_RAW),   \
>  }
>  
>  static const struct iio_chan_spec ds1803_channels[] = {
> @@ -54,6 +49,21 @@ static const struct iio_chan_spec ds1803_channels[] = {
>  	DS1803_CHANNEL(1),
>  };
>  
> +static const struct ds1803_cfg ds1803_cfg[] = {
> +	[DS1803_010] = {
> +	  .avail = { 0, 1, 255 },
Why non standard indenting.  These should be a tab in rather than
a couple of spaces.

I'll fixup whilst applying.

Jonathan

> +	  .kohms =  10,
> +	},
> +	[DS1803_050] = {
> +	  .avail = { 0, 1, 255 },
> +	  .kohms =  50,
> +	},
> +	[DS1803_100] = {
> +	  .avail = { 0, 1, 255 },
> +	  .kohms = 100,
> +	},
> +};
> +
>  static int ds1803_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan,
>  			   int *val, int *val2, long mask)
> @@ -75,7 +85,7 @@ static int ds1803_read_raw(struct iio_dev *indio_dev,
>  
>  	case IIO_CHAN_INFO_SCALE:
>  		*val = 1000 * data->cfg->kohms;
> -		*val2 = DS1803_MAX_POS;
> +		*val2 = data->cfg->avail[2]; /* Max wiper position */
>  		return IIO_VAL_FRACTIONAL;
>  	}
>  
> @@ -88,13 +98,14 @@ static int ds1803_write_raw(struct iio_dev *indio_dev,
>  {
>  	struct ds1803_data *data = iio_priv(indio_dev);
>  	int pot = chan->channel;
> +	int max_pos = data->cfg->avail[2];
>  
>  	if (val2 != 0)
>  		return -EINVAL;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> -		if (val > DS1803_MAX_POS || val < 0)
> +		if (val > max_pos || val < 0)
>  			return -EINVAL;
>  		break;
>  	default:
> @@ -104,9 +115,27 @@ static int ds1803_write_raw(struct iio_dev *indio_dev,
>  	return i2c_smbus_write_byte_data(data->client, DS1803_WRITE(pot), val);
>  }
>  
> +static int ds1803_read_avail(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     const int **vals, int *type,
> +			     int *length, long mask)
> +{
> +	struct ds1803_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		*vals = data->cfg->avail;
> +		*length = ARRAY_SIZE(data->cfg->avail);
> +		*type = IIO_VAL_INT;
> +		return IIO_AVAIL_RANGE;
> +	}
> +	return -EINVAL;
> +}
> +
>  static const struct iio_info ds1803_info = {
>  	.read_raw = ds1803_read_raw,
>  	.write_raw = ds1803_write_raw,
> +	.read_avail = ds1803_read_avail,
>  };
>  
>  static int ds1803_probe(struct i2c_client *client, const struct i2c_device_id *id)


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

* Re: [PATCH v5 0/7] iio: potentiometer: Add support for DS3502
  2022-02-24  0:47 ` [PATCH v5 0/7] iio: potentiometer: Add support for DS3502 Andy Shevchenko
@ 2022-02-26 18:41   ` Jonathan Cameron
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2022-02-26 18:41 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jagath Jog J, Lars-Peter Clausen, Slawomir Stepien, Rob Herring,
	linux-iio, devicetree, Linux Kernel Mailing List

On Thu, 24 Feb 2022 02:47:53 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Wed, Feb 23, 2022 at 6:35 PM Jagath Jog J <jagathjog1996@gmail.com> wrote:
> >
> > Add dt-bindings and support for Maxim DS3502 into existing ds1803 driver.
> > DS3502 is a 7 bit Nonvolatile Digital Potentiometer.  
> 
> LGTM, thanks!
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> for the entire series,

Applied to the togreg branch of iio.git with the various tweaks I mentioned
in reply to individual patches.

Pushed out initially as testing to let 0-day run it's much more comprehensive
set of build tests against it than I run locally.

Thanks,

Jonathan

> 
> > Changes since v4:
> > 1. Included property.h header which has device_get_match_data()
> >    function prototype.
> > 2. Removed blank space in tag block of the commit message.
> > 3. Style changes for ds1803_cfg structure.
> >
> > Changes since v3:
> > 1. Dropped the chip type switch statement in read_raw function.
> > 2. Added device specific read function pointer in their structure.
> > 3. Added two separate functions to read values from two different types
> >    of devices.
> >
> > Changes since v2:
> > 1. Addressed Andy Shevchenko comments.
> > 2. Adding device name in Kconfig file.
> > 3. Spliting up of patch into 3 patches.
> > 4. Adding channel info into ds1803_cfg in separate patch.
> > 5. Dropping the use of enum in firmware data instead using previous
> >    pointer method for accessing device specific data.
> > 6. Separate patch for using firmware provided data instead of
> >    id->driver_data.
> > 7. Adding DS3502 support in separate patch.
> >
> > Changes since v1:
> > 1. Fixes the alignment to match the open parenthesis in separate patch.
> > 2. Adding available functionality for ds1803 driver in separate patch.
> > 3. Moving maxim_potentiometer members into ds1803_cfg structure.
> > 4. Droping of the INFO_ENABLE channel type.
> > 5. Firmware entry with data is used instead of id->driver_data to
> >    to retrieve the chip specific data.
> >
> > Jagath Jog J (7):
> >   iio: potentiometer: Alignment to match the open parenthesis
> >   iio: potentiometer: Add available functionality
> >   iio: potentiometer: Add channel information in device data
> >   iio: potentiometer: Change to firmware provided data
> >   iio: potentiometer: Add device specific read_raw function
> >   iio: potentiometer: Add support for Maxim DS3502
> >   dt-bindings: iio: potentiometer: Add Maxim DS3502 in trivial-devices
> >
> >  .../devicetree/bindings/trivial-devices.yaml  |   2 +
> >  drivers/iio/potentiometer/Kconfig             |   6 +-
> >  drivers/iio/potentiometer/ds1803.c            | 170 ++++++++++++++----
> >  3 files changed, 138 insertions(+), 40 deletions(-)
> >
> > --
> > 2.17.1
> >  
> 
> 


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

end of thread, other threads:[~2022-02-26 18:34 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-23 16:35 [PATCH v5 0/7] iio: potentiometer: Add support for DS3502 Jagath Jog J
2022-02-23 16:35 ` [PATCH v5 1/7] iio: potentiometer: Alignment to match the open parenthesis Jagath Jog J
2022-02-26 18:35   ` Jonathan Cameron
2022-02-23 16:35 ` [PATCH v5 2/7] iio: potentiometer: Add available functionality Jagath Jog J
2022-02-26 18:40   ` Jonathan Cameron
2022-02-23 16:35 ` [PATCH v5 3/7] iio: potentiometer: Add channel information in device data Jagath Jog J
2022-02-23 16:35 ` [PATCH v5 4/7] iio: potentiometer: Change to firmware provided data Jagath Jog J
2022-02-23 16:35 ` [PATCH v5 5/7] iio: potentiometer: Add device specific read_raw function Jagath Jog J
2022-02-24  0:46   ` Andy Shevchenko
2022-02-26 18:39     ` Jonathan Cameron
2022-02-23 16:35 ` [PATCH v5 6/7] iio: potentiometer: Add support for Maxim DS3502 Jagath Jog J
2022-02-23 16:35 ` [PATCH v5 7/7] dt-bindings: iio: potentiometer: Add Maxim DS3502 in trivial-devices Jagath Jog J
2022-02-24  0:47 ` [PATCH v5 0/7] iio: potentiometer: Add support for DS3502 Andy Shevchenko
2022-02-26 18:41   ` 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).