* [PATCH 1/3] iio: frequency: adf4371: Create a power down/up function @ 2019-12-19 13:37 Beniamin Bia 2019-12-19 13:37 ` [PATCH 2/3] iio: frequency: adf4371: Introduce channel child nodes Beniamin Bia 2019-12-19 13:37 ` [PATCH 3/3] dt-binding: iio: Add documentation for ADF4371 channel child notes Beniamin Bia 0 siblings, 2 replies; 5+ messages in thread From: Beniamin Bia @ 2019-12-19 13:37 UTC (permalink / raw) To: jic23 Cc: knaack.h, lars, pmeerw, robh+dt, mark.rutland, Michael.Hennerich, linux-iio, devicetree, biabeniamin, Stefan Popa, Beniamin Bia From: Stefan Popa <stefan.popa@analog.com> This patch creates a helper function which powers down/up the PLL output channels. Currently, this function is called only once, but it will be needed in future patches to support multiple features. Signed-off-by: Stefan Popa <stefan.popa@analog.com> Signed-off-by: Beniamin Bia <beniamin.bia@analog.com> --- drivers/iio/frequency/adf4371.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/drivers/iio/frequency/adf4371.c b/drivers/iio/frequency/adf4371.c index ff82863cbf42..7d77ebdbea82 100644 --- a/drivers/iio/frequency/adf4371.c +++ b/drivers/iio/frequency/adf4371.c @@ -321,6 +321,24 @@ static int adf4371_set_freq(struct adf4371_state *st, unsigned long long freq, return regmap_write(st->regmap, ADF4371_REG(0x10), st->integer & 0xFF); } +static int adf4371_channel_power_down(struct adf4371_state *st, + unsigned int channel, bool power_down) +{ + unsigned int bit, readval, reg; + int ret; + + reg = adf4371_pwrdown_ch[channel].reg; + bit = adf4371_pwrdown_ch[channel].bit; + ret = regmap_read(st->regmap, reg, &readval); + if (ret < 0) + return ret; + + readval &= ~BIT(bit); + readval |= (!power_down << bit); + + return regmap_write(st->regmap, reg, readval); +} + static ssize_t adf4371_read(struct iio_dev *indio_dev, uintptr_t private, const struct iio_chan_spec *chan, @@ -372,7 +390,6 @@ static ssize_t adf4371_write(struct iio_dev *indio_dev, struct adf4371_state *st = iio_priv(indio_dev); unsigned long long freq; bool power_down; - unsigned int bit, readval, reg; int ret; mutex_lock(&st->lock); @@ -389,16 +406,7 @@ static ssize_t adf4371_write(struct iio_dev *indio_dev, if (ret) break; - reg = adf4371_pwrdown_ch[chan->channel].reg; - bit = adf4371_pwrdown_ch[chan->channel].bit; - ret = regmap_read(st->regmap, reg, &readval); - if (ret < 0) - break; - - readval &= ~BIT(bit); - readval |= (!power_down << bit); - - ret = regmap_write(st->regmap, reg, readval); + ret = adf4371_channel_power_down(st, chan->channel, power_down); break; default: ret = -EINVAL; -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] iio: frequency: adf4371: Introduce channel child nodes 2019-12-19 13:37 [PATCH 1/3] iio: frequency: adf4371: Create a power down/up function Beniamin Bia @ 2019-12-19 13:37 ` Beniamin Bia 2019-12-23 11:02 ` Jonathan Cameron 2019-12-19 13:37 ` [PATCH 3/3] dt-binding: iio: Add documentation for ADF4371 channel child notes Beniamin Bia 1 sibling, 1 reply; 5+ messages in thread From: Beniamin Bia @ 2019-12-19 13:37 UTC (permalink / raw) To: jic23 Cc: knaack.h, lars, pmeerw, robh+dt, mark.rutland, Michael.Hennerich, linux-iio, devicetree, biabeniamin, Stefan Popa, Beniamin Bia From: Stefan Popa <stefan.popa@analog.com> The ADF4371/ADF4372 devices support individual configurations for the output channels. Each child node represents a channel for which "power-up-frequency" and "output-enable" optional properties are currently supported. If the "power-up-frequency" is specified for a channel, the driver checks if the value provided is in sync with the frequencies set on the other channels. This limitation is due to the fact that all the channel frequencies are derived from the VCO fundamental frequency. If the "output-enable" property is specified, then the channel will be enabled, otherwise, the driver will initialize the defaults (RF8x will be the only enabled channel). Signed-off-by: Stefan Popa <stefan.popa@analog.com> Signed-off-by: Beniamin Bia <beniamin.bia@analog.com> --- drivers/iio/frequency/adf4371.c | 79 ++++++++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 2 deletions(-) diff --git a/drivers/iio/frequency/adf4371.c b/drivers/iio/frequency/adf4371.c index 7d77ebdbea82..e2a599b912e5 100644 --- a/drivers/iio/frequency/adf4371.c +++ b/drivers/iio/frequency/adf4371.c @@ -154,10 +154,16 @@ struct adf4371_chip_info { const struct iio_chan_spec *channels; }; +struct adf4371_channel_config { + bool enable; + unsigned long long freq; +}; + struct adf4371_state { struct spi_device *spi; struct regmap *regmap; struct clk *clkin; + struct adf4371_channel_config channel_cfg[4]; /* * Lock for accessing device registers. Some operations require * multiple consecutive R/W operations, during which the device @@ -175,6 +181,7 @@ struct adf4371_state { unsigned int mod2; unsigned int rf_div_sel; unsigned int ref_div_factor; + bool mute_till_lock_en; u8 buf[10] ____cacheline_aligned; }; @@ -480,6 +487,36 @@ static const struct iio_info adf4371_info = { .debugfs_reg_access = &adf4371_reg_access, }; +static int adf4371_channel_config(struct adf4371_state *st) +{ + unsigned long long rate; + int i, ret; + + for (i = 0; i < st->chip_info->num_channels; i++) { + if (st->channel_cfg[i].freq == 0) + continue; + + rate = adf4371_pll_fract_n_get_rate(st, i); + if (rate == 0) { + ret = adf4371_set_freq(st, st->channel_cfg[i].freq, i); + if (ret < 0) + return ret; + } else if (rate != st->channel_cfg[i].freq) { + dev_err(&st->spi->dev, + "Clock rate for chanel %d is not in sync\n", i); + return -EINVAL; + } + /* Powerup channel if the property was specified in the dt */ + if (st->channel_cfg[i].enable) { + ret = adf4371_channel_power_down(st, i, false); + if (ret < 0) + return ret; + } + } + + return 0; +} + static int adf4371_setup(struct adf4371_state *st) { unsigned int synth_timeout = 2, timeout = 1, vco_alc_timeout = 1; @@ -497,7 +534,7 @@ static int adf4371_setup(struct adf4371_state *st) return ret; /* Mute to Lock Detect */ - if (device_property_read_bool(&st->spi->dev, "adi,mute-till-lock-en")) { + if (st->mute_till_lock_en) { ret = regmap_update_bits(st->regmap, ADF4371_REG(0x25), ADF4371_MUTE_LD_MSK, ADF4371_MUTE_LD(1)); @@ -545,7 +582,11 @@ static int adf4371_setup(struct adf4371_state *st) st->buf[3] = synth_timeout; st->buf[4] = ADF4371_VCO_ALC_TOUT(vco_alc_timeout); - return regmap_bulk_write(st->regmap, ADF4371_REG(0x30), st->buf, 5); + ret = regmap_bulk_write(st->regmap, ADF4371_REG(0x30), st->buf, 5); + if (ret < 0) + return 0; + + return adf4371_channel_config(st); } static void adf4371_clk_disable(void *data) @@ -555,6 +596,36 @@ static void adf4371_clk_disable(void *data) clk_disable_unprepare(st->clkin); } +static int adf4371_parse_dt(struct adf4371_state *st) +{ + unsigned char num_channels; + unsigned int channel; + struct fwnode_handle *child; + int ret; + + if (device_property_read_bool(&st->spi->dev, "adi,mute-till-lock-en")) + st->mute_till_lock_en = true; + + num_channels = device_get_child_node_count(&st->spi->dev); + if (num_channels > st->chip_info->num_channels) + return -EINVAL; + + device_for_each_child_node(&st->spi->dev, child) { + ret = fwnode_property_read_u32(child, "reg", &channel); + if (ret) + return ret; + + ret = fwnode_property_present(child, "adi,output-enable"); + st->channel_cfg[channel].enable = ret; + + fwnode_property_read_u64(child, + "adi,power-up-frequency", + &st->channel_cfg[channel].freq); + } + + return 0; +} + static int adf4371_probe(struct spi_device *spi) { const struct spi_device_id *id = spi_get_device_id(spi); @@ -602,6 +673,10 @@ static int adf4371_probe(struct spi_device *spi) st->clkin_freq = clk_get_rate(st->clkin); + ret = adf4371_parse_dt(st); + if (ret < 0) + return ret; + ret = adf4371_setup(st); if (ret < 0) { dev_err(&spi->dev, "ADF4371 setup failed\n"); -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] iio: frequency: adf4371: Introduce channel child nodes 2019-12-19 13:37 ` [PATCH 2/3] iio: frequency: adf4371: Introduce channel child nodes Beniamin Bia @ 2019-12-23 11:02 ` Jonathan Cameron 0 siblings, 0 replies; 5+ messages in thread From: Jonathan Cameron @ 2019-12-23 11:02 UTC (permalink / raw) To: Beniamin Bia Cc: knaack.h, lars, pmeerw, robh+dt, mark.rutland, Michael.Hennerich, linux-iio, devicetree, biabeniamin, Stefan Popa On Thu, 19 Dec 2019 15:37:54 +0200 Beniamin Bia <beniamin.bia@analog.com> wrote: > From: Stefan Popa <stefan.popa@analog.com> > > The ADF4371/ADF4372 devices support individual configurations for the > output channels. Each child node represents a channel for which > "power-up-frequency" and "output-enable" optional properties are currently > supported. > > If the "power-up-frequency" is specified for a channel, the driver checks > if the value provided is in sync with the frequencies set on the other > channels. This limitation is due to the fact that all the channel > frequencies are derived from the VCO fundamental frequency. > > If the "output-enable" property is specified, then the channel will be > enabled, otherwise, the driver will initialize the defaults (RF8x will > be the only enabled channel). > > Signed-off-by: Stefan Popa <stefan.popa@analog.com> > Signed-off-by: Beniamin Bia <beniamin.bia@analog.com> The code looks good. Question outstanding is the one I raised in review of the binding doc. What's this for? Thanks, Jonathan > --- > drivers/iio/frequency/adf4371.c | 79 ++++++++++++++++++++++++++++++++- > 1 file changed, 77 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/frequency/adf4371.c b/drivers/iio/frequency/adf4371.c > index 7d77ebdbea82..e2a599b912e5 100644 > --- a/drivers/iio/frequency/adf4371.c > +++ b/drivers/iio/frequency/adf4371.c > @@ -154,10 +154,16 @@ struct adf4371_chip_info { > const struct iio_chan_spec *channels; > }; > > +struct adf4371_channel_config { > + bool enable; > + unsigned long long freq; > +}; > + > struct adf4371_state { > struct spi_device *spi; > struct regmap *regmap; > struct clk *clkin; > + struct adf4371_channel_config channel_cfg[4]; > /* > * Lock for accessing device registers. Some operations require > * multiple consecutive R/W operations, during which the device > @@ -175,6 +181,7 @@ struct adf4371_state { > unsigned int mod2; > unsigned int rf_div_sel; > unsigned int ref_div_factor; > + bool mute_till_lock_en; > u8 buf[10] ____cacheline_aligned; > }; > > @@ -480,6 +487,36 @@ static const struct iio_info adf4371_info = { > .debugfs_reg_access = &adf4371_reg_access, > }; > > +static int adf4371_channel_config(struct adf4371_state *st) > +{ > + unsigned long long rate; > + int i, ret; > + > + for (i = 0; i < st->chip_info->num_channels; i++) { > + if (st->channel_cfg[i].freq == 0) > + continue; > + > + rate = adf4371_pll_fract_n_get_rate(st, i); > + if (rate == 0) { > + ret = adf4371_set_freq(st, st->channel_cfg[i].freq, i); > + if (ret < 0) > + return ret; > + } else if (rate != st->channel_cfg[i].freq) { > + dev_err(&st->spi->dev, > + "Clock rate for chanel %d is not in sync\n", i); > + return -EINVAL; > + } > + /* Powerup channel if the property was specified in the dt */ > + if (st->channel_cfg[i].enable) { > + ret = adf4371_channel_power_down(st, i, false); > + if (ret < 0) > + return ret; > + } > + } > + > + return 0; > +} > + > static int adf4371_setup(struct adf4371_state *st) > { > unsigned int synth_timeout = 2, timeout = 1, vco_alc_timeout = 1; > @@ -497,7 +534,7 @@ static int adf4371_setup(struct adf4371_state *st) > return ret; > > /* Mute to Lock Detect */ > - if (device_property_read_bool(&st->spi->dev, "adi,mute-till-lock-en")) { > + if (st->mute_till_lock_en) { > ret = regmap_update_bits(st->regmap, ADF4371_REG(0x25), > ADF4371_MUTE_LD_MSK, > ADF4371_MUTE_LD(1)); > @@ -545,7 +582,11 @@ static int adf4371_setup(struct adf4371_state *st) > st->buf[3] = synth_timeout; > st->buf[4] = ADF4371_VCO_ALC_TOUT(vco_alc_timeout); > > - return regmap_bulk_write(st->regmap, ADF4371_REG(0x30), st->buf, 5); > + ret = regmap_bulk_write(st->regmap, ADF4371_REG(0x30), st->buf, 5); > + if (ret < 0) > + return 0; > + > + return adf4371_channel_config(st); > } > > static void adf4371_clk_disable(void *data) > @@ -555,6 +596,36 @@ static void adf4371_clk_disable(void *data) > clk_disable_unprepare(st->clkin); > } > > +static int adf4371_parse_dt(struct adf4371_state *st) > +{ > + unsigned char num_channels; > + unsigned int channel; > + struct fwnode_handle *child; > + int ret; > + > + if (device_property_read_bool(&st->spi->dev, "adi,mute-till-lock-en")) > + st->mute_till_lock_en = true; > + > + num_channels = device_get_child_node_count(&st->spi->dev); > + if (num_channels > st->chip_info->num_channels) > + return -EINVAL; > + > + device_for_each_child_node(&st->spi->dev, child) { > + ret = fwnode_property_read_u32(child, "reg", &channel); > + if (ret) > + return ret; > + > + ret = fwnode_property_present(child, "adi,output-enable"); > + st->channel_cfg[channel].enable = ret; > + > + fwnode_property_read_u64(child, > + "adi,power-up-frequency", > + &st->channel_cfg[channel].freq); > + } > + > + return 0; > +} > + > static int adf4371_probe(struct spi_device *spi) > { > const struct spi_device_id *id = spi_get_device_id(spi); > @@ -602,6 +673,10 @@ static int adf4371_probe(struct spi_device *spi) > > st->clkin_freq = clk_get_rate(st->clkin); > > + ret = adf4371_parse_dt(st); > + if (ret < 0) > + return ret; > + > ret = adf4371_setup(st); > if (ret < 0) { > dev_err(&spi->dev, "ADF4371 setup failed\n"); ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 3/3] dt-binding: iio: Add documentation for ADF4371 channel child notes 2019-12-19 13:37 [PATCH 1/3] iio: frequency: adf4371: Create a power down/up function Beniamin Bia 2019-12-19 13:37 ` [PATCH 2/3] iio: frequency: adf4371: Introduce channel child nodes Beniamin Bia @ 2019-12-19 13:37 ` Beniamin Bia 2019-12-23 11:00 ` Jonathan Cameron 1 sibling, 1 reply; 5+ messages in thread From: Beniamin Bia @ 2019-12-19 13:37 UTC (permalink / raw) To: jic23 Cc: knaack.h, lars, pmeerw, robh+dt, mark.rutland, Michael.Hennerich, linux-iio, devicetree, biabeniamin, Beniamin Bia This patch documents the ADF4371 individual channel configuration. Signed-off-by: Beniamin Bia <beniamin.bia@analog.com> --- .../bindings/iio/frequency/adf4371.yaml | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml b/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml index 7ec3ec94356b..5339c929e883 100644 --- a/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml +++ b/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml @@ -40,12 +40,48 @@ properties: output stage will shut down until the ADF4371/ADF4372 achieves lock as measured by the digital lock detect circuitry. + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + required: - compatible - reg - clocks - clock-names + patternProperties: + "^channel@[01]$": + type: object + description: Represents the external channels which are connected to the device. + + properties: + reg: + description: | + The channel number. It can have up to 3 channels on adf4372 + and 4 channels on adf4371, numbered from 0 to 3. + maxItems: 1 + + adi,output-enable: + description: | + If this property is specified, the output channel will be enabled. + If left empty, the driver will initialize the defaults (RF8x, channel 0 + will be the only one enabled). + maxItems: 1 + + adi,power-up-frequency: + description: | + Set the frequency after power up for the channel. If this property is + specified, it has to be in sync with the power up frequency set on the + other channels. This limitation is due to the fact that all the channel + frequencies are derived from the VCO fundamental frequency. + maxItems: 1 + + required: + - reg + examples: - | spi0 { @@ -55,9 +91,36 @@ examples: frequency@0 { compatible = "adi,adf4371"; reg = <0>; + + #address-cells = <1>; + #size-cells = <0>; + spi-max-frequency = <1000000>; clocks = <&adf4371_clkin>; clock-names = "clkin"; + + channel@0 { + reg = <0>; + adi,output-enable; + adi,power-up-frequency = /bits/ 64 <8000000000>; + }; + + channel@1 { + reg = <1>; + adi,output-enable; + }; + + channel@2 { + reg = <2>; + adi,output-enable; + adi,power-up-frequency = /bits/ 64 <16000000000>; + }; + + channel@3 { + reg = <3>; + adi,output-enable; + adi,power-up-frequency = /bits/ 64 <32000000000>; + }; }; }; ... -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3] dt-binding: iio: Add documentation for ADF4371 channel child notes 2019-12-19 13:37 ` [PATCH 3/3] dt-binding: iio: Add documentation for ADF4371 channel child notes Beniamin Bia @ 2019-12-23 11:00 ` Jonathan Cameron 0 siblings, 0 replies; 5+ messages in thread From: Jonathan Cameron @ 2019-12-23 11:00 UTC (permalink / raw) To: Beniamin Bia Cc: knaack.h, lars, pmeerw, robh+dt, mark.rutland, Michael.Hennerich, linux-iio, devicetree, biabeniamin On Thu, 19 Dec 2019 15:37:55 +0200 Beniamin Bia <beniamin.bia@analog.com> wrote: > This patch documents the ADF4371 individual channel configuration. > > Signed-off-by: Beniamin Bia <beniamin.bia@analog.com> > --- > .../bindings/iio/frequency/adf4371.yaml | 63 +++++++++++++++++++ > 1 file changed, 63 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml b/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml > index 7ec3ec94356b..5339c929e883 100644 > --- a/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml > +++ b/Documentation/devicetree/bindings/iio/frequency/adf4371.yaml > @@ -40,12 +40,48 @@ properties: > output stage will shut down until the ADF4371/ADF4372 achieves lock as > measured by the digital lock detect circuitry. > > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > required: > - compatible > - reg > - clocks > - clock-names > > + patternProperties: > + "^channel@[01]$": > + type: object > + description: Represents the external channels which are connected to the device. > + > + properties: > + reg: > + description: | > + The channel number. It can have up to 3 channels on adf4372 > + and 4 channels on adf4371, numbered from 0 to 3. > + maxItems: 1 > + > + adi,output-enable: > + description: | > + If this property is specified, the output channel will be enabled. > + If left empty, the driver will initialize the defaults (RF8x, channel 0 > + will be the only one enabled). If I read the driver right, this is effectively 'power up channel when driver loads'. Why do we need that in DT? A bit of udev magic or similar and the existing controls can set both this and the frequency below. Would happen a tiny bit after the driver loads, but I assume the driver current loads with the channels turned off so that shouldn't matter... > + maxItems: 1 > + > + adi,power-up-frequency: > + description: | > + Set the frequency after power up for the channel. If this property is > + specified, it has to be in sync with the power up frequency set on the > + other channels. This limitation is due to the fact that all the channel > + frequencies are derived from the VCO fundamental frequency. > + maxItems: 1 > + > + required: > + - reg > + > examples: > - | > spi0 { > @@ -55,9 +91,36 @@ examples: > frequency@0 { > compatible = "adi,adf4371"; > reg = <0>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > spi-max-frequency = <1000000>; > clocks = <&adf4371_clkin>; > clock-names = "clkin"; > + > + channel@0 { > + reg = <0>; > + adi,output-enable; > + adi,power-up-frequency = /bits/ 64 <8000000000>; > + }; > + > + channel@1 { > + reg = <1>; > + adi,output-enable; > + }; > + > + channel@2 { > + reg = <2>; > + adi,output-enable; > + adi,power-up-frequency = /bits/ 64 <16000000000>; > + }; > + > + channel@3 { > + reg = <3>; > + adi,output-enable; > + adi,power-up-frequency = /bits/ 64 <32000000000>; > + }; > }; > }; > ... ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-12-23 11:02 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-19 13:37 [PATCH 1/3] iio: frequency: adf4371: Create a power down/up function Beniamin Bia 2019-12-19 13:37 ` [PATCH 2/3] iio: frequency: adf4371: Introduce channel child nodes Beniamin Bia 2019-12-23 11:02 ` Jonathan Cameron 2019-12-19 13:37 ` [PATCH 3/3] dt-binding: iio: Add documentation for ADF4371 channel child notes Beniamin Bia 2019-12-23 11:00 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).