| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <691267ecaa66a0f6a07ec79000b80de502d9b367.camel@gmail.com>
Date: Mon, 17 Nov 2025 08:35:23 +0000
From: Nuno Sá <noname.nuno@...il.com>
To: Jonathan Cameron <jic23@...nel.org>, Antoniu Miclaus
<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
On Sat, 2025-11-15 at 17:24 +0000, Jonathan Cameron wrote:
> 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.
Maybe a sneaky way of making use of the IIO debug direct access :). But I already
asked myself the somehow related question if we should still allow IIO access even if
the clock provider is registered. I mean, for sure we would need a more "fine"
grained access but we could only forbid IIO/userspace control in case an actual
consumer of the clock asks for it.
Or maybe not worth the trouble :)
- Nuno Sá
>
> > + }
> > +
> > return devm_iio_device_register(&spi->dev, indio_dev);
> > }
> >
Powered by blists - more mailing lists