All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 01/10] dt-bindings: iio: adc: ti,ads1015: Add missing ADS1115 compatible string
@ 2022-03-20 18:14 Marek Vasut
  2022-03-20 18:14 ` [PATCH v3 02/10] dt-bindings: iio: adc: ti,ads1015: Add TLA2024 " Marek Vasut
                   ` (8 more replies)
  0 siblings, 9 replies; 25+ messages in thread
From: Marek Vasut @ 2022-03-20 18:14 UTC (permalink / raw)
  To: linux-iio
  Cc: Marek Vasut, Krzysztof Kozlowski, Andy Shevchenko,
	Andy Shevchenko, Daniel Baluta, Jonathan Cameron, Rob Herring,
	devicetree

Add missing ti,ads1115 compatible string. This compatible string is
supported by the Linux kernel driver and the ads1015 is a 12bit ADC
while ads1115 is 16bit ADC. Add the missing compatible string.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Daniel Baluta <daniel.baluta@nxp.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
---
V2: Add AB from Krzysztof
V3: No change
---
 Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
index 2c2d01bbc296d..c31c80989cc9a 100644
--- a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/iio/adc/ti,ads1015.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: TI ADS1015 4 channel I2C analog to digital converter
+title: TI ADS1015/ADS1115 4 channel I2C analog to digital converter
 
 maintainers:
   - Daniel Baluta <daniel.baluta@nxp.com>
@@ -15,7 +15,9 @@ description: |
 
 properties:
   compatible:
-    const: ti,ads1015
+    enum:
+      - ti,ads1015
+      - ti,ads1115
 
   reg:
     maxItems: 1
-- 
2.35.1


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

* [PATCH v3 02/10] dt-bindings: iio: adc: ti,ads1015: Add TLA2024 compatible string
  2022-03-20 18:14 [PATCH v3 01/10] dt-bindings: iio: adc: ti,ads1015: Add missing ADS1115 compatible string Marek Vasut
@ 2022-03-20 18:14 ` Marek Vasut
  2022-03-20 18:14 ` [PATCH v3 03/10] iio: adc: ti-ads1015: Switch to static const writeable ranges table Marek Vasut
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 25+ messages in thread
From: Marek Vasut @ 2022-03-20 18:14 UTC (permalink / raw)
  To: linux-iio
  Cc: Marek Vasut, Krzysztof Kozlowski, Andy Shevchenko,
	Andy Shevchenko, Daniel Baluta, Jonathan Cameron, Rob Herring,
	devicetree

Add ti,tla2024 compatible string. This device is compatible with
ADS1015 except it has no on-chip comparator.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Daniel Baluta <daniel.baluta@nxp.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
---
V2: Add AB from Krzysztof
V3: No change
---
 Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
index c31c80989cc9a..a3b79438a13a5 100644
--- a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
@@ -18,6 +18,7 @@ properties:
     enum:
       - ti,ads1015
       - ti,ads1115
+      - ti,tla2024
 
   reg:
     maxItems: 1
-- 
2.35.1


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

