Skip to content

iio: frequency: hmc7044: External VCO Mode Fixes #3102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
BrentK-ADI wants to merge 4 commits into
base: main
Choose a base branch
from
Open

iio: frequency: hmc7044: External VCO Mode Fixes #3102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f1c89e4
iio: frequency: hmc7044: Fix external VCO (PLL2) implementation
BrentK-ADI Jan 29, 2026
fe30f1b
dt-bindings: iio: frequency: hmc7044: add adi,pll2-pfd-invert-enable
BrentK-ADI Jan 29, 2026
b2d4ba1
iio: frequency: hmc7044: Add PLL2 PFD inversion support
BrentK-ADI Jan 29, 2026
949fce3
iio: frequency: hmc7044: Disable PLL1 when no CLKIN present
danmois Feb 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Documentation/devicetree/bindings/iio/frequency/hmc7044.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ Optional properties hmc7044:
pulse generator.
- adi,clkin0-rf-sync-enable: CLKIN0 input is used for external RF sync.
- adi,clkin1-vco-in-enable: CLKIN1 input is used for external VCO.
- adi,pll2-pfd-invert-enable: Inverts the polarity of PLL2's PFD
- adi,rf-reseeder-disable: Disables RF reseed for SYSREF.

Optional properties hmc7043:
Expand Down
174 changes: 97 additions & 77 deletions drivers/iio/frequency/hmc7044.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@
#define HMC7044_REG_PLL2_N_MSB 0x0036
#define HMC7044_N2_MSB(x) (((x) & 0xff00) >> 8)

#define HMC7044_REG_PLL2_PFD_CTRL 0x0038
#define HMC7044_PFD_UP_EN BIT(4)
#define HMC7044_PFD_DOWN_EN BIT(3)
#define HMC7044_PFD_POLARITY BIT(0)

#define HMC7044_REG_OSCOUT_PATH 0x0039
#define HMC7044_REG_OSCOUT_DRIVER_0 0x003A
#define HMC7044_REG_OSCOUT_DRIVER_1 0x003B
Expand Down Expand Up @@ -217,10 +222,13 @@

#define HMC7044_NUM_CHAN 14

#define HMC7044_LOW_VCO_MIN 2150000
#define HMC7044_LOW_VCO_MAX 2880000
#define HMC7044_HIGH_VCO_MIN 2650000
#define HMC7044_HIGH_VCO_MAX 3200000
#define HMC7044_LOW_VCO_MIN_KHZ 2150000
#define HMC7044_LOW_VCO_MAX_KHZ 2880000
#define HMC7044_HIGH_VCO_MIN_KHZ 2650000
#define HMC7044_HIGH_VCO_MAX_KHZ 3200000
Copy link
Collaborator

@nunojsa nunojsa Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of unrelated change. Adds more diff than needed... Ideally cosmetics changes like this should be done in preliminary patches and apply to the complete driver so the style is consistent.

#define HMC7044_EXT_VCO_MIN_KHZ 800000
#define HMC7044_EXT_VCO_MAX_KHZ 3200000
#define HMC7044_EXT_VCO_LOW_THRESH_KHZ 1000000

#define HMC7044_RECOMM_LCM_MIN 30000
#define HMC7044_RECOMM_LCM_MAX 70000
Expand Down Expand Up @@ -305,6 +313,7 @@
bool pll1_ref_autorevert_en;
bool clkin0_rfsync_en;
bool clkin1_vcoin_en;
bool pll2_pfd_invert_en;
bool high_performance_mode_clock_dist_en;
bool rf_reseeder_en;
bool oscout_path_en;
Expand Down Expand Up @@ -636,7 +645,7 @@
i = clk_get_nshot(sync_clk);
if (i < 0) {
clk_put(sync_clk);
return ret;

Check warning on line 648 in drivers/iio/frequency/hmc7044.c

View workflow job for this annotation

GitHub Actions / build_gcc_arm / build 

kernel_smatch: missing error code? 'ret'
}

if (i != 1) {
Expand Down Expand Up @@ -890,7 +899,7 @@
return 0;
}

