[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
<CY4PR03MB3399661C6863F9430EA86A9A9BC9A@CY4PR03MB3399.namprd03.prod.outlook.com>
Date: Mon, 17 Nov 2025 09:09:34 +0000
From: "Miclaus, Antoniu" <Antoniu.Miclaus@...log.com>
To: Jonathan Cameron <jic23@...nel.org>
CC: "robh@...nel.org" <robh@...nel.org>,
"conor+dt@...nel.org"
<conor+dt@...nel.org>,
"linux-iio@...r.kernel.org"
<linux-iio@...r.kernel.org>,
"linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>,
"devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>,
"linux-clk@...r.kernel.org"
<linux-clk@...r.kernel.org>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>, "Sa, Nuno" <Nuno.Sa@...log.com>
Subject: RE: [PATCH 2/2] iio: frequency: adf4377: add clk provider support
Hi Jonathan,
> -----Original Message-----
> From: Jonathan Cameron <jic23@...nel.org>
> Sent: Saturday, November 15, 2025 7:24 PM
> To: Miclaus, Antoniu <Antoniu.Miclaus@...log.com>
> Cc: robh@...nel.org; conor+dt@...nel.org; linux-iio@...r.kernel.org; linux-
> kernel@...r.kernel.org; devicetree@...r.kernel.org; linux-
> clk@...r.kernel.org; Michael Turquette <mturquette@...libre.com>; Stephen
> Boyd <sboyd@...nel.org>
> Subject: Re: [PATCH 2/2] iio: frequency: adf4377: add clk provider support
>
> [External]
>
> On Fri, 14 Nov 2025 12:09:08 +0000
> Antoniu Miclaus <antoniu.miclaus@...log.com> wrote:
>
> > Add clk provider feature for the adf4377.
> >
> > Even though the driver was sent as an IIO driver in most cases the
> > device is actually seen as a clock provider.
> >
> > This patch aims to cover actual usecases requested by users in order to
> > completely control the output frequencies from userspace.
> >
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@...log.com>
>
> Given this new code is basically a clock driver, I'd expect to see some
> relevant folk +CC.
>
> Added Michael, Stephen and linux-clk.
>
> One question from me right at the end around whether it makes sense
> to register an IIO device with no channels. I left the rest so it was
> easy for the people added to the thread to see all the code.
>
> Thanks,
>
> Jonathan
>
>
> > ---
> > drivers/iio/frequency/adf4377.c | 131
> +++++++++++++++++++++++++++++++-
> > 1 file changed, 129 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/frequency/adf4377.c
> b/drivers/iio/frequency/adf4377.c
> > index 08833b7035e4..08dc2110cf8c 100644
> > --- a/drivers/iio/frequency/adf4377.c
> > +++ b/drivers/iio/frequency/adf4377.c
> > @@ -8,6 +8,7 @@
> > #include <linux/bitfield.h>
> > #include <linux/bits.h>
> > #include <linux/clk.h>
> > +#include <linux/clk-provider.h>
> > #include <linux/clkdev.h>
> > #include <linux/delay.h>
> > #include <linux/device.h>
> > @@ -435,9 +436,14 @@ struct adf4377_state {
> > struct gpio_desc *gpio_ce;
> > struct gpio_desc *gpio_enclk1;
> > struct gpio_desc *gpio_enclk2;
> > + struct clk *clk;
> > + struct clk *clkout;
> > + struct clk_hw hw;
> > u8 buf[2] __aligned(IIO_DMA_MINALIGN);
> > };
> >
> > +#define to_adf4377_state(h) container_of(h, struct adf4377_state,
> hw)
> > +
> > static const char * const adf4377_muxout_modes[] = {
> > [ADF4377_MUXOUT_HIGH_Z] = "high_z",
> > [ADF4377_MUXOUT_LKDET] = "lock_detect",
> > @@ -929,6 +935,120 @@ static int adf4377_freq_change(struct
> notifier_block *nb, unsigned long action,
> > return NOTIFY_OK;
> > }
> >
> > +static void adf4377_clk_del_provider(void *data)
> > +{
> > + struct adf4377_state *st = data;
> > +
> > + of_clk_del_provider(st->spi->dev.of_node);
> > +}
> > +
> > +static unsigned long adf4377_clk_recalc_rate(struct clk_hw *hw,
> > + unsigned long parent_rate)
> > +{
> > + struct adf4377_state *st = to_adf4377_state(hw);
> > + u64 freq;
> > + int ret;
> > +
> > + ret = adf4377_get_freq(st, &freq);
> > + if (ret)
> > + return 0;
> > +
> > + return freq;
> > +}
> > +
> > +static int adf4377_clk_set_rate(struct clk_hw *hw,
> > + unsigned long rate,
> > + unsigned long parent_rate)
> > +{
> > + struct adf4377_state *st = to_adf4377_state(hw);
> > +
> > + return adf4377_set_freq(st, rate);
> > +}
> > +
> > +static int adf4377_clk_prepare(struct clk_hw *hw)
> > +{
> > + struct adf4377_state *st = to_adf4377_state(hw);
> > +
> > + return regmap_update_bits(st->regmap, 0x1a,
> ADF4377_001A_PD_CLKOUT1_MSK |
> > + ADF4377_001A_PD_CLKOUT2_MSK,
> > +
> FIELD_PREP(ADF4377_001A_PD_CLKOUT1_MSK, 0) |
> > +
> FIELD_PREP(ADF4377_001A_PD_CLKOUT2_MSK, 0));
> > +}
> > +
> > +static void adf4377_clk_unprepare(struct clk_hw *hw)
> > +{
> > + struct adf4377_state *st = to_adf4377_state(hw);
> > +
> > + regmap_update_bits(st->regmap, 0x1a,
> ADF4377_001A_PD_CLKOUT1_MSK |
> > + ADF4377_001A_PD_CLKOUT2_MSK,
> > + FIELD_PREP(ADF4377_001A_PD_CLKOUT1_MSK, 1)
> |
> > + FIELD_PREP(ADF4377_001A_PD_CLKOUT2_MSK,
> 1));
> > +}
> > +
> > +static int adf4377_clk_is_enabled(struct clk_hw *hw)
> > +{
> > + struct adf4377_state *st = to_adf4377_state(hw);
> > + unsigned int readval;
> > + int ret;
> > +
> > + ret = regmap_read(st->regmap, 0x1a, &readval);
> > + if (ret)
> > + return ret;
> > +
> > + return !(readval & (ADF4377_001A_PD_CLKOUT1_MSK |
> ADF4377_001A_PD_CLKOUT2_MSK));
> > +}
> > +
> > +static const struct clk_ops adf4377_clk_ops = {
> > + .recalc_rate = adf4377_clk_recalc_rate,
> > + .set_rate = adf4377_clk_set_rate,
> > + .prepare = adf4377_clk_prepare,
> > + .unprepare = adf4377_clk_unprepare,
> > + .is_enabled = adf4377_clk_is_enabled,
> > +};
> > +
> > +static int adf4377_clk_register(struct adf4377_state *st)
> > +{
> > + struct spi_device *spi = st->spi;
> > + struct clk_init_data init;
> > + struct clk *clk;
> > + const char *parent_name;
> > + int ret;
> > +
> > + if (!device_property_present(&spi->dev, "#clock-cells"))
> > + return 0;
> > +
> > + if (device_property_read_string(&spi->dev, "clock-output-names",
> &init.name)) {
> > + init.name = devm_kasprintf(&spi->dev, GFP_KERNEL, "%s-clk",
> > +
> fwnode_get_name(dev_fwnode(&spi->dev)));
> > + if (!init.name)
> > + return -ENOMEM;
> > + }
> > +
> > + parent_name = of_clk_get_parent_name(spi->dev.of_node, 0);
> > + if (!parent_name)
> > + return -EINVAL;
> > +
> > + init.ops = &adf4377_clk_ops;
> > + init.parent_names = &parent_name;
> > + init.num_parents = 1;
> > + init.flags = CLK_SET_RATE_PARENT;
> > +
> > + st->hw.init = &init;
> > + clk = devm_clk_register(&spi->dev, &st->hw);
> > + if (IS_ERR(clk))
> > + return PTR_ERR(clk);
> > +
> > + st->clk = clk;
> > +
> > + ret = of_clk_add_provider(spi->dev.of_node, of_clk_src_simple_get,
> clk);
> > + if (ret)
> > + return ret;
> > +
> > + st->clkout = clk;
> > +
> > + return devm_add_action_or_reset(&spi->dev,
> adf4377_clk_del_provider, st);
> > +}
> > +
> > static const struct adf4377_chip_info adf4377_chip_info = {
> > .name = "adf4377",
> > .has_gpio_enclk2 = true,
> > @@ -958,8 +1078,6 @@ static int adf4377_probe(struct spi_device *spi)
> >
> > indio_dev->info = &adf4377_info;
> > indio_dev->name = "adf4377";
> > - indio_dev->channels = adf4377_channels;
> > - indio_dev->num_channels = ARRAY_SIZE(adf4377_channels);
> >
> > st->regmap = regmap;
> > st->spi = spi;
> > @@ -979,6 +1097,15 @@ static int adf4377_probe(struct spi_device *spi)
> > if (ret)
> > return ret;
> >
> > + ret = adf4377_clk_register(st);
> > + if (ret)
> > + return ret;
> > +
> > + if (!st->clkout) {
> > + indio_dev->channels = adf4377_channels;
> > + indio_dev->num_channels = ARRAY_SIZE(adf4377_channels);
>
> Why register a channel free iio device? Probably better to just not register
> it at all in this path.
There was a similar discussion on adf4350 clock provider support and we ended up using this approach.
https://github.com/torvalds/linux/commit/a1a09713b40dfc1c0b7d1f9233a7698c93a9af05
I wanted to stick to something that was accepted at some point in time.
Regards,
Antoniu
> > + }
> > +
> > return devm_iio_device_register(&spi->dev, indio_dev);
> > }
> >
Powered by blists - more mailing lists