| Message ID | 20240612130232.7692-2-antoniu.miclaus@analog.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v4,1/2] dt-bindings: iio: adf4350: add clk provider prop | expand |
On Wed, 2024-06-12 at 16:02 +0300, Antoniu Miclaus wrote: > Add clk provider feature for the adf4350. > > 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@analog.com> > --- > changes in v4: > - rename macro to `to_adf4350_state` > - do not expose ADF4350_FREQ and ADF4350_FREQ_REFIN if driver is used as clk > provider. > - initialize flags with CLK_SET_RATE_PARENT > drivers/iio/frequency/adf4350.c | 134 +++++++++++++++++++++++++++++++- > 1 file changed, 133 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c > index 4abf80f75ef5..8309ddfca9af 100644 > --- a/drivers/iio/frequency/adf4350.c > +++ b/drivers/iio/frequency/adf4350.c > @@ -19,6 +19,7 @@ > #include <linux/gpio/consumer.h> > #include <asm/div64.h> > #include <linux/clk.h> > +#include <linux/clk-provider.h> > > #include <linux/iio/iio.h> > #include <linux/iio/sysfs.h> > @@ -36,6 +37,9 @@ struct adf4350_state { > struct gpio_desc *lock_detect_gpiod; > struct adf4350_platform_data *pdata; > struct clk *clk; > + struct clk *clkout; > + const char *clk_out_name; > + struct clk_hw hw; > unsigned long clkin; > unsigned long chspc; /* Channel Spacing */ > unsigned long fpfd; /* Phase Frequency Detector */ > @@ -61,6 +65,8 @@ struct adf4350_state { > __be32 val __aligned(IIO_DMA_MINALIGN); > }; > > +#define to_adf4350_state(_hw) container_of(_hw, struct adf4350_state, hw) > + > static struct adf4350_platform_data default_pdata = { > .channel_spacing = 10000, > .r2_user_settings = ADF4350_REG2_PD_POLARITY_POS | > @@ -370,6 +376,12 @@ static const struct iio_chan_spec_ext_info adf4350_ext_info[] > = { > { }, > }; > > +static const struct iio_chan_spec_ext_info adf4350_clk_ext_info[] = { > + _ADF4350_EXT_INFO("frequency_resolution", ADF4350_FREQ_RESOLUTION), > + _ADF4350_EXT_INFO("powerdown", ADF4350_PWRDOWN), > + { }, Do we really need powerdown? Dunno :). I would expect one of unprepare/disable to take care of that. Moreover, imagine userspace powers down the device while an in kernel consumer was using it? Not cool right ehhe? Even the frequency_resolution is arguable as that is also a DT property but to keep consistency why not? That one I can live with... Also note that you're still not including the clock maintainers in the loop. - Nuno Sá
On Wed, 12 Jun 2024 16:02:29 +0300 Antoniu Miclaus <antoniu.miclaus@analog.com> wrote: > Add clk provider feature for the adf4350. > > 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@analog.com> As Nuno reminded, this needs the clock maintainers in the loop Trivial comments inline. > +static int adf4350_clk_register(struct adf4350_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; > + > + init.name = devm_kasprintf(&spi->dev, GFP_KERNEL, "%s-clk", > + fwnode_get_name(dev_fwnode(&spi->dev))); Check for failure. > + device_property_read_string(&spi->dev, "clock-output-names", > + &init.name); Also check for failure. > + > + parent_name = of_clk_get_parent_name(spi->dev.of_node, 0); > + if (!parent_name) > + return -EINVAL; > + > + init.ops = &adf4350_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); > + > + 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, adf4350_clk_del_provider, st); > +} > + > static struct adf4350_platform_data *adf4350_parse_dt(struct device *dev) > { > struct adf4350_platform_data *pdata; > @@ -522,7 +646,6 @@ static int adf4350_probe(struct spi_device *spi) > > indio_dev->info = &adf4350_info; > indio_dev->modes = INDIO_DIRECT_MODE; > - indio_dev->channels = &adf4350_chan; > indio_dev->num_channels = 1; > > mutex_init(&st->lock); > @@ -551,6 +674,15 @@ static int adf4350_probe(struct spi_device *spi) > return ret; > } > > + ret = adf4350_clk_register(st); > + if (ret) > + return ret; > + > + if (st->clkout) > + indio_dev->channels = &adf4350_clk_chan; > + else > + indio_dev->channels = &adf4350_chan; > + > ret = devm_add_action_or_reset(&spi->dev, adf4350_power_down, indio_dev); > if (ret) > return dev_err_probe(&spi->dev, ret,
diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c index 4abf80f75ef5..8309ddfca9af 100644 --- a/drivers/iio/frequency/adf4350.c +++ b/drivers/iio/frequency/adf4350.c @@ -19,6 +19,7 @@ #include <linux/gpio/consumer.h> #include <asm/div64.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/iio/iio.h> #include <linux/iio/sysfs.h> @@ -36,6 +37,9 @@ struct adf4350_state { struct gpio_desc *lock_detect_gpiod; struct adf4350_platform_data *pdata; struct clk *clk; + struct clk *clkout; + const char *clk_out_name; + struct clk_hw hw; unsigned long clkin; unsigned long chspc; /* Channel Spacing */ unsigned long fpfd; /* Phase Frequency Detector */ @@ -61,6 +65,8 @@ struct adf4350_state { __be32 val __aligned(IIO_DMA_MINALIGN); }; +#define to_adf4350_state(_hw) container_of(_hw, struct adf4350_state, hw) + static struct adf4350_platform_data default_pdata = { .channel_spacing = 10000, .r2_user_settings = ADF4350_REG2_PD_POLARITY_POS | @@ -370,6 +376,12 @@ static const struct iio_chan_spec_ext_info adf4350_ext_info[] = { { }, }; +static const struct iio_chan_spec_ext_info adf4350_clk_ext_info[] = { + _ADF4350_EXT_INFO("frequency_resolution", ADF4350_FREQ_RESOLUTION), + _ADF4350_EXT_INFO("powerdown", ADF4350_PWRDOWN), + { }, +}; + static const struct iio_chan_spec adf4350_chan = { .type = IIO_ALTVOLTAGE, .indexed = 1, @@ -377,10 +389,122 @@ static const struct iio_chan_spec adf4350_chan = { .ext_info = adf4350_ext_info, }; +static const struct iio_chan_spec adf4350_clk_chan = { + .type = IIO_ALTVOLTAGE, + .indexed = 1, + .output = 1, + .ext_info = adf4350_clk_ext_info, +}; + static const struct iio_info adf4350_info = { .debugfs_reg_access = &adf4350_reg_access, }; +static void adf4350_clk_del_provider(void *data) +{ + struct adf4350_state *st = data; + + of_clk_del_provider(st->spi->dev.of_node); +} + +static unsigned long adf4350_clk_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct adf4350_state *st = to_adf4350_state(hw); + unsigned long long tmp; + + tmp = (u64)(st->r0_int * st->r1_mod + st->r0_fract) * st->fpfd; + do_div(tmp, st->r1_mod * (1 << st->r4_rf_div_sel)); + + return tmp; +} + +static int adf4350_clk_set_rate(struct clk_hw *hw, + unsigned long rate, + unsigned long parent_rate) +{ + struct adf4350_state *st = to_adf4350_state(hw); + + if (parent_rate == 0 || parent_rate > ADF4350_MAX_FREQ_REFIN) + return -EINVAL; + + st->clkin = parent_rate; + + return adf4350_set_freq(st, rate); +} + +static int adf4350_clk_prepare(struct clk_hw *hw) +{ + struct adf4350_state *st = to_adf4350_state(hw); + + st->regs[ADF4350_REG2] &= ~ADF4350_REG2_POWER_DOWN_EN; + + return adf4350_sync_config(st); +} + +static void adf4350_clk_unprepare(struct clk_hw *hw) +{ + struct adf4350_state *st = to_adf4350_state(hw); + + st->regs[ADF4350_REG2] |= ADF4350_REG2_POWER_DOWN_EN; + + adf4350_sync_config(st); +} + +static int adf4350_clk_is_enabled(struct clk_hw *hw) +{ + struct adf4350_state *st = to_adf4350_state(hw); + + return (st->regs[ADF4350_REG2] & ADF4350_REG2_POWER_DOWN_EN); +} + +static const struct clk_ops adf4350_clk_ops = { + .recalc_rate = adf4350_clk_recalc_rate, + .set_rate = adf4350_clk_set_rate, + .prepare = adf4350_clk_prepare, + .unprepare = adf4350_clk_unprepare, + .is_enabled = adf4350_clk_is_enabled, +}; + +static int adf4350_clk_register(struct adf4350_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; + + init.name = devm_kasprintf(&spi->dev, GFP_KERNEL, "%s-clk", + fwnode_get_name(dev_fwnode(&spi->dev))); + device_property_read_string(&spi->dev, "clock-output-names", + &init.name); + + parent_name = of_clk_get_parent_name(spi->dev.of_node, 0); + if (!parent_name) + return -EINVAL; + + init.ops = &adf4350_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); + + 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, adf4350_clk_del_provider, st); +} + static struct adf4350_platform_data *adf4350_parse_dt(struct device *dev) { struct adf4350_platform_data *pdata; @@ -522,7 +646,6 @@ static int adf4350_probe(struct spi_device *spi) indio_dev->info = &adf4350_info; indio_dev->modes = INDIO_DIRECT_MODE; - indio_dev->channels = &adf4350_chan; indio_dev->num_channels = 1; mutex_init(&st->lock); @@ -551,6 +674,15 @@ static int adf4350_probe(struct spi_device *spi) return ret; } + ret = adf4350_clk_register(st); + if (ret) + return ret; + + if (st->clkout) + indio_dev->channels = &adf4350_clk_chan; + else + indio_dev->channels = &adf4350_chan; + ret = devm_add_action_or_reset(&spi->dev, adf4350_power_down, indio_dev); if (ret) return dev_err_probe(&spi->dev, ret,
Add clk provider feature for the adf4350. 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@analog.com> --- changes in v4: - rename macro to `to_adf4350_state` - do not expose ADF4350_FREQ and ADF4350_FREQ_REFIN if driver is used as clk provider. - initialize flags with CLK_SET_RATE_PARENT drivers/iio/frequency/adf4350.c | 134 +++++++++++++++++++++++++++++++- 1 file changed, 133 insertions(+), 1 deletion(-)