linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/6] iio: adc: ad7192: Improvements
@ 2024-06-18 14:21 Alisa-Dariana Roman
  2024-06-18 14:21 ` [PATCH v5 1/6] iio: adc: ad7192: use devm_regulator_get_enable_read_voltage Alisa-Dariana Roman
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Alisa-Dariana Roman @ 2024-06-18 14:21 UTC (permalink / raw)
  To: Alisa-Dariana Roman, Jonathan Cameron, Michael Hennerich,
	linux-iio, devicetree, linux-kernel
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
	Mark Brown

Dear everyone,

Thank you very much for your feedback!

The series is based on testing branch, since the initial first commit
was applied there.

Here is the upgraded series. Please consider applying the commits in
order!

Kind regards,
Alisa-Dariana Roman.

v4 -> v5
  remove already applied commit
  modify commit message to mention the register configuration
  remove useless | from bindings
  add removed example, but still remove it in a later patch to showcase
new functionality
  remove clock-output-names since ad719x have only one clock
  new patch to replace old maintainer (i mistakenly sent it only to
linux-kernel mail at first)

v4: https://lore.kernel.org/all/20240613114001.270233-1-alisa.roman@analog.com/



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

* [PATCH v5 1/6] iio: adc: ad7192: use devm_regulator_get_enable_read_voltage
  2024-06-18 14:21 [PATCH v5 0/6] iio: adc: ad7192: Improvements Alisa-Dariana Roman
@ 2024-06-18 14:21 ` Alisa-Dariana Roman
  2024-06-23 15:59   ` Jonathan Cameron
  2024-06-18 14:21 ` [PATCH v5 2/6] dt-bindings: iio: adc: ad7192: Update clock config Alisa-Dariana Roman
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Alisa-Dariana Roman @ 2024-06-18 14:21 UTC (permalink / raw)
  To: Alisa-Dariana Roman, Jonathan Cameron, Michael Hennerich,
	linux-iio, devicetree, linux-kernel
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
	Mark Brown, David Lechner

From: David Lechner <dlechner@baylibre.com>

This makes use of the new devm_regulator_get_enable_read_voltage()
function to reduce boilerplate code.

Error messages have changed slightly since there are now fewer places
where we print an error. The rest of the logic of selecting which
supply to use as the reference voltage remains the same.

Also 1000 is replaced by MILLI in a few places for consistency.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ad7192.c | 103 ++++++++++++++-------------------------
 1 file changed, 36 insertions(+), 67 deletions(-)

diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index c7fb51a90e87..334ab90991d4 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -200,8 +200,6 @@ struct ad7192_chip_info {
 
 struct ad7192_state {
 	const struct ad7192_chip_info	*chip_info;
-	struct regulator		*avdd;
-	struct regulator		*vref;
 	struct clk			*mclk;
 	u16				int_vref_mv;
 	u32				aincom_mv;
@@ -1189,18 +1187,12 @@ static const struct ad7192_chip_info ad7192_chip_info_tbl[] = {
 	},
 };
 
-static void ad7192_reg_disable(void *reg)
-{
-	regulator_disable(reg);
-}
-
 static int ad7192_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
 	struct ad7192_state *st;
 	struct iio_dev *indio_dev;
-	struct regulator *aincom;
-	int ret;
+	int ret, avdd_mv;
 
 	if (!spi->irq)
 		return dev_err_probe(dev, -ENODEV, "Failed to get IRQ\n");
@@ -1218,72 +1210,49 @@ static int ad7192_probe(struct spi_device *spi)
 	 * Newer firmware should provide a zero volt fixed supply if wired to
 	 * ground.
 	 */
-	aincom = devm_regulator_get_optional(dev, "aincom");
-	if (IS_ERR(aincom)) {
-		if (PTR_ERR(aincom) != -ENODEV)
-			return dev_err_probe(dev, PTR_ERR(aincom),
-					     "Failed to get AINCOM supply\n");
-
-		st->aincom_mv = 0;
-	} else {
-		ret = regulator_enable(aincom);
-		if (ret)
-			return dev_err_probe(dev, ret,
-					     "Failed to enable specified AINCOM supply\n");
-
-		ret = devm_add_action_or_reset(dev, ad7192_reg_disable, aincom);
-		if (ret)
-			return ret;
-
-		ret = regulator_get_voltage(aincom);
-		if (ret < 0)
-			return dev_err_probe(dev, ret,
-					     "Device tree error, AINCOM voltage undefined\n");
-		st->aincom_mv = ret / MILLI;
+	ret = devm_regulator_get_enable_read_voltage(dev, "aincom");
+	if (ret < 0 && ret != -ENODEV)
+		return dev_err_probe(dev, ret, "Failed to get AINCOM voltage\n");
+
+	st->aincom_mv = ret == -ENODEV ? 0 : ret / MILLI;
+
+	/* AVDD can optionally be used as reference voltage */
+	ret = devm_regulator_get_enable_read_voltage(dev, "avdd");
+	if (ret == -ENODEV || ret == -EINVAL) {
+		int ret2;
+
+		/*
+		 * We get -EINVAL if avdd is a supply with unknown voltage. We
+		 * still need to enable it since it is also a power supply.
+		 */
+		ret2 = devm_regulator_get_enable(dev, "avdd");
+		if (ret2)
+			return dev_err_probe(dev, ret2,
+					     "Failed to enable AVDD supply\n");
+	} else if (ret < 0) {
+		return dev_err_probe(dev, ret, "Failed to get AVDD voltage\n");
 	}
 
-	st->avdd = devm_regulator_get(dev, "avdd");
-	if (IS_ERR(st->avdd))
-		return PTR_ERR(st->avdd);
-
-	ret = regulator_enable(st->avdd);
-	if (ret)
-		return dev_err_probe(dev, ret,
-				     "Failed to enable specified AVdd supply\n");
-
-	ret = devm_add_action_or_reset(dev, ad7192_reg_disable, st->avdd);
-	if (ret)
-		return ret;
+	avdd_mv = ret == -ENODEV || ret == -EINVAL ? 0 : ret / MILLI;
 
 	ret = devm_regulator_get_enable(dev, "dvdd");
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to enable specified DVdd supply\n");
 
-	st->vref = devm_regulator_get_optional(dev, "vref");
-	if (IS_ERR(st->vref)) {
-		if (PTR_ERR(st->vref) != -ENODEV)
-			return PTR_ERR(st->vref);
-
-		ret = regulator_get_voltage(st->avdd);
-		if (ret < 0)
-			return dev_err_probe(dev, ret,
-					     "Device tree error, AVdd voltage undefined\n");
-	} else {
-		ret = regulator_enable(st->vref);
-		if (ret)
-			return dev_err_probe(dev, ret,
-					     "Failed to enable specified Vref supply\n");
-
-		ret = devm_add_action_or_reset(dev, ad7192_reg_disable, st->vref);
-		if (ret)
-			return ret;
-
-		ret = regulator_get_voltage(st->vref);
-		if (ret < 0)
-			return dev_err_probe(dev, ret,
-					     "Device tree error, Vref voltage undefined\n");
+	/*
+	 * This is either REFIN1 or REFIN2 depending on adi,refin2-pins-enable.
+	 * If this supply is not present, fall back to AVDD as reference.
+	 */
+	ret = devm_regulator_get_enable_read_voltage(dev, "vref");
+	if (ret == -ENODEV) {
+		if (avdd_mv == 0)
+			return dev_err_probe(dev, -ENODEV,
+					     "No reference voltage available\n");
+	} else if (ret < 0) {
+		return ret;
 	}
-	st->int_vref_mv = ret / 1000;
+
+	st->int_vref_mv = ret == -ENODEV ? avdd_mv : ret / MILLI;
 
 	st->chip_info = spi_get_device_match_data(spi);
 	indio_dev->name = st->chip_info->name;
-- 
2.34.1


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

* [PATCH v5 2/6] dt-bindings: iio: adc: ad7192: Update clock config
  2024-06-18 14:21 [PATCH v5 0/6] iio: adc: ad7192: Improvements Alisa-Dariana Roman
  2024-06-18 14:21 ` [PATCH v5 1/6] iio: adc: ad7192: use devm_regulator_get_enable_read_voltage Alisa-Dariana Roman
@ 2024-06-18 14:21 ` Alisa-Dariana Roman
  2024-06-18 15:07   ` Conor Dooley
  2024-06-18 14:21 ` [PATCH v5 3/6] " Alisa-Dariana Roman
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Alisa-Dariana Roman @ 2024-06-18 14:21 UTC (permalink / raw)
  To: Alisa-Dariana Roman, Jonathan Cameron, Michael Hennerich,
	linux-iio, devicetree, linux-kernel
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
	Mark Brown

There are actually 4 configuration modes of clock source for AD719X
devices. Either a crystal can be attached externally between MCLK1 and
MCLK2 pins, or an external CMOS-compatible clock can drive the MCLK2
pin. The other 2 modes make use of the 4.92MHz internal clock.

To configure external clock as either a crystal or a CMOS-compatible
clock, changing the register settings is necessary. Therefore, add clock
name xtal alongside mclk. By selecting one or the other, the register is
configured.

The presence of an external clock source is optional, not required. When
absent, internal clock is used. Modify required property accordingly.

Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
---
 .../devicetree/bindings/iio/adc/adi,ad7192.yaml      | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
index a03da9489ed9..67384bed4cd3 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
@@ -39,11 +39,15 @@ properties:
 
   clocks:
     maxItems: 1
-    description: phandle to the master clock (mclk)
+    description:
+      Optionally, either a crystal can be attached externally between MCLK1 and
+      MCLK2 pins, or an external CMOS-compatible clock can drive the MCLK2
+      pin. If absent, internal 4.92MHz clock is used.
 
   clock-names:
-    items:
-      - const: mclk
+    enum:
+      - xtal
+      - mclk
 
   interrupts:
     maxItems: 1
@@ -135,8 +139,6 @@ patternProperties:
 required:
   - compatible
   - reg
-  - clocks
-  - clock-names
   - interrupts
   - dvdd-supply
   - avdd-supply
-- 
2.34.1


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

* [PATCH v5 3/6] iio: adc: ad7192: Update clock config
  2024-06-18 14:21 [PATCH v5 0/6] iio: adc: ad7192: Improvements Alisa-Dariana Roman
  2024-06-18 14:21 ` [PATCH v5 1/6] iio: adc: ad7192: use devm_regulator_get_enable_read_voltage Alisa-Dariana Roman
  2024-06-18 14:21 ` [PATCH v5 2/6] dt-bindings: iio: adc: ad7192: Update clock config Alisa-Dariana Roman
@ 2024-06-18 14:21 ` Alisa-Dariana Roman
  2024-06-18 14:21 ` [PATCH v5 4/6] dt-bindings: iio: adc: ad7192: Add clock provider Alisa-Dariana Roman
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Alisa-Dariana Roman @ 2024-06-18 14:21 UTC (permalink / raw)
  To: Alisa-Dariana Roman, Jonathan Cameron, Michael Hennerich,
	linux-iio, devicetree, linux-kernel
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
	Mark Brown

There are actually 4 configuration modes of clock source for AD719X
devices. Either a crystal can be attached externally between MCLK1 and
MCLK2 pins, or an external CMOS-compatible clock can drive the MCLK2
pin. The other 2 modes make use of the 4.92MHz internal clock.

Removed properties adi,int-clock-output-enable and adi,clock-xtal were
undocumented. Use cleaner alternative of configuring external clock by
using clock names mclk and xtal.

Removed functionality of AD7192_CLK_INT_CO restored in complementary
patch.

Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
---
 drivers/iio/adc/ad7192.c | 56 ++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index 334ab90991d4..940517df5429 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -396,25 +396,37 @@ static inline bool ad7192_valid_external_frequency(u32 freq)
 		freq <= AD7192_EXT_FREQ_MHZ_MAX);
 }
 
-static int ad7192_clock_select(struct ad7192_state *st)
+static const char *const ad7192_clock_names[] = {
+	"xtal",
+	"mclk"
+};
+
+static int ad7192_clock_setup(struct ad7192_state *st)
 {
 	struct device *dev = &st->sd.spi->dev;
-	unsigned int clock_sel;
-
-	clock_sel = AD7192_CLK_INT;
+	int ret;
 
-	/* use internal clock */
-	if (!st->mclk) {
-		if (device_property_read_bool(dev, "adi,int-clock-output-enable"))
-			clock_sel = AD7192_CLK_INT_CO;
+	ret = device_property_match_property_string(dev, "clock-names",
+						    ad7192_clock_names,
+						    ARRAY_SIZE(ad7192_clock_names));
+	if (ret < 0) {
+		st->clock_sel = AD7192_CLK_INT;
+		st->fclk = AD7192_INT_FREQ_MHZ;
 	} else {
-		if (device_property_read_bool(dev, "adi,clock-xtal"))
-			clock_sel = AD7192_CLK_EXT_MCLK1_2;
-		else
-			clock_sel = AD7192_CLK_EXT_MCLK2;
+		st->clock_sel = AD7192_CLK_EXT_MCLK1_2 + ret;
+
+		st->mclk = devm_clk_get_enabled(dev, ad7192_clock_names[ret]);
+		if (IS_ERR(st->mclk))
+			return dev_err_probe(dev, PTR_ERR(st->mclk),
+					     "Failed to get mclk\n");
+
+		st->fclk = clk_get_rate(st->mclk);
+		if (!ad7192_valid_external_frequency(st->fclk))
+			return dev_err_probe(dev, -EINVAL,
+					     "External clock frequency out of bounds\n");
 	}
 
-	return clock_sel;
+	return 0;
 }
 
 static int ad7192_setup(struct iio_dev *indio_dev, struct device *dev)
@@ -1275,21 +1287,9 @@ static int ad7192_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	st->fclk = AD7192_INT_FREQ_MHZ;
-
-	st->mclk = devm_clk_get_optional_enabled(dev, "mclk");
-	if (IS_ERR(st->mclk))
-		return PTR_ERR(st->mclk);
-
-	st->clock_sel = ad7192_clock_select(st);
-
-	if (st->clock_sel == AD7192_CLK_EXT_MCLK1_2 ||
-	    st->clock_sel == AD7192_CLK_EXT_MCLK2) {
-		st->fclk = clk_get_rate(st->mclk);
-		if (!ad7192_valid_external_frequency(st->fclk))
-			return dev_err_probe(dev, -EINVAL,
-					     "External clock frequency out of bounds\n");
-	}
+	ret = ad7192_clock_setup(st);
+	if (ret)
+		return ret;
 
 	ret = ad7192_setup(indio_dev, dev);
 	if (ret)
-- 
2.34.1


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

* [PATCH v5 4/6] dt-bindings: iio: adc: ad7192: Add clock provider
  2024-06-18 14:21 [PATCH v5 0/6] iio: adc: ad7192: Improvements Alisa-Dariana Roman
                   ` (2 preceding siblings ...)
  2024-06-18 14:21 ` [PATCH v5 3/6] " Alisa-Dariana Roman
@ 2024-06-18 14:21 ` Alisa-Dariana Roman
  2024-06-18 15:04   ` Conor Dooley
  2024-06-18 14:21 ` [PATCH v5 5/6] " Alisa-Dariana Roman
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Alisa-Dariana Roman @ 2024-06-18 14:21 UTC (permalink / raw)
  To: Alisa-Dariana Roman, Jonathan Cameron, Michael Hennerich,
	linux-iio, devicetree, linux-kernel
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
	Mark Brown

Internal clock of AD719X devices can be made available on MCLK2 pin. Add
clock provider to support this functionality.

Modify second example to showcase this mode.

Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
---
 .../devicetree/bindings/iio/adc/adi,ad7192.yaml          | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
index 67384bed4cd3..e31436484372 100644
--- a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
@@ -42,13 +42,17 @@ properties:
     description:
       Optionally, either a crystal can be attached externally between MCLK1 and
       MCLK2 pins, or an external CMOS-compatible clock can drive the MCLK2
-      pin. If absent, internal 4.92MHz clock is used.
+      pin. If absent, internal 4.92MHz clock is used, which can be made
+      available on MCLK2 pin.
 
   clock-names:
     enum:
       - xtal
       - mclk
 
+  "#clock-cells":
+    const: 0
+
   interrupts:
     maxItems: 1
 
@@ -204,8 +208,7 @@ examples:
             spi-max-frequency = <1000000>;
             spi-cpol;
             spi-cpha;
-            clocks = <&ad7192_mclk>;
-            clock-names = "mclk";
+            #clock-cells = <0>;
             interrupts = <25 0x2>;
             interrupt-parent = <&gpio>;
             aincom-supply = <&aincom>;
-- 
2.34.1


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

* [PATCH v5 5/6] iio: adc: ad7192: Add clock provider
  2024-06-18 14:21 [PATCH v5 0/6] iio: adc: ad7192: Improvements Alisa-Dariana Roman
                   ` (3 preceding siblings ...)
  2024-06-18 14:21 ` [PATCH v5 4/6] dt-bindings: iio: adc: ad7192: Add clock provider Alisa-Dariana Roman
@ 2024-06-18 14:21 ` Alisa-Dariana Roman
  2024-06-18 14:21 ` [PATCH v5 6/6] MAINTAINERS: Update AD7192 driver maintainer Alisa-Dariana Roman
  2024-06-23 16:00 ` [PATCH v5 0/6] iio: adc: ad7192: Improvements Jonathan Cameron
  6 siblings, 0 replies; 11+ messages in thread
From: Alisa-Dariana Roman @ 2024-06-18 14:21 UTC (permalink / raw)
  To: Alisa-Dariana Roman, Jonathan Cameron, Michael Hennerich,
	linux-iio, devicetree, linux-kernel
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
	Mark Brown

Internal clock of AD719X devices can be made available on MCLK2 pin. Add
clock provider to support this functionality.

Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
---
 drivers/iio/adc/ad7192.c | 89 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index 940517df5429..90763c14679d 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -8,6 +8,7 @@
 #include <linux/interrupt.h>
 #include <linux/bitfield.h>
 #include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
 #include <linux/slab.h>
@@ -201,6 +202,7 @@ struct ad7192_chip_info {
 struct ad7192_state {
 	const struct ad7192_chip_info	*chip_info;
 	struct clk			*mclk;
+	struct clk_hw			int_clk_hw;
 	u16				int_vref_mv;
 	u32				aincom_mv;
 	u32				fclk;
@@ -401,6 +403,88 @@ static const char *const ad7192_clock_names[] = {
 	"mclk"
 };
 
+static struct ad7192_state *clk_hw_to_ad7192(struct clk_hw *hw)
+{
+	return container_of(hw, struct ad7192_state, int_clk_hw);
+}
+
+static unsigned long ad7192_clk_recalc_rate(struct clk_hw *hw,
+					    unsigned long parent_rate)
+{
+	return AD7192_INT_FREQ_MHZ;
+}
+
+static int ad7192_clk_output_is_enabled(struct clk_hw *hw)
+{
+	struct ad7192_state *st = clk_hw_to_ad7192(hw);
+
+	return st->clock_sel == AD7192_CLK_INT_CO;
+}
+
+static int ad7192_clk_prepare(struct clk_hw *hw)
+{
+	struct ad7192_state *st = clk_hw_to_ad7192(hw);
+	int ret;
+
+	st->mode &= ~AD7192_MODE_CLKSRC_MASK;
+	st->mode |= AD7192_CLK_INT_CO;
+
+	ret = ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
+	if (ret)
+		return ret;
+
+	st->clock_sel = AD7192_CLK_INT_CO;
+
+	return 0;
+}
+
+static void ad7192_clk_unprepare(struct clk_hw *hw)
+{
+	struct ad7192_state *st = clk_hw_to_ad7192(hw);
+	int ret;
+
+	st->mode &= ~AD7192_MODE_CLKSRC_MASK;
+	st->mode |= AD7192_CLK_INT;
+
+	ret = ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
+	if (ret)
+		return;
+
+	st->clock_sel = AD7192_CLK_INT;
+}
+
+static const struct clk_ops ad7192_int_clk_ops = {
+	.recalc_rate = ad7192_clk_recalc_rate,
+	.is_enabled = ad7192_clk_output_is_enabled,
+	.prepare = ad7192_clk_prepare,
+	.unprepare = ad7192_clk_unprepare,
+};
+
+static int ad7192_register_clk_provider(struct ad7192_state *st)
+{
+	struct device *dev = &st->sd.spi->dev;
+	struct clk_init_data init = {};
+	int ret;
+
+	if (!IS_ENABLED(CONFIG_COMMON_CLK))
+		return 0;
+
+	init.name = devm_kasprintf(dev, GFP_KERNEL, "%s-clk",
+				   fwnode_get_name(dev_fwnode(dev)));
+	if (!init.name)
+		return -ENOMEM;
+
+	init.ops = &ad7192_int_clk_ops;
+
+	st->int_clk_hw.init = &init;
+	ret = devm_clk_hw_register(dev, &st->int_clk_hw);
+	if (ret)
+		return ret;
+
+	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
+					   &st->int_clk_hw);
+}
+
 static int ad7192_clock_setup(struct ad7192_state *st)
 {
 	struct device *dev = &st->sd.spi->dev;
@@ -412,6 +496,11 @@ static int ad7192_clock_setup(struct ad7192_state *st)
 	if (ret < 0) {
 		st->clock_sel = AD7192_CLK_INT;
 		st->fclk = AD7192_INT_FREQ_MHZ;
+
+		ret = ad7192_register_clk_provider(st);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to register clock provider\n");
 	} else {
 		st->clock_sel = AD7192_CLK_EXT_MCLK1_2 + ret;
 
-- 
2.34.1


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

* [PATCH v5 6/6] MAINTAINERS: Update AD7192 driver maintainer
  2024-06-18 14:21 [PATCH v5 0/6] iio: adc: ad7192: Improvements Alisa-Dariana Roman
                   ` (4 preceding siblings ...)
  2024-06-18 14:21 ` [PATCH v5 5/6] " Alisa-Dariana Roman
@ 2024-06-18 14:21 ` Alisa-Dariana Roman
  2024-06-23 16:00 ` [PATCH v5 0/6] iio: adc: ad7192: Improvements Jonathan Cameron
  6 siblings, 0 replies; 11+ messages in thread
From: Alisa-Dariana Roman @ 2024-06-18 14:21 UTC (permalink / raw)
  To: Alisa-Dariana Roman, Jonathan Cameron, Michael Hennerich,
	linux-iio, devicetree, linux-kernel
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
	Mark Brown

Alexandru Tachici has not been active. Also the email address included
is not reachable anymore. I was assigned to work on the driver instead.

Remove Alexandru Tachici and add myself as maintainer of AD7192 driver.

Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 24d372f7653e..d4a958b4341e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1217,7 +1217,7 @@ F:	Documentation/devicetree/bindings/iio/adc/adi,ad7091r*
 F:	drivers/iio/adc/ad7091r*
 
 ANALOG DEVICES INC AD7192 DRIVER
-M:	Alexandru Tachici <alexandru.tachici@analog.com>
+M:	Alisa-Dariana Roman <alisa.roman@analog.com>
 L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
-- 
2.34.1


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

* Re: [PATCH v5 4/6] dt-bindings: iio: adc: ad7192: Add clock provider
  2024-06-18 14:21 ` [PATCH v5 4/6] dt-bindings: iio: adc: ad7192: Add clock provider Alisa-Dariana Roman
@ 2024-06-18 15:04   ` Conor Dooley
  0 siblings, 0 replies; 11+ messages in thread
From: Conor Dooley @ 2024-06-18 15:04 UTC (permalink / raw)
  To: Alisa-Dariana Roman
  Cc: Alisa-Dariana Roman, Jonathan Cameron, Michael Hennerich,
	linux-iio, devicetree, linux-kernel, Lars-Peter Clausen,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Liam Girdwood, Mark Brown

[-- Attachment #1: Type: text/plain, Size: 1904 bytes --]

On Tue, Jun 18, 2024 at 05:21:36PM +0300, Alisa-Dariana Roman wrote:
> Internal clock of AD719X devices can be made available on MCLK2 pin. Add
> clock provider to support this functionality.
> 
> Modify second example to showcase this mode.
> 
> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
> ---
>  .../devicetree/bindings/iio/adc/adi,ad7192.yaml          | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> index 67384bed4cd3..e31436484372 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> @@ -42,13 +42,17 @@ properties:
>      description:
>        Optionally, either a crystal can be attached externally between MCLK1 and
>        MCLK2 pins, or an external CMOS-compatible clock can drive the MCLK2
> -      pin. If absent, internal 4.92MHz clock is used.
> +      pin. If absent, internal 4.92MHz clock is used, which can be made
> +      available on MCLK2 pin.
>  
>    clock-names:
>      enum:
>        - xtal
>        - mclk
>  
> +  "#clock-cells":
> +    const: 0
> +
>    interrupts:
>      maxItems: 1
>  
> @@ -204,8 +208,7 @@ examples:
>              spi-max-frequency = <1000000>;
>              spi-cpol;
>              spi-cpha;
> -            clocks = <&ad7192_mclk>;
> -            clock-names = "mclk";
> +            #clock-cells = <0>;

I don't see why the example needs to change here.
If these are mutually exclusive, I think the binding should enforce that
if clock-cells is present that clock-names cannot contain "mclk".

>              interrupts = <25 0x2>;
>              interrupt-parent = <&gpio>;
>              aincom-supply = <&aincom>;
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH v5 2/6] dt-bindings: iio: adc: ad7192: Update clock config
  2024-06-18 14:21 ` [PATCH v5 2/6] dt-bindings: iio: adc: ad7192: Update clock config Alisa-Dariana Roman
@ 2024-06-18 15:07   ` Conor Dooley
  0 siblings, 0 replies; 11+ messages in thread
From: Conor Dooley @ 2024-06-18 15:07 UTC (permalink / raw)
  To: Alisa-Dariana Roman
  Cc: Alisa-Dariana Roman, Jonathan Cameron, Michael Hennerich,
	linux-iio, devicetree, linux-kernel, Lars-Peter Clausen,
	Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Liam Girdwood, Mark Brown

[-- Attachment #1: Type: text/plain, Size: 807 bytes --]

On Tue, Jun 18, 2024 at 05:21:34PM +0300, Alisa-Dariana Roman wrote:
> There are actually 4 configuration modes of clock source for AD719X
> devices. Either a crystal can be attached externally between MCLK1 and
> MCLK2 pins, or an external CMOS-compatible clock can drive the MCLK2
> pin. The other 2 modes make use of the 4.92MHz internal clock.
> 
> To configure external clock as either a crystal or a CMOS-compatible
> clock, changing the register settings is necessary. Therefore, add clock
> name xtal alongside mclk. By selecting one or the other, the register is
> configured.
> 
> The presence of an external clock source is optional, not required. When
> absent, internal clock is used. Modify required property accordingly.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH v5 1/6] iio: adc: ad7192: use devm_regulator_get_enable_read_voltage
  2024-06-18 14:21 ` [PATCH v5 1/6] iio: adc: ad7192: use devm_regulator_get_enable_read_voltage Alisa-Dariana Roman
@ 2024-06-23 15:59   ` Jonathan Cameron
  0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2024-06-23 15:59 UTC (permalink / raw)
  To: Alisa-Dariana Roman
  Cc: Alisa-Dariana Roman, Jonathan Cameron, Michael Hennerich,
	linux-iio, devicetree, linux-kernel, Lars-Peter Clausen,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
	Mark Brown, David Lechner

On Tue, 18 Jun 2024 17:21:33 +0300
Alisa-Dariana Roman <alisadariana@gmail.com> wrote:

> From: David Lechner <dlechner@baylibre.com>
> 
> This makes use of the new devm_regulator_get_enable_read_voltage()
> function to reduce boilerplate code.
> 
> Error messages have changed slightly since there are now fewer places
> where we print an error. The rest of the logic of selecting which
> supply to use as the reference voltage remains the same.
> 
> Also 1000 is replaced by MILLI in a few places for consistency.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
Hi Alisa-Dariana,

When you post a patch from someone else as part of your series, you have
to follow the Developer Certificate of Origin rules and that means
you need to add your own SoB after David's. Without that I can't pick it
up :(

If nothing else comes up, just reply to this saying you forgot it and
include it in that email and it should be sufficient.

Jonathan

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

* Re: [PATCH v5 0/6] iio: adc: ad7192: Improvements
  2024-06-18 14:21 [PATCH v5 0/6] iio: adc: ad7192: Improvements Alisa-Dariana Roman
                   ` (5 preceding siblings ...)
  2024-06-18 14:21 ` [PATCH v5 6/6] MAINTAINERS: Update AD7192 driver maintainer Alisa-Dariana Roman
@ 2024-06-23 16:00 ` Jonathan Cameron
  6 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2024-06-23 16:00 UTC (permalink / raw)
  To: Alisa-Dariana Roman
  Cc: Alisa-Dariana Roman, Jonathan Cameron, Michael Hennerich,
	linux-iio, devicetree, linux-kernel, Lars-Peter Clausen,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Liam Girdwood,
	Mark Brown

On Tue, 18 Jun 2024 17:21:32 +0300
Alisa-Dariana Roman <alisadariana@gmail.com> wrote:

> Dear everyone,
> 
> Thank you very much for your feedback!
> 
> The series is based on testing branch, since the initial first commit
> was applied there.
> 
> Here is the upgraded series. Please consider applying the commits in
> order!

Other than the tag issue + answering Conor's question this looks
good to me

Jonathan

> 
> Kind regards,
> Alisa-Dariana Roman.
> 
> v4 -> v5
>   remove already applied commit
>   modify commit message to mention the register configuration
>   remove useless | from bindings
>   add removed example, but still remove it in a later patch to showcase
> new functionality
>   remove clock-output-names since ad719x have only one clock
>   new patch to replace old maintainer (i mistakenly sent it only to
> linux-kernel mail at first)
> 
> v4: https://lore.kernel.org/all/20240613114001.270233-1-alisa.roman@analog.com/
> 
> 


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

end of thread, other threads:[~2024-06-23 16:00 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-18 14:21 [PATCH v5 0/6] iio: adc: ad7192: Improvements Alisa-Dariana Roman
2024-06-18 14:21 ` [PATCH v5 1/6] iio: adc: ad7192: use devm_regulator_get_enable_read_voltage Alisa-Dariana Roman
2024-06-23 15:59   ` Jonathan Cameron
2024-06-18 14:21 ` [PATCH v5 2/6] dt-bindings: iio: adc: ad7192: Update clock config Alisa-Dariana Roman
2024-06-18 15:07   ` Conor Dooley
2024-06-18 14:21 ` [PATCH v5 3/6] " Alisa-Dariana Roman
2024-06-18 14:21 ` [PATCH v5 4/6] dt-bindings: iio: adc: ad7192: Add clock provider Alisa-Dariana Roman
2024-06-18 15:04   ` Conor Dooley
2024-06-18 14:21 ` [PATCH v5 5/6] " Alisa-Dariana Roman
2024-06-18 14:21 ` [PATCH v5 6/6] MAINTAINERS: Update AD7192 driver maintainer Alisa-Dariana Roman
2024-06-23 16:00 ` [PATCH v5 0/6] iio: adc: ad7192: Improvements Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).