* [PATCH v3 03/10] iio: adc: ti-ads1015: Switch to static const writeable ranges table
  2022-03-20 18:14 [PATCH v3 01/10] dt-bindings: iio: adc: ti,ads1015: Add missing ADS1115 compatible string Marek Vasut
  2022-03-20 18:14 ` [PATCH v3 02/10] dt-bindings: iio: adc: ti,ads1015: Add TLA2024 " Marek Vasut
@ 2022-03-20 18:14 ` Marek Vasut
  2022-03-20 18:14 ` [PATCH v3 04/10] iio: adc: ti-ads1015: Deduplicate channel macros Marek Vasut
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 25+ messages in thread
From: Marek Vasut @ 2022-03-20 18:14 UTC (permalink / raw)
  To: linux-iio
  Cc: Marek Vasut, Andy Shevchenko, Andy Shevchenko, Daniel Baluta,
	Jonathan Cameron

Switch the driver from code implementing test whether a regmap register
is writeable to static const tables describing the test. No functional
change.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Daniel Baluta <daniel.baluta@nxp.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
V2: No change
V3: No change
---
 drivers/iio/adc/ti-ads1015.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 068efbce17103..85932b9dc166a 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -273,23 +273,20 @@ static void ads1015_event_channel_disable(struct ads1015_data *data, int chan)
 	data->event_channel = ADS1015_CHANNELS;
 }
 
-static bool ads1015_is_writeable_reg(struct device *dev, unsigned int reg)
-{
-	switch (reg) {
-	case ADS1015_CFG_REG:
-	case ADS1015_LO_THRESH_REG:
-	case ADS1015_HI_THRESH_REG:
-		return true;
-	default:
-		return false;
-	}
-}
+static const struct regmap_range ads1015_writeable_ranges[] = {
+	regmap_reg_range(ADS1015_CFG_REG, ADS1015_HI_THRESH_REG),
+};
+
+static const struct regmap_access_table ads1015_writeable_table = {
+	.yes_ranges = ads1015_writeable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(ads1015_writeable_ranges),
+};
 
 static const struct regmap_config ads1015_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 16,
 	.max_register = ADS1015_HI_THRESH_REG,
-	.writeable_reg = ads1015_is_writeable_reg,
+	.wr_table = &ads1015_writeable_table,
 };
 
 static const struct iio_chan_spec ads1015_channels[] = {
-- 
2.35.1


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

* [PATCH v3 04/10] iio: adc: ti-ads1015: Deduplicate channel macros
  2022-03-20 18:14 [PATCH v3 01/10] dt-bindings: iio: adc: ti,ads1015: Add missing ADS1115 compatible string Marek Vasut
  2022-03-20 18:14 ` [PATCH v3 02/10] dt-bindings: iio: adc: ti,ads1015: Add TLA2024 " Marek Vasut
  2022-03-20 18:14 ` [PATCH v3 03/10] iio: adc: ti-ads1015: Switch to static const writeable ranges table Marek Vasut
@ 2022-03-20 18:14 ` Marek Vasut
  2022-03-20 18:14 ` [PATCH v3 05/10] iio: adc: ti-ads1015: Make channel event_spec optional Marek Vasut
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 25+ messages in thread
From: Marek Vasut @ 2022-03-20 18:14 UTC (permalink / raw)
  To: linux-iio
  Cc: Marek Vasut, Andy Shevchenko, Andy Shevchenko, Daniel Baluta,
	Jonathan Cameron

These macros differ only in the number of valid bits of each ADC sample
and the shift of those bits, i.e. ADS1015 is 12bit ADC shifted by 4 left,
ADS1115 is 16bit ADC shifted by 0. No functional change.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Daniel Baluta <daniel.baluta@nxp.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
V2: No change
V3: No change
---
 drivers/iio/adc/ti-ads1015.c | 86 +++++++++---------------------------
 1 file changed, 22 insertions(+), 64 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 85932b9dc166a..fc3381ff34710 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -134,7 +134,7 @@ static const struct iio_event_spec ads1015_events[] = {
 	},
 };
 
-#define ADS1015_V_CHAN(_chan, _addr) {				\
+#define ADS1015_V_CHAN(_chan, _addr, _realbits, _shift) {	\
 	.type = IIO_VOLTAGE,					\
 	.indexed = 1,						\
 	.address = _addr,					\
@@ -145,9 +145,9 @@ static const struct iio_event_spec ads1015_events[] = {
 	.scan_index = _addr,					\
 	.scan_type = {						\
 		.sign = 's',					\
-		.realbits = 12,					\
+		.realbits = (_realbits),			\
 		.storagebits = 16,				\
-		.shift = 4,					\
+		.shift = (_shift),				\
 		.endianness = IIO_CPU,				\
 	},							\
 	.event_spec = ads1015_events,				\
@@ -155,7 +155,7 @@ static const struct iio_event_spec ads1015_events[] = {
 	.datasheet_name = "AIN"#_chan,				\
 }
 
-#define ADS1015_V_DIFF_CHAN(_chan, _chan2, _addr) {		\
+#define ADS1015_V_DIFF_CHAN(_chan, _chan2, _addr, _realbits, _shift) { \
 	.type = IIO_VOLTAGE,					\
 	.differential = 1,					\
 	.indexed = 1,						\
@@ -168,51 +168,9 @@ static const struct iio_event_spec ads1015_events[] = {
 	.scan_index = _addr,					\
 	.scan_type = {						\
 		.sign = 's',					\
-		.realbits = 12,					\
-		.storagebits = 16,				\
-		.shift = 4,					\
-		.endianness = IIO_CPU,				\
-	},							\
-	.event_spec = ads1015_events,				\
-	.num_event_specs = ARRAY_SIZE(ads1015_events),		\
-	.datasheet_name = "AIN"#_chan"-AIN"#_chan2,		\
-}
-
-#define ADS1115_V_CHAN(_chan, _addr) {				\
-	.type = IIO_VOLTAGE,					\
-	.indexed = 1,						\
-	.address = _addr,					\
-	.channel = _chan,					\
-	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
-				BIT(IIO_CHAN_INFO_SCALE) |	\
-				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
-	.scan_index = _addr,					\
-	.scan_type = {						\
-		.sign = 's',					\
-		.realbits = 16,					\
-		.storagebits = 16,				\
-		.endianness = IIO_CPU,				\
-	},							\
-	.event_spec = ads1015_events,				\
-	.num_event_specs = ARRAY_SIZE(ads1015_events),		\
-	.datasheet_name = "AIN"#_chan,				\
-}
-
-#define ADS1115_V_DIFF_CHAN(_chan, _chan2, _addr) {		\
-	.type = IIO_VOLTAGE,					\
-	.differential = 1,					\
-	.indexed = 1,						\
-	.address = _addr,					\
-	.channel = _chan,					\
-	.channel2 = _chan2,					\
-	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
-				BIT(IIO_CHAN_INFO_SCALE) |	\
-				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
-	.scan_index = _addr,					\
-	.scan_type = {						\
-		.sign = 's',					\
-		.realbits = 16,					\
+		.realbits = (_realbits),			\
 		.storagebits = 16,				\
+		.shift = (_shift),				\
 		.endianness = IIO_CPU,				\
 	},							\
 	.event_spec = ads1015_events,				\
@@ -290,26 +248,26 @@ static const struct regmap_config ads1015_regmap_config = {
 };
 
 static const struct iio_chan_spec ads1015_channels[] = {
-	ADS1015_V_DIFF_CHAN(0, 1, ADS1015_AIN0_AIN1),
-	ADS1015_V_DIFF_CHAN(0, 3, ADS1015_AIN0_AIN3),
-	ADS1015_V_DIFF_CHAN(1, 3, ADS1015_AIN1_AIN3),
-	ADS1015_V_DIFF_CHAN(2, 3, ADS1015_AIN2_AIN3),
-	ADS1015_V_CHAN(0, ADS1015_AIN0),
-	ADS1015_V_CHAN(1, ADS1015_AIN1),
-	ADS1015_V_CHAN(2, ADS1015_AIN2),
-	ADS1015_V_CHAN(3, ADS1015_AIN3),
+	ADS1015_V_DIFF_CHAN(0, 1, ADS1015_AIN0_AIN1, 12, 4),
+	ADS1015_V_DIFF_CHAN(0, 3, ADS1015_AIN0_AIN3, 12, 4),
+	ADS1015_V_DIFF_CHAN(1, 3, ADS1015_AIN1_AIN3, 12, 4),
+	ADS1015_V_DIFF_CHAN(2, 3, ADS1015_AIN2_AIN3, 12, 4),
+	ADS1015_V_CHAN(0, ADS1015_AIN0, 12, 4),
+	ADS1015_V_CHAN(1, ADS1015_AIN1, 12, 4),
+	ADS1015_V_CHAN(2, ADS1015_AIN2, 12, 4),
+	ADS1015_V_CHAN(3, ADS1015_AIN3, 12, 4),
 	IIO_CHAN_SOFT_TIMESTAMP(ADS1015_TIMESTAMP),
 };
 
 static const struct iio_chan_spec ads1115_channels[] = {
-	ADS1115_V_DIFF_CHAN(0, 1, ADS1015_AIN0_AIN1),
-	ADS1115_V_DIFF_CHAN(0, 3, ADS1015_AIN0_AIN3),
-	ADS1115_V_DIFF_CHAN(1, 3, ADS1015_AIN1_AIN3),
-	ADS1115_V_DIFF_CHAN(2, 3, ADS1015_AIN2_AIN3),
-	ADS1115_V_CHAN(0, ADS1015_AIN0),
-	ADS1115_V_CHAN(1, ADS1015_AIN1),
-	ADS1115_V_CHAN(2, ADS1015_AIN2),
-	ADS1115_V_CHAN(3, ADS1015_AIN3),
+	ADS1015_V_DIFF_CHAN(0, 1, ADS1015_AIN0_AIN1, 16, 0),
+	ADS1015_V_DIFF_CHAN(0, 3, ADS1015_AIN0_AIN3, 16, 0),
+	ADS1015_V_DIFF_CHAN(1, 3, ADS1015_AIN1_AIN3, 16, 0),
+	ADS1015_V_DIFF_CHAN(2, 3, ADS1015_AIN2_AIN3, 16, 0),
+	ADS1015_V_CHAN(0, ADS1015_AIN0, 16, 0),
+	ADS1015_V_CHAN(1, ADS1015_AIN1, 16, 0),
+	ADS1015_V_CHAN(2, ADS1015_AIN2, 16, 0),
+	ADS1015_V_CHAN(3, ADS1015_AIN3, 16, 0),
 	IIO_CHAN_SOFT_TIMESTAMP(ADS1015_TIMESTAMP),
 };
 
-- 
2.35.1


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

* [PATCH v3 05/10] iio: adc: ti-ads1015: Make channel event_spec optional
  2022-03-20 18:14 [PATCH v3 01/10] dt-bindings: iio: adc: ti,ads1015: Add missing ADS1115 compatible string Marek Vasut
                   ` (2 preceding siblings ...)
  2022-03-20 18:14 ` [PATCH v3 04/10] iio: adc: ti-ads1015: Deduplicate channel macros Marek Vasut
@ 2022-03-20 18:14 ` Marek Vasut
  2022-03-20 18:14 ` [PATCH v3 06/10] iio: adc: ti-ads1015: Add TLA2024 support Marek Vasut
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 25+ messages in thread
From: Marek Vasut @ 2022-03-20 18:14 UTC (permalink / raw)
  To: linux-iio
  Cc: Marek Vasut, Andy Shevchenko, Andy Shevchenko, Daniel Baluta,
	Jonathan Cameron

Pass event_spec and num_event_specs to ADS1015_V_CHAN and ADS1015_V_DIFF_CHAN
macros, to make it possible to pass no event_spec at all for chips which have
no comparator and thus no events. No functional change.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Daniel Baluta <daniel.baluta@nxp.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
V2: No change
V3: No change
---
 drivers/iio/adc/ti-ads1015.c | 60 +++++++++++++++++++++++-------------
 1 file changed, 38 insertions(+), 22 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index fc3381ff34710..7d0c0552f425c 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -134,7 +134,7 @@ static const struct iio_event_spec ads1015_events[] = {
 	},
 };
 
-#define ADS1015_V_CHAN(_chan, _addr, _realbits, _shift) {	\
+#define ADS1015_V_CHAN(_chan, _addr, _realbits, _shift, _event_spec, _num_event_specs) { \
 	.type = IIO_VOLTAGE,					\
 	.indexed = 1,						\
 	.address = _addr,					\
@@ -150,12 +150,12 @@ static const struct iio_event_spec ads1015_events[] = {
 		.shift = (_shift),				\
 		.endianness = IIO_CPU,				\
 	},							\
-	.event_spec = ads1015_events,				\
-	.num_event_specs = ARRAY_SIZE(ads1015_events),		\
+	.event_spec = (_event_spec),				\
+	.num_event_specs = (_num_event_specs),			\
 	.datasheet_name = "AIN"#_chan,				\
 }
 
-#define ADS1015_V_DIFF_CHAN(_chan, _chan2, _addr, _realbits, _shift) { \
+#define ADS1015_V_DIFF_CHAN(_chan, _chan2, _addr, _realbits, _shift, _event_spec, _num_event_specs) { \
 	.type = IIO_VOLTAGE,					\
 	.differential = 1,					\
 	.indexed = 1,						\
@@ -173,8 +173,8 @@ static const struct iio_event_spec ads1015_events[] = {
 		.shift = (_shift),				\
 		.endianness = IIO_CPU,				\
 	},							\
-	.event_spec = ads1015_events,				\
-	.num_event_specs = ARRAY_SIZE(ads1015_events),		\
+	.event_spec = (_event_spec),				\
+	.num_event_specs = (_num_event_specs),			\
 	.datasheet_name = "AIN"#_chan"-AIN"#_chan2,		\
 }
 
@@ -248,26 +248,42 @@ static const struct regmap_config ads1015_regmap_config = {
 };
 
 static const struct iio_chan_spec ads1015_channels[] = {
-	ADS1015_V_DIFF_CHAN(0, 1, ADS1015_AIN0_AIN1, 12, 4),
-	ADS1015_V_DIFF_CHAN(0, 3, ADS1015_AIN0_AIN3, 12, 4),
-	ADS1015_V_DIFF_CHAN(1, 3, ADS1015_AIN1_AIN3, 12, 4),
-	ADS1015_V_DIFF_CHAN(2, 3, ADS1015_AIN2_AIN3, 12, 4),
-	ADS1015_V_CHAN(0, ADS1015_AIN0, 12, 4),
-	ADS1015_V_CHAN(1, ADS1015_AIN1, 12, 4),
-	ADS1015_V_CHAN(2, ADS1015_AIN2, 12, 4),
-	ADS1015_V_CHAN(3, ADS1015_AIN3, 12, 4),
+	ADS1015_V_DIFF_CHAN(0, 1, ADS1015_AIN0_AIN1, 12, 4,
+			    ads1015_events, ARRAY_SIZE(ads1015_events)),
+	ADS1015_V_DIFF_CHAN(0, 3, ADS1015_AIN0_AIN3, 12, 4,
+			    ads1015_events, ARRAY_SIZE(ads1015_events)),
+	ADS1015_V_DIFF_CHAN(1, 3, ADS1015_AIN1_AIN3, 12, 4,
+			    ads1015_events, ARRAY_SIZE(ads1015_events)),
+	ADS1015_V_DIFF_CHAN(2, 3, ADS1015_AIN2_AIN3, 12, 4,
+			    ads1015_events, ARRAY_SIZE(ads1015_events)),
+	ADS1015_V_CHAN(0, ADS1015_AIN0, 12, 4,
+		       ads1015_events, ARRAY_SIZE(ads1015_events)),
+	ADS1015_V_CHAN(1, ADS1015_AIN1, 12, 4,
+		       ads1015_events, ARRAY_SIZE(ads1015_events)),
+	ADS1015_V_CHAN(2, ADS1015_AIN2, 12, 4,
+		       ads1015_events, ARRAY_SIZE(ads1015_events)),
+	ADS1015_V_CHAN(3, ADS1015_AIN3, 12, 4,
+		       ads1015_events, ARRAY_SIZE(ads1015_events)),
 	IIO_CHAN_SOFT_TIMESTAMP(ADS1015_TIMESTAMP),
 };
 
 static const struct iio_chan_spec ads1115_channels[] = {
-	ADS1015_V_DIFF_CHAN(0, 1, ADS1015_AIN0_AIN1, 16, 0),
-	ADS1015_V_DIFF_CHAN(0, 3, ADS1015_AIN0_AIN3, 16, 0),
-	ADS1015_V_DIFF_CHAN(1, 3, ADS1015_AIN1_AIN3, 16, 0),
-	ADS1015_V_DIFF_CHAN(2, 3, ADS1015_AIN2_AIN3, 16, 0),
-	ADS1015_V_CHAN(0, ADS1015_AIN0, 16, 0),
-	ADS1015_V_CHAN(1, ADS1015_AIN1, 16, 0),
-	ADS1015_V_CHAN(2, ADS1015_AIN2, 16, 0),
-	ADS1015_V_CHAN(3, ADS1015_AIN3, 16, 0),
+	ADS1015_V_DIFF_CHAN(0, 1, ADS1015_AIN0_AIN1, 16, 0,
+			    ads1015_events, ARRAY_SIZE(ads1015_events)),
+	ADS1015_V_DIFF_CHAN(0, 3, ADS1015_AIN0_AIN3, 16, 0,
+			    ads1015_events, ARRAY_SIZE(ads1015_events)),
+	ADS1015_V_DIFF_CHAN(1, 3, ADS1015_AIN1_AIN3, 16, 0,
+			    ads1015_events, ARRAY_SIZE(ads1015_events)),
+	ADS1015_V_DIFF_CHAN(2, 3, ADS1015_AIN2_AIN3, 16, 0,
+			    ads1015_events, ARRAY_SIZE(ads1015_events)),
+	ADS1015_V_CHAN(0, ADS1015_AIN0, 16, 0,
+		       ads1015_events, ARRAY_SIZE(ads1015_events)),
+	ADS1015_V_CHAN(1, ADS1015_AIN1, 16, 0,
+		       ads1015_events, ARRAY_SIZE(ads1015_events)),
+	ADS1015_V_CHAN(2, ADS1015_AIN2, 16, 0,
+		       ads1015_events, ARRAY_SIZE(ads1015_events)),
+	ADS1015_V_CHAN(3, ADS1015_AIN3, 16, 0,
+		       ads1015_events, ARRAY_SIZE(ads1015_events)),
 	IIO_CHAN_SOFT_TIMESTAMP(ADS1015_TIMESTAMP),
 };
 
-- 
2.35.1


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

* [PATCH v3 06/10] iio: adc: ti-ads1015: Add TLA2024 support
  2022-03-20 18:14 [PATCH v3 01/10] dt-bindings: iio: adc: ti,ads1015: Add missing ADS1115 compatible string Marek Vasut
                   ` (3 preceding siblings ...)
  2022-03-20 18:14 ` [PATCH v3 05/10] iio: adc: ti-ads1015: Make channel event_spec optional Marek Vasut
@ 2022-03-20 18:14 ` Marek Vasut
  2022-03-20 18:14 ` [PATCH v3 07/10] iio: adc: ti-ads1015: Add static assert to test if shifted realbits fit into storagebits Marek Vasut
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 25+ messages in thread
From: Marek Vasut @ 2022-03-20 18:14 UTC (permalink / raw)
  To: linux-iio
  Cc: Marek Vasut, Andy Shevchenko, Andy Shevchenko, Daniel Baluta,
	Jonathan Cameron

Add support for TI TLA2024 ADC. This chip is compatible with ADS1015
except it has no comparator in it, hence the comparator configuration
bits are missing in Configuration Register and the Hi_Thresh/Lo_Thresh
registers are missing as well and so is event support.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Daniel Baluta <daniel.baluta@nxp.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
V2: No change
V3: Use ADS1015 attr group, that's the correct one for this part
---
 drivers/iio/adc/ti-ads1015.c | 53 ++++++++++++++++++++++++++++++++++--
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 7d0c0552f425c..51ab8bb3d9f1d 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -80,6 +80,7 @@ enum chip_ids {
 	ADSXXXX = 0,
 	ADS1015,
 	ADS1115,
+	TLA2024,
 };
 
 enum ads1015_channels {
@@ -247,6 +248,22 @@ static const struct regmap_config ads1015_regmap_config = {
 	.wr_table = &ads1015_writeable_table,
 };
 
+static const struct regmap_range tla2024_writeable_ranges[] = {
+	regmap_reg_range(ADS1015_CFG_REG, ADS1015_CFG_REG),
+};
+
+static const struct regmap_access_table tla2024_writeable_table = {
+	.yes_ranges = tla2024_writeable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(tla2024_writeable_ranges),
+};
+
+static const struct regmap_config tla2024_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 16,
+	.max_register = ADS1015_CFG_REG,
+	.wr_table = &tla2024_writeable_table,
+};
+
 static const struct iio_chan_spec ads1015_channels[] = {
 	ADS1015_V_DIFF_CHAN(0, 1, ADS1015_AIN0_AIN1, 12, 4,
 			    ads1015_events, ARRAY_SIZE(ads1015_events)),
@@ -287,6 +304,19 @@ static const struct iio_chan_spec ads1115_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(ADS1015_TIMESTAMP),
 };
 
+static const struct iio_chan_spec tla2024_channels[] = {
+	ADS1015_V_DIFF_CHAN(0, 1, ADS1015_AIN0_AIN1, 12, 4, NULL, 0),
+	ADS1015_V_DIFF_CHAN(0, 3, ADS1015_AIN0_AIN3, 12, 4, NULL, 0),
+	ADS1015_V_DIFF_CHAN(1, 3, ADS1015_AIN1_AIN3, 12, 4, NULL, 0),
+	ADS1015_V_DIFF_CHAN(2, 3, ADS1015_AIN2_AIN3, 12, 4, NULL, 0),
+	ADS1015_V_CHAN(0, ADS1015_AIN0, 12, 4, NULL, 0),
+	ADS1015_V_CHAN(1, ADS1015_AIN1, 12, 4, NULL, 0),
+	ADS1015_V_CHAN(2, ADS1015_AIN2, 12, 4, NULL, 0),
+	ADS1015_V_CHAN(3, ADS1015_AIN3, 12, 4, NULL, 0),
+	IIO_CHAN_SOFT_TIMESTAMP(ADS1015_TIMESTAMP),
+};
+
+
 #ifdef CONFIG_PM
 static int ads1015_set_power_state(struct ads1015_data *data, bool on)
 {
@@ -823,6 +853,12 @@ static const struct iio_info ads1115_info = {
 	.attrs          = &ads1115_attribute_group,
 };
 
+static const struct iio_info tla2024_info = {
+	.read_raw	= ads1015_read_raw,
+	.write_raw	= ads1015_write_raw,
+	.attrs          = &ads1015_attribute_group,
+};
+
 static int ads1015_client_get_channels_config(struct i2c_client *client)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(client);
@@ -937,6 +973,12 @@ static int ads1015_probe(struct i2c_client *client,
 		indio_dev->info = &ads1115_info;
 		data->data_rate = (unsigned int *) &ads1115_data_rate;
 		break;
+	case TLA2024:
+		indio_dev->channels = tla2024_channels;
+		indio_dev->num_channels = ARRAY_SIZE(tla2024_channels);
+		indio_dev->info = &tla2024_info;
+		data->data_rate = (unsigned int *) &ads1015_data_rate;
+		break;
 	default:
 		dev_err(&client->dev, "Unknown chip %d\n", chip);
 		return -EINVAL;
@@ -957,7 +999,9 @@ static int ads1015_probe(struct i2c_client *client,
 	/* we need to keep this ABI the same as used by hwmon ADS1015 driver */
 	ads1015_get_channels_config(client);
 
-	data->regmap = devm_regmap_init_i2c(client, &ads1015_regmap_config);
+	data->regmap = devm_regmap_init_i2c(client, (chip == TLA2024) ?
+					    &tla2024_regmap_config :
+					    &ads1015_regmap_config);
 	if (IS_ERR(data->regmap)) {
 		dev_err(&client->dev, "Failed to allocate register map\n");
 		return PTR_ERR(data->regmap);
@@ -971,7 +1015,7 @@ static int ads1015_probe(struct i2c_client *client,
 		return ret;
 	}
 
-	if (client->irq) {
+	if (client->irq && chip != TLA2024) {
 		unsigned long irq_trig =
 			irqd_get_trigger_type(irq_get_irq_data(client->irq));
 		unsigned int cfg_comp_mask = ADS1015_CFG_COMP_QUE_MASK |
@@ -1073,6 +1117,7 @@ static const struct dev_pm_ops ads1015_pm_ops = {
 static const struct i2c_device_id ads1015_id[] = {
 	{"ads1015", ADS1015},
 	{"ads1115", ADS1115},
+	{"tla2024", TLA2024},
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, ads1015_id);
@@ -1086,6 +1131,10 @@ static const struct of_device_id ads1015_of_match[] = {
 		.compatible = "ti,ads1115",
 		.data = (void *)ADS1115
 	},
+	{
+		.compatible = "ti,tla2024",
+		.data = (void *)TLA2024
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, ads1015_of_match);
-- 
2.35.1


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

* [PATCH v3 07/10] iio: adc: ti-ads1015: Add static assert to test if shifted realbits fit into storagebits
  2022-03-20 18:14 [PATCH v3 01/10] dt-bindings: iio: adc: ti,ads1015: Add missing ADS1115 compatible string Marek Vasut
                   ` (4 preceding siblings ...)
  2022-03-20 18:14 ` [PATCH v3 06/10] iio: adc: ti-ads1015: Add TLA2024 support Marek Vasut
@ 2022-03-20 18:14 ` Marek Vasut
  2022-03-20 18:14 ` [PATCH v3 08/10] iio: adc: ti-ads1015: Convert to OF match data Marek Vasut
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 25+ messages in thread
From: Marek Vasut @ 2022-03-20 18:14 UTC (permalink / raw)
  To: linux-iio
  Cc: Marek Vasut, Andy Shevchenko, Andy Shevchenko, Daniel Baluta,
	Jonathan Cameron

Add compile-time static_assert wrapper to verify that shifted realbits
fit into storagebits. The macro is implemented in a more generic way so
it can be used to verify other values if required.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Daniel Baluta <daniel.baluta@nxp.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
V2: New patch
V3: No change
---
 drivers/iio/adc/ti-ads1015.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 51ab8bb3d9f1d..73d848804a12d 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -135,6 +135,28 @@ static const struct iio_event_spec ads1015_events[] = {
 	},
 };
 
+/*
+ * Compile-time check whether _fitbits can accommodate up to _testbits
+ * bits. Returns _fitbits on success, fails to compile otherwise.
+ *
+ * The test works such that it multiplies constant _fitbits by constant
+ * double-negation of size of a non-empty structure, i.e. it multiplies
+ * constant _fitbits by constant 1 in each successful compilation case.
+ * The non-empty structure may contain C11 _Static_assert(), make use of
+ * this and place the kernel variant of static assert in there, so that
+ * it performs the compile-time check for _testbits <= _fitbits. Note
+ * that it is not possible to directly use static_assert in compound
+ * statements, hence this convoluted construct.
+ */
+#define FIT_CHECK(_testbits, _fitbits)					\
+	(								\
+		(_fitbits) *						\
+		!!sizeof(struct {					\
+			static_assert((_testbits) <= (_fitbits));	\
+			int pad;					\
+		})							\
+	)
+
 #define ADS1015_V_CHAN(_chan, _addr, _realbits, _shift, _event_spec, _num_event_specs) { \
 	.type = IIO_VOLTAGE,					\
 	.indexed = 1,						\
@@ -147,7 +169,7 @@ static const struct iio_event_spec ads1015_events[] = {
 	.scan_type = {						\
 		.sign = 's',					\
 		.realbits = (_realbits),			\
-		.storagebits = 16,				\
+		.storagebits = FIT_CHECK((_realbits) + (_shift), 16),	\
 		.shift = (_shift),				\
 		.endianness = IIO_CPU,				\
 	},							\
@@ -170,7 +192,7 @@ static const struct iio_event_spec ads1015_events[] = {
 	.scan_type = {						\
 		.sign = 's',					\
 		.realbits = (_realbits),			\
-		.storagebits = 16,				\
+		.storagebits = FIT_CHECK((_realbits) + (_shift), 16),	\
 		.shift = (_shift),				\
 		.endianness = IIO_CPU,				\
 	},							\
-- 
2.35.1


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

* [PATCH v3 08/10] iio: adc: ti-ads1015: Convert to OF match data
  2022-03-20 18:14 [PATCH v3 01/10] dt-bindings: iio: adc: ti,ads1015: Add missing ADS1115 compatible string Marek Vasut
                   ` (5 preceding siblings ...)
  2022-03-20 18:14 ` [PATCH v3 07/10] iio: adc: ti-ads1015: Add static assert to test if shifted realbits fit into storagebits Marek Vasut
@ 2022-03-20 18:14 ` Marek Vasut
  2022-03-21  9:24   ` Andy Shevchenko
  2022-03-20 18:14 ` [PATCH v3 09/10] iio: adc: ti-ads1015: Replace data_rate with chip data struct ads1015_data Marek Vasut
  2022-03-20 18:14 ` [PATCH v3 10/10] iio: adc: ti-ads1015: Switch to read_avail Marek Vasut
  8 siblings, 1 reply; 25+ messages in thread
From: Marek Vasut @ 2022-03-20 18:14 UTC (permalink / raw)
  To: linux-iio
  Cc: Marek Vasut, Andy Shevchenko, Andy Shevchenko, Daniel Baluta,
	Jonathan Cameron

Replace chip type enumeration in match data with pointer to static constant
structure which contain all the different chip properties in one place, and
then replace handling of chip type in probe() with simple copy of fields in
the new match data structure into struct iio_dev.

This reduces code and increases static data.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Daniel Baluta <daniel.baluta@nxp.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
V3: New patch
---
 drivers/iio/adc/ti-ads1015.c | 109 ++++++++++++++++++-----------------
 1 file changed, 56 insertions(+), 53 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 73d848804a12d..19e75ebdddd49 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -76,11 +76,12 @@
 #define ADS1015_DEFAULT_DATA_RATE	4
 #define ADS1015_DEFAULT_CHAN		0
 
-enum chip_ids {
-	ADSXXXX = 0,
-	ADS1015,
-	ADS1115,
-	TLA2024,
+struct ads1015_chip_data {
+	struct iio_chan_spec const	*channels;
+	int				num_channels;
+	const struct iio_info		*info;
+	const unsigned int		*data_rate;
+	bool				has_comparator;
 };
 
 enum ads1015_channels {
@@ -226,7 +227,7 @@ struct ads1015_data {
 	unsigned int comp_mode;
 	struct ads1015_thresh_data thresh_data[ADS1015_CHANNELS];
 
-	unsigned int *data_rate;
+	const unsigned int *data_rate;
 	/*
 	 * Set to true when the ADC is switched to the continuous-conversion
 	 * mode and exits from a power-down state.  This flag is used to avoid
@@ -961,12 +962,21 @@ static int ads1015_set_conv_mode(struct ads1015_data *data, int mode)
 static int ads1015_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
+	const struct ads1015_chip_data *chip;
 	struct iio_dev *indio_dev;
 	struct ads1015_data *data;
 	int ret;
-	enum chip_ids chip;
 	int i;
 
+	chip = (const struct ads1015_chip_data *)
+		device_get_match_data(&client->dev);
+	if (!chip)
+		chip = (const struct ads1015_chip_data *)id->driver_data;
+	if (!chip) {
+		dev_err(&client->dev, "Unknown chip\n");
+		return -EINVAL;
+	}
+
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
 	if (!indio_dev)
 		return -ENOMEM;
@@ -979,34 +989,12 @@ static int ads1015_probe(struct i2c_client *client,
 	indio_dev->name = ADS1015_DRV_NAME;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	chip = (uintptr_t)device_get_match_data(&client->dev);
-	if (chip == ADSXXXX)
-		chip = id->driver_data;
-	switch (chip) {
-	case ADS1015:
-		indio_dev->channels = ads1015_channels;
-		indio_dev->num_channels = ARRAY_SIZE(ads1015_channels);
-		indio_dev->info = &ads1015_info;
-		data->data_rate = (unsigned int *) &ads1015_data_rate;
-		break;
-	case ADS1115:
-		indio_dev->channels = ads1115_channels;
-		indio_dev->num_channels = ARRAY_SIZE(ads1115_channels);
-		indio_dev->info = &ads1115_info;
-		data->data_rate = (unsigned int *) &ads1115_data_rate;
-		break;
-	case TLA2024:
-		indio_dev->channels = tla2024_channels;
-		indio_dev->num_channels = ARRAY_SIZE(tla2024_channels);
-		indio_dev->info = &tla2024_info;
-		data->data_rate = (unsigned int *) &ads1015_data_rate;
-		break;
-	default:
-		dev_err(&client->dev, "Unknown chip %d\n", chip);
-		return -EINVAL;
-	}
-
+	indio_dev->channels = chip->channels;
+	indio_dev->num_channels = chip->num_channels;
+	indio_dev->info = chip->info;
+	data->data_rate = chip->data_rate;
 	data->event_channel = ADS1015_CHANNELS;
+
 	/*
 	 * Set default lower and upper threshold to min and max value
 	 * respectively.
@@ -1021,9 +1009,9 @@ static int ads1015_probe(struct i2c_client *client,
 	/* we need to keep this ABI the same as used by hwmon ADS1015 driver */
 	ads1015_get_channels_config(client);
 
-	data->regmap = devm_regmap_init_i2c(client, (chip == TLA2024) ?
-					    &tla2024_regmap_config :
-					    &ads1015_regmap_config);
+	data->regmap = devm_regmap_init_i2c(client, chip->has_comparator ?
+					    &ads1015_regmap_config :
+					    &tla2024_regmap_config);
 	if (IS_ERR(data->regmap)) {
 		dev_err(&client->dev, "Failed to allocate register map\n");
 		return PTR_ERR(data->regmap);
@@ -1037,7 +1025,7 @@ static int ads1015_probe(struct i2c_client *client,
 		return ret;
 	}
 
-	if (client->irq && chip != TLA2024) {
+	if (client->irq && chip->has_comparator) {
 		unsigned long irq_trig =
 			irqd_get_trigger_type(irq_get_irq_data(client->irq));
 		unsigned int cfg_comp_mask = ADS1015_CFG_COMP_QUE_MASK |
@@ -1136,27 +1124,42 @@ static const struct dev_pm_ops ads1015_pm_ops = {
 			   ads1015_runtime_resume, NULL)
 };
 
+static const struct ads1015_chip_data ads1015_data = {
+	.channels	= ads1015_channels,
+	.num_channels	= ARRAY_SIZE(ads1015_channels),
+	.info		= &ads1015_info,
+	.data_rate	= ads1015_data_rate,
+	.has_comparator	= true,
+};
+
+static const struct ads1015_chip_data ads1115_data = {
+	.channels	= ads1115_channels,
+	.num_channels	= ARRAY_SIZE(ads1115_channels),
+	.info		= &ads1115_info,
+	.data_rate	= ads1115_data_rate,
+	.has_comparator	= true,
+};
+
+static const struct ads1015_chip_data tla2024_data = {
+	.channels	= tla2024_channels,
+	.num_channels	= ARRAY_SIZE(tla2024_channels),
+	.info		= &tla2024_info,
+	.data_rate	= ads1015_data_rate,
+	.has_comparator	= false,
+};
+
 static const struct i2c_device_id ads1015_id[] = {
-	{"ads1015", ADS1015},
-	{"ads1115", ADS1115},
-	{"tla2024", TLA2024},
+	{ "ads1015", (kernel_ulong_t)&ads1015_data },
+	{ "ads1115", (kernel_ulong_t)&ads1115_data },
+	{ "tla2024", (kernel_ulong_t)&tla2024_data },
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, ads1015_id);
 
 static const struct of_device_id ads1015_of_match[] = {
-	{
-		.compatible = "ti,ads1015",
-		.data = (void *)ADS1015
-	},
-	{
-		.compatible = "ti,ads1115",
-		.data = (void *)ADS1115
-	},
-	{
-		.compatible = "ti,tla2024",
-		.data = (void *)TLA2024
-	},
+	{ .compatible = "ti,ads1015", .data = &ads1015_data },
+	{ .compatible = "ti,ads1115", .data = &ads1115_data },
+	{ .compatible = "ti,tla2024", .data = &tla2024_data },
 	{}
 };
 MODULE_DEVICE_TABLE(of, ads1015_of_match);
-- 
2.35.1


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

* [PATCH v3 09/10] iio: adc: ti-ads1015: Replace data_rate with chip data struct ads1015_data
  2022-03-20 18:14 [PATCH v3 01/10] dt-bindings: iio: adc: ti,ads1015: Add missing ADS1115 compatible string Marek Vasut
                   ` (6 preceding siblings ...)
  2022-03-20 18:14 ` [PATCH v3 08/10] iio: adc: ti-ads1015: Convert to OF match data Marek Vasut
@ 2022-03-20 18:14 ` Marek Vasut
  2022-03-21  9:26   ` Andy Shevchenko
  2022-03-20 18:14 ` [PATCH v3 10/10] iio: adc: ti-ads1015: Switch to read_avail Marek Vasut
  8 siblings, 1 reply; 25+ messages in thread
