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

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

David Lechner posted 4 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH 4/4] iio: adc: ad7124: add clock output support
Posted by David Lechner 2 months, 1 week ago 
Add support for the AD7124's internal clock output. If the #clock-cells
property is present, turn on the internal clock output during probe.

If both the clocks and #clock-names properties are present (not allowed
by devicetree bindings), assume that an external clock is being used so
that we don't accidentally have two outputs fighting each other.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---

We could make this fancier and only turn on the output on demand of a
clock consumer, but then we have to deal with locking of the SPI bus
to be able to write to the register. So I opted for the simpler
solution of always turning it on during probe. This would only be used
for synchronizing with other similar ADCs, so implementing the functions
for a more general-purpose clock seems a bit overkill.
---
 drivers/iio/adc/ad7124.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
index b0b03f838eed730347a3afcd759be7c1a8ab201e..b18229ff037596c6e98e12dc22b1552bf13fdc4e 100644
--- a/drivers/iio/adc/ad7124.c
+++ b/drivers/iio/adc/ad7124.c
@@ -7,6 +7,7 @@
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -125,10 +126,12 @@ static const unsigned int ad7124_reg_size[] = {
 	3, 3, 3, 3, 3
 };
 
+#define AD7124_INT_CLK_HZ 614400
+
 static const int ad7124_master_clk_freq_hz[3] = {
-	[AD7124_LOW_POWER] = 76800,
-	[AD7124_MID_POWER] = 153600,
-	[AD7124_FULL_POWER] = 614400,
+	[AD7124_LOW_POWER] = AD7124_INT_CLK_HZ / 8,
+	[AD7124_MID_POWER] = AD7124_INT_CLK_HZ / 4,
+	[AD7124_FULL_POWER] = AD7124_INT_CLK_HZ,
 };
 
 static const char * const ad7124_ref_names[] = {
@@ -1163,6 +1166,32 @@ static int ad7124_setup(struct ad7124_state *st)
 		}
 
 		clk_sel = AD7124_ADC_CONTROL_CLK_SEL_INT;
+	} else if (!device_property_present(dev, "clocks") &&
+		   device_property_present(dev, "clock-names")) {
+		struct clk_hw *clk_hw;
+		char *name;
+
+		name = devm_kasprintf(dev, GFP_KERNEL, "%s-clk",
+				      fwnode_get_name(dev_fwnode(dev)));
+		if (!name)
+			return -ENOMEM;
+
+		clk_hw = devm_clk_hw_register_fixed_rate(dev, name, NULL, 0,
+							 AD7124_INT_CLK_HZ);
+		if (IS_ERR(clk_hw))
+			return dev_err_probe(dev, PTR_ERR(clk_hw), "Failed to register clock provider\n");
+
+		ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
+						  clk_hw);
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to add clock provider\n");
+
+		/*
+		 * Treat the clock as always on. This way we don't have to deal
+		 * with someone trying to enable/disable the clock while we are
+		 * reading samples.
+		 */
+		clk_sel = AD7124_ADC_CONTROL_CLK_SEL_INT_OUT;
 	} else {
 		struct clk *clk;
 

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

> Add support for the AD7124's internal clock output. If the #clock-cells
> property is present, turn on the internal clock output during probe.
> 
> If both the clocks and #clock-names properties are present (not allowed
> by devicetree bindings), assume that an external clock is being used so
> that we don't accidentally have two outputs fighting each other.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
> 
> We could make this fancier and only turn on the output on demand of a
> clock consumer, but then we have to deal with locking of the SPI bus
> to be able to write to the register. So I opted for the simpler
> solution of always turning it on during probe. This would only be used
> for synchronizing with other similar ADCs, so implementing the functions
> for a more general-purpose clock seems a bit overkill.
Seems reasonable.  One comment inline.
> ---
>  drivers/iio/adc/ad7124.c | 35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
> index b0b03f838eed730347a3afcd759be7c1a8ab201e..b18229ff037596c6e98e12dc22b1552bf13fdc4e 100644
> --- a/drivers/iio/adc/ad7124.c
> +++ b/drivers/iio/adc/ad7124.c
> @@ -7,6 +7,7 @@
>  #include <linux/bitfield.h>
>  #include <linux/bitops.h>
>  #include <linux/clk.h>
> +#include <linux/clk-provider.h>
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
> @@ -125,10 +126,12 @@ static const unsigned int ad7124_reg_size[] = {
>  	3, 3, 3, 3, 3
>  };
>  
> +#define AD7124_INT_CLK_HZ 614400
> +
>  static const int ad7124_master_clk_freq_hz[3] = {
> -	[AD7124_LOW_POWER] = 76800,
> -	[AD7124_MID_POWER] = 153600,
> -	[AD7124_FULL_POWER] = 614400,
> +	[AD7124_LOW_POWER] = AD7124_INT_CLK_HZ / 8,
> +	[AD7124_MID_POWER] = AD7124_INT_CLK_HZ / 4,
> +	[AD7124_FULL_POWER] = AD7124_INT_CLK_HZ,
>  };
>  
>  static const char * const ad7124_ref_names[] = {
> @@ -1163,6 +1166,32 @@ static int ad7124_setup(struct ad7124_state *st)
>  		}
>  
>  		clk_sel = AD7124_ADC_CONTROL_CLK_SEL_INT;
> +	} else if (!device_property_present(dev, "clocks") &&
> +		   device_property_present(dev, "clock-names")) {
> +		struct clk_hw *clk_hw;
> +		char *name;
> +
> +		name = devm_kasprintf(dev, GFP_KERNEL, "%s-clk",
> +				      fwnode_get_name(dev_fwnode(dev)));

I think for anything that isn't const the clock core will copy the name
during registration.  Ultimately __clk_register()
https://elixir.bootlin.com/linux/v6.15.8/source/drivers/clk/clk.c#L4342

As such tying this to devm lifespans is excessive.  Should be fine
using a __free(kfree) to clean it up at the end of this scope.

> +		if (!name)
> +			return -ENOMEM;
> +
> +		clk_hw = devm_clk_hw_register_fixed_rate(dev, name, NULL, 0,
> +							 AD7124_INT_CLK_HZ);
> +		if (IS_ERR(clk_hw))
> +			return dev_err_probe(dev, PTR_ERR(clk_hw), "Failed to register clock provider\n");
> +
> +		ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
> +						  clk_hw);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Failed to add clock provider\n");
> +
> +		/*
> +		 * Treat the clock as always on. This way we don't have to deal
> +		 * with someone trying to enable/disable the clock while we are
> +		 * reading samples.
> +		 */
> +		clk_sel = AD7124_ADC_CONTROL_CLK_SEL_INT_OUT;
>  	} else {
>  		struct clk *clk;
>  
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.