* [PATCH 0/6] iio: adc: ad7380: fix several supplies issues
@ 2024-10-07 15:45 Julien Stephan
2024-10-07 15:45 ` [PATCH 1/6] dt-bindings: iio: adc: ad7380: remove voltage reference for supplies Julien Stephan
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Julien Stephan @ 2024-10-07 15:45 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sá,
David Lechner, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Jonathan Corbet
Cc: linux-iio, devicetree, linux-kernel, Conor Dooley,
Jonathan Cameron, linux-doc, Julien Stephan
Hello,
This series tries to fix several issues found on the ad7380 driver about
supplies:
- vcc and vlogic are required, but are not retrieved and enabled in the
probe function
- ad7380-4 is the only device from the family that does not have internal
reference and uses REFIN instead of REFIO for external reference.
driver, bindings, and doc are fixed accordingly
Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
Julien Stephan (6):
dt-bindings: iio: adc: ad7380: remove voltage reference for supplies
dt-bindings: iio: adc: ad7380: fix ad7380-4 reference supply
iio: adc: ad7380: use devm_regulator_get_enable_read_voltage()
iio: adc: ad7380: add missing supplies
iio: adc: ad7380: fix supplies for ad7380-4
docs: iio: ad7380: fix supply for ad7380-4
.../devicetree/bindings/iio/adc/adi,ad7380.yaml | 32 +++--
Documentation/iio/ad7380.rst | 13 +-
drivers/iio/adc/ad7380.c | 136 ++++++++++++---------
3 files changed, 110 insertions(+), 71 deletions(-)
---
base-commit: 8bea3878a1511bceadc2fbf284b00bcc5a2ef28d
change-id: 20241004-ad7380-fix-supplies-3677365cf8aa
Best regards,
--
Julien Stephan <jstephan@baylibre.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/6] dt-bindings: iio: adc: ad7380: remove voltage reference for supplies
2024-10-07 15:45 [PATCH 0/6] iio: adc: ad7380: fix several supplies issues Julien Stephan
@ 2024-10-07 15:45 ` Julien Stephan
2024-10-08 7:51 ` Krzysztof Kozlowski
2024-10-07 15:45 ` [PATCH 2/6] dt-bindings: iio: adc: ad7380: fix ad7380-4 reference supply Julien Stephan
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Julien Stephan @ 2024-10-07 15:45 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sá,
David Lechner, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Jonathan Corbet
Cc: linux-iio, devicetree, linux-kernel, Conor Dooley,
Jonathan Cameron, linux-doc, Julien Stephan
Voltages may not bo valid for future compatible parts, so remove them and
remove useless description
Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml
index bd19abb867d9..72c51b3de97b 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml
@@ -55,17 +55,9 @@ properties:
spi-cpol: true
spi-cpha: true
- vcc-supply:
- description: A 3V to 3.6V supply that powers the chip.
-
- vlogic-supply:
- description:
- A 1.65V to 3.6V supply for the logic pins.
-
- refio-supply:
- description:
- A 2.5V to 3.3V supply for the external reference voltage. When omitted,
- the internal 2.5V reference is used.
+ vcc-supply: true
+ vlogic-supply: true
+ refio-supply: true
aina-supply:
description:
--
2.46.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/6] dt-bindings: iio: adc: ad7380: fix ad7380-4 reference supply
2024-10-07 15:45 [PATCH 0/6] iio: adc: ad7380: fix several supplies issues Julien Stephan
2024-10-07 15:45 ` [PATCH 1/6] dt-bindings: iio: adc: ad7380: remove voltage reference for supplies Julien Stephan
@ 2024-10-07 15:45 ` Julien Stephan
2024-10-08 7:52 ` Krzysztof Kozlowski
2024-10-07 15:45 ` [PATCH 3/6] iio: adc: ad7380: use devm_regulator_get_enable_read_voltage() Julien Stephan
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Julien Stephan @ 2024-10-07 15:45 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sá,
David Lechner, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Jonathan Corbet
Cc: linux-iio, devicetree, linux-kernel, Conor Dooley,
Jonathan Cameron, linux-doc, Julien Stephan
ad7380-4 is the only device from ad738x family that doesn't have an
internal reference. Moreover its external reference is called REFIN in
the datasheet while all other use REFIO as an optional external
reference. If refio-supply is omitted the internal reference is
used.
Fix the binding by adding refin-supply and makes it required for
ad7380-4 only.
Fixes: 1a291cc8ee17 ("dt-bindings: iio: adc: ad7380: add support for ad738x-4 4 channels variants")
Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
.../devicetree/bindings/iio/adc/adi,ad7380.yaml | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml
index 72c51b3de97b..74d82721637c 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml
@@ -58,6 +58,7 @@ properties:
vcc-supply: true
vlogic-supply: true
refio-supply: true
+ refin-supply: true
aina-supply:
description:
@@ -127,6 +128,23 @@ allOf:
ainc-supply: false
aind-supply: false
+ # ad7380-4 uses refin-supply as external reference.
+ # All other chips from ad738x family use refio as optional external reference.
+ # When refio-supply is omitted, internal reference is used.
+ - if:
+ properties:
+ compatible:
+ enum:
+ - adi,ad7380-4
+ then:
+ properties:
+ refio-supply: false
+ required:
+ - refin-supply
+ else:
+ properties:
+ refin-supply: false
+
examples:
- |
#include <dt-bindings/interrupt-controller/irq.h>
--
2.46.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/6] iio: adc: ad7380: use devm_regulator_get_enable_read_voltage()
2024-10-07 15:45 [PATCH 0/6] iio: adc: ad7380: fix several supplies issues Julien Stephan
2024-10-07 15:45 ` [PATCH 1/6] dt-bindings: iio: adc: ad7380: remove voltage reference for supplies Julien Stephan
2024-10-07 15:45 ` [PATCH 2/6] dt-bindings: iio: adc: ad7380: fix ad7380-4 reference supply Julien Stephan
@ 2024-10-07 15:45 ` Julien Stephan
2024-10-07 15:45 ` [PATCH 4/6] iio: adc: ad7380: add missing supplies Julien Stephan
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Julien Stephan @ 2024-10-07 15:45 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sá,
David Lechner, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Jonathan Corbet
Cc: linux-iio, devicetree, linux-kernel, Conor Dooley,
Jonathan Cameron, linux-doc, Julien Stephan
Use devm_regulator_get_enable_read_voltage() to simplify the code.
Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
drivers/iio/adc/ad7380.c | 81 +++++++++++++-----------------------------------
1 file changed, 21 insertions(+), 60 deletions(-)
diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
index e8bddfb0d07d..e033c7341911 100644
--- a/drivers/iio/adc/ad7380.c
+++ b/drivers/iio/adc/ad7380.c
@@ -956,7 +956,7 @@ static const struct iio_info ad7380_info = {
.debugfs_reg_access = &ad7380_debugfs_reg_access,
};
-static int ad7380_init(struct ad7380_state *st, struct regulator *vref)
+static int ad7380_init(struct ad7380_state *st, bool external_ref_en)
{
int ret;
@@ -968,13 +968,13 @@ static int ad7380_init(struct ad7380_state *st, struct regulator *vref)
if (ret < 0)
return ret;
- /* select internal or external reference voltage */
- ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG1,
- AD7380_CONFIG1_REFSEL,
- FIELD_PREP(AD7380_CONFIG1_REFSEL,
- vref ? 1 : 0));
- if (ret < 0)
- return ret;
+ if (external_ref_en) {
+ /* select external reference voltage */
+ ret = regmap_set_bits(st->regmap, AD7380_REG_ADDR_CONFIG1,
+ AD7380_CONFIG1_REFSEL);
+ if (ret < 0)
+ return ret;
+ }
/* This is the default value after reset. */
st->oversampling_ratio = 1;
@@ -987,16 +987,11 @@ static int ad7380_init(struct ad7380_state *st, struct regulator *vref)
FIELD_PREP(AD7380_CONFIG2_SDO, 1));
}
-static void ad7380_regulator_disable(void *p)
-{
- regulator_disable(p);
-}
-
static int ad7380_probe(struct spi_device *spi)
{
struct iio_dev *indio_dev;
struct ad7380_state *st;
- struct regulator *vref;
+ bool external_ref_en;
int ret, i;
indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
@@ -1009,37 +1004,17 @@ static int ad7380_probe(struct spi_device *spi)
if (!st->chip_info)
return dev_err_probe(&spi->dev, -EINVAL, "missing match data\n");
- vref = devm_regulator_get_optional(&spi->dev, "refio");
- if (IS_ERR(vref)) {
- if (PTR_ERR(vref) != -ENODEV)
- return dev_err_probe(&spi->dev, PTR_ERR(vref),
- "Failed to get refio regulator\n");
-
- vref = NULL;
- }
-
/*
* If there is no REFIO supply, then it means that we are using
* the internal 2.5V reference, otherwise REFIO is reference voltage.
*/
- if (vref) {
- ret = regulator_enable(vref);
- if (ret)
- return ret;
+ ret = devm_regulator_get_enable_read_voltage(&spi->dev, "refio");
+ if (ret < 0 && ret != -ENODEV)
+ return dev_err_probe(&spi->dev, ret,
+ "Failed to get refio regulator\n");
- ret = devm_add_action_or_reset(&spi->dev,
- ad7380_regulator_disable, vref);
- if (ret)
- return ret;
-
- ret = regulator_get_voltage(vref);
- if (ret < 0)
- return ret;
-
- st->vref_mv = ret / 1000;
- } else {
- st->vref_mv = AD7380_INTERNAL_REF_MV;
- }
+ external_ref_en = ret != -ENODEV;
+ st->vref_mv = external_ref_en ? ret / 1000 : AD7380_INTERNAL_REF_MV;
if (st->chip_info->num_vcm_supplies > ARRAY_SIZE(st->vcm_mv))
return dev_err_probe(&spi->dev, -EINVAL,
@@ -1050,27 +1025,13 @@ static int ad7380_probe(struct spi_device *spi)
* input pin.
*/
for (i = 0; i < st->chip_info->num_vcm_supplies; i++) {
- struct regulator *vcm;
-
- vcm = devm_regulator_get(&spi->dev,
- st->chip_info->vcm_supplies[i]);
- if (IS_ERR(vcm))
- return dev_err_probe(&spi->dev, PTR_ERR(vcm),
- "Failed to get %s regulator\n",
- st->chip_info->vcm_supplies[i]);
+ const char *vcm = st->chip_info->vcm_supplies[i];
- ret = regulator_enable(vcm);
- if (ret)
- return ret;
-
- ret = devm_add_action_or_reset(&spi->dev,
- ad7380_regulator_disable, vcm);
- if (ret)
- return ret;
-
- ret = regulator_get_voltage(vcm);
+ ret = devm_regulator_get_enable_read_voltage(&spi->dev, vcm);
if (ret < 0)
- return ret;
+ return dev_err_probe(&spi->dev, ret,
+ "Failed to get %s regulator\n",
+ vcm);
st->vcm_mv[i] = ret / 1000;
}
@@ -1135,7 +1096,7 @@ static int ad7380_probe(struct spi_device *spi)
if (ret)
return ret;
- ret = ad7380_init(st, vref);
+ ret = ad7380_init(st, external_ref_en);
if (ret)
return ret;
--
2.46.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/6] iio: adc: ad7380: add missing supplies
2024-10-07 15:45 [PATCH 0/6] iio: adc: ad7380: fix several supplies issues Julien Stephan
` (2 preceding siblings ...)
2024-10-07 15:45 ` [PATCH 3/6] iio: adc: ad7380: use devm_regulator_get_enable_read_voltage() Julien Stephan
@ 2024-10-07 15:45 ` Julien Stephan
2024-10-08 8:31 ` kernel test robot
2024-10-07 15:45 ` [PATCH 5/6] iio: adc: ad7380: fix supplies for ad7380-4 Julien Stephan
2024-10-07 15:45 ` [PATCH 6/6] docs: iio: ad7380: fix supply " Julien Stephan
5 siblings, 1 reply; 15+ messages in thread
From: Julien Stephan @ 2024-10-07 15:45 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sá,
David Lechner, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Jonathan Corbet
Cc: linux-iio, devicetree, linux-kernel, Conor Dooley,
Jonathan Cameron, linux-doc, Julien Stephan
vcc and vlogic are required but are not retrieved and enabled in the
probe. Add them.
In order to prepare support for additional parts requiring different
supplies, add vcc and vlogic to the platform specific structures
Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
drivers/iio/adc/ad7380.c | 43 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
index e033c7341911..9ef44b605144 100644
--- a/drivers/iio/adc/ad7380.c
+++ b/drivers/iio/adc/ad7380.c
@@ -75,6 +75,7 @@
#define T_CONVERT_NS 190 /* conversion time */
#define T_CONVERT_0_NS 10 /* 1st conversion start time (oversampling) */
#define T_CONVERT_X_NS 500 /* xth conversion start time (oversampling) */
+#define T_POWERUP_MS 5 /* Power up */
struct ad7380_timing_specs {
const unsigned int t_csh_ns; /* CS minimum high time */
@@ -86,6 +87,8 @@ struct ad7380_chip_info {
unsigned int num_channels;
unsigned int num_simult_channels;
bool has_mux;
+ const char * const *supplies;
+ unsigned int num_supplies;
const char * const *vcm_supplies;
unsigned int num_vcm_supplies;
const unsigned long *available_scan_masks;
@@ -243,6 +246,10 @@ DEFINE_AD7380_8_CHANNEL(ad7386_4_channels, 16, 0, u);
DEFINE_AD7380_8_CHANNEL(ad7387_4_channels, 14, 0, u);
DEFINE_AD7380_8_CHANNEL(ad7388_4_channels, 12, 0, u);
+static const char * const ad7380_supplies[] = {
+ "vcc", "vlogic",
+};
+
static const char * const ad7380_2_channel_vcm_supplies[] = {
"aina", "ainb",
};
@@ -338,6 +345,8 @@ static const struct ad7380_chip_info ad7380_chip_info = {
.channels = ad7380_channels,
.num_channels = ARRAY_SIZE(ad7380_channels),
.num_simult_channels = 2,
+ .supplies = ad7380_supplies,
+ .num_supplies = ARRAY_SIZE(ad7380_supplies),
.available_scan_masks = ad7380_2_channel_scan_masks,
.timing_specs = &ad7380_timing,
};
@@ -347,6 +356,8 @@ static const struct ad7380_chip_info ad7381_chip_info = {
.channels = ad7381_channels,
.num_channels = ARRAY_SIZE(ad7381_channels),
.num_simult_channels = 2,
+ .supplies = ad7380_supplies,
+ .num_supplies = ARRAY_SIZE(ad7380_supplies),
.available_scan_masks = ad7380_2_channel_scan_masks,
.timing_specs = &ad7380_timing,
};
@@ -356,6 +367,8 @@ static const struct ad7380_chip_info ad7383_chip_info = {
.channels = ad7383_channels,
.num_channels = ARRAY_SIZE(ad7383_channels),
.num_simult_channels = 2,
+ .supplies = ad7380_supplies,
+ .num_supplies = ARRAY_SIZE(ad7380_supplies),
.vcm_supplies = ad7380_2_channel_vcm_supplies,
.num_vcm_supplies = ARRAY_SIZE(ad7380_2_channel_vcm_supplies),
.available_scan_masks = ad7380_2_channel_scan_masks,
@@ -367,6 +380,8 @@ static const struct ad7380_chip_info ad7384_chip_info = {
.channels = ad7384_channels,
.num_channels = ARRAY_SIZE(ad7384_channels),
.num_simult_channels = 2,
+ .supplies = ad7380_supplies,
+ .num_supplies = ARRAY_SIZE(ad7380_supplies),
.vcm_supplies = ad7380_2_channel_vcm_supplies,
.num_vcm_supplies = ARRAY_SIZE(ad7380_2_channel_vcm_supplies),
.available_scan_masks = ad7380_2_channel_scan_masks,
@@ -378,6 +393,8 @@ static const struct ad7380_chip_info ad7386_chip_info = {
.channels = ad7386_channels,
.num_channels = ARRAY_SIZE(ad7386_channels),
.num_simult_channels = 2,
+ .supplies = ad7380_supplies,
+ .num_supplies = ARRAY_SIZE(ad7380_supplies),
.has_mux = true,
.available_scan_masks = ad7380_2x2_channel_scan_masks,
.timing_specs = &ad7380_timing,
@@ -388,6 +405,8 @@ static const struct ad7380_chip_info ad7387_chip_info = {
.channels = ad7387_channels,
.num_channels = ARRAY_SIZE(ad7387_channels),
.num_simult_channels = 2,
+ .supplies = ad7380_supplies,
+ .num_supplies = ARRAY_SIZE(ad7380_supplies),
.has_mux = true,
.available_scan_masks = ad7380_2x2_channel_scan_masks,
.timing_specs = &ad7380_timing,
@@ -398,6 +417,8 @@ static const struct ad7380_chip_info ad7388_chip_info = {
.channels = ad7388_channels,
.num_channels = ARRAY_SIZE(ad7388_channels),
.num_simult_channels = 2,
+ .supplies = ad7380_supplies,
+ .num_supplies = ARRAY_SIZE(ad7380_supplies),
.has_mux = true,
.available_scan_masks = ad7380_2x2_channel_scan_masks,
.timing_specs = &ad7380_timing,
@@ -408,6 +429,8 @@ static const struct ad7380_chip_info ad7380_4_chip_info = {
.channels = ad7380_4_channels,
.num_channels = ARRAY_SIZE(ad7380_4_channels),
.num_simult_channels = 4,
+ .supplies = ad7380_supplies,
+ .num_supplies = ARRAY_SIZE(ad7380_supplies),
.available_scan_masks = ad7380_4_channel_scan_masks,
.timing_specs = &ad7380_4_timing,
};
@@ -417,6 +440,8 @@ static const struct ad7380_chip_info ad7381_4_chip_info = {
.channels = ad7381_4_channels,
.num_channels = ARRAY_SIZE(ad7381_4_channels),
.num_simult_channels = 4,
+ .supplies = ad7380_supplies,
+ .num_supplies = ARRAY_SIZE(ad7380_supplies),
.available_scan_masks = ad7380_4_channel_scan_masks,
.timing_specs = &ad7380_4_timing,
};
@@ -426,6 +451,8 @@ static const struct ad7380_chip_info ad7383_4_chip_info = {
.channels = ad7383_4_channels,
.num_channels = ARRAY_SIZE(ad7383_4_channels),
.num_simult_channels = 4,
+ .supplies = ad7380_supplies,
+ .num_supplies = ARRAY_SIZE(ad7380_supplies),
.vcm_supplies = ad7380_4_channel_vcm_supplies,
.num_vcm_supplies = ARRAY_SIZE(ad7380_4_channel_vcm_supplies),
.available_scan_masks = ad7380_4_channel_scan_masks,
@@ -437,6 +464,8 @@ static const struct ad7380_chip_info ad7384_4_chip_info = {
.channels = ad7384_4_channels,
.num_channels = ARRAY_SIZE(ad7384_4_channels),
.num_simult_channels = 4,
+ .supplies = ad7380_supplies,
+ .num_supplies = ARRAY_SIZE(ad7380_supplies),
.vcm_supplies = ad7380_4_channel_vcm_supplies,
.num_vcm_supplies = ARRAY_SIZE(ad7380_4_channel_vcm_supplies),
.available_scan_masks = ad7380_4_channel_scan_masks,
@@ -448,6 +477,8 @@ static const struct ad7380_chip_info ad7386_4_chip_info = {
.channels = ad7386_4_channels,
.num_channels = ARRAY_SIZE(ad7386_4_channels),
.num_simult_channels = 4,
+ .supplies = ad7380_supplies,
+ .num_supplies = ARRAY_SIZE(ad7380_supplies),
.has_mux = true,
.available_scan_masks = ad7380_2x4_channel_scan_masks,
.timing_specs = &ad7380_4_timing,
@@ -458,6 +489,8 @@ static const struct ad7380_chip_info ad7387_4_chip_info = {
.channels = ad7387_4_channels,
.num_channels = ARRAY_SIZE(ad7387_4_channels),
.num_simult_channels = 4,
+ .supplies = ad7380_supplies,
+ .num_supplies = ARRAY_SIZE(ad7380_supplies),
.has_mux = true,
.available_scan_masks = ad7380_2x4_channel_scan_masks,
.timing_specs = &ad7380_4_timing,
@@ -468,6 +501,8 @@ static const struct ad7380_chip_info ad7388_4_chip_info = {
.channels = ad7388_4_channels,
.num_channels = ARRAY_SIZE(ad7388_4_channels),
.num_simult_channels = 4,
+ .supplies = ad7380_supplies,
+ .num_supplies = ARRAY_SIZE(ad7380_supplies),
.has_mux = true,
.available_scan_masks = ad7380_2x4_channel_scan_masks,
.timing_specs = &ad7380_4_timing,
@@ -1004,6 +1039,14 @@ static int ad7380_probe(struct spi_device *spi)
if (!st->chip_info)
return dev_err_probe(&spi->dev, -EINVAL, "missing match data\n");
+ devm_regulator_bulk_get_enable(&spi->dev, st->chip_info->num_supplies,
+ st->chip_info->supplies);
+
+ if (ret)
+ return dev_err_probe(&spi->dev, ret,
+ "Failed to enable power supplies\n");
+ msleep(T_POWERUP_MS);
+
/*
* If there is no REFIO supply, then it means that we are using
* the internal 2.5V reference, otherwise REFIO is reference voltage.
--
2.46.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/6] iio: adc: ad7380: fix supplies for ad7380-4
2024-10-07 15:45 [PATCH 0/6] iio: adc: ad7380: fix several supplies issues Julien Stephan
` (3 preceding siblings ...)
2024-10-07 15:45 ` [PATCH 4/6] iio: adc: ad7380: add missing supplies Julien Stephan
@ 2024-10-07 15:45 ` Julien Stephan
2024-10-07 15:45 ` [PATCH 6/6] docs: iio: ad7380: fix supply " Julien Stephan
5 siblings, 0 replies; 15+ messages in thread
From: Julien Stephan @ 2024-10-07 15:45 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sá,
David Lechner, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Jonathan Corbet
Cc: linux-iio, devicetree, linux-kernel, Conor Dooley,
Jonathan Cameron, linux-doc, Julien Stephan
ad7380-4 is the only device in the family that does not have an internal
reference. It uses "refin" as a required external reference.
All other devices in the family use "refio"" as an optional external
reference.
Fixes: 737413da8704 ("iio: adc: ad7380: add support for ad738x-4 4 channels variants")
Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
drivers/iio/adc/ad7380.c | 36 ++++++++++++++++++++++++++----------
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
index 9ef44b605144..e9784769baa9 100644
--- a/drivers/iio/adc/ad7380.c
+++ b/drivers/iio/adc/ad7380.c
@@ -89,6 +89,7 @@ struct ad7380_chip_info {
bool has_mux;
const char * const *supplies;
unsigned int num_supplies;
+ bool external_ref_only;
const char * const *vcm_supplies;
unsigned int num_vcm_supplies;
const unsigned long *available_scan_masks;
@@ -431,6 +432,7 @@ static const struct ad7380_chip_info ad7380_4_chip_info = {
.num_simult_channels = 4,
.supplies = ad7380_supplies,
.num_supplies = ARRAY_SIZE(ad7380_supplies),
+ .external_ref_only = true,
.available_scan_masks = ad7380_4_channel_scan_masks,
.timing_specs = &ad7380_4_timing,
};
@@ -1047,17 +1049,31 @@ static int ad7380_probe(struct spi_device *spi)
"Failed to enable power supplies\n");
msleep(T_POWERUP_MS);
- /*
- * If there is no REFIO supply, then it means that we are using
- * the internal 2.5V reference, otherwise REFIO is reference voltage.
- */
- ret = devm_regulator_get_enable_read_voltage(&spi->dev, "refio");
- if (ret < 0 && ret != -ENODEV)
- return dev_err_probe(&spi->dev, ret,
- "Failed to get refio regulator\n");
+ if (st->chip_info->external_ref_only) {
+ ret = devm_regulator_get_enable_read_voltage(&spi->dev,
+ "refin");
+ if (ret < 0)
+ return dev_err_probe(&spi->dev, ret,
+ "Failed to get refin regulator\n");
+
+ st->vref_mv = ret / 1000;
- external_ref_en = ret != -ENODEV;
- st->vref_mv = external_ref_en ? ret / 1000 : AD7380_INTERNAL_REF_MV;
+ /* these chips don't have a register bit for this */
+ external_ref_en = false;
+ } else {
+ /*
+ * If there is no REFIO supply, then it means that we are using
+ * the internal reference, otherwise REFIO is reference voltage.
+ */
+ ret = devm_regulator_get_enable_read_voltage(&spi->dev,
+ "refio");
+ if (ret < 0 && ret != -ENODEV)
+ return dev_err_probe(&spi->dev, ret,
+ "Failed to get refio regulator\n");
+
+ external_ref_en = ret != -ENODEV;
+ st->vref_mv = external_ref_en ? ret / 1000 : AD7380_INTERNAL_REF_MV;
+ }
if (st->chip_info->num_vcm_supplies > ARRAY_SIZE(st->vcm_mv))
return dev_err_probe(&spi->dev, -EINVAL,
--
2.46.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 6/6] docs: iio: ad7380: fix supply for ad7380-4
2024-10-07 15:45 [PATCH 0/6] iio: adc: ad7380: fix several supplies issues Julien Stephan
` (4 preceding siblings ...)
2024-10-07 15:45 ` [PATCH 5/6] iio: adc: ad7380: fix supplies for ad7380-4 Julien Stephan
@ 2024-10-07 15:45 ` Julien Stephan
5 siblings, 0 replies; 15+ messages in thread
From: Julien Stephan @ 2024-10-07 15:45 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Nuno Sá,
David Lechner, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Jonathan Corbet
Cc: linux-iio, devicetree, linux-kernel, Conor Dooley,
Jonathan Cameron, linux-doc, Julien Stephan
ad7380-4 is the only device from ad738x family that doesn't have an
internal reference. Moreover it's external reference is called REFIN in
the datasheet while all other use REFIO as an optional external
reference. Update documentation to highlight this.
Fixes: 3e82dfc82f38 ("docs: iio: new docs for ad7380 driver")
Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
Documentation/iio/ad7380.rst | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/Documentation/iio/ad7380.rst b/Documentation/iio/ad7380.rst
index 9c784c1e652e..6f70b49b9ef2 100644
--- a/Documentation/iio/ad7380.rst
+++ b/Documentation/iio/ad7380.rst
@@ -41,13 +41,22 @@ supports only 1 SDO line.
Reference voltage
-----------------
-2 possible reference voltage sources are supported:
+ad7380-4
+~~~~~~~~
+
+ad7380-4 supports only an external reference voltage (2.5V to 3.3V). It must be
+declared in the device tree as ``refin-supply``.
+
+All other devices from ad738x family
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+All other devices from ad738x support 2 possible reference voltage sources:
- Internal reference (2.5V)
- External reference (2.5V to 3.3V)
The source is determined by the device tree. If ``refio-supply`` is present,
-then the external reference is used, else the internal reference is used.
+then it is used as external reference, else the internal reference is used.
Oversampling and resolution boost
---------------------------------
--
2.46.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/6] dt-bindings: iio: adc: ad7380: remove voltage reference for supplies
2024-10-07 15:45 ` [PATCH 1/6] dt-bindings: iio: adc: ad7380: remove voltage reference for supplies Julien Stephan
@ 2024-10-08 7:51 ` Krzysztof Kozlowski
0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-10-08 7:51 UTC (permalink / raw)
To: Julien Stephan
Cc: Lars-Peter Clausen, Michael Hennerich, Nuno Sá,
David Lechner, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Jonathan Corbet,
linux-iio, devicetree, linux-kernel, Conor Dooley,
Jonathan Cameron, linux-doc
On Mon, Oct 07, 2024 at 05:45:44PM +0200, Julien Stephan wrote:
> Voltages may not bo valid for future compatible parts, so remove them and
> remove useless description
>
> Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> ---
> Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml | 14 +++-----------
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml
> index bd19abb867d9..72c51b3de97b 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml
> @@ -55,17 +55,9 @@ properties:
> spi-cpol: true
> spi-cpha: true
>
> - vcc-supply:
> - description: A 3V to 3.6V supply that powers the chip.
> -
> - vlogic-supply:
> - description:
> - A 1.65V to 3.6V supply for the logic pins.
> -
> - refio-supply:
> - description:
> - A 2.5V to 3.3V supply for the external reference voltage. When omitted,
> - the internal 2.5V reference is used.
This is valid description. I would say all of them are useful, not
useless.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] dt-bindings: iio: adc: ad7380: fix ad7380-4 reference supply
2024-10-07 15:45 ` [PATCH 2/6] dt-bindings: iio: adc: ad7380: fix ad7380-4 reference supply Julien Stephan
@ 2024-10-08 7:52 ` Krzysztof Kozlowski
2024-10-10 18:22 ` Jonathan Cameron
0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-10-08 7:52 UTC (permalink / raw)
To: Julien Stephan
Cc: Lars-Peter Clausen, Michael Hennerich, Nuno Sá,
David Lechner, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Jonathan Corbet,
linux-iio, devicetree, linux-kernel, Conor Dooley,
Jonathan Cameron, linux-doc
On Mon, Oct 07, 2024 at 05:45:45PM +0200, Julien Stephan wrote:
> ad7380-4 is the only device from ad738x family that doesn't have an
> internal reference. Moreover its external reference is called REFIN in
> the datasheet while all other use REFIO as an optional external
> reference. If refio-supply is omitted the internal reference is
> used.
>
> Fix the binding by adding refin-supply and makes it required for
> ad7380-4 only.
Maybe let's just use refio as refin? Reference-IO fits here well.
Otherwise you have two supplies for the same.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/6] iio: adc: ad7380: add missing supplies
2024-10-07 15:45 ` [PATCH 4/6] iio: adc: ad7380: add missing supplies Julien Stephan
@ 2024-10-08 8:31 ` kernel test robot
0 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2024-10-08 8:31 UTC (permalink / raw)
To: Julien Stephan, Lars-Peter Clausen, Michael Hennerich,
Nuno Sá, David Lechner, Jonathan Cameron, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Liam Girdwood, Mark Brown,
Jonathan Corbet
Cc: llvm, oe-kbuild-all, linux-iio, devicetree, linux-kernel,
linux-doc, Julien Stephan
Hi Julien,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 8bea3878a1511bceadc2fbf284b00bcc5a2ef28d]
url: https://github.com/intel-lab-lkp/linux/commits/Julien-Stephan/dt-bindings-iio-adc-ad7380-remove-voltage-reference-for-supplies/20241007-234838
base: 8bea3878a1511bceadc2fbf284b00bcc5a2ef28d
patch link: https://lore.kernel.org/r/20241007-ad7380-fix-supplies-v1-4-badcf813c9b9%40baylibre.com
patch subject: [PATCH 4/6] iio: adc: ad7380: add missing supplies
config: x86_64-buildonly-randconfig-003-20241008 (https://download.01.org/0day-ci/archive/20241008/202410081608.ZxEPPZ0u-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241008/202410081608.ZxEPPZ0u-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410081608.ZxEPPZ0u-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/iio/adc/ad7380.c:1045:6: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
1045 | if (ret)
| ^~~
drivers/iio/adc/ad7380.c:1030:9: note: initialize the variable 'ret' to silence this warning
1030 | int ret, i;
| ^
| = 0
1 warning generated.
vim +/ret +1045 drivers/iio/adc/ad7380.c
1024
1025 static int ad7380_probe(struct spi_device *spi)
1026 {
1027 struct iio_dev *indio_dev;
1028 struct ad7380_state *st;
1029 bool external_ref_en;
1030 int ret, i;
1031
1032 indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
1033 if (!indio_dev)
1034 return -ENOMEM;
1035
1036 st = iio_priv(indio_dev);
1037 st->spi = spi;
1038 st->chip_info = spi_get_device_match_data(spi);
1039 if (!st->chip_info)
1040 return dev_err_probe(&spi->dev, -EINVAL, "missing match data\n");
1041
1042 devm_regulator_bulk_get_enable(&spi->dev, st->chip_info->num_supplies,
1043 st->chip_info->supplies);
1044
> 1045 if (ret)
1046 return dev_err_probe(&spi->dev, ret,
1047 "Failed to enable power supplies\n");
1048 msleep(T_POWERUP_MS);
1049
1050 /*
1051 * If there is no REFIO supply, then it means that we are using
1052 * the internal 2.5V reference, otherwise REFIO is reference voltage.
1053 */
1054 ret = devm_regulator_get_enable_read_voltage(&spi->dev, "refio");
1055 if (ret < 0 && ret != -ENODEV)
1056 return dev_err_probe(&spi->dev, ret,
1057 "Failed to get refio regulator\n");
1058
1059 external_ref_en = ret != -ENODEV;
1060 st->vref_mv = external_ref_en ? ret / 1000 : AD7380_INTERNAL_REF_MV;
1061
1062 if (st->chip_info->num_vcm_supplies > ARRAY_SIZE(st->vcm_mv))
1063 return dev_err_probe(&spi->dev, -EINVAL,
1064 "invalid number of VCM supplies\n");
1065
1066 /*
1067 * pseudo-differential chips have common mode supplies for the negative
1068 * input pin.
1069 */
1070 for (i = 0; i < st->chip_info->num_vcm_supplies; i++) {
1071 const char *vcm = st->chip_info->vcm_supplies[i];
1072
1073 ret = devm_regulator_get_enable_read_voltage(&spi->dev, vcm);
1074 if (ret < 0)
1075 return dev_err_probe(&spi->dev, ret,
1076 "Failed to get %s regulator\n",
1077 vcm);
1078
1079 st->vcm_mv[i] = ret / 1000;
1080 }
1081
1082 st->regmap = devm_regmap_init(&spi->dev, NULL, st, &ad7380_regmap_config);
1083 if (IS_ERR(st->regmap))
1084 return dev_err_probe(&spi->dev, PTR_ERR(st->regmap),
1085 "failed to allocate register map\n");
1086
1087 /*
1088 * Setting up xfer structures for both normal and sequence mode. These
1089 * struct are used for both direct read and triggered buffer. Additional
1090 * fields will be set up in ad7380_update_xfers() based on the current
1091 * state of the driver at the time of the read.
1092 */
1093
1094 /*
1095 * In normal mode a read is composed of two steps:
1096 * - first, toggle CS (no data xfer) to trigger a conversion
1097 * - then, read data
1098 */
1099 st->normal_xfer[0].cs_change = 1;
1100 st->normal_xfer[0].cs_change_delay.value = st->chip_info->timing_specs->t_csh_ns;
1101 st->normal_xfer[0].cs_change_delay.unit = SPI_DELAY_UNIT_NSECS;
1102 st->normal_xfer[1].rx_buf = st->scan_data;
1103
1104 spi_message_init_with_transfers(&st->normal_msg, st->normal_xfer,
1105 ARRAY_SIZE(st->normal_xfer));
1106 /*
1107 * In sequencer mode a read is composed of four steps:
1108 * - CS toggle (no data xfer) to get the right point in the sequence
1109 * - CS toggle (no data xfer) to trigger a conversion of AinX0 and
1110 * acquisition of AinX1
1111 * - 2 data reads, to read AinX0 and AinX1
1112 */
1113 st->seq_xfer[0].cs_change = 1;
1114 st->seq_xfer[0].cs_change_delay.value = st->chip_info->timing_specs->t_csh_ns;
1115 st->seq_xfer[0].cs_change_delay.unit = SPI_DELAY_UNIT_NSECS;
1116 st->seq_xfer[1].cs_change = 1;
1117 st->seq_xfer[1].cs_change_delay.value = st->chip_info->timing_specs->t_csh_ns;
1118 st->seq_xfer[1].cs_change_delay.unit = SPI_DELAY_UNIT_NSECS;
1119
1120 st->seq_xfer[2].rx_buf = st->scan_data;
1121 st->seq_xfer[2].cs_change = 1;
1122 st->seq_xfer[2].cs_change_delay.value = st->chip_info->timing_specs->t_csh_ns;
1123 st->seq_xfer[2].cs_change_delay.unit = SPI_DELAY_UNIT_NSECS;
1124
1125 spi_message_init_with_transfers(&st->seq_msg, st->seq_xfer,
1126 ARRAY_SIZE(st->seq_xfer));
1127
1128 indio_dev->channels = st->chip_info->channels;
1129 indio_dev->num_channels = st->chip_info->num_channels;
1130 indio_dev->name = st->chip_info->name;
1131 indio_dev->info = &ad7380_info;
1132 indio_dev->modes = INDIO_DIRECT_MODE;
1133 indio_dev->available_scan_masks = st->chip_info->available_scan_masks;
1134
1135 ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
1136 iio_pollfunc_store_time,
1137 ad7380_trigger_handler,
1138 &ad7380_buffer_setup_ops);
1139 if (ret)
1140 return ret;
1141
1142 ret = ad7380_init(st, external_ref_en);
1143 if (ret)
1144 return ret;
1145
1146 return devm_iio_device_register(&spi->dev, indio_dev);
1147 }
1148
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] dt-bindings: iio: adc: ad7380: fix ad7380-4 reference supply
2024-10-08 7:52 ` Krzysztof Kozlowski
@ 2024-10-10 18:22 ` Jonathan Cameron
2024-10-14 9:00 ` Julien Stephan
0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2024-10-10 18:22 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Julien Stephan, Lars-Peter Clausen, Michael Hennerich,
Nuno Sá, David Lechner, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Jonathan Corbet,
linux-iio, devicetree, linux-kernel, Conor Dooley,
Jonathan Cameron, linux-doc
On Tue, 8 Oct 2024 09:52:50 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Mon, Oct 07, 2024 at 05:45:45PM +0200, Julien Stephan wrote:
> > ad7380-4 is the only device from ad738x family that doesn't have an
> > internal reference. Moreover its external reference is called REFIN in
> > the datasheet while all other use REFIO as an optional external
> > reference. If refio-supply is omitted the internal reference is
> > used.
> >
> > Fix the binding by adding refin-supply and makes it required for
> > ad7380-4 only.
>
> Maybe let's just use refio as refin? Reference-IO fits here well.
> Otherwise you have two supplies for the same.
Whilst it is ugly, the effort this series is going to in order
to paper over a naming mismatch makes me agree with Krzysztof.
I think adding a comment to the dt-binding would be sensible
though as people might fall into this hole.
Other than the missing ret =, rest of series looks fine to me
Jonathan
>
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] dt-bindings: iio: adc: ad7380: fix ad7380-4 reference supply
2024-10-10 18:22 ` Jonathan Cameron
@ 2024-10-14 9:00 ` Julien Stephan
2024-10-14 18:37 ` Jonathan Cameron
0 siblings, 1 reply; 15+ messages in thread
From: Julien Stephan @ 2024-10-14 9:00 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Krzysztof Kozlowski, Lars-Peter Clausen, Michael Hennerich,
Nuno Sá, David Lechner, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Jonathan Corbet,
linux-iio, devicetree, linux-kernel, Conor Dooley,
Jonathan Cameron, linux-doc
Le jeu. 10 oct. 2024 à 20:22, Jonathan Cameron <jic23@kernel.org> a écrit :
>
> On Tue, 8 Oct 2024 09:52:50 +0200
> Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> > On Mon, Oct 07, 2024 at 05:45:45PM +0200, Julien Stephan wrote:
> > > ad7380-4 is the only device from ad738x family that doesn't have an
> > > internal reference. Moreover its external reference is called REFIN in
> > > the datasheet while all other use REFIO as an optional external
> > > reference. If refio-supply is omitted the internal reference is
> > > used.
> > >
> > > Fix the binding by adding refin-supply and makes it required for
> > > ad7380-4 only.
> >
> > Maybe let's just use refio as refin? Reference-IO fits here well.
> > Otherwise you have two supplies for the same.
> Whilst it is ugly, the effort this series is going to in order
> to paper over a naming mismatch makes me agree with Krzysztof.
>
> I think adding a comment to the dt-binding would be sensible
> though as people might fall into this hole.
>
Hi Jonathan and Krzysztof,
I am currently adding support for another chip to this family
(ADAQ4380-4) and it also uses REFIN.. but in another way ad7380-4
does..
So:
- ad7380-4 does not have any internal reference and use a mandatory
refin supply as external reference
- adaq4380-4 does not have external reference but uses a 3V internal
reference derived from a 5V mandatory refin supply
- all others (AFAIK) use an optional refio external reference. If
omitted, use an internal 2.5V reference.
I am not sure using a single refio-supply for all will make things
clearer.. What do you think? Should I also send the adaq series now to
bring more context? (I wanted feedback on this series first).
Cheers
Julien
> Other than the missing ret =, rest of series looks fine to me
>
> Jonathan
>
> >
> > Best regards,
> > Krzysztof
> >
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] dt-bindings: iio: adc: ad7380: fix ad7380-4 reference supply
2024-10-14 9:00 ` Julien Stephan
@ 2024-10-14 18:37 ` Jonathan Cameron
2024-10-15 9:10 ` Julien Stephan
0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2024-10-14 18:37 UTC (permalink / raw)
To: Julien Stephan
Cc: Krzysztof Kozlowski, Lars-Peter Clausen, Michael Hennerich,
Nuno Sá, David Lechner, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Jonathan Corbet,
linux-iio, devicetree, linux-kernel, Conor Dooley,
Jonathan Cameron, linux-doc
On Mon, 14 Oct 2024 11:00:39 +0200
Julien Stephan <jstephan@baylibre.com> wrote:
> Le jeu. 10 oct. 2024 à 20:22, Jonathan Cameron <jic23@kernel.org> a écrit :
> >
> > On Tue, 8 Oct 2024 09:52:50 +0200
> > Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > > On Mon, Oct 07, 2024 at 05:45:45PM +0200, Julien Stephan wrote:
> > > > ad7380-4 is the only device from ad738x family that doesn't have an
> > > > internal reference. Moreover its external reference is called REFIN in
> > > > the datasheet while all other use REFIO as an optional external
> > > > reference. If refio-supply is omitted the internal reference is
> > > > used.
> > > >
> > > > Fix the binding by adding refin-supply and makes it required for
> > > > ad7380-4 only.
> > >
> > > Maybe let's just use refio as refin? Reference-IO fits here well.
> > > Otherwise you have two supplies for the same.
> > Whilst it is ugly, the effort this series is going to in order
> > to paper over a naming mismatch makes me agree with Krzysztof.
> >
> > I think adding a comment to the dt-binding would be sensible
> > though as people might fall into this hole.
> >
>
> Hi Jonathan and Krzysztof,
>
> I am currently adding support for another chip to this family
> (ADAQ4380-4) and it also uses REFIN.. but in another way ad7380-4
> does..
> So:
> - ad7380-4 does not have any internal reference and use a mandatory
> refin supply as external reference
> - adaq4380-4 does not have external reference but uses a 3V internal
> reference derived from a 5V mandatory refin supply
> - all others (AFAIK) use an optional refio external reference. If
> omitted, use an internal 2.5V reference.
>
> I am not sure using a single refio-supply for all will make things
> clearer.. What do you think? Should I also send the adaq series now to
> bring more context? (I wanted feedback on this series first).
>
Sounds like that context would be useful if you have it more or less
ready to send anyway. I don't have particularly strong views on this
either way. If we 'fix' the case you have here, old bindings should
continue to work for the part you are moving over (though no need
to have them in the dt-bindings file).
Jonathan
> Cheers
> Julien
>
> > Other than the missing ret =, rest of series looks fine to me
> >
> > Jonathan
> >
> > >
> > > Best regards,
> > > Krzysztof
> > >
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] dt-bindings: iio: adc: ad7380: fix ad7380-4 reference supply
2024-10-14 18:37 ` Jonathan Cameron
@ 2024-10-15 9:10 ` Julien Stephan
2024-10-18 18:09 ` Jonathan Cameron
0 siblings, 1 reply; 15+ messages in thread
From: Julien Stephan @ 2024-10-15 9:10 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Krzysztof Kozlowski, Lars-Peter Clausen, Michael Hennerich,
Nuno Sá, David Lechner, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Jonathan Corbet,
linux-iio, devicetree, linux-kernel, Conor Dooley,
Jonathan Cameron, linux-doc
Le lun. 14 oct. 2024 à 20:37, Jonathan Cameron <jic23@kernel.org> a écrit :
>
> On Mon, 14 Oct 2024 11:00:39 +0200
> Julien Stephan <jstephan@baylibre.com> wrote:
>
> > Le jeu. 10 oct. 2024 à 20:22, Jonathan Cameron <jic23@kernel.org> a écrit :
> > >
> > > On Tue, 8 Oct 2024 09:52:50 +0200
> > > Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >
> > > > On Mon, Oct 07, 2024 at 05:45:45PM +0200, Julien Stephan wrote:
> > > > > ad7380-4 is the only device from ad738x family that doesn't have an
> > > > > internal reference. Moreover its external reference is called REFIN in
> > > > > the datasheet while all other use REFIO as an optional external
> > > > > reference. If refio-supply is omitted the internal reference is
> > > > > used.
> > > > >
> > > > > Fix the binding by adding refin-supply and makes it required for
> > > > > ad7380-4 only.
> > > >
> > > > Maybe let's just use refio as refin? Reference-IO fits here well.
> > > > Otherwise you have two supplies for the same.
> > > Whilst it is ugly, the effort this series is going to in order
> > > to paper over a naming mismatch makes me agree with Krzysztof.
> > >
> > > I think adding a comment to the dt-binding would be sensible
> > > though as people might fall into this hole.
> > >
> >
> > Hi Jonathan and Krzysztof,
> >
> > I am currently adding support for another chip to this family
> > (ADAQ4380-4) and it also uses REFIN.. but in another way ad7380-4
> > does..
> > So:
> > - ad7380-4 does not have any internal reference and use a mandatory
> > refin supply as external reference
> > - adaq4380-4 does not have external reference but uses a 3V internal
> > reference derived from a 5V mandatory refin supply
> > - all others (AFAIK) use an optional refio external reference. If
> > omitted, use an internal 2.5V reference.
> >
> > I am not sure using a single refio-supply for all will make things
> > clearer.. What do you think? Should I also send the adaq series now to
> > bring more context? (I wanted feedback on this series first).
> >
>
> Sounds like that context would be useful if you have it more or less
> ready to send anyway. I don't have particularly strong views on this
> either way. If we 'fix' the case you have here, old bindings should
> continue to work for the part you are moving over (though no need
> to have them in the dt-bindings file).
>
Hi Jonathan,
Just sent the new series with an RFC tag.
Cheers
Julien
> Jonathan
>
> > Cheers
> > Julien
> >
> > > Other than the missing ret =, rest of series looks fine to me
> > >
> > > Jonathan
> > >
> > > >
> > > > Best regards,
> > > > Krzysztof
> > > >
> > >
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] dt-bindings: iio: adc: ad7380: fix ad7380-4 reference supply
2024-10-15 9:10 ` Julien Stephan
@ 2024-10-18 18:09 ` Jonathan Cameron
0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2024-10-18 18:09 UTC (permalink / raw)
To: Julien Stephan
Cc: Krzysztof Kozlowski, Lars-Peter Clausen, Michael Hennerich,
Nuno Sá, David Lechner, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Jonathan Corbet,
linux-iio, devicetree, linux-kernel, Conor Dooley,
Jonathan Cameron, linux-doc
On Tue, 15 Oct 2024 11:10:52 +0200
Julien Stephan <jstephan@baylibre.com> wrote:
> Le lun. 14 oct. 2024 à 20:37, Jonathan Cameron <jic23@kernel.org> a écrit :
> >
> > On Mon, 14 Oct 2024 11:00:39 +0200
> > Julien Stephan <jstephan@baylibre.com> wrote:
> >
> > > Le jeu. 10 oct. 2024 à 20:22, Jonathan Cameron <jic23@kernel.org> a écrit :
> > > >
> > > > On Tue, 8 Oct 2024 09:52:50 +0200
> > > > Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > >
> > > > > On Mon, Oct 07, 2024 at 05:45:45PM +0200, Julien Stephan wrote:
> > > > > > ad7380-4 is the only device from ad738x family that doesn't have an
> > > > > > internal reference. Moreover its external reference is called REFIN in
> > > > > > the datasheet while all other use REFIO as an optional external
> > > > > > reference. If refio-supply is omitted the internal reference is
> > > > > > used.
> > > > > >
> > > > > > Fix the binding by adding refin-supply and makes it required for
> > > > > > ad7380-4 only.
> > > > >
> > > > > Maybe let's just use refio as refin? Reference-IO fits here well.
> > > > > Otherwise you have two supplies for the same.
> > > > Whilst it is ugly, the effort this series is going to in order
> > > > to paper over a naming mismatch makes me agree with Krzysztof.
> > > >
> > > > I think adding a comment to the dt-binding would be sensible
> > > > though as people might fall into this hole.
> > > >
> > >
> > > Hi Jonathan and Krzysztof,
> > >
> > > I am currently adding support for another chip to this family
> > > (ADAQ4380-4) and it also uses REFIN.. but in another way ad7380-4
> > > does..
> > > So:
> > > - ad7380-4 does not have any internal reference and use a mandatory
> > > refin supply as external reference
> > > - adaq4380-4 does not have external reference but uses a 3V internal
> > > reference derived from a 5V mandatory refin supply
> > > - all others (AFAIK) use an optional refio external reference. If
> > > omitted, use an internal 2.5V reference.
> > >
> > > I am not sure using a single refio-supply for all will make things
> > > clearer.. What do you think? Should I also send the adaq series now to
> > > bring more context? (I wanted feedback on this series first).
> > >
> >
> > Sounds like that context would be useful if you have it more or less
> > ready to send anyway. I don't have particularly strong views on this
> > either way. If we 'fix' the case you have here, old bindings should
> > continue to work for the part you are moving over (though no need
> > to have them in the dt-bindings file).
> >
>
> Hi Jonathan,
>
> Just sent the new series with an RFC tag.
https://lore.kernel.org/all/20241015-ad7380-add-adaq4380-4-support-v1-1-d2e1a95fb248@baylibre.com/
Examples in there look strong enough reason that we are going to need
refin-supply in the binding anyway shortly. So might as well use it for this
part as well.
Just include a reference to that patch under the --- in v2.
+ see if you can keep the description from patch 1 and fix the assignment issue
the bot found.
Thanks,
Jonathan
>
>
> Cheers
> Julien
>
> > Jonathan
> >
> > > Cheers
> > > Julien
> > >
> > > > Other than the missing ret =, rest of series looks fine to me
> > > >
> > > > Jonathan
> > > >
> > > > >
> > > > > Best regards,
> > > > > Krzysztof
> > > > >
> > > >
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-10-18 18:09 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-07 15:45 [PATCH 0/6] iio: adc: ad7380: fix several supplies issues Julien Stephan
2024-10-07 15:45 ` [PATCH 1/6] dt-bindings: iio: adc: ad7380: remove voltage reference for supplies Julien Stephan
2024-10-08 7:51 ` Krzysztof Kozlowski
2024-10-07 15:45 ` [PATCH 2/6] dt-bindings: iio: adc: ad7380: fix ad7380-4 reference supply Julien Stephan
2024-10-08 7:52 ` Krzysztof Kozlowski
2024-10-10 18:22 ` Jonathan Cameron
2024-10-14 9:00 ` Julien Stephan
2024-10-14 18:37 ` Jonathan Cameron
2024-10-15 9:10 ` Julien Stephan
2024-10-18 18:09 ` Jonathan Cameron
2024-10-07 15:45 ` [PATCH 3/6] iio: adc: ad7380: use devm_regulator_get_enable_read_voltage() Julien Stephan
2024-10-07 15:45 ` [PATCH 4/6] iio: adc: ad7380: add missing supplies Julien Stephan
2024-10-08 8:31 ` kernel test robot
2024-10-07 15:45 ` [PATCH 5/6] iio: adc: ad7380: fix supplies for ad7380-4 Julien Stephan
2024-10-07 15:45 ` [PATCH 6/6] docs: iio: ad7380: fix supply " Julien Stephan
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).