From: Marek Vasut @ 2022-03-20 18:14 UTC (permalink / raw)
  To: linux-iio
  Cc: Marek Vasut, Andy Shevchenko, Andy Shevchenko, Daniel Baluta,
	Jonathan Cameron

Instead of storing only data_rate in private data, store pointer to the
whole chip data and use the data_rate from chip data throughout the driver.
No functional change. This is done in preparation for switch to read_avail().

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Daniel Baluta <daniel.baluta@nxp.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
V3: New patch
---
 drivers/iio/adc/ti-ads1015.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 19e75ebdddd49..66431d1445d9b 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -227,7 +227,7 @@ struct ads1015_data {
 	unsigned int comp_mode;
 	struct ads1015_thresh_data thresh_data[ADS1015_CHANNELS];
 
-	const unsigned int *data_rate;
+	const struct ads1015_chip_data *chip;
 	/*
 	 * Set to true when the ADC is switched to the continuous-conversion
 	 * mode and exits from a power-down state.  This flag is used to avoid
@@ -368,6 +368,7 @@ static int ads1015_set_power_state(struct ads1015_data *data, bool on)
 static
 int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
 {
+	const unsigned int *data_rate = data->chip->data_rate;
 	int ret, pga, dr, dr_old, conv_time;
 	unsigned int old, mask, cfg;
 
@@ -402,8 +403,8 @@ int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
 	}
 	if (data->conv_invalid) {
 		dr_old = (old & ADS1015_CFG_DR_MASK) >> ADS1015_CFG_DR_SHIFT;
-		conv_time = DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr_old]);
-		conv_time += DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr]);
+		conv_time = DIV_ROUND_UP(USEC_PER_SEC, data_rate[dr_old]);
+		conv_time += DIV_ROUND_UP(USEC_PER_SEC, data_rate[dr]);
 		conv_time += conv_time / 10; /* 10% internal clock inaccuracy */
 		usleep_range(conv_time, conv_time + 1);
 		data->conv_invalid = false;
@@ -470,7 +471,7 @@ static int ads1015_set_data_rate(struct ads1015_data *data, int chan, int rate)
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(ads1015_data_rate); i++) {
-		if (data->data_rate[i] == rate) {
+		if (data->chip->data_rate[i] == rate) {
 			data->channel_data[chan].data_rate = i;
 			return 0;
 		}
@@ -528,7 +529,7 @@ static int ads1015_read_raw(struct iio_dev *indio_dev,
 		break;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		idx = data->channel_data[chan->address].data_rate;
-		*val = data->data_rate[idx];
+		*val = data->chip->data_rate[idx];
 		ret = IIO_VAL_INT;
 		break;
 	default:
@@ -588,7 +589,7 @@ static int ads1015_read_event(struct iio_dev *indio_dev,
 		dr = data->channel_data[chan->address].data_rate;
 		comp_queue = data->thresh_data[chan->address].comp_queue;
 		period = ads1015_comp_queue[comp_queue] *
-			USEC_PER_SEC / data->data_rate[dr];
+			USEC_PER_SEC / data->chip->data_rate[dr];
 
 		*val = period / USEC_PER_SEC;
 		*val2 = period % USEC_PER_SEC;
@@ -610,6 +611,7 @@ static int ads1015_write_event(struct iio_dev *indio_dev,
 	int val2)
 {
 	struct ads1015_data *data = iio_priv(indio_dev);
+	const unsigned int *data_rate = data->chip->data_rate;
 	int realbits = chan->scan_type.realbits;
 	int ret = 0;
 	long long period;
@@ -635,7 +637,7 @@ static int ads1015_write_event(struct iio_dev *indio_dev,
 
 		for (i = 0; i < ARRAY_SIZE(ads1015_comp_queue) - 1; i++) {
 			if (period <= ads1015_comp_queue[i] *
-					USEC_PER_SEC / data->data_rate[dr])
+					USEC_PER_SEC / data_rate[dr])
 				break;
 		}
 		data->thresh_data[chan->address].comp_queue = i;
@@ -992,7 +994,7 @@ static int ads1015_probe(struct i2c_client *client,
 	indio_dev->channels = chip->channels;
 	indio_dev->num_channels = chip->num_channels;
 	indio_dev->info = chip->info;
-	data->data_rate = chip->data_rate;
+	data->chip = chip;
 	data->event_channel = ADS1015_CHANNELS;
 
 	/*
-- 
2.35.1


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

* [PATCH v3 10/10] iio: adc: ti-ads1015: Switch to read_avail
  2022-03-20 18:14 [PATCH v3 01/10] dt-bindings: iio: adc: ti,ads1015: Add missing ADS1115 compatible string Marek Vasut
                   ` (7 preceding siblings ...)
  2022-03-20 18:14 ` [PATCH v3 09/10] iio: adc: ti-ads1015: Replace data_rate with chip data struct ads1015_data Marek Vasut
@ 2022-03-20 18:14 ` Marek Vasut
  2022-03-21  9:27   ` Andy Shevchenko
  2022-03-22 21:15   ` Jonathan Cameron
  8 siblings, 2 replies; 25+ messages in thread
From: Marek Vasut @ 2022-03-20 18:14 UTC (permalink / raw)
  To: linux-iio
  Cc: Marek Vasut, Andy Shevchenko, Andy Shevchenko, Daniel Baluta,
	Jonathan Cameron

Replace sysfs attributes with read_avail() callback. This also permits
removal of ads1115_info, since the scale attribute tables are now part
of chip data.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Daniel Baluta <daniel.baluta@nxp.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
V3: New patch
---
 drivers/iio/adc/ti-ads1015.c | 102 +++++++++++++++++++----------------
 1 file changed, 57 insertions(+), 45 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 66431d1445d9b..b1257f65d7431 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -81,6 +81,9 @@ struct ads1015_chip_data {
 	int				num_channels;
 	const struct iio_info		*info;
 	const unsigned int		*data_rate;
+	const unsigned int		data_rate_len;
+	const unsigned int		*scale;
+	const unsigned int		scale_len;
 	bool				has_comparator;
 };
 
@@ -108,10 +111,18 @@ static const unsigned int ads1115_data_rate[] = {
  * Translation from PGA bits to full-scale positive and negative input voltage
  * range in mV
  */
-static int ads1015_fullscale_range[] = {
+static const int ads1015_fullscale_range[] = {
 	6144, 4096, 2048, 1024, 512, 256, 256, 256
 };
 
+static const int ads1015_scale[] = {	/* 12bit ADC */
+	256, 11, 512, 11, 1024, 11, 2048, 11, 4096, 11, 6144, 11
+};
+
+static const int ads1115_scale[] = {	/* 16bit ADC */
+	256, 15, 512, 15, 1024, 15, 2048, 15, 4096, 15, 6144, 15
+};
+
 /*
  * Translation from COMP_QUE field value to the number of successive readings
  * exceed the threshold values before an interrupt is generated
@@ -166,6 +177,9 @@ static const struct iio_event_spec ads1015_events[] = {
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
 				BIT(IIO_CHAN_INFO_SCALE) |	\
 				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+	.info_mask_shared_by_all_available =			\
+				BIT(IIO_CHAN_INFO_SCALE) |	\
+				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
 	.scan_index = _addr,					\
 	.scan_type = {						\
 		.sign = 's',					\
@@ -189,6 +203,9 @@ static const struct iio_event_spec ads1015_events[] = {
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
 				BIT(IIO_CHAN_INFO_SCALE) |	\
 				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+	.info_mask_shared_by_all_available =			\
+				BIT(IIO_CHAN_INFO_SCALE) |	\
+				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
 	.scan_index = _addr,					\
 	.scan_type = {						\
 		.sign = 's',					\
@@ -470,7 +487,7 @@ static int ads1015_set_data_rate(struct ads1015_data *data, int chan, int rate)
 {
 	int i;
 
-	for (i = 0; i < ARRAY_SIZE(ads1015_data_rate); i++) {
+	for (i = 0; i < data->chip->data_rate_len; i++) {
 		if (data->chip->data_rate[i] == rate) {
 			data->channel_data[chan].data_rate = i;
 			return 0;
@@ -480,6 +497,32 @@ static int ads1015_set_data_rate(struct ads1015_data *data, int chan, int rate)
 	return -EINVAL;
 }
 
+static int ads1015_read_avail(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      const int **vals, int *type, int *length,
+			      long mask)
+{
+	struct ads1015_data *data = iio_priv(indio_dev);
+
+	if (chan->type != IIO_VOLTAGE)
+		return -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*type = IIO_VAL_FRACTIONAL_LOG2;
+		*vals =  data->chip->scale;
+		*length = data->chip->scale_len;
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*type = IIO_VAL_INT;
+		*vals = data->chip->data_rate;
+		*length = data->chip->data_rate_len;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
 static int ads1015_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan, int *val,
 			    int *val2, long mask)
@@ -828,60 +871,20 @@ static const struct iio_buffer_setup_ops ads1015_buffer_setup_ops = {
 	.validate_scan_mask = &iio_validate_scan_mask_onehot,
 };
 
-static IIO_CONST_ATTR_NAMED(ads1015_scale_available, scale_available,
-	"3 2 1 0.5 0.25 0.125");
-static IIO_CONST_ATTR_NAMED(ads1115_scale_available, scale_available,
-	"0.1875 0.125 0.0625 0.03125 0.015625 0.007813");
-
-static IIO_CONST_ATTR_NAMED(ads1015_sampling_frequency_available,
-	sampling_frequency_available, "128 250 490 920 1600 2400 3300");
-static IIO_CONST_ATTR_NAMED(ads1115_sampling_frequency_available,
-	sampling_frequency_available, "8 16 32 64 128 250 475 860");
-
-static struct attribute *ads1015_attributes[] = {
-	&iio_const_attr_ads1015_scale_available.dev_attr.attr,
-	&iio_const_attr_ads1015_sampling_frequency_available.dev_attr.attr,
-	NULL,
-};
-
-static const struct attribute_group ads1015_attribute_group = {
-	.attrs = ads1015_attributes,
-};
-
-static struct attribute *ads1115_attributes[] = {
-	&iio_const_attr_ads1115_scale_available.dev_attr.attr,
-	&iio_const_attr_ads1115_sampling_frequency_available.dev_attr.attr,
-	NULL,
-};
-
-static const struct attribute_group ads1115_attribute_group = {
-	.attrs = ads1115_attributes,
-};
-
 static const struct iio_info ads1015_info = {
+	.read_avail	= ads1015_read_avail,
 	.read_raw	= ads1015_read_raw,
 	.write_raw	= ads1015_write_raw,
 	.read_event_value = ads1015_read_event,
 	.write_event_value = ads1015_write_event,
 	.read_event_config = ads1015_read_event_config,
 	.write_event_config = ads1015_write_event_config,
-	.attrs          = &ads1015_attribute_group,
-};
-
-static const struct iio_info ads1115_info = {
-	.read_raw	= ads1015_read_raw,
-	.write_raw	= ads1015_write_raw,
-	.read_event_value = ads1015_read_event,
-	.write_event_value = ads1015_write_event,
-	.read_event_config = ads1015_read_event_config,
-	.write_event_config = ads1015_write_event_config,
-	.attrs          = &ads1115_attribute_group,
 };
 
 static const struct iio_info tla2024_info = {
+	.read_avail	= ads1015_read_avail,
 	.read_raw	= ads1015_read_raw,
 	.write_raw	= ads1015_write_raw,
-	.attrs          = &ads1015_attribute_group,
 };
 
 static int ads1015_client_get_channels_config(struct i2c_client *client)
@@ -1131,14 +1134,20 @@ static const struct ads1015_chip_data ads1015_data = {
 	.num_channels	= ARRAY_SIZE(ads1015_channels),
 	.info		= &ads1015_info,
 	.data_rate	= ads1015_data_rate,
+	.data_rate_len	= ARRAY_SIZE(ads1015_data_rate),
+	.scale		= ads1015_scale,
+	.scale_len	= ARRAY_SIZE(ads1015_scale),
 	.has_comparator	= true,
 };
 
 static const struct ads1015_chip_data ads1115_data = {
 	.channels	= ads1115_channels,
 	.num_channels	= ARRAY_SIZE(ads1115_channels),
-	.info		= &ads1115_info,
+	.info		= &ads1015_info,
 	.data_rate	= ads1115_data_rate,
+	.data_rate_len	= ARRAY_SIZE(ads1115_data_rate),
+	.scale		= ads1115_scale,
+	.scale_len	= ARRAY_SIZE(ads1115_scale),
 	.has_comparator	= true,
 };
 
@@ -1147,6 +1156,9 @@ static const struct ads1015_chip_data tla2024_data = {
 	.num_channels	= ARRAY_SIZE(tla2024_channels),
 	.info		= &tla2024_info,
 	.data_rate	= ads1015_data_rate,
+	.data_rate_len	= ARRAY_SIZE(ads1015_data_rate),
+	.scale		= ads1015_scale,
+	.scale_len	= ARRAY_SIZE(ads1015_scale),
 	.has_comparator	= false,
 };
 
-- 
2.35.1


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

* Re: [PATCH v3 08/10] iio: adc: ti-ads1015: Convert to OF match data
  2022-03-20 18:14 ` [PATCH v3 08/10] iio: adc: ti-ads1015: Convert to OF match data Marek Vasut
@ 2022-03-21  9:24   ` Andy Shevchenko
  2022-03-22 21:00     ` Jonathan Cameron
  0 siblings, 1 reply; 25+ messages in thread
From: Andy Shevchenko @ 2022-03-21  9:24 UTC (permalink / raw)
  To: Marek Vasut; +Cc: linux-iio, Andy Shevchenko, Daniel Baluta, Jonathan Cameron

On Sun, Mar 20, 2022 at 8:14 PM Marek Vasut <marex@denx.de> wrote:
>
> Replace chip type enumeration in match data with pointer to static constant
> structure which contain all the different chip properties in one place, and

contains

> then replace handling of chip type in probe() with simple copy of fields in
> the new match data structure into struct iio_dev.
>
> This reduces code and increases static data.

I like this change! My comments below.

...

> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>

If you use mine @kernel.org address it will be enough and reduces a
lot of noise in the commit messages.

> Cc: Daniel Baluta <daniel.baluta@nxp.com>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>

...

> +       chip = (const struct ads1015_chip_data *)
> +               device_get_match_data(&client->dev);

Redundant casting. After dropping it it will become one line.

> +       if (!chip)
> +               chip = (const struct ads1015_chip_data *)id->driver_data;

> +       if (!chip) {
> +               dev_err(&client->dev, "Unknown chip\n");
> +               return -EINVAL;

return dev_err_probe(...);

> +       }

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v3 09/10] iio: adc: ti-ads1015: Replace data_rate with chip data struct ads1015_data
  2022-03-20 18:14 ` [PATCH v3 09/10] iio: adc: ti-ads1015: Replace data_rate with chip data struct ads1015_data Marek Vasut
@ 2022-03-21  9:26   ` Andy Shevchenko
  2022-03-21 14:41     ` Marek Vasut
  0 siblings, 1 reply; 25+ messages in thread
From: Andy Shevchenko @ 2022-03-21  9:26 UTC (permalink / raw)
  To: Marek Vasut; +Cc: linux-iio, Andy Shevchenko, Daniel Baluta, Jonathan Cameron

On Sun, Mar 20, 2022 at 8:14 PM Marek Vasut <marex@denx.de> wrote:
>
> Instead of storing only data_rate in private data, store pointer to the
> whole chip data and use the data_rate from chip data throughout the driver.
> No functional change. This is done in preparation for switch to read_avail().

switching

...

>                         if (period <= ads1015_comp_queue[i] *
> -                                       USEC_PER_SEC / data->data_rate[dr])
> +                                       USEC_PER_SEC / data_rate[dr])

I would put these two to one line.

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v3 10/10] iio: adc: ti-ads1015: Switch to read_avail
  2022-03-20 18:14 ` [PATCH v3 10/10] iio: adc: ti-ads1015: Switch to read_avail Marek Vasut
@ 2022-03-21  9:27   ` Andy Shevchenko
  2022-03-21 14:44     ` Marek Vasut
  2022-03-22 21:15   ` Jonathan Cameron
  1 sibling, 1 reply; 25+ messages in thread
From: Andy Shevchenko @ 2022-03-21  9:27 UTC (permalink / raw)
  To: Marek Vasut; +Cc: linux-iio, Andy Shevchenko, Daniel Baluta, Jonathan Cameron

On Sun, Mar 20, 2022 at 8:14 PM Marek Vasut <marex@denx.de> wrote:
>
> Replace sysfs attributes with read_avail() callback. This also permits
> removal of ads1115_info, since the scale attribute tables are now part
> of chip data.

...

> +static const int ads1015_fullscale_range[] = {
>         6144, 4096, 2048, 1024, 512, 256, 256, 256

Keep a comma at the end.
Also applies to the rest of the modified data structures below.

>  };

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v3 09/10] iio: adc: ti-ads1015: Replace data_rate with chip data struct ads1015_data
  2022-03-21  9:26   ` Andy Shevchenko
@ 2022-03-21 14:41     ` Marek Vasut
  2022-03-21 16:17       ` Andy Shevchenko
  0 siblings, 1 reply; 25+ messages in thread
From: Marek Vasut @ 2022-03-21 14:41 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-iio, Andy Shevchenko, Daniel Baluta, Jonathan Cameron

On 3/21/22 10:26, Andy Shevchenko wrote:
> On Sun, Mar 20, 2022 at 8:14 PM Marek Vasut <marex@denx.de> wrote:
>>
>> Instead of storing only data_rate in private data, store pointer to the
>> whole chip data and use the data_rate from chip data throughout the driver.
>> No functional change. This is done in preparation for switch to read_avail().
> 
> switching

Fixed

> ...
> 
>>                          if (period <= ads1015_comp_queue[i] *
>> -                                       USEC_PER_SEC / data->data_rate[dr])
>> +                                       USEC_PER_SEC / data_rate[dr])
> 
> I would put these two to one line.

That'd make the line over 80 chars long, is that OK in iio now ?

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

* Re: [PATCH v3 10/10] iio: adc: ti-ads1015: Switch to read_avail
  2022-03-21  9:27   ` Andy Shevchenko
@ 2022-03-21 14:44     ` Marek Vasut
  2022-03-21 16:19       ` Andy Shevchenko
  0 siblings, 1 reply; 25+ messages in thread
From: Marek Vasut @ 2022-03-21 14:44 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-iio, Andy Shevchenko, Daniel Baluta, Jonathan Cameron

On 3/21/22 10:27, Andy Shevchenko wrote:
> On Sun, Mar 20, 2022 at 8:14 PM Marek Vasut <marex@denx.de> wrote:
>>
>> Replace sysfs attributes with read_avail() callback. This also permits
>> removal of ads1115_info, since the scale attribute tables are now part
>> of chip data.
> 
> ...
> 
>> +static const int ads1015_fullscale_range[] = {
>>          6144, 4096, 2048, 1024, 512, 256, 256, 256
> 
> Keep a comma at the end.
> Also applies to the rest of the modified data structures below.

Why ? We don't expect these arrays to grow new values, if there is ever 
some new ADC, it will likely have its own array.

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

* Re: [PATCH v3 09/10] iio: adc: ti-ads1015: Replace data_rate with chip data struct ads1015_data
  2022-03-21 14:41     ` Marek Vasut
@ 2022-03-21 16:17       ` Andy Shevchenko
  2022-03-22 20:48         ` Jonathan Cameron
  0 siblings, 1 reply; 25+ messages in thread
From: Andy Shevchenko @ 2022-03-21 16:17 UTC (permalink / raw)
  To: Marek Vasut; +Cc: linux-iio, Daniel Baluta, Jonathan Cameron

On Mon, Mar 21, 2022 at 03:41:20PM +0100, Marek Vasut wrote:
> On 3/21/22 10:26, Andy Shevchenko wrote:
> > On Sun, Mar 20, 2022 at 8:14 PM Marek Vasut <marex@denx.de> wrote:

...

> > >                          if (period <= ads1015_comp_queue[i] *
> > > -                                       USEC_PER_SEC / data->data_rate[dr])
> > > +                                       USEC_PER_SEC / data_rate[dr])

			if (period <= ads1015_comp_queue[i] * USEC_PER_SEC / data_rate[dr])

> > I would put these two to one line.
> 
> That'd make the line over 80 chars long, is that OK in iio now ?

If not (Jonathan, what's your opinion?), you can do a better splitting

			if (period <=
			    ads1015_comp_queue[i] * USEC_PER_SEC / data_rate[dr])

although I think it's worse than putting on one line.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v3 10/10] iio: adc: ti-ads1015: Switch to read_avail
  2022-03-21 14:44     ` Marek Vasut
@ 2022-03-21 16:19       ` Andy Shevchenko
  2022-03-21 19:46         ` Marek Vasut
  0 siblings, 1 reply; 25+ messages in thread
From: Andy Shevchenko @ 2022-03-21 16:19 UTC (permalink / raw)
  To: Marek Vasut; +Cc: linux-iio, Daniel Baluta, Jonathan Cameron

On Mon, Mar 21, 2022 at 03:44:24PM +0100, Marek Vasut wrote:
> On 3/21/22 10:27, Andy Shevchenko wrote:
> > On Sun, Mar 20, 2022 at 8:14 PM Marek Vasut <marex@denx.de> wrote:

...

> > > +static const int ads1015_fullscale_range[] = {
> > >          6144, 4096, 2048, 1024, 512, 256, 256, 256
> > 
> > Keep a comma at the end.
> > Also applies to the rest of the modified data structures below.
> 
> Why ? We don't expect these arrays to grow new values, if there is ever some
> new ADC, it will likely have its own array.

Just for the sake of a common style. But it's not critical,
so I leave it to the maintainers.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v3 10/10] iio: adc: ti-ads1015: Switch to read_avail
  2022-03-21 16:19       ` Andy Shevchenko
@ 2022-03-21 19:46         ` Marek Vasut
  2022-03-22 20:50           ` Jonathan Cameron
  0 siblings, 1 reply; 25+ messages in thread
From: Marek Vasut @ 2022-03-21 19:46 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-iio, Daniel Baluta, Jonathan Cameron

On 3/21/22 17:19, Andy Shevchenko wrote:
> On Mon, Mar 21, 2022 at 03:44:24PM +0100, Marek Vasut wrote:
>> On 3/21/22 10:27, Andy Shevchenko wrote:
>>> On Sun, Mar 20, 2022 at 8:14 PM Marek Vasut <marex@denx.de> wrote:
> 
> ...
> 
>>>> +static const int ads1015_fullscale_range[] = {
>>>>           6144, 4096, 2048, 1024, 512, 256, 256, 256
>>>
>>> Keep a comma at the end.
>>> Also applies to the rest of the modified data structures below.
>>
>> Why ? We don't expect these arrays to grow new values, if there is ever some
>> new ADC, it will likely have its own array.
> 
> Just for the sake of a common style. But it's not critical,
> so I leave it to the maintainers.

The common style of the other arrays in that driver is without a 
trailing comma, hence the way it is right now is the common style.

I do however understand the rationale why a trailing comma in structures 
makes sense (you don't have to add it in some subsequent patch, which 
does only cause churn and unrelated line changes), but I don't think 
that applies in this particular case.

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

* Re: [PATCH v3 09/10] iio: adc: ti-ads1015: Replace data_rate with chip data struct ads1015_data
  2022-03-21 16:17       ` Andy Shevchenko
@ 2022-03-22 20:48         ` Jonathan Cameron
  0 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2022-03-22 20:48 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Marek Vasut, linux-iio, Daniel Baluta

On Mon, 21 Mar 2022 18:17:28 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Mon, Mar 21, 2022 at 03:41:20PM +0100, Marek Vasut wrote:
> > On 3/21/22 10:26, Andy Shevchenko wrote:  
> > > On Sun, Mar 20, 2022 at 8:14 PM Marek Vasut <marex@denx.de> wrote:  
> 
> ...
> 
> > > >                          if (period <= ads1015_comp_queue[i] *
> > > > -                                       USEC_PER_SEC / data->data_rate[dr])
> > > > +                                       USEC_PER_SEC / data_rate[dr])  
> 
> 			if (period <= ads1015_comp_queue[i] * USEC_PER_SEC / data_rate[dr])
> 
> > > I would put these two to one line.  
> > 
> > That'd make the line over 80 chars long, is that OK in iio now ? 
 
Depends on readability.  Where it doesn't hurt such as line breaks in
between functional parameters I prefer we stick to sub 80 chars.

> 
> If not (Jonathan, what's your opinion?), you can do a better splitting
> 
> 			if (period <=
> 			    ads1015_comp_queue[i] * USEC_PER_SEC / data_rate[dr])
> 
> although I think it's worse than putting on one line.
> 

Agreed. This particular case is more readable on one line.


Thanks,

Jonathan



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

* Re: [PATCH v3 10/10] iio: adc: ti-ads1015: Switch to read_avail
  2022-03-21 19:46         ` Marek Vasut
@ 2022-03-22 20:50           ` Jonathan Cameron
  0 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2022-03-22 20:50 UTC (permalink / raw)
  To: Marek Vasut; +Cc: Andy Shevchenko, linux-iio, Daniel Baluta

On Mon, 21 Mar 2022 20:46:04 +0100
Marek Vasut <marex@denx.de> wrote:

> On 3/21/22 17:19, Andy Shevchenko wrote:
> > On Mon, Mar 21, 2022 at 03:44:24PM +0100, Marek Vasut wrote:  
> >> On 3/21/22 10:27, Andy Shevchenko wrote:  
> >>> On Sun, Mar 20, 2022 at 8:14 PM Marek Vasut <marex@denx.de> wrote:  
> > 
> > ...
> >   
> >>>> +static const int ads1015_fullscale_range[] = {
> >>>>           6144, 4096, 2048, 1024, 512, 256, 256, 256  
> >>>
> >>> Keep a comma at the end.
> >>> Also applies to the rest of the modified data structures below.  
> >>
> >> Why ? We don't expect these arrays to grow new values, if there is ever some
> >> new ADC, it will likely have its own array.  
> > 
> > Just for the sake of a common style. But it's not critical,
> > so I leave it to the maintainers.  
> 
> The common style of the other arrays in that driver is without a 
> trailing comma, hence the way it is right now is the common style.
> 
> I do however understand the rationale why a trailing comma in structures 
> makes sense (you don't have to add it in some subsequent patch, which 
> does only cause churn and unrelated line changes), but I don't think 
> that applies in this particular case.

Agreed. I tend to let this go with local style in a driver for
these cases and generally let either go in new drivers as long
as they are consistent.

Jonathan

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

* Re: [PATCH v3 08/10] iio: adc: ti-ads1015: Convert to OF match data
  2022-03-21  9:24   ` Andy Shevchenko
@ 2022-03-22 21:00     ` Jonathan Cameron
  2022-03-22 21:41       ` Marek Vasut
  0 siblings, 1 reply; 25+ messages in thread
From: Jonathan Cameron @ 2022-03-22 21:00 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Marek Vasut, linux-iio, Andy Shevchenko, Daniel Baluta

On Mon, 21 Mar 2022 11:24:12 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sun, Mar 20, 2022 at 8:14 PM Marek Vasut <marex@denx.de> wrote:
> >
> > Replace chip type enumeration in match data with pointer to static constant
> > structure which contain all the different chip properties in one place, and  
> 
> contains
> 
> > then replace handling of chip type in probe() with simple copy of fields in
> > the new match data structure into struct iio_dev.
> >
> > This reduces code and increases static data.  
> 
> I like this change! My comments below.
Nice work indeed. Nothing else from me on this one.

I like the fact you also got rid of some odd casting away of const
whilst you were doing this.

Jonathan

> 
> ...
> 
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: Andy Shevchenko <andy.shevchenko@gmail.com>  
> 
> If you use mine @kernel.org address it will be enough and reduces a
> lot of noise in the commit messages.
> 
> > Cc: Daniel Baluta <daniel.baluta@nxp.com>
> > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>  
> 
> ...
> 
> > +       chip = (const struct ads1015_chip_data *)
> > +               device_get_match_data(&client->dev);  
> 
> Redundant casting. After dropping it it will become one line.
> 
> > +       if (!chip)
> > +               chip = (const struct ads1015_chip_data *)id->driver_data;  
> 
> > +       if (!chip) {
> > +               dev_err(&client->dev, "Unknown chip\n");
> > +               return -EINVAL;  
> 
> return dev_err_probe(...);
> 
> > +       }  
> 


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

* Re: [PATCH v3 10/10] iio: adc: ti-ads1015: Switch to read_avail
  2022-03-20 18:14 ` [PATCH v3 10/10] iio: adc: ti-ads1015: Switch to read_avail Marek Vasut
  2022-03-21  9:27   ` Andy Shevchenko
@ 2022-03-22 21:15   ` Jonathan Cameron
  2022-03-22 21:39     ` Marek Vasut
  1 sibling, 1 reply; 25+ messages in thread
From: Jonathan Cameron @ 2022-03-22 21:15 UTC (permalink / raw)
  To: Marek Vasut; +Cc: linux-iio, Andy Shevchenko, Andy Shevchenko, Daniel Baluta

On Sun, 20 Mar 2022 19:14:28 +0100
Marek Vasut <marex@denx.de> wrote:

> Replace sysfs attributes with read_avail() callback. This also permits
> removal of ads1115_info, since the scale attribute tables are now part
> of chip data.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Daniel Baluta <daniel.baluta@nxp.com>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> V3: New patch
> ---
>  drivers/iio/adc/ti-ads1015.c | 102 +++++++++++++++++++----------------
>  1 file changed, 57 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index 66431d1445d9b..b1257f65d7431 100644
> --- a/drivers/iio/adc/ti-ads1015.c
> +++ b/drivers/iio/adc/ti-ads1015.c
> @@ -81,6 +81,9 @@ struct ads1015_chip_data {
>  	int				num_channels;
>  	const struct iio_info		*info;
>  	const unsigned int		*data_rate;
Should probably change this to signed.

> +	const unsigned int		data_rate_len;
> +	const unsigned int		*scale;

Why unsigned int given we use it as an array of signed ints?

> +	const unsigned int		scale_len;
>  	bool				has_comparator;
>  };
>  
> @@ -108,10 +111,18 @@ static const unsigned int ads1115_data_rate[] = {
>   * Translation from PGA bits to full-scale positive and negative input voltage
>   * range in mV
>   */
> -static int ads1015_fullscale_range[] = {
> +static const int ads1015_fullscale_range[] = {
Technically unrelated but good fix and not hurting patch readability significantly
so perhaps not worth a separate patch.

>  	6144, 4096, 2048, 1024, 512, 256, 256, 256
>  };
>  
> +static const int ads1015_scale[] = {	/* 12bit ADC */
> +	256, 11, 512, 11, 1024, 11, 2048, 11, 4096, 11, 6144, 11
I wonder if it's worth either using a 2D array and casting
a dimension away, or perhaps just formatting these pair wise
so we can see what is going on more obviously?

I don't feel strongly about this so up to you.

> +};
> +
> +static const int ads1115_scale[] = {	/* 16bit ADC */
> +	256, 15, 512, 15, 1024, 15, 2048, 15, 4096, 15, 6144, 15
> +};


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

* Re: [PATCH v3 10/10] iio: adc: ti-ads1015: Switch to read_avail
  2022-03-22 21:15   ` Jonathan Cameron
@ 2022-03-22 21:39     ` Marek Vasut
  2022-03-22 22:03       ` Marek Vasut
  0 siblings, 1 reply; 25+ messages in thread
From: Marek Vasut @ 2022-03-22 21:39 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, Andy Shevchenko, Andy Shevchenko, Daniel Baluta

On 3/22/22 22:15, Jonathan Cameron wrote:

[...]

>> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
>> index 66431d1445d9b..b1257f65d7431 100644
>> --- a/drivers/iio/adc/ti-ads1015.c
>> +++ b/drivers/iio/adc/ti-ads1015.c
>> @@ -81,6 +81,9 @@ struct ads1015_chip_data {
>>   	int				num_channels;
>>   	const struct iio_info		*info;
>>   	const unsigned int		*data_rate;
> Should probably change this to signed.

The data_rate values are unsigned ... why ?

>> +	const unsigned int		data_rate_len;
>> +	const unsigned int		*scale;
> 
> Why unsigned int given we use it as an array of signed ints?

Scale is also unsigned ... why ?

>> +	const unsigned int		scale_len;
>>   	bool				has_comparator;
>>   };
>>   
>> @@ -108,10 +111,18 @@ static const unsigned int ads1115_data_rate[] = {
>>    * Translation from PGA bits to full-scale positive and negative input voltage
>>    * range in mV
>>    */
>> -static int ads1015_fullscale_range[] = {
>> +static const int ads1015_fullscale_range[] = {
> Technically unrelated but good fix and not hurting patch readability significantly
> so perhaps not worth a separate patch.
> 
>>   	6144, 4096, 2048, 1024, 512, 256, 256, 256
>>   };
>>   
>> +static const int ads1015_scale[] = {	/* 12bit ADC */
>> +	256, 11, 512, 11, 1024, 11, 2048, 11, 4096, 11, 6144, 11
> I wonder if it's worth either using a 2D array and casting
> a dimension away, or perhaps just formatting these pair wise
> so we can see what is going on more obviously?
> 
> I don't feel strongly about this so up to you.

If we are trying to get rid of the type casts, then pair format it is.

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

* Re: [PATCH v3 08/10] iio: adc: ti-ads1015: Convert to OF match data
  2022-03-22 21:00     ` Jonathan Cameron
@ 2022-03-22 21:41       ` Marek Vasut
  0 siblings, 0 replies; 25+ messages in thread
From: Marek Vasut @ 2022-03-22 21:41 UTC (permalink / raw)
  To: Jonathan Cameron, Andy Shevchenko
  Cc: linux-iio, Andy Shevchenko, Daniel Baluta

On 3/22/22 22:00, Jonathan Cameron wrote:
> On Mon, 21 Mar 2022 11:24:12 +0200
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
>> On Sun, Mar 20, 2022 at 8:14 PM Marek Vasut <marex@denx.de> wrote:
>>>
>>> Replace chip type enumeration in match data with pointer to static constant
>>> structure which contain all the different chip properties in one place, and
>>
>> contains
>>
>>> then replace handling of chip type in probe() with simple copy of fields in
>>> the new match data structure into struct iio_dev.
>>>
>>> This reduces code and increases static data.
>>
>> I like this change! My comments below.
> Nice work indeed. Nothing else from me on this one.
> 
> I like the fact you also got rid of some odd casting away of const
> whilst you were doing this.

[...]

>>> +       chip = (const struct ads1015_chip_data *)
>>> +               device_get_match_data(&client->dev);
>>
>> Redundant casting. After dropping it it will become one line.
>>
>>> +       if (!chip)
>>> +               chip = (const struct ads1015_chip_data *)id->driver_data;

I don't think I can get rid of this ^ id->driver_data one though.

The device_get_match_data() yes, that cast can be removed.

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

* Re: [PATCH v3 10/10] iio: adc: ti-ads1015: Switch to read_avail
  2022-03-22 21:39     ` Marek Vasut
@ 2022-03-22 22:03       ` Marek Vasut
  0 siblings, 0 replies; 25+ messages in thread
From: Marek Vasut @ 2022-03-22 22:03 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, Andy Shevchenko, Andy Shevchenko, Daniel Baluta

On 3/22/22 22:39, Marek Vasut wrote:
> On 3/22/22 22:15, Jonathan Cameron wrote:
> 
> [...]
> 
>>> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
>>> index 66431d1445d9b..b1257f65d7431 100644
>>> --- a/drivers/iio/adc/ti-ads1015.c
>>> +++ b/drivers/iio/adc/ti-ads1015.c
>>> @@ -81,6 +81,9 @@ struct ads1015_chip_data {
>>>       int                num_channels;
>>>       const struct iio_info        *info;
>>>       const unsigned int        *data_rate;
>> Should probably change this to signed.
> 
> The data_rate values are unsigned ... why ?
> 
>>> +    const unsigned int        data_rate_len;
>>> +    const unsigned int        *scale;
>>
>> Why unsigned int given we use it as an array of signed ints?
> 
> Scale is also unsigned ... why ?

Nevermind, I turned all the ranges into unsigned int and sent V4.

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

end of thread, other threads:[~2022-03-22 22:03 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-20 18:14 [PATCH v3 01/10] dt-bindings: iio: adc: ti,ads1015: Add missing ADS1115 compatible string Marek Vasut
2022-03-20 18:14 ` [PATCH v3 02/10] dt-bindings: iio: adc: ti,ads1015: Add TLA2024 " Marek Vasut
2022-03-20 18:14 ` [PATCH v3 03/10] iio: adc: ti-ads1015: Switch to static const writeable ranges table Marek Vasut
2022-03-20 18:14 ` [PATCH v3 04/10] iio: adc: ti-ads1015: Deduplicate channel macros Marek Vasut
2022-03-20 18:14 ` [PATCH v3 05/10] iio: adc: ti-ads1015: Make channel event_spec optional Marek Vasut
2022-03-20 18:14 ` [PATCH v3 06/10] iio: adc: ti-ads1015: Add TLA2024 support Marek Vasut
2022-03-20 18:14 ` [PATCH v3 07/10] iio: adc: ti-ads1015: Add static assert to test if shifted realbits fit into storagebits Marek Vasut
2022-03-20 18:14 ` [PATCH v3 08/10] iio: adc: ti-ads1015: Convert to OF match data Marek Vasut
2022-03-21  9:24   ` Andy Shevchenko
2022-03-22 21:00     ` Jonathan Cameron
2022-03-22 21:41       ` Marek Vasut
2022-03-20 18:14 ` [PATCH v3 09/10] iio: adc: ti-ads1015: Replace data_rate with chip data struct ads1015_data Marek Vasut
2022-03-21  9:26   ` Andy Shevchenko
2022-03-21 14:41     ` Marek Vasut
2022-03-21 16:17       ` Andy Shevchenko
2022-03-22 20:48         ` Jonathan Cameron
2022-03-20 18:14 ` [PATCH v3 10/10] iio: adc: ti-ads1015: Switch to read_avail Marek Vasut
2022-03-21  9:27   ` Andy Shevchenko
2022-03-21 14:44     ` Marek Vasut
2022-03-21 16:19       ` Andy Shevchenko
2022-03-21 19:46         ` Marek Vasut
2022-03-22 20:50           ` Jonathan Cameron
2022-03-22 21:15   ` Jonathan Cameron
2022-03-22 21:39     ` Marek Vasut
2022-03-22 22:03       ` Marek Vasut

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.