if (hmc->device_id == HMC7044 && !hmc->clkin1_vcoin_en) {
if (hmc->device_id == HMC7044) {
ret = hmc7044_read(indio_dev,
HMC7044_REG_PLL1_STATUS, &pll1_stat);
if (ret < 0)
Expand All @@ -910,29 +919,58 @@
return ret;

active = HMC7044_PLL1_ACTIVE_CLKIN(pll1_stat);
} else {
active = 1;
}

if (hmc->device_id == HMC7043)
active = 0;

if (hmc->clkin_freq_ccf[active])

Check failure on line 927 in drivers/iio/frequency/hmc7044.c

View workflow job for this annotation

GitHub Actions / build_gcc_arm / build 

kernel_smatch: uninitialized symbol 'active'.

Check warning on line 927 in drivers/iio/frequency/hmc7044.c

View workflow job for this annotation

GitHub Actions / build_llvm_x86_64 / build 

clang_analyzer: Array subscript is undefined [core.uninitialized.ArraySubscript]
927 | if (hmc->clkin_freq_ccf[active])
| ^ ~~~~~~
clkin_freq = hmc->clkin_freq_ccf[active];
else
clkin_freq = hmc->clkin_freq[active];

if (hmc->device_id == HMC7044 && !hmc->clkin1_vcoin_en)
if (hmc->device_id == HMC7044)
dev_info(&hmc->spi->dev,
"PLL1: %s, CLKIN%u @ %u Hz, PFD: %u kHz - PLL2: %s @ %u.%06u MHz\n",
pll1_fsm_states[HMC7044_PLL1_FSM_STATE(pll1_stat)],
active, clkin_freq, hmc->pll1_pfd,
HMC7044_PLL2_LOCK_DETECT(alarm_stat) ?
"Locked" : "Unlocked", hmc->pll2_freq / 1000000,
hmc->pll2_freq % 1000000);
else
dev_info(&hmc->spi->dev, "CLKIN%u @ %u.%06u MHz\n", active,
clkin_freq / 1000000, clkin_freq % 1000000);

return 0;
}

