1. Patchew
  2. linux
  3. [PATCH 0/4] iio: adc: ad7124: proper clock support
  4. View patch

[PATCH 3/4] iio: adc: ad7124: add external clock support

David Lechner posted 4 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH 3/4] iio: adc: ad7124: add external clock support
Posted by David Lechner 2 months, 1 week ago 
Add support for an external clock source to the AD7124 ADC driver.

Previously, the driver only supported using the internal clock and had
bad devicetree bindings that used a fake clock to essentially select
the power mode. This is preserved for backwards compatibility.

If the clock is not named "mclk", then we know that the devicetree is
using the correct bindings and we can configure the chip to use an
external clock source rather than internal.

Also drop a redundant comment when configuring the register fields
instead of adding more.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ad7124.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
index f587574e8a24040540a470e13fed412ec9c81971..b0b03f838eed730347a3afcd759be7c1a8ab201e 100644
--- a/drivers/iio/adc/ad7124.c
+++ b/drivers/iio/adc/ad7124.c
@@ -18,6 +18,7 @@
 #include <linux/property.h>
 #include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
+#include <linux/units.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/adc/ad_sigma_delta.h>
@@ -44,6 +45,11 @@
 #define AD7124_STATUS_POR_FLAG			BIT(4)
 
 /* AD7124_ADC_CONTROL */
+#define AD7124_ADC_CONTROL_CLK_SEL		GENMASK(1, 0)
+#define AD7124_ADC_CONTROL_CLK_SEL_INT			0
+#define AD7124_ADC_CONTROL_CLK_SEL_INT_OUT		1
+#define AD7124_ADC_CONTROL_CLK_SEL_EXT			2
+#define AD7124_ADC_CONTROL_CLK_SEL_EXT_DIV4		3
 #define AD7124_ADC_CONTROL_MODE			GENMASK(5, 2)
 #define AD7124_ADC_CONTROL_MODE_CONTINUOUS		0
 #define AD7124_ADC_CONTROL_MODE_SINGLE			1
@@ -1112,7 +1118,7 @@ static int ad7124_parse_channel_config(struct iio_dev *indio_dev,
 static int ad7124_setup(struct ad7124_state *st)
 {
 	struct device *dev = &st->sd.spi->dev;
-	unsigned int power_mode;
+	unsigned int power_mode, clk_sel;
 	struct clk *mclk;
 	int i, ret;
 
@@ -1155,9 +1161,41 @@ static int ad7124_setup(struct ad7124_state *st)
 			if (ret)
 				return dev_err_probe(dev, ret, "Failed to set mclk rate\n");
 		}
+
+		clk_sel = AD7124_ADC_CONTROL_CLK_SEL_INT;
+	} else {
+		struct clk *clk;
+
+		clk = devm_clk_get_optional_enabled(dev, NULL);
+		if (IS_ERR(clk))
+			return dev_err_probe(dev, PTR_ERR(clk), "Failed to get external clock\n");
+
+		if (clk) {
+			unsigned long clk_hz;
+
+			clk_hz = clk_get_rate(clk);
+			if (!clk_hz)
+				return dev_err_probe(dev, -EINVAL, "Failed to get external clock rate\n");
+
+			/*
+			 * The external clock may be 4x the nominal clock rate,
+			 * in which case the ADC needs to be configured to
+			 * divide it by 4. Using MEGA is a bit arbitrary, but
+			 * the expected clock rates are either 614.4 kHz or
+			 * 2.4576 MHz, so this should work.
+			 */
+			if (clk_hz > MEGA)
+				clk_sel = AD7124_ADC_CONTROL_CLK_SEL_EXT_DIV4;
+			else
+				clk_sel = AD7124_ADC_CONTROL_CLK_SEL_EXT;
+		} else {
+			clk_sel = AD7124_ADC_CONTROL_CLK_SEL_INT;
+		}
 	}
 
-	/* Set the power mode */
+	st->adc_control &= ~AD7124_ADC_CONTROL_CLK_SEL;
+	st->adc_control |= FIELD_PREP(AD7124_ADC_CONTROL_CLK_SEL, clk_sel);
+
 	st->adc_control &= ~AD7124_ADC_CONTROL_POWER_MODE;
 	st->adc_control |= FIELD_PREP(AD7124_ADC_CONTROL_POWER_MODE, power_mode);
 

-- 
2.43.0
Re: [PATCH 3/4] iio: adc: ad7124: add external clock support
Posted by Jonathan Cameron 2 months, 1 week ago 
On Thu, 24 Jul 2025 18:25:24 -0500
David Lechner <dlechner@baylibre.com> wrote:

> Add support for an external clock source to the AD7124 ADC driver.
> 
> Previously, the driver only supported using the internal clock and had
> bad devicetree bindings that used a fake clock to essentially select
> the power mode. This is preserved for backwards compatibility.
> 
> If the clock is not named "mclk", then we know that the devicetree is
> using the correct bindings and we can configure the chip to use an
> external clock source rather than internal.
> 
> Also drop a redundant comment when configuring the register fields
> instead of adding more.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
Hi David,

A few trivial things inline.

> ---
>  drivers/iio/adc/ad7124.c | 42 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
> index f587574e8a24040540a470e13fed412ec9c81971..b0b03f838eed730347a3afcd759be7c1a8ab201e 100644
> --- a/drivers/iio/adc/ad7124.c
> +++ b/drivers/iio/adc/ad7124.c
> @@ -18,6 +18,7 @@
>  #include <linux/property.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/spi/spi.h>
> +#include <linux/units.h>
>  
>  #include <linux/iio/iio.h>
>  #include <linux/iio/adc/ad_sigma_delta.h>
> @@ -44,6 +45,11 @@
>  #define AD7124_STATUS_POR_FLAG			BIT(4)
>  
>  /* AD7124_ADC_CONTROL */
> +#define AD7124_ADC_CONTROL_CLK_SEL		GENMASK(1, 0)
> +#define AD7124_ADC_CONTROL_CLK_SEL_INT			0
> +#define AD7124_ADC_CONTROL_CLK_SEL_INT_OUT		1
> +#define AD7124_ADC_CONTROL_CLK_SEL_EXT			2
> +#define AD7124_ADC_CONTROL_CLK_SEL_EXT_DIV4		3
>  #define AD7124_ADC_CONTROL_MODE			GENMASK(5, 2)
>  #define AD7124_ADC_CONTROL_MODE_CONTINUOUS		0
>  #define AD7124_ADC_CONTROL_MODE_SINGLE			1
> @@ -1112,7 +1118,7 @@ static int ad7124_parse_channel_config(struct iio_dev *indio_dev,
>  static int ad7124_setup(struct ad7124_state *st)
>  {
>  	struct device *dev = &st->sd.spi->dev;
> -	unsigned int power_mode;
> +	unsigned int power_mode, clk_sel;
>  	struct clk *mclk;
>  	int i, ret;
>  
> @@ -1155,9 +1161,41 @@ static int ad7124_setup(struct ad7124_state *st)
>  			if (ret)
>  				return dev_err_probe(dev, ret, "Failed to set mclk rate\n");
>  		}
> +
> +		clk_sel = AD7124_ADC_CONTROL_CLK_SEL_INT;
> +	} else {
> +		struct clk *clk;
> +
> +		clk = devm_clk_get_optional_enabled(dev, NULL);
> +		if (IS_ERR(clk))
> +			return dev_err_probe(dev, PTR_ERR(clk), "Failed to get external clock\n");

Somme very long lines here where breaks won't hurt readability.

> +			return dev_err_probe(dev, PTR_ERR(clk),
					     "Failed to get external clock\n");

> +
> +		if (clk) {
> +			unsigned long clk_hz;
> +
> +			clk_hz = clk_get_rate(clk);
> +			if (!clk_hz)
> +				return dev_err_probe(dev, -EINVAL, "Failed to get external clock rate\n");

Add a break.

> +
> +			/*
> +			 * The external clock may be 4x the nominal clock rate,
> +			 * in which case the ADC needs to be configured to
> +			 * divide it by 4. Using MEGA is a bit arbitrary, but
> +			 * the expected clock rates are either 614.4 kHz or
> +			 * 2.4576 MHz, so this should work.
> +			 */
> +			if (clk_hz > MEGA)
> +				clk_sel = AD7124_ADC_CONTROL_CLK_SEL_EXT_DIV4;
> +			else
> +				clk_sel = AD7124_ADC_CONTROL_CLK_SEL_EXT;
> +		} else {
> +			clk_sel = AD7124_ADC_CONTROL_CLK_SEL_INT;
> +		}
>  	}
>  
> -	/* Set the power mode */
> +	st->adc_control &= ~AD7124_ADC_CONTROL_CLK_SEL;
> +	st->adc_control |= FIELD_PREP(AD7124_ADC_CONTROL_CLK_SEL, clk_sel);
> +
>  	st->adc_control &= ~AD7124_ADC_CONTROL_POWER_MODE;
>  	st->adc_control |= FIELD_PREP(AD7124_ADC_CONTROL_POWER_MODE, power_mode);
>  
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.