static int hmc7044_validate_pll2_freq(struct hmc7044 *hmc, unsigned long pll2_freq_khz)
{
unsigned long limit_max;
unsigned long limit_min;

if (hmc->clkin1_vcoin_en) {
limit_max = HMC7044_EXT_VCO_MAX_KHZ;
limit_min = HMC7044_EXT_VCO_MIN_KHZ;
} else {
limit_max = HMC7044_HIGH_VCO_MAX_KHZ;
limit_min = HMC7044_LOW_VCO_MIN_KHZ;
}

if (pll2_freq_khz < limit_min ||
pll2_freq_khz > limit_max) {
if (hmc->ignore_vco_limits) {
/*
* Debug only, is at own risk!
* May fail across process, voltage and temperature
*/
dev_warn(&hmc->spi->dev,
"PLL2 frequency %lu kHz is out of range, ignoring limits\n",
pll2_freq_khz);
} else {
dev_err(&hmc->spi->dev,
"PLL2 frequency %lu kHz is out of range (%lu - %lu)\n",
pll2_freq_khz, limit_min, limit_max);
return -EINVAL;
}
}

return 0;
}
Expand All @@ -948,19 +986,20 @@
{
struct hmc7044 *hmc = iio_priv(indio_dev);
struct hmc7044_chan_spec *chan;
bool high_vco_en;
bool pll2_freq_doubler_en;
unsigned long vcxo_freq, pll2_freq;
unsigned long clkin_freq[4];
unsigned long lcm_freq;
unsigned int in_prescaler[5];
unsigned int vco_sel;
unsigned long pll1_lock_detect;
unsigned long n1, r1;
unsigned long n, r;
unsigned long pfd1_freq;
unsigned long vco_limit;
unsigned long n2[2], r2[2];
unsigned int i, c, ref_en = 0;
u32 pll1_stat;
int ret;

vcxo_freq = hmc->vcxo_freq / 1000;
Expand Down Expand Up @@ -1000,7 +1039,7 @@
pfd1_freq = vcxo_freq / n1;

n = n1;
r = r1;

Check warning on line 1042 in drivers/iio/frequency/hmc7044.c

View workflow job for this annotation

GitHub Actions / build_llvm_x86_64 / build 

clang_analyzer: Value stored to 'r' is never read [deadcode.DeadStores]
1042 | r = r1;
| ^ ~~
while (pfd1_freq > hmc->pfd1_limit) {
do {
n++;
Expand All @@ -1018,28 +1057,26 @@

hmc->pll1_pfd = pfd1_freq;

if (pll2_freq < HMC7044_LOW_VCO_MIN ||
pll2_freq > HMC7044_HIGH_VCO_MAX) {
if (hmc->ignore_vco_limits) {
/* Debug only, is at own risk! May fail across process, voltage and temperature */
dev_warn(&hmc->spi->dev,
"PLL2 frequency %lu kHz is out of range, "
"ignoring limits\n", pll2_freq);
} else {
dev_err(&hmc->spi->dev,
"PLL2 frequency %lu kHz is out of range (%u - %u)\n",
pll2_freq, HMC7044_LOW_VCO_MIN / 1000,
HMC7044_HIGH_VCO_MAX / 1000);
return -EINVAL;
ret = hmc7044_validate_pll2_freq(hmc, pll2_freq);
if (ret)
return ret;
Copy link
Collaborator

@nunojsa nunojsa Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, the above helper could have been added in a preliminary patch. It makes the reviewer life easier


if (hmc->clkin1_vcoin_en) {
vco_sel = HMC7044_VCO_EXT;
if (pll2_freq < HMC7044_EXT_VCO_LOW_THRESH_KHZ) {
ret = hmc7044_write(indio_dev, HMC7044_CLK_INPUT_CTRL,
HMC7044_LOW_FREQ_INPUT_MODE);
if (ret)
return ret;
}
} else {
vco_limit = (HMC7044_LOW_VCO_MAX_KHZ + HMC7044_HIGH_VCO_MIN_KHZ) / 2;
if (pll2_freq >= vco_limit)
vco_sel = HMC7044_VCO_HIGH;
else
vco_sel = HMC7044_VCO_LOW;
}

vco_limit = (HMC7044_LOW_VCO_MAX + HMC7044_HIGH_VCO_MIN) / 2;
if (pll2_freq >= vco_limit)
high_vco_en = true;
else
high_vco_en = false;

/* fVCO / N2 = fVCXO * doubler / R2 */
pll2_freq_doubler_en = true;
rational_best_approximation(pll2_freq, vcxo_freq * 2,
Expand Down Expand Up @@ -1110,40 +1147,28 @@
return ret;

/* Program PLL2 */
ret = hmc7044_write(indio_dev, HMC7044_REG_PLL2_PFD_CTRL,
HMC7044_PFD_UP_EN | HMC7044_PFD_DOWN_EN |
(hmc->pll2_pfd_invert_en ? HMC7044_PFD_POLARITY : 0));
if (ret)
return ret;

/* Select the VCO range */

if (hmc->clkin1_vcoin_en) {
hmc->pll2_freq = hmc->clkin_freq_ccf[1] ?
hmc->clkin_freq_ccf[1] : hmc->clkin_freq[1];

if (hmc->pll2_freq < 1000000000U) {
ret = hmc7044_write(indio_dev, HMC7044_CLK_INPUT_CTRL,
HMC7044_LOW_FREQ_INPUT_MODE);
if (ret)
return ret;
}
ret = hmc7044_write(indio_dev, HMC7044_REG_EN_CTRL_0,
(hmc->rf_reseeder_en ? HMC7044_RF_RESEEDER_EN : 0) |
HMC7044_VCO_SEL(vco_sel) |
HMC7044_SYSREF_TIMER_EN | HMC7044_PLL2_EN |
(ref_en ? HMC7044_PLL1_EN : 0));
if (ret)
return ret;

ret = hmc7044_write(indio_dev, HMC7044_REG_EN_CTRL_0,
(hmc->rf_reseeder_en ? HMC7044_RF_RESEEDER_EN : 0) |
HMC7044_VCO_SEL(0) |
HMC7044_SYSREF_TIMER_EN);
if (ret)
return ret;
if (!ref_en)
dev_info(&hmc->spi->dev,
"PLL1 disabled, no valid CLKIN reference\n");

if (hmc->clkin1_vcoin_en) {
ret = hmc7044_write(indio_dev, HMC7044_REG_SYNC, HMC7044_SYNC_RETIME);
Copy link
Contributor

@mhennerich mhennerich Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HMC7044_SYNC_RETIME seems to be removed entirely? Is this on purpose?

Copy link
Collaborator Author

@BrentK-ADI BrentK-ADI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. This was not intentional. Added back in.

if (ret)
return ret;
} else {
ret = hmc7044_write(indio_dev, HMC7044_REG_EN_CTRL_0,
(hmc->rf_reseeder_en ? HMC7044_RF_RESEEDER_EN : 0) |
HMC7044_VCO_SEL(high_vco_en ?
HMC7044_VCO_HIGH :
HMC7044_VCO_LOW) |
HMC7044_SYSREF_TIMER_EN | HMC7044_PLL2_EN |
HMC7044_PLL1_EN);
if (ret)
return ret;
}

if (hmc->pll2_cap_bank_sel != ~0) {
Expand Down Expand Up @@ -1185,7 +1210,7 @@
if (ret)
return ret;
/* Set the lock detect timer threshold */
ret = hmc7044_write(indio_dev, HMC7044_REG_PLL1_LOCK_DETECT,

Check warning on line 1213 in drivers/iio/frequency/hmc7044.c

View workflow job for this annotation

GitHub Actions / build_llvm_x86_64 / build 

clang_analyzer: Value stored to 'ret' is never read [deadcode.DeadStores]
1213 | ret = hmc7044_write(indio_dev, HMC7044_REG_PLL1_LOCK_DETECT,
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1214 | HMC7044_LOCK_DETECT_TIMER(pll1_lock_detect));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
HMC7044_LOCK_DETECT_TIMER(pll1_lock_detect));

/* Set the LCM */
Expand Down Expand Up @@ -1354,17 +1379,11 @@
if (ret)
return ret;

if (!hmc->clkin1_vcoin_en) {
u32 pll1_stat;

ret = hmc7044_read(indio_dev, HMC7044_REG_PLL1_STATUS, &pll1_stat);
if (ret < 0)
return ret;
ret = hmc7044_read(indio_dev, HMC7044_REG_PLL1_STATUS, &pll1_stat);
if (ret < 0)
return ret;

c = HMC7044_PLL1_ACTIVE_CLKIN(pll1_stat);
} else {
c = 1; /* CLKIN1 */
}
c = HMC7044_PLL1_ACTIVE_CLKIN(pll1_stat);

for (i = 0; i < hmc->num_channels; i++) {
chan = &hmc->channels[i];
Expand Down Expand Up @@ -1663,7 +1682,7 @@
return ret;

hmc->pll2_cap_bank_sel = ~0;
ret = of_property_read_u32(np, "adi,pll2-autocal-bypass-manual-cap-bank-sel",

Check warning on line 1685 in drivers/iio/frequency/hmc7044.c

View workflow job for this annotation

GitHub Actions / build_llvm_x86_64 / build 

clang_analyzer: Value stored to 'ret' is never read [deadcode.DeadStores]
1685 | ret = of_property_read_u32(np, "adi,pll2-autocal-bypass-manual-cap-bank-sel",
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1686 | &hmc->pll2_cap_bank_sel);
| ~~~~~~~~~~~~~~~~~~~~~~~~
&hmc->pll2_cap_bank_sel);

ret = of_property_read_u32_array(np, "adi,gpi-controls",
Expand Down Expand Up @@ -1697,6 +1716,11 @@

hmc->clkin1_vcoin_en =
of_property_read_bool(np, "adi,clkin1-vco-in-enable");
hmc->pll2_pfd_invert_en =
of_property_read_bool(np, "adi,pll2-pfd-invert-enable");

if (hmc->pll2_pfd_invert_en && !hmc->clkin1_vcoin_en)
dev_warn(dev, "PLL2 PFD inverted using internal VCO.\n");
Copy link
Collaborator

@nunojsa nunojsa Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure. Is this something allowable or should we just return error here?


hmc->ignore_vco_limits = of_property_read_bool(np, "adi,ignore-vco-limits");
hmc->sync_through_pll2_force_r2_eq_1 =
Expand Down Expand Up @@ -1918,10 +1942,7 @@
if (ret < 0)
return ret;

if (hmc->clkin1_vcoin_en)
active = 1;
else
active = HMC7044_PLL1_ACTIVE_CLKIN(pll1_stat);
active = HMC7044_PLL1_ACTIVE_CLKIN(pll1_stat);

if (hmc->clkin_freq_ccf[active])
clkin_freq = hmc->clkin_freq_ccf[active];
Expand Down Expand Up @@ -2021,7 +2042,7 @@
return 0;
}

rem = hmc7044_get_rem(hmc->pll2_freq, divisor);

Check warning on line 2045 in drivers/iio/frequency/hmc7044.c

View workflow job for this annotation

GitHub Actions / build_llvm_x86_64 / build 

clang_analyzer: Value stored to 'rem' is never read [deadcode.DeadStores]
2045 | rem = hmc7044_get_rem(hmc->pll2_freq, divisor);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

dev_dbg(&hmc->spi->dev,
"%s: dividend=%llu divisor=%llu GCD=%llu rem=%llu (hmc->pll2_freq=%u)",
Expand Down Expand Up @@ -2139,7 +2160,7 @@
return ret;
}
} else {
if (hmc->device_id == HMC7044 && !hmc->clkin0_rfsync_en && !hmc->clkin1_vcoin_en) {
if (hmc->device_id == HMC7044 && !hmc->clkin0_rfsync_en) {
ret = hmc7044_sync_pin_set(indio_dev, HMC7044_SYNC_PIN_SYNC);
if (ret)
return ret;
Expand All @@ -2151,8 +2172,7 @@
}

ret = hmc7044_toggle_bit(indio_dev, HMC7044_REG_REQ_MODE_0,
HMC7044_RESTART_DIV_FSM, (hmc->device_id == HMC7044 &&
!hmc->clkin1_vcoin_en) ? 10000 : 1000);
HMC7044_RESTART_DIV_FSM, (hmc->device_id == HMC7044) ? 10000 : 1000);
if (ret)
return ret;

Expand Down Expand Up @@ -2237,7 +2257,7 @@
__func__, val & 0xFF);
}

if (hmc->device_id == HMC7044 && !hmc->clkin0_rfsync_en && !hmc->clkin1_vcoin_en) {
if (hmc->device_id == HMC7044 && !hmc->clkin0_rfsync_en) {
ret = hmc7044_sync_pin_set(indio_dev, HMC7044_SYNC_PIN_PULSE_GEN_REQ);
if (ret)
return ret;
Expand Down
Loading